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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org