diff --git a/samples/concepts.js b/samples/concepts.js index 9e1eb7419..bccde8aa8 100644 --- a/samples/concepts.js +++ b/samples/concepts.js @@ -1020,6 +1020,14 @@ class Transaction extends TestHelper { // Overwrite so the real Datastore instance is used in `transferFunds`. datastore = this.datastore; + const beforeBalance = datastore.int({ + propertyName: 'balance', + integerValue: originalBalance - amountToTransfer, + }); + const afterBalance = datastore.int({ + propertyName: 'balance', + integerValue: originalBalance + amountToTransfer, + }); try { await this.restoreBankAccountBalances({ keys: [fromKey, toKey], @@ -1033,14 +1041,8 @@ class Transaction extends TestHelper { const accounts = results.map(result => result[0]); // Restore `datastore` to the mock API. datastore = datastoreMock; - assert.strictEqual( - accounts[0].balance, - originalBalance - amountToTransfer - ); - assert.strictEqual( - accounts[1].balance, - originalBalance + amountToTransfer - ); + assert.deepStrictEqual(accounts[0].balance, beforeBalance); + assert.deepStrictEqual(accounts[1].balance, afterBalance); } catch (err) { datastore = datastoreMock; throw err; diff --git a/samples/test/concepts.test.js b/samples/test/concepts.test.js index ef977bd29..93581b2d1 100644 --- a/samples/test/concepts.test.js +++ b/samples/test/concepts.test.js @@ -92,6 +92,14 @@ describe('concepts', () => { it('performs a query with multi sort', () => query.testMultiSort()); it('performs a kindless query', () => query.testKindlessQuery()); it('performs a projection query', () => { + const priorities = transaction.datastore.int({ + integerValue: 4, + propertyName: 'priority', + }); + const percentCompletes = transaction.datastore.int({ + integerValue: 10, + propertyName: 'percent_complete', + }); return entity .testProperties() .then(() => { @@ -103,8 +111,8 @@ describe('concepts', () => { }) .then(results => { assert.deepStrictEqual(results, { - priorities: [4], - percentCompletes: [10], + priorities: [priorities], + percentCompletes: [percentCompletes], }); }); }); diff --git a/src/entity.ts b/src/entity.ts index f729e8ff7..21b953709 100644 --- a/src/entity.ts +++ b/src/entity.ts @@ -52,7 +52,7 @@ export namespace entity { * provided. * * @class - * @param {number} value The double value. + * @param {number|string} value The double value. * * @example * ``` @@ -61,20 +61,38 @@ export namespace entity { * const aDouble = datastore.double(7.3); * ``` */ - export class Double { + export class Double extends Number { + private _entityPropertyName: string | undefined; type: string; value: number; - constructor(value: number) { + constructor(value: number | string | ValueProto) { + const inferredValue = + typeof value === 'object' ? value.doubleValue : value; + super(inferredValue); + /** * @name Double#type * @type {string} */ this.type = 'DatastoreDouble'; + + this._entityPropertyName = + typeof value === 'object' ? value.propertyName : undefined; + /** * @name Double#value * @type {number} */ - this.value = value; + this.value = Number(inferredValue); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + valueOf(): any { + return Number(this.value); + } + + toJSON(): any { + return {type: this.type, value: this.value}; } } @@ -495,7 +513,7 @@ export namespace entity { * * @private * @param {object} valueProto The protobuf Value message to convert. - * @param {boolean | IntegerTypeCastOptions} [wrapNumbers=false] Wrap values of integerValue type in + * @param {boolean | IntegerTypeCastOptions} [wrapNumbers=true] Wrap values of integerValue type in * {@link Datastore#Int} objects. * If a `boolean`, this will wrap values in {@link Datastore#Int} objects. * If an `object`, this will return a value returned by @@ -545,10 +563,21 @@ export namespace entity { } case 'doubleValue': { + if (wrapNumbers === undefined) { + wrapNumbers = true; + } + + if (wrapNumbers) { + return new entity.Double(valueProto); + } return Number(value); } case 'integerValue': { + if (wrapNumbers === undefined) { + wrapNumbers = true; + } + return wrapNumbers ? typeof wrapNumbers === 'object' ? new entity.Int(valueProto, wrapNumbers).valueOf() @@ -604,23 +633,6 @@ export namespace entity { return valueProto; } - if (typeof value === 'number') { - 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()' to preserve accuracy during the upload." - ); - } - value = new entity.Int(value); - } else { - value = new entity.Double(value); - } - } - if (isDsInt(value)) { valueProto.integerValue = value.value; return valueProto; @@ -631,6 +643,40 @@ export namespace entity { return valueProto; } + if (typeof value === 'number') { + const integerOutOfBoundsWarning = + "IntegerOutOfBoundsWarning: the value for '" + + property + + "' property is outside of bounds of a JavaScript Number.\n" + + "Use 'Datastore.int()' or " + + "'Datastore.double()' to preserve consistent " + + 'Datastore types in your database.'; + + const typeCastWarning = + "TypeCastWarning: the value for '" + + property + + "' property is a JavaScript Number.\n" + + "Use 'Datastore.int()' or " + + "'Datastore.double()' to preserve consistent " + + 'Datastore types in your database.'; + + if (Number.isInteger(value)) { + if (!Number.isSafeInteger(value)) { + process.emitWarning(integerOutOfBoundsWarning); + } else { + warn('TypeCastWarning', typeCastWarning); + } + value = new entity.Int(value); + valueProto.integerValue = value.value; + return valueProto; + } else { + warn('TypeCastWarning', typeCastWarning); + value = new entity.Double(value); + valueProto.doubleValue = value.value; + return valueProto; + } + } + if (isDsGeoPoint(value)) { valueProto.geoPointValue = value.value; return valueProto; @@ -691,6 +737,14 @@ export namespace entity { throw new Error('Unsupported field value, ' + value + ', was provided.'); } + const warningTypesIssued = new Set(); + const warn = (warningName: string, warningMessage: string) => { + if (!warningTypesIssued.has(warningName)) { + warningTypesIssued.add(warningName); + process.emitWarning(warningMessage); + } + }; + /** * Convert any entity protocol to a plain object. * diff --git a/src/index.ts b/src/index.ts index 272559872..04d033c0c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1369,11 +1369,11 @@ class Datastore extends DatastoreRequest { * ]); * ``` */ - static int(value: number | string) { + static int(value: number | string | ValueProto) { return new entity.Int(value); } - int(value: number | string) { + int(value: number | string | ValueProto) { return Datastore.int(value); } diff --git a/src/query.ts b/src/query.ts index a605165e8..ba3566c4e 100644 --- a/src/query.ts +++ b/src/query.ts @@ -417,7 +417,7 @@ class Query { * [here](https://cloud.google.com/datastore/docs/articles/balancing-strong-and-eventual-consistency-with-google-cloud-datastore). * @param {object} [options.gaxOptions] Request configuration options, outlined * here: https://googleapis.github.io/gax-nodejs/global.html#CallOptions. - * @param {boolean | IntegerTypeCastOptions} [options.wrapNumbers=false] + * @param {boolean | IntegerTypeCastOptions} [options.wrapNumbers=True] * Wrap values of integerValue type in {@link Datastore#Int} objects. * If a `boolean`, this will wrap values in {@link Datastore#Int} objects. * If an `object`, this will return a value returned by diff --git a/src/request.ts b/src/request.ts index 733d1bc49..6da6d30e6 100644 --- a/src/request.ts +++ b/src/request.ts @@ -443,7 +443,7 @@ class DatastoreRequest { * [here](https://cloud.google.com/datastore/docs/articles/balancing-strong-and-eventual-consistency-with-google-cloud-datastore). * @param {object} [options.gaxOptions] Request configuration options, outlined * here: https://googleapis.github.io/gax-nodejs/global.html#CallOptions. - * @param {boolean | IntegerTypeCastOptions} [options.wrapNumbers=false] + * @param {boolean | IntegerTypeCastOptions} [options.wrapNumbers=true] * Wrap values of integerValue type in {@link Datastore#Int} objects. * If a `boolean`, this will wrap values in {@link Datastore#Int} objects. * If an `object`, this will return a value returned by diff --git a/system-test/datastore.ts b/system-test/datastore.ts index 875a4133e..97b7591ff 100644 --- a/system-test/datastore.ts +++ b/system-test/datastore.ts @@ -20,6 +20,7 @@ import * as yaml from 'js-yaml'; import {Datastore, Index} from '../src'; import {google} from '../protos/protos'; import {Storage} from '@google-cloud/storage'; +import {entity} from '../src/entity'; describe('Datastore', () => { const testKinds: string[] = []; @@ -72,11 +73,11 @@ describe('Datastore', () => { publishedAt: new Date(), author: 'Silvano', isDraft: false, - wordCount: 400, - rating: 5.0, + wordCount: new entity.Int({propertyName: 'wordCount', integerValue: 400}), + rating: new entity.Double({propertyName: 'rating', doubleValue: 5.0}), likes: null, metadata: { - views: 100, + views: new entity.Int({propertyName: 'views', integerValue: 100}), }, }; @@ -273,6 +274,40 @@ describe('Datastore', () => { await datastore.delete(postKey); }); + it('should save/get an int-able double value via Datastore.', async () => { + const postKey = datastore.key('Team'); + const points = Datastore.double(2); + await datastore.save({key: postKey, data: {points}}); + let [entity] = await datastore.get(postKey, {wrapNumbers: true}); + // Expect content is stored in datastore as a double with wrapping to + // return a wrapped double + assert.strictEqual(entity.points.type, 'DatastoreDouble'); + assert.strictEqual(entity.points.value, 2); + + [entity] = await datastore.get(postKey, {wrapNumbers: false}); + // Verify that when requested with wrapNumbers false, we get a plain + // javascript Number 2. + assert.strictEqual(entity.points, 2); + + [entity] = await datastore.get(postKey); + // Expect without any options, a wrapped double to be returned. + assert.strictEqual(entity.points.type, 'DatastoreDouble'); + assert.strictEqual(entity.points.value, 2); + + // Save the data again, get again, ensuring that along the way it isn't + // somehow changed to another numeric type. + await datastore.save(entity); + [entity] = await datastore.get(postKey); + // expect as we saved, that this property is still a DatastoreDouble. + assert.strictEqual(entity.points.type, 'DatastoreDouble'); + assert.strictEqual(entity.points.value, 2); + + // Verify that DatastoreDouble implement Number behavior + assert.strictEqual(entity.points + 1, 3); + + await datastore.delete(postKey); + }); + it('should wrap specified properties via IntegerTypeCastOptions.properties', async () => { const postKey = datastore.key('Scores'); const largeIntValueAsString = '9223372036854775807'; @@ -518,7 +553,7 @@ describe('Datastore', () => { }, }); const [entity] = await datastore.get(key); - assert.strictEqual(entity.year, integerValue); + assert.strictEqual(entity.year.valueOf(), integerValue); }); it('should save and decode a double', async () => { @@ -532,7 +567,7 @@ describe('Datastore', () => { }, }); const [entity] = await datastore.get(key); - assert.strictEqual(entity.nines, doubleValue); + assert.strictEqual(entity.nines.valueOf(), doubleValue); }); it('should save and decode a geo point', async () => { @@ -890,7 +925,7 @@ describe('Datastore', () => { datastore.get(key), ]); assert.strictEqual(typeof deletedEntity, 'undefined'); - assert.strictEqual(fetchedEntity.rating, 10); + assert.strictEqual(fetchedEntity.rating.valueOf(), 10); }); it('should use the last modification to a key', async () => { diff --git a/test/entity.ts b/test/entity.ts index 84ca5b3dd..627f95905 100644 --- a/test/entity.ts +++ b/test/entity.ts @@ -17,7 +17,7 @@ import {beforeEach, afterEach, describe, it} from 'mocha'; import * as extend from 'extend'; import * as sinon from 'sinon'; import {Datastore} from '../src'; -import {Entity} from '../src/entity'; +import {entity, Entity} from '../src/entity'; import {IntegerTypeCastOptions} from '../src/query'; export function outOfBoundsError(opts: { @@ -54,11 +54,44 @@ describe('entity', () => { }); describe('Double', () => { + function checkDouble(double: entity.Double, comparisonValue: number) { + assert.strictEqual(double.value, comparisonValue); + assert.strictEqual(double.valueOf(), comparisonValue); + assert.deepStrictEqual(double.toJSON(), { + type: 'DatastoreDouble', + value: comparisonValue, + }); + } it('should store the value', () => { const value = 8.3; - const double = new entity.Double(value); - assert.strictEqual(double.value, value); + checkDouble(double, value); + }); + it('should store the rounded value', () => { + const value = 8.0; + const double = new entity.Double(value); + checkDouble(double, value); + }); + it('should store the stringified value', () => { + const value = '8.3'; + const double = new entity.Double(value); + checkDouble(double, 8.3); + }); + it('should store from a value proto', () => { + const valueProto = { + valueType: 'doubleValue', + doubleValue: 8, + }; + const double = new entity.Double(valueProto); + checkDouble(double, 8); + }); + it('should store from a value proto with a stringified value', () => { + const valueProto = { + valueType: 'doubleValue', + doubleValue: '8', + }; + const double = new entity.Double(valueProto); + checkDouble(double, 8); }); }); @@ -428,7 +461,7 @@ describe('entity', () => { ); }); - it('should not wrap numbers by default', () => { + it('should wrap numbers by default', () => { const decodeValueProto = entity.decodeValueProto; entity.decodeValueProto = ( valueProto: {}, @@ -439,7 +472,25 @@ describe('entity', () => { return decodeValueProto(valueProto, wrapNumbers); }; - assert.deepStrictEqual(entity.decodeValueProto(valueProto), [intValue]); + assert.deepStrictEqual(entity.decodeValueProto(valueProto), [ + new entity.Int(intValue), + ]); + }); + + it('should not wrap numbers with option', () => { + const decodeValueProto = entity.decodeValueProto; + entity.decodeValueProto = ( + valueProto: {}, + wrapNumbers?: boolean | {} + ) => { + assert.strictEqual(wrapNumbers, false); + + return decodeValueProto(valueProto, wrapNumbers); + }; + + assert.deepStrictEqual(entity.decodeValueProto(valueProto, false), [ + intValue, + ]); }); it('should wrap numbers with an option', () => { @@ -551,14 +602,18 @@ describe('entity', () => { }; describe('default `wrapNumbers: undefined`', () => { - it('should not wrap ints by default', () => { - assert.strictEqual( - typeof entity.decodeValueProto(valueProto), - 'number' - ); + it('should wrap ints by default', () => { + const decoded = entity.decodeValueProto(valueProto); + assert.strictEqual(typeof decoded, 'object'); + assert.strictEqual(typeof decoded.value, 'string'); + assert.strictEqual(typeof decoded.valueOf(), 'number'); + }); + it('should not wrap ints', () => { + const decoded = entity.decodeValueProto(valueProto, false); + assert.strictEqual(decoded, 8); }); - it('should throw if integer value is outside of bounds', () => { + it('should throw if integer value is outside of bounds and unwrapping', () => { const largeIntegerValue = Number.MAX_SAFE_INTEGER + 1; const smallIntegerValue = Number.MIN_SAFE_INTEGER - 1; @@ -575,11 +630,11 @@ describe('entity', () => { }; assert.throws(() => { - entity.decodeValueProto(valueProto); + entity.decodeValueProto(valueProto, false); }, outOfBoundsError(valueProto)); assert.throws(() => { - entity.decodeValueProto(valueProto2); + entity.decodeValueProto(valueProto2, false); }, outOfBoundsError(valueProto2)); }); }); @@ -661,16 +716,28 @@ describe('entity', () => { const decodedValue = entity.decodeValueProto(valueProto); assert.deepStrictEqual(decodedValue, expectedValue); }); - - it('should decode doubles', () => { - const expectedValue = 8.3; + describe('doubleValues', () => { + const expectedDecodedValue = 8.3; const valueProto = { valueType: 'doubleValue', - doubleValue: expectedValue, + doubleValue: expectedDecodedValue, }; - assert.strictEqual(entity.decodeValueProto(valueProto), expectedValue); + it('should wrap doubles by default', () => { + const decoded = entity.decodeValueProto(valueProto); + const decodedWithWrapping = entity.decodeValueProto(valueProto, true); + assert.deepStrictEqual(decoded, decodedWithWrapping); + assert.strictEqual(typeof decoded, 'object'); + assert.strictEqual(decoded.valueOf(), expectedDecodedValue); + assert.strictEqual(decoded.value, expectedDecodedValue); + }); + it('should not wrap doubles', () => { + assert.strictEqual( + entity.decodeValueProto(valueProto, false), + expectedDecodedValue + ); + }); }); it('should decode keys', () => { @@ -723,6 +790,51 @@ describe('entity', () => { }); }); + describe('encode for request', () => { + it('request should receive doubleValue', done => { + const datastore = new Datastore({}); + const key = new entity.Key({ + namespace: 'namespace', + path: ['Company', 123], + }); + const points = Datastore.double(2); + const entities = [ + { + key, + data: {points}, + }, + ]; + + datastore.request_ = (data: any) => { + // By the time the request is made, the original object has already been + // transformed into a raw request. + assert.deepStrictEqual(data, { + client: 'DatastoreClient', + method: 'commit', + reqOpts: { + mutations: [ + { + upsert: { + key: { + path: [{kind: 'Company', id: 123}], + partitionId: {namespaceId: 'namespace'}, + }, + properties: { + points: {doubleValue: 2}, + }, + }, + }, + ], + }, + gaxOpts: {}, + }); + done(); + }; + + datastore.save(entities, assert.ifError); + }); + }); + describe('encodeValue', () => { it('should encode a boolean', () => { const value = true; @@ -744,13 +856,26 @@ describe('entity', () => { assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto); }); - it('should encode an int', () => { + it('should encode an int', done => { const value = 8; const expectedValueProto = { integerValue: value, }; + const property = 'undefined'; + const expectedWarning = + "TypeCastWarning: the value for '" + + property + + "' property is a JavaScript Number.\n" + + "Use 'Datastore.int()' or " + + "'Datastore.double()' to preserve consistent " + + 'Datastore types in your database.'; + process.once('warning', warning => { + assert.strictEqual(warning.message, expectedWarning); + done(); + }); + entity.Int = function (value_: {}) { assert.strictEqual(value_, value); this.value = value_; @@ -759,17 +884,49 @@ describe('entity', () => { assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto); }); - it('should emit warning on out of bounce int', done => { + it('should encode an int but only warn once', done => { + const value = 8; + + const expectedValueProto = { + integerValue: value, + }; + + const property = 'undefined'; + const expectedWarning = + "TypeCastWarning: the value for '" + + property + + "' property is a JavaScript Number.\n" + + "Use 'Datastore.int()' or " + + "'Datastore.double()' to preserve consistent " + + 'Datastore types in your database.'; + process.once('warning', warning => { + assert.strictEqual(warning.message, expectedWarning); + done(); + }); + + entity.Int = function (value_: {}) { + assert.strictEqual(value_, value); + this.value = value_; + }; + + assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto); + // if an error is reported on this, done() is called more than once and + // should cause failure. + assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto); + }); + + it('should emit warning on out of bounds int', done => { const largeIntValue = 9223372036854775807; const property = 'largeInt'; const expectedWarning = - 'IntegerOutOfBoundsWarning: ' + - "the value for '" + + "IntegerOutOfBoundsWarning: the value for '" + property + "' property is outside of bounds of a JavaScript Number.\n" + - "Use 'Datastore.int()' to preserve accuracy during the upload."; + "Use 'Datastore.int()' or " + + "'Datastore.double()' to preserve consistent " + + 'Datastore types in your database.'; - process.on('warning', warning => { + process.once('warning', warning => { assert.strictEqual(warning.message, expectedWarning); done(); }); @@ -786,13 +943,27 @@ describe('entity', () => { assert.deepStrictEqual(entity.encodeValue(value), expectedValueProto); }); - it('should encode a double', () => { + it('should encode a double', done => { const value = 8.3; const expectedValueProto = { doubleValue: value, }; + const property = 'undefined'; + const expectedWarning = + "TypeCastWarning: the value for '" + + property + + "' property is a JavaScript Number.\n" + + "Use 'Datastore.int()' or " + + "'Datastore.double()' to preserve consistent " + + 'Datastore types in your database.'; + + process.once('warning', warning => { + assert.strictEqual(warning.message, expectedWarning); + done(); + }); + entity.Double = function (value_: {}) { assert.strictEqual(value_, value); this.value = value_; diff --git a/test/index.ts b/test/index.ts index 06cbe9849..91ba9bcdb 100644 --- a/test/index.ts +++ b/test/index.ts @@ -1316,7 +1316,13 @@ describe('Datastore', () => { arrayValue: { values: [ { - integerValue: '0', + entityValue: { + properties: { + value: { + integerValue: '0', + }, + }, + }, }, { nullValue: 0, @@ -1342,7 +1348,7 @@ describe('Datastore', () => { data: { stringField: 'string value', nullField: null, - arrayField: [0, null], + arrayField: [datastore.int(0), null], objectField: null, }, excludeLargeProperties: true, @@ -1445,7 +1451,7 @@ describe('Datastore', () => { data: { value: { a: 'b', - c: [1, 2, 3], + c: [datastore.int(1), datastore.int(2), datastore.int(3)], }, }, }, diff --git a/test/request.ts b/test/request.ts index 158929001..edf3efede 100644 --- a/test/request.ts +++ b/test/request.ts @@ -370,6 +370,17 @@ describe('Request', () => { found: [ { entity: { + key: { + partitionId: { + projectId: 'grape-spaceship-123', + }, + path: [ + { + kind: 'Post', + name: 'post1', + }, + ], + }, properties: { [propertyName]: { integerValue: largeInt, @@ -382,7 +393,7 @@ describe('Request', () => { }); }; - const stream = request.createReadStream(key); + const stream = request.createReadStream(key, {wrapNumbers: false}); stream .on('data', () => {}) @@ -927,6 +938,17 @@ describe('Request', () => { entityResults: [ { entity: { + key: { + partitionId: { + projectId: 'grape-spaceship-123', + }, + path: [ + { + kind: 'Post', + name: 'post1', + }, + ], + }, properties: { [propertyName]: { integerValue: largeInt, @@ -940,7 +962,7 @@ describe('Request', () => { }); }; - const stream = request.runQueryStream({}); + const stream = request.runQueryStream({}, {wrapNumbers: false}); stream .on('error', (err: Error) => {