fix(model): Don't deserialize option values as Id's if option type is String

[suneettipirneni]

Because of the way ValueEnvelope is declared it automatically puts a higher precedence on deserializing to an Id rather than a String.

This behavior is desired because currently, the deserialization of option values doesn't take the kind of the option JSON into account. So if an option actually is a snowflake type it would be serialized as such instead of becoming a String. This, however, does not account for situations where the string is a parseable Id, but is supposed to be represented as a String regardless.

As a result, this PR removes ValueEnvelope::Id. This means that means we initially deserialize any non-boolean or non-numeric values as Strings. Later on, when checking against the option kind we disambiguate the previously deserialized string and parse into the appropriate Id corresponding to the kind.

Closes #2178

[Erk-]

Can you add a test for this case so it does not happen again in the future?

[vilgotf]

@suneettipirneni Sorry, it seems that my suggestion did not work after all. Testing locally with the latest commit resulted in the following error: invalid type: string "<SNOWFLAKE>", expected channel id. Intresting that serde-test does not result in the same code path as real world usage.

Otherwise this looks good.

[suneettipirneni]

@suneettipirneni Sorry, it seems that my suggestion did not work after all. Testing locally with the latest commit resulted in the following error: invalid type: string "<SNOWFLAKE>", expected channel id. Intresting that serde-test does not result in the same code path as real world usage.

No worries, I've reverted the commit.

[MaxOhn]

Now that things might get deserialized into an allocated String without having the need to, it might be a good time to consider serde_json::RawValue.
I'm unsure myself if it's sufficiently beneficial but I figured I throw it into the room at least.

The variant ValueEnvelope::String could hold a &'v RawValue or Box<RawValue> and then depending on what's needed one would parse the value accordingly at a later point.

This might be a topic for a subsequent new issue.

Edit: Scratch that, simd-json doesn't do well with RawValue 😅

[itohatweb]

Refer to #2190 (comment)

[itohatweb]

LGTM, just a small nit-pick if you don't mind.