Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JsPath.prune should not repath on success #171

Open
steinybot opened this issue Jul 31, 2018 · 4 comments
Open

JsPath.prune should not repath on success #171

steinybot opened this issue Jul 31, 2018 · 4 comments

Comments

@steinybot
Copy link

steinybot commented Jul 31, 2018

Play JSON Version (2.5.x / etc)

2.6.9

API (Scala / Java / Neither / Both)

Scala

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

macOS 10.13.6

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

java version "1.8.0_172"
Java(TM) SE Runtime Environment (build 1.8.0_172-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.172-b11, mixed mode)

Library Dependencies

N/A

Expected Behavior

scala> :paste
// Entering paste mode (ctrl-D to finish)

import play.api.libs.json._

val js = Json.parse("""{
                       |  "a": 1,
                       |  "b": 2,
                       |  "c": 3
                       |}""".stripMargin)
val removeAAndB = (__ \ 'a).json.prune andThen (__ \ 'b).json.prune
val removeAAndBPickD = removeAAndB andThen (__ \ 'd).json.pick

// Exiting paste mode, now interpreting.

import play.api.libs.json._
js: play.api.libs.json.JsValue = {"a":1,"b":2,"c":3}
removeAAndB: play.api.libs.json.Reads[play.api.libs.json.JsObject] = play.api.libs.json.Reads$$anon$8@29d080fc
removeAAndBPickD: play.api.libs.json.Reads[play.api.libs.json.JsValue] = play.api.libs.json.Reads$$anon$8@1ed855bd

scala> removeAAndB.reads(js)
res0: play.api.libs.json.JsResult[play.api.libs.json.JsObject] = JsSuccess({"c":3},/a/b)

scala> removeAAndBPickD.reads(js)
res1: play.api.libs.json.JsResult[play.api.libs.json.JsValue] = JsError(List((/a/b/d,List(JsonValidationError(List(error.path.missing),WrappedArray())))))

Should evaluate to:

JsError(List((/d,List(JsonValidationError(List(error.path.missing),WrappedArray())))))

Actual Behavior

It evaluates to:

JsError(List((/a/b/d,List(JsonValidationError(List(error.path.missing),WrappedArray())))))

The issue is that prune is effectively repathing the result and so it does not compose properly with subsequent Reads. It makes sense to use the path in the error cases but not success.

Reproducible Test Case

See above.

@steinybot
Copy link
Author

steinybot commented Jul 31, 2018

A similar combination can be achieved with update and copyFrom although the solution is less obvious:

scala> :paste
// Entering paste mode (ctrl-D to finish)

val duplicateAAndB = __.json.update((__ \ 'aa).json.copyFrom((__ \ 'a).json.pick)) andThen __.json.update((__ \ 'bb).json.copyFrom((__ \ 'b).json.pick))
val duplicateAAndBPickD = duplicateAAndB andThen (__ \ 'd).json.pick

// Exiting paste mode, now interpreting.

duplicateAAndB: play.api.libs.json.Reads[play.api.libs.json.JsObject] = play.api.libs.json.Reads$$anon$8@3f28e6d4
duplicateAAndBPickD: play.api.libs.json.Reads[play.api.libs.json.JsValue] = play.api.libs.json.Reads$$anon$8@5fa6ae08

scala> duplicateAAndB.reads(js)
res3: play.api.libs.json.JsResult[play.api.libs.json.JsObject] = JsSuccess({"a":1,"b":2,"c":3,"aa":1,"bb":2},/a/b)

scala> duplicateAAndBPickD.reads(js)
res4: play.api.libs.json.JsResult[play.api.libs.json.JsValue] = JsError(List((/a/b/d,List(JsonValidationError(List(error.path.missing),WrappedArray())))))

@steinybot
Copy link
Author

On second thought I think that the real issue is that andThen should not be concatenating the paths. The only time that concatenation is correct is if the first Reads has picked a value from a path and not combined that with anything else (e.g. only trivial pick or copyFrom). Assuming that this works in other cases seems like the greater of two evils.

@cchantep
Copy link
Member

cchantep commented Aug 4, 2018

Better to first discuss it, until then this one can hardly be actionable. Best regards.

@steinybot
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants