| daniel added a comment. |
Yes, as Leszek said: hooks should be avoided whenever possible. They are easy to introduce, but they rely on global state. Because of that, they hide information flow, and make testing hard.
So, EditEntity::getChangeOps should use a callback defined in the entity type definition to construct ChangeOps from JSON. The current hard-coded logic in EditEntity should be moved accordingly. I don't think all of that should go into a callback, we'll probably want something like a ChangeOpDeserializer interface (think of the JSON as serialized ChangeOps). The callback would return a list ChangeOpDeserializer appropriate for a given entity type. Ideally, we wouldn't inject an untyped callback, but we'd have a ChangeOpDeserializerFactory interface... but that may be overkill.
The above model is just off the top of my head. There may be better ways. There is already a ChangeOpFactoryProvider and several XyzChangeOpFactory classes that will have to fit into this somehow. The important factors are:
- EditEntity should not know how to construct ChangeOps
- the entity type definition array should provide a callback that somehow provides the knowledge of how to construct a list of ChangeOps from the JSON input to EditEntity.
- that callback (or something that wraps it, or something that was constructed by it) needs to be injected into EditEntity.
Note that EditEntity currently uses pseudo-injection by accessing global state in the constructor, and it does not provide an overrideServices method. Changing this to using proper injection would also be nice. To achieve this, the API module can be registered using a callback instead of a class name.
When working on this, please file the appropriate tickets first.
Cc: Jakob_WMDE, WMDE-leszek, Aklapper, Addshore, Ladsgroup, daniel, D3r1ck01, Izno, Wikidata-bugs, aude, Mbch331
_______________________________________________ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
