fix(excludefromindexes): update logic to add all properties of Array embedded entities (#182)

sebelga · web-flow · commit c9da35b8b081 · 2019-09-11T08:32:22.000+02:00
The v4.2.0 of the Datastore client allows wildcard "*" to target all the properties of an embedded entity. The logic to define the Array of excludeFromIndexes has been updated to make use of it. fix #132
diff --git a/lib/entity.js b/lib/entity.js
@@ -16,7 +16,8 @@ class Entity {
     constructor(data, id, ancestors, namespace, key) {
         this.className = 'Entity';
         this.schema = this.constructor.schema;
-        this.excludeFromIndexes = [];
+        this.excludeFromIndexes = {};
+
         /**
          * Object to store custom data for the entity.
          * In some cases we might want to add custom data onto the entity
@@ -36,8 +37,8 @@ class Entity {
 
         this.setId();
 
-        // create entityData from data passed
-        this.entityData = buildEntityData(this, data || {});
+        // create entityData from data provided
+        this.____buildEntityData(data || {});
 
         /**
          * Create virtual properties (getters and setters for entityData object)
@@ -333,6 +334,88 @@ class Entity {
 
         return entityData;
     }
+
+    ____buildEntityData(data) {
+        const { schema } = this;
+        const isJoiSchema = schema.isJoi;
+
+        // If Joi schema, get its default values
+        if (isJoiSchema) {
+            const { error, value } = schema.validateJoi(data);
+
+            if (!error) {
+                this.entityData = { ...value };
+            }
+        }
+
+        this.entityData = { ...this.entityData, ...data };
+
+        let isArray;
+        let isObject;
+
+        Object.entries(schema.paths).forEach(([key, prop]) => {
+            const hasValue = {}.hasOwnProperty.call(this.entityData, key);
+            const isOptional = {}.hasOwnProperty.call(prop, 'optional') && prop.optional !== false;
+            const isRequired = {}.hasOwnProperty.call(prop, 'required') && prop.required === true;
+
+            // Set Default Values
+            if (!isJoiSchema && !hasValue && !isOptional) {
+                let value = null;
+
+                if ({}.hasOwnProperty.call(prop, 'default')) {
+                    if (typeof prop.default === 'function') {
+                        value = prop.default();
+                    } else {
+                        value = prop.default;
+                    }
+                }
+
+                if (({}).hasOwnProperty.call(defaultValues.__map__, value)) {
+                    /**
+                   * If default value is in the gstore.defaultValue hashTable
+                   * then execute the handler for that shortcut
+                   */
+                    value = defaultValues.__handler__(value);
+                } else if (value === null && {}.hasOwnProperty.call(prop, 'values') && !isRequired) {
+                    // Default to first value of the allowed values if **not** required
+                    [value] = prop.values;
+                }
+
+                this.entityData[key] = value;
+            }
+
+            // Set excludeFromIndexes
+            // ----------------------
+            isArray = prop.type === Array || (prop.joi && prop.joi._type === 'array');
+            isObject = prop.type === Object || (prop.joi && prop.joi._type === 'object');
+
+            if (prop.excludeFromIndexes === true) {
+                if (isArray) {
+                    // We exclude both the array values + all the child properties of object items
+                    this.excludeFromIndexes[key] = [`${key}[]`, `${key}[].*`];
+                } else if (isObject) {
+                    // We exclude the emmbeded entity + all its properties
+                    this.excludeFromIndexes[key] = [key, `${key}.*`];
+                } else {
+                    this.excludeFromIndexes[key] = [key];
+                }
+            } else if (prop.excludeFromIndexes !== false) {
+                const excludedArray = arrify(prop.excludeFromIndexes);
+                if (isArray) {
+                    // The format to exclude a property from an embedded entity inside
+                    // an array is: "myArrayProp[].embeddedKey"
+                    this.excludeFromIndexes[key] = excludedArray.map(propExcluded => `${key}[].${propExcluded}`);
+                } else if (isObject) {
+                // The format to exclude a property from an embedded entity
+                    // is: "myEmbeddedEntity.key"
+                    this.excludeFromIndexes[key] = excludedArray.map(propExcluded => `${key}.${propExcluded}`);
+                }
+            }
+        });
+
+        // add Symbol Key to the entityData
+        this.entityData[this.gstore.ds.KEY] = this.entityKey;
+    }
 }
 
 // Private
