Roundtripping key coding strategies

[Morten_Bek_Ditlevsen]

Hi all,
Late last year I initiated the topic: [Pre-pitch] Roundtripping key coding strategies.

The topic highlights a well known issue with the current .convertToSnakeCase keyEncodingStrategy of JSONEncoder and .convertFromSnakeCase keyDecodingStrategy of JSONDecoder.

I will highlight the issue here with examples from the linked topic.

The core issue is that not all keys round-trip.

As an example, consider the following struct:

struct Person: Codable {
  var imageURL: URL
}

Using .convertToSnakeCase with JSONEncoder will produce:

{
  "image_url": "..."
}

But the corresponding decoding will go from snake case to camel case by a fixed algorithm, trying to look up the key: imageUrl, which does not exist.

This is a common source of bugs when using key coding strategies, and at least in code bases that I am familiar with, the workaround is often to add custom coding keys like:

enum CodingKeys: String, CodingKey {
  case imageURL = "imageUrl"
}

This allows the imageURL property to roundtrip when used with the snake case encoding and decoding, but this is a 'leaky abstraction'.

Codable entities and the encoder/decoder they are used with are supposed to be decoupled, but in this situation, the developer needs to know if the codable entity is used with an encoder/decoder pair that use key transformations - and also need to remember to map the key correctly, so that it will be 'found' when converting back from snake case to camel case.

Often I have seen attempts to 'fix' the behavior with the notation you would use if you didn't apply a key coding strategy:

enum CodingKeys: String, CodingKey {
  case imageURL = "image_url"
}

which of course is no good when used with snake case conversion, since the key that will be looked up is "imageUrl".

In other words: the work-around to fix this issue is a bit counter-intuitive and also has the issue that you 'tie' the coding key to a specific encoding/decoding use case.

Although the issue is in the domain of Foundation, I can't see a fix to the issue that does not also involve the Swift standard library and this was the reasoning for bringing up the topic in the first place.

After a small bit of welcome feedback I turned the pre-pitch into a pitch ([Pitch] Roundtripping key coding strategies) and got a slight bit of extra feedback.

I presented the possible SE proposal to the Core Team where I received some very appropriate feedback:

My initial 'pre-pitch', 'pitch' and SE proposal were all presented in these forums in a very formal style - and with lists of pros and cons that left little room for discussion or engagement from the community and possibly from the Foundation team.

So now I would like to re-open the discussion, but hopefully in a style that allows more people to engage.

The questions I would like help answering are:

1. Is this issue big enough to warrant a fix?
2. Can the issue be fixed in Foundation alone? How could such fixes look?
3. Would a fix require changes to the standard libraries? How could such fixes look? Is the issue big enough to warrant changes to the Codable system in the standard libraries?

I'll take the liberty to tag some of the commenters of the previous topics:

@itaiferber @gwendal.roue @Nickolas_Pohilets @hisekaldma @tomerd

Any feedback is more than welcome, and I hope to be able to engage people from the Foundation team too.

[gwendal.roue (Gwendal Roué)]

Hi Morten,

I don't have much to say about the potential solution, but I agree there is a problem.

A few days ago, I saw myself advising to a co-worker: "Oh, you should rename xxxID to xxxId, because this will avoid problems with Codable." This was partially inexact, because we were not using any key coding strategy. Yet I have somehow internalized that there is a trap lurking in this area, and kind of cargo-culted the naming of Codable properties: this is concerning.

If xxxId vs xxxID is not such a big deal, xxxUrl vs xxxURL totally is. We are definitely supposed to name our properties xxxURL.

EDIT: oh, and my implementation of key coding strategies for GRDB the SQLite library is a straight copy from the standard one (so that people do not have to learn anything new) - with exactly the same problems.

[itaiferber]

Thanks for bringing this back up, Morten. I think my gut feeling has remained the same since last time:

@itaiferber

Gut feeling: this is a problem mostly created by JSONEncoder for itself — I'm not sure how many other encoders allow key modification in this way. It may be entirely sufficient to confine the solution there

