[GitHub] [nifi] thenatog commented on a diff in pull request #6154: NIFI-10070 - Updated merging of status DTO for ControllerService and …

[GitBox <gi...@apache.org>]

thenatog commented on code in PR #6154:
URL: https://github.com/apache/nifi/pull/6154#discussion_r931289222


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/ReportingTaskEntityMerger.java:
##########
@@ -29,6 +29,17 @@
 
 public class ReportingTaskEntityMerger implements ComponentEntityMerger<ReportingTaskEntity> {
 
+    @Override
+    public void merge(ReportingTaskEntity clientEntity, Map<NodeIdentifier, ReportingTaskEntity> entityMap) {
+        ComponentEntityMerger.super.merge(clientEntity, entityMap);
+        for (Map.Entry<NodeIdentifier, ReportingTaskEntity> entry : entityMap.entrySet()) {
+            final ReportingTaskEntity entityStatus = entry.getValue();
+            if (clientEntity != entityStatus) {

Review Comment:
   I tested this out, I found that .equals() will likely return true because the object contents are very often the same, however the check was intended to skip only the clientEntity object that is inside the entityMap so we don't merge the clientEntity with itself and it uses the same reference in that map. So whilst the objects are the same, we don't want to skip over them in the case of merging their active threads for example.
   
   Let me know if this sounds wrong.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org