Adding Timestamp class #220

schmidt-sebastian · 2018-06-25T20:02:53Z

This ports the new Timestamp class to Node JS.

Also:

Added GeoPoint range validation.
Fixed two docs issues.

A follow-up PR will use the new Timestamp type in the .createTime, .updateTime and .readTime.

codecov · 2018-06-25T20:14:36Z

Codecov Report

Merging #220 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #220   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     13    +1     
  Lines        1799   1834   +35     
=====================================
+ Hits         1799   1834   +35

Impacted Files	Coverage Δ
src/timestamp.js	`100% <100%> (ø)`
src/document.js	`100% <100%> (ø)`	⬆️
src/index.js	`100% <100%> (ø)`	⬆️
src/validate.js	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e7619a...c5e5d18. Read the comment docs.

var-const

Please see the question about truncation. Otherwise LGTM with very small nits.

src/index.js

+   * `Timestamp` instead.
+   * <br/>NOTE: in the future `timestampsInSnapshots: true` will become the
+   * default and this option will be removed so you should change your code to
+   * use Timestamp now and opt-in to this new behavior as soon as you can.


src/timestamp.js

+'use strict';
+
+const is = require('is');
+const validate = require('./validate')();


src/timestamp.js

+   */
+  isEqual(other) {
+    return (
+      this === other ||


src/validate.js

+          return false;
+        }
+        if (value < min || value > max) {
+          throw new Error(`Value must be within [${min}, ${max}] inclusive.`);


test/document.js


    assert.throws(() => {
      new Firestore.GeoPoint();
-    }, expectedErr);
+    }, /Argument "latitude" is not a valid number/);


types/firestore.d.ts

+     *
+     * NOTE: in the future `timestampsInSnapshots: true` will become the
+     * default and this option will be removed so you should change your code to
+     * use Timestamp now and opt-in to this new behavior as soon as you can.


test/timestamp.js

