fix: multi plugin id was read from the wrong place

[henryiii]

Found when working on henryiii/validate-pyproject-schema-store#117. Looks like a unit testing bug. There might be an issue with getting the ordering wrong too (it's reporting all the tools are coming from the old location, which won't allow extra schemas to load). Looking into that, might be related though.

[henryiii]

I'm not sure this was all that important, it just causes the verbose printout to be less helpful. I'm not sure the verbose printout is all that helpful with this, though:

So maybe we can store the plugin name it came from too and use that.

Haven't worked out why the ordering is wrong yet, though. It seems like we have a test for it.

[abravalheri]

So maybe we can store the plugin name it came from too and use that.

You mean for the sotred_plugin.id?

If that is the idea, maybe something like this?

class StoredPlugin:
    def __init__(self, tool: str, schema: Schema, fallback_id: str): ...

    @property
    def id(self) -> str:
        return self.schema.get("$id", self._fallback_id)

...

def load_from_multi_entry_point(...):
    ...
    base_id = entry_point.name
    for tool, schema in output["tools"].items():
        yield StoredPlugin(tool, schema, f"{base_id}:{tool}")
    for i, schema in enumerate(output.get("schemas", [])):
        yield StoredPlugin("", schema, f"{base_id}:{i}")

Or use base_id = f"{fn.__module__}.{fn.__name__}"

(Untested code)

Is that more or less what you had in mind in this suggestion?

[henryiii]

Yes, that's roughly what I was thinking. I think we use name or id in a few places, need to make sure none of those assume id is unique. If id is $id, then it is unique. Looks like your first suggestion would handle that, though. 👍

I was able to reproduce the sorting bug.

[abravalheri]

I suppose that if the schema is not a tool, it is also reasonable to require $id to be present? (Otherwise how is the schema referenced?).

[abravalheri]

Thank you very much.

Haven't worked out why the ordering is wrong yet, though. It seems like we have a test for it.

I suppose we can handle it separately.