ibessonov commented on code in PR #5728:
URL: https://github.com/apache/ignite-3/pull/5728#discussion_r2068584661
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/DeletedKeysFilter.java:
##########
@@ -0,0 +1,51 @@
+package org.apache.ignite.internal.configuration;
+
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+class DeletedKeysFilter {
+ /**
+ * Filters out keys that match the deleted prefixes and returns the
ignored keys.
+ *
+ * @param values The map of values to filter.
+ * @param deletedPrefixes The collection of deleted key patterns.
+ * @return A collection of keys that were ignored.
+ */
+ static List<String> ignoreDeleted(
+ Map<String, ? extends Serializable> values,
+ Collection<String> deletedPrefixes
+ ) {
+ if (deletedPrefixes.isEmpty()) {
+ return List.of();
+ }
+
+ List<Pattern> patterns = deletedPrefixes.stream()
+ .map(deletedKey -> deletedKey.replace(".", "\\.").replace("*",
".*") + ".*")
+ .map(Pattern::compile)
+ .collect(Collectors.toList());
+
+ List<String> ignoredKeys = values.keySet().stream()
+ .filter(serializable -> isDeleted(serializable, patterns))
Review Comment:
`serializable` - strange name
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/DeletedConfigurationTest.java:
##########
@@ -0,0 +1,97 @@
+package org.apache.ignite.internal.configuration;
+
+import static
org.apache.ignite.configuration.annotation.ConfigurationType.LOCAL;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.Stream;
+import org.apache.ignite.configuration.RootKey;
+import org.apache.ignite.configuration.annotation.ConfigurationRoot;
+import org.apache.ignite.configuration.annotation.Value;
+import
org.apache.ignite.internal.configuration.storage.TestConfigurationStorage;
+import
org.apache.ignite.internal.configuration.validation.TestConfigurationValidator;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+class DeletedConfigurationTest {
+ private static final String DELETED_INDIVIDUAL_PROPERTY =
"individual.property";
+ private static final String DELETED_SUBTREE = "subtree";
+ private static final String SUBTREE_NAMED_LIST = "subtree.named.*.list";
+ private static final String NESTED_NAMED_LISTS = "nested.*.named.*.list";
+
+ private static final Collection<String> DELETED_KEYS = List.of(
+ DELETED_INDIVIDUAL_PROPERTY,
+ DELETED_SUBTREE,
+ SUBTREE_NAMED_LIST,
+ NESTED_NAMED_LISTS
+ );
+
+ private static final String VALID_KEY = "key.someValue";
+
+ @ConfigurationRoot(rootName = "key", type = LOCAL)
+ public static class DeletedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public String someValue = "default";
+ }
+
+ private static final ConfigurationTreeGenerator GENERATOR = new
ConfigurationTreeGenerator(DeletedTestConfiguration.KEY);
+
+ private final TestConfigurationStorage storage = new
TestConfigurationStorage(LOCAL);
+
+ @AfterAll
+ public static void afterAll() {
+ GENERATOR.close();
+ }
+
+ @ParameterizedTest
+ @MethodSource("deletedPropertiesProvider")
+ public void testPropertyDeleted(Map<String, String> deletedProperties) {
Review Comment:
There's no test where we would simulate user explicitly trying to set the
value for deleted property. Will it work?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/DeletedKeysFilter.java:
##########
@@ -0,0 +1,51 @@
+package org.apache.ignite.internal.configuration;
+
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+class DeletedKeysFilter {
+ /**
+ * Filters out keys that match the deleted prefixes and returns the
ignored keys.
+ *
+ * @param values The map of values to filter.
+ * @param deletedPrefixes The collection of deleted key patterns.
+ * @return A collection of keys that were ignored.
+ */
+ static List<String> ignoreDeleted(
+ Map<String, ? extends Serializable> values,
+ Collection<String> deletedPrefixes
+ ) {
+ if (deletedPrefixes.isEmpty()) {
+ return List.of();
+ }
+
+ List<Pattern> patterns = deletedPrefixes.stream()
+ .map(deletedKey -> deletedKey.replace(".", "\\.").replace("*",
".*") + ".*")
+ .map(Pattern::compile)
+ .collect(Collectors.toList());
Review Comment:
Why don't we pass a list of patterns then? I don't like the fact that we're
doing it every time
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java:
##########
@@ -150,6 +154,8 @@ private static class StorageRoots {
/** Full storage data. */
private final Data data;
+ private final Collection<String> ignoredKeys;
Review Comment:
I don't understand why it needs to be here, let's discuss it when you finish
adding a new test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
