Should Reads compose and andThen repath?

(Jason Pickens) #1

I had raised play-json/issues/171 when I discovered some rather surprising error messages when composing Reads using andThen.

The thing to discuss here is whether or not andThen and compose should use repath, effectively concatenating the paths of both Reads.

One example of where it doesn’t make sense to do so is if we start with:

  "a": 1,
  "b": 2,
  "c": 3

Now we read it with:

(__ \ 'a).json.prune andThen (__ \ 'b).json.prune andThen (__ \ 'd).json.pick

Of course this will fail since /d does not exist but what you get is:


The path in the error is messed up. What happened is that the intermediate success of the two prunes actually concatenated the paths of the very things they removed.

The only time when it would make sense to repath is when the first Reads has traversed down the tree and not retained anything that it traversed such as a pick or possibly a copyFrom.

I have since replaced all uses of andThen and compose with custom versions that keep the path unchanged.

(Claudio Bley) #2


In the same vain, I just discovered that flatMap on JsSuccess also uses repath which messes up the resulting path too:

for {
  a ←  (__ \ 'a).read[Int].reads(abc)
  b ← (__ \ 'b).read[Int].reads(abc)
} yield a + b

play.api.libs.json.JsResult[Int] = JsSuccess(3,/a/b)

It even gets worse if you do deeper lookups, as the paths get munged together. Which is really unhelpful in case of a JsError.

IMO, it makes no sense to repath within a flatMap either, since the value of the inner JsResult does not necessarily depend on the value of the first result. It would only ever make sense if the result is a JsValue itself, so that a lookup can further drill down into the structure:

val json = Json.parse("""{"item": {"name": "x"} }""")

for {
  item ← (__ \ 'item).read[JsObject].reads(json)
  name ← (__ \ 'name).read[String].reads(item)
} yield name

play.api.libs.json.JsResult[String] = JsSuccess(x,/item/name)

I think a repath should never happen automatically, the user should be able to manually repath when reading values from a “subdocument”.