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

feat: add warning when int value is too large during upload #591

Merged
merged 9 commits into from
Feb 27, 2020
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

# [Google Cloud Datastore: Node.js Client](https://github.com/googleapis/nodejs-datastore)

[![release level](https://img.shields.io/badge/release%20level-general%20availability%20%28GA%29-brightgreen.svg?style=flat)](https://cloud.google.com/terms/launch-stages)
[![release level](https://img.shields.io/badge/release%20level-general%20availability%20%28GA%29-brightgreen.svg?style=flat)](https://cloud.google.com/products#product-launch-stages)
[![npm version](https://img.shields.io/npm/v/@google-cloud/datastore.svg)](https://www.npmjs.org/package/@google-cloud/datastore)
[![codecov](https://img.shields.io/codecov/c/github/googleapis/nodejs-datastore/master.svg?style=flat)](https://codecov.io/gh/googleapis/nodejs-datastore)

Expand Down Expand Up @@ -132,7 +132,7 @@ are addressed with the highest priority.

More Information: [Google Cloud Platform Launch Stages][launch_stages]

[launch_stages]: https://cloud.google.com/terms/launch-stages
[launch_stages]: https://cloud.google.com/products#product-launch-stages

## Contributing

Expand Down
21 changes: 15 additions & 6 deletions src/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ export namespace entity {
* // }
*/
// tslint:disable-next-line no-any
export function encodeValue(value?: any): ValueProto {
export function encodeValue(value: any, property: string): ValueProto {
const valueProto: ValueProto = {};

if (is.boolean(value)) {
Expand All @@ -579,7 +579,16 @@ export namespace entity {
}

if (typeof value === 'number') {
if (value % 1 === 0) {
if (Number.isInteger(value)) {
if (!Number.isSafeInteger(value)) {
process.emitWarning(
'IntegerOutOfBoundsWarning: ' +
"the value for '" +
property +
"' property is outside of bounds of a JavaScript Number.\n" +
"Use 'Datastore.int(<integer_value_as_string>)' to preserve accuracy during the upload."
);
}
value = new entity.Int(value);
} else {
value = new entity.Double(value);
Expand Down Expand Up @@ -624,7 +633,7 @@ export namespace entity {

if (Array.isArray(value)) {
valueProto.arrayValue = {
values: value.map(entity.encodeValue),
values: value.map(val => entity.encodeValue(val, property)),
};
return valueProto;
}
Expand All @@ -640,7 +649,7 @@ export namespace entity {

for (const prop in value) {
if (value.hasOwnProperty(prop)) {
value[prop] = entity.encodeValue(value[prop]);
value[prop] = entity.encodeValue(value[prop], prop);
}
}
}
Expand Down Expand Up @@ -744,7 +753,7 @@ export namespace entity {

properties: Object.keys(properties).reduce(
(encoded, key) => {
encoded[key] = entity.encodeValue(properties[key]);
encoded[key] = entity.encodeValue(properties[key], key);
return encoded;
},
// tslint:disable-next-line no-any
Expand Down Expand Up @@ -1204,7 +1213,7 @@ export namespace entity {
if (filter.name === '__key__') {
value.keyValue = entity.keyToKeyProto(filter.val);
} else {
value = entity.encodeValue(filter.val);
value = entity.encodeValue(filter.val, filter.name);
}

return {
Expand Down
5 changes: 4 additions & 1 deletion src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,10 @@ class DatastoreRequest {
acc: EntityProtoReduceAccumulator,
data: EntityProtoReduceData
) => {
const value = entity.encodeValue(data.value);
const value = entity.encodeValue(
data.value,
data.name.toString()
);

if (typeof data.excludeFromIndexes === 'boolean') {
const excluded = data.excludeFromIndexes;
Expand Down
11 changes: 11 additions & 0 deletions system-test/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,17 @@ describe('Datastore', () => {
await datastore.delete(postKey);
});

it('should accurately save/get a large int value via Datastore.int()', async () => {
const postKey = datastore.key('Team');
const largeIntValueAsString = '9223372036854775807';
const points = Datastore.int(largeIntValueAsString);
await datastore.save({key: postKey, data: {points}});
const [entity] = await datastore.get(postKey, {wrapNumbers: true});
assert.strictEqual(entity.points.value, largeIntValueAsString);
assert.throws(() => entity.points.valueOf());
await datastore.delete(postKey);
});

it('should save/get/delete with a key name', async () => {
const postKey = datastore.key(['Post', 'post1']);
await datastore.save({key: postKey, data: post});
Expand Down
17 changes: 17 additions & 0 deletions test/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,23 @@ describe('entity', () => {
assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto);
});

it('should emit warning on out of bounce int', done => {
const largeIntValue = 9223372036854775807;
const property = 'largeInt';
const expectedWarning =
'IntegerOutOfBoundsWarning: ' +
"the value for '" +
property +
"' property is outside of bounds of a JavaScript Number.\n" +
"Use 'Datastore.int(<integer_value_as_string>)' to preserve accuracy during the upload.";

process.on('warning', warning => {
assert.strictEqual(warning.message, expectedWarning);
done();
});
entity.encodeValue(largeIntValue, property);
});

it('should encode an Int object', () => {
const value = new entity.Int(3);

Expand Down