fix(ref-imp): #997 - increment updateCommitment if failed to apply an…

… update patch
decentralized-identity · Jan 6, 2021 · 6df555d · 6df555d
1 parent 4a3575e
commit 6df555d
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 17 deletions.
diff --git a/lib/core/versions/latest/OperationProcessor.ts b/lib/core/versions/latest/OperationProcessor.ts
@@ -163,26 +163,24 @@ export default class OperationProcessor implements IOperationProcessor {
       return didState;
     };
 
+    // Passed all verifications, must update the update commitment value even if the application of patches fail.
+    const newDidState = {
+      nextRecoveryCommitmentHash: didState.nextRecoveryCommitmentHash,
+      document: didState.document,
+      nextUpdateCommitmentHash: operation.delta.updateCommitment,
+      lastOperationTransactionNumber: anchoredOperationModel.transactionNumber
+    };
+
     let resultingDocument;
     try {
       resultingDocument = await DocumentComposer.applyUpdateOperation(operation, didState.document);
+      newDidState.document = resultingDocument;
     } catch (error) {
       const didUniqueSuffix = anchoredOperationModel.didUniqueSuffix;
       const transactionNumber = anchoredOperationModel.transactionNumber;
       Logger.info(`Unable to apply document patch in transaction number ${transactionNumber} for DID ${didUniqueSuffix}: ${SidetreeError.stringify(error)}.`);
-
-      // Return the given DID state if error is encountered applying the patches.
-      return didState;
     }
 
-    const newDidState = {
-      nextRecoveryCommitmentHash: didState.nextRecoveryCommitmentHash,
-      // New values below.
-      document: resultingDocument,
-      nextUpdateCommitmentHash: operation.delta!.updateCommitment,
-      lastOperationTransactionNumber: anchoredOperationModel.transactionNumber
-    };
-
     return newDidState;
   }
 

diff --git a/tests/core/OperationProcessor.spec.ts b/tests/core/OperationProcessor.spec.ts
@@ -626,7 +626,7 @@ describe('OperationProcessor', async () => {
         };
 
         const modifiedUpdateOperation = await UpdateOperation.parse(anchoredUpdateOperationModel.operationBuffer);
-        // set to undefined to satify the test condition of it being undefined
+        // set to undefined to satisfy the test condition of it being undefined
         (modifiedUpdateOperation.delta as any) = undefined;
         // mock the function to return the modified result
         spyOn(UpdateOperation, 'parse').and.returnValue(Promise.resolve(modifiedUpdateOperation));
@@ -639,7 +639,7 @@ describe('OperationProcessor', async () => {
         expect(newDidState!.document.publicKeys.length).toEqual(1);
       });
 
-      it('should not apply update operation if delta deos not match delta hash', async () => {
+      it('should not apply update operation if delta does not match delta hash', async () => {
         // Create an update using the create operation generated in `beforeEach()`.
         const [additionalKey] = await OperationGenerator.generateKeyPair(`new-key1`);
         const updateOperationRequest = await OperationGenerator.createUpdateOperationRequestForAddingAKey(
@@ -660,7 +660,7 @@ describe('OperationProcessor', async () => {
         };
 
         const modifiedUpdateOperation = await UpdateOperation.parse(anchoredUpdateOperationModel.operationBuffer);
-        // set to empty object to satify the test condition of not matching delta hash
+        // set to empty object to satisfy the test condition of not matching delta hash
         (modifiedUpdateOperation.delta as any) = {};
         // mock the function to return the modified result
         spyOn(UpdateOperation, 'parse').and.returnValue(Promise.resolve(modifiedUpdateOperation));
@@ -673,15 +673,16 @@ describe('OperationProcessor', async () => {
         expect(newDidState!.document.publicKeys.length).toEqual(1);
       });
 
-      it('should not apply update operation if document compose fails to apply the operation', async () => {
+      it('should treat update a success and increment update commitment if any patch failed to apply.', async () => {
         // Create an update using the create operation generated in `beforeEach()`.
         const [additionalKey] = await OperationGenerator.generateKeyPair(`new-key1`);
+        const nextUpdateCommitment = OperationGenerator.generateRandomHash();
         const updateOperationRequest = await OperationGenerator.createUpdateOperationRequestForAddingAKey(
           didUniqueSuffix,
           signingPublicKey.publicKeyJwk,
           signingPrivateKey,
           additionalKey,
-          OperationGenerator.generateRandomHash()
+          nextUpdateCommitment
         );
         const operationBuffer = Buffer.from(JSON.stringify(updateOperationRequest));
         const anchoredUpdateOperationModel: AnchoredOperationModel = {
@@ -693,10 +694,16 @@ describe('OperationProcessor', async () => {
           operationIndex: 2
         };
 
+        // Intentionally failing update patches.
         spyOn(DocumentComposer, 'applyUpdateOperation').and.throwError('Expected test error');
 
+        // The expected outcome of patch application failure (but all integrity checks including schema checks are passed )is:
+        // 1. Operation is considered successful.
+        // 2. Update commitment is updated/incremented.
+        // 3. DID document state remains the unchanged (same as prior to the patches being applied).
         const newDidState = await operationProcessor.apply(anchoredUpdateOperationModel, didState);
-        expect(newDidState!.lastOperationTransactionNumber).toEqual(1);
+        expect(newDidState!.lastOperationTransactionNumber).toEqual(2);
+        expect(newDidState!.nextUpdateCommitmentHash).toEqual(nextUpdateCommitment);
         expect(newDidState!.document).toBeDefined();
 
         // The count of public keys should remain 1, not 2.