Preserving order of ctor args

[ahjohannessen]

Hi,

When I upgraded from spray-json 1.2.6 to 1.3.0 I noticed a change with regards to generated json. The below test did not fail earlier.

Is ordering of fields in generated json for a case class not consistent with its constructor args?

...
case class Bibby(
  id: Int, firstName: String, lastName: String, cid: UUID, time: ZonedDateTime
)
...

"Ordering" should "be as constructor args" in {

  val body =
    HttpEntity(ContentType(`application/json`, `UTF-8`),
    """{
      |  "id": 1,
      |  "firstName": "Jack",
      |  "lastName": "Black",
      |  "cid": "4adbc012-617b-487b-ab31-ac8ba7892714",
      |  "time": "2014-05-23T10:30:00Z"
      |}""".
      stripMarginWithNewline("\n")
    )

  val bibby = Bibby(
    id = 1,
    firstName = "Jack",
    lastName = "Black",
    cid = UUID.fromString("4adbc012-617b-487b-ab31-ac8ba7892714"),
    time = ZonedDateTime.parse("2014-05-23T10:30:00Z")
  )

  marshal(bibby) should equal(Right(body))
}

This results in a failing test:

[info] - should be as constructor args *** FAILED ***
[info]   Right(HttpEntity(application/json; charset=UTF-8,{
[info]     "lastName": "Black",
[info]     "firstName": "Jack",
[info]     "id": 1,
[info]     "cid": "4adbc012-617b-487b-ab31-ac8ba7892714",
[info]     "time": "2014-05-23T10:30:00Z"
[info]   })) did not equal Right(HttpEntity(application/json; charset=UTF-8,{
[info]     "id": 1,
[info]     "firstName": "Jack",
[info]     "lastName": "Black",
[info]     "cid": "4adbc012-617b-487b-ab31-ac8ba7892714",
[info]     "time": "2014-05-23T10:30:00Z"
[info]   })) ...

Just to note, there is nothing wrong with the (un)marshalling and the following passes:

marshal(bibby).fold(
  t ⇒ fail(t),
  e ⇒ e.as[Bibby] should equal(Right(bibby))
)

[sirthias]

This is a perf-related change introduced with 1.3.0.
Before we were using a ListMap to keep the members of a JSON object, which maintains the original order of addition to the map.
However, a ListMap has terrible perf characteristics, which is why we changed to a HashMap with 1.3.0. An unfortunate side effect is that the member order is now not stable anymore, which is not a problem per se (as JSON is inherently order agnostic with regard to object members) but does complicate tests.

[jrudolph]

It would be nice if that could be made configurable. It is already possible by manually providing a ListMap to the JsObject constructor.

I think we could quite easily do it by providing an overrideable method in ProductFormat for the case class -> JsObject conversion. We could go the same way for JsonParser or just make it a configuration setting there. Then, we need to decide about the defaults... :)

[ahjohannessen]

Ok, it is not that big of a deal for me. Explicitly defining the json in my tests is mostly just for "documentation" for our angularjs front-end, but now it is a bit weird because of ordering :)

However, it would be nice if one could turn on some switch in tests to get stable order, perhaps switching to LinkedHashMap which maintains insert order, I believe. It might be as simple as having a setting in reference.conf that one could override in test projects.

[ahjohannessen]

Either way, you are welcome to close the issue, just wanted to know why my tests suddenly failed :)

[maspwr]

I just hit this issue with parseJson. It's not a huge problem (I'll work around it), but I think a configuration option would be nice even with the performance caveat. Having the structure of the JSON change just due to parsing it is slightly surprising.

[jeffrey-aguilera]

print composed with parse should be the identity function. Changing to a ListMap breaks this most fundamental expectation.

[betandr]

Initially I was going to ask for a configurable 'ordered' or 'not ordered' JSON renderer too but the more I think about it, the happier I am. JSON's spec states it's unordered and maybe by establishing the expectation of ordered JSON in the past we've kind-of enabled bad practice such as using order of appearance to be a sort order. In the API I'm building at the moment I have a 'sort' field which provides the order in which objects should be handled, which means that the order they are in the JSON is immaterial. We also have a schema linked to the feed which we can use to describe the content of the doc.

[sirthias]

I agree with @betandr here. JSON members are unordered, so if your code relies on any ordering you should establish it yourself rather than requiring your dependencies to adhere to any particular order.

[tohenryliu]

I do a agree with the points made here. Json fields ordering is not in the spec.
Though a config option possibly in those JsonFormats wouldn't hurt. For applications where performance isn't critical, order preserving can be had with a trade-off.
Also considering the code needed to expose that is not a lot.
Or is it still too much a distraction from the library's principal?

[sirthias]

@tohenryliu Can't you satisfy your ordering needs on your layer? Similarly to what @betandr described?

[tohenryliu]

I think my request is a bit unreasonable. We do not need ordering for any machine consumers.
It's only useful when I exam the json manually, possibly try to match with the case class values for dev/test purposes.
also when diff between different results.

I don't feel so strongly, but the team has developed the habit to rely on that.

would you accept a Pull Request if I attempt to wrote something for that?

[sirthias]

@tohenryliu If you'd like to establish a specific kind of structure for your raw JSON (e.g. for your tests) you could always add a separate reformatting stage that does exactly what you want.
For example, some people might prefer the "original" member order as in the respective domain model (case) class and others an alphabetical order.

[betandr]

Although maybe we should question tests that rely on spring-json anyway. Maybe it'd be best to mock objects or have static JSON fixtures rather than testing that spray-json returns the correct value. Testing your own code which uses spray-json may well be the wrong thing.

The really good thing about stuff like this is that it often exposes bad practices and makes us think about how we're approaching things. :)

[maspwr]

Might as well throw my updated opinion into the mix. Thinking of this more, I'm quite happy with spray-json doing what it does well, and keeping a small API. I think the most I'd request at this point is an updated docs section mentioning this behavior and possibly even an example how one could enforce ordering on their own.

[fommil]

There is no ordering guarantee in the JSON format (or indeed in the containing JavaScript).

I've often thought about writing a custom serialiser for sorted maps and the like, but ultimately preservation of ordering in Spray JSON would be futile because as soon as it goes through any other service it could change. Assuming ordering in maps would mean that you're restricting your use of JSON to "only things that produce and consume it with the ordering caveat".

IMHO, if one requires order in JSON then the only way to enforce it is to use an array representation.

[magicgoose]

I'd +1 this, the option to keep declared order is a must for a library like this.

preservation of ordering in Spray JSON would be futile because as soon as it goes through any other service it could change

That's a totally different problem and it's the user's problem. There are use cases for preserving declaration order. Why not have it? I'm sure it's not too hard :)

[betandr]

Quote @magicgoose

That's a totally different problem and it's the user's problem.

If JSON is unmarshalled to objects and and marshalled again into JSON then it could very well be in a different order and while it's one argument to say that it's the user's problem, it's really a non-deterministic problem and relying on order of appearance is a Bad Idea(tm).

There are use cases for preserving declaration order. Why not have it?

The JSON spec explicitly says:

"An object is an unordered set of name/value pairs"

If you do have a collection of objects, putting them in an array will preserve the order, however.

I'm sure it's not too hard :)

Famous last words! ;) Although the reason the order is not preserved, as @sirthias has previously explained, is because the implementation being used to handle JSON would be much slower if it had to do this. We really should be doing everything we can to dissuade people from a really bad practice of relying on insertion order...there's always an alternative!