exceptionfactory commented on code in PR #7191:
URL: https://github.com/apache/nifi/pull/7191#discussion_r1260220993
##########
nifi-api/src/main/java/org/apache/nifi/flow/VersionedPropertyDescriptor.java:
##########
@@ -62,6 +63,15 @@ public void setSensitive(boolean sensitive) {
this.sensitive = sensitive;
}
+ @ApiModelProperty("Whether or not the property is user-defined")
+ public boolean isDynamic() {
+ return dynamic;
+ }
+
+ public void setDynamic(boolean dynamic) {
Review Comment:
Can you provide some background on this addition? The current framework
approach does not require this dynamic status to be persisted. If this is
necessary, it may be best to split off and implement separately.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/registry/flow/mapping/NiFiRegistryFlowMapper.java:
##########
@@ -567,7 +589,7 @@ private Map<String, String> mapProperties(final
ComponentNode component, final C
return mapped;
}
- private String encrypt(final String value) {
+ protected String encrypt(final String value) {
Review Comment:
Is there a reason for changing the visibility on this method?
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-rules/src/main/java/org/apache/nifi/flowanalysis/rules/DisallowComponentType.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.flowanalysis.rules;
+
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.flow.VersionedComponent;
+import org.apache.nifi.flow.VersionedExtensionComponent;
+import org.apache.nifi.flowanalysis.AbstractFlowAnalysisRule;
+import org.apache.nifi.flowanalysis.ComponentAnalysisResult;
+import org.apache.nifi.flowanalysis.FlowAnalysisRuleContext;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+
+@Tags({"component", "processor", "controller service", "type"})
+@CapabilityDescription("Produces rule violations for each component (i.e.
processors or controller services) of a given type.")
+public class DisallowComponentType extends AbstractFlowAnalysisRule {
+ public static final PropertyDescriptor COMPONENT_TYPE = new
PropertyDescriptor.Builder()
+ .name("component-type")
+ .displayName("Component Type")
+ .description("Components of the given type will produce a rule
violation (i.e. they shouldn't exist).")
+ .required(true)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+ .defaultValue(null)
+ .build();
+
+ private final static List<PropertyDescriptor> propertyDescriptors;
+
+ static {
+ List<PropertyDescriptor> _propertyDescriptors = new ArrayList<>();
Review Comment:
The `_` character should be avoided as the first character in variable
names. Recommend just `descriptors`, or a wrapped declaration like
`Collections.unmodifiableList(Arrays.asList(...))`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/validation/StandardRuleViolationsManager.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * 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.validation;
+
+import org.apache.nifi.flow.VersionedComponent;
+import org.apache.nifi.flow.VersionedProcessGroup;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.stream.Collectors;
+
Review Comment:
Given the complexity of some operations in this class, it would be helpful
to add some basic method comments describe the basic implementation approach.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/FlowAnalysisRuleViolationDTO.java:
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.web.api.dto;
+
+import javax.xml.bind.annotation.XmlType;
+
+/**
+ * A result of a rule violation produced during a flow analysis
+ */
+@XmlType(name = "flowAnalysisResult")
+public class FlowAnalysisRuleViolationDTO {
Review Comment:
Recommend adjusting either the XmlType name or the class name to be similar,
such as naming the class `FlowAnalysisResultDTO`
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-rules/src/main/java/org/apache/nifi/flowanalysis/rules/DisallowComponentType.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.flowanalysis.rules;
+
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.flow.VersionedComponent;
+import org.apache.nifi.flow.VersionedExtensionComponent;
+import org.apache.nifi.flowanalysis.AbstractFlowAnalysisRule;
+import org.apache.nifi.flowanalysis.ComponentAnalysisResult;
+import org.apache.nifi.flowanalysis.FlowAnalysisRuleContext;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+
+@Tags({"component", "processor", "controller service", "type"})
+@CapabilityDescription("Produces rule violations for each component (i.e.
processors or controller services) of a given type.")
+public class DisallowComponentType extends AbstractFlowAnalysisRule {
+ public static final PropertyDescriptor COMPONENT_TYPE = new
PropertyDescriptor.Builder()
+ .name("component-type")
+ .displayName("Component Type")
+ .description("Components of the given type will produce a rule
violation (i.e. they shouldn't exist).")
Review Comment:
The description should indicate that the Component Type can be a full class
name, or a short class name. Matching both may be too far-reaching, as it seems
like it be better to just match the full class name, avoiding the possibility
and matching components with the same name, but different packages. This could
occur if there is a custom subclass that should be preferred.
--
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]
