Calling Storable.update when there are no dirty properties actually does an update instead of being "smart" and ignoring the update request. The old behavior was non-intuitive and interfered with expected trigger behavior.

3 files changed, 28 insertions, 104 deletions

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index d8c31bb..eadcc09 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -32,6 +32,9 @@ Carbonado change history
 - Added trigger support for after loads and queries.

 - Removed vestigial support for wrapping Storables.

 - Storable toString and toStringKeyOnly methods skip uninitialized properties.

+- Calling Storable.update when there are no dirty properties actually does an

+  update instead of being "smart" and ignoring the update request. The old

+  behavior was non-intuitive and interfered with expected trigger behavior.

 

 1.1 to 1.1.2

 -------------------------------

diff --git a/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java b/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java
index ea61306..6a66528 100644
--- a/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java
+++ b/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java
@@ -1314,52 +1314,6 @@ public final class StorableGenerator<S extends Storable> {
             Label tryStart = addGetTriggerAndEnterTxn

                 (b, UPDATE_OP, forTryVar, false, triggerVar, txnVar, stateVar);

 

-            // If no properties are dirty, then don't update.

-            Label doUpdate = b.createLabel();

-            branchIfDirty(b, true, doUpdate);

-

-            // Even though there was no update, still need tryLoad side-effect.

-            {

-                // if (txn != null) {

-                //     txn.exit();

-                // }

-                b.loadLocal(txnVar);

-                Label noTxn = b.createLabel();

-                b.ifNullBranch(noTxn, true);

-                b.loadLocal(txnVar);

-                b.invokeInterface(transactionType, EXIT_METHOD_NAME, null, null);

-                noTxn.setLocation();

-

-                Label tryStart2 = b.createLabel().setLocation();

-                b.loadThis();

-                b.invokeVirtual(DO_TRY_LOAD_METHOD_NAME, TypeDesc.BOOLEAN, null);

-

-                Label notUpdated = b.createLabel();

-                b.ifZeroComparisonBranch(notUpdated, "==");

-

-                // Only mark properties clean if doTryLoad returned true.

-                b.loadThis();

-                b.invokeVirtual(MARK_ALL_PROPERTIES_CLEAN, null, null);

-                b.loadConstant(true);

-                b.returnValue(TypeDesc.BOOLEAN);

-

-                notUpdated.setLocation();

-

-                // Mark properties dirty, to be consistent with a delete side-effect.

-                b.loadThis();

-                b.invokeVirtual(MARK_PROPERTIES_DIRTY, null, null);

-                b.loadConstant(false);

-                b.returnValue(TypeDesc.BOOLEAN);

-

-                Label tryEnd = b.createLabel().setLocation();

-                b.exceptionHandler(tryStart2, tryEnd, FetchException.class.getName());

-                b.invokeVirtual(FetchException.class.getName(), "toPersistException",

-                                TypeDesc.forClass(PersistException.class), null);

-                b.throwObject();

-            }

-

-            doUpdate.setLocation();

-

             // Call doTryUpdate.

             b.loadThis();

             b.invokeVirtual(DO_TRY_UPDATE_METHOD_NAME, TypeDesc.BOOLEAN, null);

diff --git a/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java b/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java
index 7b9804d..c3adeed 100644
--- a/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java
+++ b/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java
@@ -131,69 +131,36 @@ class ReplicationTrigger<S extends Storable> extends Trigger<S> {
     private Object beforeUpdate(S replica, boolean forTry) throws PersistException {

         final S master = mMasterStorage.prepare();

         replica.copyPrimaryKeyProperties(master);

+        replica.copyVersionProperty(master);

+        replica.copyDirtyProperties(master);

 

-        if (!replica.hasDirtyProperties()) {

-            // Nothing to update, but must load from master anyhow, since

-            // update must always perform a fresh load as a side-effect. We

-            // cannot simply call update on the master, since it may need a

-            // version property to be set. Setting the version has the

-            // side-effect of making the storable look dirty, so the master

-            // will perform an update. This in turn causes the version to

-            // increase for no reason.

-            try {

-                if (forTry) {

-                    if (!master.tryLoad()) {

-                        // Master record does not exist. To ensure consistency,

-                        // delete record from replica.

-                        tryDeleteReplica(replica);

-                        throw abortTry();

-                    }

-                } else {

-                    try {

-                        master.load();

-                    } catch (FetchNoneException e) {

-                        // Master record does not exist. To ensure consistency,

-                        // delete record from replica.

-                        tryDeleteReplica(replica);

-                        throw e;

-                    }

+        try {

+            if (forTry) {

+                if (!master.tryUpdate()) {

+                    // Master record does not exist. To ensure consistency,

+                    // delete record from replica.

+                    tryDeleteReplica(replica);

+                    throw abortTry();

                 }

-            } catch (FetchException e) {

-                throw e.toPersistException

-                    ("Could not load master object for update: " + master.toStringKeyOnly());

-            }

-        } else {

-            replica.copyVersionProperty(master);

-            replica.copyDirtyProperties(master);

-

-            try {

-                if (forTry) {

-                    if (!master.tryUpdate()) {

-                        // Master record does not exist. To ensure consistency,

-                        // delete record from replica.

-                        tryDeleteReplica(replica);

-                        throw abortTry();

-                    }

-                } else {

-                    try {

-                        master.update();

-                    } catch (PersistNoneException e) {

-                        // Master record does not exist. To ensure consistency,

-                        // delete record from replica.

-                        tryDeleteReplica(replica);

-                        throw e;

-                    }

+            } else {

+                try {

+                    master.update();

+                } catch (PersistNoneException e) {

+                    // Master record does not exist. To ensure consistency,

+                    // delete record from replica.

+                    tryDeleteReplica(replica);

+                    throw e;

                 }

-            } catch (OptimisticLockException e) {

-                // This may be caused by an inconsistency between replica and

-                // master.

+            }

+        } catch (OptimisticLockException e) {

+            // This may be caused by an inconsistency between replica and

+            // master.

 

-                repair(replica);

+            repair(replica);

 

-                // Throw original exception since we don't know what the user's

-                // intentions really are.

-                throw e;

-            }

+            // Throw original exception since we don't know what the user's

+            // intentions really are.

+            throw e;

         }

 

         // Copy master properties back, since its repository may have