Details
-
Improvement
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
-
None
Description
NOTE: description might not be finished.
Problem
Currently configuration schema structure is very restricted and doesn't allow any variations in it. This approach comes with a set of problems.
- How do you properly configure IpFinder? For each type of finder you only need a few properties.
- How do you configure SQL indexes? Pretty much the same problem.
Interface
For the solution we need to expand abilities of configuration schemas. I propose the following option:
// Configuration schema that contains polymorphic field. @Config class TableConfigurationSchema { @NamedConfigValue public IndexConfigurationSchema indexes; } // Base class for polymorphic value. Explicitly has all subclasses // in its description to simplify incremental code generation. //TODO: Maybe we cab avoid this part by using "originating elements" in annotation processing, // we'll check that during POC phase. @PolymorphicConfig(impl = { HashIndexConfigurationSchema.class, TreeIndexConfigurationSchema.class, }) class IndexConfigurationSchema { // This annotation shows that current field defines implementation. // Specific values are present in implementations declarations. @Id @Immutable @Value public String type; } // One of implementations for index. Id value is defined in annotation. @PolymorphicInstance(id = "hash") public static class HashIndexConfigurationSchema extends IndexConfigurationSchema { @Immutable @Value public String column; } // Other implementation for index. @PolymorphicInstance(id = "tree") public static class TreeIndexConfigurationSchema extends IndexConfigurationSchema { @Immutable @Value public String[] columns; }
Generated API
We need to tweak API a little bit. I'd love to see as few changes as possible, so my vision is something like this:
TableConfiguration tableCfg = ...; tableCfg.indexes().create("hashIndexName", index -> // Id sets up automatically by the call. index.asHashIndex().changeColumn("columnName") ).get(); tableCfg.indexes().update("hashIndexName", index -> // Any cast is allowed to do in change request. // But this update will fail during processing. index.asTreeIndex().changeColumns("a", "b") ); IndexConfiguration indexCfg = tableCfg.indexes().get("hashIndexName"); // This must be an instance of "HashIndexConfiguration". HashIndexConfiguration hashCfg = (HashIndexConfiguration)indexCfg; // This must be instance of HashIndexView, IndexView indexView = indexCfg.value(); // Maybe this is redundant, I don't know. assert indexView.isHashIndex(); // Returns the same object with a safe cast. // Maybe this is redundant as well and regular cast would be enough. HashIndexView hashView = indexView.asHashIndex();
Implementation Notes
There are quite a few places that need to be tweaked here. First of all, let's examine ConfigurationImpl classes:
// Let's assume for a moment that "index" is not a named list, this is more convenient for current example. final class TableConfigurationImpl extends DynamicConfiguration<TableView, TableChange> implements TableConfiguration { // Polymorphic fields won't be final anymore. private IndexConfigurationImpl index; public TableConfigurationImpl(List<String> prefix, String key, RootKey<?, ?> rootKey, ConfigurationChanger changer) { super(prefix, key, rootKey, changer); // No more "add" method invocations. "members" becomes mutable collections. // There's no point in making specific code generation for the situation when it's static. } @Override public final IndexConfiguration index() { // It's important to note that we can't just read field content because its type might have been changed. // This scenario was impossible before polymorphic types. refreshValue(); return index; } @Override protected final synchronized void beforeRefreshValue(IndexView newValue) { // New value must be reinstantiated if its type changed. } @Override public Map<String, ConfigurationProperty<?, ?>> members() { refreshValue(); // Return map computed from current fields OR throw this method away completely and replace it // with proper "getMember". BTW, NamedListConfiguration would need "getAllNames" method or something close to it. // So inner nodes and named list nodes will probably only use one of these, we might put different methods // into different classes. } }
It's important to note that refreshValue method will work differently. You can't just use find because it doesn't know anything about polymorphism. We will need a new specific find for this case that will check every polymorphic type in chain. It's pretty easy to implement tho.
Now nodes. It's a bit more complicated.
final class TableNode extends InnerNode implements TableView, TableChange { private final TableConfigurationSchema _spec = new ...; private IndexNode index; // This one is the same. @Override public IndexNode changeIndex(Consumer<IndexChange> indexConsumer) { ... } // But this one is not. @Override public IndexView index() { // We need to downcast view object here. return index.specificView(); } // Traverse methods remain intact. @Override public void construct(String key, ConfigurationSource src) throws NoSuchElementException { switch (key) { case "index": if (src == null) child = null; // Creating new node. else if (child == null) { switch (src.readMandatoryProperty("type")) { case "hash": src.descend(index = new IndexNode("hash")); break; case "tree": src.descend(index = new IndexNode("tree")); break; default: throw new ...; } } // Updating existing node. else { String id = src.readMandatoryProperty("type"); if (id.equals(index.type())) src.descend(index = (IndexNode)index.copy()); else { // Copy-paste. This can probably be compacted, I just don't want to mess with code // complexity in example. switch (id) { case "hash": src.descend(index = new IndexNode("hash")); // Or copy+setType? break; case "tree": src.descend(index = new IndexNode("tree")); break; default: throw new ...; } } } break; default: throw new NoSuchElementException(key); } } }
Index node. All types of names collisions are allowed.
// This is already different enough. We have no "View" interface and extended "Change" interface. // Second one will contain methods like "asHashIndex". final class IndexNode extends InnerNode implements IndexChangeEx { // Specs are lazy now. private HashIndexConfigurationSchema _spec$HashIndex; private TreeIndexConfigurationSchema _spec$TreeIndex; // Inlined fields. private String type; private String column$HashIndex; private String[] columns$TreeIndex; // Constructors... // New methods: public IndexView specificView() { switch (type) { case "hash": return this.new HashIndexNode(); // Inner class, described later. case "tree": return this.new TreeIndexNode(); default: throw new ...; } } // This methods is required to transform Ex change to a specific one. @Override public HashIndexChange asHashIndex() { if (type == null) type = "hash"; else if (!"hash".equals(type)) throw new ...; return this.new HashNode(); } @Override public <T> void traverseChildren(ConfigurationVisitor<T> visitor) { visitor.visitInnerNode("type", type); switch (type) { case "hash": visitor.visitLeafNode("column", column$HashIndex); break; case "tree": visitor.visitLeafNode("columns", columns$TreeIndex); break; } } @Override public void construct(String key, ConfigurationSource src) throws NoSuchElementException { switch (key) { case "type": assert src != null; index = src.unwrap(String.class); // Validation - switch by all known types. break; } switch (type) { case "hash": switch (key) { case "column": column$HashIndex = src == null ? null : src.unwrap(String.class); break; default: throw new NoSuchElementException(key); } break; case "tree": ... break; default: throw new ...; } } @Override public Class<?> schemaType() { switch (type) { case "hash": // I know that we could just return class. Such code is required right now so that spec fields // won't be unused during compilation. Bad reasoning, maybe we'll fix it. if (_spec$HashIndex == null) _spec$HashIndex = new HashIndexConfigurationSchema(); return _spec$HashIndex.getClass(); ... } } // Same for constructDefault, you get the idea. // Finally, private inner classes for specific change/view private class HashIndexNode implements HashIndexView, HashIndexChange { // And those implement IndexView and IndexChange. // All view and change methods will delegate to the fields of IndexNode, // validating "type" field at the same time. // No traversing / constructing here, everything's simple. } }
Attachments
Issue Links
- is related to
-
IGNITE-15853 Add api description of polymorphic configuration to modules/configuration/README.md
- Resolved
- links to