JSON field ordering bug with Scala 3

levinson commented 5 months ago

Play JSON Version (2.5.x / etc)

Affected versions:

"com.typesafe.play" %% "play-json" % "2.10.5"
"org.playframework" %% "play-json" % "3.0.3"

API (Scala / Java / Neither / Both)

Scala

Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)

MacOS 14.3.1

JDK (Oracle 1.8.0_72, OpenJDK 1.8.x, Azul Zing)

java version "17.0.10" 2024-01-16 LTS Java(TM) SE Runtime Environment (build 17.0.10+11-LTS-240) Java HotSpot(TM) 64-Bit Server VM (build 17.0.10+11-LTS-240, mixed mode, sharing)

Expected Behavior

Expected JSON ordering to be preserved when serializing model class.

Actual Behavior

JSON field ordering is not preserved when compiling using Scala 3

Reproducible Test Case

I created the following project to demonstrate the issue: https://github.com/levinson/play-json-scala3-ordering-bug

Posting the source code here as well for reference:

build.sbt:

scalaVersion := "2.13.14"
// TODO: Test fails when using Scala 3
//scalaVersion := "3.3.3"

libraryDependencies ++= Seq(
  "com.typesafe.play" %% "play-json" % "2.10.5",
  //"org.playframework" %% "play-json" % "3.0.3", // same behavior
  "org.scalatest" %% "scalatest" % "3.2.18" % Test
)

JsonSerializersTest.scala

import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec
import play.api.libs.json._

case class Model(metadata: Option[String], region: String)

object JsonSerializers {
  implicit val modelFmt: Format[Model] = Json.format[Model]
}

class JsonSerializersTest extends AnyWordSpec with Matchers {

  import JsonSerializers._

  "JsonSerializers" should {
    "preserve ordering" in {
      val model = Model(Some("metadata"), "region")
      val serialized = Json.stringify(Json.toJson(model))
      println("serialized is " + serialized)
      serialized shouldBe """{"metadata":"metadata","region":"region"}"""
    }
  }
}

jonaskoelker-jypo commented 4 months ago

My semi-systematic experimentation suggests that the observed field order will be:

all non-Option fields, in declaration order (relative to one another); then
all Option fields, in declaration order (relative to one another)

By declaration order, I mean the same order as productElementNames.

So if either all fields are Options or zero fields are Options, the existing code will do the right thing.

Here's a blueprint for a workaround:

def workaround[T <: Product](w: OWrites[T]): OWrites[T] = {
  OWrites.transform(w) { case (product, misorderedObject) =>
    val pairs = misorderedObject.value
    JsObject(
      product.productElementNames
        .map(key => pairs.get(key).map(key -> _))
        .collect { case Some(pair) => pair }
        .toSeq
    )
  }
}

implicit val writes: OWrites[Foo] = workaround(Json.writes[Foo])

This can be simplified—you can use apply instead of get and then remove the collect—if all fields are always present in the output, e.g. if you use JsonConfiguration(optionHandlers = OptionHandlers.WritesNull) in a relatively simple context.

It is of note that the bug seems to still occur if all fields are present in the output—so it must depend on the optionality in the declaration, not in the output.

In version 3.0.3 I did some jump-to-source maneuvers in my application:

OptionHandlers.WritesNull.writeHandler refers to
JsPath.writeOptionWithNull which refers to
Writes.at, which is really PathWrites.at, which refers to
JsPath.createObj, at https://github.com/playframework/play-json/blob/3.0.3/play-json/shared/src/main/scala/play/api/libs/json/JsPath.scala#L133, which has a note saying "// TODO implement it correctly (doesn't merge )"

That sounds suspicious, but it's no smoking gun. I dug around some more, and found this:

A list of fields is separated by whether they are optional or not: https://github.com/playframework/play-json/blob/51bd5d6117e4103fc2c4a4bffd2dabb24da58d8f/play-json/shared/src/main/scala-3/play/api/libs/json/JsMacroImpl.scala#L670
Data derived from this partition are concatenated here: https://github.com/playframework/play-json/blob/51bd5d6117e4103fc2c4a4bffd2dabb24da58d8f/play-json/shared/src/main/scala-3/play/api/libs/json/JsMacroImpl.scala#L769

This looks like the cause of exactly the pattern I have observed, i.e. a smoking gun.

Across my work I think I have only seen the bug in those projects that use Scala 3. This makes sense, the apparent smoking gun is in a scala3-specific folder, so one would expect the bug to occur if and only if using Scala 3. This matches what @levinson wrote in the issue-creating post: "TODO: Test fails when using Scala 3".

I hope this helps.

gbarmashiahflir commented 2 months ago

Happens here as well. Having for example this class:

object MetricStatistics{
  implicit val format: Format[MetricStatistics] = Json.format[MetricStatistics]
}

case class MetricStatistics(from: Instant, to: Instant, min: Option[Double], max: Option[Double], average: Option[Double], count: Long, centroid: Option[GeoLocation])

In scala 2.13, generated json was according to field order, but after upgrading to Scala 3, this is the order of an example:

    {
        "count": 6,
        "min": 10.3,
        "to": "2010-06-24T09:55:00Z",
        "max": 100,
        "from": "1980-06-24T05:55:00Z",
        "centroid": {
            "lat": 0,
            "lon": 0
        },
        "average": 38.926
    }

It breaks a lot of tests we have :( Is there a fix in the plan?

playframework / play-json