fix: Handle excludeLarge properties for arrays properly (#1265)

* Write a sample test for what save output should be

* Add a test for a bunch of complex excludeFromIndex

* Change the replace function so it works everywhere

* Modify the expected value

Make it contain longStringArray

* Correct the test for evaluating arrays

* Break up the entityToEntityProto into separate par

* Fix the tests to work with new addExcludeFromIndex

* Break up the entityToEntityProto into separate par

* Fix the tests to work with new addExcludeFromIndex

* Revert "Break up the entityToEntityProto into separate par"

This reverts commit 60dabd7.

* Revert "Fix the tests to work with new addExcludeFromIndex"

This reverts commit ba4e82b.

* Remove source code changes

* Skip the test that looks at arrays

* Remove some of the wildcard indexes

This is in excludeFromIndexes - replace them with literals.

* Remove another boolean value from excludeFromIndex

* Eliminate duplicate expectedMutations

* Eliminate duplicate runTest function

* Begin to set up the async tests

* Pack all tests into the async framework

* Delete tests addressed by async

* Get rid of test functions and inline everything

* Add comment

* Add comments describing each test case

* Remove only

* Add another test looks at name/value not in array

* Add a test for excludeLarge properties and name/va

* Fix the test so that the array case matches

Matches the single case

* should pass the right properties for an array

* Rename the test

* Change name to entityName

* Add two tests that capture the array encoding

encoding problem

* Change expected output of arrays

* Remove only

* Add 2 more tests to ensure behaviour is preserved

* Make argument more specific for excludeFromIndexes

* Use excludeLargeProperties for array/non-array cas

* Fix the test to include entity proto

* Correct mistakes in initial implementation

* Don’t skip any of the tests

* Remove only

* Add check for excludeLargeProperties

* Remove TODO

src/entity.ts

@@ -778,7 +778,6 @@ export namespace entity {
    */
   export function entityToEntityProto(entityObject: EntityObject): EntityProto {
     const properties = entityObject.data;
-    const excludeFromIndexes = entityObject.excludeFromIndexes;
 
     const entityProto: EntityProto = {
       key: null,
@@ -793,6 +792,15 @@ export namespace entity {
       ),
     };
 
+    addExcludeFromIndexes(entityObject.excludeFromIndexes, entityProto);
+
+    return entityProto;
+  }
+
+  export function addExcludeFromIndexes(
+    excludeFromIndexes: string[] | undefined,
+    entityProto: EntityProto
+  ): EntityProto {
     if (excludeFromIndexes && excludeFromIndexes.length > 0) {
       excludeFromIndexes.forEach((excludePath: string) => {
         excludePathFromEntity(entityProto, excludePath);

src/index.ts

@@ -70,6 +70,7 @@ import {AggregateQuery} from './aggregate';
 import {SaveEntity} from './interfaces/save';
 
 const {grpc} = new GrpcClient();
+const addExcludeFromIndexes = entity.addExcludeFromIndexes;
 
 export type PathType = string | number | entity.Int;
 export interface BooleanObject {
@@ -1110,22 +1111,30 @@ class Datastore extends DatastoreRequest {
           }
         }
 
-        if (entityObject.excludeLargeProperties) {
-          entityObject.excludeFromIndexes = entity.findLargeProperties_(
-            entityObject.data,
-            '',
-            entityObject.excludeFromIndexes
-          );
-        }
-
         if (!entity.isKeyComplete(entityObject.key)) {
           insertIndexes[index] = true;
         }
 
-        // @TODO remove in @google-cloud/[email protected]
-        // This was replaced with a more efficient mechanism in the top-level
-        // `excludeFromIndexes` option.
         if (Array.isArray(entityObject.data)) {
+          // This code populates the excludeFromIndexes list with the right values.
+          if (entityObject.excludeLargeProperties) {
+            entityObject.data.forEach(
+              (data: {
+                name: {
+                  toString(): string;
+                };
+                value: Entity;
+                excludeFromIndexes?: boolean;
+              }) => {
+                entityObject.excludeFromIndexes = entity.findLargeProperties_(
+                  data.value,
+                  data.name.toString(),
+                  entityObject.excludeFromIndexes
+                );
+              }
+            );
+          }
+          // This code builds the right entityProto from the entityObject
           entityProto.properties = entityObject.data.reduce(
             (
               acc: EntityProtoReduceAccumulator,
@@ -1156,7 +1165,18 @@ class Datastore extends DatastoreRequest {
             },
             {}
           );
+          // This code adds excludeFromIndexes in the right places
+          addExcludeFromIndexes(entityObject.excludeFromIndexes, entityProto);
         } else {
+          // This code populates the excludeFromIndexes list with the right values.
+          if (entityObject.excludeLargeProperties) {
+            entityObject.excludeFromIndexes = entity.findLargeProperties_(
+              entityObject.data,
+              '',
+              entityObject.excludeFromIndexes
+            );
+          }
+          // This code builds the right entityProto from the entityObject
           entityProto = entity.entityToEntityProto(entityObject);
         }
 

test/gapic-mocks/commit.ts

@@ -28,31 +28,6 @@ describe('Commit', () => {
   const clientName = 'DatastoreClient';
   const datastore = getInitializedDatastoreClient();
 
-  function setCommitComparison(
-    compareFn: (request: protos.google.datastore.v1.ICommitRequest) => void
-  ) {
-    const dataClient = datastore.clients_.get(clientName);
-    if (dataClient) {
-      dataClient.commit = (
-        request: protos.google.datastore.v1.ICommitRequest,
-        options: CallOptions,
-        callback: (
-          err?: unknown,
-          res?: protos.google.datastore.v1.ICommitResponse
-        ) => void
-      ) => {
-        try {
-          compareFn(request);
-        } catch (e) {
-          callback(e);
-        }
-        callback(null, {
-          mutationResults: [],
-        });
-      };
-    }
-  }
-
   const key = {
     path: [
       {
@@ -315,7 +290,7 @@ describe('Commit', () => {
         },
         {
           name: 'should pass the right properties for a simple name/value pair in an array with excludeFromIndexes list',
-          skipped: true,
+          skipped: false,
           entities: [
             {
               name: 'entityName',
@@ -380,7 +355,7 @@ describe('Commit', () => {
         {
           // This test case reproduces https://github.com/googleapis/nodejs-datastore/issues/1242
           name: 'should pass the right request with a nested field',
-          skipped: true,
+          skipped: false,
           entities: [
             {
               name: 'field_b',
@@ -480,7 +455,7 @@ describe('Commit', () => {
         {
           // Just like the previous test, but entities are wrapped in an array
           name: 'should pass the right properties for an array with name/value pairs and excludeLargeProperties',
-          skipped: true,
+          skipped: false,
           entities: [
             {
               name: 'entityName',

test/index.ts

@@ -89,6 +89,7 @@ const fakeEntityInit: any = {
   keyToKeyProto: entity.keyToKeyProto,
   encodeValue: entity.encodeValue,
   entityToEntityProto: entity.entityToEntityProto,
+  addExcludeFromIndexes: entity.addExcludeFromIndexes,
   findLargeProperties_: entity.findLargeProperties_,
   URLSafeKey: entity.URLSafeKey,
 };