Tolerate nested JSON objects while getting URL #10370
Merged
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.
Discussion
The firebase storage emulator returns JSON that contains a
metadata
object that is empty (implying swift[String: Any]
type), whereas the cloud API does not (so the current swift[String: String]
type works for it)Thus, attempting to get a download URL against the storage emulator fails since the type checking fails though it does work against cloud APIs.
This may be a bug in the firestore emulator, as it may not be emitting JSON correctly,
However following the theory of "be conservative in what you emit, but liberal in what you accept", it seems slightly better to make a patch here so an empty metadata object is okay. I could be wrong! That would require communication direct between firebase-ios-sdk + firebase-tools crew and could easily be sorted out I'm sure.
In the meantime, here's a patch.
Testing
Here is the JSON from the emulator, logged out from the code, the metadata object is the very last one:
With just two type coercions back to String from Any, both of which have fallbacks if the cast is invalid, download URLs come through the new Swift code from the storage emulator just fine.
react-native-firebase test rig exercising this is in progress, but in general it should be reproducible by:
The problem is also noted in FlutterFire repo, where a test was disabled to workaround this, for now:
firebase/flutterfire#9708
https://github.com/firebase/flutterfire/pull/9708/files#diff-99b8ba26ff8b661cc82057ebd3c6d8121e8e6e95695955d1b5848c3f08e30c91R141-R143
API Changes
us make Firebase APIs better, please propose your change in a feature request so that we
can discuss it together.