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