vlix / safe-json Goto Github PK

We've come across a situation where we receive JSON from the outside (which doesn't adhere to SafeJSON's versioning) and we want to force the parsing of a specific version.

Might be nice to have a function like:

setVersion :: SafeJSON a => Version a -> Value -> Value
setVersion (Version (Just v)) (Object o) = Object $ HM.insert versionField v o
setVersion (Version (Just v)) val = object [ dataVersionField .= v, dataField .= val]

(and catch the case if the version of a is Nothing as well, obv)

[EDIT] And maybe a TypeApplications version?

setVersion_ :: forall a. SafeJSON a => Value -> Value
setVersion_ val = case mVersion of
  Nothing -> val
  Just v -> case val of
    Object o -> Object $ HM.insert versionField v o
    _ -> object [ dataVersionField .= v, dataField .= val]
  where mVersion = Version v :: Version a

    [ 2 of 12] Compiling Instances [Test.QuickCheck.Instances changed]

    /var/stackage/work/unpack-dir/unpacked/safe-json-1.1.0-bed3444b0fc22b82b5c91ce51ee6c0c5bfd1787b701f4463511e75e5693602ea/test/Instances.hs:52:10: error:
        Duplicate instance declarations:
          instance Ord Value -- Defined at test/Instances.hs:52:10
          instance Ord Value
            -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.Internal’
       |
    52 | instance Ord Value where
       |          ^^^^^^^^^

Will add the tests to expected failures in stackage for now, that can be reverted once this has been fixed (a PR would be appreciated!)

These two packages have had version increases which might still work with safe-json

This might help in switching from aeson over to safe-json and make functions like removeVersion clearer in that they remove the "safeness".

This only concerns the default implementation of safeTo and safeFrom. When starting off with SafeJSON, it might be easier to just put the implementations in safeTo and safeFrom, instead of seperate From-/ToJSON instances. This also reduces the need to still import aeson, since you don't have to use parseJSON and toJSON in (probably) the same modules you're defining the SafeJSON instance.

Would a getVersion :: Aeson.Value -> Maybe (Version a) be a good addition?

The SafeJSON a => constraint in the Product instance might be superfluous.

I remember removing it from Sum, because it wasn't necessary, but I guess I might have forgotten to take it out of Product as well.

The export should either not be there, or documented properly.

In Data.SafeJSON.Test, the type signatures that have MigrateFrom a ~ b can have the other places where MigrateFrom a is used, replaced with just b, this will make it a bit less verbose.

Since update to 1.0.0 this is where it can be adjusted.

The test functions like migrateRoundTripProp don't need a second type application.
Adjust examples, but note that adding the second type gives guarantee it's migrating from the right type.

Also add tests for sum types that defer versioning to the things it can be, i.e. sum types that just contain types with SafeJSON instances:

data WhichMessage = Status StatusMessage
                  | Menu MenuEvent

Where the implementation is just:

instance SafeJSON WhichType where
  version = noVersion
  safeFrom val = contained $ Status <$> safeFromJSON val
                         <|> Menu <$> safeFromJSON val
  safeTo = contained . \case
    Status upd -> safeToJSON upd
    Menu evt -> safeToJSON evt

The caveat here, is that somewhere in the chain, there could potentially be the same numbered version of one type, that might get parsed the same as another type with the same numerical version.

Would make the defining of safeFrom more consistent. Like you could define FromJSON parsers using <|>

Maybe it's nice to also have a strippedSafeToJSON that's defined as removeVersion . safeToJSON?
And maybe add like strippedEncode defined as encode . strippedSafeToJSON?

Add ./test/json/ directory with next sdist version to have tests run on stackage.

Add a not in the second line of noVersion's description ([...] that are NOT tagged [...])

Seeing THIS, there are types that don't have links to their definition. (Word32, Typeable, etc.) Somehow fix that?

Since safe-json-1.0.0, the SafeJSON class doesn't require FromJSON or ToJSON instances (but still defaults to those if not defined) so a review of the documentation might be in order. (also to add usage/explanation of the new parsing functions, e.g. .:$ etc.)

testRoundTrip has the description:

Tests that the following holds:

Just a == parseMaybe (safeFromJSON . safeToJSON) a

Which should be parseMaybe safeFromJSON (safeToJSON a)

Have Data.Aeson.Safe also re-export Parser from Data.Aeson.Types

I could open a PR for this, but I figured a discussion might be better first.

Right now there's no way to use aeson's faster toEncoding JSON serialization methods (if they're defined) with safe-json.

As far as I can tell, it should be possible to add support for this with a new method on SafeJSON where a -> Contained Encoding. It could be optional (like aeson), with the default implementation calling safeTo encoding the resulting Value.

It gets a bit tricky with something like .=$, though. I imagine we would need to define a SafeKeyValue class so that .=$ could use safeToJSON on Pair and Object types, but safeToEncoding (or whatever it might be called) on Series types.

This should hopefully add all the instances to the Haddock docs where the class is described. It's annoying to not be able to see at a glance which types already have a SafeJSON instance.

setVersion should have a @since 1.0.0 added to it's description.