[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535547981
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/validation/MaxValidator.java
##
@@ -0,0 +1,42 @@
+/*
+ * 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.internal.validation;
+
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+
+/**
+ * Validate that field value is not greater than some maximum value.
+ *
+ * @param Root configuration type.
+ */
+public class MaxValidator> extends
FieldValidator {
+/** Maximum value. */
+private final long maxValue;
+
+/** Constructor. */
+public MaxValidator(long maxValue, String message) {
+super(message);
+this.maxValue = maxValue;
+}
+
+/** {@inheritDoc} */
+@Override public void validate(Number value, C newRoot, C oldRoot) {
+if (value.longValue() > maxValue)
+throw new ConfigurationValidationException(message);
Review comment:
If we make this exception checked, then every configuration API call like
```
local.baseline.enabled(false);
```
will have to be wrapped in try-catch with ConfigurationValidationException.
I think it for sure needs further discussion, but we can leave it as-is for
now, as changing runtime to checked is no biggie
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535440706
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/validation/FieldValidator.java
##
@@ -0,0 +1,46 @@
+/*
+ * 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.internal.validation;
+
+import java.io.Serializable;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+
+/**
+ * Base class for field validator. Contains exception message.
+ * @param Field type.
+ * @param Root configuration type.
+ */
+public abstract class FieldValidator> {
+/** Validation error message. */
+protected final String message;
+
+/** Constructor. */
+protected FieldValidator(String message) {
+this.message = message;
+}
+
+/**
+ * Validate field.
+ *
+ * @param value New value.
+ * @param newRoot New configuration root.
+ * @param oldRoot Old configuration root.
+ */
+public abstract void validate(T value, C newRoot, C oldRoot);
Review comment:
I decided to go with the runtime exception in the end (added it to the
signature and docs of course)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535440706
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/validation/FieldValidator.java
##
@@ -0,0 +1,46 @@
+/*
+ * 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.internal.validation;
+
+import java.io.Serializable;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+
+/**
+ * Base class for field validator. Contains exception message.
+ * @param Field type.
+ * @param Root configuration type.
+ */
+public abstract class FieldValidator> {
+/** Validation error message. */
+protected final String message;
+
+/** Constructor. */
+protected FieldValidator(String message) {
+this.message = message;
+}
+
+/**
+ * Validate field.
+ *
+ * @param value New value.
+ * @param newRoot New configuration root.
+ * @param oldRoot Old configuration root.
+ */
+public abstract void validate(T value, C newRoot, C oldRoot);
Review comment:
I decided to go with the checked exception (added it to the signature
and docs ofcourse)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5: URL: https://github.com/apache/ignite-3/pull/5#discussion_r535418386 ## File path: modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java ## @@ -0,0 +1,692 @@ +/* + * 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.processor.internal; Review comment: Reasonable, will do 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535417248
##
File path: modules/config-sample/pom.xml
##
@@ -0,0 +1,86 @@
+
+
+
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.ignite
+ignite-configuration-sample
+3.0-SNAPSHOT
+http://ignite.apache.org
+
+
+1.8
+1.8
+
+
+
+
+log4j
+log4j
+1.2.17
+test
+
+
+
+com.typesafe
+config
+1.4.1
+
+
+
+org.apache.ignite
+ignite-configuration
+${project.version}
+
+
+junit
+junit
+4.13
Review comment:
Changed to JUnit5, but this method is going to be removed (it's just a
demo), and also we don't really have a lot of options, as there is no single
parent pom or smth like that. AFAIK there is no decision on dependency
management yet, so I'd leave it be
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535414280
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/validation/FieldValidator.java
##
@@ -0,0 +1,46 @@
+/*
+ * 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.internal.validation;
+
+import java.io.Serializable;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+
+/**
+ * Base class for field validator. Contains exception message.
+ * @param Field type.
+ * @param Root configuration type.
+ */
+public abstract class FieldValidator> {
+/** Validation error message. */
+protected final String message;
+
+/** Constructor. */
+protected FieldValidator(String message) {
Review comment:
Yes, why?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535413713
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/selector/BaseSelectors.java
##
@@ -0,0 +1,125 @@
+/*
+ * 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.internal.selector;
+
+import java.lang.invoke.MethodHandle;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+import org.apache.ignite.configuration.internal.property.Modifier;
+
+/**
+ * Base selector holder.
+ */
+public class BaseSelectors {
+/** Map from string representation of selector to {@link SelectorHolder}.
*/
+private static final Map selectors = new
HashMap<>();
+
+/**
+ * Get selector from selectors map by key.
+ *
+ * Valid formats for selector key:
+ *
+ * root.inner.option.field in case of static config field
+ * root.inner.named[name].field in case of dynamic (named) config
field
+ *
+ *
+ * @param name Selector name.
+ * @return Selector.
+ */
+public static , B extends
Modifier, C, D, E> Selector find(String name) {
Review comment:
Changed to actual type names, but not sure how to format signature THAT
big
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535411746
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/selector/BaseSelectors.java
##
@@ -0,0 +1,125 @@
+/*
+ * 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.internal.selector;
+
+import java.lang.invoke.MethodHandle;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+import org.apache.ignite.configuration.internal.property.Modifier;
+
+/**
+ * Base selector holder.
+ */
+public class BaseSelectors {
+/** Map from string representation of selector to {@link SelectorHolder}.
*/
+private static final Map selectors = new
HashMap<>();
+
+/**
+ * Get selector from selectors map by key.
+ *
+ * Valid formats for selector key:
+ *
+ * root.inner.option.field in case of static config field
+ * root.inner.named[name].field in case of dynamic (named) config
field
+ *
+ *
+ * @param name Selector name.
+ * @return Selector.
+ */
+public static , B extends
Modifier, C, D, E> Selector find(String name) {
Review comment:
I personally used it to find selectors in the CLI tool demo. I'm not
sure if we can make it private though
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535408185
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/selector/BaseSelectors.java
##
@@ -0,0 +1,125 @@
+/*
+ * 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.internal.selector;
+
+import java.lang.invoke.MethodHandle;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+import org.apache.ignite.configuration.internal.property.Modifier;
+
+/**
+ * Base selector holder.
+ */
+public class BaseSelectors {
+/** Map from string representation of selector to {@link SelectorHolder}.
*/
+private static final Map selectors = new
HashMap<>();
+
+/**
+ * Get selector from selectors map by key.
+ *
+ * Valid formats for selector key:
+ *
+ * root.inner.option.field in case of static config field
+ * root.inner.named[name].field in case of dynamic (named) config
field
+ *
+ *
+ * @param name Selector name.
+ * @return Selector.
+ */
+public static , B extends
Modifier, C, D, E> Selector find(String name) {
+String[] splitten = name.split("\\.");
Review comment:
`true`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535407691
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/property/NamedList.java
##
@@ -0,0 +1,51 @@
+/*
+ * 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.internal.property;
+
+import java.util.Map;
+
+/**
+ * This class holds named configurations in VIEW object.
+ */
+public class NamedList {
+/** Named values. */
+private final Map values;
+
+/**
+ * Constructor.
+ * @param values Named values.
+ */
+public NamedList(Map values) {
+this.values = values;
+}
+
+/**
+ * Get named values.
+ * @return Named values.
+ */
+public Map getValues() {
+return values;
+}
+
+/** {@inheritDoc} */
+@Override public String toString() {
+return "NamedList{" +
Review comment:
We actually don't need that at all, I'm just going to remove it
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535405567
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/annotation/Validate.java
##
@@ -0,0 +1,77 @@
+/*
+ * 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.internal.annotation;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Repeatable;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import org.apache.ignite.configuration.internal.validation.FieldValidator;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.RetentionPolicy.SOURCE;
+
+/**
+ * This annotation applies custom validation to configuration field, for
example:
+ *
+ * public class ConfSchema {
+ * {@literal @}Validate(SomeCustomValidator.class)
+ * private String value;
+ * }
+ *
+ *
+ * If you need multiple custom validations:
+ *
+ * public class ConfSchema {
+ * {@literal @}Validate.List({
Review comment:
Yeah, right, I actually use the correct way to repeat annotations in the
example, just forgot about the doc
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5: URL: https://github.com/apache/ignite-3/pull/5#discussion_r535404023 ## File path: modules/configuration/src/main/java/org/apache/ignite/configuration/internal/annotation/Config.java ## @@ -0,0 +1,69 @@ +/* + * 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.internal.annotation; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import org.apache.ignite.configuration.internal.DynamicConfiguration; + +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.SOURCE; + +/** + * This annotation, if applied to a class, marks it as a configuration schema. + * Annotation processor generates a couple of classes for each configuration schema: Review comment: You're right 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535400429
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicConfiguration.java
##
@@ -0,0 +1,147 @@
+/*
+ * 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.internal;
+
+import java.io.Serializable;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.configuration.internal.property.DynamicProperty;
+import org.apache.ignite.configuration.internal.property.Modifier;
+import org.apache.ignite.configuration.internal.selector.BaseSelectors;
+import org.apache.ignite.configuration.internal.validation.FieldValidator;
+
+/**
+ * This class represents configuration root or node.
+ */
+public abstract class DynamicConfiguration implements
Modifier {
+/** Fully qualified name of the configuration. */
+protected final String qualifiedName;
+
+/** Configuration key. */
+protected final String key;
+
+/** Configuration prefix. */
+protected final String prefix;
+
+/** Configuration members (leafs and nodes). */
+protected final Map> members = new HashMap<>();
+
+/** Root configuration node. */
+protected final DynamicConfiguration root;
+
+/** {@code true} if this is a member of {@link NamedListConfiguration}. */
+protected final boolean isNamed;
+
+/** Configurator that this configuration is attached to. */
+protected final Configurator>
configurator;
+
+/**
+ * Constructor.
+ * @param prefix Configuration prefix.
+ * @param key Configuration key.
+ * @param isNamed Is this a part of named configuration.
+ * @param configurator Configurator that this object is attached to.
+ * @param root Root configuration.
+ */
+protected DynamicConfiguration(
+String prefix,
+String key,
+boolean isNamed,
+Configurator> configurator,
+DynamicConfiguration root
+) {
+this.prefix = prefix;
+this.isNamed = isNamed;
+this.configurator = configurator;
+
+this.key = key;
+if (root == null)
+this.qualifiedName = key;
+else {
+if (isNamed)
+qualifiedName = String.format("%s[%s]", prefix, key);
Review comment:
Actually, we can't move past this formatting, as we need to differ
static part of the name and dynamic
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535360759
##
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/Configurator.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.internal;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.ignite.configuration.internal.property.DynamicProperty;
+import org.apache.ignite.configuration.internal.property.Modifier;
+import org.apache.ignite.configuration.internal.property.PropertyListener;
+import org.apache.ignite.configuration.internal.selector.Selector;
+import org.apache.ignite.configuration.internal.validation.FieldValidator;
+import org.apache.ignite.configuration.internal.validation.MemberKey;
+
+public class Configurator> {
+
+private final ConfigurationStorage storage;
+
+private final T root;
+
+private final Map>>> fieldValidators = new HashMap<>();
+
+public Configurator(ConfigurationStorage storage,
Function, T> rootBuilder) {
+this.storage = storage;
+this.root = rootBuilder.apply(this);
+this.init();
+}
+
+private void init() {
+List props = new ArrayList<>();
+
+props.forEach(property -> {
Review comment:
Whoops, correct
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535360071
##
File path:
modules/configuration-annotation-processor/src/test/resources/org/apache/ignite/configuration/processor/internal/TestConfigurationSchema.java
##
@@ -0,0 +1,22 @@
+package org.apache.ignite.configuration.processor.internal;
+
+import org.apache.ignite.configuration.internal.annotation.Config;
+import org.apache.ignite.configuration.internal.annotation.Value;
+
+@Config(value = "test", root = true)
+public class TestConfigurationSchema {
+@Value
+private String value1;
+
+@Value
+private long primitiveLong;
+
+@Value
+private Long boxedLong;
+
+@Value
+private int primitiveInt;
+
+@Value
+private Integer boxedInt;
+}
Review comment:
This is a test java file in resources, I'm not sure if it should follow
our style guides as we only need in the test's runtime.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535350963
##
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/validation/ValidationGenerator.java
##
@@ -0,0 +1,111 @@
+/*
+ * 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.processor.internal.validation;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.MirroredTypesException;
+import javax.lang.model.type.TypeMirror;
+import javax.validation.constraints.Max;
+import javax.validation.constraints.Min;
+import javax.validation.constraints.NotNull;
+import com.squareup.javapoet.CodeBlock;
+import org.apache.ignite.configuration.internal.DynamicConfiguration;
+import org.apache.ignite.configuration.internal.annotation.Validate;
+import org.apache.ignite.configuration.internal.validation.MaxValidator;
+import org.apache.ignite.configuration.internal.validation.MinValidator;
+import org.apache.ignite.configuration.internal.validation.NotNullValidator;
+
+/**
+ * Class that handles validation generation.
+ */
+public class ValidationGenerator {
+/** Private constructor. */
+private ValidationGenerator() {
+}
+
+/**
+ * Generate validation block.
+ *
+ * @param variableElement Configuration field.
+ * @return Code block for field validation.
+ */
+public static CodeBlock generateValidators(VariableElement
variableElement) {
+List validators = new ArrayList<>();
+final Min minAnnotation = variableElement.getAnnotation(Min.class);
+if (minAnnotation != null) {
+final long minValue = minAnnotation.value();
+final String message = minAnnotation.message();
+final CodeBlock build = CodeBlock.builder().add("new $T<$T>($L, $S)", MinValidator.class, DynamicConfiguration.class, minValue,
message).build();
+validators.add(build);
+}
+
+final Max maxAnnotation = variableElement.getAnnotation(Max.class);
+if (maxAnnotation != null) {
+final long maxValue = maxAnnotation.value();
+final String message = maxAnnotation.message();
+final CodeBlock build = CodeBlock.builder().add("new $T<$T>($L, $S)", MaxValidator.class, DynamicConfiguration.class, maxValue,
message).build();
+validators.add(build);
+}
+
+final NotNull notNull = variableElement.getAnnotation(NotNull.class);
+if (notNull != null) {
+final String message = notNull.message();
+final CodeBlock build = CodeBlock.builder().add("new $T<$T>($S)", NotNullValidator.class, DynamicConfiguration.class, message).build();
+validators.add(build);
+}
+
+List validateAnnotations = new ArrayList<>();
+
+final Validate.List validateAnnotationsList =
variableElement.getAnnotation(Validate.List.class);
+
+if (validateAnnotationsList != null) {
+
validateAnnotations.addAll(Arrays.asList(validateAnnotationsList.value()));
+}
+
+final Validate validateAnnotationSingle =
variableElement.getAnnotation(Validate.class);
+
+if (validateAnnotationSingle != null) {
+validateAnnotations.add(validateAnnotationSingle);
+}
+
+validateAnnotations.forEach(validateAnnotation -> {
+List values = null;
+try {
+validateAnnotation.value();
+} catch (MirroredTypesException e) {
+values = e.getTypeMirrors();
Review comment:
Done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535251190
##
File path: modules/configuration-annotation-processor/pom.xml
##
@@ -0,0 +1,94 @@
+
+
+
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.ignite
+ignite-configuration-annotation-processor
+3.0-SNAPSHOT
+http://ignite.apache.org
+
+
+1.8
+1.8
+
+
+
+
+log4j
+log4j
+1.2.17
+test
+
+
+
+com.squareup
+javapoet
+1.13.0
+
+
+
+org.apache.ignite
+ignite-configuration
+${project.version}
+
+
+
+org.mockito
+mockito-core
+3.4.6
+test
+
+
+
+com.google.testing.compile
+compile-testing
+0.19
+test
+
+
+
+org.junit.jupiter
+junit-jupiter-engine
+5.6.2
+test
+
+
+
+fr.inria.gforge.spoon
+spoon-core
Review comment:
I'm not an expert either, but as I understand MIT license is permissive
and compatible with the Apache license. Also, we won't be distributing Spoon as
it's only a test dependency.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5: URL: https://github.com/apache/ignite-3/pull/5#discussion_r535248128 ## File path: modules/config-sample/src/main/java/org/apache/ignite/configuration/internal/AutoAdjustConfigurationSchema.java ## @@ -0,0 +1,43 @@ +/* + * 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.internal; Review comment: Sure, this module is just a demo 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5: URL: https://github.com/apache/ignite-3/pull/5#discussion_r535247519 ## File path: modules/configuration-annotation-processor/pom.xml ## @@ -0,0 +1,94 @@ + + + + + +http://maven.apache.org/POM/4.0.0; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd;> +4.0.0 + +org.apache.ignite +ignite-configuration-annotation-processor +3.0-SNAPSHOT +http://ignite.apache.org + + +1.8 Review comment: Done (but I hope we'll move to Gradle) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [ignite-3] SammyVimes commented on a change in pull request #5: IGNITE-13562 Unified configuration initial prototype
SammyVimes commented on a change in pull request #5:
URL: https://github.com/apache/ignite-3/pull/5#discussion_r535244939
##
File path: modules/config-sample/pom.xml
##
@@ -0,0 +1,86 @@
+
+
+
+
+
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+4.0.0
+
+org.apache.ignite
+ignite-configuration-sample
+3.0-SNAPSHOT
+http://ignite.apache.org
+
+
+1.8
+1.8
+
+
+
+
+log4j
+log4j
+1.2.17
+test
+
+
+
+com.typesafe
+config
+1.4.1
+
+
+
+org.apache.ignite
+ignite-configuration
+${project.version}
+
+
+junit
+junit
+4.13
+test
+
+
+
+
+
+
+org.apache.maven.plugins
+maven-compiler-plugin
+3.8.1
+
+
+
org.apache.ignite.configuration.processor.internal.Processor
Review comment:
Done
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
