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


##########
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:
   If I may add my 2 cents.
   I would also interpret the spec in the same way as @rmannibucau , in 
essence, Liskovs Principle has to be followed and precondition weakening should 
be possible. Thus, for retaining an interface constraint, the implementation 
also has to declare the very same constraint, otherwise its a valid constraint 
weakening and the parent constraint should be ignored.
   The rules from ยง5.6.5 seem to be ambiguous or incomplete for this case, and 
should be clarified.
   
   In this regard, issues like 
https://issues.apache.org/jira/projects/TOMEE/issues/TOMEE-4449 seem to be 
not-spec-compliant.
   
   How about a configurable switch? Like already proposed in 
https://issues.apache.org/jira/projects/BVAL/issues/BVAL-175.
   It seems like BVal considered the parent method constraints till version 
2.0.2, so keeping this behaviour configurable, but have the spec compliant way 
as default, i.e. parent constraints are not considered to support the weakening 
case. Since both ways also differ in performance, a switch looks like a good 
compromise, so that user can choose by themselves what they prefer.



-- 
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