@@ -367,90 +450,6 @@ function createKey(self, id, ancestors, namespace) {
     return namespace ? self.gstore.ds.key({ namespace, path }) : self.gstore.ds.key(path);
 }
 
-function buildEntityData(self, data) {
-    const { schema } = self;
-    const isJoiSchema = schema.isJoi;
-
-    let entityData;
-
-    // If Joi schema, get its default values
-    if (isJoiSchema) {
-        const { error, value } = schema.validateJoi(data);
-
-        if (!error) {
-            entityData = { ...value };
-        }
-    }
-
-    entityData = { ...entityData, ...data };
-
-    let isTypeArray;
-
-    Object.keys(schema.paths).forEach(k => {
-        const prop = schema.paths[k];
-        const hasValue = {}.hasOwnProperty.call(entityData, k);
-        const isOptional = {}.hasOwnProperty.call(prop, 'optional') && prop.optional !== false;
-        const isRequired = {}.hasOwnProperty.call(prop, 'required') && prop.required === true;
-
-        // Set Default Values
-        if (!isJoiSchema && !hasValue && !isOptional) {
-            let value = null;
-
-            if ({}.hasOwnProperty.call(prop, 'default')) {
-                if (typeof prop.default === 'function') {
-                    value = prop.default();
-                } else {
-                    value = prop.default;
-                }
-            }
-
-            if (({}).hasOwnProperty.call(defaultValues.__map__, value)) {
-                /**
-                 * If default value is in the gstore.defaultValue hashTable
-                 * then execute the handler for that shortcut
-                 */
-                value = defaultValues.__handler__(value);
-            } else if (value === null && {}.hasOwnProperty.call(prop, 'values') && !isRequired) {
-                // Default to first value of the allowed values if **not** required
-                [value] = prop.values;
-            }
-
-            entityData[k] = value;
-        }
-
-        // Set excludeFromIndexes
-        // ----------------------
-        isTypeArray = prop.type === 'array' || (prop.joi && prop.joi._type === 'array');
-
-        if (prop.excludeFromIndexes === true && !isTypeArray) {
-            self.excludeFromIndexes.push(k);
-        } else if (!is.boolean(prop.excludeFromIndexes)) {
-            // For embedded entities we can set which properties are excluded from indexes
-            // by passing a string|array of properties
-
-            let formatted;
-            const exFromIndexes = arrify(prop.excludeFromIndexes);
-
-            if (prop.type === 'array') {
-                // The format to exclude a property from an embedded entity inside
-                // an array is: "myArrayProp[].embeddedKey"
-                formatted = exFromIndexes.map(excluded => `${k}[].${excluded}`);
-            } else {
-                // The format to exclude a property from an embedded entity
-                // is: "myEmbeddedEntity.key"
-                formatted = exFromIndexes.map(excluded => `${k}.${excluded}`);
-            }
-
-            self.excludeFromIndexes = [...self.excludeFromIndexes, ...formatted];
-        }
-    });
-
-    // add Symbol Key to the entityData
-    entityData[self.gstore.ds.KEY] = self.entityKey;
-
-    return entityData;
-}
-
 function registerHooksFromSchema(self) {
     const callQueue = self.schema.callQueue.entity;
 
diff --git a/lib/serializers/datastore.js b/lib/serializers/datastore.js
@@ -36,41 +36,11 @@ function toDatastore(entity, options = {}) {
     // ---------
 
     function getExcludeFromIndexes() {
-        const excluded = [...entity.excludeFromIndexes] || [];
-        let isArray;
-        let isObject;
-        let propConfig;
-        let propValue;
-
-        Object.keys(data).forEach(prop => {
-            propValue = entity.entityData[prop];
-            if (propValue === null) {
-                return;
-            }
-            propConfig = entity.schema.paths[prop];
-
-            isArray = propConfig && (propConfig.type === 'array'
-              || (propConfig.joi && propConfig.joi._type === 'array'));
-
-            isObject = propConfig && (propConfig.type === 'object'
-            || (propConfig.joi && propConfig.joi._type === 'object'));
-
-            if (isArray && propConfig.excludeFromIndexes === true) {
-                // We exclude all the primitives from Array
-                // The format is "entityProp[]"
-                excluded.push(`${prop}[]`);
-            } else if (isObject && propConfig.excludeFromIndexes === true) {
-                // For "object" type we automatically set all its properties to excludeFromIndexes: true
-                // which is what most of us expect.
-                Object.keys(propValue).forEach(k => {
-                    // We add the embedded property to our Array of excludedFromIndexes
-                    // The format is "entityProp.entityKey"
-                    excluded.push(`${prop}.${k}`);
-                });
-            }
-        });
-
-        return excluded;
+        return Object.entries(data)
+            .filter(({ 1: value }) => value !== null)
+            .map(([key]) => entity.excludeFromIndexes[key])
+            .filter(v => v !== undefined)
+            .reduce((acc, arr) => [...acc, ...arr], []);
     }
 }
 
diff --git a/package.json b/package.json
@@ -99,6 +99,6 @@
     "yargs": "^14.0.0"
   },
   "peerDependencies": {
-    "@google-cloud/datastore": ">= 3.0.0 < 5"
+    "@google-cloud/datastore": ">= 4.2.0 < 5"
   }
 }
