Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Check property existence for exclude from indexes with wildcard #1114

Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
e02139b
Add a test for testing without mocks
danieljbruce May 24, 2023
f139770
Do a refactor on the test
danieljbruce May 24, 2023
c8dd740
improve the test so that the error bubbles up
danieljbruce May 24, 2023
bdbe4c7
isolate the latter test right now
danieljbruce May 24, 2023
24975b1
test fixes
danieljbruce May 26, 2023
d129b17
Add two more tests for excludeFromIndexes on array
danieljbruce May 26, 2023
860afcb
Move the tests over to the index file
danieljbruce May 26, 2023
099e8ef
Merge branch 'main' into check-property-existence-for-excludeFromInde…
danieljbruce May 26, 2023
73cb4f1
Merge branch 'main' into check-property-existence-for-excludeFromInde…
danieljbruce Sep 28, 2023
1b19906
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce Sep 28, 2023
be6b20e
PR update
danieljbruce Sep 28, 2023
87a3f80
Merge branch 'check-property-existence-for-excludeFromIndexes-with-wi…
danieljbruce Sep 28, 2023
f0900e6
Create a function to refactor the tests
danieljbruce Sep 28, 2023
3e1fa90
All tests for this feature now use same script
danieljbruce Sep 28, 2023
e43a1c3
Add parameterized testing
danieljbruce Sep 28, 2023
b21bd99
Eliminate function and inline code
danieljbruce Sep 28, 2023
234f501
Add JSON stringify to test title
danieljbruce Sep 28, 2023
17a5f2d
Organize code so that similar variables are used
danieljbruce Sep 28, 2023
aedf0c9
inline getExpectedConfig function
danieljbruce Sep 28, 2023
af1130f
Separate tests into blocks and inline code
danieljbruce Sep 28, 2023
b127359
Add types to parameters passed into async
danieljbruce Sep 28, 2023
66d0a37
Eliminate unnecessary variable assignment
danieljbruce Sep 28, 2023
94d7a24
Ran linter and simplified source changes
danieljbruce Sep 28, 2023
d9da7da
Add blank line back for easier diff reading
danieljbruce Sep 28, 2023
0cd7cbc
Try again, eliminate the blank line
danieljbruce Sep 28, 2023
377770b
Additional adjustment to entity first path part
danieljbruce Sep 28, 2023
df3f376
Define isFirstPathPartUndefined
danieljbruce Sep 28, 2023
4f54d94
Rename the variable to align with what it does
danieljbruce Sep 28, 2023
a101f27
run linter
danieljbruce Sep 28, 2023
2253740
Revert "run linter"
danieljbruce Sep 28, 2023
8db651a
Revert "Rename the variable to align with what it does"
danieljbruce Sep 28, 2023
b798ec3
Revert "Define isFirstPathPartUndefined"
danieljbruce Sep 28, 2023
3133879
Refactor check out for seeing if defined
danieljbruce Sep 28, 2023
b148c95
Move comment to more appropriate place
danieljbruce Sep 28, 2023
e0dfef3
Merge branch 'main' of https://github.com/googleapis/nodejs-datastore…
danieljbruce Sep 29, 2023
d1ff7d6
Replace comments with description
danieljbruce Sep 29, 2023
620ee00
Eliminate prefix. Only use description
danieljbruce Sep 29, 2023
e0d770f
fix typo
danieljbruce Sep 29, 2023
e1aecb6
lowercase convention
danieljbruce Sep 29, 2023
d75a732
Merge branch 'main' into check-property-existence-for-excludeFromInde…
danieljbruce Sep 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions src/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@ export namespace entity {
if (
firstPathPartIsArray &&
// check also if the property in question is actually an array value.
typeof entity.properties![firstPathPart] !== 'undefined' &&
ruyadorno marked this conversation as resolved.
Show resolved Hide resolved
entity.properties![firstPathPart].arrayValue &&
// check if wildcard is not applied
!hasWildCard
Expand All @@ -879,7 +880,12 @@ export namespace entity {
);
}
});
} else if (firstPathPartIsArray && hasWildCard && remainderPath === '*') {
} else if (
firstPathPartIsArray &&
hasWildCard &&
remainderPath === '*' &&
typeof entity.properties![firstPathPart] !== 'undefined'
) {
const array = entity.properties![firstPathPart].arrayValue;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
array.values.forEach((value: any) => {
Expand All @@ -898,22 +904,26 @@ export namespace entity {
excludePathFromEntity(entity, newPath);
});
} else {
if (hasWildCard && remainderPath === '*') {
const parentEntity = entity.properties![firstPathPart].entityValue;

if (parentEntity) {
Object.keys(parentEntity.properties).forEach(path => {
const newPath = parentEntity.properties[path].arrayValue
? path + '[].*'
: path + '.*';
excludePathFromEntity(parentEntity, newPath);
});
if (typeof entity.properties![firstPathPart] !== 'undefined') {
if (hasWildCard && remainderPath === '*') {
ruyadorno marked this conversation as resolved.
Show resolved Hide resolved
const parentEntity =
entity.properties![firstPathPart].entityValue;

if (parentEntity) {
Object.keys(parentEntity.properties).forEach(path => {
const newPath = parentEntity.properties[path].arrayValue
? path + '[].*'
: path + '.*';
excludePathFromEntity(parentEntity, newPath);
});
} else {
excludePathFromEntity(entity, firstPathPart);
}
} else {
excludePathFromEntity(entity, firstPathPart);
const parentEntity =
entity.properties![firstPathPart].entityValue;
excludePathFromEntity(parentEntity, remainderPath);
}
} else {
const parentEntity = entity.properties![firstPathPart].entityValue;
excludePathFromEntity(parentEntity, remainderPath);
}
}
}
Expand Down
182 changes: 180 additions & 2 deletions test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import * as proxyquire from 'proxyquire';
import {PassThrough, Readable} from 'stream';

import * as ds from '../src';
import {DatastoreOptions} from '../src';
import {Datastore, DatastoreOptions} from '../src';
import {Datastore as OriginalDatastore} from '../src';
import {entity, Entity, EntityProto, EntityObject} from '../src/entity';
import {RequestConfig} from '../src/request';
import {RequestCallback, RequestConfig} from '../src/request';
import * as is from 'is';
import * as sinon from 'sinon';
import * as extend from 'extend';
Expand Down Expand Up @@ -2150,4 +2151,181 @@ describe('Datastore', () => {
assert.strictEqual(key.name, 'Test');
});
});

