Open janjaali opened 4 years ago
Anybody some spare time to have a look at this one?
Thanks for the suggestion. I agree it would be better if we could improve type-safety for maps but at this point we cannot make an incompatible change like this. If we want to do it, it needs to be in a way that allows us to keep the existing signature (and maybe deprecate that one or remove the implicit
status).
So, if I get you right, you are suggesting to drop the implicit status from the existing signature (did so in the PR) and remain with the new signature?
As mentioned in the description, the change of the "implicit signature" probably won't affect users since the only currently supported JsonFormat for keys is JsonString, otherwise the serialization just fails. This implicit constraint is caught by the implicit StringKeyableFormat
that was added to the DefaultJsonProtocol
.
Maybe it's not a PR for this release train but for the next :)
This PR aims both to resolve an unclear behavior of the
mapFormat
method and improve the extensibility of this method. ThemapFormat
method provides aRootJsonFormat
to serializescala.Predef#Map
s asJsObject
s and vice versa.Uncertain behavior
The signature of the
mapFormat
method incorrectly implies that all types of keys are applicable to serialize aMap
as aJsObject
as long as these keys are serializable asJsValue
. Thus trying to serialize a Map with a key which is not serialized asJsString
results in aSerializationException
.This PR will modify the method signature such that an implicit
KeyableFormat
instead of aJsonFormat
is required. AKeyableFormat
implies that a key must be serializable as aString
- the only valid key format in JSON - and it can be deserialized back from aString
. This change will improve the type-safety of the method and prevent surprisingSerializationException
s while serializing aMap
without a valid key format.Extensibility
The newly introduced
KeyableFormat
trait is extendable and enables user to provide their own format to serialize their keys. A simple and probably most generousKeyableFormat
forString
is provided in theKeyableFormats
object.Is this is a breaking change?
I would argue partially: The signature of an implicit method was changed by replacing the implicit
JsonFormat
argument for the key to an implicitKeyableFormat
and theDefaultJsonProtocol
provides the defaultStringKeyableFormat
. Thus most users may not be affected by this change. Nevertheless it is a change in a public API and should be considered as breaking change.