Fix comparison of title in equals() and hashCode() of ProblemDetail#30294
Fix comparison of title in equals() and hashCode() of ProblemDetail#30294quaff wants to merge 1 commit intospring-projects:mainfrom
title in equals() and hashCode() of ProblemDetail#30294Conversation
Lazy computed title property should be taken into account
equals() and hashCode() in ProblemDetail
simonbasle
left a comment
There was a problem hiding this comment.
thanks for the PR @quaff but what's the rationale behind this?
using the getTitle() over this.title isn't that much of an improvement because if the field is not set, the getter derives the result from the this.status field (namely, the reason phrase of the enum which is constant). since this.status already participates in both equals() and hashcode(), this change doesn't add new information to these methods.
it even arguably can make them less performant, because now instead of a simple null equality + int comparison, it adds resolution of an enum from the status code + getting the enum's reason phrase to the mix.
so overall I don't see the benefit of this change.
You can review the test I added, currently the ResponseEntity<ProblemDetail> resp = this.testRestTemplate.exchange(
RequestEntity.method(HttpMethod.GET, path).header(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE).build(),
ProblemDetail.class);
ProblemDetail expected = ProblemDetail.forStatusAndDetail(HttpStatus.CONFLICT, "Conflicts arise");
expected.setTitle(HttpStatus.CONFLICT.getReasonPhrase());
expected.setInstance(URI.create(path));
assertThat(resp.getBody()).isEqualTo(expected);Title must be explicit set before comparison, but it is implicit at server side, that's not idiomatic. |
rstoyanchev
left a comment
There was a problem hiding this comment.
Yes that makes sense to use getTitle() to get a consistent value.
equals() and hashCode() in ProblemDetailtitle in equals() and hashCode() of ProblemDetail
Lazy computed title property should be taken into account See gh-30294
Lazy computed title property should be taken into account