describe('Without using mocks', () => {
const Datastore = OriginalDatastore;
describe('using save to evaluate excludeFromIndexes', () => {
function getExpectedConfig(
properties: any,
namespace: string | undefined
) {
return {
client: 'DatastoreClient',
method: 'commit',
gaxOpts: {},
reqOpts: {
mutations: [
{
upsert: {
key: {
path: [{kind: 'Post', name: 'Post1'}],
partitionId: {
namespaceId: namespace,
},
},
properties,
},
},
],
},
};
}
describe('when the property is contained in excludeFromIndexes', () => {
const datastore = new Datastore({
namespace: `${Date.now()}`,
});
const namespace = datastore.namespace;
const key = datastore.key(['Post', 'Post1']);
it('should encode a request without excludeFromIndexes', async () => {
const expectedConfig = getExpectedConfig(
{k: {stringValue: 'v'}},
namespace
);
datastore.request_ = (
config: RequestConfig,
callback: RequestCallback
) => {
try {
assert.deepStrictEqual(config, expectedConfig);
callback(null, 'some-data');
} catch (e: any) {
callback(e);
}
};
const results = await datastore.save({
key,
data: {k: 'v'},
});
assert.deepStrictEqual(results, ['some-data']);
});
it('should ignore non-existent property in excludeFromIndexes', async () => {
const expectedConfig = getExpectedConfig(
{k: {stringValue: 'v', excludeFromIndexes: true}},
namespace
);
datastore.request_ = (
config: RequestConfig,
callback: RequestCallback
) => {
try {
assert.deepStrictEqual(config, expectedConfig);
callback(null, 'some-data');
} catch (e: any) {
callback(e);
}
};
const results = await datastore.save({
key,
data: {k: 'v'},
excludeFromIndexes: ['k', 'k.*'],
});
assert.deepStrictEqual(results, ['some-data']);
});
});
describe('when the property is not contained in excludeFromIndexes', () => {
const datastore = new Datastore({
namespace: `${Date.now()}`,
});
const namespace = datastore.namespace;
const expectedConfig = getExpectedConfig({}, namespace);
datastore.request_ = (
config: RequestConfig,
callback: RequestCallback
) => {
try {
assert.deepStrictEqual(config, expectedConfig);
callback(null, 'some-data');
} catch (e: any) {
callback(e);
}
};
const key = datastore.key(['Post', 'Post1']);
it('should encode a request without excludeFromIndexes', async () => {
const results = await datastore.save({
key,
data: {},
});
assert.deepStrictEqual(results, ['some-data']);
});
it('should ignore non-existent property in excludeFromIndexes', async () => {
const results = await datastore.save({
key,
data: {},
excludeFromIndexes: [
'non_exist_property', // this just ignored
'non_exist_property.*', // should also be ignored
],
});
assert.deepStrictEqual(results, ['some-data']);
});
});
describe('when the property is not contained in excludeFromIndexes and the property is an array', () => {
const datastore = new Datastore({
namespace: `${Date.now()}`,
});
const namespace = datastore.namespace;
const key = datastore.key(['Post', 'Post1']);
it('should encode a request when there is no wildcard', async () => {
const expectedConfig = getExpectedConfig(
{k: {stringValue: 'v'}},
namespace
);
datastore.request_ = (
config: RequestConfig,
callback: RequestCallback
) => {
try {
assert.deepStrictEqual(config, expectedConfig);
callback(null, 'some-data');
} catch (e: any) {
callback(e);
}
};
const results = await datastore.save({
key,
data: {k: 'v'},
excludeFromIndexes: [
'non_exist_property[]', // this just ignored
],
});
assert.deepStrictEqual(results, ['some-data']);
});
it('should encode a request when using a wildcard', async () => {
const expectedConfig = getExpectedConfig(
{k: {stringValue: 'v'}},
namespace
);
datastore.request_ = (
config: RequestConfig,
callback: RequestCallback
) => {
try {
assert.deepStrictEqual(config, expectedConfig);
callback(null, 'some-data');
} catch (e: any) {
callback(e);
}
};
const results = await datastore.save({
key,
data: {k: 'v'},
excludeFromIndexes: [
'non_exist_property[].*', // this just ignored
],
});
assert.deepStrictEqual(results, ['some-data']);
});
});
});
});
});