Port rest-data-panache to RESTEasy Reactive #19801
Conversation
RESTEasy Classic supports this, so let's remain compatible
This is mentioned by the method's Javadoc, even though the TCK doesn't test it
Before this change the custom serializer would not take the registered Jackson modules into account
quarkus-bot
bot
added
area/core
area/hibernate-validator
| .and().body("parameterViolations[0].path", equalTo("add.arg0.title")) | ||
| .and().body("parameterViolations[0].message", equalTo("must not be blank")); | ||
| .and().body("violations[0].field", equalTo("add.arg0.title")) | ||
| .and().body("violations[0].message", equalTo("must not be blank")); |
There was a problem hiding this comment.
Hmmm. Why do we have a difference here? Shouldn't we try to stay on par with what RESTEasy classic does?
There was a problem hiding this comment.
I don't think so.
This different structure of the response has been part of RESTEasy Reactive from day 1.
Furthermore, this behavior is not part of the spec, so RESTEasy's was just doing its own thing, and FWIW, it didn't seem to be emulated by other frameworks. What we have here is very similar to what popular approaches like Zalando Problem have.
Of course the case can be made that we should follow RESTEasy's lead for most things, but I would say that we should only do that for things that really do make sense.
There was a problem hiding this comment.
Well, the problem is that we introduce a difference for absolutely no value. field is definitely less correct than path and we will have people migrating from RESTEasy classic soon and they will have problems with it.
I would rather fix it now and be consistent rather than having a difference between the two implementations for absolutely no value that will haunt us forever.
I don't even understand why you did things differently in the first place and it should probably have been discussed. If there is a value I don't see, I'm all ears.
There was a problem hiding this comment.
I personally thought the RESTEasy format sucked, so I looked for what other options existed, eventually settling on what Reactive Routes was doing.
I am very much inclined to not change this, unless everyone else thinks otherwise.
There was a problem hiding this comment.
Here is my proposal: Since this work is pretty much unrelated to the violations report, we get this in (barring of course whatever other changes may be needed) and then I send an email to dev list asking peoples opinion. If the majority think we should change it, I'll do that.
|
@stuartwdouglas @FroMage are you OK with this? |
This change is related to quarkusio#19801
This also contains a bunch of small fixes for bugs that surfaced during the port
Resolves: #19771