exceptionfactory commented on code in PR #8005:
URL: https://github.com/apache/nifi/pull/8005#discussion_r1435301326
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateJson.java:
##########
@@ -79,65 +92,69 @@
}
)
public class ValidateJson extends AbstractProcessor {
- public enum SchemaVersion implements DescribedValue {
- DRAFT_4("Draft Version 4", "Draft 4", VersionFlag.V4),
- DRAFT_6("Draft Version 6", "Draft 6", VersionFlag.V6),
- DRAFT_7("Draft Version 7", "Draft 7", VersionFlag.V7),
- DRAFT_2019_09("Draft Version 2019-09", "Draft 2019-09",
VersionFlag.V201909),
- DRAFT_2020_12("Draft Version 2020-12", "Draft 2020-12",
VersionFlag.V202012);
-
- private final String description;
- private final String displayName;
- private final VersionFlag versionFlag;
-
- SchemaVersion(String description, String displayName, VersionFlag
versionFlag) {
- this.description = description;
- this.displayName = displayName;
- this.versionFlag = versionFlag;
- }
- @Override
- public String getValue() {
- return name();
- }
+ public static final String SCHEMA_NAME_PROPERTY_NAME = "Schema Name";
+ public static final String SCHEMA_CONTENT_PROPERTY_NAME = "JSON Schema";
+ public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final AllowableValue SCHEMA_NAME_PROPERTY = new
AllowableValue("schema-name", "Use '" + SCHEMA_NAME_PROPERTY_NAME + "'
Property",
+ "The name of the Schema to use is specified by the '" +
SCHEMA_NAME_PROPERTY_NAME +
+ "' Property. The value of this property is used to lookup
the Schema in the configured JSON Schema Registry service.");
+ public static final AllowableValue SCHEMA_CONTENT_PROPERTY = new
AllowableValue("schema-content-property", "Use '" +
SCHEMA_CONTENT_PROPERTY_NAME + "' Property",
+ "A URL or file path to the JSON schema or the actual JSON schema
is specified by the '" + SCHEMA_CONTENT_PROPERTY_NAME + "' Property. " +
+ "No matter how the JSON schema is specified, it must be a
valid JSON schema");
Review Comment:
It would be helpful to switch from the `AllowableValue` class definition to
an enum that implements `DescribedValue`, making it easier to work with
selected values in the component itself.
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateJson.java:
##########
@@ -79,65 +92,69 @@
}
)
public class ValidateJson extends AbstractProcessor {
- public enum SchemaVersion implements DescribedValue {
- DRAFT_4("Draft Version 4", "Draft 4", VersionFlag.V4),
- DRAFT_6("Draft Version 6", "Draft 6", VersionFlag.V6),
- DRAFT_7("Draft Version 7", "Draft 7", VersionFlag.V7),
- DRAFT_2019_09("Draft Version 2019-09", "Draft 2019-09",
VersionFlag.V201909),
- DRAFT_2020_12("Draft Version 2020-12", "Draft 2020-12",
VersionFlag.V202012);
-
- private final String description;
- private final String displayName;
- private final VersionFlag versionFlag;
-
- SchemaVersion(String description, String displayName, VersionFlag
versionFlag) {
- this.description = description;
- this.displayName = displayName;
- this.versionFlag = versionFlag;
- }
- @Override
- public String getValue() {
- return name();
- }
+ public static final String SCHEMA_NAME_PROPERTY_NAME = "Schema Name";
+ public static final String SCHEMA_CONTENT_PROPERTY_NAME = "JSON Schema";
+ public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final AllowableValue SCHEMA_NAME_PROPERTY = new
AllowableValue("schema-name", "Use '" + SCHEMA_NAME_PROPERTY_NAME + "'
Property",
+ "The name of the Schema to use is specified by the '" +
SCHEMA_NAME_PROPERTY_NAME +
+ "' Property. The value of this property is used to lookup
the Schema in the configured JSON Schema Registry service.");
+ public static final AllowableValue SCHEMA_CONTENT_PROPERTY = new
AllowableValue("schema-content-property", "Use '" +
SCHEMA_CONTENT_PROPERTY_NAME + "' Property",
+ "A URL or file path to the JSON schema or the actual JSON schema
is specified by the '" + SCHEMA_CONTENT_PROPERTY_NAME + "' Property. " +
+ "No matter how the JSON schema is specified, it must be a
valid JSON schema");
- @Override
- public String getDisplayName() {
- return displayName;
- }
+ private static final List<AllowableValue> STRATEGY_LIST =
Arrays.asList(SCHEMA_NAME_PROPERTY, SCHEMA_CONTENT_PROPERTY);
- @Override
- public String getDescription() {
- return description;
- }
+ public static final PropertyDescriptor SCHEMA_ACCESS_STRATEGY = new
PropertyDescriptor.Builder()
+ .name("schema-access-strategy")
+ .displayName("Schema Access Strategy")
Review Comment:
For new property values, it is best to keep the `name` and `displayName` the
same.
```suggestion
.name("Schema Access Strategy")
.displayName("Schema Access Strategy")
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateJson.java:
##########
@@ -200,6 +248,20 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
return;
}
+ if(isNameStrategy(context)) {
Review Comment:
Spacing:
```suggestion
if (isNameStrategy(context)) {
```
##########
nifi-commons/nifi-json-schema-api/src/main/java/org/apache/nifi/schema/access/JsonSchema.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.nifi.schema.access;
Review Comment:
Based on renaming of the modules, it seems like it would be best to rename
this package:
```suggestion
package org.apache.nifi.json.schema;
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateJson.java:
##########
@@ -79,65 +92,69 @@
}
)
public class ValidateJson extends AbstractProcessor {
- public enum SchemaVersion implements DescribedValue {
- DRAFT_4("Draft Version 4", "Draft 4", VersionFlag.V4),
- DRAFT_6("Draft Version 6", "Draft 6", VersionFlag.V6),
- DRAFT_7("Draft Version 7", "Draft 7", VersionFlag.V7),
- DRAFT_2019_09("Draft Version 2019-09", "Draft 2019-09",
VersionFlag.V201909),
- DRAFT_2020_12("Draft Version 2020-12", "Draft 2020-12",
VersionFlag.V202012);
-
- private final String description;
- private final String displayName;
- private final VersionFlag versionFlag;
-
- SchemaVersion(String description, String displayName, VersionFlag
versionFlag) {
- this.description = description;
- this.displayName = displayName;
- this.versionFlag = versionFlag;
- }
- @Override
- public String getValue() {
- return name();
- }
+ public static final String SCHEMA_NAME_PROPERTY_NAME = "Schema Name";
+ public static final String SCHEMA_CONTENT_PROPERTY_NAME = "JSON Schema";
+ public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final AllowableValue SCHEMA_NAME_PROPERTY = new
AllowableValue("schema-name", "Use '" + SCHEMA_NAME_PROPERTY_NAME + "'
Property",
+ "The name of the Schema to use is specified by the '" +
SCHEMA_NAME_PROPERTY_NAME +
+ "' Property. The value of this property is used to lookup
the Schema in the configured JSON Schema Registry service.");
+ public static final AllowableValue SCHEMA_CONTENT_PROPERTY = new
AllowableValue("schema-content-property", "Use '" +
SCHEMA_CONTENT_PROPERTY_NAME + "' Property",
+ "A URL or file path to the JSON schema or the actual JSON schema
is specified by the '" + SCHEMA_CONTENT_PROPERTY_NAME + "' Property. " +
+ "No matter how the JSON schema is specified, it must be a
valid JSON schema");
- @Override
- public String getDisplayName() {
- return displayName;
- }
+ private static final List<AllowableValue> STRATEGY_LIST =
Arrays.asList(SCHEMA_NAME_PROPERTY, SCHEMA_CONTENT_PROPERTY);
- @Override
- public String getDescription() {
- return description;
- }
+ public static final PropertyDescriptor SCHEMA_ACCESS_STRATEGY = new
PropertyDescriptor.Builder()
+ .name("schema-access-strategy")
+ .displayName("Schema Access Strategy")
+ .description("Specifies how to obtain the schema that is to be
used for interpreting the data.")
+ .allowableValues(STRATEGY_LIST.toArray(new AllowableValue[0]))
+ .defaultValue(SCHEMA_CONTENT_PROPERTY.getValue())
+ .required(true)
+ .build();
- public VersionFlag getVersionFlag() {
- return versionFlag;
- }
- }
+ public static final PropertyDescriptor SCHEMA_NAME = new
PropertyDescriptor.Builder()
+ .name("schema-name")
+ .displayName(SCHEMA_NAME_PROPERTY_NAME)
+ .description("Specifies the name of the schema to lookup in the
Schema Registry property")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .defaultValue("${schema.name}")
+ .dependsOn(SCHEMA_ACCESS_STRATEGY, SCHEMA_NAME_PROPERTY)
+ .required(false)
+ .build();
- public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final PropertyDescriptor SCHEMA_REGISTRY = new
PropertyDescriptor.Builder()
+ .name("schema-registry")
+ .displayName("Schema Registry")
+ .description("Specifies the Controller Service to use for the
Schema Registry")
Review Comment:
Recommend adding the word `JSON` to help avoid confusion with other Schema
Registries.
```suggestion
.description("Specifies the Controller Service to use for the
JSON Schema Registry")
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateJson.java:
##########
@@ -79,65 +92,69 @@
}
)
public class ValidateJson extends AbstractProcessor {
- public enum SchemaVersion implements DescribedValue {
- DRAFT_4("Draft Version 4", "Draft 4", VersionFlag.V4),
- DRAFT_6("Draft Version 6", "Draft 6", VersionFlag.V6),
- DRAFT_7("Draft Version 7", "Draft 7", VersionFlag.V7),
- DRAFT_2019_09("Draft Version 2019-09", "Draft 2019-09",
VersionFlag.V201909),
- DRAFT_2020_12("Draft Version 2020-12", "Draft 2020-12",
VersionFlag.V202012);
-
- private final String description;
- private final String displayName;
- private final VersionFlag versionFlag;
-
- SchemaVersion(String description, String displayName, VersionFlag
versionFlag) {
- this.description = description;
- this.displayName = displayName;
- this.versionFlag = versionFlag;
- }
- @Override
- public String getValue() {
- return name();
- }
+ public static final String SCHEMA_NAME_PROPERTY_NAME = "Schema Name";
+ public static final String SCHEMA_CONTENT_PROPERTY_NAME = "JSON Schema";
+ public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final AllowableValue SCHEMA_NAME_PROPERTY = new
AllowableValue("schema-name", "Use '" + SCHEMA_NAME_PROPERTY_NAME + "'
Property",
+ "The name of the Schema to use is specified by the '" +
SCHEMA_NAME_PROPERTY_NAME +
+ "' Property. The value of this property is used to lookup
the Schema in the configured JSON Schema Registry service.");
+ public static final AllowableValue SCHEMA_CONTENT_PROPERTY = new
AllowableValue("schema-content-property", "Use '" +
SCHEMA_CONTENT_PROPERTY_NAME + "' Property",
+ "A URL or file path to the JSON schema or the actual JSON schema
is specified by the '" + SCHEMA_CONTENT_PROPERTY_NAME + "' Property. " +
+ "No matter how the JSON schema is specified, it must be a
valid JSON schema");
- @Override
- public String getDisplayName() {
- return displayName;
- }
+ private static final List<AllowableValue> STRATEGY_LIST =
Arrays.asList(SCHEMA_NAME_PROPERTY, SCHEMA_CONTENT_PROPERTY);
- @Override
- public String getDescription() {
- return description;
- }
+ public static final PropertyDescriptor SCHEMA_ACCESS_STRATEGY = new
PropertyDescriptor.Builder()
+ .name("schema-access-strategy")
+ .displayName("Schema Access Strategy")
+ .description("Specifies how to obtain the schema that is to be
used for interpreting the data.")
+ .allowableValues(STRATEGY_LIST.toArray(new AllowableValue[0]))
Review Comment:
Switching to `DescribedValue` will also allow providing a class reference to
the enum instead of this approach.
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ValidateJson.java:
##########
@@ -79,65 +92,69 @@
}
)
public class ValidateJson extends AbstractProcessor {
- public enum SchemaVersion implements DescribedValue {
- DRAFT_4("Draft Version 4", "Draft 4", VersionFlag.V4),
- DRAFT_6("Draft Version 6", "Draft 6", VersionFlag.V6),
- DRAFT_7("Draft Version 7", "Draft 7", VersionFlag.V7),
- DRAFT_2019_09("Draft Version 2019-09", "Draft 2019-09",
VersionFlag.V201909),
- DRAFT_2020_12("Draft Version 2020-12", "Draft 2020-12",
VersionFlag.V202012);
-
- private final String description;
- private final String displayName;
- private final VersionFlag versionFlag;
-
- SchemaVersion(String description, String displayName, VersionFlag
versionFlag) {
- this.description = description;
- this.displayName = displayName;
- this.versionFlag = versionFlag;
- }
- @Override
- public String getValue() {
- return name();
- }
+ public static final String SCHEMA_NAME_PROPERTY_NAME = "Schema Name";
+ public static final String SCHEMA_CONTENT_PROPERTY_NAME = "JSON Schema";
+ public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final AllowableValue SCHEMA_NAME_PROPERTY = new
AllowableValue("schema-name", "Use '" + SCHEMA_NAME_PROPERTY_NAME + "'
Property",
+ "The name of the Schema to use is specified by the '" +
SCHEMA_NAME_PROPERTY_NAME +
+ "' Property. The value of this property is used to lookup
the Schema in the configured JSON Schema Registry service.");
+ public static final AllowableValue SCHEMA_CONTENT_PROPERTY = new
AllowableValue("schema-content-property", "Use '" +
SCHEMA_CONTENT_PROPERTY_NAME + "' Property",
+ "A URL or file path to the JSON schema or the actual JSON schema
is specified by the '" + SCHEMA_CONTENT_PROPERTY_NAME + "' Property. " +
+ "No matter how the JSON schema is specified, it must be a
valid JSON schema");
- @Override
- public String getDisplayName() {
- return displayName;
- }
+ private static final List<AllowableValue> STRATEGY_LIST =
Arrays.asList(SCHEMA_NAME_PROPERTY, SCHEMA_CONTENT_PROPERTY);
- @Override
- public String getDescription() {
- return description;
- }
+ public static final PropertyDescriptor SCHEMA_ACCESS_STRATEGY = new
PropertyDescriptor.Builder()
+ .name("schema-access-strategy")
+ .displayName("Schema Access Strategy")
+ .description("Specifies how to obtain the schema that is to be
used for interpreting the data.")
+ .allowableValues(STRATEGY_LIST.toArray(new AllowableValue[0]))
+ .defaultValue(SCHEMA_CONTENT_PROPERTY.getValue())
+ .required(true)
+ .build();
- public VersionFlag getVersionFlag() {
- return versionFlag;
- }
- }
+ public static final PropertyDescriptor SCHEMA_NAME = new
PropertyDescriptor.Builder()
+ .name("schema-name")
+ .displayName(SCHEMA_NAME_PROPERTY_NAME)
+ .description("Specifies the name of the schema to lookup in the
Schema Registry property")
+ .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
+ .defaultValue("${schema.name}")
+ .dependsOn(SCHEMA_ACCESS_STRATEGY, SCHEMA_NAME_PROPERTY)
+ .required(false)
+ .build();
- public static final String ERROR_ATTRIBUTE_KEY = "json.validation.errors";
+ public static final PropertyDescriptor SCHEMA_REGISTRY = new
PropertyDescriptor.Builder()
+ .name("schema-registry")
+ .displayName("Schema Registry")
Review Comment:
For differentiation, it is probably best to include `JSON` in the property
name:
```suggestion
.name("JSON Schema Registry")
.displayName("JSON Schema Registry")
```
--
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]
