Merge pull request #10 from GitLiveApp/delete-field-from-target-when-not-in-source

nbransby · web-flow · commit fe29649e3516 · 2020-07-23T12:33:23.000+01:00
Delete field from target when not in source
diff --git a/README.md b/README.md
@@ -25,14 +25,18 @@ integrify({ config: { functions, db } });
 module.exports.replicateMasterToDetail = integrify({
   rule: 'REPLICATE_ATTRIBUTES',
   source: {
-    collection: 'master',
+    source: {
+    collection: 'master', // <-- This will append {masterId}
+    // OR
+    collection: 'master/{masterId}', // <-- Can be any string as in Firebase
+  },
   },
   targets: [
     {
       collection: 'detail1',
       foreignKey: 'masterId',
       attributeMapping: {
-        masterField1: 'detail1Field1',
+        masterField1: 'detail1Field1', // If an field is missing after the update, the field will be deleted
         masterField2: 'detail1Field2',
       },
     },
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "integrify",
-  "version": "3.0.1",
+  "version": "4.0.0",
   "description": "Enforce referential integrity in Firestore using Cloud Functions",
   "keywords": [
     "firebase",
diff --git a/src/rules/replicateAttributes.ts b/src/rules/replicateAttributes.ts
@@ -1,4 +1,6 @@
-import { Config, Rule } from '../common';
+import { Config, Rule, getPrimaryKey } from '../common';
+import { firestore } from 'firebase-admin';
+const FieldValue = firestore.FieldValue;
 
 export interface ReplicateAttributesRule extends Rule {
   source: {
@@ -40,6 +42,11 @@ export function integrifyReplicateAttributes(
     });
   });
 
+  const { hasPrimaryKey, primaryKey } = getPrimaryKey(rule.source.collection);
+  if (!hasPrimaryKey) {
+    rule.source.collection = `${rule.source.collection}/{${primaryKey}}`;
+  }
+
   // Create map of master attributes to track for replication
   const trackedMasterAttributes = {};
   rule.targets.forEach(target => {
@@ -49,12 +56,18 @@ export function integrifyReplicateAttributes(
   });
 
   return functions.firestore
-    .document(`${rule.source.collection}/{masterId}`)
+    .document(rule.source.collection)
     .onUpdate((change, context) => {
-      const masterId = context.params.masterId;
+      // Get the last {...} in the source collection
+      const primaryKeyValue = context.params[primaryKey];
+      if (!primaryKeyValue) {
+        throw new Error(
+          `integrify: Missing a primary key [${primaryKey}] in the source params`
+        );
+      }
       const newValue = change.after.data();
       console.log(
-        `integrify: Detected update in [${rule.source.collection}], id [${masterId}], new value:`,
+        `integrify: Detected update in [${rule.source.collection}], id [${primaryKeyValue}], new value:`,
         newValue
       );
 
@@ -80,19 +93,19 @@ export function integrifyReplicateAttributes(
         return null;
       }
 
-      // Loop over each target specification to replicate atributes
+      // Loop over each target specification to replicate attributes
       const db = config.config.db;
       rule.targets.forEach(target => {
         const targetCollection = target.collection;
         const update = {};
 
-        // Create "update" mapping each changed attribute from source => target
-        Object.keys(newValue).forEach(changedAttribute => {
-          if (target.attributeMapping[changedAttribute]) {
-            update[target.attributeMapping[changedAttribute]] =
-              newValue[changedAttribute];
-          }
+        // Create "update" mapping each changed attribute from source => target,
+        // if delete is set delete field
+        Object.keys(target.attributeMapping).forEach(changedAttribute => {
+          update[target.attributeMapping[changedAttribute]] =
+            newValue[changedAttribute] || FieldValue.delete();
         });
+
         console.log(
           `integrify: On collection ${
             target.isCollectionGroup ? 'group ' : ''
@@ -110,7 +123,7 @@ export function integrifyReplicateAttributes(
         }
         promises.push(
           whereable
-            .where(target.foreignKey, '==', masterId)
+            .where(target.foreignKey, '==', primaryKeyValue)
             .get()
             .then(detailDocs => {
               detailDocs.forEach(detailDoc => {
diff --git a/test/functions/index.js b/test/functions/index.js
@@ -39,6 +39,55 @@ module.exports.replicateMasterToDetail = integrify({
   },
 });
 
+module.exports.replicateMasterDeleteWhenEmpty = integrify({
+  rule: 'REPLICATE_ATTRIBUTES',
+  source: {
+    collection: 'master/{primaryKey}',
+  },
+  targets: [
+    {
+      collection: 'detail1',
+      foreignKey: 'tempId',
+      attributeMapping: {
+        masterDetail1: 'foreignDetail1',
+        masterDetail2: 'foreignDetail2',
+      },
+    },
+  ],
+  hooks: {
+    pre: (change, context) => {
+      setState({
+        change,
+        context,
+      });
+    },
+  },
+});
+
+module.exports.replicateReferencesWithMissingKey = integrify({
+  rule: 'REPLICATE_ATTRIBUTES',
+  source: {
+    collection: 'master/{masterId}',
+  },
+  targets: [
+    {
+      collection: 'detail1',
+      foreignKey: 'randomId',
+      attributeMapping: {
+        masterDetail1: 'foreignDetail1',
+      },
+    },
+  ],
+  hooks: {
+    pre: (snap, context) => {
+      setState({
+        snap,
+        context,
+      });
+    },
+  },
+});
+
 module.exports.deleteReferencesToMaster = integrify({
   rule: 'DELETE_REFERENCES',
   source: {
diff --git a/test/functions/integrify.rules.js b/test/functions/integrify.rules.js
@@ -27,6 +27,39 @@ module.exports = [
       },
     ],
   },
+  {
+    rule: 'REPLICATE_ATTRIBUTES',
+    name: 'replicateMasterDeleteWhenEmpty',
+    source: {
+      collection: 'master/{primaryKey}',
+    },
+    targets: [
+      {
+        collection: 'detail1',
+        foreignKey: 'tempId',
+        attributeMapping: {
+          masterDetail1: 'foreignDetail1',
+          masterDetail2: 'foreignDetail2',
+        },
+      },
+    ],
+  },
+  {
+    rule: 'REPLICATE_ATTRIBUTES',
+    name: 'replicateReferencesWithMissingKey',
+    source: {
+      collection: 'master/{masterId}',
+    },
+    targets: [
+      {
+        collection: 'detail1',
+        foreignKey: 'randomId',
+        attributeMapping: {
+          masterDetail1: 'foreignDetail1',
+        },
+      },
+    ],
+  },
   {
     rule: 'DELETE_REFERENCES',
     name: 'deleteReferencesToMaster',
diff --git a/test/unit.test.js b/test/unit.test.js
@@ -17,7 +17,7 @@ admin.initializeApp({
 const db = admin.firestore();
 
 async function clearFirestore() {
-  const collections = ['detail1', 'detail2', 'somecoll'];
+  const collections = ['detail1', 'detail2', 'detail3', 'somecoll'];
   for (const collection of collections) {
     const { docs } = await admin
       .firestore()
@@ -55,12 +55,20 @@ testsuites.forEach(testsuite => {
     testPrimaryKey(sut, t, name));
   test(`[${name}] test target collection parameter swap`, async t =>
     testTargetVariableSwap(sut, t, name));
+
+  // Standard functionality
   test(`[${name}] test replicate attributes`, async t =>
     testReplicateAttributes(sut, t, name));
   test(`[${name}] test delete references`, async t =>
     testDeleteReferences(sut, t, name));
   test(`[${name}] test maintain count`, async t => testMaintainCount(sut, t));
 
+  // Added by GitLive
+  test(`[${name}] test replicate attributes delete when field is not there`, async t =>
+    testReplicateAttributesDeleteEmpty(sut, t, name));
+  test(`[${name}] test replicate attributes with missing primary key in source reference`, async t =>
+    testReplicateMissingSourceCollectionKey(sut, t, name));
+
   test(`[${name}] test delete with masterId in target reference`, async t =>
     testDeleteParamReferences(sut, t, name));
   test(`[${name}] test delete with snapshot fields in target reference`, async t =>
@@ -72,7 +80,6 @@ testsuites.forEach(testsuite => {
 
   test(`[${name}] test delete all sub-collections in target reference`, async t =>
     testDeleteAllSubCollections(sut, t, name));
-
   test(`[${name}] test delete missing arguments error`, async t =>
     testDeleteMissingArgumentsError(sut, t, name));
 });
@@ -163,7 +170,9 @@ async function testReplicateAttributes(sut, t, name) {
 
   // Call trigger to replicate attributes from master
   const beforeSnap = fft.firestore.makeDocumentSnapshot(
-    {},
+    {
+      masterField5: 'missing',
+    },
     `master/${masterId}`
   );
   const afterSnap = fft.firestore.makeDocumentSnapshot(
@@ -226,6 +235,137 @@ async function testReplicateAttributes(sut, t, name) {
   await t.pass();
 }
 
+async function testReplicateAttributesDeleteEmpty(sut, t, name) {
+  // Add a couple of detail documents to follow master
+  const primaryKey = makeid();
+  await db.collection('detail1').add({
+    tempId: primaryKey,
+    foreignDetail1: 'foreign_detail_1',
+    foreignDetail2: 'foreign_detail_2',
+  });
+
+  // Call trigger to replicate attributes from master
+  const beforeSnap = fft.firestore.makeDocumentSnapshot(
+    {
+      masterDetail1: 'after1',
+      masterDetail2: 'after2',
+    },
+    `master/${primaryKey}`
+  );
+  const afterSnap = fft.firestore.makeDocumentSnapshot(
+    {
+      masterDetail2: 'after3',
+    },
+    `master/${primaryKey}`
+  );
+  const change = fft.makeChange(beforeSnap, afterSnap);
+  const wrapped = fft.wrap(sut.replicateMasterDeleteWhenEmpty);
+  setState({
+    change: null,
+    context: null,
+  });
+  await wrapped(change, {
+    params: {
+      primaryKey: primaryKey,
+    },
+  });
+
+  // Assert pre-hook was called (only for rules-in-situ)
+  if (name === 'rules-in-situ') {
+    const state = getState();
+    t.truthy(state.change);
+    t.truthy(state.context);
+    t.is(state.context.params.primaryKey, primaryKey);
+  }
+
+  // Assert that attributes get replicated to detail documents
+  await assertQuerySizeEventually(
+    db
+      .collection('detail1')
+      .where('tempId', '==', primaryKey)
+      .where('foreignDetail1', '==', 'foreign_detail_1'),
+    0
+  );
+  await assertQuerySizeEventually(
+    db
+      .collection('detail1')
+      .where('tempId', '==', primaryKey)
+      .where('foreignDetail2', '==', 'after3'),
+    1
+  );
+
+  await t.pass();
+}
+
+async function testReplicateMissingSourceCollectionKey(sut, t, name) {
+  // Create some docs referencing master doc
+  const randomId = makeid();
+  await db.collection('detail1').add({
+    tempId: randomId,
+    foreignDetail1: 'foreign_detail_1',
+    foreignDetail2: 'foreign_detail_2',
+  });
+
+  // Trigger function to delete references
+  const beforeSnap = fft.firestore.makeDocumentSnapshot(
+    {
+      masterDetail1: 'after1',
+      masterDetail2: 'after2',
+    },
+    `master/${randomId}`
+  );
+  const afterSnap = fft.firestore.makeDocumentSnapshot(
+    {
+      masterDetail2: 'after3',
+    },
+    `master/${randomId}`
+  );
+  const change = fft.makeChange(beforeSnap, afterSnap);
+  const wrapped = fft.wrap(sut.replicateReferencesWithMissingKey);
+  setState({
+    snap: null,
+    context: null,
+  });
+
+  const error = await t.throwsAsync(async () => {
+    await wrapped(change, {
+      params: {
+        randomId: randomId,
+      },
+    });
+  });
+
+  t.is(
+    error.message,
+    'integrify: Missing a primary key [masterId] in the source params'
+  );
+
+  // Assert pre-hook was called (only for rules-in-situ)
+  if (name === 'rules-in-situ') {
+    const state = getState();
+    t.is(state.snap, null);
+    t.is(state.context, null);
+  }
+
+  // Assert referencing docs were not deleted
+  await assertQuerySizeEventually(
+    db
+      .collection('detail1')
+      .where('tempId', '==', randomId)
+      .where('foreignDetail1', '==', 'foreign_detail_1'),
+    1
+  );
+  await assertQuerySizeEventually(
+    db
+      .collection('detail1')
+      .where('tempId', '==', randomId)
+      .where('foreignDetail2', '==', 'foreign_detail_2'),
+    1
+  );
+
+  t.pass();
+}
+
 async function testDeleteReferences(sut, t, name) {
   // Create some docs referencing master doc
   const masterId = makeid();

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "integrify",`
`3`		`- "version": "3.0.1",`
	`3`	`+ "version": "4.0.0",`
`4`	`4`	`"description": "Enforce referential integrity in Firestore using Cloud Functions",`
`5`	`5`	`"keywords": [`
`6`	`6`	`"firebase",`