io.micronaut.validation.Validated does not work on final classes
#1291
Comments
modern reflection also uses runtime bytecode generation in combination with or instead of native calls. The only difference is that micronaut tries to use compile-time bytecode rewriting/generation. Whereas I'm a bit upset that "modern" frameworks resort to such a strange and uncivilized solution like generating proxies around validated objects i.e. this is not a service endpoint or something worth intercepting. How are they suppose to handle new java There are some precedents of us adding feature flags like Here's some of the alternatives (no guarantee that these will work, but):
@Immutable interface X {
int attr();
class Builder extends ImmutableX.Builder {}
}
@Immutable interface X {
int attr();
@Value.Check default void validate() {
if (attr() < 0) throw or report something
}
}
|
|
I will not argue, but that is an unexpected stance from someone working on this library. I mean, why does Immutables extend my class? Why not just inject random byte code? I.e., why not use Lombok? (Which does have a non final option, though I am no Lombok exert). All leading and rhetorical questions I completely understand your frustration, though. I write frameworks and I hate things like this, too, so I know where you are coming from Feel free to close this if it will not be considered. In meantime, I will try the proposed solutions and see if anything stands out |
Just to clarify, we doing legitimate annotation processing by providing generating types which extends user written types or act as supertypes to the user written types. We're completely follow JLS and compilation related spec (including annotation processing JSR etc): we analyze type and method signatures and generate standalone Java files. You define structure of a datatype, Immutables processor generates an implementation. It just cannot be criticized by "why does Immutables extend my class", it's legit, you've added annotation to generate derived implementation – that's exactly what annotation processor were designed for. |
|
I thought the Lombok thing would get to you, which is why your stance on
To which I say: what do you think Micronaut is doing? I think Micronaut does what Immutables does: generate real code I can look at They are generating compile time code using preprocessors. They could have gone the Lombok route and totally bypassed JLS restrictions as you point out (which is why I brought Lombok up in a provoking manner) or the Spring route and bypassed it using reflection or runtime proxies But, since they generate source code instead of inject bytecode, they have to follow the rules. And so, in this particular case, they cannot extend and wrap a final class to instrument it with generated JSR validation code. Thus the ask for an option to make the class not be final (which I still think does nothing to invalidate the immutable nature of the class). However, as I said, I understand your stance, but hopefully this helps you understand where I am coming from I suspect you might see more of this as java web frameworks like Micronaut, Quarkus, Helidon, and even Spring move away from runtime reflection and towards compiling stuff ahead of time because how else are you really going to implement validation during pre-compile time other than extending the class and doing aspect oriented injection stuff? |
|
ok, I stand corrected about their AOP, it was known to me that Micronaut uses annotation processor for dependency injection and wiring, but I was under wrong impression that for AOP they use bytecode post-(compilation) or pre-(classloading)processing. If AOP uses annotation processing then it's changes very little if anything as my major concern is "how" it is used that it requires data types to be non-final. I went on to actually check what is going on there. That caught my attention Around advice that ensures an objects methods are validated.. After reading this I've started to google for more detailed documentation, but I've found couple of pages but none of those matches the issue described in this Github ticket. At least in my head, so I'm not sure that I'm understand things.
import io.micronaut.http.HttpResponse;
import io.micronaut.http.annotation.Body;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Post;
import io.micronaut.validation.Validated;
import javax.validation.Valid;
import java.util.Collections;
@Validated
@Controller("/email")
public class EmailController {
@Post("/send")
public HttpResponse send(@Body @Valid Email email) {
return HttpResponse.ok(Collections.singletonMap("msg", "OK"));
}
}
// Annotate the controller with Validated
// Annotate the POJO to validate with @ValidIf that is the case, then immutable
Regardless of the integration path, you should clearly understand when and what calls will be "intercepted" and how actual validation will kick in. Will it be during immutable construction, or before passing it to controller/or another adviced method. My apologies if I've got something wrong again, but I currently don't understand the issue after looking at Micronaut documentation. |
|
First, I misspoke about Micronaut generating source code during pre-compilation phase; it was just my IDE de-compiling stuff and me not paying attention. Micronaut seems to generate hints from the annotations that would otherwise be read at run time and generates a bunch of helper classes for each kind of validation. That much I kind of knew before but I never stared at the classes before because they are just generated junk for the most part. However, after doing said staring, I cannot see anywhere where the class is actually extended. Maybe I should look at the source of the pre-processor to see what it is doing when it checks for Regarding #1, you raise a good point. My issue arose while using a swagger code generator which spits out source code. It chooses to annotate the datatype; not the controller. Let me try reversing that to align with the examples in the documentation and see if that does not resolve things before I waste your time on this. If it works, I will move my beef there Regarding #2, I do not think Micronaut has their own validator. The docs suggest adding |
|
Okay, seems like this was a waste of your time, for which I apologize The swagger codegen tool I was using was annotating the model classes so I assumed that was the correct thing to do. Seems like that is not the case. Adding annotations on the model class does not seem to enforce validation. Will have to follow up with codegen project to see why they are doing this The minimum that seems to be required is to annotate the rest controller method argument with Example that does the correct thing by enforcing payload size: @Controller("/v1")
public class BasicRestService {
@Post("/hello")
public String sayHello(@Valid final Payload payload) {
return String.format("Hola, %s!\n", payload.getName());
}
}
@Introspected
@Immutable
@JsonDeserialize(as = ImmutablePayload.class)
public interface Payload {
@Size(min = 1, max = 11)
String getName();
}I did learn that if you do not include Again, sorry to waste your time by not doing proper due diligence |
Using Micronaut
@Validatedappears to generate proxies which extend the classCompiling the following:
Results in following:
Note the compile error is likely from Micronaut's pre-processor. it goes away if not using
@JsonSerializebut the usecase here is to use this as a payload so not really an optionMicronaut is a compile time oriented dependency injector and does not rely on reflection, so I get why Micronaut probably needs to extend the class to make a proxy
I also get why Immutables makes the class
final(at least I think). But the constructor isprivateso extending it is already hampered by that, plus everything is basicallyfinalwithin the class anyway, so I do not think making itfinalreally buys much in terms of safetyI guess I could generate the Immutable class, add it to my source file, make it non-final, comment out the
@Immutableannotation, and repeat this exercise if/when the class changesIs there any middle ground?
The text was updated successfully, but these errors were encountered: