-
Notifications
You must be signed in to change notification settings - Fork 76
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
Fixing Database emulation #278
Changes from all commits
bdb0abe
a1a8ee2
cd4f1dd
a2980ff
17f7d16
4a0eebd
02910d1
b246aa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -379,7 +379,7 @@ func _on_FirebaseAuth_request_completed(result : int, response_code : int, heade | |
var json_result = JSON.parse(bod) | ||
if json_result.error != OK: | ||
Firebase._printerr("Error while parsing auth body json") | ||
emit_signal("auth_request", "Error while parsing auth body json", json_result) | ||
emit_signal("auth_request", ERR_PARSE_ERROR, "Error while parsing auth body json") | ||
return | ||
res = json_result.result | ||
|
||
|
@@ -390,7 +390,8 @@ func _on_FirebaseAuth_request_completed(result : int, response_code : int, heade | |
Requests.EXCHANGE_TOKEN: | ||
emit_signal("token_exchanged", true) | ||
begin_refresh_countdown() | ||
emit_signal("auth_request", "Refresh token countdown", auth) | ||
# Refresh token countdown | ||
emit_signal("auth_request", 1, auth) | ||
Comment on lines
+393
to
+394
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A token refresh is not an error, and 1 is expected on success |
||
else: | ||
match res.kind: | ||
RESPONSE_SIGNUP: | ||
|
@@ -439,7 +440,7 @@ func load_auth() -> void: | |
var err = encrypted_file.open_encrypted_with_pass("user://user.auth", File.READ, _config.apiKey) | ||
if err != OK: | ||
Firebase._printerr("Error Opening Firebase Auth File. Error Code: " + err) | ||
emit_signal("auth_request", "Error Opening Firebase Auth File. Error Code: " + err, auth) | ||
emit_signal("auth_request", err, "Error Opening Firebase Auth File.") | ||
else: | ||
var encrypted_file_data = parse_json(encrypted_file.get_line()) | ||
manual_token_refresh(encrypted_file_data) | ||
|
@@ -463,7 +464,7 @@ func check_auth_file() -> void: | |
load_auth() | ||
else: | ||
Firebase._printerr("Encrypted Firebase Auth file does not exist") | ||
emit_signal("auth_request", "Encrypted Firebase Auth file does not exist", auth) | ||
emit_signal("auth_request", ERR_DOES_NOT_EXIST, "Encrypted Firebase Auth file does not exist") | ||
|
||
|
||
# Function used to change the email account for the currently logged in user | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,13 +20,13 @@ func _set_config(config_json : Dictionary) -> void: | |
func _check_emulating() -> void : | ||
## Check emulating | ||
if not Firebase.emulating: | ||
_base_url = _config.databaseURL + "/" # + ListName + _json_list_tag + _auth_tag + _auth.idtoken | ||
_base_url = _config.databaseURL | ||
else: | ||
var port : String = _config.emulators.ports.realtimeDatabase | ||
if port == "": | ||
Firebase._printerr("You are in 'emulated' mode, but the port for Realtime Database has not been configured.") | ||
else: | ||
_base_url = "http://localhost:{port}/?ns={projectId}".format({ port = port, projectId = _config.projectId }) + "/" | ||
_base_url = "http://localhost" | ||
Comment on lines
+23
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've moved the addition of the separator to the reference.gd |
||
|
||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ const _accept_header : String = "accept: text/event-stream" | |
const _auth_variable_begin : String = "[" | ||
const _auth_variable_end : String = "]" | ||
const _filter_tag : String = "&" | ||
const _escaped_quote : String = "\"" | ||
const _escaped_quote : String = '"' | ||
const _equal_tag : String = "=" | ||
const _key_filter_tag : String = "$key" | ||
|
||
|
@@ -69,9 +69,12 @@ func set_listener(listener_ref : Node) -> void: | |
_listener = listener_ref | ||
add_child(_listener) | ||
_listener.connect("new_sse_event", self, "on_new_sse_event") | ||
var base_url = _get_list_url().trim_suffix(_separator) | ||
var base_url = _get_list_url(false).trim_suffix(_separator) | ||
var extended_url = _separator + _db_path + _get_remaining_path(false) | ||
_listener.connect_to_host(base_url, extended_url) | ||
var port = -1 | ||
if Firebase.emulating: | ||
port = int(_config.emulators.ports.realtimeDatabase) | ||
_listener.connect_to_host(base_url, extended_url, port) | ||
Comment on lines
+74
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -1 is the default value |
||
|
||
func on_new_sse_event(headers : Dictionary, event : String, data : Dictionary) -> void: | ||
if data: | ||
|
@@ -133,13 +136,22 @@ func get_data() -> Dictionary: | |
return _store.get_data() | ||
|
||
func _get_remaining_path(is_push : bool = true) -> String: | ||
var remaining_path = "" | ||
if !_filter_query or is_push: | ||
return _json_list_tag + _query_tag + _auth_tag + Firebase.Auth.auth.idtoken | ||
remaining_path = _json_list_tag + _query_tag + _auth_tag + Firebase.Auth.auth.idtoken | ||
else: | ||
return _json_list_tag + _query_tag + _get_filter() + _filter_tag + _auth_tag + Firebase.Auth.auth.idtoken | ||
remaining_path = _json_list_tag + _query_tag + _get_filter() + _filter_tag + _auth_tag + Firebase.Auth.auth.idtoken | ||
|
||
if Firebase.emulating: | ||
remaining_path += "&ns="+_config.projectId+"-default-rtdb" | ||
Comment on lines
+145
to
+146
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When emulating a namespace must be specified, it is your projectId + the extra content there. |
||
|
||
return remaining_path | ||
|
||
func _get_list_url() -> String: | ||
return Firebase.Database._base_url | ||
func _get_list_url(with_port:bool = true) -> String: | ||
var url = Firebase.Database._base_url.trim_suffix(_separator) | ||
if with_port and Firebase.emulating: | ||
url += ":" + _config.emulators.ports.realtimeDatabase | ||
return url + _separator | ||
Comment on lines
+150
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. defaults to expected behavior, but this could likely be better since with_port is only actually useful when emulating. |
||
|
||
|
||
func _get_filter(): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,7 @@ func _check_emulating() -> void : | |
if port == "": | ||
Firebase._printerr("You are in 'emulated' mode, but the port for Dynamic Links has not been configured.") | ||
else: | ||
_base_url = "http://127.0.0.1:{port}/{version}/".format({ version = _API_VERSION, port = port }) | ||
_base_url = "http://localhost:{port}/{version}/".format({ version = _API_VERSION, port = port }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noticed some spots using 127.0.0.1 instead of localhost |
||
|
||
|
||
var _link_request_body : Dictionary = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,8 +65,11 @@ var _config : Dictionary = { | |
"functions" : "", | ||
"storage" : "", | ||
"dynamicLinks" : "" | ||
} | ||
} | ||
} | ||
}, | ||
"workarounds":{ | ||
"database_connection_closed_issue": false, # fixes https://github.com/firebase/firebase-tools/issues/3329 | ||
} | ||
Comment on lines
+70
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a flag for the workaround disabling it by default since it only effects emulation |
||
} | ||
|
||
func _ready() -> void: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,24 +140,24 @@ func _check_emulating() -> void : | |
if port == "": | ||
Firebase._printerr("You are in 'emulated' mode, but the port for Cloud Functions has not been configured.") | ||
else: | ||
_base_url = "http://127.0.0.1:{port}/{projectId}/{zone}/".format({ port = port, zone = _config.functionsGeoZone, projectId = _config.projectId }) | ||
_base_url = "http://localhost:{port}/{projectId}/{zone}/".format({ port = port, zone = _config.functionsGeoZone, projectId = _config.projectId }) | ||
|
||
|
||
func _pooled_request(task : FunctionTask) -> void: | ||
if _offline: | ||
task._on_request_completed(HTTPRequest.RESULT_CANT_CONNECT, 404, PoolStringArray(), PoolByteArray()) | ||
return | ||
|
||
if not auth and not Firebase.emulating: | ||
if not auth: | ||
Firebase._print("Unauthenticated request issued...") | ||
Firebase.Auth.login_anonymous() | ||
var result : Array = yield(Firebase.Auth, "auth_request") | ||
if result[0] != 1: | ||
_check_auth_error(result[0], result[1]) | ||
Firebase._print("Client connected as Anonymous") | ||
|
||
if not Firebase.emulating: | ||
task._headers = Array(task._headers) + [_AUTHORIZATION_HEADER + auth.idtoken] | ||
|
||
task._headers = Array(task._headers) + [_AUTHORIZATION_HEADER + auth.idtoken] | ||
Comment on lines
+151
to
+160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With changes to the auth.gd error codes this does not fail like it normally would. |
||
|
||
var http_request : HTTPRequest | ||
for request in _http_request_pool: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,6 @@ var use_ssl | |
var verify_host | ||
var told_to_connect = false | ||
var connection_in_progress = false | ||
var request_in_progress = false | ||
var is_requested = false | ||
var response_body = PoolByteArray() | ||
|
||
|
@@ -48,6 +47,19 @@ func attempt_to_request(httpclient_status): | |
if err == OK: | ||
is_requested = true | ||
|
||
func _parse_response_body(headers): | ||
var body = response_body.get_string_from_utf8() | ||
if body: | ||
var event_data = get_event_data(body) | ||
if event_data.event != "keep-alive" and event_data.event != continue_internal: | ||
var result = JSON.parse(event_data.data).result | ||
if response_body.size() > 0 and result: # stop here if the value doesn't parse | ||
response_body.resize(0) | ||
emit_signal("new_sse_event", headers, event_data.event, result) | ||
else: | ||
if event_data.event != continue_internal: | ||
response_body.resize(0) | ||
Comment on lines
+50
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moved this to it's own function (with headers just as a passthrough) |
||
|
||
func _process(delta): | ||
if !told_to_connect: | ||
return | ||
|
@@ -61,33 +73,32 @@ func _process(delta): | |
httpclient.poll() | ||
var httpclient_status = httpclient.get_status() | ||
if !is_requested: | ||
if !request_in_progress: | ||
attempt_to_request(httpclient_status) | ||
attempt_to_request(httpclient_status) | ||
return | ||
|
||
var httpclient_has_response = httpclient.has_response() | ||
|
||
if httpclient_has_response or httpclient_status == HTTPClient.STATUS_BODY: | ||
|
||
if httpclient.has_response(): | ||
var headers = httpclient.get_response_headers_as_dictionary() | ||
|
||
httpclient.poll() | ||
var chunk = httpclient.read_response_body_chunk() | ||
if(chunk.size() == 0): | ||
return | ||
else: | ||
response_body = response_body + chunk | ||
|
||
var body = response_body.get_string_from_utf8() | ||
if body: | ||
var event_data = get_event_data(body) | ||
if event_data.event != "keep-alive" and event_data.event != continue_internal: | ||
var result = JSON.parse(event_data.data).result | ||
if response_body.size() > 0 and result: # stop here if the value doesn't parse | ||
response_body.resize(0) | ||
emit_signal("new_sse_event", headers, event_data.event, result) | ||
if httpclient_status == HTTPClient.STATUS_BODY: | ||
httpclient.poll() | ||
var chunk = httpclient.read_response_body_chunk() | ||
if(chunk.size() == 0): | ||
return | ||
else: | ||
if event_data.event != continue_internal: | ||
response_body.resize(0) | ||
response_body = response_body + chunk | ||
|
||
_parse_response_body(headers) | ||
|
||
elif Firebase.emulating and Firebase._config.workarounds.database_connection_closed_issue: | ||
# Emulation does not send the close connection header currently, so we need to manually read the response body | ||
# see issue https://github.com/firebase/firebase-tools/issues/3329 in firebase-tools | ||
# also comment https://github.com/GodotNuts/GodotFirebase/issues/154#issuecomment-831377763 which explains the issue | ||
while httpclient.connection.get_available_bytes(): | ||
var data = httpclient.connection.get_partial_data(1) | ||
if data[0] == OK: | ||
response_body.append_array(data[1]) | ||
if response_body.size() > 0: | ||
_parse_response_body(headers) | ||
|
||
func get_event_data(body : String) -> Dictionary: | ||
var result = {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/LeonardMeagher2/GodotFirebase/blob/4a0eebdc433355e800467a8465ec93e007180b8b/addons/godot-firebase/auth/auth.gd#L14
The documentation for this signal says error should have error codes and a message