Firebase Database API #92

hiranya911 · 2018-02-15T01:56:02Z

This PR introduces the database package for interacting with the Firebase Database. This is a programming layer on top of the REST interface of Database. It closely mirrors the database API found in the Firebase Python Admin SDK, and does not include real time event listeners.

go/firebase-go-admin-db

bklimt

This is really good code. I just have a bunch of nits.

bklimt · 2018-02-15T23:27:15Z

firebase.go

@@ -55,16 +48,20 @@ const firebaseEnvName = "FIREBASE_CONFIG"

 // An App holds configuration and state common to all Firebase services that are exposed from the SDK.
 type App struct {
+	ao            map[string]interface{}


Can we rename this something more descriptive, like maybe authOverride?

bklimt · 2018-02-15T23:32:38Z

db/query.go

+	typeNull      = 0
+	typeBoolFalse = 1
+	typeBoolTrue  = 2
+	typeNumeric   = 3


Does RTDB have any special cases like Infinity or NaN? I remember those required some tricky sorting logic in Firestore.

I don't think so. Since all values are essentially constrained by the json type system, this doesn't affect RTDB as far as i can tell.

bklimt · 2018-02-15T23:42:56Z

db/db.go

+)
+
+const userAgentFormat = "Firebase/HTTP/%s/%s/AdminGo"
+const invalidChars = "[].#$"


For such a small set, this is the right way to do this. But I want to bring unicode.RangeTable to your attention, as it would be the right way to do this if the set were much more complicated.

https://golang.org/pkg/unicode/#Is

bklimt · 2018-02-15T23:47:02Z

db/db.go

+type Client struct {
+	hc  *internal.HTTPClient
+	url string
+	ao  string


I think a longer name would be more appropriate here. With hc, it's at least obvious that it's an HTTPClient. But ao is just a string, and has no comment, so it's pretty unclear.

bklimt · 2018-02-15T23:47:59Z

db/db.go

+	} else if p.Scheme != "https" {
+		return nil, fmt.Errorf("invalid database URL (incorrect scheme): %q", c.URL)
+	} else if !strings.HasSuffix(p.Host, ".firebaseio.com") {
+		return nil, fmt.Errorf("invalid database URL (incorrest host): %q", c.URL)


"incorrect"

bklimt · 2018-02-15T23:51:32Z

db/query.go

+type Query struct {
+	client              *Client
+	path                string
+	ob                  orderBy


Can we maybe call this "order"?

bklimt · 2018-02-16T00:13:00Z

db/query_test.go

+	want []interface{}
+}{
+	{
+		resp: map[string]interface{}{"k1": 1, "k2": 2, "k3": 3},


I wish it could do the left-to-right type inference that would let you omit the types from these composite literals, but this seems like the correct way to do it.

bklimt · 2018-02-16T00:14:52Z

db/ref.go

+	"golang.org/x/net/context"
+)
+
+const txnRetries = 25


Maybe we should comment on this. Is 25 what the clients use as well?

Added comment. 25 is the magic number used in other SDKs as well.

bklimt · 2018-02-16T00:24:16Z

db/ref_test.go

+	})
+}
+
+func TestWerlformedHttpError(t *testing.T) {


lol "Werlformed" isn't. 😄

bklimt · 2018-02-16T00:26:26Z

internal/internal.go

+	Opts    []option.ClientOption
+	URL     string
+	Version string
+	AO      map[string]interface{}


Same comments as above.

avishalom · 2018-02-16T00:53:11Z

db/db.go

+	} else if p.Scheme != "https" {
+		return nil, fmt.Errorf("invalid database URL (incorrect scheme): %q", c.URL)
+	} else if !strings.HasSuffix(p.Host, ".firebaseio.com") {
+		return nil, fmt.Errorf("invalid database URL (incorrest host): %q", c.URL)


Since we are requiring a specific suffix, and a specific scheme, why not state them in the error message?

avishalom · 2018-02-16T01:08:16Z

db/query.go

+//
+// The resulting Query will only return child nodes with a value greater than or equal to v.
+func (q *Query) StartAt(v interface{}) *Query {
+	q2 := new(Query)


Here and below, &Query{} would be more consistent with our current SDK.

avishalom · 2018-02-16T16:25:23Z

testdata/dinosaurs.json

+    "stegosaurus": 5,
+    "triceratops": 22
+  }
+}


