Page MenuHomePhabricator

Move ID assignment out of EntityStore
Open, Needs TriagePublic


Currently, EntityStore allows entities with no ID set to be passed to saveEntity(). A fresh ID will then be assigned before saving.

In practice, this feature is only used for setting up test fixtures. In production code, the caller controls what ID is assigned and when. This seems more sensible, since if and how IDs can be assigned depends on the entity type, and perhaps also on the context / use case.

The following steps are needed to improve the situation:

  • saveEntity should require the ID to be set. This means a lot of test cases need fixing.
  • assignFreshId should be removed from the EntityStore interface. It seems like we would currently need to repeat the old logic in the following places: MockEntityStore, ModifyEntity, SpecialNewEntity, and perhaps EntitySavingHelper (once EntitySavingHelper can also create entities).
  • IdGenerator should take entity types, not content model IDs, as a parameter. The IdGenerator interface does not specify what "$type" is, but we can't change the interpretation on a live system without migrating the counters in the database table used to track ID creation. (Changing this is not strictly needed, but would allow us to use IdGenerator directly in the places that need to assign ids).