Fix export users #1690
Merged
Fix export users #1690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@kossnocorp thanks for your contribution! Assigning this one to @bkendall who reviewed code on this path recently. |
bkendall
requested changes
Oct 7, 2019
There was a problem hiding this comment.
This looks like a reasonable fix to me. Thanks for the PR. A couple things to clean up in the test, but should be easy enough.
Please also
- Sync with
master - Add an entry to
CHANGELOG.md, maybe:
* Fixes invalid JSON output in `auth.export` within a scripting environment.
|
@bkendall thank you for the review, I'll address that. |
When `serialExportUsers` function is called more than once,
it adds a redundant comma, which breaks the JSON format.
The first call:
```
{"users": [
{
"localId": "DxADx87BFyPe8HhD4dOAyuwXECu1",
...
```
The second call (and all consecutive):
```
{"users": [
,
{
"localId": "DxADx87BFyPe8HhD4dOAyuwXECu1",
...
```
When `serialExportUsers` function is called more than once, it adds a redundant comma, which breaks the JSON format. The bug is caused by a global variable `jsonSep` in an internal `_writeUsersToFile` function. I assume that it was first introduced to make JSON streamable. I changed the code to make it generate `writeUsersToFile` shared among recursive calls.
bkendall
approved these changes
Oct 8, 2019
|
Thanks for the PR! |
Elgarni
added a commit
to Elgarni/firebase-tools
that referenced
this pull request
Oct 9, 2019
* master: change out tslint for eslint, new publish config (firebase#486) Remove scripts/package.json and update firebase version. (firebase#1704) Fix functions deploy integration test script (firebase#1703) Loop the modules properties without prototype methods. Fixes firebase#1687 (firebase#1694) Allow customers to configure the db setting 'strictTriggerValidation' (firebase#1702) [firebase-release] Removed change log and reset repo after 7.5.0 release 7.5.0 Fix export users (firebase#1690) Fix port issues in WSL (firebase#1699) Unremove but deprecate separate port for WebChannel. (firebase#1698) Release Firestore Emulator 1.9.0 and remove WebChannel workaround. (firebase#1689) update handlebars dependency (firebase#1686) [firebase-release] Removed change log and reset repo after 7.4.0 release 7.4.0 remove items from previews list (firebase#488)
|
You're very welcome, thank you for a great platform! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a bug in
auth.exportcommand. WhenserialExportUsersfunction (used by the command) is called more than once, it adds a redundant comma, which breaks the JSON format.The first call:
The second call (and all consecutive):
The bug is caused by a global variable
jsonSepin an internal_writeUsersToFilefunction. I assume that it was first introduced to make JSON streamable.I changed the code to make it generate
writeUsersToFileshared among recursive calls.Scenarios Tested
When the package is used via Node.js API:
Always produce valid JSON. Before the changes, the second
tools.auth.exportcall produces malformed JSON (see the,as the first array element):