DefaultJsonProtocol.viaSeq looses ordering for ordered iterables

[swsnr]

DefaultJsonProtocol.viaSeq doesn't preserve ordering when using with some ordered collections, e.g. TreeSet; the following Scalacheck property consistently fails [1]:

"DefaultJsonProtocol.viaSeq preserves ordering" in prop { s: TreeSet[Long] =>
  DefaultJsonProtocol.viaSeq[TreeSet[Long], Long](TreeSet(_: _*)).write(s) must_== s.toList.toJson
}

The root cause is that viaSeq directly maps over the given input iterable which moves from TreeSet to Set, because mapping a TreeSet without an ordering isn't possible.

I'm not sure whether this behaviour is intended, but it's at least surprising.

---

[1]: Roughly translated from the following scalatest property in the test suite of the project I'm working on; I've never used specs2 and am not at all familiar with its syntax.

  property("DefaultJsonProtocol.viaSeq preserves ordering") {
    // Spray JSON viaSeq does not maintain ordering
    forAll { s: TreeSet[Long] =>
      assert(DefaultJsonProtocol.viaSeq[TreeSet[Long], Long](TreeSet(_: _*)).write(s) === s.toList.toJson)
    }
  }

[jrudolph]

Thanks, that's an interesting observation. The minimum property should be that spray-json should make sure that a round-trip from an ordered collection to json and back should lead to the same result. That should be valid at least.

But I see how you would expect that the entries are also sorted on the json side of things.

The root cause is that toSeq directly maps over the given input iterable which moves from TreeSet to Set, because mapping a TreeSet without an ordering isn't possible.

Yep, indeed. I filed https://github.com/scala/bug/issues/11987 but don't expect it can be fixed.

[jrudolph]

Using JsArray(iterable.iterator.map(_.toJson).toVector) would fix it.

[swsnr]

@jrudolph That's more or less what we're using now.

[swsnr]

I think in this context it's also worth pointing out that spray-json should preserve the ordering in the source domain, that is, before converting elements of the collection to JSON, because the ordering of JSON values might be quite different from the ordering of the collection (think off, e.g. a TreeSet with an inverse ordering).

[jrudolph]

I think in this context it's also worth pointing out that spray-json should preserve the ordering in the source domain, that is, before converting elements of the collection to JSON, because the ordering of JSON values might be quite different from the ordering of the collection (think off, e.g. a TreeSet with an inverse ordering).

Not sure what you mean. What would be the alternative?

[swsnr]

@jrudolph Well, when serializing a TreeSet you shouldn't order by the JSON value of its elements, but by the elements itself :)

[swsnr]

@jrudolph I'd just like to check what's the way forward here… do you expect me to open a pull request to fix this issue?

[jrudolph]

Yes, please. In that case, I'd say adding .iterator would probably be safest.

[swsnr]

@jrudolph Will do.

[swsnr]

@jrudolph I opened #330.