sialcasa / mvvmFX

an Application Framework for implementing the MVVM Pattern with JavaFX
Apache License 2.0
489 stars 104 forks source link

ClassNotFoundException for PostConstruct with Java 9 #547

Closed nils-christian closed 6 years ago

nils-christian commented 6 years ago

Hi,

With the transition Java 8 -> Java 9, the rt.jar was (I think) removed and the classes were moved to other modules. This means also, that some of the classes are now part of the Java EE API and no longer part of the Java SE API. This includes the class "PostConstruct", which is by default no longer available in a JRE 9. As you check for this annotation in ViewLoaderReflectionUtils.initializeViewModel(ViewModelType), one gets a class not found exception: Caused by: java.lang.ClassNotFoundException: javax.annotation.PostConstruct

Possible workarounds:

I am not sure what would be a good solution for the mvvmFX code. Maybe catching the exception at least? Using reflection to identify the annotation by name?

This issue is related to #440.

manuel-mauky commented 6 years ago

Thanks for this hint. There was a similar question on stackoverflow some time ago but I wasn't sure what the best solution for this issue would be. I think there are 3 possible solutions (in this order):

1) check by name 2) catch the exception and rethrow it with a meaningful description of how to fix the issue. 3) add javax.annotation-api to dependencies of mvvmFX

nils-christian commented 6 years ago

Hi,

Maybe:

However, mvvmFX is not really using the PostConstruct method. It just checks for it. One could also argue that it is not the task of mvvmFX to check for such invalid constructs, especially because there is more than one CDI framework out there which could initialize the view model a second time. And if PostConstruct is no longer part of the SE library with Java 9, maybe mvvmFX shouldn't even try and check this.

Best regards,

Nils

manuel-mauky commented 6 years ago

Strictly speaking it's not the task of mvvmFX to check for this. On the other hand, for users of the framework this error is easy to make and hard to find. And it is not unusual to make this misconfiguration. Our goal is to make development as easy as possible and to point users to problems as good as possible. So I think we should keep this check.

I have an alternative solution:

boolean postConstructAnnotationPresent = Arrays.stream(initMethod.getAnnotations())
                    .map(Annotation::annotationType)
                    .map(Class::getName)
                    .anyMatch(annotationType -> annotationType.equals("javax.annotation.PostConstruct"));

if(postConstructAnnotationPresent) {
    throw new IllegalStateException(...);
}

What do you think?

nils-christian commented 6 years ago

Hi,

I see. Well, from the looks of it, your solution should work. One could think about a solution that wouldn't require iterating over all annotations from the method, but then again the initialize method is not the most often called method and shouldn't have that much annotations anyway. I would prefer this solution.

Best regards

Nils