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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 9 commits into from
Feb 27, 2020
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ed337b9
feat: add warning on encodeValue about too large int value
AVaksman Feb 18, 2020
27ec908
Merge branch 'master' into upload_large_int
bcoe Feb 20, 2020
f88f356
chore: add comment on integer vs float verification logic
AVaksman Feb 20, 2020
9c5be6e
Merge branch 'master' into upload_large_int
AVaksman Feb 20, 2020
c9fd85a
refactor: Number.isInteger for int vs float verification logic
AVaksman Feb 21, 2020
880e260
fix: update link in README.md
AVaksman Feb 21, 2020
339ecf6
refactor: warning prints the property name of the out of bounds integ…
AVaksman Feb 26, 2020
f03c014
Merge branch 'master' into upload_large_int
AVaksman Feb 27, 2020
bd66ac4
Merge branch 'master' into upload_large_int
AVaksman Feb 27, 2020
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
10 changes: 10 additions & 0 deletions src/entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,17 @@ export namespace entity {
}

if (typeof value === 'number') {
// verify that 'value' is an integer and not a float i.e. no decimals.
if (value % 1 === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of value % 1, this seems like it could use a comment (even though it wasn't added by you).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Would it be preferable to do

if (Number.isInteger(value)) {
...
}

instead? Here the method's name speaks for itself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method name is nice 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped the logic toNumber.isInteger(value)

if (!Number.isSafeInteger(value)) {
process.emitWarning(
'Integer value ' +
value +
' is outside of bounds of a JavaScript Number\n' +
stephenplusplus marked this conversation as resolved.
Show resolved Hide resolved
"Use 'Datastore.int(<integer_value_as_string>)' to preserve accuracy during the upload.",
'IntegerOutOfBoundsWarning'
);
}
value = new entity.Int(value);
} else {
value = new entity.Double(value);
Expand Down
11 changes: 11 additions & 0 deletions system-test/datastore.ts
View file
Open in desktop
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
16 changes: 16 additions & 0 deletions test/entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,22 @@ describe('entity', () => {
assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto);
});

it('should emit warning on out of bounce int', done => {
const largeIntValue = 9223372036854775807;
const expectedWarning =
'Integer value ' +
largeIntValue +
' 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.name, 'IntegerOutOfBoundsWarning');
assert.strictEqual(warning.message, expectedWarning);
done();
});
entity.encodeValue(largeIntValue);
});

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

Expand Down