Refactor JSON format for attribute errors in Elastic4Play#490
Open
cchantep wants to merge 5 commits into
Open
Conversation
… format (without implementation change)
Introduce common function `attributeErrorWrites`:
- Enforce format consistency between the different error kinds (for the `type` discriminator and `message` fields).
- Avoid calling macro materialization (micro-optimization) on each writes call (`OWrite[..] { Json.writes[..]/* !! */.writes(..) ... }`).
…ibuteCheckingError (no implementation change on itself)
…eckingError: - Remove 'manual' discriminator from individual writes - Use Play JSON capability to automatically derive `OWrites` for sealed trait - Make sure the macro is configured with custom discriminator (field & value)
cchantep
commented
Jun 19, 2026
| Json.writes[MissingAttributeError].writes(mae) + | ||
| ("type" -> JsString("MissingAttributeError")) + | ||
| ("message" -> JsString(mae.toString)) | ||
| private def attributeErrorWrites[E <: AttributeError](underlying: OWrites[E]): OWrites[E] = { |
Author
There was a problem hiding this comment.
The current invalidFormatAttributeErrorWrites/unknownAttributeErrorWrites/updateReadOnlyAttributeErrorWrites/missingAttributeErrorWrites are private, only for attributeCheckingExceptionWrites to use them, so change there (such as removing the type field) do not introduce regression, as long as there is no regression in attributeCheckingExceptionWrites.
| private[elastic4play] implicit val missingAttributeErrorWrites: OWrites[MissingAttributeError] = attributeErrorWrites(Json.writes[MissingAttributeError]) | ||
|
|
||
| implicit val attributeCheckingExceptionWrites: OWrites[AttributeCheckingError] = { | ||
| val pkgName = classOf[AttributeError].getPackage.getName + '.' |
Author
There was a problem hiding this comment.
As runtime invariant, it allows to move the *AttributeError to a different without change there.
| implicit val attributeCheckingExceptionWrites: OWrites[AttributeCheckingError] = { | ||
| val pkgName = classOf[AttributeError].getPackage.getName + '.' | ||
|
|
||
| implicit def errWrites: OWrites[AttributeError] = Json.configured(JsonConfiguration( |
Author
There was a problem hiding this comment.
Custom discriminator configuration (Play JSON default is "_json": "full.qualified.type.ClassName", the current Elastic4Play is "type": "ClassName") to avoid regression.
| Json.toJsObject(missingAttrErr) must_=== missingAttrErrJson | ||
| } | ||
|
|
||
| "support AttributeCheckingError" in { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Macro derivation of OWrites for individual *AttributeError types
Current
JsonFormatcall macro materialization on each call for the individual attribute error type.When the current
attributeCheckingExceptionWritesis used it could also materialize multiple time the sameOWrites[*AttributeError]for multiple value of the same kind inerrors.This could be improved, as such
OWritesinstances are invariants (do not depend on call).'Manual' OWrites for AttributeCheckingError
The current
OWritesinstance for typeAttributeCheckingErroris 'manually' implemented (see bellow).It could benefit from using the capability of macro derivation to supported sealed family (there
AttributeError), so that if a new kind of error is added, it would be supported there without change.Approach
A new test suit is introduced, with test(s) covering the existing implementation before any change, to make sure there is no regression.