-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
BUG DECODING KEYS FROM FIRESTORE. #12729
Comments
I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight. |
Workaround: Create a custom init(from decoder: Decoder), decode it with the container using [String:String] and then reduce Into your [StringRawValueEnum:String] |
Hi @jesus-mg-ios! I tried to reproduce your problem with no luck. How is your I constructed a local test with:
And it seems to work for me. Could you provide a little more code explaining the problem? |
Thanks @tom-andersen the example should look like this:
|
I wrote the following test, and it passes in my enironment:
What are you doing different? |
The only thing that I'm doing differently is getting data from the snapshot listener, also not using a local simulator (that I don't know if it's your case) and using optional properties of the model (but as far as I see, it doesn't depend on using decodeIfPresent or not). The model is created from the firebase admin/console with firebase database (not realtime database), not from local, so maybe it is something to consider, but the structure decode works with the workaround, so, it's a factor that shouldn't affect it. |
Do you mind sharing a repro? |
It sounds to me like you're getting a default behavior of Codable that is a bit unfortunate. https://github.com/apple/swift-evolution/blob/main/proposals/0320-codingkeyrepresentable.md You need to conform your key type to CodingKeyRepresentable. That should do it. Please let me know if it helps! |
Thanks for sharing this valuable information @mortenbekditlevsen. I propose to extend the current implementation of Firebase decoder/encoder. In firebase a Map only can do it by using keys as string, because is a json representation constraint. So we could manage it using something like this:
Thoughts @tom-andersen |
My two cents: the change would affect any user importing Firebase. And even though the existing behavior seems like a bug (and I suspect an oversight in the Codable system from the start), then we can't definitely say that no people are relying on the behavior. The change would be behavior breaking for them. So as annoying as the issue is, it's just one of those things that we probably can't ever fix... |
Why would it be a breaking change? @mortenbekditlevsen In this case, I think that would depend on the code position of decoding or encoding, adding a step more for people who do fail into failing decoding/coding error |
If people are relying on the fact that any non-String key in a Dictionary by default will encode to and decode from an array of pairs of keys and values, then importing Firebase will change that behavior. And not just for encodings to and from Firestore or the Real-time database, but for all other Codable purposes too. |
I disagree because the people who start to decode from an array of keys and values, will not affected by the change, nor people using Firestore or real-time databases. As you said "will encode to and decode from an array of pairs of keys and values", so -> they will not encode to and decode from a dictionary, defining their data to decode as an Array, different case managed. Thoughts @tom-andersen |
I'll try explaining myself a little better. It just doesn't encode to the format you expect (and I totally get why you expect that). I modified Tom's test to a small playground example:
The print statement at (1) gives: So: a list of alternating keys and values - and not what you expect (you expect the more natural map representation). But it is still a valid serialization, and it still decodes completely fine. So if you apply your suggested change (similarly to at some point starting to conform your
In this example, I have now made the key support encoding to and decoding from a map. I try decoding the previously encoded data which now fails. That is what I mean by a behavior breaking change. Another note is that the current behavior aligns with the behavior of |
Okay, now I get you. Your case said that someone could be trying to decode an array of things (that is prone to error because it must be generated with even elements) to a dictionary, and it's fine because it decodes and codes well. The thing that I didn't get is why the decode fails on my side, using the Firestore decoder, when data is populated from the admin console. Even if I change to [String: String] instead of [CharacterAlignment: String], it works like a charm. The key point is. Why does the Custom Firebase Decoder say "Expected to decode Array but found a dictionary instead." when I'm creating a a model with a dictionary. |
The behavior is inherited from Apple's JSONDecoder. The reason is historical, but in the lines of: Thus, the strategy was: if the key is not a String, we encode keys and values alternately into an unkeyedContainer (array/list). This is why you see that it expects the (encoded) data to be in the shape of an array. When decoding. At the time they missed the case: but what if your key still encodes as a String. Specifically: what if it's RawRepresentable as a String. And that's what SE-0320 tries to deal with. But it's too late to make a global fix. It must be done case by case, which is a real shame. |
Description
The firestore decoder keep in mind _JSONStringDictionaryDecodableMarker.Type. So for maps, if we have something like this:
{ "name": {"KEY": VALUE } }
and we decode it with a codable structure with a property of type
[String: String]
it works.The Behavior using
[EnumStringRawValue: String]
does fail into:Because in this case key is always represented as a String
The expected behavior is to decode a map with an enum string raw representable if it can fit on the string raw representable cases.
Reproducing the issue
Add a document with a map in Firestore.
Try to decode it with document.data() passing as an argument your struct or class metatype. Inside of the struct or class metatype define a map with StringRawValueEnum as key.
Firebase SDK Version
10.23.0
Xcode Version
15.2
Installation Method
Swift Package Manager
Firebase Product(s)
Firestore
Targeted Platforms
iOS
Relevant Log Output
No response
If using Swift Package Manager, the project's Package.resolved
Expand
Package.resolved
snippetReplace this line with the contents of your Package.resolved.
If using CocoaPods, the project's Podfile.lock
Expand
Podfile.lock
snippetReplace this line with the contents of your Podfile.lock!
The text was updated successfully, but these errors were encountered: