Lehel44 commented on code in PR #6650:
URL: https://github.com/apache/nifi/pull/6650#discussion_r1022159847
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
+ * If this method is called multiple times, each with a different
dependency, then a relationship is established such that this Property is
relevant only if all dependencies are satisfied.
Review Comment:
```suggestion
* If this method is called multiple times with different
dependencies, a relationship is established so that the original Property is
only relevant if all dependencies are satisfied.
```
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
Review Comment:
The naming "this Property" and "given property" is not trivial. I suggest
renaming it to "dependent" or "original" property" and "conditional property"
even if it's not consistent with other Javadoc parts. If this is agreeable,
it'd apply to the remaining javadoc, too.
```suggestion
* Establishes a relationship between the original Property and the
conditional property by declaring that the original Property is only relevant
if the conditional Property value equals to one of the given
* {@link DescribedValue} arguments.
```
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
+ * If this method is called multiple times, each with a different
dependency, then a relationship is established such that this Property is
relevant only if all dependencies are satisfied.
+ *
+ * In the case that this property is NOT considered to be relevant
(meaning that it depends on a property whose value is not specified, or whose
value does not match one of the given
+ * Described Values), the property will not be shown in the
component's configuration in the User Interface. Additionally, this property's
value will not be considered for
+ * validation. That is, if this property is configured with an invalid
value and this property depends on Property Foo, and Property Foo does not have
a value set, then the component
+ * will still be valid, because the value of this property is
irrelevant.
+ *
+ * If the given property is not relevant (because its dependencies are
not satisfied), this property is also considered not to be valid.
+ *
+ * @param property the property that must be set in order for this
property to become relevant
Review Comment:
```suggestion
* @param property the property that must be set in order for this
property to become relevant
```
```suggestion
* @param property the conditional property
```
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
+ * If this method is called multiple times, each with a different
dependency, then a relationship is established such that this Property is
relevant only if all dependencies are satisfied.
+ *
+ * In the case that this property is NOT considered to be relevant
(meaning that it depends on a property whose value is not specified, or whose
value does not match one of the given
+ * Described Values), the property will not be shown in the
component's configuration in the User Interface. Additionally, this property's
value will not be considered for
+ * validation. That is, if this property is configured with an invalid
value and this property depends on Property Foo, and Property Foo does not have
a value set, then the component
+ * will still be valid, because the value of this property is
irrelevant.
+ *
+ * If the given property is not relevant (because its dependencies are
not satisfied), this property is also considered not to be valid.
+ *
+ * @param property the property that must be set in order for this
property to become relevant
+ * @param firstDependentValue the first value for the given property
for which this Property is relevant
Review Comment:
Please update the javadoc in case of renaming the parameters below.
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/notification/http/HttpNotificationService.java:
##########
@@ -127,14 +124,6 @@ public class HttpNotificationService extends
AbstractNotificationService {
.sensitive(true)
.build();
- public static final PropertyDescriptor SSL_ALGORITHM = new
PropertyDescriptor.Builder()
Review Comment:
Was this intentional?
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
+ * If this method is called multiple times, each with a different
dependency, then a relationship is established such that this Property is
relevant only if all dependencies are satisfied.
+ *
+ * In the case that this property is NOT considered to be relevant
(meaning that it depends on a property whose value is not specified, or whose
value does not match one of the given
+ * Described Values), the property will not be shown in the
component's configuration in the User Interface. Additionally, this property's
value will not be considered for
+ * validation. That is, if this property is configured with an invalid
value and this property depends on Property Foo, and Property Foo does not have
a value set, then the component
+ * will still be valid, because the value of this property is
irrelevant.
+ *
+ * If the given property is not relevant (because its dependencies are
not satisfied), this property is also considered not to be valid.
Review Comment:
This one is difficult to understand. AFAIK "relevant" means that a
property's dependencies are satisfied and it's displayed on the UI. If any of
the dependent properties are not relevant, for example:
`DummyProperty.
.dependsOn(dependentProperty1, DescribedValue1)
`
where `dependentProperty1` is also not relevant for some reason, does it
make DummyProperty **not relevant** or **invalid**?
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
+ * If this method is called multiple times, each with a different
dependency, then a relationship is established such that this Property is
relevant only if all dependencies are satisfied.
+ *
+ * In the case that this property is NOT considered to be relevant
(meaning that it depends on a property whose value is not specified, or whose
value does not match one of the given
+ * Described Values), the property will not be shown in the
component's configuration in the User Interface. Additionally, this property's
value will not be considered for
+ * validation. That is, if this property is configured with an invalid
value and this property depends on Property Foo, and Property Foo does not have
a value set, then the component
+ * will still be valid, because the value of this property is
irrelevant.
Review Comment:
```suggestion
* If this property is NOT considered relevant (it is dependent on a
property whose value is not specified or whose value does not match one of the
given
* Described Values), it will not be displayed in the component's
configuration on the User Interface. Additionally, the value of this property
will not be validated.
* That is, even if the value of this property is invalid but it
depends on Property Foo, which also does not have a value set, the component
* will still be valid since the value of this property is
irrelevant.
```
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -338,6 +337,23 @@ public Builder defaultValue(final String value) {
return this;
}
+ /**
+ * Specifies the initial value and the default value that will be used
+ * if the user does not specify a value. When {@link #build()} is
Review Comment:
```suggestion
* if no value is specified by the user. When {@link #build()} is
```
##########
nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java:
##########
@@ -591,6 +606,38 @@ public Builder dependsOn(final PropertyDescriptor
property, final String firstDe
return dependsOn(property, dependentValues);
}
+ /**
+ * Establishes a relationship between this Property and the given
property by declaring that this Property is only relevant if the given Property
has a value equal to one of the given
+ * {@link DescribedValue} arguments.
+ * If this method is called multiple times, each with a different
dependency, then a relationship is established such that this Property is
relevant only if all dependencies are satisfied.
+ *
+ * In the case that this property is NOT considered to be relevant
(meaning that it depends on a property whose value is not specified, or whose
value does not match one of the given
+ * Described Values), the property will not be shown in the
component's configuration in the User Interface. Additionally, this property's
value will not be considered for
+ * validation. That is, if this property is configured with an invalid
value and this property depends on Property Foo, and Property Foo does not have
a value set, then the component
+ * will still be valid, because the value of this property is
irrelevant.
+ *
+ * If the given property is not relevant (because its dependencies are
not satisfied), this property is also considered not to be valid.
+ *
+ * @param property the property that must be set in order for this
property to become relevant
+ * @param firstDependentValue the first value for the given property
for which this Property is relevant
+ * @param additionalDependentValues any other values for the given
property for which this Property is relevant
+ * @return the builder
+ */
+ public Builder dependsOn(final PropertyDescriptor property, final
DescribedValue firstDependentValue, final DescribedValue...
additionalDependentValues) {
Review Comment:
I think the original Property is dependent on the conditional properties,
I'd rename them to `firstConditionalValue` and `additionalConditionalValues`.
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/notification/http/HttpNotificationService.java:
##########
@@ -92,15 +92,13 @@ public class HttpNotificationService extends
AbstractNotificationService {
public static final PropertyDescriptor PROP_TRUSTSTORE_PASSWORD = new
PropertyDescriptor.Builder()
.name("Truststore Password")
.description("The password for the Truststore")
- .defaultValue(null)
Review Comment:
Does removing these null default values come from any of the current changes?
--
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]
