exceptionfactory commented on code in PR #7401:
URL: https://github.com/apache/nifi/pull/7401#discussion_r1245364933
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/WEB-INF/partials/canvas/import-flow-version-dialog.jsp:
##########
@@ -17,6 +17,11 @@
<%@ page contentType="text/html" pageEncoding="UTF-8" session="false" %>
<div id="import-flow-version-dialog" layout="column" class="hidden
large-dialog">
<div class="dialog-content">
+ <div class="setting keep-parameter-context">
+ <div id="keepExistingParameterContext" class="nf-checkbox
checkbox-checked"></div>
+ <div class="nf-checkbox-label">Keep Existing Parameter
Contexts</div>
+ <div class="fa fa-question-circle" alt="Info" title="When
unchecked, only directly associated Parameter Contexts will be copied,
inherited ones with no direct assignment on a Process Group are not subject of
this."></div>
Review Comment:
Recommend adjusting the wording:
```suggestion
<div class="fa fa-question-circle" alt="Info" title="When not
selected, only directly associated Parameter Contexts will be copied, inherited
Contexts with no direct assignment to a Process Group are ignored"></div>
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/ParameterContextNameCollusionResolver.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.util;
+
+import org.apache.nifi.web.api.dto.ParameterContextDTO;
+import org.apache.nifi.web.api.entity.ParameterContextEntity;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+class ParameterContextNameCollusionResolver {
+ private static final String LINEAGE_FORMAT = "^(?<name>.+?)(
\\((?<index>[0-9]+)\\))?$";
+ private static final Pattern LINEAGE_PATTERN =
Pattern.compile(LINEAGE_FORMAT);
+
+ ParameterContextNameCollusionResolver(final
Supplier<Collection<ParameterContextEntity>> parameterContextSource) {
+ this.parameterContextSource = parameterContextSource;
+ }
+
+ private final Supplier<Collection<ParameterContextEntity>>
parameterContextSource;
+
+ public String resolveNameCollusion(final String
originalParameterContextName) {
+ final Matcher lineageMatcher =
LINEAGE_PATTERN.matcher(originalParameterContextName);
+
+ if (!lineageMatcher.matches()) {
+ throw new IllegalArgumentException("Existing Parameter Context
name \"(" + originalParameterContextName + "\") cannot be processed");
+ }
+
+ final String lineName = lineageMatcher.group("name");
+ final String originalIndex = lineageMatcher.group("index");
Review Comment:
Recommend defining static variables for `name` and `index` and reusing
through this method.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/ParameterContextNameCollusionResolver.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.util;
+
+import org.apache.nifi.web.api.dto.ParameterContextDTO;
+import org.apache.nifi.web.api.entity.ParameterContextEntity;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+class ParameterContextNameCollusionResolver {
Review Comment:
`Collusion` needs to be replaced with `Collision` across all references.
```suggestion
class ParameterContextNameCollisionResolver {
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-flow-version.js:
##########
@@ -1045,7 +1048,8 @@
return $.ajax({
type: 'POST',
data: JSON.stringify(processGroupEntity),
- url: '../nifi-api/process-groups/' +
encodeURIComponent(nfCanvasUtils.getGroupId()) + '/process-groups',
+ url: '../nifi-api/process-groups/' +
encodeURIComponent(nfCanvasUtils.getGroupId()) + '/process-groups?'
+ + $.param({'keepExistingParameterContext' :
$('#keepExistingParameterContext').hasClass('checkbox-checked')}) ,
Review Comment:
Recommend assigning a value for checked or unchecked to make this easier to
read.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/ParameterContextNameCollusionResolver.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.util;
+
+import org.apache.nifi.web.api.dto.ParameterContextDTO;
+import org.apache.nifi.web.api.entity.ParameterContextEntity;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.function.Supplier;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+class ParameterContextNameCollusionResolver {
+ private static final String LINEAGE_FORMAT = "^(?<name>.+?)(
\\((?<index>[0-9]+)\\))?$";
+ private static final Pattern LINEAGE_PATTERN =
Pattern.compile(LINEAGE_FORMAT);
+
+ ParameterContextNameCollusionResolver(final
Supplier<Collection<ParameterContextEntity>> parameterContextSource) {
+ this.parameterContextSource = parameterContextSource;
+ }
+
+ private final Supplier<Collection<ParameterContextEntity>>
parameterContextSource;
+
+ public String resolveNameCollusion(final String
originalParameterContextName) {
+ final Matcher lineageMatcher =
LINEAGE_PATTERN.matcher(originalParameterContextName);
+
+ if (!lineageMatcher.matches()) {
+ throw new IllegalArgumentException("Existing Parameter Context
name \"(" + originalParameterContextName + "\") cannot be processed");
+ }
+
+ final String lineName = lineageMatcher.group("name");
+ final String originalIndex = lineageMatcher.group("index");
+
+ // Candidates cannot be cached because new context might be added
between calls
+ final Set<ParameterContextDTO> candidates =
parameterContextSource.get()
+ .stream()
+ .map(pc -> pc.getComponent())
+ .filter(dto -> dto.getName().startsWith(lineName))
+ .collect(Collectors.toSet());
+
+ int biggestIndex = (originalIndex == null) ? 0 :
Integer.valueOf(originalIndex);
+
+ for (final ParameterContextDTO candidate : candidates) {
+ final Matcher matcher =
LINEAGE_PATTERN.matcher(candidate.getName());
+
+ if (matcher.matches() && lineName.equals(matcher.group("name"))) {
+ final String indexGroup = matcher.group("index");
+
+ if (indexGroup != null) {
+ int biggestIndexCandidate = Integer.valueOf(indexGroup);
+
+ if (biggestIndexCandidate > biggestIndex) {
+ biggestIndex = biggestIndexCandidate;
+ }
+ }
+ }
+ }
+
+ return new StringBuilder(lineName).append(" (").append(biggestIndex +
1).append(')').toString();
Review Comment:
Recommend replacing `StringBuilder` with `String.format()` for better
readability.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java:
##########
@@ -1969,8 +1970,12 @@ public Response createProcessGroup(
@ApiParam(
value = "The process group configuration details.",
required = true
- ) final ProcessGroupEntity requestProcessGroupEntity) {
-
+ )
+ final ProcessGroupEntity requestProcessGroupEntity,
+ @QueryParam("keepExistingParameterContext")
+ @DefaultValue("true")
+ final boolean keepExistingParameterContext
Review Comment:
In terms of supporting potential API evolution, what do you think of making
this an `enum` named `ParameterContextHandlingStrategy`? The default value
could be `KEEP_EXISTING`, and the alternative could be `REPLACE` or similar?
That would provide more flexibility and also help clarifying the options.
--
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]
