Uploaded image for project: 'Ignite'
  1. Ignite
  2. IGNITE-14645

Support polymorphic configuration nodes.

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • None
    • 3.0.0-alpha4
    • 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.

      1. How do you properly configure IpFinder? For each type of finder you only need a few properties.
      2. 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

          Activity

            People

              ktkalenko@gridgain.com Kirill Tkalenko
              ibessonov Ivan Bessonov
              Ivan Bessonov Ivan Bessonov
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 2h 50m
                  2h 50m