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

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


The following commit(s) were added to refs/heads/ignite-3.1.0 by this push:
     new 98b51671d12 IGNITE-26690 Fix named list configuration updating (#6766)
98b51671d12 is described below

commit 98b51671d126d773e17d3d1308b2c11c15f8e1a1
Author: Mikhail <[email protected]>
AuthorDate: Wed Oct 15 16:08:59 2025 +0300

    IGNITE-26690 Fix named list configuration updating (#6766)
---
 .../storage/LocalFileConfigurationStorage.java     | 85 +++++++++-------------
 .../storage/LocalFileConfigurationStorageTest.java | 58 ++++++++++++++-
 2 files changed, 92 insertions(+), 51 deletions(-)

diff --git 
a/modules/configuration-storage/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
 
b/modules/configuration-storage/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
index 081d19e3151..003c39e0caa 100644
--- 
a/modules/configuration-storage/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
+++ 
b/modules/configuration-storage/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
@@ -18,6 +18,7 @@
 package org.apache.ignite.internal.configuration.storage;
 
 import static java.util.Collections.emptyNavigableMap;
+import static java.util.concurrent.CompletableFuture.completedFuture;
 import static java.util.stream.Collectors.toMap;
 import static 
org.apache.ignite.internal.configuration.util.ConfigurationFlattener.createFlattenedUpdatesMap;
 import static 
org.apache.ignite.internal.configuration.util.ConfigurationUtil.fillFromPrefixMap;
@@ -29,9 +30,7 @@ import com.typesafe.config.Config;
 import com.typesafe.config.ConfigException.Parse;
 import com.typesafe.config.ConfigFactory;
 import com.typesafe.config.ConfigObject;
-import com.typesafe.config.ConfigParseOptions;
 import com.typesafe.config.ConfigRenderOptions;
-import com.typesafe.config.ConfigSyntax;
 import com.typesafe.config.ConfigValue;
 import com.typesafe.config.impl.ConfigImpl;
 import java.io.IOException;
@@ -53,19 +52,19 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
-import org.apache.ignite.configuration.ConfigurationDynamicDefaultsPatcher;
 import org.apache.ignite.configuration.ConfigurationModule;
 import org.apache.ignite.configuration.KeyIgnorer;
 import org.apache.ignite.configuration.annotation.ConfigurationType;
 import 
org.apache.ignite.configuration.validation.ConfigurationValidationException;
 import org.apache.ignite.configuration.validation.ValidationIssue;
-import 
org.apache.ignite.internal.configuration.ConfigurationDynamicDefaultsPatcherImpl;
 import org.apache.ignite.internal.configuration.ConfigurationTreeGenerator;
 import org.apache.ignite.internal.configuration.NodeConfigCreateException;
 import org.apache.ignite.internal.configuration.NodeConfigParseException;
 import org.apache.ignite.internal.configuration.NodeConfigWriteException;
 import org.apache.ignite.internal.configuration.SuperRoot;
+import org.apache.ignite.internal.configuration.SuperRootChangeImpl;
 import org.apache.ignite.internal.configuration.hocon.HoconConverter;
+import org.apache.ignite.internal.configuration.tree.ConfigurationSource;
 import org.apache.ignite.internal.configuration.tree.ConverterToMapVisitor;
 import 
org.apache.ignite.internal.configuration.validation.ConfigurationDuplicatesValidator;
 import org.apache.ignite.internal.future.InFlightFutures;
@@ -145,61 +144,57 @@ public class LocalFileConfigurationStorage implements 
ConfigurationStorage {
         checkAndRestoreConfigFile();
     }
 
-    /**
-     * Patch the local configs with defaults from provided {@link 
ConfigurationModule}.
-     *
-     * @param hocon Config string in Hocon format.
-     * @param module Configuration module, which provides configuration 
patches.
-     * @return Patched config string in Hocon format.
-     */
-    private String patch(String hocon, ConfigurationModule module) {
-        if (module == null) {
-            return hocon;
-        }
-
-        ConfigurationDynamicDefaultsPatcher localCfgDynamicDefaultsPatcher = 
new ConfigurationDynamicDefaultsPatcherImpl(
-                module,
-                generator
-        );
-
-        return localCfgDynamicDefaultsPatcher.patchWithDynamicDefaults(hocon);
-    }
-
     @Override
     public CompletableFuture<Data> readDataOnRecovery() {
         lock.writeLock().lock();
         try {
+            // Here we don't use ConfigurationDynamicDefaultsPatcher because 
it works only on Hocon string representation level.
+            // But it's not applicable here because we need to produce map 
presentation with same ids in names lists.
+            // Each tree walk for string to map mapping produce different ids 
by design.
             String hocon = readHoconFromFile();
-            String hoconWithDynamicDefaults = patch(hocon, module);
+            SuperRoot superRoot = convertToSuperRoot(hocon);
+
+            Map<String, Serializable> transformedHocon = 
transformToMap(superRoot);
 
-            transformToMap(hocon).forEach((key, value) -> {
+            transformedHocon.forEach((key, value) -> {
                 if (value != null) { // Filter defaults.
                     latest.put(key, value);
                 }
             });
 
-            return CompletableFuture.completedFuture(new 
Data(transformToMap(hoconWithDynamicDefaults), lastRevision));
+            if (module != null) {
+                module.patchConfigurationWithDynamicDefaults(new 
SuperRootChangeImpl(superRoot));
+                transformedHocon = transformToMap(superRoot);
+            }
+
+            return completedFuture(new Data(transformedHocon, lastRevision));
         } finally {
             lock.writeLock().unlock();
         }
     }
 
-    private Map<String, Serializable> transformToMap(String hocon) {
-        SuperRoot superRoot = generator.createSuperRoot();
-        SuperRoot copiedSuperRoot = superRoot.copy();
+    private SuperRoot convertToSuperRoot(String hocon) {
+        try {
+            Config config = ConfigFactory.parseString(hocon);
+            KeyIgnorer keyIgnorer = module == null ? s -> false : 
KeyIgnorer.fromDeletedPrefixes(module.deletedPrefixes());
+
+            ConfigurationSource hoconSource = 
HoconConverter.hoconSource(config.root(), keyIgnorer);
 
-        KeyIgnorer keyIgnorer = module == null ? s -> false : 
KeyIgnorer.fromDeletedPrefixes(module.deletedPrefixes());
+            SuperRoot superRoot = generator.createSuperRoot();
+            hoconSource.descend(superRoot);
 
-        HoconConverter.hoconSource(parseConfig(hocon).root(), 
keyIgnorer).descend(copiedSuperRoot);
+            return superRoot;
+        } catch (Exception e) {
+            throw new ConfigurationValidationException("Failed to parse HOCON: 
" + e.getMessage());
+        }
+    }
 
-        return createFlattenedUpdatesMap(superRoot, copiedSuperRoot, 
emptyNavigableMap())
+    private Map<String, Serializable> transformToMap(SuperRoot superRoot) {
+        return createFlattenedUpdatesMap(generator.createSuperRoot(), 
superRoot, emptyNavigableMap())
                 .entrySet()
                 .stream()
                 .filter(e -> e.getValue() != null)
-                .collect(toMap(
-                        Entry::getKey,
-                        Entry::getValue)
-                );
+                .collect(toMap(Entry::getKey, Entry::getValue));
     }
 
     private String readHoconFromFile() {
@@ -220,21 +215,11 @@ public class LocalFileConfigurationStorage implements 
ConfigurationStorage {
         }
     }
 
-    private Config parseConfig(String confString) {
-        try {
-            ConfigParseOptions parseOptions = 
ConfigParseOptions.defaults().setSyntax(ConfigSyntax.CONF).setAllowMissing(false);
-
-            return ConfigFactory.parseString(confString, parseOptions);
-        } catch (Parse e) {
-            throw new NodeConfigParseException("Failed to parse config content 
from file " + configPath, e);
-        }
-    }
-
     @Override
     public CompletableFuture<Map<String, ? extends Serializable>> 
readAllLatest(String prefix) {
         lock.readLock().lock();
         try {
-            return CompletableFuture.completedFuture(
+            return completedFuture(
                     latest.entrySet()
                             .stream()
                             .filter(entry -> entry.getKey().startsWith(prefix))
@@ -249,7 +234,7 @@ public class LocalFileConfigurationStorage implements 
ConfigurationStorage {
     public CompletableFuture<Serializable> readLatest(String key) {
         lock.readLock().lock();
         try {
-            return CompletableFuture.completedFuture(latest.get(key));
+            return completedFuture(latest.get(key));
         } finally {
             lock.readLock().unlock();
         }
@@ -303,7 +288,7 @@ public class LocalFileConfigurationStorage implements 
ConfigurationStorage {
 
     @Override
     public CompletableFuture<Long> lastRevision() {
-        return CompletableFuture.completedFuture(lastRevision);
+        return completedFuture(lastRevision);
     }
 
     @Override
diff --git 
a/modules/configuration-storage/src/test/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorageTest.java
 
b/modules/configuration-storage/src/test/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorageTest.java
index c78382a5033..6d46b528df8 100644
--- 
a/modules/configuration-storage/src/test/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorageTest.java
+++ 
b/modules/configuration-storage/src/test/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorageTest.java
@@ -50,12 +50,18 @@ import org.apache.ignite.configuration.KeyIgnorer;
 import org.apache.ignite.configuration.annotation.Config;
 import org.apache.ignite.configuration.annotation.ConfigValue;
 import org.apache.ignite.configuration.annotation.ConfigurationRoot;
+import org.apache.ignite.configuration.annotation.InjectedName;
 import org.apache.ignite.configuration.annotation.NamedConfigValue;
+import org.apache.ignite.configuration.annotation.PolymorphicConfig;
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+import org.apache.ignite.configuration.annotation.PolymorphicId;
 import org.apache.ignite.configuration.annotation.PublicName;
 import org.apache.ignite.configuration.annotation.Value;
 import 
org.apache.ignite.configuration.validation.ConfigurationValidationException;
 import org.apache.ignite.internal.configuration.ConfigurationTreeGenerator;
 import org.apache.ignite.internal.configuration.TestConfigurationChanger;
+import org.apache.ignite.internal.configuration.hocon.HoconConverter;
+import org.apache.ignite.internal.configuration.tree.ConfigurationSource;
 import 
org.apache.ignite.internal.configuration.validation.ConfigurationValidatorImpl;
 import org.apache.ignite.internal.testframework.WorkDirectory;
 import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
@@ -85,7 +91,11 @@ public class LocalFileConfigurationStorageTest {
 
     @BeforeAll
     public static void beforeAll() {
-        treeGenerator = new ConfigurationTreeGenerator(TopConfiguration.KEY, 
TopEmptyConfiguration.KEY);
+        treeGenerator = new ConfigurationTreeGenerator(
+                List.of(TopConfiguration.KEY, TopEmptyConfiguration.KEY),
+                List.of(),
+                List.of(FirstNamedListConfigurationSchema.class)
+        );
     }
 
     @AfterAll
@@ -545,6 +555,28 @@ public class LocalFileConfigurationStorageTest {
         assertThat(storage.write(Map.of(), storage.localRevision().get() + 1), 
willCompleteSuccessfully());
     }
 
+    @Test
+    void updateAfterRestart() throws Exception {
+        assertThat(configFileContent(), emptyString());
+
+        // Initialize value
+        com.typesafe.config.Config config = 
ConfigFactory.parseString("top.polyNamedList = 
[{name=name1,strVal=foo,type=first}]");
+        ConfigurationSource source = HoconConverter.hoconSource(config.root());
+
+        changer.start();
+        assertThat(changer.change(source), willCompleteSuccessfully());
+
+        // Force restart of the storage
+        after();
+        before();
+
+        config = 
ConfigFactory.parseString("top.polyNamedList.name1.strVal=strVal1");
+        source = HoconConverter.hoconSource(config.root());
+
+        changer.start();
+        assertThat(changer.change(source), willCompleteSuccessfully());
+    }
+
     private String configFileContent() throws IOException {
         return Files.readString(getConfigFile());
     }
@@ -569,6 +601,9 @@ public class LocalFileConfigurationStorageTest {
         @Deprecated
         @Value(hasDefault = true)
         public int deprecated = 0;
+
+        @NamedConfigValue
+        public PolyNamedListConfigurationSchema polyNamedList;
     }
 
 
@@ -601,4 +636,25 @@ public class LocalFileConfigurationStorageTest {
         @Value(hasDefault = true)
         public int intVal = 1;
     }
+
+    /**
+     * Polymorphic configuration schema.
+     */
+    @PolymorphicConfig
+    public static class PolyNamedListConfigurationSchema {
+        @PolymorphicId
+        public String type;
+
+        @InjectedName
+        public String name;
+    }
+
+    /**
+     * Simple implementation of polymorphic base config.
+     */
+    @PolymorphicConfigInstance("first")
+    public static class FirstNamedListConfigurationSchema extends 
PolyNamedListConfigurationSchema {
+        @Value(hasDefault = true)
+        public String strVal = "foo";
+    }
 }

Reply via email to