+        .then(res => {
+          const timestamp = res.get('moonLanding');
+          assert.equal(timestamp.seconds, -14182920);
+          assert.equal(timestamp.nanoseconds, 123000123);


schmidt-sebastian

Feedback addressed. Thanks for your review!

src/index.js

+   * `Timestamp` instead.
+   * <br/>NOTE: in the future `timestampsInSnapshots: true` will become the
+   * default and this option will be removed so you should change your code to
+   * use Timestamp now and opt-in to this new behavior as soon as you can.


src/timestamp.js

+'use strict';
+
+const is = require('is');
+const validate = require('./validate')();


src/timestamp.js

+   */
+  isEqual(other) {
+    return (
+      this === other ||


src/validate.js

+          return false;
+        }
+        if (value < min || value > max) {
+          throw new Error(`Value must be within [${min}, ${max}] inclusive.`);


test/document.js


    assert.throws(() => {
      new Firestore.GeoPoint();
-    }, expectedErr);
+    }, /Argument "latitude" is not a valid number/);


test/timestamp.js

+        .then(res => {
+          const timestamp = res.get('moonLanding');
+          assert.equal(timestamp.seconds, -14182920);
+          assert.equal(timestamp.nanoseconds, 123000123);


types/firestore.d.ts

+     *
+     * NOTE: in the future `timestampsInSnapshots: true` will become the
+     * default and this option will be removed so you should change your code to
+     * use Timestamp now and opt-in to this new behavior as soon as you can.


schmidt-sebastian · 2018-06-26T17:30:04Z

Adding @stephenplusplus / @JustinBeckwith for approval. FYI: @var-const is a member of the client SDK team.

…estore into timestamps

types/firestore.d.ts

+    /**
+     * The number of seconds of UTC time since Unix epoch 1970-01-01T00:00:00Z.
+     */
+    readonly seconds: number;


Edalbrelord · 2018-07-20T19:10:20Z

I am using the Admin SDK yes, what release date would you be looking at for this new SDK?

…

Op 20 jul. 2018, om 21:06 heeft Sebastian Schmidt ***@***.***> het volgende geschreven: @schmidt-sebastian commented on this pull request. In types/firestore.d.ts <#220 (comment)>: > + * Creates a new timestamp. + * + * @param seconds The number of seconds of UTC time since Unix epoch + * 1970-01-01T00:00:00Z. Must be from 0001-01-01T00:00:00Z to + * 9999-12-31T23:59:59Z inclusive. + * @param nanoseconds The non-negative fractions of a second at nanosecond + * resolution. Negative second values with fractions must still have + * non-negative nanoseconds values that count forward in time. Must be from + * 0 to 999,999,999 inclusive. + */ + constructor(seconds: number, nanoseconds: number); + + /** + * The number of seconds of UTC time since Unix epoch 1970-01-01T00:00:00Z. + */ + readonly seconds: number; Hi! Are you using the Admin SDK? If you are using the Admin SDK, you have to use the Admin-exported type of Timestamp (admin.firestore.Timestamp). That, however, is not possible until the next release of the Admin SDK, which will include the definition for this type (as per firebase/firebase-admin-node#309 <firebase/firebase-admin-node#309>). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#220 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALznB50LR7zYW5eIk8WEZm1Lh1mPLMsbks5uIio6gaJpZM4U2xIK>.

hiranya911 · 2018-07-20T19:13:23Z

@Edalbrelord we can get a patch release out sometime next week.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 25, 2018

Adding Timestamp class

f9a03ef

schmidt-sebastian force-pushed the timestamps branch from ad115af to f9a03ef Compare June 25, 2018 20:14

schmidt-sebastian added 2 commits June 25, 2018 13:15

Merge branch 'master' into timestamps

006bde7

Adding no-console

fa5c112

var-const reviewed Jun 25, 2018

View reviewed changes

schmidt-sebastian force-pushed the timestamps branch from e620cfd to 2326edb Compare June 26, 2018 00:25

schmidt-sebastian commented Jun 26, 2018

View reviewed changes

Addressing feedback

e3d913e

schmidt-sebastian force-pushed the timestamps branch from 2326edb to e3d913e Compare June 26, 2018 00:26

schmidt-sebastian added 3 commits June 25, 2018 17:27

Merge branch 'master' into timestamps

3c583c4

Merge branch 'master' into timestamps

f991e97

Adding examples

1e19341

var-const approved these changes Jun 26, 2018

View reviewed changes

schmidt-sebastian assigned JustinBeckwith and stephenplusplus Jun 26, 2018

schmidt-sebastian added 2 commits June 26, 2018 10:42

Updating SystemTests

b7e722c

Merge branch 'timestamps' of https://github.com/googleapis/nodejs-fir…

c5e5d18

…estore into timestamps

stephenplusplus approved these changes Jun 26, 2018

View reviewed changes

schmidt-sebastian merged commit 0ce0ed8 into master Jun 26, 2018

Edalbrelord reviewed Jul 20, 2018

View reviewed changes

types/firestore.d.ts

/**

* The number of seconds of UTC time since Unix epoch 1970-01-01T00:00:00Z.

*/

readonly seconds: number;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

schmidt-sebastian deleted the timestamps branch July 24, 2018 04:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Timestamp class #220

Adding Timestamp class #220

schmidt-sebastian commented Jun 25, 2018 •

edited

Loading

codecov bot commented Jun 25, 2018 •

edited

Loading

var-const left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian commented Jun 26, 2018 •

edited

Loading

This comment was marked as spam.

This comment was marked as spam.

Edalbrelord commented Jul 20, 2018 via email

hiranya911 commented Jul 20, 2018

Adding Timestamp class #220

Adding Timestamp class #220

Conversation

schmidt-sebastian commented Jun 25, 2018 • edited Loading

codecov bot commented Jun 25, 2018 • edited Loading

Codecov Report

var-const left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian commented Jun 26, 2018 • edited Loading

This comment was marked as spam.

This comment was marked as spam.

Edalbrelord commented Jul 20, 2018 via email

hiranya911 commented Jul 20, 2018

schmidt-sebastian commented Jun 25, 2018 •

edited

Loading

codecov bot commented Jun 25, 2018 •

edited

Loading

schmidt-sebastian commented Jun 26, 2018 •

edited

Loading