fix: model hydration is broken when an array is empty (#10)

* docs: add jsdoc information for Persist.Type * fix: allow for empty arrays on models when hydrating
acodeninja · Sep 10, 2024 · a22f397 · a22f397
1 parent 3096e64
commit a22f397
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 11 deletions.
diff --git a/src/SchemaCompiler.test.js b/src/SchemaCompiler.test.js
@@ -20,6 +20,10 @@ const schema = {
     requiredBoolean: Type.Boolean.required,
     date: Type.Date,
     requiredDate: Type.Date.required,
+    emptyArrayOfStrings: Type.Array.of(Type.String),
+    emptyArrayOfNumbers: Type.Array.of(Type.Number),
+    emptyArrayOfBooleans: Type.Array.of(Type.Boolean),
+    emptyArrayOfDates: Type.Array.of(Type.Date),
     arrayOfString: Type.Array.of(Type.String),
     arrayOfNumber: Type.Array.of(Type.Number),
     arrayOfBoolean: Type.Array.of(Type.Boolean),
@@ -78,6 +82,30 @@ const invalidDataErrors = [{
     message: 'must match format "iso-date-time"',
     params: {format: 'iso-date-time'},
     schemaPath: '#/properties/date/format',
+}, {
+    instancePath: '/emptyArrayOfStrings',
+    keyword: 'type',
+    message: 'must be array',
+    params: {type: 'array'},
+    schemaPath: '#/properties/emptyArrayOfStrings/type',
+}, {
+    instancePath: '/emptyArrayOfNumbers',
+    keyword: 'type',
+    message: 'must be array',
+    params: {type: 'array'},
+    schemaPath: '#/properties/emptyArrayOfNumbers/type',
+}, {
+    instancePath: '/emptyArrayOfBooleans',
+    keyword: 'type',
+    message: 'must be array',
+    params: {type: 'array'},
+    schemaPath: '#/properties/emptyArrayOfBooleans/type',
+}, {
+    instancePath: '/emptyArrayOfDates',
+    keyword: 'type',
+    message: 'must be array',
+    params: {type: 'array'},
+    schemaPath: '#/properties/emptyArrayOfDates/type',
 }, {
     instancePath: '/arrayOfString/0',
     keyword: 'type',
@@ -163,6 +191,10 @@ test('.compile(schema) has the given schema associated with it', t => {
             requiredBoolean: {type: 'boolean'},
             date: {type: 'string', format: 'iso-date-time'},
             requiredDate: {type: 'string', format: 'iso-date-time'},
+            emptyArrayOfStrings: {type: 'array', items: {type: 'string'}},
+            emptyArrayOfNumbers: {type: 'array', items: {type: 'number'}},
+            emptyArrayOfBooleans: {type: 'array', items: {type: 'boolean'}},
+            emptyArrayOfDates: {type: 'array', items: {type: 'string', format: 'iso-date-time'}},
             arrayOfString: {type: 'array', items: {type: 'string'}},
             arrayOfNumber: {type: 'array', items: {type: 'number'}},
             arrayOfBoolean: {type: 'array', items: {type: 'boolean'}},
@@ -236,6 +268,24 @@ test('.compile(MainModel) has the given schema associated with it', t => {
             requiredArrayOfNumber: {type: 'array', items: {type: 'number'}},
             requiredArrayOfBoolean: {type: 'array', items: {type: 'boolean'}},
             requiredArrayOfDate: {type: 'array', items: {type: 'string', format: 'iso-date-time'}},
+            emptyArrayOfStrings: {type: 'array', items: {type: 'string'}},
+            emptyArrayOfNumbers: {type: 'array', items: {type: 'number'}},
+            emptyArrayOfBooleans: {type: 'array', items: {type: 'boolean'}},
+            emptyArrayOfDates: {type: 'array', items: {type: 'string', format: 'iso-date-time'}},
+            emptyArrayOfModels: {
+                type: 'array',
+                items: {
+                    type: 'object',
+                    additionalProperties: false,
+                    required: ['id'],
+                    properties: {
+                        id: {
+                            type: 'string',
+                            pattern: '^LinkedManyModel/[A-Z0-9]+$',
+                        },
+                    },
+                },
+            },
             requiredLinked: {
                 type: 'object',
                 additionalProperties: false,

diff --git a/src/engine/FileEngine.test.js b/src/engine/FileEngine.test.js
@@ -532,5 +532,13 @@ test('FileEngine.hydrate(model)', async t => {
         filesystem,
     }).hydrate(dryModel);
 
+    assertions.calledWith(t, filesystem.readFile, '/tmp/fileEngine/MainModel/000000000000.json');
+    assertions.calledWith(t, filesystem.readFile, '/tmp/fileEngine/CircularModel/000000000000.json');
+    assertions.calledWith(t, filesystem.readFile, '/tmp/fileEngine/LinkedModel/000000000000.json');
+    assertions.calledWith(t, filesystem.readFile, '/tmp/fileEngine/LinkedModel/111111111111.json');
+    assertions.calledWith(t, filesystem.readFile, '/tmp/fileEngine/LinkedManyModel/000000000000.json');
+    assertions.calledWith(t, filesystem.readFile, '/tmp/fileEngine/CircularManyModel/000000000000.json');
+
+    t.is(filesystem.readFile.getCalls().length, 6);
     t.deepEqual(hydratedModel, model);
 });
diff --git a/src/engine/HTTPEngine.test.js b/src/engine/HTTPEngine.test.js
@@ -885,13 +885,7 @@ test('HTTPEngine.hydrate(model)', async t => {
     const dryModel = new MainModel();
     dryModel.id = 'MainModel/000000000000';
 
-    const fetch = stubFetch({}, [
-        getTestModelInstance(valid),
-        getTestModelInstance({
-            id: 'MainModel/111111111111',
-            string: 'another string',
-        }),
-    ]);
+    const fetch = stubFetch({}, [getTestModelInstance(valid)]);
 
     const hydratedModel = await HTTPEngine.configure({
         host: 'https://example.com',
@@ -906,5 +900,7 @@ test('HTTPEngine.hydrate(model)', async t => {
     assertions.calledWith(t, fetch, new URL('https://example.com/test/LinkedManyModel/000000000000.json'), {headers: {Accept: 'application/json'}});
     assertions.calledWith(t, fetch, new URL('https://example.com/test/CircularManyModel/000000000000.json'), {headers: {Accept: 'application/json'}});
 
+    t.is(fetch.getCalls().length, 6);
+
     t.deepEqual(hydratedModel, model);
 });
diff --git a/src/engine/S3Engine.test.js b/src/engine/S3Engine.test.js
@@ -936,5 +936,31 @@ test('S3Engine.hydrate(model)', async t => {
         client,
     }).hydrate(dryModel);
 
+    assertions.calledWith(t, client.send, new GetObjectCommand({
+        Bucket: 'test-bucket',
+        Key: 'test/MainModel/000000000000.json',
+    }));
+    assertions.calledWith(t, client.send, new GetObjectCommand({
+        Bucket: 'test-bucket',
+        Key: 'test/CircularModel/000000000000.json',
+    }));
+    assertions.calledWith(t, client.send, new GetObjectCommand({
+        Bucket: 'test-bucket',
+        Key: 'test/LinkedModel/000000000000.json',
+    }));
+    assertions.calledWith(t, client.send, new GetObjectCommand({
+        Bucket: 'test-bucket',
+        Key: 'test/LinkedModel/111111111111.json',
+    }));
+    assertions.calledWith(t, client.send, new GetObjectCommand({
+        Bucket: 'test-bucket',
+        Key: 'test/LinkedManyModel/000000000000.json',
+    }));
+    assertions.calledWith(t, client.send, new GetObjectCommand({
+        Bucket: 'test-bucket',
+        Key: 'test/CircularManyModel/000000000000.json',
+    }));
+
+    t.is(client.send.getCalls().length, 6);
     t.deepEqual(hydratedModel, model);
 });
diff --git a/src/type/Model.js b/src/type/Model.js
@@ -118,9 +118,13 @@ export default class Model {
     }
 
     static isDryModel(possibleDryModel) {
-        return (
-            Object.keys(possibleDryModel).includes('id') &&
-            !!possibleDryModel.id.match(/[A-Za-z]+\/[A-Z0-9]+/)
-        );
+        try {
+            return (
+                Object.keys(possibleDryModel).includes('id') &&
+                !!possibleDryModel.id.match(/[A-Za-z]+\/[A-Z0-9]+/)
+            );
+        } catch (_) {
+            return false;
+        }
     }
 }
diff --git a/src/type/Model.test.js b/src/type/Model.test.js
@@ -33,6 +33,7 @@ test('model.toData() returns an object representation of the model', t => {
         requiredLinked: {id: 'LinkedModel/111111111111'},
         circular: {id: 'CircularModel/000000000000'},
         linkedMany: [{id: 'LinkedManyModel/000000000000'}],
+        emptyArrayOfModels: [],
         circularMany: [{id: 'CircularManyModel/000000000000'}],
     });
 });

diff --git a/src/type/index.js b/src/type/index.js
@@ -7,6 +7,17 @@ import NumberType from './simple/NumberType.js';
 import SlugType from './resolved/SlugType.js';
 import StringType from './simple/StringType.js';
 
+/**
+ * @class Type
+ * @property {StringType} String
+ * @property {NumberType} Number
+ * @property {BooleanType} Boolean
+ * @property {DateType} Date
+ * @property {ArrayType} Array
+ * @property {CustomType} Custom
+ * @property {ResolvedType} Resolved
+ * @property {Model} Model
+ */
 const Type = {};
 
 Type.String = StringType;
@@ -15,6 +26,11 @@ Type.Boolean = BooleanType;
 Type.Date = DateType;
 Type.Array = ArrayType;
 Type.Custom = CustomType;
+
+/**
+ * @class ResolvedType
+ * @property {SlugType} Slug
+ */
 Type.Resolved = {Slug: SlugType};
 Type.Model = Model;
 

diff --git a/test/fixtures/TestModel.js b/test/fixtures/TestModel.js
@@ -10,6 +10,10 @@ export const valid = {
     requiredBoolean: true,
     date: new Date().toISOString(),
     requiredDate: new Date().toISOString(),
+    emptyArrayOfStrings: [],
+    emptyArrayOfNumbers: [],
+    emptyArrayOfBooleans: [],
+    emptyArrayOfDates: [],
     arrayOfString: ['String'],
     arrayOfNumber: [24.5],
     arrayOfBoolean: [false],
@@ -29,6 +33,10 @@ export const invalid = {
     requiredBoolean: undefined,
     date: 'not-a-date',
     requiredDate: undefined,
+    emptyArrayOfStrings: 'not-a-list',
+    emptyArrayOfNumbers: 'not-a-list',
+    emptyArrayOfBooleans: 'not-a-list',
+    emptyArrayOfDates: 'not-a-list',
     arrayOfString: [true],
     arrayOfNumber: ['string'],
     arrayOfBoolean: [15.8],
@@ -97,6 +105,11 @@ export class MainModel extends Type.Model {
     static requiredBoolean = Type.Boolean.required;
     static date = Type.Date;
     static requiredDate = Type.Date.required;
+    static emptyArrayOfStrings = Type.Array.of(Type.String);
+    static emptyArrayOfNumbers = Type.Array.of(Type.Number);
+    static emptyArrayOfBooleans = Type.Array.of(Type.Boolean);
+    static emptyArrayOfDates = Type.Array.of(Type.Date);
+    static emptyArrayOfModels = () => Type.Array.of(LinkedManyModel);
     static arrayOfString = Type.Array.of(Type.String);
     static arrayOfNumber = Type.Array.of(Type.Number);
     static arrayOfBoolean = Type.Array.of(Type.Boolean);
@@ -145,6 +158,12 @@ export function getTestModelInstance(data = {}) {
     if (data.arrayOfDate) model.arrayOfDate = data.arrayOfDate.map(d => DateType.isDate(d) ? new Date(d) : d);
     if (data.requiredArrayOfDate) model.requiredArrayOfDate = data.requiredArrayOfDate.map(d => DateType.isDate(d) ? new Date(d) : d);
 
+    if (!model.emptyArrayOfStrings) model.emptyArrayOfStrings = [];
+    if (!model.emptyArrayOfNumbers) model.emptyArrayOfNumbers = [];
+    if (!model.emptyArrayOfBooleans) model.emptyArrayOfBooleans = [];
+    if (!model.emptyArrayOfDates) model.emptyArrayOfDates = [];
+    if (!model.emptyArrayOfModels) model.emptyArrayOfModels = [];
+
     const circular = new CircularModel({linked: model});
     circular.id = circular.id.replace(/[a-zA-Z0-9]+$/, '000000000000');
     model.circular = circular;