From a principled standpoint (at least for me), adjusting the raw values of a type's CodingKeys to match what a single encoder does isn't (or shouldn't be) the right approach. Unless one overrides the Codable methods to use multiple pairs of keys, they're reused for all types — which means that a "quick" adjustment today to "fix" the results from JSON{En,De}coder means that you propagate this weirdness to other formats/encoders.

To answer your questions, from my perspective:

1. Yes, I think this issue warrants fixing
2. I believe the issue can be fixed in Foundation alone. Details below
3. I don't believe fixes would need to apply to the standard libraries; this is really between a type and JSON{En,De}coder

I haven't had the opportunity to think this through entirely, but a thought: back in the mists of time, @Tony_Parker and I discussed the possibility of having JSON{En,De}coder build in a list of known abbreviations which we could correctly identify as terms which would be correctly cased ("HTML", "URL", etc.) with the default .convert{To,From}SnakeCase strategies, but didn't get to explore this idea at the time. I wasn't a fan of having Foundation hard-code a list of known values like this, and I still don't, but luckily it could be easy to extend to users:

enum JSONDecoder.KeyDecodingStrategy {
    // ...

    // Not entirely thought through: some options to maybe allow/ignore
    // recognition at the beginning of a case name, like
    // `htmlForURL` vs. `HTMLForURL`
    case convertFromSnakeCaseWithKnownAbbreviations(Set<String>, options: SomeOptionType)

    // ...
}

Sadly, I don't believe adding associated values to existing enum cases is ABI-stable, or else we could do something like

enum JSONDecoder.KeyDecodingStrategy {
    case convertFromSnakeCase(knownAbbreviations: Set<String> = [], options: SomeOptionType = .default)
}

to maintain source stability, and not have to come up with a new case entirely.

Either way, this could give users an "in" to provide abbreviations their CodingKeys use across an entire payload, and theoretically, they could revert changes to their CodingKeys' raw values.

It would be up to them to scan through their CodingKeys to find these abbreviations, or Foundation could provide at least a default list (JSONDecoder.KeyDecodingStrategy.defaultKnownAbbreviations?) that could be extended with additional values before being passed in to the enum case. (I can see folks cargo-culting a "preferred" list of abbreviations either way, regardless of application, but I don't think that's worth worrying about.)

---

I haven't had time to fully think through this idea or its implications, so please poke holes if this obviously won't work for some reason, but I think this might be at least a compelling place to start exploring a fix in a contained way.

(Sadly, this does leave other encoders/decoders in the lurch if users have already adjusted CodingKeys for this purpose and used them for other formats — but my gut feeling is that fixing the issue "at the source" might be a good start.)

[hisekaldma (Jonathan Hise Kaldma)]

I think the whole concept of key conversion is a bit suspect.

Yes, it can be awfully practical, but I feel like it goes against the separation of concerns between Codable type and encoder/decoder. The whole point of Codable is that a type can define the format/schema that it is serialized/deserialized to/from without knowing anything about the metaformat (e.g. JSON) it is serialized/deserialized to/from, and the encoder/decoder can serialize/deserialize types to/from a metaformat (e.g. JSON) without knowing anything about those types. When the encoder/decoder starts changing keys, that’s no longer true. Now the type’s format/schema is partly determined by the encoder/decoder, and so wherever you’re going to serialize/deserialize that type you need to make sure to configure the encoder/decoder correctly.

Changing keys in the encoder/decoder also works against one of the best things with Codable: synthesis. When you’re using synthesized Codable, a type’s serialization format/schema is determined at compile time by a simple code transformation. But if the encoder starts messing with the keys, the format/schema is now partly determined at runtime instead, and by an external library nonetheless. That makes the serialization code harder to reason about. You can no longer just look at a type and "see" how it will be encoded. Personally, I’ve always avoided .convertToSnakeCase for this reason. Writing out coding keys manually is a small price to pay for predictable serialization code.

So in general, I think the less the standard library knows about key conversion, the better. If specific encoders/decoders want to offer key conversion strategies, it’s really up to them to make that conversion predictable and configurable enough.

[Jon_Shier]

@hisekaldma

That makes the serialization code harder to reason about. You can no longer just look at a type and "see" how it will be encoded.

You can't make that guarantee anyway. Coders can do anything they want, including removing keys altogether.

@hisekaldma

So in general, I think the less the standard library knows about key conversion, the better. If specific encoders/decoders want to offer key conversion strategies, it’s really up to them to make that conversion predictable and configurable enough.

Given that these transforms are necessary and common, and given the standard library defines the core Codable types, it seems like there should be one representation of this functionality rather than n representations. Functionality should also be common between coders so that one coder's snake case doesn't differ from the others. If you find this philosophically objectionable, feel free not to use it as you already do, but that should prevent others from reaping the benefits of such a move.

[itaiferber]

@hisekaldma

I feel like it goes against the separation of concerns between Codable type and encoder/decoder. The whole point of Codable is that a type can define the format/schema that it is serialized/deserialized to/from without knowing anything about the metaformat (e.g. JSON) it is serialized/deserialized to/from, and the encoder/decoder can serialize/deserialize types to/from a metaformat (e.g. JSON) without knowing anything about those types. When the encoder/decoder starts changing keys, that’s no longer true.

Codable, by design, tries to strike a balance between three potentially-different parties:

1. The model type, which chooses its preferred keys, values, and container type,
2. The encoder/decoder translating from the model type to a specific format, and
3. The top-level entity responsible for encoding/decoding a schema in the first place

In different cases, different parties may have more information about what might be appropriate to do in a certain circumstance:

• The model type (1) may have a preferred container format that conflicts with what is possible in an encoding format; in this case, the encoder/decoder (2) may detect this and convert to and from what the encoding format actually supports in order to keep this detail invisible to (1)
  • For example, some encoding formats don't support the equivalent of keyed containers. An encoder for this format can totally legally participate in the Codable APIs if it knows how to translate keyed containers into a form that the format actually supports, and its decoder equivalent knows how to convert that form back
• The top-level entity (3) may have knowledge about their use-case that (2) and (1) don't, so the encoder may offer knobs to tweak behavior to allow this to be expressed
  • For instance, many servers have specific data format requirements for accepting data that neither (1) nor (2) could possibly know about, nor could (3) reasonably inform them of these requirements (for example: key capitalization, spelling, casing, etc.). (3) has no choice but to override certain decisions that (1) and (2) make about encoding to get the data into the form they need

Customization is important and afforded to (2) and (3) to help them be as useful as possible, without requiring mucking around with the encoded data after-the-fact.

However, it is true that in many (if not most) cases, party (1) is also the same as party (3), constructing both the model type and encoding the top-level data for use. When this is the case, it's hard, if not impossible, to prevent them from reaching for knobs to tweak data at both layers depending on what is most convenient, instead of what might be the most "correct". From a certain point of view, you could say that they should really be modifying keys at layer (1) to match what they need, because the model types should decide how to encode; from another, you could say that the key types at layer (1) should absolutely be agnostic of the end result of encoding, and it's really at layer (3) where they should be overriding keys in order to keep (1) entirely independent from the format.

In either case, what we have right now is something that I feel is the worst of both worlds: developers try to tweak the knob as party (3), but it doesn't work in all cases for them, so they have to also start messing with CodingKey values as party (1). It would be great for consistency, in my opinion, to offer a solution that will work in pretty much all cases at either layer, so this mixed customization can go away.

[Morten_Bek_Ditlevsen]

Hi Itai,
I'm back at the keyboard after a long (and most excellent) summer break.

@itaiferber

I haven't had the opportunity to think this through entirely, but a thought: back in the mists of time, @Tony_Parker and I discussed the possibility of having JSON{En,De}coder build in a list of known abbreviations which we could correctly identify as terms which would be correctly cased ("HTML", "URL", etc.) with the default .convert{To,From}SnakeCase strategies, but didn't get to explore this idea at the time. I wasn't a fan of having Foundation hard-code a list of known values like this, and I still don't, but luckily it could be easy to extend to users:

This could definitely be a fix for a large percentage of the issues that people are likely to run in to today - at least when using English abbreviations. But I do think that it feels a bit like a workaround, and when you do need to extend this list, it might be unclear why this is needed.

There's also an aspect that this perhaps does not help with - namely the case of decoding in a dynamic fashion using the allKeys api together with a CodingKey that returns a key for any String:

For a String based enum CodingKey like:

enum CodingKeys: String, CodingKey {
  case imageURL
}

and the encoded string version of the key: image_url, the enum initializer would return nil for the default 'from snake case' conversion: 'imageUrl', and in this case your suggested strategy could try replacing 'Url' with 'URL' and try initializing the coding key with 'imageURL', which would succeed.

But if we consider the dynamic case, where the coding key is something like:

struct MyCodingKey: CodingKey {
    let stringValue: String
    let intValue: Int?
    
    init(stringValue: String) {
        self.stringValue = stringValue
        self.intValue = Int(stringValue)
    }
    
    init(intValue: Int) {
        self.stringValue = "\(intValue)"
        self.intValue = intValue
    }
}

This coding key has a non-failing initializer, so the algorithm applying the list of abbreviations would never know if it were 'correct' to use a key based on the string 'imageUrl' or 'imageURL'...

These considerations reinforce the point of view that there is a huge difference between the kind of enum based coding keys that you get automatically synthesized - and the dynamic case where the coding keys can hold any value and allKeys is used to inspect the payload.

With this consideration in mind, I think that it would be a noble goal to fix the autosynthesized Codable conformance 100% with no workarounds or extra knowledge needed. This fix will of course also work for all manual Codable conformances that do not use the allKeys api.

My personal guess is that such a solution will solve a much larger percentage of issues than the extensible list of abbreviations will solve on it's own. And it needs less 'magic sauce' in my point of view.

Remaining is the 'dynamic' decoding with flexible coding keys and using the allKeys api.
Regarding that we can state:

1. The combination of dynamic keys and JSONDecoder key decoding strategies is already somewhat broken (please see [Pre-pitch] Roundtripping key coding strategies - #13 by Morten_Bek_Ditlevsen)
2. Adding a list of known abbreviations will not fix dynamic key decoding since we cannot know if these dynamic keys follow the 'rules' defined by this list.
3. A solution that always returns an empty array for the allKeys api would make the situation for dynamic key decoding worse.
4. The only way to completely fix the dynamic key coding is to not transform the keys - either by not using any key coding strategies OR by adding a protocol to the standard library that suggests to encoders and decoders that they should not encode or decode the keys even though they might have a key coding strategy set.

Combining these things I propose another solution that can be implemented entirely in JSONEncoder and JSONDecoder in Foundation, namely the solution previously suggested at: [Pre-pitch] Roundtripping key coding strategies - #13 by Morten_Bek_Ditlevsen

To recap: a .useSnakeCase key encoding strategy on JSONEncoder and a similarly named key decoding strategy on JSONEncoder. These strategies transform keys in the direction of the CodingKey to String representation - both when encoding and decoding - and thus there is no loss due to having two separate transforms.

In order to support the allKeys API, the solution would, however, fall back to the existing 'fromSnakeCase' transformation - thus doing the exact same thing as the .fromSnakeCase key decoding strategy does for the allKeys api today.

This solution would work for all synthesized CodingKey conformance as well as all conformances that do not use allKeys - and for Decodable conformances that use the allKeys api, the situation is status quo compared to the current .fromSnakeCase key decoding strategy.

It could be a future direction to try and fix the dynamic decoding situation using a 'leave my coding keys alone' marker protocol.

What do you think? Do you agree that the suggested list of abbreviations will not fix dynamic parsing, or might I be overlooking something?

I'd love to hear any thoughts that any readers of this topic may have.
Also from any members of the Foundation team. :-)
CC: @tomerd @Tony_Parker

[Morten_Bek_Ditlevsen]

The solution presented above would have the curious (but unfortunately useless) property that if it was implemented as a replacement for the existing .convertFromSnakeCase keyDecodingStrategy, then it would be fully backwards compatible. In the sense that existing code would behave exactly the same with the new implementation as the old.

But this property is useless since new code (where you have keys like imageURL) would only behave as expected with the new implementation and not the old.

I just thought it fun that 'backwards compatible' is not always 'enough'.