ibessonov commented on a change in pull request #273:
URL: https://github.com/apache/ignite-3/pull/273#discussion_r688459605
##########
File path:
modules/cli/src/main/java/org/apache/ignite/cli/spec/ConfigCommandSpec.java
##########
@@ -81,10 +94,28 @@
@CommandLine.Mixin
private CfgHostnameOptions cfgHostnameOptions;
+ /**
+ * Configuration type: {@code node} or {@code cluster}.
+ *
+ * TODO: Fix in https://issues.apache.org/jira/browse/IGNITE-15285
Review comment:
Same here
##########
File path:
modules/cli/src/integrationTest/java/org/apache/ignite/cli/ConfigCommandTest.java
##########
@@ -85,6 +85,7 @@ public void setAndGetWithManualHost() {
"set",
"--node-endpoint",
"localhost:" + restPort,
+ "--type", "node", //TODO: Fix in
https://issues.apache.org/jira/browse/IGNITE-15285
Review comment:
Please change the link here and in other places
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -210,36 +193,27 @@ public void register(ConfigurationStorage
configurationStorage) throws Configura
superRoot.addRoot(rootKey, rootNode);
}
- StorageRoots storageRoots = new StorageRoots(superRoot,
data.changeId());
+ storageRoots = new StorageRoots(superRoot, data.changeId());
- storagesRootsMap.put(configurationStorage.type(), storageRoots);
-
- configurationStorage.registerConfigurationListener(changedEntries ->
updateFromListener(
- configurationStorage.type(),
- changedEntries
- ));
+ storage.registerConfigurationListener(this::updateFromListener);
}
/**
* Initializes the configuration storage - reads data and sets default
values for missing configuration properties.
- * @param storageType Storage type.
+ *
* @throws ConfigurationValidationException If configuration validation
failed.
- * @throws ConfigurationChangeException If configuration framework failed
to add default values and save them to
- * storage.
+ * @throws ConfigurationChangeException If configuration framework failed
to add default values and save them to storage.
*/
- public void initialize(ConfigurationType storageType) throws
ConfigurationValidationException, ConfigurationChangeException {
- ConfigurationStorage configurationStorage =
storageInstances.get(storageType);
-
- assert configurationStorage != null : storageType;
-
+ public void initializeDefaults() throws ConfigurationValidationException,
ConfigurationChangeException {
try {
- ConfigurationSource defaultsCfgSource = new ConfigurationSource() {
+ ConfigurationSource defaultsCfgSrc = new ConfigurationSource() {
Review comment:
I believe you're using abbreviation plugin. Can we use "source" as a
full word if it's a part of the name?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -100,73 +105,58 @@
private final SuperRoot roots;
/** Version associated with the currently known storage state. */
- private final long version;
+ private final long ver;
- /** */
- private StorageRoots(SuperRoot roots, long version) {
+ /**
+ * Constructor.
+ *
+ * @param roots Forest.
+ * @param ver Version associated with the currently known storage
state.
+ */
+ private StorageRoots(SuperRoot roots, long ver) {
Review comment:
Same here, we don't have to use short names
##########
File path: modules/rest/src/main/java/org/apache/ignite/rest/RestModule.java
##########
@@ -53,90 +55,88 @@
* Refer to default config file in resources for the example.
*/
public class RestModule implements IgniteComponent {
- /** */
+ /** Default port. */
public static final int DFLT_PORT = 10300;
- /** */
- private static final String CONF_URL = "/management/v1/configuration/";
+ /** Node configuration route. */
+ private static final String NODE_CFG_URL =
"/management/v1/configuration/node/";
- /** */
+ /** Cluster configuration route. */
+ private static final String CLUSTER_CFG_URL =
"/management/v1/configuration/cluster/";
+
+ /** Path parameter. */
private static final String PATH_PARAM = "selector";
/** Ignite logger. */
private final IgniteLogger LOG = IgniteLogger.forClass(RestModule.class);
- /** */
- private final ConfigurationRegistry sysConf;
+ /** Node configuration register. */
+ private final ConfigurationRegistry nodeCfgRegistry;
+
+ /** Presentation of node configuration. */
+ private final ConfigurationPresentation<String> nodeCfgPresentation;
- /** */
- private volatile ConfigurationPresentation<String> presentation;
+ /** Presentation of cluster configuration. */
+ private final ConfigurationPresentation<String> clusterCfgPresentation;
/**
* Creates a new instance of REST module.
*
* @param nodeCfgMgr Node configuration manager.
+ * @param clusterCfgMgr Cluster configuration manager.
*/
- public RestModule(ConfigurationManager nodeCfgMgr) {
- sysConf = nodeCfgMgr.configurationRegistry();
+ public RestModule(
+ ConfigurationManager nodeCfgMgr,
+ ConfigurationManager clusterCfgMgr
+ ) {
+ nodeCfgRegistry = nodeCfgMgr.registry();
+
+ nodeCfgPresentation = new HoconPresentation(nodeCfgMgr.registry());
+ clusterCfgPresentation = new
HoconPresentation(clusterCfgMgr.registry());
}
/** {@inheritDoc} */
@Override public void start() {
- presentation = new HoconPresentation(sysConf);
-
var router = new Router();
router
- .get(CONF_URL, (req, resp) -> {
- resp.json(presentation.represent());
- })
- .get(CONF_URL + ":" + PATH_PARAM, (req, resp) -> {
- String cfgPath = req.queryParams().get(PATH_PARAM);
- try {
- resp.json(presentation.representByPath(cfgPath));
- }
- catch (IllegalArgumentException pathE) {
- ErrorResult eRes = new
ErrorResult("CONFIG_PATH_UNRECOGNIZED", pathE.getMessage());
-
- resp.status(BAD_REQUEST);
- resp.json(Map.of("error", eRes));
- }
- })
- .put(CONF_URL, HttpHeaderValues.APPLICATION_JSON, (req, resp) -> {
- try {
- presentation.update(
- req
- .request()
- .content()
-
.readCharSequence(req.request().content().readableBytes(),
StandardCharsets.UTF_8)
- .toString());
- }
- catch (IllegalArgumentException e) {
- ErrorResult eRes = new
ErrorResult("INVALID_CONFIG_FORMAT", e.getMessage());
-
- resp.status(BAD_REQUEST);
- resp.json(Map.of("error", eRes));
- }
- catch (ConfigurationValidationException e) {
- ErrorResult eRes = new ErrorResult("VALIDATION_EXCEPTION",
e.getMessage());
-
- resp.status(BAD_REQUEST);
- resp.json(Map.of("error", eRes));
- }
- catch (IgniteException e) {
- ErrorResult eRes = new
ErrorResult("APPLICATION_EXCEPTION", e.getMessage());
-
- resp.status(BAD_REQUEST);
- resp.json(Map.of("error", eRes));
- }
- });
+ .get(
+ NODE_CFG_URL,
+ (req, resp) -> resp.json(nodeCfgPresentation.represent())
+ )
+ .get(
+ CLUSTER_CFG_URL,
+ (req, resp) -> resp.json(clusterCfgPresentation.represent())
+ )
+ .get(
+ NODE_CFG_URL + ":" + PATH_PARAM,
+ (req, resp) -> handleRepresentByPath(req, resp,
nodeCfgPresentation)
+ )
+ .get(
+ CLUSTER_CFG_URL + ":" + PATH_PARAM,
+ (req, resp) -> handleRepresentByPath(req, resp,
clusterCfgPresentation)
+ )
+ .put(
+ NODE_CFG_URL,
+ APPLICATION_JSON,
+ (req, resp) -> handleUpdate(req, resp, nodeCfgPresentation)
+ )
+ .put(
+ CLUSTER_CFG_URL,
+ APPLICATION_JSON,
+ (req, resp) -> handleUpdate(req, resp, clusterCfgPresentation)
+ );
startRestEndpoint(router);
}
- /** */
- private ChannelFuture startRestEndpoint(Router router) {
- RestView restConfigurationView =
sysConf.getConfiguration(RestConfiguration.KEY).value();
+ /**
+ * Start endpoint.
+ *
+ * @param router Dispatcher of http requests.
+ */
+ private void startRestEndpoint(Router router) {
Review comment:
What's the deal with changing return type of the method?
##########
File path:
modules/configuration-annotation-processor/src/test/java/org/apache/ignite/internal/configuration/TestConfigurationChanger.java
##########
@@ -28,20 +35,40 @@
/** Runtime implementations generator for node classes. */
private final ConfigurationAsmGenerator cgen;
+ /** Root keys. */
+ private final Collection<RootKey<?, ?>> rootKeys;
+
/**
+ * Constructor.
+ *
* @param cgen Runtime implementations generator for node classes. Will be
used to instantiate nodes objects.
+ * @param rootKeys Configuration root keys.
+ * @param validators Validators.
+ * @param storage Configuration storage.
+ * @throws IllegalArgumentException If the configuration type of the root
keys is not equal to the storage type.
*/
- public TestConfigurationChanger(ConfigurationAsmGenerator cgen) {
- super((oldRoot, newRoot, revision) -> completedFuture(null));
+ public TestConfigurationChanger(
+ ConfigurationAsmGenerator cgen,
+ Collection<RootKey<?, ?>> rootKeys,
+ Map<Class<? extends Annotation>, Set<Validator<? extends Annotation,
?>>> validators,
+ ConfigurationStorage storage
+ ) {
+ super(
+ (oldRoot, newRoot, revision) -> completedFuture(null),
+ rootKeys,
+ validators,
+ storage
+ );
this.cgen = cgen;
+ this.rootKeys = rootKeys;
}
/** {@inheritDoc} */
- @Override public void addRootKey(RootKey<?, ?> rootKey) {
- super.addRootKey(rootKey);
+ @Override public void start() throws ConfigurationChangeException {
+ rootKeys.forEach(key -> cgen.compileRootSchema(key.schemaClass()));
Review comment:
You can do this in constructor and remove new field
##########
File path:
modules/cli/src/main/java/org/apache/ignite/cli/builtins/config/ConfigurationClient.java
##########
@@ -69,21 +69,24 @@ public ConfigurationClient(HttpClient httpClient) {
* @param host String representation of server node host.
* @param port Host REST port.
* @param rawHoconPath HOCON dot-delimited path of requested configuration.
+ * @param type Configuration type: {@code node} or {@code cluster}.
+ * TODO: Fix in https://issues.apache.org/jira/browse/IGNITE-15285
Review comment:
It this a valid TODO syntax? Can IDEA recognize it? I would suggest
creating additional single-line comment instead
##########
File path:
modules/cli/src/test/java/org/apache/ignite/cli/IgniteCliInterfaceTest.java
##########
@@ -508,22 +508,22 @@ void get() throws IOException, InterruptedException {
"}\n", out.toString());
}
- /** */
+ /** //TODO: Fix in https://issues.apache.org/jira/browse/IGNITE-15285
*/
Review comment:
What is this syntax?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -100,73 +105,58 @@
private final SuperRoot roots;
/** Version associated with the currently known storage state. */
- private final long version;
+ private final long ver;
Review comment:
What's the reason for shortening the name? If anything, you should have
renamed it to "changeId" :)
##########
File path:
modules/configuration-annotation-processor/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java
##########
@@ -76,17 +76,13 @@
/** */
@BeforeEach
public void before() {
- var testConfigurationStorage = new
TestConfigurationStorage(ConfigurationType.LOCAL);
+ var testConfigurationStorage = new TestConfigurationStorage(LOCAL);
- registry = new ConfigurationRegistry(
- Collections.singletonList(ParentConfiguration.KEY),
- Collections.emptyMap(),
- Collections.singletonList(testConfigurationStorage)
- );
+ registry = new ConfigurationRegistry(List.of(ParentConfiguration.KEY),
Map.of(), testConfigurationStorage);
registry.start();
- registry.startStorageConfigurations(testConfigurationStorage.type());
+ registry.initializeDefaultsStorage();
Review comment:
Word "Storage" seems out of place
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -100,73 +105,58 @@
private final SuperRoot roots;
/** Version associated with the currently known storage state. */
- private final long version;
+ private final long ver;
- /** */
- private StorageRoots(SuperRoot roots, long version) {
+ /**
+ * Constructor.
+ *
+ * @param roots Forest.
+ * @param ver Version associated with the currently known storage
state.
+ */
+ private StorageRoots(SuperRoot roots, long ver) {
this.roots = roots;
- this.version = version;
+ this.ver = ver;
}
}
- /** Lazy annotations cache for configuration schema fields. */
- private final Map<MemberKey, Annotation[]> cachedAnnotations = new
ConcurrentHashMap<>();
-
- /** Storage instances by their classes. Comes in handy when all you have
is {@link RootKey}. */
- private final Map<ConfigurationType, ConfigurationStorage>
storageInstances = new HashMap<>();
-
/**
+ * Constructor.
+ *
* @param notificator Closure to execute when update from the storage is
received.
+ * @param rootKeys Configuration root keys.
+ * @param validators Validators.
+ * @param storage Configuration storage.
+ * @throws IllegalArgumentException If the configuration type of the root
keys is not equal to the storage type.
*/
- public ConfigurationChanger(Notificator notificator) {
- this.notificator = notificator;
- }
-
- /**
- * Adds a single validator instance.
- * @param annotationType Annotation type for validated fields.
- * @param validator Validator instance for this annotation.
- * @param <A> Annotation type.
- */
- public <A extends Annotation> void addValidator(Class<A> annotationType,
Validator<A, ?> validator) {
- validators
- .computeIfAbsent(annotationType, a -> new HashSet<>())
- .add(validator);
- }
-
- /**
- * Adds multiple validators instances.
- * @param annotationType Annotation type for validated fields.
- * @param validators Set of validator instancec for this annotation.
- */
- public void addValidators(Class<? extends Annotation> annotationType,
Set<Validator<? extends Annotation, ?>> validators) {
- this.validators
- .computeIfAbsent(annotationType, a -> new HashSet<>())
- .addAll(validators);
- }
+ public ConfigurationChanger(
+ Notificator notificator,
+ Collection<RootKey<?, ?>> rootKeys,
+ Map<Class<? extends Annotation>, Set<Validator<? extends Annotation,
?>>> validators,
+ ConfigurationStorage storage
+ ) {
+ checkConfigurationType(rootKeys, storage);
- /**
- * Registers an additional root key.
- * @param rootKey Root key instance.
- */
- public void addRootKey(RootKey<?, ?> rootKey) {
- assert !storageInstances.containsKey(rootKey.type());
+ this.notificator = notificator;
+ this.validators = validators;
+ this.storage = storage;
- rootKeys.put(rootKey.key(), rootKey);
+ this.rootKeys = rootKeys.stream().collect(toMap(RootKey::key,
identity()));
}
/**
* Created new {@code Node} object that corresponds to passed root keys
root configuration node.
Review comment:
Please fix my typo, it should start with "Creates"
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -251,30 +225,27 @@ public void initialize(ConfigurationType storageType)
throws ConfigurationValida
throw (ConfigurationChangeException)cause;
throw new ConfigurationChangeException(
- "Failed to write defalut configuration values into the storage
" + configurationStorage.getClass(), e
+ "Failed to write defalut configuration values into the storage
" + storage.getClass(), e
);
}
catch (InterruptedException e) {
throw new ConfigurationChangeException(
- "Failed to initialize configuration storage " +
configurationStorage.getClass(), e
+ "Failed to initialize configuration storage " +
storage.getClass(), e
);
}
}
/**
* Changes the configuration.
- * @param source Configuration source to create patch from.
- * @param storage Expected storage for the changes. If null, a derived
storage will be used
- * unconditionaly.
+ *
+ * @param src Configuration source to create patch from.
* @return Future that is completed on change completion.
*/
- public CompletableFuture<Void> change(
- ConfigurationSource source,
- @Nullable ConfigurationStorage storage
- ) {
+ public CompletableFuture<Void> change(ConfigurationSource src) {
Set<ConfigurationType> storagesTypes = new HashSet<>();
ConstructableTreeNode collector = new ConstructableTreeNode() {
Review comment:
I think that this all monstrosity with the collector should be removed
or heavily cut. We don't need this logic anymore since changer only knows about
one storage
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -251,30 +225,27 @@ public void initialize(ConfigurationType storageType)
throws ConfigurationValida
throw (ConfigurationChangeException)cause;
throw new ConfigurationChangeException(
- "Failed to write defalut configuration values into the storage
" + configurationStorage.getClass(), e
+ "Failed to write defalut configuration values into the storage
" + storage.getClass(), e
Review comment:
```suggestion
"Failed to write default configuration values into the
storage " + storage.getClass(), e
```
##########
File path:
modules/affinity/src/main/java/org/apache/ignite/internal/affinity/AffinityManager.java
##########
@@ -128,7 +128,7 @@ public AffinityManager(
* @return A future which will complete when the assignment is calculated.
*/
public CompletableFuture<Boolean> calculateAssignments(UUID tblId, String
tblName) {
- TableConfiguration tblConfig = configurationMgr.configurationRegistry()
+ TableConfiguration tblConfig = configurationMgr.registry()
Review comment:
Can you please rename it back?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -302,19 +274,18 @@ public void initialize(ConfigurationType storageType)
throws ConfigurationValida
)
);
}
-
- ConfigurationStorage actualStorage =
storageInstances.get(storagesTypes.iterator().next());
-
- if (storage != null && storage != actualStorage) {
+ else if (storagesTypes.iterator().next() != storage.type()) {
return CompletableFuture.failedFuture(
new ConfigurationChangeException("Mismatched storage passed.")
);
}
- return changeInternally(source, actualStorage);
+ return changeInternally(src);
}
- /** Stop component. */
+ /**
+ * Stop component.
Review comment:
I saw that you changes multi-line comments to single-line somewhere (in
RootKey, I think). What was your motivation?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
##########
@@ -167,7 +155,7 @@ public void startStorageConfigurations(ConfigurationType
storageType) {
* @throws IllegalArgumentException If {@code path} is not found in
current configuration.
*/
public <T> T represent(List<String> path, ConfigurationVisitor<T> visitor)
throws IllegalArgumentException {
- SuperRoot mergedSuperRoot = changer.mergedSuperRoot();
+ SuperRoot mergedSuperRoot = changer.superRoot();
Review comment:
Cool! We don't need merged superRoot anymore! Can you rename variable as
well?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -251,30 +225,27 @@ public void initialize(ConfigurationType storageType)
throws ConfigurationValida
throw (ConfigurationChangeException)cause;
throw new ConfigurationChangeException(
- "Failed to write defalut configuration values into the storage
" + configurationStorage.getClass(), e
+ "Failed to write defalut configuration values into the storage
" + storage.getClass(), e
);
}
catch (InterruptedException e) {
throw new ConfigurationChangeException(
- "Failed to initialize configuration storage " +
configurationStorage.getClass(), e
+ "Failed to initialize configuration storage " +
storage.getClass(), e
);
}
}
/**
* Changes the configuration.
- * @param source Configuration source to create patch from.
- * @param storage Expected storage for the changes. If null, a derived
storage will be used
- * unconditionaly.
+ *
+ * @param src Configuration source to create patch from.
* @return Future that is completed on change completion.
*/
- public CompletableFuture<Void> change(
- ConfigurationSource source,
- @Nullable ConfigurationStorage storage
- ) {
+ public CompletableFuture<Void> change(ConfigurationSource src) {
Review comment:
Why do we need these unnecessary renames?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -60,38 +59,44 @@
* Class that handles configuration changes, by validating them, passing to
storage and listening to storage updates.
*/
public abstract class ConfigurationChanger {
- /** */
+ /** Thread pool. */
private final ForkJoinPool pool = new ForkJoinPool(2);
- /** */
- private final Map<String, RootKey<?, ?>> rootKeys = new TreeMap<>();
+ /** Lazy annotations cache for configuration schema fields. */
+ private final Map<MemberKey, Annotation[]> cachedAnnotations = new
ConcurrentHashMap<>();
+
+ /** Closure to execute when an update from the storage is received. */
+ private final Notificator notificator;
+
+ /** Root keys. Mapping: {@link RootKey#key()} -> identity (itself). */
+ private final Map<String, RootKey<?, ?>> rootKeys;
+
+ /** Validators. */
+ private final Map<Class<? extends Annotation>, Set<Validator<? extends
Annotation, ?>>> validators;
- /** Map that has all the trees in accordance to their storages. */
- private final Map<ConfigurationType, StorageRoots> storagesRootsMap = new
ConcurrentHashMap<>();
+ /** Configuration storage. */
+ private final ConfigurationStorage storage;
- /** Annotation classes mapped to validator objects. */
- private Map<Class<? extends Annotation>, Set<Validator<?, ?>>> validators
= new HashMap<>();
+ /** Storage trees. */
+ private volatile StorageRoots storageRoots;
/**
- * Closure interface to be used by the configuration changer. An instance
of this closure is passed into the constructor and
- * invoked every time when there's an update from any of the storages.
+ * Closure interface to be used by the configuration changer. An instance
of this closure is passed into
+ * the constructor and invoked every time when there's an update from any
of the storages.
*/
@FunctionalInterface
public interface Notificator {
/**
* Invoked every time when the configuration is updated.
+ *
* @param oldRoot Old roots values. All these roots always belong to a
single storage.
* @param newRoot New values for the same roots as in {@code oldRoot}.
* @param storageRevision Revision of the storage.
- * @return Not-null future that must signify when processing is
completed. Exceptional completion is not
- * expected.
+ * @return Future that must signify when processing is completed.
Exceptional completion is not expected.
*/
- @NotNull CompletableFuture<Void> notify(SuperRoot oldRoot, SuperRoot
newRoot, long storageRevision);
+ CompletableFuture<Void> notify(SuperRoot oldRoot, SuperRoot newRoot,
long storageRevision);
Review comment:
Why have you removed @NotNull?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
##########
@@ -65,61 +63,54 @@
/** Root keys. */
private final Collection<RootKey<?, ?>> rootKeys;
- /** Validators. */
- private final Map<Class<? extends Annotation>, Set<Validator<? extends
Annotation, ?>>> validators;
+ /** Configuration change handler. */
+ private final ConfigurationChanger changer;
- /** Configuration storages. */
- private final Collection<ConfigurationStorage> configurationStorages;
-
- /** */
- private volatile ConfigurationChanger changer;
-
- /** */
+ /** Configuration generator. */
private final ConfigurationAsmGenerator cgen = new
ConfigurationAsmGenerator();
/**
* Constructor.
*
* @param rootKeys Configuration root keys.
* @param validators Validators.
- * @param configurationStorages Configuration Storages.
+ * @param storage Configuration storage.
+ * @throws IllegalArgumentException If the configuration type of the root
keys is not equal to the storage type.
*/
public ConfigurationRegistry(
Collection<RootKey<?, ?>> rootKeys,
Map<Class<? extends Annotation>, Set<Validator<? extends Annotation,
?>>> validators,
- Collection<ConfigurationStorage> configurationStorages
+ ConfigurationStorage storage
) {
+ checkConfigurationType(rootKeys, storage);
+
this.rootKeys = rootKeys;
- this.validators = validators;
- this.configurationStorages = configurationStorages;
- }
- /** {@inheritDoc} */
- @Override public void start() {
- this.changer = new ConfigurationChanger(this::notificator) {
+ Map<Class<? extends Annotation>, Set<Validator<? extends Annotation,
?>>> validators0 = new HashMap<>(validators);
+
+ validators0.put(Min.class, Set.of(new MinValidator()));
Review comment:
Technically this is a bug, set of validators for Min may already existed
and you just erased it
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationManager.java
##########
@@ -19,97 +19,85 @@
import java.lang.annotation.Annotation;
import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import com.typesafe.config.ConfigFactory;
import com.typesafe.config.ConfigObject;
import org.apache.ignite.configuration.RootKey;
-import org.apache.ignite.configuration.annotation.ConfigurationType;
import org.apache.ignite.configuration.validation.Validator;
import org.apache.ignite.internal.configuration.hocon.HoconConverter;
import org.apache.ignite.internal.configuration.storage.ConfigurationStorage;
import org.apache.ignite.internal.manager.IgniteComponent;
+import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.checkConfigurationType;
+
/**
* Configuration manager is responsible for handling configuration lifecycle
and provides configuration API.
*/
public class ConfigurationManager implements IgniteComponent {
/** Configuration registry. */
- private final ConfigurationRegistry confRegistry;
-
- /** Type mapped to configuration storage. */
- private final Map<ConfigurationType, ConfigurationStorage>
configurationStorages;
+ private final ConfigurationRegistry registry;
/**
- * The constructor.
+ * Constructor.
*
* @param rootKeys Configuration root keys.
* @param validators Validators.
- * @param configurationStorages Configuration storages.
+ * @param storage Configuration storage.
+ * @throws IllegalArgumentException If the configuration type of the root
keys is not equal to the storage type.
*/
public ConfigurationManager(
Collection<RootKey<?, ?>> rootKeys,
Map<Class<? extends Annotation>, Set<Validator<? extends Annotation,
?>>> validators,
- Collection<ConfigurationStorage> configurationStorages
+ ConfigurationStorage storage
) {
- HashMap<ConfigurationType, ConfigurationStorage> storageByType = new
HashMap<>();
+ checkConfigurationType(rootKeys, storage);
- for (ConfigurationStorage storage : configurationStorages) {
- assert !storageByType.containsKey(storage.type()) : "Two or more
storage have the same configuration type [type=" + storage.type() + ']';
-
- storageByType.put(storage.type(), storage);
- }
-
- this.configurationStorages = Map.copyOf(storageByType);
+ registry = new ConfigurationRegistry(rootKeys, validators, storage);
+ }
- confRegistry = new ConfigurationRegistry(rootKeys, validators,
configurationStorages);
+ /**
+ * Constructor.
+ *
+ * @param rootKeys Configuration root keys.
+ * @param storage Configuration storage.
+ * @throws IllegalArgumentException If the configuration type of the root
keys is not equal to the storage type.
+ */
+ public ConfigurationManager(Collection<RootKey<?, ?>> rootKeys,
ConfigurationStorage storage) {
Review comment:
Is it used somewhere? I saw that tests use empty map for validators,
right?
##########
File path:
modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java
##########
@@ -68,9 +71,10 @@
@BeforeEach
public void createRegistry() throws ExecutionException,
InterruptedException {
confRegistry = new ConfigurationRegistry(
- Collections.singleton(TablesConfiguration.KEY),
- Collections.singletonMap(TableValidator.class,
Collections.singleton(SchemaTableValidatorImpl.INSTANCE)),
- Collections.singleton(new
TestConfigurationStorage(ConfigurationType.DISTRIBUTED)));
+ List.of(TablesConfiguration.KEY),
+ Map.of(TableValidator.class,
Set.of(SchemaTableValidatorImpl.INSTANCE)),
Review comment:
Please add the same thing in IgnitionImpl :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
