Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I always get confused myself with the new changes (especially read-only vs. status), so I thought it would probably be good to summarize all the index invariants regarding state changes in one place, ensure they hold and also add tests to ensure we don't break this in the future.
As said, I get confused myself, so please correct me if we have decided on something different for one of these:
- If an index is enabled, it must always have a server, which must also be enabled.
If an index has a server set, the server must be enabled.This was true for D7, but we don't want it anymore in D8, do I remember correctly? (Why?)- If an index is enabled (and only then), its new/changed/deleted items are tracked (regardless of read-only). (Do I remember this right?)
- When an index is removed from a server (either to another one, or to none, or it is deleted), the server's
removeIndex()method is called.¹ - When an index is added to a server (either from another one, or none, or it is newly inserted), the server's
addIndex()method is called.¹ - When an index is updated without its server changing (and a server set), the server's
updateIndex()method is called.¹
¹ Should these always hold, or only if the index is enabled? (In my opinion: always. The other option is to remove the index from the server (at least internally, with Server::removeIndex()) temporarily when disabling it (the index), and later re-adding it when it is re-enabled.)
All of these should be checked when an index is saved (pre/post, depending on what makes sense) or deleted (where it makes sense), but they could be broken if, e.g., the server becomes invalid for some reason. If any of these are violated at some other point (e.g., in getServer()), this would be a reason for throwing an exception.
Also, some we should discuss:
- Should it be possible to change an index's datasources after it is created? If yes, that's also something we have to react to. (Probably yes, to enable the "one index per site" basic case we want to reach.)
- Same question for the tracker – yes or no?
Any others?
Comments
Comment #1
freblasty CreditAttribution: freblasty commentedCurrently the default tracker requires a writable index to track changes. Don't know whether this might have been discussed during szeged.
Comment #2
drunken monkeyJust discussed this with Nick, we came up with the following:
removeIndex()method is called when an index attached to a server is disabled (and when it gets moved to another server, or "no server", or deleted). Likewise,addIndex()is called when an enabled index gets added to the server, or when an index on the server gets enabled.Before I update the summary, does anyone want to add something to this, or object to some parts?
Then, we'd still need to make sure these invariants are always properly checked, and we'd need to add tests checking them. Probably, these should also be added somewhere in the documentation – maybe in the documentation of the index class properties.
(Also, while implementing this, it might be a good moment to move the
startTracking()andstopTracking()methods from the datasource to the index class. These really shouldn't be datasource-specific at all. OnlygetEntityIds()would then be needed, probably renamed togetAllItemIds()or something.)Comment #3
Nick_vhStarted working on this. Will probably start with tests first
Comment #4
Nick_vhComment #5
Nick_vhCommitted a whole lot more
The only condition left is to test if it reacts correctly when the index changes server.
Comment #6
Nick_vhComment #7
Nick_vhRead-only shouldn't influence tracking at all, just whether indexing items works. For not tracking any items, we could add a new tracker implementation, "Don't track", that just does nothing in its methods.Disabled indexes are internally treated as not lying on any server. I.e., the server's removeIndex() method is called when an index attached to a server is disabled and when it gets moved to another server, or "no server", or deleted.reindex() is called when an index switches serversaddIndex() is called when an enabled index gets added to the server, or when an index on the server gets enabled.Changing datasources should also be possible. This will delete the items from the removed datasources from the index and add the new items to the trackerChanging tracker should also be possible. This will delete the items from the old tracker and add the new items to the new tracker. This will not delete the indexAll done, all tests passing, bunch of tracking tests added and reworked the postSave method for the index entity. See commit : http://drupalcode.org/sandbox/daeron/2091893.git/commit/0b32845