avishalom

LGTM!
(general question for this and the other SDKs, do we want to clean up after the integration tests. not for this PR... but maybe later)

…100)

* Adding database snippets * Adding query snippets * Added complex query samples * Updated variable name * Fixing a typo * Fixing query example * Updated DB snippets to use GetOrdered() * Removing unnecessary placeholders in Fatalln() calls * Removing unnecessary placeholders in Fatalln() calls

* Lint (#96) * fix misspelling * add check error * missing copyright * Doc (#97) * update readme with Authentication Guide & Release Notes * fix a misspelling : separately * fix missing newline before package * add Go Report Card + update doc * add travis build for go versions 1.7.x -> 1.10.x (#98) * add build for go version 1.6.x -> 1.10.x * fix 1.10 version * fix context to golang.org/x/net/context for go 1.6 compatibility * add race detector + go vet on build + build without failure on go unstable * add go16 et go17 file due to req.withcontext which is only go 1.7 * fix context package * update go16.go to remove WithContext * update bad import * remove unused func * finally use ctxhttp.Do with multiple build version * ignore integration package for install * fix go get command * put go 1.6.X in allow_failures dur to test failure * fix inversion of code * remove go 1.6 support * revert initial version with req.WithContext * fix travis to support go 1.10.x * nits * Import context from standard package (#101) * Import context from standard package. * Firebase Database API (#92) * Experimental RTDB code * Added ref.Set() * Added Push(), Update(), Remove() and tests * Adding Transaction() support * Fixed Transaction() API * Code cleanup * Implemented Query() API * Added GetIfChanged() and integration tests * More integration tests * Updated unit test * More integration tests * Integration tests for queries * Auth override support and more tests * More test cases; AuthOverride support in App * Implemented AuthOverride support; Added tests * Implementing the new API * More code cleanup * Code clean up * Refactored the http client code * More tests * Boosted test coverage to 97% * Better error messages in tests; Added license headers * Added documentatioon and cleaned up tests * Fixing a build break * Finishing up documentation * More test cases * Implemented a reusable HTTP client API * Added test cases * Comment clean up * Using the shared http client API * Simplified the usage by adding HTTPClient * using the new client API * Using the old ctx import * Using the old context import * Refactored db code * More refactoring * Support for arbitrary entity types in the request * Renamed fields; Added documentation * Removing a redundant else case * Code readability improvements * Cleaned up the RTDB HTTP client code * Added shallow reads support; Added the new txn API * Implementing GetOrdered() for queries * Adding more sorting tests * Added Query ordering tests * Fixing some lint errors and compilation errors * Removing unused function * Cleaned up unit tests for db * Updated query impl and tests * Added integration tests for ordered queries * Removed With*() from query functions * Updated change log; Added more tests * Support for database url in auto init * Support for loading auth overrides from env * Removed db.AuthOverride type * Renamed ao to authOverride everywhere; Other code review nits * Introducing the QueryNode interface to handle ordered query results (#100) * Database Sample Snippets (#102) * Adding database snippets * Adding query snippets * Added complex query samples * Updated variable name * Fixing a typo * Fixing query example * Updated DB snippets to use GetOrdered() * Removing unnecessary placeholders in Fatalln() calls * Removing unnecessary placeholders in Fatalln() calls * Handling FCM canonical error codes (#103) * Formatting test file with gofmt (#104) * Bumped version to 2.6.0 (#105)

* Renamed some tests and test parameters for clarity, and adhere to Go conventions (#74) * clean unused types (#76) * Create CHANGELOG.md (#75) (#79) * Create CHANGELOG.md Initial changelog based on https://firebase.google.com/support/release-notes/admin/go * change instance ID format (#82) Changing the format of the "non-existing" instance ID in the integration tests to comply with the expected iid format. * Import context from golang.org/x/net/ for 1.6 compatibility (#87) * import golang.org/x/net/context instead of context for 1.6 compatibility * Document non existing name in integration tests for iid (#85) * Revoke Tokens (#77) Adding TokensValidAfterMillis property, RevokeRefreshTokens(), and VerifyIDTokenAndCheckRevoked(). * Firebase Cloud Messaging API (#81) * Adding Firebase Cloud Messaging (#62) * initial commit for adding Firebase Cloud Messaging * add validator * use http const in messaging test * add client version header for stats * init integration test * add integration test (validated on IOS today) * add comment with URL to enable Firebase Cloud Messaging API * fix broken test * add integration tests * accept a Message instead of RequestMessage + and rename method + send / sendDryRun * update fcm url * rollback url endpoint * fix http constants, change responseMessage visibility, change map[string]interface{} as map[string]string * fix http constants * fix integration tests * fix APNS naming * add validators * Added APNS types; Updated tests * Added more tests; Fixed APNS serialization * Updated documentation * Improved error handling inFCM * Added utils file * Updated integration tests * Implemented topic management operations * Added integration tests * Updated CHANGELOG * Addressing code review comments * Supporting 0 valued Aps.Badge * Addressing some review comments * Removed some unused vars * Accepting prefixed topic names (#84) * Accepting prefixed topic named * Added a comment * Using new FCM error codes (#89) * Bumped version to 2.5.0 (#90) * Lint (#96) * fix misspelling * add check error * missing copyright * Doc (#97) * update readme with Authentication Guide & Release Notes * fix a misspelling : separately * fix missing newline before package * add Go Report Card + update doc * add travis build for go versions 1.7.x -> 1.10.x (#98) * add build for go version 1.6.x -> 1.10.x * fix 1.10 version * fix context to golang.org/x/net/context for go 1.6 compatibility * add race detector + go vet on build + build without failure on go unstable * add go16 et go17 file due to req.withcontext which is only go 1.7 * fix context package * update go16.go to remove WithContext * update bad import * remove unused func * finally use ctxhttp.Do with multiple build version * ignore integration package for install * fix go get command * put go 1.6.X in allow_failures dur to test failure * fix inversion of code * remove go 1.6 support * revert initial version with req.WithContext * fix travis to support go 1.10.x * nits * Import context from standard package (#101) * Import context from standard package. * Firebase Database API (#92) * Experimental RTDB code * Added ref.Set() * Added Push(), Update(), Remove() and tests * Adding Transaction() support * Fixed Transaction() API * Code cleanup * Implemented Query() API * Added GetIfChanged() and integration tests * More integration tests * Updated unit test * More integration tests * Integration tests for queries * Auth override support and more tests * More test cases; AuthOverride support in App * Implemented AuthOverride support; Added tests * Implementing the new API * More code cleanup * Code clean up * Refactored the http client code * More tests * Boosted test coverage to 97% * Better error messages in tests; Added license headers * Added documentatioon and cleaned up tests * Fixing a build break * Finishing up documentation * More test cases * Implemented a reusable HTTP client API * Added test cases * Comment clean up * Using the shared http client API * Simplified the usage by adding HTTPClient * using the new client API * Using the old ctx import * Using the old context import * Refactored db code * More refactoring * Support for arbitrary entity types in the request * Renamed fields; Added documentation * Removing a redundant else case * Code readability improvements * Cleaned up the RTDB HTTP client code * Added shallow reads support; Added the new txn API * Implementing GetOrdered() for queries * Adding more sorting tests * Added Query ordering tests * Fixing some lint errors and compilation errors * Removing unused function * Cleaned up unit tests for db * Updated query impl and tests * Added integration tests for ordered queries * Removed With*() from query functions * Updated change log; Added more tests * Support for database url in auto init * Support for loading auth overrides from env * Removed db.AuthOverride type * Renamed ao to authOverride everywhere; Other code review nits * Introducing the QueryNode interface to handle ordered query results (#100) * Database Sample Snippets (#102) * Adding database snippets * Adding query snippets * Added complex query samples * Updated variable name * Fixing a typo * Fixing query example * Updated DB snippets to use GetOrdered() * Removing unnecessary placeholders in Fatalln() calls * Removing unnecessary placeholders in Fatalln() calls * Handling FCM canonical error codes (#103) * Formatting test file with gofmt (#104) * Bumped version to 2.6.0 (#105) * Formatting (simplification) changes (#107) * Checking for unformatted files in CI (#108) * Checking for unformatted files in CI * Adding newline at eof * Document Minimum Go Version (#111) * Fix invalid endpoint URL for topic unsubscribe (#114) * Fix error message for missing user (#113) * Update CHANGELOG.md (#117) * Removing unused member from auth.Client (#118) * Support Go 1.6 (#120) * all: use golang.org/x/net/context * internal: use ctxhttp to use /x/ context The 1.6 Request type doesn't have WithContext. * all: don't use subtests to keep 1.6 compatibility * integration: use float64 for fields with exp value Values like -7e+07 cannot be parsed into ints in Go 1.6. So, use floats instead. * integration/messaging: use t.Fatal not log.Fatal * travis: add 1.6.x * changelog: mention addition of 1.6 support * readme: mention go version support * Bumped version to 2.6.1 (#121) * Changlog updates (#123)

hiranya911 added 30 commits October 21, 2017 16:32

Experimental RTDB code

ef0a863

Added ref.Set()

becb82f

Added Push(), Update(), Remove() and tests

2d4e342

Adding Transaction() support

6bf42cf

Fixed Transaction() API

0a21f4e

Code cleanup

0fcbbce

Implemented Query() API

65870f7

Added GetIfChanged() and integration tests

cad2bfe

More integration tests

40c0691

Updated unit test

0e85fa2

More integration tests

20fdbf3

Integration tests for queries

ef547a6

Auth override support and more tests

b4e127d

More test cases; AuthOverride support in App

6af57b0

Implemented AuthOverride support; Added tests

da6fc75

Implementing the new API

641dbaa

More code cleanup

f82a3e0

Code clean up

6ecb8de

Refactored the http client code

deb4eac

More tests

caeaafe

Boosted test coverage to 97%

bf092a2

Better error messages in tests; Added license headers

8450363

Added documentatioon and cleaned up tests

993f639

Fixing a build break

63a5ace

Finishing up documentation

835adaa

More test cases

57263ad

Implemented a reusable HTTP client API

e557b9a

Merge branch 'hkj-http-client' into hkj-rtdb

2c30329

Added test cases

f69b5ad

Comment clean up

d5a5fae

hiranya911 added 7 commits February 14, 2018 14:41

Cleaned up unit tests for db

a65e3e4

Updated query impl and tests

d734308

Added integration tests for ordered queries

0007b6b

Removed With*() from query functions

f0f2063

Merge branch 'dev' into hkj-rtdb

acf4fbd

Updated change log; Added more tests

0e095a0

Support for database url in auto init

0bbe372

hiranya911 requested review from avishalom and bklimt February 15, 2018 02:52

hiranya911 assigned bklimt Feb 15, 2018

hiranya911 added 2 commits February 15, 2018 15:05

Support for loading auth overrides from env

f4cf85a

Removed db.AuthOverride type

14b3591

bklimt approved these changes Feb 16, 2018

View reviewed changes

hiranya911 assigned avishalom and unassigned bklimt Feb 16, 2018

avishalom reviewed Feb 16, 2018

View reviewed changes

avishalom approved these changes Feb 16, 2018

View reviewed changes

hiranya911 assigned hiranya911 and unassigned avishalom Feb 16, 2018

hiranya911 added 5 commits February 16, 2018 11:42

Renamed ao to authOverride everywhere; Other code review nits

c38f24e

Merge branch 'dev' into hkj-rtdb

6365187

Introducing the QueryNode interface to handle ordered query results (#…

db9249e

…100)

Merged with dev

7bc0e2a

hiranya911 merged commit c9be1e9 into dev Feb 28, 2018

hiranya911 deleted the hkj-rtdb branch February 28, 2018 01:13

hiranya911 mentioned this pull request Mar 27, 2018

FR: Support for Realtime Database #35

Closed

Firebase Database API #92

Firebase Database API #92

Uh oh!

Conversation

hiranya911 commented Feb 15, 2018

Uh oh!

bklimt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avishalom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!