diff --git a/test/entity-test.js b/test/entity-test.js
@@ -72,7 +72,7 @@ describe('Entity', () => {
             assert.isDefined(entity.schema);
             assert.isDefined(entity.pre);
             assert.isDefined(entity.post);
-            expect(entity.excludeFromIndexes).deep.equal([]);
+            expect(entity.excludeFromIndexes).deep.equal({});
         });
 
         it('should add data passed to entityData', () => {
@@ -87,7 +87,7 @@ describe('Entity', () => {
 
         it('should not add any data if nothing is passed', () => {
             schema = new Schema({
-                name: { type: 'string', optional: true },
+                name: { type: String, optional: true },
             });
             GstoreModel = gstore.model('BlogPost', schema);
 
@@ -102,10 +102,10 @@ describe('Entity', () => {
             }
 
             schema = new Schema({
-                name: { type: 'string', default: 'John' },
-                lastname: { type: 'string' },
+                name: { type: String, default: 'John' },
+                lastname: { type: String },
                 email: { optional: true },
-                generatedValue: { type: 'string', default: fn },
+                generatedValue: { type: String, default: fn },
                 availableValues: { values: ['a', 'b', 'c'] },
                 availableValuesRequired: { values: ['a', 'b', 'c'], required: true },
             });
@@ -159,7 +159,7 @@ describe('Entity', () => {
         it('should call handler for default values in gstore.defaultValues constants', () => {
             sinon.spy(gstore.defaultValues, '__handler__');
             schema = new Schema({
-                createdOn: { type: 'dateTime', default: gstore.defaultValues.NOW },
+                createdOn: { type: Date, default: gstore.defaultValues.NOW },
             });
             GstoreModel = gstore.model('BlogPost', schema);
             entity = new GstoreModel({});
@@ -170,7 +170,7 @@ describe('Entity', () => {
 
         it('should not add default to optional properties', () => {
             schema = new Schema({
-                name: { type: 'string' },
+                name: { type: String },
                 email: { optional: true },
             });
             GstoreModel = gstore.model('BlogPost', schema);
@@ -183,20 +183,27 @@ describe('Entity', () => {
         it('should create its array of excludeFromIndexes', () => {
             schema = new Schema({
                 name: { excludeFromIndexes: true },
-                age: { excludeFromIndexes: true, type: 'int' },
-                embedded: { excludeFromIndexes: ['prop1', 'prop2'] },
-                arrayValue: { excludeFromIndexes: 'property', type: 'array' },
+                age: { excludeFromIndexes: true, type: Number },
+                embedded: { type: Object, excludeFromIndexes: ['prop1', 'prop2'] },
+                embedded2: { type: Object, excludeFromIndexes: true },
+                arrayValue: { excludeFromIndexes: 'property', type: Array },
                 // Array in @google-cloud have to be set on the data value
-                arrayValue2: { excludeFromIndexes: true, type: 'array' },
+                arrayValue2: { excludeFromIndexes: true, type: Array },
                 arrayValue3: { excludeFromIndexes: true, joi: Joi.array() },
             });
             GstoreModel = gstore.model('BlogPost', schema);
 
             entity = new GstoreModel({ name: 'John' });
 
-            expect(entity.excludeFromIndexes).deep.equal([
-                'name', 'age', 'embedded.prop1', 'embedded.prop2', 'arrayValue[].property',
-            ]);
+            expect(entity.excludeFromIndexes).deep.equal({
+                name: ['name'],
+                age: ['age'],
+                embedded: ['embedded.prop1', 'embedded.prop2'],
+                embedded2: ['embedded2', 'embedded2.*'],
+                arrayValue: ['arrayValue[].property'],
+                arrayValue2: ['arrayValue2[]', 'arrayValue2[].*'],
+                arrayValue3: ['arrayValue3[]', 'arrayValue3[].*'],
+            });
         });
 
         describe('should create Datastore Key', () => {
@@ -1102,7 +1109,7 @@ describe('Entity', () => {
         });
 
         it('should update modifiedOn to new Date if property in Schema', () => {
-            schema = new Schema({ modifiedOn: { type: 'datetime' } });
+            schema = new Schema({ modifiedOn: { type: Date } });
             GstoreModel = gstore.model('BlogPost', schema);
             entity = new GstoreModel({});
 
@@ -1114,7 +1121,7 @@ describe('Entity', () => {
         });
 
         it('should convert plain geo object (latitude, longitude) to datastore GeoPoint', () => {
-            schema = new Schema({ location: { type: 'geoPoint' } });
+            schema = new Schema({ location: { type: Schema.Types.GeoPoint } });
             GstoreModel = gstore.model('Car', schema);
             entity = new GstoreModel({
                 location: {
diff --git a/test/model-test.js b/test/model-test.js
@@ -1334,7 +1334,12 @@ describe('Model', () => {
 
             const entity = new GstoreModel({});
 
-            expect(entity.excludeFromIndexes).deep.equal(['lastname', 'age'].concat(arr));
+            expect(entity.excludeFromIndexes).deep.equal({
+                lastname: ['lastname'],
+                age: ['age'],
+                newProp: ['newProp'],
+                url: ['url'],
+            });
             expect(schema.path('newProp').optional).equal(true);
         });
 
@@ -1344,7 +1349,10 @@ describe('Model', () => {
 
             const entity = new GstoreModel({});
 
-            expect(entity.excludeFromIndexes).deep.equal(['lastname', 'age']);
+            expect(entity.excludeFromIndexes).deep.equal({
+                lastname: ['lastname'],
+                age: ['age'],
+            });
             assert.isUndefined(schema.path('lastname').optional);
             expect(schema.path('lastname').excludeFromIndexes).equal(true);
         });
diff --git a/test/serializers/datastore-test.js b/test/serializers/datastore-test.js

Original file line number	Diff line number	Diff line change
`@@ -99,6 +99,6 @@`
`99`	`99`	`"yargs": "^14.0.0"`
`100`	`100`	`},`
`101`	`101`	`"peerDependencies": {`
`102`		`- "@google-cloud/datastore": ">= 3.0.0 < 5"`
	`102`	`+ "@google-cloud/datastore": ">= 4.2.0 < 5"`
`103`	`103`	`}`
`104`	`104`	`}`