Skip to content

Support the RTDB emulator#3491

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.

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 6 commits into from
Aug 5, 2019
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

This adds support for the emulator and also for the "ns" query param.\

Technically, iOS doesn't need the "ns" query param since "localhost" is a valid RTDB namespace name and we could just use "localhost" as the namespace. But since I just went all out on Android (firebase/firebase-android-sdk#680), I decided to make this PR equally awesome. Plus who doesn't like some good URL decoding/encoding mess.

@yuchenshi
Copy link
Member

We still need people to be able to specify the database name (for example to test triggers), so just accepting localhost is not a great answer. So, thanks for the PR and it's really important!

Copy link
Contributor

@mikelehenmikelehen left a comment

Choose a reason for hiding this comment

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

LGTM with nits.

to the emulator, specify "http://<emulator_host>/" as your Database URL
(via `Database.database(url:)`).
If you refer to your emulator host by IP rather than by domain name, you may
also need to specify a namespace ("http://<emulator_host>/?ns=<project_id>").
Copy link
Contributor

Choose a reason for hiding this comment

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

"namespace" => " project id" ? Or some other implementation-independent term.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Done.

bool secure;

if (urlComponents.port != nil) {
secure = [urlComponents.scheme isEqualToString:@"https"];
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, on web I think we support wss:// URLs (which IIRC disables long-polling). I wonder if we should try for some consistency.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I added support for "wss".

secure = [urlComponents.scheme isEqualToString:@"https"];
host = [host stringByAppendingFormat:@":%@", urlComponents.port];
} else if ( [urlComponents.host isEqualToString:@"localhost"]) {
secure = NO;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we do this? Did the old code do this?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Seems like this is not winning anyone over, so I dropped this from the iOS port (will drop the special casing for Android too).

}


NSString* pathString = [self decodePath:[NSString stringWithFormat:@"/%@", originalPathString]];
FPath * path = [[FPath alloc] initWith:pathString];
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like there are extra spaces here? Why does clang-format not fix that?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed manually. Seems like we are excluded from the formatter (but it still takes 10 seconds to run). I'll get the formatting enabled for RTDB in a follow up.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

This source is now clang-formatted.

Copy link
ContributorAuthor

@schmidt-sebastianschmidt-sebastian left a comment

Choose a reason for hiding this comment

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

Comments addressed.

to the emulator, specify "http://<emulator_host>/" as your Database URL
(via `Database.database(url:)`).
If you refer to your emulator host by IP rather than by domain name, you may
also need to specify a namespace ("http://<emulator_host>/?ns=<project_id>").
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Done.

bool secure;

if (urlComponents.port != nil) {
secure = [urlComponents.scheme isEqualToString:@"https"];
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I added support for "wss".

secure = [urlComponents.scheme isEqualToString:@"https"];
host = [host stringByAppendingFormat:@":%@", urlComponents.port];
} else if ( [urlComponents.host isEqualToString:@"localhost"]) {
secure = NO;
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Seems like this is not winning anyone over, so I dropped this from the iOS port (will drop the special casing for Android too).

}


NSString* pathString = [self decodePath:[NSString stringWithFormat:@"/%@", originalPathString]];
FPath * path = [[FPath alloc] initWith:pathString];
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed manually. Seems like we are excluded from the formatter (but it still takes 10 seconds to run). I'll get the formatting enabled for RTDB in a follow up.

@schmidt-sebastianschmidt-sebastian merged commit c706ce9 into masterAug 5, 2019
@schmidt-sebastianschmidt-sebastian deleted the mrschmidt/emulator branch August 5, 2019 23:54
@firebasefirebase locked and limited conversation to collaborators Oct 10, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4 participants
@schmidt-sebastian@yuchenshi@mikelehen@googlebot
close