SYNCT-98: Corrections after review.

api/src/main/java/org/openmrs/module/sync2/api/impl/SyncPullServiceImpl.java

@@ -42,7 +42,7 @@ public AuditMessage pullDataFromParentAndSave(String category, Map<String, Strin
         String localPull = getPullUrl(resourceLinks, clientName, CHILD);
         String localPush = getPushUrl(resourceLinks, clientName, CHILD);
 
-        boolean pulledObjectExist = false;
+        boolean pullToTheLocal = false;
         LOGGER.info(String.format("Pull category: %s, address: %s, action: %s", category, parentPull, action));
         String uuid = extractUUIDFromResourceLinks(resourceLinks);
 
@@ -56,11 +56,9 @@ public AuditMessage pullDataFromParentAndSave(String category, Map<String, Strin
 
         try {
             Object pulledObject = action.equals(ACTION_VOIDED) ? uuid : syncClient.pullData(category, clientName, parentPull, PARENT);
-            Object localPulledObject = syncClient.pullData(category, clientName, localPull, CHILD);
-            pulledObjectExist = pulledObject != null ?
-                    compareLocalAndPulled(clientName, category, pulledObject, localPulledObject) : true;
+            pullToTheLocal = shouldPullObject(pulledObject, category,clientName, localPull);
 
-            if (!pulledObjectExist) {
+            if (pullToTheLocal) {
                 syncClient.pushData(pulledObject, clientName, localPush, action, CHILD);
             }
 
@@ -72,7 +70,7 @@ public AuditMessage pullDataFromParentAndSave(String category, Map<String, Strin
             auditMessage.setSuccess(false);
             auditMessage.setDetails(ExceptionUtils.getFullStackTrace(e));
         } finally {
-            if (!pulledObjectExist) {
+            if (pullToTheLocal) {
                 auditMessage = syncAuditService.saveAuditMessage(auditMessage);
             }
         }
@@ -86,4 +84,19 @@ public AuditMessage pullDataFromParentAndSave(String category, Map<String, Strin
         return pullDataFromParentAndSave(category, resourceLinks, action, clientName);
     }
 
+    /**
+     *
+     * @param   pulledObject the object from the parent instance
+     * @param   category the category of the object. Represents name of the object class
+     * @param   clientName the name of the used client i.e. rest, fhir
+     * @param   url the url to pull local instance of the object
+     *
+     * @return  true if the parent and local objects are not equal.
+     *          false if the objects are equal or pulled object from the parent instance doesn't exists.
+     */
+    private boolean shouldPullObject(Object pulledObject, String category, String clientName, String url) {
+        Object localPulledObject = syncClient.pullData(category, clientName, url, CHILD);
+        return  pulledObject != null ? !compareLocalAndPulled(clientName, category, pulledObject, localPulledObject) : false;
+    }
+
 }

api/src/main/java/org/openmrs/module/sync2/api/impl/SyncPushServiceImpl.java

@@ -40,7 +40,7 @@ public AuditMessage readDataAndPushToParent(String category, Map<String, String>
         String localPull = getPullUrl(resourceLinks, clientName, CHILD);
         String parentPull = getPullUrl(resourceLinks, clientName, PARENT);
 
-        boolean pulledObjectExist = false;
+        boolean pushToTheParent = true;
         LOGGER.info(String.format("SyncPushService category: %s, address: %s, action: %s", category, parentPush, action));
         String uuid = extractUUIDFromResourceLinks(resourceLinks);
 
@@ -53,11 +53,9 @@ public AuditMessage readDataAndPushToParent(String category, Map<String, String>
 
         try {
             Object localObj = action.equals(ACTION_VOIDED) ? uuid : syncClient.pullData(category, clientName, localPull, CHILD);
-            Object parentObj = syncClient.pullData(category, clientName, parentPull, PARENT);
-            pulledObjectExist = localObj != null ?
-                    compareLocalAndPulled(clientName, category, localObj, parentObj) : true;
+            pushToTheParent = shouldPushObject(localObj, category, clientName, parentPull);
 
-            if (!pulledObjectExist) {
+            if (pushToTheParent) {
                 syncClient.pushData(localObj, clientName, parentPush, action, PARENT);
             }
 
@@ -68,7 +66,7 @@ public AuditMessage readDataAndPushToParent(String category, Map<String, String>
             auditMessage.setSuccess(false);
             auditMessage.setDetails(ExceptionUtils.getFullStackTrace(e));
         } finally {
-            if (!pulledObjectExist) {
+            if (pushToTheParent) {
                 auditMessage = syncAuditService.saveAuditMessage(auditMessage);
             }
         }
@@ -81,4 +79,19 @@ public AuditMessage readDataAndPushToParent(String category, Map<String, String>
         String clientName = SyncUtils.selectAppropriateClientName(resourceLinks);
         return readDataAndPushToParent(category, resourceLinks, action, clientName);
     }
+
+    /**
+     *
+     * @param   localObj the object from the local instance
+     * @param   category the category of the object. Represents name of the object class
+     * @param   clientName the name of the used client i.e. rest, fhir
+     * @param   url the url to pull parent instance of the object
+     *
+     * @return  true if the parent and local objects are not equal.
+     *          false if the objects are equal or pulled object from the local instance doesn't exists.
+     */
+    private boolean shouldPushObject(Object localObj, String category, String clientName, String url) {
+        Object parentObj = syncClient.pullData(category, clientName, url, PARENT);
+        return localObj != null ? !compareLocalAndPulled(clientName, category, localObj, parentObj) : false;
+    }
 }

api/src/main/java/org/openmrs/module/sync2/api/utils/SyncUtils.java

@@ -175,20 +175,23 @@ public static Class getClass(String client, String category) {
     public static boolean compareLocalAndPulled(String clientName, String category, Object from, Object dest) {
         boolean result = false;
 
-        if (null != dest && !(from instanceof String)) {
-            switch (clientName) {
-                case REST_CLIENT_KEY:
-                    RestResource restLocal = RestResourceCreationUtil.createRestResourceFromOpenMRSData((OpenmrsObject) dest);
-                    RestResource restPulled = RestResourceCreationUtil.createRestResourceFromOpenMRSData((OpenmrsObject) from);
-
-                    result = restLocal.equals(restPulled);
-                    break;
-                case FHIR_CLIENT_KEY:
-                    result = category.equals(CATEGORY_PATIENT) ?
-                            FHIRPatientUtil.compareCurrentPatients(dest, from) : dest.equals(from);
-                    break;
-                default:
-                    result = dest.equals(from);
+        if (null != dest && null != from) {
+            //If 'from' is  instance of String it represent uuid and should be used to delete object action.
+            if (!(from instanceof String)) {
+                switch (clientName) {
+                    case REST_CLIENT_KEY:
+                        RestResource obj1 = RestResourceCreationUtil.createRestResourceFromOpenMRSData((OpenmrsObject) dest);
+                        RestResource obj2 = RestResourceCreationUtil.createRestResourceFromOpenMRSData((OpenmrsObject) from);
+
+                        result = obj1.equals(obj2);
+                        break;
+                    case FHIR_CLIENT_KEY:
+                        result = category.equals(CATEGORY_PATIENT) ?
+                                FHIRPatientUtil.compareCurrentPatients(dest, from) : dest.equals(from);
+                        break;
+                    default:
+                        result = dest.equals(from);
+                }
             }
         }