exceptionfactory commented on code in PR #10668:
URL: https://github.com/apache/nifi/pull/10668#discussion_r2637154118
##########
nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/main/java/org/apache/nifi/registry/flow/diff/StandardFlowComparator.java:
##########
@@ -565,6 +565,18 @@ private void compare(final VersionedProcessGroup groupA,
final VersionedProcessG
final boolean shouldCompareVersioned =
flowCoordinateDifferences.stream()
.anyMatch(diff -> !diff.getFieldName().isPresent() ||
!diff.getFieldName().get().equals(FLOW_VERSION)) ||
flowComparatorVersionedStrategy == FlowComparatorVersionedStrategy.DEEP;
final boolean bothGroupsVersioned = groupACoordinates != null &&
groupBCoordinates != null;
+ final boolean bothGroupsVersionedAndSame = bothGroupsVersioned &&
flowCoordinateDifferences.isEmpty();
+
+ // When both groups are versioned with the SAME coordinates (same
registry, bucket, flow, and version),
+ // we should NOT compare their contents because the version being the
same implies the contents are identical.
+ // This is important because the component identifiers in the registry
snapshot may differ from local identifiers,
+ // which would incorrectly report components as added/removed even
though the PG is up-to-date.
+ // This also means that we will ignore any potential local
modifications in a nested versioned process group.
+ // In that case we expect the user to directly list changes on the
nested versioned process group.
+ if (bothGroupsVersionedAndSame) {
+ return;
+ }
Review Comment:
The thorough comment is helpful. I recommend avoiding the additional
variable declaration:
```suggestion
// When both groups are versioned with the SAME coordinates (same
registry, bucket, flow, and version),
// we should NOT compare their contents because the version being
the same implies the contents are identical.
// This is important because the component identifiers in the
registry snapshot may differ from local identifiers,
// which would incorrectly report components as added/removed even
though the PG is up-to-date.
// This also means that we will ignore any potential local
modifications in a nested versioned process group.
// In that case we expect the user to directly list changes on the
nested versioned process group.
if (bothGroupsVersioned && flowCoordinateDifferences.isEmpty()) {
return;
}
```
##########
nifi-registry/nifi-registry-core/nifi-registry-flow-diff/src/main/java/org/apache/nifi/registry/flow/diff/StandardFlowComparator.java:
##########
@@ -565,6 +565,18 @@ private void compare(final VersionedProcessGroup groupA,
final VersionedProcessG
final boolean shouldCompareVersioned =
flowCoordinateDifferences.stream()
Review Comment:
With the short-circuit return, it seems like this line should be moved down,
closer to where it is actually used.
--
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]
