You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2021/02/19 08:55:17 UTC

[ignite-3] branch main updated: IGNITE-14181 Support arrays of primitives in the configuration schema (#54)

This is an automated email from the ASF dual-hosted git repository.

ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new db91bfb  IGNITE-14181 Support arrays of primitives in the configuration schema (#54)
db91bfb is described below

commit db91bfb420f098c3c245261327e2ef80e042fba1
Author: Semyon Danilov <sa...@yandex.ru>
AuthorDate: Fri Feb 19 11:55:09 2021 +0300

    IGNITE-14181 Support arrays of primitives in the configuration schema (#54)
---
 .../processor/internal/ITProcessorTest.java        |  12 ++-
 .../configuration/processor/internal/Types.java    |   3 +
 .../internal/TestConfigurationSchema.java          |   3 +
 .../processor/internal/Processor.java              |  87 ++++++++++++-----
 .../sample/ConfigurationArrayTest.java             | 105 +++++++++++++++++++++
 .../ignite/configuration/ConfigurationChanger.java |  75 ++-------------
 .../internal/util/ConfigurationUtil.java           |  63 +++++++++++++
 7 files changed, 256 insertions(+), 92 deletions(-)

diff --git a/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/ITProcessorTest.java b/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/ITProcessorTest.java
index 5dfa487..dce8d54 100644
--- a/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/ITProcessorTest.java
+++ b/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/ITProcessorTest.java
@@ -57,7 +57,8 @@ public class ITProcessorTest extends AbstractProcessorTest {
             hasFields(
                 "value1", Types.STRING,
                 "primitiveLong", Types.LONG,
-                "primitiveInt", Types.INT
+                "primitiveInt", Types.INT,
+                "stringArray", Types.STRING_ARRAY
             )
         );
 
@@ -66,7 +67,8 @@ public class ITProcessorTest extends AbstractProcessorTest {
             hasMethods(
                 "value1()", Types.STRING,
                 "primitiveLong()", Types.LONG,
-                "primitiveInt()", Types.INT
+                "primitiveInt()", Types.INT,
+                "stringArray()", Types.STRING_ARRAY
             )
         );
 
@@ -75,7 +77,8 @@ public class ITProcessorTest extends AbstractProcessorTest {
             hasFields(
                 "value1", Types.STRING,
                 "primitiveLong", Types.LONG,
-                "primitiveInt", Types.INT
+                "primitiveInt", Types.INT,
+                "stringArray", Types.STRING_ARRAY
             )
         );
 
@@ -89,7 +92,8 @@ public class ITProcessorTest extends AbstractProcessorTest {
                 "primitiveInt()", Types.INT,
                 "withValue1(java.lang.String)", initTypeName,
                 "withPrimitiveLong(java.lang.Long)", initTypeName,
-                "withPrimitiveInt(java.lang.Integer)", initTypeName
+                "withPrimitiveInt(java.lang.Integer)", initTypeName,
+                "withStringArray(java.lang.String[])", initTypeName
             )
         );
     }
diff --git a/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/Types.java b/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/Types.java
index 150d6c0..108731e 100644
--- a/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/Types.java
+++ b/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/configuration/processor/internal/Types.java
@@ -33,6 +33,9 @@ public class Types {
     /** String. */
     public static final String STRING = PKG_JAVA_LANG + "String";
 
+    /** String array. */
+    public static final String STRING_ARRAY = PKG_JAVA_LANG + "String[]";
+
     /** Double. */
     public static final String DOUBLE = PKG_JAVA_LANG + "Double";
 
diff --git a/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/configuration/processor/internal/TestConfigurationSchema.java b/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/configuration/processor/internal/TestConfigurationSchema.java
index 7847c4f..90fe957 100644
--- a/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/configuration/processor/internal/TestConfigurationSchema.java
+++ b/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/configuration/processor/internal/TestConfigurationSchema.java
@@ -30,4 +30,7 @@ public class TestConfigurationSchema {
 
     @Value
     private int primitiveInt;
+
+    @Value
+    private String[] stringArray;
 }
diff --git a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java
index fe46cd9..ef1797e 100644
--- a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java
+++ b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java
@@ -18,6 +18,7 @@
 package org.apache.ignite.configuration.processor.internal;
 
 import com.squareup.javapoet.AnnotationSpec;
+import com.squareup.javapoet.ArrayTypeName;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.CodeBlock;
 import com.squareup.javapoet.FieldSpec;
@@ -280,19 +281,13 @@ public class Processor extends AbstractProcessor {
 
                 final Value valueAnnotation = field.getAnnotation(Value.class);
                 if (valueAnnotation != null) {
-                    switch (baseType.toString()) {
-                        case "boolean":
-                        case "int":
-                        case "long":
-                        case "double":
-                        case "java.lang.String":
-                            break;
-
-                        default:
-                            throw new ProcessorException(
-                                "@Value " + clazz.getQualifiedName() + "." + field.getSimpleName() + " field must" +
-                                    " have one of the following types: boolean, int, long, double, String."
-                            );
+                    // Must be a primitive or an array of the primitives (including java.lang.String)
+                    if (!isPrimitiveOrArrayOfPrimitives(baseType)) {
+                        throw new ProcessorException(
+                            "@Value " + clazz.getQualifiedName() + "." + field.getSimpleName() + " field must" +
+                                " have one of the following types: boolean, int, long, double, String or an array of " +
+                                "aforementioned type."
+                        );
                     }
 
                     // Create value (DynamicProperty<>) field
@@ -901,25 +896,29 @@ public class Processor extends AbstractProcessor {
             String fieldName = field.getSimpleName().toString();
             TypeName schemaFieldType = TypeName.get(field.asType());
 
-            boolean leafField = schemaFieldType.isPrimitive() || !((ClassName)schemaFieldType).simpleName().contains("ConfigurationSchema");
+            boolean isArray = schemaFieldType instanceof ArrayTypeName;
+
+            boolean leafField = isPrimitiveOrArrayOfPrimitives(schemaFieldType)
+                || !((ClassName)schemaFieldType).simpleName().contains("ConfigurationSchema");
+
             boolean namedListField = field.getAnnotation(NamedConfigValue.class) != null;
 
-            TypeName viewFieldType = schemaFieldType.isPrimitive() ? schemaFieldType : ClassName.get(
+            TypeName viewFieldType = leafField ? schemaFieldType : ClassName.get(
                 ((ClassName)schemaFieldType).packageName(),
                 ((ClassName)schemaFieldType).simpleName().replace("ConfigurationSchema", "View")
             );
 
-            TypeName changeFieldType = schemaFieldType.isPrimitive() ? schemaFieldType : ClassName.get(
+            TypeName changeFieldType = leafField ? schemaFieldType : ClassName.get(
                 ((ClassName)schemaFieldType).packageName(),
                 ((ClassName)schemaFieldType).simpleName().replace("ConfigurationSchema", "Change")
             );
 
-            TypeName initFieldType = schemaFieldType.isPrimitive() ? schemaFieldType : ClassName.get(
+            TypeName initFieldType = leafField ? schemaFieldType : ClassName.get(
                 ((ClassName)schemaFieldType).packageName(),
                 ((ClassName)schemaFieldType).simpleName().replace("ConfigurationSchema", "Init")
             );
 
-            TypeName nodeFieldType = schemaFieldType.isPrimitive() ? schemaFieldType.box() : ClassName.get(
+            TypeName nodeFieldType = leafField ? schemaFieldType.box() : ClassName.get(
                 ((ClassName)schemaFieldType).packageName() + (leafField ? "" : ".impl"),
                 ((ClassName)schemaFieldType).simpleName().replace("ConfigurationSchema", "Node")
             );
@@ -955,11 +954,18 @@ public class Processor extends AbstractProcessor {
                 }
 
                 {
+                    CodeBlock getStatement;
+
+                    if (isArray)
+                        getStatement = CodeBlock.builder().add("return $L.clone()", fieldName).build();
+                    else
+                        getStatement = CodeBlock.builder().add("return $L", fieldName).build();
+
                     MethodSpec.Builder nodeGetMtdBuilder = MethodSpec.methodBuilder(fieldName)
                         .addAnnotation(Override.class)
                         .addModifiers(PUBLIC)
                         .returns(leafField ? viewFieldType : nodeFieldType)
-                        .addStatement("return $L", fieldName);
+                        .addStatement(getStatement);
 
                     nodeClsBuilder.addMethod(nodeGetMtdBuilder.build());
                 }
@@ -988,9 +994,16 @@ public class Processor extends AbstractProcessor {
                         .returns(nodeClsName);
 
                     if (valAnnotation != null) {
+                        CodeBlock changeStatement;
+
+                        if (isArray)
+                            changeStatement = CodeBlock.builder().add("this.$L = $L.clone()", fieldName, fieldName).build();
+                        else
+                            changeStatement = CodeBlock.builder().add("this.$L = $L", fieldName, fieldName).build();
+
                         nodeChangeMtdBuilder
                             .addParameter(changeFieldType, fieldName)
-                            .addStatement("this.$L = $L", fieldName, fieldName);
+                            .addStatement(changeStatement);
                     }
                     else {
                         String paramName = fieldName + "Consumer";
@@ -1046,9 +1059,16 @@ public class Processor extends AbstractProcessor {
                         .returns(nodeClsName);
 
                     if (valAnnotation != null) {
+                        CodeBlock initStatement;
+
+                        if (isArray)
+                            initStatement = CodeBlock.builder().add("this.$L = $L.clone()", fieldName, fieldName).build();
+                        else
+                            initStatement = CodeBlock.builder().add("this.$L = $L", fieldName, fieldName).build();
+
                         nodeInitMtdBuilder
                             .addParameter(initFieldType, fieldName)
-                            .addStatement("this.$L = $L", fieldName, fieldName);
+                            .addStatement(initStatement);
                     }
                     else {
                         String paramName = fieldName + "Consumer";
@@ -1339,6 +1359,31 @@ public class Processor extends AbstractProcessor {
             .build();
     }
 
+    /**
+     * Checks whether TypeName is a primitive (or String) or an array of primitives (or Strings)
+     * @param typeName TypeName.
+     * @return {@code true} if type is primitive or array.
+     */
+    private boolean isPrimitiveOrArrayOfPrimitives(TypeName typeName) {
+        String type = typeName.toString();
+
+        if (typeName instanceof ArrayTypeName)
+            type = ((ArrayTypeName) typeName).componentType.toString();
+
+        switch (type) {
+            case "boolean":
+            case "int":
+            case "long":
+            case "double":
+            case "java.lang.String":
+                return true;
+
+            default:
+                return false;
+
+        }
+    }
+
     /** {@inheritDoc} */
     @Override public Set<String> getSupportedAnnotationTypes() {
         return Set.of(Config.class.getCanonicalName(), ConfigurationRoot.class.getCanonicalName());
diff --git a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/ConfigurationArrayTest.java b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/ConfigurationArrayTest.java
new file mode 100644
index 0000000..a565653
--- /dev/null
+++ b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/ConfigurationArrayTest.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.configuration.sample;
+
+import java.io.Serializable;
+import java.util.function.Supplier;
+import org.apache.ignite.configuration.annotation.Config;
+import org.apache.ignite.configuration.annotation.Value;
+import org.apache.ignite.configuration.sample.impl.TestArrayNode;
+import org.apache.ignite.configuration.tree.ConfigurationVisitor;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+
+/**
+ * Test configuration with array of primitive type (or {@link String}) fields.
+ */
+public class ConfigurationArrayTest {
+    /** */
+    @Config
+    public static class TestArrayConfigurationSchema {
+        /** */
+        @Value
+        private String[] array;
+    }
+
+    /**
+     * Test array node init operation.
+     */
+    @Test
+    public void testInit() {
+        var arrayNode = new TestArrayNode();
+
+        Supplier<String[]> initialSupplier = () -> {
+            return new String[]{"test1", "test2"};
+        };
+
+        final String[] initialValue = initialSupplier.get();
+        arrayNode.initArray(initialValue);
+
+        // test that field is not the same as initialValue
+        assertNotSame(getArray(arrayNode), initialValue);
+
+        // test that init method set values successfully
+        assertThat(arrayNode.array(), is(initialSupplier.get()));
+
+        // test that returned array is a copy of the field
+        assertNotSame(getArray(arrayNode), arrayNode.array());
+    }
+
+    /**
+     * Test array node change operation.
+     */
+    @Test
+    public void testChange() {
+        var arrayNode = new TestArrayNode();
+
+        Supplier<String[]> changeSupplier = () -> {
+            return new String[]{"foo", "bar"};
+        };
+
+        final String[] changeValue = changeSupplier.get();
+        arrayNode.changeArray(changeValue);
+
+        // test that field is not the same as initialValue
+        assertNotSame(getArray(arrayNode), changeValue);
+
+        // test that change method set values successfully
+        assertThat(arrayNode.array(), is(changeSupplier.get()));
+
+        // test that returned array is a copy of the field
+        assertNotSame(getArray(arrayNode), arrayNode.array());
+    }
+
+    /**
+     * Get array field from ArrayNode
+     * @param arrayNode ArrayNode.
+     * @return Array field value.
+     */
+    private String[] getArray(TestArrayNode arrayNode) {
+        return arrayNode.traverseChild("array", new ConfigurationVisitor<String[]>() {
+            /** {@inheritDoc} */
+            @Override public String[] visitLeafNode(String key, Serializable val) {
+                return (String[]) val;
+            }
+        });
+    }
+}
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
index d97d7c1..b840246 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
@@ -25,17 +25,15 @@ import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
-import org.apache.ignite.configuration.internal.util.ConfigurationUtil;
 import org.apache.ignite.configuration.storage.ConfigurationStorage;
 import org.apache.ignite.configuration.storage.Data;
 import org.apache.ignite.configuration.storage.StorageException;
-import org.apache.ignite.configuration.tree.ConfigurationVisitor;
-import org.apache.ignite.configuration.tree.InnerNode;
-import org.apache.ignite.configuration.tree.NamedListNode;
 import org.apache.ignite.configuration.tree.TraversableTreeNode;
 import org.apache.ignite.configuration.validation.ConfigurationValidationException;
 import org.apache.ignite.configuration.validation.ValidationIssue;
 
+import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.nodeToFlatMap;
+
 /**
  * Class that handles configuration changes, by validating them, passing to storage and listening to storage updates.
  */
@@ -98,10 +96,14 @@ public class ConfigurationChanger {
         return fut;
     }
 
-    /** */
+    /**
+     * Internal configuration change method that completes provided future.
+     * @param changes Map of changes by root key.
+     * @param fut Future, that must be completed after changes are written to the storage.
+     */
     private void change0(Map<RootKey<?>, TraversableTreeNode> changes, CompletableFuture<?> fut) {
         Map<String, Serializable> allChanges = changes.entrySet().stream()
-            .map((Map.Entry<RootKey<?>, TraversableTreeNode> change) -> convertChangesToMap(change.getKey(), change.getValue()))
+            .map((Map.Entry<RootKey<?>, TraversableTreeNode> change) -> nodeToFlatMap(change.getKey(), change.getValue()))
             .flatMap(map -> map.entrySet().stream())
             .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
 
@@ -162,67 +164,6 @@ public class ConfigurationChanger {
     }
 
     /**
-     * Convert a traversable tree to a map of qualified keys to values.
-     * @param rootKey Root configuration key.
-     * @param node Tree.
-     * @return Map of changes.
-     */
-    private Map<String, Serializable> convertChangesToMap(RootKey<?> rootKey, TraversableTreeNode node) {
-        Map<String, Serializable> values = new HashMap<>();
-
-        node.accept(rootKey.key(), new ConfigurationVisitor<>() {
-            /** Current key, aggregated by visitor. */
-            StringBuilder currentKey = new StringBuilder();
-
-            /** {@inheritDoc} */
-            @Override public Void visitLeafNode(String key, Serializable val) {
-                values.put(currentKey.toString() + key, val);
-
-                return null;
-            }
-
-            /** {@inheritDoc} */
-            @Override public Void visitInnerNode(String key, InnerNode node) {
-                if (node == null)
-                    return null;
-
-                int previousKeyLength = currentKey.length();
-
-                currentKey.append(key).append('.');
-
-                node.traverseChildren(this);
-
-                currentKey.setLength(previousKeyLength);
-
-                return null;
-            }
-
-            /** {@inheritDoc} */
-            @Override public <N extends InnerNode> Void visitNamedListNode(String key, NamedListNode<N> node) {
-                int previousKeyLength = currentKey.length();
-
-                if (key != null)
-                    currentKey.append(key).append('.');
-
-                for (String namedListKey : node.namedListKeys()) {
-                    int loopPreviousKeyLength = currentKey.length();
-
-                    currentKey.append(ConfigurationUtil.escape(namedListKey)).append('.');
-
-                    node.get(namedListKey).traverseChildren(this);
-
-                    currentKey.setLength(loopPreviousKeyLength);
-                }
-
-                currentKey.setLength(previousKeyLength);
-
-                return null;
-            }
-        });
-        return values;
-    }
-
-    /**
      * Results of the validation.
      */
     private static final class ValidationResult {
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
index 23e84f0..79d569f 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
@@ -19,11 +19,13 @@ package org.apache.ignite.configuration.internal.util;
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.RandomAccess;
 import java.util.stream.Collectors;
+import org.apache.ignite.configuration.RootKey;
 import org.apache.ignite.configuration.tree.ConfigurationSource;
 import org.apache.ignite.configuration.tree.ConfigurationVisitor;
 import org.apache.ignite.configuration.tree.ConstructableTreeNode;
@@ -211,6 +213,67 @@ public class ConfigurationUtil {
     }
 
     /**
+     * Convert a traversable tree to a map of qualified keys to values.
+     * @param rootKey Root configuration key.
+     * @param node Tree.
+     * @return Map of changes.
+     */
+    public static Map<String, Serializable> nodeToFlatMap(RootKey<?> rootKey, TraversableTreeNode node) {
+        Map<String, Serializable> values = new HashMap<>();
+
+        node.accept(rootKey.key(), new ConfigurationVisitor<>() {
+            /** Current key, aggregated by visitor. */
+            StringBuilder currentKey = new StringBuilder();
+
+            /** {@inheritDoc} */
+            @Override public Void visitLeafNode(String key, Serializable val) {
+                values.put(currentKey.toString() + key, val);
+
+                return null;
+            }
+
+            /** {@inheritDoc} */
+            @Override public Void visitInnerNode(String key, InnerNode node) {
+                if (node == null)
+                    return null;
+
+                int previousKeyLength = currentKey.length();
+
+                currentKey.append(key).append('.');
+
+                node.traverseChildren(this);
+
+                currentKey.setLength(previousKeyLength);
+
+                return null;
+            }
+
+            /** {@inheritDoc} */
+            @Override public <N extends InnerNode> Void visitNamedListNode(String key, NamedListNode<N> node) {
+                int previousKeyLength = currentKey.length();
+
+                if (key != null)
+                    currentKey.append(key).append('.');
+
+                for (String namedListKey : node.namedListKeys()) {
+                    int loopPreviousKeyLength = currentKey.length();
+
+                    currentKey.append(ConfigurationUtil.escape(namedListKey)).append('.');
+
+                    node.get(namedListKey).traverseChildren(this);
+
+                    currentKey.setLength(loopPreviousKeyLength);
+                }
+
+                currentKey.setLength(previousKeyLength);
+
+                return null;
+            }
+        });
+        return values;
+    }
+
+    /**
      * Apply changes on top of existing node. Creates completely new object while reusing parts of the original tree
      * that weren't modified.
      *