feat(rtdb): Connect to RTDB emulator when valid emulator URL is passed OR env vars are set correctly #299

sarajmunjal · 2019-08-12T20:52:59Z

This PR makes it possible for the Java admin SDK to talk to the RTDB emulator instead of the actual database instance.
This can happen in the following fashion:-

If the user passes in a valid emulator URL (http scheme only, host can't contain firebaseio.com, has the ns query parameter)
Else, if the environment variable FIREBASE_RTDB_EMULATOR_HOST is set to a value of the form host:port.

If either 1. or 2. is true, we decide to talk to the RTDB emulator. In such cases, we

provide a valid emulator URL (either by replacing the supplied URL or making a new one)
replace existing credentials with EmulatorCredentials, which use owner as the access token.

URL replacement happens in three places:-

while constructing FirebaseOptions instances in FirebaseApp
when a specific databaseUrl is passed in FirebaseDatabase.getInstance(String url)
when a specific URL with path is passed in FirebaseDatabase.getReferenceFromUrl(String url)

Tests have been added with different combinations of passed-in URLs and environment variable values for classes FirebaseApp, FirebaseDatabase, and also for EmulatorHelper: the helper class that actually does URL replacement if necessary.

Notes for reviewers
I have made changes to existing behavior to Utilities.parseUrl(String url): these changes were necessary to recognize the ns query parameter, and also to suppost valid <net_loc> URL semantics where a URL like http://localhost:8080/ is deemed valid.
These changes are nearly identical to changes to the same file in this Android SDK PR.

RELEASE NOTE: Developers can now test Realtime Database API calls by directing the SDK traffic to the RTDB emulator. Set the FIREBASE_DATABASE_EMULATOR_HOST environment variable to specify the emulator endpoint in host:port format.

…MULATOR_HOST=true

hiranya911

Please fix the test failures.

src/main/java/com/google/firebase/database/FirebaseDatabase.java

…ogic for inferring them correctly

src/main/java/com/google/firebase/FirebaseApp.java

src/main/java/com/google/firebase/database/connection/NettyWebSocketClient.java

src/test/java/com/google/firebase/database/FirebaseDatabaseTest.java

src/test/java/com/google/firebase/testing/TestUtils.java

schmidt-sebastian

Thanks for this! I think from a functional perspective this is fine, but I wonder if there is a way to make it more obvious when we are talking out the environment variable and when we are talking about a database URL.

Furthermore, all existing SDKs always blindly overwrite any URL if the emulator host environment variable is set. I wonder if we should do the same?

src/main/java/com/google/firebase/database/FirebaseDatabase.java

src/main/java/com/google/firebase/FirebaseApp.java

src/main/java/com/google/firebase/FirebaseOptions.java

src/main/java/com/google/firebase/database/connection/NettyWebSocketClient.java

src/main/java/com/google/firebase/database/util/EmulatorHelper.java

src/main/java/com/google/firebase/database/utilities/Utilities.java

src/test/java/com/google/firebase/database/util/EmulatorHelperTest.java

hiranya911

Just a few suggestions, the main one being handling emulator credentials all within the FirebaseDatabase implementation. Basically allow empty credentials in FirebaseOptions when configured for emulator mode, and then init EmulatorCredentials in FirebaseDatabase.

src/main/java/com/google/firebase/FirebaseOptions.java

src/main/java/com/google/firebase/database/util/EmulatorHelper.java

src/main/java/com/google/firebase/FirebaseApp.java

src/main/java/com/google/firebase/database/util/EmulatorHelper.java

src/test/java/com/google/firebase/database/utilities/UtilitiesTest.java

…FirebaseDatabase only, updated test cases

src/main/java/com/google/firebase/FirebaseOptions.java

src/test/java/com/google/firebase/database/utilities/UtilitiesTest.java

… a valid URL through the databaseUrl field

yuchenshi

Code LGTM to me. Please address the comments and get an approval from at least one other reviewer before merging.

schmidt-sebastian

I left some more comments around URL parsing, but the rest of the code and the plumbing looks fantastic now! Thanks for your perseverance.

src/main/java/com/google/firebase/FirebaseApp.java

src/main/java/com/google/firebase/database/FirebaseDatabase.java

src/main/java/com/google/firebase/database/core/JvmAuthTokenProvider.java

src/main/java/com/google/firebase/database/util/EmulatorHelper.java

src/main/java/com/google/firebase/database/utilities/Utilities.java

src/test/java/com/google/firebase/database/FirebaseDatabaseTest.java

hiranya911

Thanks for seeing this through. Just one comment regarding the core SDK. I'll defer to @schmidt-sebastian regarding the changes in the database package.

src/main/java/com/google/firebase/FirebaseApp.java

hiranya911

LGTM with one comment. Feel free to merge once that comment and any other open comments from @schmidt-sebastian are addressed.

src/main/java/com/google/firebase/FirebaseOptions.java

schmidt-sebastian

This looks great! Thanks for all the updates!

src/test/java/com/google/firebase/database/FirebaseDatabaseTest.java

sarajmunjal added 2 commits August 12, 2019 12:25

Talk to RTDB emulator instead of database instance if FIREBASE_RTDB_E…

de2f793

…MULATOR_HOST=true

Added test for talking to rtdb emulator when vars are set

d61ca78

sarajmunjal requested review from hiranya911 and yuchenshi August 12, 2019 20:53

yuchenshi removed request for hiranya911 and yuchenshi August 12, 2019 20:58

hiranya911 suggested changes Aug 13, 2019

View reviewed changes

src/main/java/com/google/firebase/database/FirebaseDatabase.java Outdated Show resolved Hide resolved

hiranya911 assigned sarajmunjal Aug 13, 2019

yuchenshi changed the title ~~Connect to RTDB emulator when correct vars are set~~ [WIP] Connect to RTDB emulator when correct vars are set Aug 13, 2019

sarajmunjal added 2 commits August 16, 2019 18:12

Updated code and added tests to reflect valid emulator URLs and the l…

f036ce3

…ogic for inferring them correctly

Remove stray character

3f39fda

sarajmunjal requested a review from schmidt-sebastian August 17, 2019 01:43

sarajmunjal changed the title ~~[WIP] Connect to RTDB emulator when correct vars are set~~ Connect to RTDB emulator when correct vars are set Aug 17, 2019

sarajmunjal changed the title ~~Connect to RTDB emulator when correct vars are set~~ Connect to RTDB emulator when valid emulator URL is passed OR env vars are set correctly Aug 17, 2019

sarajmunjal assigned hiranya911 and yuchenshi and unassigned sarajmunjal Aug 17, 2019

sarajmunjal commented Aug 17, 2019

View reviewed changes

src/main/java/com/google/firebase/FirebaseApp.java Outdated Show resolved Hide resolved

Renamed setup and teardown methods to be correct

eeb38e5

sarajmunjal commented Aug 17, 2019

View reviewed changes

src/main/java/com/google/firebase/database/connection/NettyWebSocketClient.java Outdated Show resolved Hide resolved

sarajmunjal commented Aug 17, 2019

View reviewed changes

src/test/java/com/google/firebase/database/FirebaseDatabaseTest.java Outdated Show resolved Hide resolved

sarajmunjal commented Aug 17, 2019

View reviewed changes

src/test/java/com/google/firebase/testing/TestUtils.java Show resolved Hide resolved

schmidt-sebastian reviewed Aug 19, 2019

View reviewed changes

src/test/java/com/google/firebase/database/util/EmulatorHelperTest.java Outdated Show resolved Hide resolved

hiranya911 suggested changes Aug 19, 2019

View reviewed changes

yuchenshi reviewed Aug 19, 2019

View reviewed changes

src/test/java/com/google/firebase/database/utilities/UtilitiesTest.java Show resolved Hide resolved

sarajmunjal added 3 commits August 20, 2019 19:21

Addressed most review comments, moved emulator URL override check to …

e70c474

…FirebaseDatabase only, updated test cases

Added deferred credentials functionality

9ffb379

Verified and fixed parsing of encoded URLs

b28f277

sarajmunjal commented Aug 21, 2019

View reviewed changes

src/main/java/com/google/firebase/FirebaseOptions.java Show resolved Hide resolved

yuchenshi reviewed Aug 21, 2019

View reviewed changes

src/test/java/com/google/firebase/database/utilities/UtilitiesTest.java Outdated Show resolved Hide resolved

sarajmunjal added 4 commits August 21, 2019 14:05

Fixed another round of reveiw comments

4d41237

Fixed checkstyle errors

a69b40e

Allowed HTTPS connections to the emulator, only if the user passes in…

21461bd

… a valid URL through the databaseUrl field

Minor changes

6acfc27

sarajmunjal unassigned hiranya911, sarajmunjal and yuchenshi Aug 21, 2019

Minor fixes

f9d9975

sarajmunjal assigned yuchenshi Aug 21, 2019

yuchenshi approved these changes Aug 22, 2019

View reviewed changes

sarajmunjal assigned schmidt-sebastian and unassigned yuchenshi Aug 22, 2019

schmidt-sebastian reviewed Aug 22, 2019

View reviewed changes

schmidt-sebastian removed their assignment Aug 22, 2019

hiranya911 suggested changes Aug 22, 2019

View reviewed changes

src/main/java/com/google/firebase/FirebaseApp.java Outdated Show resolved Hide resolved

hiranya911 assigned sarajmunjal Aug 22, 2019

sarajmunjal added 2 commits August 22, 2019 17:35

Fixes from review comments

9df8f99

Fixed test imports

7718d63

hiranya911 approved these changes Aug 23, 2019

View reviewed changes

src/main/java/com/google/firebase/FirebaseOptions.java Outdated Show resolved Hide resolved

Handled + in URLs and added test

71d51f3

sarajmunjal assigned schmidt-sebastian and unassigned sarajmunjal Aug 23, 2019

schmidt-sebastian approved these changes Aug 23, 2019

View reviewed changes

src/test/java/com/google/firebase/database/FirebaseDatabaseTest.java Outdated Show resolved Hide resolved

schmidt-sebastian assigned sarajmunjal Aug 23, 2019

hiranya911 changed the title ~~Connect to RTDB emulator when valid emulator URL is passed OR env vars are set correctly~~ feat(rtdb): Connect to RTDB emulator when valid emulator URL is passed OR env vars are set correctly Aug 23, 2019

hiranya911 added the release-note label Aug 23, 2019

Inlined variable declarations

097fdc7

sarajmunjal merged commit 9ace7b1 into master Aug 23, 2019

sarajmunjal deleted the sarajmun-connect-rtdb-emulator branch August 23, 2019 21:42

feat(rtdb): Connect to RTDB emulator when valid emulator URL is passed OR env vars are set correctly #299

feat(rtdb): Connect to RTDB emulator when valid emulator URL is passed OR env vars are set correctly #299

Uh oh!

Conversation

sarajmunjal commented Aug 12, 2019 • edited by hiranya911 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuchenshi left a comment

Choose a reason for hiding this comment

Uh oh!

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sarajmunjal commented Aug 12, 2019 •

edited by hiranya911

Loading