[jira] [Assigned] (IGNITE-14645) Support polymorphic configuration nodes.

["Kirill Tkalenko (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IGNITE-14645?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kirill Tkalenko reassigned IGNITE-14645:
----------------------------------------

    Assignee: Kirill Tkalenko  (was: Ivan Bessonov)

> Support polymorphic configuration nodes.
> ----------------------------------------
>
>                 Key: IGNITE-14645
>                 URL: https://issues.apache.org/jira/browse/IGNITE-14645
>             Project: Ignite
>          Issue Type: Improvement
>            Reporter: Ivan Bessonov
>            Assignee: Kirill Tkalenko
>            Priority: Major
>              Labels: iep-55, ignite-3
>
> NOTE: description might not be finished.
> h3. 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.
> h3. Interface
> For the solution we need to expand abilities of configuration schemas. I propose the following option:
> {code:java}
> // 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;
> }
> {code}
> h3. 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:
> {code:java}
> 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();{code}
> h3. Implementation Notes
> There are quite a few places that need to be tweaked here. First of all, let's examine {{ConfigurationImpl}} classes:
>  
> {code:java}
> // 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.
>     }
> }{code}
> 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. 
> {code:java}
> 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);
>         }
>     }
> }{code}
>  Index node. All types of names collisions are allowed.
> {code:java}
> // 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.
>     }
> }{code}
>  
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)