Allow dynamic properties in ProblemDetail

[rstoyanchev]

There is a need for a generic map of properties in ProblemDetail for properties that are not known ahead of time, and cannot be exposed as setters from a subclass. Based on feedback on #27052 (comment) and #27052 (comment),

[vy]

@rstoyanchev, getting unknown properties mapped to a single field can be tricky for Jackson. Even though this ticket is closed, I would strongly encourage adding tests for checking whether both de- and serialization works as expected.

[rstoyanchev]

@vy, happy to re-open and make further updates. That said, from Jackson's perspective, isn't it just a straight-forward Map field to serialize and deserialize? In other words, could you clarify the tricky part is you have in mind?

[vy]

I think we have a misunderstanding about the use case here. Let me try to clarify it with an example. Let the server have a CustomProblemDetail extending from ProblemDetail:

import org.springframework.http.ProblemDetail;

public class CustomProblemDetail extends ProblemDetail {

    private final String host;

    public CustomProblemDetail(ProblemDetail parent, String host) {
        super(parent);
        this.host = host;
    }

    public void setHost(String host) {
        this.host = host;
    }

    public String getHost() {
        return this.host;
    }

}

Due to a service failure, the server responds with a CustomProblemDetail and the client receives the following JSON:

{
  "type": "/problem/bad-request",
  "title": "Bad Request",
  "status": 400,
  "detail": "miserable failure",
  "instance": "/greeting",
  "host": "awesome-x3csd3.bol.com"
}

Note the extra host entry at the bottom.

Since the client doesn't know about the CustomProblemDetail class, it will try to deserialize this JSON to a ProblemDetail. It would expect ProblemDetail#properties to contain a single entry: host=awesome-x3csd3.bol.com, though it won't since we didn't use any @JsonAnyGetter, @JsonAnySetter, etc. magic there. Hence the host information get totally lost during deserialization and this should exactly be the goal this feature aims to prevent.

[rstoyanchev]

Thanks for clarifying.

The goal is to avoid dependency on any serialization technology at the lowest level. ProblemDetail is the basic abstraction that Spring MVC and WebFlux can use to raise exceptions with the standard fields, and that also helps to enable a range of features for handling and rendering such responses.

Extra properties and serialization magic remain as a separate layer, through sub-classes and a global exception handler cloning the original exception to create the sub-class, as with your CustomProblemDetail above.

Originally I left Map<String, Object> properties out of ProblemDetail, thinking that sub-classes would declare it and add @JsonAnyGetter/Setter. I introduced it here, thinking it would be useful still for the base class to provide a way to add dynamic properties from the server side, but your example above is with a property that's known on the server side, but not on the client side. So I'm not sure if dynamic properties from the server side are something all that useful? In any case, the expectation is still for a sub-class to override getProperties and setProperty and to add @JsonAnyGetter/Setter, which does mean that out of the box, a properties: {} map is what gets serialized. If that seems not unacceptable, then it doesn't make sense to have properties in ProblemDetail after all.

I'm thinking that we should add a JacksonProblemDetail sub-class with the @JsonAnyGetter/Setter annotations and apply it from ResponseEntityExceptionHandler, if Jackson is present, or perhaps even create it by default for Spring MVC/WebFlux exceptions when Jackson is present. Then a CustomProblemDetail could also extend from JacksonProblemDetail.

What do you think?

[vy]

I am happy to hear that we are on the same page. (And thanks so much for hearing me out! 🙏)

So I'm not sure if dynamic properties from the server side are all something useful?

I think they certainly are! Ideally people shouldn't subclass ProblemDetail and should simply use its properties to tailor the content. So please keep it.

I'm thinking that we should add a JacksonProblemDetail sub-class with the @JsonAnyGetter/Setter annotations and apply it from ResponseEntityExceptionHandler, if Jackson is present, or perhaps even create it by default for Spring MVC/WebFlux exceptions when Jackson is present. Then a CustomProblemDetail could also extend from JacksonProblemDetail.

What about introducing a ProblemDetailJacksonMixIn and registering it to the available ObjectMapper bean and/or as an ObjectMapper customizer? This way, if I am not mistaken, ProblemDetail subclasses employing Jackson for serialization will implicitly use the mix-in for the properties field, unless they override it or use a custom ObjectMapper.

The reason I prefer a mix-in compared to a JacksonProblemDetail, the former will implicitly work for any ProblemDetail subclass thrown against Jackson, whereas the latter approach expects the user to always subclass from JacksonProblemDetail instead.

[rstoyanchev]

No worries, and thanks for tracking this and providing feedback. This is very much appreciated!

Good suggestion for the Jackson mix-in. We'll go with that and register it automatically from Jackson2ObjectMapperBuilder so it will require no further configuration. The only drawback is that properties will render as a nested map with other libraries but Jackson is popular and comparable solutions could probably be found for others too. The benefit of built-in support for additional properties, and reducing the need for sub-classing, that just works with Jackson is overall a good place to be.