Skip to content

Commit 9c7730a

Browse files
fix: Add excludeFromIndexes in the proper places for large properties of nested fields (#1266)
* test: Restrict types inside the save function (#1264) * Add interfaces that restrict values passed to save * Add interfaces for save We need help from the compiler so these interfaces will be useful for determining the shape of the data. * Modify current interfaces to make a closer match The interfaces should match the actual data types that get passed in. * key and data in PrepareEntityObjectResponse option These properties are actually optional * Document all the interfaces added for the save fn * Add two tests showing name/value should be provide * Make the SaveNonArrayData more specific. save doesn’t encode strings or ints or any values that don’t have keys properly so we should restrict the interface to what it does include. * Add header * Test should be more flexible * Add a class for elements with ToString * Define the toString class inline * Eliminate links to points in code * test: Add tests that describe how save should encode excludeFromIndexes for a complex case (#1263) * 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 * 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 * Correct test - it should apply excludeFromIndexes * Update the test for the nested value * 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 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent bbcfc3e commit 9c7730a

File tree

7 files changed

+745
-16
lines changed

7 files changed

+745
-16
lines changed

protos/protos.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/entity.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,6 @@ export namespace entity {
778778
*/
779779
export function entityToEntityProto(entityObject: EntityObject): EntityProto {
780780
const properties = entityObject.data;
781-
const excludeFromIndexes = entityObject.excludeFromIndexes;
782781

783782
const entityProto: EntityProto = {
784783
key: null,
@@ -793,6 +792,15 @@ export namespace entity {
793792
),
794793
};
795794

795+
addExcludeFromIndexes(entityObject.excludeFromIndexes, entityProto);
796+
797+
return entityProto;
798+
}
799+
800+
export function addExcludeFromIndexes(
801+
excludeFromIndexes: string[] | undefined,
802+
entityProto: EntityProto
803+
): EntityProto {
796804
if (excludeFromIndexes && excludeFromIndexes.length > 0) {
797805
excludeFromIndexes.forEach((excludePath: string) => {
798806
excludePathFromEntity(entityProto, excludePath);
@@ -1463,7 +1471,7 @@ export interface ResponseResult {
14631471

14641472
export interface EntityObject {
14651473
data: {[k: string]: Entity};
1466-
excludeFromIndexes: string[];
1474+
excludeFromIndexes?: string[];
14671475
}
14681476

14691477
export interface Json {

src/index.ts

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ import {Transaction} from './transaction';
6767
import {promisifyAll} from '@google-cloud/promisify';
6868
import {google} from '../protos/protos';
6969
import {AggregateQuery} from './aggregate';
70+
import {SaveEntity} from './interfaces/save';
7071

7172
const {grpc} = new GrpcClient();
73+
const addExcludeFromIndexes = entity.addExcludeFromIndexes;
7274

7375
export type PathType = string | number | entity.Int;
7476
export interface BooleanObject {
@@ -1076,7 +1078,7 @@ class Datastore extends DatastoreRequest {
10761078
gaxOptionsOrCallback?: CallOptions | SaveCallback,
10771079
cb?: SaveCallback
10781080
): void | Promise<SaveResponse> {
1079-
entities = arrify(entities);
1081+
entities = arrify(entities) as SaveEntity[];
10801082
const gaxOptions =
10811083
typeof gaxOptionsOrCallback === 'object' ? gaxOptionsOrCallback : {};
10821084
const callback =
@@ -1109,22 +1111,30 @@ class Datastore extends DatastoreRequest {
11091111
}
11101112
}
11111113

1112-
if (entityObject.excludeLargeProperties) {
1113-
entityObject.excludeFromIndexes = entity.findLargeProperties_(
1114-
entityObject.data,
1115-
'',
1116-
entityObject.excludeFromIndexes
1117-
);
1118-
}
1119-
11201114
if (!entity.isKeyComplete(entityObject.key)) {
11211115
insertIndexes[index] = true;
11221116
}
11231117

1124-
// @TODO remove in @google-cloud/[email protected]
1125-
// This was replaced with a more efficient mechanism in the top-level
1126-
// `excludeFromIndexes` option.
11271118
if (Array.isArray(entityObject.data)) {
1119+
// This code populates the excludeFromIndexes list with the right values.
1120+
if (entityObject.excludeLargeProperties) {
1121+
entityObject.data.forEach(
1122+
(data: {
1123+
name: {
1124+
toString(): string;
1125+
};
1126+
value: Entity;
1127+
excludeFromIndexes?: boolean;
1128+
}) => {
1129+
entityObject.excludeFromIndexes = entity.findLargeProperties_(
1130+
data.value,
1131+
data.name.toString(),
1132+
entityObject.excludeFromIndexes
1133+
);
1134+
}
1135+
);
1136+
}
1137+
// This code builds the right entityProto from the entityObject
11281138
entityProto.properties = entityObject.data.reduce(
11291139
(
11301140
acc: EntityProtoReduceAccumulator,
@@ -1155,7 +1165,18 @@ class Datastore extends DatastoreRequest {
11551165
},
11561166
{}
11571167
);
1168+
// This code adds excludeFromIndexes in the right places
1169+
addExcludeFromIndexes(entityObject.excludeFromIndexes, entityProto);
11581170
} else {
1171+
// This code populates the excludeFromIndexes list with the right values.
1172+
if (entityObject.excludeLargeProperties) {
1173+
entityObject.excludeFromIndexes = entity.findLargeProperties_(
1174+
entityObject.data,
1175+
'',
1176+
entityObject.excludeFromIndexes
1177+
);
1178+
}
1179+
// This code builds the right entityProto from the entityObject
11591180
entityProto = entity.entityToEntityProto(entityObject);
11601181
}
11611182

src/interfaces/save.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2024 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import {Entity, entity} from '../entity';
16+
17+
/*
18+
Entity data passed into save in non array form will be of type SaveNonArrayData
19+
and does not require name and value properties.
20+
*/
21+
type SaveNonArrayData = {
22+
[k: string]: Entity;
23+
};
24+
25+
/*
26+
Entity data passed into save in an array form will be of type SaveArrayData
27+
so will have name and value defined because the source code of save requires a
28+
name and a value to be defined or else an error will be thrown.
29+
*/
30+
interface SaveArrayData {
31+
name: {
32+
toString(): string;
33+
};
34+
value: Entity;
35+
excludeFromIndexes?: boolean;
36+
}
37+
38+
/*
39+
When saving an entity, data in the data property of the entity is of type
40+
SaveDataValue. The data can either be in array form in which case it will
41+
match the SaveArrayData[] data type or it can be in non-array form where
42+
it will match the SaveNonArrayData data type.
43+
*/
44+
export type SaveDataValue = SaveArrayData[] | SaveNonArrayData;
45+
46+
/*
47+
An Entity passed into save will include a Key object contained either inside
48+
a `key` property or inside a property indexed by the Key Symbol. If it is
49+
the former then it will be of type SaveEntityWithoutKeySymbol.
50+
*/
51+
interface SaveEntityWithoutKeySymbol {
52+
key: entity.Key;
53+
data: SaveDataValue;
54+
excludeFromIndexes?: string[];
55+
}
56+
57+
/*
58+
An Entity passed into save will include a Key object contained either inside
59+
a `key` property or inside a property indexed by the Key Symbol. If it is
60+
the latter then it will be of type SaveEntityWithKeySymbol.
61+
*/
62+
interface SaveEntityWithKeySymbol {
63+
[entity.KEY_SYMBOL]: entity.Key;
64+
data: SaveDataValue;
65+
}
66+
67+
/*
68+
Entities passed into the first argument of the save function are expected to be
69+
of type SaveEntity[] after being turned into an array. We could change the
70+
signature of save later to enforce this, but doing so would be a breaking change
71+
so we just cast this value to SaveEntity[] for now to enable strong type
72+
enforcement throughout this function.
73+
*/
74+
export type SaveEntity = SaveEntityWithoutKeySymbol | SaveEntityWithKeySymbol;

src/request.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import {RunOptions} from './transaction';
6868
import * as protos from '../protos/protos';
6969
import {serializer} from 'google-gax';
7070
import * as gax from 'google-gax';
71+
import {SaveDataValue} from './interfaces/save';
7172
type JSONValue =
7273
| string
7374
| number
@@ -1412,8 +1413,10 @@ export interface PrepareEntityObject {
14121413
[key: string]: google.datastore.v1.Key | undefined;
14131414
}
14141415
export interface PrepareEntityObjectResponse {
1415-
key?: google.datastore.v1.Key;
1416-
data?: google.datastore.v1.Entity;
1416+
key?: entity.Key;
1417+
data?: SaveDataValue;
1418+
excludeFromIndexes?: string[];
1419+
excludeLargeProperties?: boolean;
14171420
method?: string;
14181421
}
14191422
export interface RequestCallback {

0 commit comments

Comments
 (0)