Voting starts in March for the Drupal Association Board election.
Typed config's Mapping and Sequence implement ComplexDataInterface and ListInterface. Both implementations are wrong, have a lot of bugs/issues and still worse, none of the interfaces fits 100% these classes intended behavior. Some of the issues:
- Sequence *cannot* implement ListInterface::getItemDefinition() as we simply don't have a unique definition for all items
- Sequence keys may not be numeric if we pretend it to fit any configuration sequence. For this reason and the previous one doesn't fit at all ListInterface
- None of these interfaces can fit some use cases in configuration data, like the ones explained in #2 and #3 (Variable list of items keyed by strings or without explicit keys , like for the case of plugin lists)
- Since the list of items (for both Mapping and Sequence) is built dinamically and we need to parse the actual configuration data for it, they need additional methods on top of those of generic typed data.
- Some parameters of these interfaces like ComplexDataInterface::getProperties($include_computed = FALSE) are meaningless for this usage of them
- Other methods implemented by Mapping and Sequence like get() can take nested keys that while handy for configuration, don't fit the interface definition
Also they are poorly documented and documentation is not up to latest coding standards.
All this is due to a number of reasons, from a poor original implementation to interfaces evolving later on and classes not being properly updated for it -sometimes a method was just copied over, sometimes it was just not possible to implement new methods properly-. But mostly because ComplexDataInterface and ListInterface are more targetted at Entities/Fields/FieldItems that doesn't really fit their usage in Typed Config.
Add a new interface that really fits typed configuration and can be cleanly implemented.
Simplify Mapping and Sequence from the unneeded complexity inherited from trying to reuse/implement these interfaces.
Further simplification, moving common code of Sequence/Mapping elements one level up to ArrayElement.
Add proper documentation for all these classes and interfaces and explain existing classes.
Review and commit.
Further simplification is still possible, though we are trying to keep the patch as small as possible.
User interface changes
Some, but all of them internal to the configuration system. Changes in core modules or other components are really small -a few parameter types- since most of these methods were never used, so only tests need to be updated.
These API changes are:
- New TypedConfigInterface that is used by both Mapping and Sequence instead of ComplexDataInterface/ListInterface
- Mapping and Sequence are not implementing anymore ArrayAccess interface, that was only used in texts anyway
- TypedConfigManager::get() returns TypedConfigInterface instead of Element (that was an abstract parent class)
Original report by @chx
- The class doxygen is useless. Is it a list of items? Can the keys be strings? What are some examples? When should I use something else for an array
getItemDefinitioncontains a fatal error. No, seriously, look at it:
buildDataDefinitionis called with two arguments, it requires three. What does this say about test coverage? Do we need this method if it's not called anywhere, code or test?
- All the method doxygen are the old style implements instead of inheritdoc.
onChangeis looking for
parentwhich is not a defined property nor is it clear how would it be set.
This is the diagram of how classes interfaces look before and after this patch.