jungm commented on code in PR #191:
URL: https://github.com/apache/bval/pull/191#discussion_r2096053738


##########
bval-jsr/src/main/java/org/apache/bval/cdi/BValExtension.java:
##########
@@ -132,28 +134,31 @@ public <A> void processAnnotatedType(final @Observes 
ProcessAnnotatedType<A> pat
         final int modifiers = javaClass.getModifiers();
         if (!javaClass.isInterface() && !javaClass.isAnonymousClass() && 
!Modifier.isFinal(modifiers) && !Modifier.isAbstract(modifiers)) {
             try {
-                Queue<Class<?>> toProcess = new LinkedList<>();
-                toProcess.add(annotatedType.getJavaClass());
+                Queue<AnnotatedType<?>> toProcess = new LinkedList<>();
+                toProcess.add(annotatedType);
 
                 while (!toProcess.isEmpty()) {
-                    Class<?> now = toProcess.poll();
-                    Executable[] methods = now.getMethods();
-                    Executable[] constructors = now.getConstructors();
+                    AnnotatedType<?> now = toProcess.poll();
+                    Set<? extends AnnotatedMethod<?>> methods = 
now.getMethods();
+                    Set<? extends AnnotatedConstructor<?>> constructors = 
now.getConstructors();
 
                     if (hasValidation(now)
-                            || hasValidation(methods) || 
hasValidation(constructors)
-                            || hasParamsWithValidation(methods) || 
hasParamsWithValidation(constructors)) {
+                            || methods.stream().anyMatch(this::hasValidation)
+                            || 
constructors.stream().anyMatch(this::hasValidation)
+                            || 
methods.stream().anyMatch(this::hasParamsWithValidation)
+                            || 
constructors.stream().anyMatch(this::hasParamsWithValidation)) {
                         pat.setAnnotatedType(new 
BValAnnotatedType<>(annotatedType));
-
                         break;
                     }
 
-                    // Nothing found, collect superclass/interface and repeat 
(See BVAL-222)
-                    if (now.getSuperclass() != Object.class && 
now.getSuperclass() != null) {
-                        toProcess.add(now.getSuperclass());
+                    Class<?> superclass = now.getJavaClass().getSuperclass();

Review Comment:
   IMO the example in the test should work:
   - GreetingService defines a method with preconditions
   - GreetingServiceImpl overrides it, one could argue it weakens preconditions 
but there is also no way for it to "re enforce" the precondition from the 
parent. I would assume that to be happening by default because of ยง3.1
   - IntermediateGreetingService/LastGreetingService do not overwrite the 
method anymore, so nothing on preconditions/postconditions should change
   
   This is also how the validation ends up working, but the algorithm in the 
CDI extension must IMO look at the entire type hierarchy to decide whether or 
not to enable the BValInterceptor. And I think it is better to have false 
positives than false negatives at detecting what needs the interceptor (of 
course the goal should be to already be 100% aligned with the spec in the 
algorithm in BValExtension)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@bval.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to