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 all 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
14 changes: 11 additions & 3 deletions src/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -852,8 +852,11 @@ export namespace entity {
return;
}

const isFirstPathPartDefined =
entity.properties![firstPathPart] !== undefined;
if (
firstPathPartIsArray &&
isFirstPathPartDefined &&
// check also if the property in question is actually an array value.
entity.properties![firstPathPart].arrayValue &&
// check if wildcard is not applied
Expand All @@ -879,7 +882,12 @@ export namespace entity {
);
}
});
} else if (firstPathPartIsArray && hasWildCard && remainderPath === '*') {
} else if (
firstPathPartIsArray &&
hasWildCard &&
remainderPath === '*' &&
isFirstPathPartDefined
) {
const array = entity.properties![firstPathPart].arrayValue;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
array.values.forEach((value: any) => {
Expand All @@ -898,7 +906,7 @@ export namespace entity {
excludePathFromEntity(entity, newPath);
});
} else {
if (hasWildCard && remainderPath === '*') {
if (hasWildCard && remainderPath === '*' && isFirstPathPartDefined) {
const parentEntity = entity.properties![firstPathPart].entityValue;

if (parentEntity) {
Expand All @@ -911,7 +919,7 @@ export namespace entity {
} else {
excludePathFromEntity(entity, firstPathPart);
}
} else {
} else if (isFirstPathPartDefined) {
const parentEntity = entity.properties![firstPathPart].entityValue;
excludePathFromEntity(parentEntity, remainderPath);
}
Expand Down
136 changes: 133 additions & 3 deletions test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,23 @@ import {PassThrough, Readable} from 'stream';

import * as ds from '../src';
import {Datastore, DatastoreOptions} from '../src';
import {entity, Entity, EntityProto, EntityObject} from '../src/entity';
import {RequestConfig} from '../src/request';
import {Datastore as OriginalDatastore} from '../src';
import {
entity,
Entity,
EntityProto,
EntityObject,
Entities,
} from '../src/entity';
import {RequestCallback, RequestConfig} from '../src/request';
import * as is from 'is';
import * as sinon from 'sinon';
import * as extend from 'extend';
const async = require('async');
import {google} from '../protos/protos';

// eslint-disable-next-line @typescript-eslint/no-var-requires
const v1 = require('../src/v1/index.js');
const async = require('async');

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const fakeEntityInit: any = {
Expand Down Expand Up @@ -2319,6 +2327,128 @@ async.each(
});
});

describe('without using mocks', () => {
describe('on save tests', () => {
const onSaveTests = [
{
description:
'should encode a save request without excludeFromIndexes',
properties: {k: {stringValue: 'v'}},
entitiesWithoutKey: {data: {k: 'v'}},
},
{
description:
'should add exclude from indexes to property k and ignore excludeFromIndexes with wildcard',
properties: {k: {stringValue: 'v', excludeFromIndexes: true}},
entitiesWithoutKey: {
data: {k: 'v'},
excludeFromIndexes: ['k', 'k.*'],
},
},
{
description:
'should encode a save request without properties and without excludeFromIndexes',
properties: {},
entitiesWithoutKey: {data: {}},
},
{
description:
'should encode a save request with no properties ignoring excludeFromIndexes for a property not on save data',
properties: {},
entitiesWithoutKey: {
data: {},
excludeFromIndexes: [
'non_exist_property', // this just ignored
'non_exist_property.*', // should also be ignored
],
},
},
{
description:
'should encode a save request with one property ignoring excludeFromIndexes for a property not on save data',
properties: {k: {stringValue: 'v'}},
entitiesWithoutKey: {
data: {k: 'v'},
excludeFromIndexes: [
'non_exist_property[]', // this just ignored
],
},
},
{
description:
'should encode a save request with one property ignoring excludeFromIndexes for a property with a wildcard not on save data',
properties: {k: {stringValue: 'v'}},
entitiesWithoutKey: {
data: {k: 'v'},
excludeFromIndexes: [
'non_exist_property[].*', // this just ignored
],
},
},
];

async.each(
onSaveTests,
(onSaveTest: {
description: string;
properties: google.datastore.v1.IValue;
entitiesWithoutKey: Entities;
}) => {
it(`${onSaveTest.description}`, async () => {
const datastore = new OriginalDatastore({
namespace: `${Date.now()}`,
});
{
// This block of code mocks out request_ to check values passed into it.
const expectedConfig = {
client: 'DatastoreClient',
method: 'commit',
gaxOpts: {},
reqOpts: {
mutations: [
{
upsert: {
key: {
path: [{kind: 'Post', name: 'Post1'}],
partitionId: {
namespaceId: datastore.namespace,
},
},
properties: onSaveTest.properties,
},
},
],
},
};
// Mock out the request function to compare config passed into it.
datastore.request_ = (
config: RequestConfig,
callback: RequestCallback
) => {
try {
assert.deepStrictEqual(config, expectedConfig);
callback(null, 'some-data');
} catch (e: any) {
callback(e);
}
};
}
{
// Attach key to entities parameter passed in and run save with those parameters.
const key = datastore.key(['Post', 'Post1']);
const entities = Object.assign(
{key},
onSaveTest.entitiesWithoutKey
);
const results = await datastore.save(entities);
assert.deepStrictEqual(results, ['some-data']);
}
});
}
);
});
});

describe('multi-db support', () => {
it('should get the database id from the client', async () => {
const otherDatastore = new Datastore({
Expand Down
Loading