From 871f2bfef227d9ac6e8f3d2c1a789d18fa23a564 Mon Sep 17 00:00:00 2001
From: Aron Roberts <aron@socrates.berkeley.edu>
Date: Fri, 14 Feb 2014 15:51:31 -0800
Subject: [PATCH] CSPACE-6308: Don't blank out computed current location value
 in a Cataloging record if the most recent movement has a blank current
 location value.

---
 .../listener/UpdateObjectLocationOnMove.java  | 34 +++++-----
 .../listener/listener-update-object-loc.xml   | 63 +++----------------
 2 files changed, 25 insertions(+), 72 deletions(-)

diff --git a/3rdparty/nuxeo/nuxeo-platform-listener/updateobjectlocationonmove/src/main/java/org/collectionspace/services/listener/UpdateObjectLocationOnMove.java b/3rdparty/nuxeo/nuxeo-platform-listener/updateobjectlocationonmove/src/main/java/org/collectionspace/services/listener/UpdateObjectLocationOnMove.java
index c7ff0a336..1b9b6f04b 100644
--- a/3rdparty/nuxeo/nuxeo-platform-listener/updateobjectlocationonmove/src/main/java/org/collectionspace/services/listener/UpdateObjectLocationOnMove.java
+++ b/3rdparty/nuxeo/nuxeo-platform-listener/updateobjectlocationonmove/src/main/java/org/collectionspace/services/listener/UpdateObjectLocationOnMove.java
@@ -1,6 +1,5 @@
 package org.collectionspace.services.listener;
 
-import java.io.Serializable;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.collectionspace.services.common.api.RefNameUtils;
@@ -37,11 +36,16 @@ public class UpdateObjectLocationOnMove extends AbstractUpdateObjectLocationValu
         // reference (refName) to an authority term (such as a storage
         // location or organization term):
         //
-        // * If it is not blank ...
-        // * Is then capable of being successfully parsed by an authority item parser.
-        if (Tools.notBlank(currentLocationRefName)
-                && RefNameUtils.parseAuthorityTermInfo(currentLocationRefName) == null) {
-            logger.warn("Could not parse current location refName '" + currentLocationRefName + "'");
+        // * Is not blank
+        // * Is capable of being successfully parsed by an authority item parser.
+        if (Tools.isBlank(currentLocationRefName)) {
+            if (logger.isTraceEnabled()) {
+                logger.trace("Current location in Movement record was blank");
+            }
+            return collectionObjectDocModel;
+        } else if (RefNameUtils.parseAuthorityTermInfo(currentLocationRefName) == null) {
+            logger.warn(String.format("Could not parse current location refName '%s' in Movement record",
+                    currentLocationRefName));
             return collectionObjectDocModel;
         } else {
             if (logger.isTraceEnabled()) {
@@ -59,22 +63,14 @@ public class UpdateObjectLocationOnMove extends AbstractUpdateObjectLocationValu
             logger.trace("Existing computedCurrentLocation refName=" + existingComputedCurrentLocationRefName);
         }
 
-        // If the new value is blank, any non-blank existing value should always
-        // be overwritten ('nulled out') with a blank value.
-        if (Tools.isBlank(currentLocationRefName) && Tools.notBlank(existingComputedCurrentLocationRefName)) {
-            collectionObjectDocModel.setProperty(COLLECTIONOBJECTS_COMMON_SCHEMA,
-                    COMPUTED_CURRENT_LOCATION_PROPERTY, (Serializable) null);
-            // Otherwise, if the new value is not blank, and
-            // * the existing value is blank, or
-            // * the new value is different than the existing value ...
-        } else if (Tools.notBlank(currentLocationRefName) &&
-                    (Tools.isBlank(existingComputedCurrentLocationRefName)
-                    || !currentLocationRefName.equals(existingComputedCurrentLocationRefName))) {
+        // If the new value is not blank (redundant with a check just above, but
+        // a quick, extra guard) and the new value is different than the existing value ...
+        if ( (Tools.notBlank(currentLocationRefName)) && (!currentLocationRefName.equals(existingComputedCurrentLocationRefName))) {
             if (logger.isTraceEnabled()) {
                 logger.trace("computedCurrentLocation refName requires updating.");
             }
-            // ... update the existing value in the CollectionObject with the
-            // new value from the Movement.
+            // ... update the existing value (in the CollectionObject) with the
+            // new value (from the Movement).
             collectionObjectDocModel.setProperty(COLLECTIONOBJECTS_COMMON_SCHEMA,
                     COMPUTED_CURRENT_LOCATION_PROPERTY, currentLocationRefName);
 
diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/listener/listener-update-object-loc.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/listener/listener-update-object-loc.xml
index fd1202e68..752390a51 100644
--- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/listener/listener-update-object-loc.xml
+++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/listener/listener-update-object-loc.xml
@@ -347,67 +347,24 @@
         
         <!--
             This test verifies that the computedCurrentLocation field
-            value has been set to null, because that field will *not*
-            be emitted in the response payload when it has a null value.
-            
-            This is an inherently fragile test, in comparing tree structures,
-            and will break if there are any changes to any of the relevant
-            schemas, including collectionspace_core, collectionobjects_common,
-            and account_permission.
-            
-            Uncomment only when working on the computed current location
-            event listener / handler. - ADR - 2013-02-14
+            value in the CollectionObject record will NOT be set to null,
+            if the most recent movement has an empty (blank) current location.
+            per CSPACE-6308.
         -->
-        <!--
         <test ID="readCollectionObject1AfterMovement3BlankCurrentLocationUpdate">
             <method>GET</method>
             <uri>/cspace-services/collectionobjects/${createCollectionObject1.CSID}</uri>
             <response>
-                <filename>listener/res/collectionobject-without-current-location.res.xml</filename>
-                <expected level="TREE" />
-                <parts>
-                    <part>
-                        <label>collectionobjects_common</label>
-                    </part>
-                </parts>
-            </response>
-            <expectedCodes>200</expectedCodes>
-        </test>
-        -->
-        
-        <test ID="updateMovement3WithBlankCurrentLocationSecondTime">
-            <method>PUT</method>
-            <uri>/cspace-services/movements/${createMovement2.CSID}</uri>
-            <filename>listener/movement.xml</filename>
-            <vars>
-                <var ID="currentLocation"></var>
-                <var ID="locationDate">1800-02-01</var>
-            </vars>
-            <expectedCodes>200</expectedCodes>
-        </test>
-        
-        <!--        
-            Uncomment only when working on the computed current location
-            event listener / handler. - ADR - 2013-02-14
-            
-            (See comments above for more details.)
-        -->
-        <!--
-        <test ID="readCollectionObject1AfterMovement3BlankCurrentLocationUpdateSecondTime">
-            <method>GET</method>
-            <uri>/cspace-services/collectionobjects/${createCollectionObject1.CSID}</uri>
-            <response>
-                <filename>listener/res/collectionobject-without-current-location.res.xml</filename>
-                <expected level="TREE" />
-                <parts>
-                    <part>
-                        <label>collectionobjects_common</label>
-                    </part>
-                </parts>
+                <expected level="ADDOK" />
+                <filename>listener/res/collectionobject.res.xml</filename>
+                <vars>
+                    <!-- The current location value most recently stored in this -->
+                    <!-- CollectionObject should still be present. -->
+                    <var ID="computedCurrentLocationValue">${updateMovement3.currentLocation}</var>
+                </vars>
             </response>
             <expectedCodes>200</expectedCodes>
         </test>
-        -->
         
         <test ID="updateMovement3WithNonBlankCurrentLocation">
             <method>PUT</method>
-- 
2.47.3