Mark soft conflict if not contiguous with trunk (#1187)

* Mark soft conflict if not contiguous with trunk

* Remove unneeded code and make other small changes

* query to check for non-contiguous interrupted branches

* add more comments and explanation to tests

---------

Co-authored-by: Matthew White <[email protected]>

lib/data/entity.js

@@ -397,6 +397,55 @@ const diffEntityData = (defData) => {
   return diffs;
 };
 
+// Copied from frontend
+// Offline branch
+class Branch {
+  // firstUpdate is the first offline update (not create) to be processed from
+  // the branch.
+  constructor(firstUpdate) {
+    if (firstUpdate.trunkVersion != null) {
+      const { trunkVersion } = firstUpdate;
+      /* this.lastContiguousWithTrunk is the version number of the last version
+      from the branch that is contiguous with the trunk version. In other words,
+      it is the version number of the last version where there has been no
+      update from outside the branch between the version and the trunk version.
+      this.lastContiguousWithTrunk is not related to branch order: as long as
+      there hasn't been an update from outside the branch, the branch is
+      contiguous, regardless of the order of the updates within it. */
+      this.lastContiguousWithTrunk = firstUpdate.version === trunkVersion + 1
+        ? firstUpdate.version
+        : 0;
+    } else {
+      /*
+      If the entity was both created and updated offline before being sent to
+      the server, then there technically isn't a trunk version. On the flip
+      side, there also isn't a contiguity problem -- except in one special case.
+      If the submission for the entity creation is sent and processed, but the
+      submission for the update is not sent at the same time for some reason,
+      then it's possible for another update to be made between the two. Once the
+      original update is sent and processed, it will no longer be contiguous
+      with the entity creation.
+
+      Another special case is if the submission for the entity creation was sent
+      late and processed out of order. In that case, firstUpdate.version === 1.
+      There's again no contiguity problem (just an order problem), so
+      lastContiguousWithTrunk should equal 1.
+
+      The normal case is if firstUpdate.version === 2.
+      */
+      this.lastContiguousWithTrunk = firstUpdate.version === 2 ? 2 : 1;
+    }
+
+    this.id = firstUpdate.branchId;
+  }
+
+  add(version) {
+    if (version.baseVersion === this.lastContiguousWithTrunk &&
+      version.version === version.baseVersion + 1)
+      this.lastContiguousWithTrunk = version.version;
+  }
+}
+
 // Returns an array of properties which are different between
 // `dataReceived` and `otherVersionData`
 const getDiffProp = (dataReceived, otherVersionData) =>
@@ -419,7 +468,18 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => {
 
   const relevantBaseVersions = new Set();
 
+  const branches = new Map();
+
   for (const def of defs) {
+    // build up branches
+    const { branchId } = def;
+    if (branchId != null) {
+      const existingBranch = branches.get(branchId);
+      if (existingBranch == null)
+        branches.set(branchId, new Branch(def));
+      else
+        existingBranch.add(def);
+    }
 
     const v = mergeLeft(def.forApi(),
       {
@@ -438,7 +498,10 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => {
       v.source = event.source;
 
     if (v.version > 1) { // v.root is false here - can use either
-      const conflict = v.version !== (v.baseVersion + 1);
+      const baseNotContiguousWithTrunk = v.branchId != null &&
+        branches.get(v.branchId).lastContiguousWithTrunk < v.baseVersion;
+      const conflict = v.version !== (v.baseVersion + 1) ||
+        baseNotContiguousWithTrunk;
 
       v.baseDiff = getDiffProp(v.dataReceived, { ...defs[v.baseVersion - 1].data, label: defs[v.baseVersion - 1].label });
 

lib/model/query/entities.js

@@ -304,10 +304,15 @@ const _updateEntity = (dataset, entityData, submissionId, submissionDef, submiss
       serverDiffData.label = serverEntity.aux.currentVersion.label;
 
     conflictingProperties = Object.keys(clientEntity.def.dataReceived).filter(key => key in serverDiffData && clientEntity.def.dataReceived[key] !== serverDiffData[key]);
-
     if (conflict !== ConflictType.HARD) { // We don't want to downgrade conflict here
       conflict = conflictingProperties.length > 0 ? ConflictType.HARD : ConflictType.SOFT;
     }
+  } else {
+    // This may still be a soft conflict if the new version is not contiguous with this branch's trunk version
+    const interrupted = await Entities._interruptedBranch(serverEntity.id, clientEntity);
+    if (interrupted && conflict !== ConflictType.HARD) {
+      conflict = ConflictType.SOFT;
+    }
   }
 
   // merge data
@@ -541,6 +546,29 @@ const processSubmissionEvent = (event, parentEvent) => (container) =>
 ////////////////////////////////////////////////////////////////////////////////
 // Submission processing helper functions
 
+// Used by _updateEntity to determine if a new offline update is contiguous with its trunk version
+// by searching for an interrupting version with a different or null branchId that has a higher
+// version than the trunk version of the given branch.
+const _interruptedBranch = (entityId, clientEntity) => async ({ maybeOne }) => {
+  // If there is no branchId, the branch cannot be interrupted
+  if (clientEntity.def.branchId == null)
+    return false;
+
+  // look for a version of a different branch that has a version
+  // higher than the trunkVersion, which indicates an interrupting version.
+  // if trunkVersion is null (becuase it is part of a branch beginning with
+  // an offline create), look for a version higher than 1 because version
+  // 1 is implicitly the create action of that offline branch.
+  const interruptingVersion = await maybeOne(sql`
+  SELECT version
+  FROM entity_defs
+  WHERE "branchId" IS DISTINCT FROM ${clientEntity.def.branchId}
+    AND version > ${clientEntity.def.trunkVersion || 1}
+    AND "entityId" = ${entityId}
+    LIMIT 1`);
+  return interruptingVersion.isDefined();
+};
+
 // Used by _computeBaseVersion to hold submissions that are not yet ready to be processed
 const _holdSubmission = (eventId, submissionId, submissionDefId, entityUuid, branchId, branchBaseVersion) => async ({ run }) => run(sql`
   INSERT INTO entity_submission_backlog ("auditId", "submissionId", "submissionDefId", "entityUuid", "branchId", "branchBaseVersion", "loggedAt")
@@ -780,7 +808,7 @@ module.exports = {
   createSource,
   createMany,
   _createEntity, _updateEntity,
-  _computeBaseVersion,
+  _computeBaseVersion, _interruptedBranch,
   _holdSubmission, _checkHeldSubmission,
   _getNextHeldSubmissionInBranch, _deleteHeldSubmissionByEventId,
   _getHeldSubmissionsAsEvents,