gresockj commented on a change in pull request #5086:
URL: https://github.com/apache/nifi/pull/5086#discussion_r637882602
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/SnippetUtils.java
##########
@@ -299,62 +315,10 @@ public FlowSnippetDTO populateFlowSnippet(final Snippet
snippet, final boolean r
return snippetDto;
}
- /**
- * Finds all Controller Services that are referenced in the given Process
Group (and child Process Groups, recursively), and
- * adds them to the given servicesByGroup map
- *
- * @param group the Process Group to start from
- * @param dto the DTO representation of the Process Group
- * @param allServicesReferenced a Set of all Controller Service DTO's that
have already been referenced; used to dedupe services
- * @param contentsByGroup a Map of Process Group ID to the Process Group's
contents
- * @param highestGroupId the UUID of the 'highest' process group in the
snippet
- */
- private void addControllerServices(final ProcessGroup group, final
ProcessGroupDTO dto, final Set<ControllerServiceDTO> allServicesReferenced,
- final boolean includeControllerServices, final Set<String>
visitedGroupIds, final Map<String, FlowSnippetDTO> contentsByGroup, final
String highestGroupId) {
-
- final FlowSnippetDTO contents = dto.getContents();
- contentsByGroup.put(dto.getId(), contents);
- if (contents == null) {
- return;
- }
-
- // include this group in the ancestry for this snippet, services only
get included if the includeControllerServices
- // flag is set or if the service is defined within this groups
hierarchy within the snippet
- visitedGroupIds.add(group.getIdentifier());
-
- for (final ProcessorNode procNode : group.getProcessors()) {
- // Include all referenced services that are not already included
in this snippet.
-
getControllerServices(procNode.getEffectivePropertyValues()).stream()
- .filter(allServicesReferenced::add)
- .filter(svc -> includeControllerServices ||
visitedGroupIds.contains(svc.getParentGroupId()))
- .forEach(svc -> {
- final String svcGroupId = svc.getParentGroupId();
- final String destinationGroupId =
contentsByGroup.containsKey(svcGroupId) ? svcGroupId : highestGroupId;
- svc.setParentGroupId(destinationGroupId);
- final FlowSnippetDTO snippetDto =
contentsByGroup.get(destinationGroupId);
- if (snippetDto != null) {
- Set<ControllerServiceDTO> services =
snippetDto.getControllerServices();
- if (services == null) {
-
snippetDto.setControllerServices(Collections.singleton(svc));
- } else {
- services.add(svc);
- snippetDto.setControllerServices(services);
- }
- }
- });
- }
-
- // Map child process group ID to the child process group for easy
lookup
- final Map<String, ProcessGroupDTO> childGroupMap =
contents.getProcessGroups().stream()
- .collect(Collectors.toMap(ComponentDTO::getId, childGroupDto ->
childGroupDto));
-
- for (final ProcessGroup childGroup : group.getProcessGroups()) {
- final ProcessGroupDTO childDto =
childGroupMap.get(childGroup.getIdentifier());
- if (childDto == null) {
- continue;
- }
-
- addControllerServices(childGroup, childDto, allServicesReferenced,
includeControllerServices, visitedGroupIds, contentsByGroup, highestGroupId);
+ private void fillContentsByGroupMap(ProcessGroupDTO processGroup,
Map<String, FlowSnippetDTO> contentByGroupMap) {
+ for (ProcessGroupDTO group:
processGroup.getContents().getProcessGroups()) {
Review comment:
final group
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/SnippetUtils.java
##########
@@ -235,23 +234,41 @@ public FlowSnippetDTO populateFlowSnippet(final Snippet
snippet, final boolean r
}
// add any process groups
- final Set<ProcessGroupDTO> processGroups = new LinkedHashSet<>();
- if (!snippet.getProcessGroups().isEmpty()) {
- for (final String childGroupId :
snippet.getProcessGroups().keySet()) {
- final ProcessGroup childGroup =
processGroup.getProcessGroup(childGroupId);
- if (childGroup == null) {
- throw new IllegalStateException("A process group in this
snippet could not be found.");
- }
-
- final ProcessGroupDTO childGroupDto =
dtoFactory.createProcessGroupDto(childGroup, recurse);
- processGroups.add(childGroupDto);
-
- // maintain a listing of visited groups starting with each
group in the snippet. this is used to determine
- // whether a referenced controller service should be included
in the resulting snippet. if the service is
- // defined at groupId or one of it's ancestors, its considered
outside of this snippet and will only be included
- // when the includeControllerServices is set to true. this
happens above when considering the processors in this snippet
- final Set<String> visitedGroupIds = new HashSet<>();
- addControllerServices(childGroup, childGroupDto,
allServicesReferenced, includeControllerServices, visitedGroupIds,
contentsByGroup, processGroup.getIdentifier());
+ final ProcessGroupDTO highestProcessGroupDTO =
dtoFactory.createProcessGroupDto(processGroup, recurse);
+ final Set<ProcessGroupDTO> processGroups =
highestProcessGroupDTO.getContents().getProcessGroups();
+ fillContentsByGroupMap(highestProcessGroupDTO, contentsByGroup);
+
+ // maintain a listing of visited groups starting with each group in
the snippet. this is used to determine
Review comment:
For style purposes, can we capitalize each sentence?
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/SnippetUtils.java
##########
@@ -299,62 +315,10 @@ public FlowSnippetDTO populateFlowSnippet(final Snippet
snippet, final boolean r
return snippetDto;
}
- /**
- * Finds all Controller Services that are referenced in the given Process
Group (and child Process Groups, recursively), and
- * adds them to the given servicesByGroup map
- *
- * @param group the Process Group to start from
- * @param dto the DTO representation of the Process Group
- * @param allServicesReferenced a Set of all Controller Service DTO's that
have already been referenced; used to dedupe services
- * @param contentsByGroup a Map of Process Group ID to the Process Group's
contents
- * @param highestGroupId the UUID of the 'highest' process group in the
snippet
- */
- private void addControllerServices(final ProcessGroup group, final
ProcessGroupDTO dto, final Set<ControllerServiceDTO> allServicesReferenced,
- final boolean includeControllerServices, final Set<String>
visitedGroupIds, final Map<String, FlowSnippetDTO> contentsByGroup, final
String highestGroupId) {
-
- final FlowSnippetDTO contents = dto.getContents();
- contentsByGroup.put(dto.getId(), contents);
- if (contents == null) {
- return;
- }
-
- // include this group in the ancestry for this snippet, services only
get included if the includeControllerServices
- // flag is set or if the service is defined within this groups
hierarchy within the snippet
- visitedGroupIds.add(group.getIdentifier());
-
- for (final ProcessorNode procNode : group.getProcessors()) {
- // Include all referenced services that are not already included
in this snippet.
-
getControllerServices(procNode.getEffectivePropertyValues()).stream()
- .filter(allServicesReferenced::add)
- .filter(svc -> includeControllerServices ||
visitedGroupIds.contains(svc.getParentGroupId()))
- .forEach(svc -> {
- final String svcGroupId = svc.getParentGroupId();
- final String destinationGroupId =
contentsByGroup.containsKey(svcGroupId) ? svcGroupId : highestGroupId;
- svc.setParentGroupId(destinationGroupId);
- final FlowSnippetDTO snippetDto =
contentsByGroup.get(destinationGroupId);
- if (snippetDto != null) {
- Set<ControllerServiceDTO> services =
snippetDto.getControllerServices();
- if (services == null) {
-
snippetDto.setControllerServices(Collections.singleton(svc));
- } else {
- services.add(svc);
- snippetDto.setControllerServices(services);
- }
- }
- });
- }
-
- // Map child process group ID to the child process group for easy
lookup
- final Map<String, ProcessGroupDTO> childGroupMap =
contents.getProcessGroups().stream()
- .collect(Collectors.toMap(ComponentDTO::getId, childGroupDto ->
childGroupDto));
-
- for (final ProcessGroup childGroup : group.getProcessGroups()) {
- final ProcessGroupDTO childDto =
childGroupMap.get(childGroup.getIdentifier());
- if (childDto == null) {
- continue;
- }
-
- addControllerServices(childGroup, childDto, allServicesReferenced,
includeControllerServices, visitedGroupIds, contentsByGroup, highestGroupId);
+ private void fillContentsByGroupMap(ProcessGroupDTO processGroup,
Map<String, FlowSnippetDTO> contentByGroupMap) {
Review comment:
We can declare these parameters as final
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/util/SnippetUtils.java
##########
@@ -235,23 +234,41 @@ public FlowSnippetDTO populateFlowSnippet(final Snippet
snippet, final boolean r
}
// add any process groups
- final Set<ProcessGroupDTO> processGroups = new LinkedHashSet<>();
- if (!snippet.getProcessGroups().isEmpty()) {
- for (final String childGroupId :
snippet.getProcessGroups().keySet()) {
- final ProcessGroup childGroup =
processGroup.getProcessGroup(childGroupId);
- if (childGroup == null) {
- throw new IllegalStateException("A process group in this
snippet could not be found.");
- }
-
- final ProcessGroupDTO childGroupDto =
dtoFactory.createProcessGroupDto(childGroup, recurse);
- processGroups.add(childGroupDto);
-
- // maintain a listing of visited groups starting with each
group in the snippet. this is used to determine
- // whether a referenced controller service should be included
in the resulting snippet. if the service is
- // defined at groupId or one of it's ancestors, its considered
outside of this snippet and will only be included
- // when the includeControllerServices is set to true. this
happens above when considering the processors in this snippet
- final Set<String> visitedGroupIds = new HashSet<>();
- addControllerServices(childGroup, childGroupDto,
allServicesReferenced, includeControllerServices, visitedGroupIds,
contentsByGroup, processGroup.getIdentifier());
+ final ProcessGroupDTO highestProcessGroupDTO =
dtoFactory.createProcessGroupDto(processGroup, recurse);
+ final Set<ProcessGroupDTO> processGroups =
highestProcessGroupDTO.getContents().getProcessGroups();
+ fillContentsByGroupMap(highestProcessGroupDTO, contentsByGroup);
+
+ // maintain a listing of visited groups starting with each group in
the snippet. this is used to determine
+ // whether a referenced controller service should be included in the
resulting snippet. if the service is
+ // defined at groupId or one of it's ancestors, its considered outside
of this snippet and will only be included
+ // when the includeControllerServices is set to true. this happens
above when considering the processors in this snippet
+ final Set<String> visitedGroupIds = new HashSet<>();
+ for (String groupIdentifier : contentsByGroup.keySet()) {
Review comment:
Let's declare groupIdentifier as final
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]