Less racing, more versioning.

autopulated · Mar 5, 2024 · e8f4a83 · e8f4a83
1 parent dd62488
commit e8f4a83
Show file tree

Hide file tree

Showing 7 changed files with 222 additions and 19 deletions.
diff --git a/index.js b/index.js
@@ -5,7 +5,7 @@ const { Schema } = require('./lib/schema.js');
 
 const {
     DocId, Timestamp, Binary,
-    DocIdField, TypeField, CreatedAtField, UpdatedAtField
+    DocIdField, TypeField, VersionField, CreatedAtField, UpdatedAtField
 } = require('./lib/shared.js');
 
 const createLogger = (loggingOptions) => {
@@ -56,6 +56,7 @@ function DynamoDM(options) {
         // Special Schema fields
         DocIdField,
         TypeField,
+        VersionField,
         CreatedAtField,
         UpdatedAtField,
     };

diff --git a/lib/model.js b/lib/model.js
@@ -119,6 +119,9 @@ class BaseModel {
         if (!params[schema.typeFieldName]){
             params[schema.typeFieldName] = schema.name;
         }
+        if (schema.versionFieldName && !params[schema.versionFieldName]) {
+            params[schema.versionFieldName] = 0;
+        }
         // when creating via unmarshalling, the unmarshall process will have
         // already validated the data against the schema, so this step can be
         // skipped:
@@ -390,7 +393,6 @@ class BaseModel {
         const DerivedModel = this.constructor;
         const schema = DerivedModel[kModelSchema];
         const table  = DerivedModel[kModelTable];
-        // TODO, should add a version field, and use a ConditionExpression on its equality to the current version https://stackoverflow.com/questions/46531331/how-to-prevent-a-dynamodb-item-being-overwritten-if-an-entry-already-exists
         if (!table[kTableIsReady]) {
             await table.ready();
         }
@@ -421,14 +423,32 @@ class BaseModel {
                 delete properties[k];
             }
         }
-        // if the model is new, check that we are not saving a duplicate:
         const commandArgs = {
             TableName: table.name,
             Item: properties
         };
         if (this.#modelIsNew) {
+            // if the model is new, check that we are not saving a duplicate:
             commandArgs.ConditionExpression = 'attribute_not_exists(#idFieldName)';
             commandArgs.ExpressionAttributeNames = { '#idFieldName': schema.idFieldName };
+            if(schema.versionFieldName) {
+                properties[schema.versionFieldName] = 1;
+            }
+        } else if (schema.versionFieldName){
+            const previousVersion = properties[schema.versionFieldName];
+            if (!properties[schema.versionFieldName]) {
+                this.#logger.warn('Adding missing version field %s to document %s.', schema.versionFieldName, this.id);
+                properties[schema.versionFieldName] = 1;
+                // we can still make sure that another process is not adding the version field in parallel:
+                commandArgs.ConditionExpression = 'attribute_not_exists(#v)';
+                commandArgs.ExpressionAttributeNames = { '#v': schema.versionFieldName };
+            } else {
+                properties[schema.versionFieldName] += 1;
+                // otherwise, check that the version field is the same as it was when we loaded this model. Each save increments the version.
+                commandArgs.ConditionExpression = '#v = :v';
+                commandArgs.ExpressionAttributeNames = { '#v': schema.versionFieldName };
+                commandArgs.ExpressionAttributeValues = { ':v': previousVersion };
+            }
         }
         const command = new PutCommand(commandArgs);
         this.#logger.trace({command}, 'save %s', this.id);
@@ -437,29 +457,43 @@ class BaseModel {
             this.#logger.trace({response}, 'save %s response', this.id);
         } catch (e) {
             if(e.name === 'ConditionalCheckFailedException') {
-                throw new Error(`An item already exists with id field .${schema.idFieldName}="${this[schema.idFieldName]}"`);
+                if (this.#modelIsNew) {
+                    throw new Error(`An item already exists with id field .${schema.idFieldName}="${this[schema.idFieldName]}"`);
+                } else {
+                    throw new Error(`Version error: the model .${schema.idFieldName}="${this[schema.idFieldName]}" was updated by another process between loading and saving.`);
+                }
             } else {
                 /* c8 ignore next 2 */
                 throw e;
             }
         }
         // after saving once, we're no longer new
         this.#modelIsNew = false;
+        if (schema.versionFieldName) {
+            // and increment the visible version
+            this[schema.versionFieldName] = properties[schema.versionFieldName];
+        }
         return this;
     }
 
     async #remove(){
         const table = this.constructor[kModelTable],
              schema = this.constructor[kModelSchema];
-        // TODO, should add a version field, and use a ConditionExpression on its equaltiy to the current version https://stackoverflow.com/questions/46531331/how-to-prevent-a-dynamodb-item-being-overwritten-if-an-entry-already-exists
         if (!table[kTableIsReady]) {
             /* c8 ignore next 2 */
             await table.ready();
         }
-        const command = new DeleteCommand({
+        const commandArgs = {
             TableName: table.name,
             Key: { [schema.idFieldName]: this[schema.idFieldName] }
-        });
+        };
+        // check that the version field is the same as it was when we loaded this model.
+        if (schema.versionFieldName) {
+            commandArgs.ConditionExpression = '#v = :v';
+            commandArgs.ExpressionAttributeNames = { '#v': schema.versionFieldName };
+            commandArgs.ExpressionAttributeValues = { ':v': this[schema.versionFieldName] };
+        }
+        const command = new DeleteCommand(commandArgs);
         this.#logger.trace({command}, 'remove %s', this.id);
         const data = await table[kTableDDBClient].send(command);
         this.#logger.trace({response: data}, 'remove %s response', this.id);

diff --git a/lib/schema.js b/lib/schema.js
@@ -16,6 +16,7 @@ const {
 
     DocIdField,
     TypeField,
+    VersionField,
     CreatedAtField,
     UpdatedAtField,
 
@@ -124,6 +125,7 @@ class Schema {
     name = '';
     idFieldName = '';
     typeFieldName = '';
+    versionFieldName = '';
     createdAtFieldName = '';
     updatedAtFieldName = '';
     source = null;
@@ -140,7 +142,7 @@ class Schema {
     [kSchemaUnMarshall] = null;
 
     constructor(name, schemaSource, options) {
-        const { index, generateId } = options ?? {};
+        const { index, generateId, versioning } = options ?? {};
         if (['object', 'undefined'].includes(typeof schemaSource) === false) {
             throw new Error('Invalid schema: must be an object or undefined.');
         }
@@ -161,6 +163,15 @@ class Schema {
         this.createdAtFieldName = findPropertyValue(schemaSourceProps, CreatedAtField, 'Duplicate createdAt field.');
         this.updatedAtFieldName = findPropertyValue(schemaSourceProps, UpdatedAtField, 'Duplicate updatedAt field.');
 
+        const versionFieldName = findPropertyValue(schemaSourceProps, VersionField, 'Duplicate version field.');
+        if (versioning === false) {
+            if (versionFieldName) {
+                options.logger.warn(`options.versioning is false, so the ${this.name} Schema properties VersionField .${versionFieldName} is ignored`);
+            }
+        } else {
+            this.versionFieldName = versionFieldName || 'v';
+        }
+
         if (schemaSource?.type && schemaSource?.type !== 'object') {
             throw new Error('Schema type must be object (or can be omitted).');
         }
@@ -171,9 +182,14 @@ class Schema {
             // ensure optional required fields are present in schema:
             {
                 [this.idFieldName]: DocIdField,
-                [this.typeFieldName]: TypeField
+                [this.typeFieldName]: TypeField,
+            },
+            {
+                ...(this.versionFieldName?  {[this.versionFieldName]: VersionField} : {})
             }
+
         );
+
         const schemaRequired = [...new Set([this.idFieldName, this.typeFieldName, ...(schemaSource?.required ?? [])])];
 
         schemaSource = {

diff --git a/lib/shared.js b/lib/shared.js
@@ -137,10 +137,12 @@ const DocId = { type:'string', minLength:1, maxLength:1024 };
 const Timestamp = { extendedType: kExtendedTypeDate };
 const Binary = { extendedType: kExtendedTypeBuffer };
 const TypeFieldType = { type:'string', minLength:1, maxLength:1024 };
+const VersionFieldType = { type:'integer', minimum:0 };
 
 // Built-in schema types that are compared by identity in order to identify special field names
 const DocIdField = Object.assign({}, DocId);
 const TypeField = Object.assign({}, TypeFieldType);
+const VersionField = Object.assign({}, VersionFieldType);
 const CreatedAtField = Object.assign({}, Timestamp);
 const UpdatedAtField = Object.assign({}, Timestamp);
 
@@ -172,10 +174,10 @@ module.exports = {
     DocId,
     Timestamp,
     Binary,
-    TypeFieldType,
 
     DocIdField,
     TypeField,
+    VersionField,
     CreatedAtField,
     UpdatedAtField,
 

diff --git a/test/crud.js b/test/crud.js
@@ -17,7 +17,7 @@ t.test('crud:', async t => {
             cccc:         {type: 'string'},
             blob:         DynamoDM.Binary,
             createdAt:    DynamoDM.CreatedAtField,
-            updatedAt:    DynamoDM.UpdatedAtField,
+            updatedAt:    DynamoDM.UpdatedAtField
         },
         required: ['id', 'aaaa', 'bbbb'],
         additionalProperties: false