Clean up and adds new test cases

src/entity.js

@@ -2359,7 +2359,6 @@ class Entity {
     // change, and we also don't want to trigger the setters of any attributes watching these facets because that
     // should only happen when an attribute is changed.
     const attributesAndComposites = {
-      // ...update.composites,
       ...preparedUpdateValues,
     };
     const {
@@ -3016,7 +3015,7 @@ class Entity {
     let completeFacets = this._expectIndexFacets(
       { ...setAttributes, ...validationAssistance },
       { ...keyAttributes },
-        { set },
+      { set },
     );
 
     let deletedKeys = [];
@@ -3066,13 +3065,13 @@ class Entity {
     let completeFacets = this._expectIndexFacets(
       { ...set },
       { ...composite, ...keyAttributes },
-        { utilizeIncludedOnlyIndexes: true },
+      { utilizeIncludedOnlyIndexes: true },
     );
 
     const removedKeyImpact = this._expectIndexFacets(
       { ...removed },
       { ...keyAttributes },
-        { skipConditionCheck: true }
+      { skipConditionCheck: true }
     );
 
     // complete facets, only includes impacted facets which likely does not include the updateIndex which then needs to be added here.
@@ -3146,7 +3145,7 @@ class Entity {
   /* istanbul ignore next */
   _getIndexImpact(attributes = {}, included = {}, { utilizeIncludedOnlyIndexes, skipConditionCheck } = {}) {
     // beware: this entire algorithm stinks and needs to be completely refactored. It does redundant loops and fights
-    // itself the whole way through. I'm sorry.
+    // itself the whole way through. I am sorry.
     let includedFacets = Object.keys(included);
     let impactedIndexes = {};
     let conditions = {};
@@ -3266,7 +3265,6 @@ class Entity {
     for (const { index, missing, definition } of indexesWithMissingComposites) {
       const indexConditionIsDefined = this._indexConditionIsDefined(index);
 
-
       // `skipConditionCheck` is being used by update `remove`. If Attributes are being removed then the condition check
       // is meaningless and ElectroDB should uphold its obligation to keep keys and attributes in sync.
       // `index === TableIndex` is a special case where we don't need to check the condition because the main table is immutable
@@ -3288,7 +3286,6 @@ class Entity {
             .map(({name}) => name)
 
         if (missingAttributes.length) {
-          // const missingAttributes = missing.length ? missing : definition.all.filter(({name}) => !attributes[name] || !included[name]);
           throw new e.ElectroError(e.ErrorCodes.IncompleteIndexCompositesAttributesProvided, `Incomplete composite attributes provided for index ${index}. Write operations that include composite attributes, for indexes with a condition callback defined, must always provide values for every index composite. This is to ensure consistency between index values and attribute values. Missing composite attributes identified: ${u.commaSeparatedString(missingAttributes)}`);
         }
 

test/ts_connected.entity.spec.ts

@@ -3925,6 +3925,104 @@ describe("index condition", () => {
         });
       }
     });
+
+    const conditionOptions = [
+      [() => true, 'returns true'] as const,
+      [() => false, 'returns false'] as const,
+      [undefined, 'is not defined'] as const,
+    ] as const;
+    // should not use attributes provided via composite to identify a conditional index requires recalculation
+    // `when performing an update that impacts an index with a condition that ${descripto}`
+    describe(`when performing an update set that impacts an index and the provided composite attributes overlap with another index`, () => {
+      for (const [condition1, description1] of conditionOptions) {
+        for (const [condition2, description2] of conditionOptions) {
+          it(`should not throw because it believes additional attributes need to be provided on the overlapped index, when the impacted index has a condition that ${description1} and the overlapped index has a condtion that ${description2}`, () => {
+            const entityName = uuid();
+            const serviceName = uuid();
+
+            const entity = new Entity({
+              model: {
+                version: '0',
+                entity: entityName,
+                service: serviceName,
+              },
+              attributes: {
+                prop1: {
+                  type: 'string'
+                },
+                prop2: {
+                  type: 'string'
+                },
+                prop3: {
+                  type: 'string'
+                },
+                prop4: {
+                  type: 'string'
+                },
+                prop5: {
+                  type: 'string'
+                },
+                prop6: {
+                  type: 'string'
+                },
+                prop7: {
+                  type: 'string'
+                },
+                prop8: {
+                  type: 'string'
+                },
+              },
+              indexes: {
+                test1: {
+                  pk: {
+                    field: 'pk',
+                    composite: ['prop1']
+                  },
+                  sk: {
+                    field: 'sk',
+                    composite: ['prop2']
+                  }
+                },
+                test2: {
+                  condition: condition1,
+                  index: 'gsi1pk-gsi1sk-index',
+                  pk: {
+                    field: 'gsi1pk',
+                    composite: ['prop3', 'prop5']
+                  },
+                  sk: {
+                    field: 'gsi1sk',
+                    composite: ['prop4', 'prop6']
+                  }
+                },
+                test3: {
+                  condition: condition2,
+                  index: 'gsi2pk-gsi2sk-index',
+                  pk: {
+                    field: 'gsi2pk',
+                    composite: ['prop3', 'prop7']
+                  },
+                  sk: {
+                    field: 'gsi2sk',
+                    composite: ['prop4', 'prop8']
+                  }
+                }
+              }
+            }, {client, table});
+
+            expect(() => {
+              entity.update({ prop1: uuid(), prop2: uuid() })
+                  .set({ prop5: uuid(), prop6: uuid() })
+                  // prop3 and prop4 overlap on both test1 and test2 indexes, this should not cause the ElectroDB to throw
+                  // because it thinks test2 should have prop7 and prop8 provided. test2 is not trying to be recalculated,
+                  // and doesn't need to be because composite doesn't mutate, so throwing would be horrible DX.
+                  .composite({ prop3: uuid(), prop4: uuid() })
+                  .params();
+            }).to.not.throw();
+          });
+        }
+      }
+    });
   }
 
   it('should fix gh issue 366', async () => {