Skip to content

Make ServerValue.increment() public #2852

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

Merged
merged 2 commits into from
Apr 7, 2020
Merged

Make ServerValue.increment() public #2852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84d3068
Make ServerValue.increment() public
schmidt-sebastian Apr 2, 2020
83c3dbe
Update to use Object
schmidt-sebastian Apr 2, 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
5 changes: 2 additions & 3 deletions packages/database-types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,8 @@ export interface Reference extends Query {
}

export interface ServerValue {
TIMESTAMP: {
'.sv': string;
};
TIMESTAMP: Object;
increment(delta: number) : Object;
}

export interface ThenableReference extends Reference, Promise<Reference> {}
Expand Down
5 changes: 4 additions & 1 deletion packages/database/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Unreleased
- [feature] Added ServerValue.increment() to support atomic field value increments
without transactions.

# Released
- [fixed] Fixed an issue that caused large numeric values with leading zeros to
not always be sorted correctly.

- [changed] Internal cleanup to Node.JS support.

# 6.4.0
Expand Down
4 changes: 2 additions & 2 deletions packages/database/src/api/Database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ export class Database implements FirebaseService {
TIMESTAMP: {
'.sv': 'timestamp'
},
_increment: (x: number) => {
increment: (delta: number) => {
return {
'.sv': {
'increment': x
'increment': delta
}
};
}
Expand Down
10 changes: 5 additions & 5 deletions packages/database/test/servervalues.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('ServerValue tests', () => {
// node (i.e. ChildrenNode.EMPTY_NODE) because there is not yet any synced
// data.
const node = getRandomNode() as Reference;
await node.set(Database.ServerValue._increment(1));
await node.set(Database.ServerValue.increment(1));
});

describe('handles increments', () => {
Expand All @@ -64,7 +64,7 @@ describe('ServerValue tests', () => {
node.database.goOffline();
}

const addOne = Database.ServerValue._increment(1);
const addOne = Database.ServerValue.increment(1);

const values: any = [];
const expected: any = [];
Expand Down Expand Up @@ -110,7 +110,7 @@ describe('ServerValue tests', () => {
});

await node.update({
'child/increment': Database.ServerValue._increment(1),
'child/increment': Database.ServerValue.increment(1),
'literal': 5
});
expect(value).to.deep.equal({
Expand All @@ -121,7 +121,7 @@ describe('ServerValue tests', () => {
});

await node.update({
'child/increment': Database.ServerValue._increment(41)
'child/increment': Database.ServerValue.increment(41)
});
expect(value).to.deep.equal({
'literal': 5,
Expand All @@ -141,7 +141,7 @@ describe('ServerValue tests', () => {
const racers = 100;

for (let i = 0; i < racers; i++) {
all.push(node.set(Database.ServerValue._increment(1)));
all.push(node.set(Database.ServerValue.increment(1)));
}
await Promise.all(all);

Expand Down
9 changes: 9 additions & 0 deletions packages/firebase/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6920,6 +6920,15 @@ declare namespace firebase.database.ServerValue {
* ```
*/
var TIMESTAMP: Object;

/**
* Returns a placeholder value that can be used to atomically increment the
Copy link
Member

Choose a reason for hiding this comment

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

Consider the documentation from Firestore: https://googleapis.dev/nodejs/firestore/latest/FieldValue.html#.increment

Copy link
Member

@inlined inlined Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

To the degree that we follow the same rules of course. We overflow to float instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the JS SDK itself, we don't do anything special though. Internally, we use floating point arithmetic (with its limited 2^53 precision) and once we round-trip through the server we gain land in JavaScript number land (please let me know if I am wrong here).

Firestore is a little different in that it can retain the actual value for 2^53 + 1 on the backend, whereas JavaScript cannot.

To me, it seems like the RTDB just uses JavaScript number semantics all throughout - is that correct? I can modify the comment to say that we are using floating point arithmetic throughout (similar to https://googleapis.dev/nodejs/firestore/latest/FieldValue.html#.increment), but I want to make sure that my understanding is correct.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it just makes a difference that whole numbers would be interpreted as ints in other SDKs whereas a fractional number will cause other SDKs to see floats. OTOH that isn't different from any other field, so it doesn't really matter.

* current database value by the provided delta.
*
* @param delta the amount to modify the current value atomically.
* @return a placeholder value for modifying data atomically server-side.
*/
function increment(delta: number) : Object;
}

/**
Expand Down