rmannibucau commented on code in PR #191:
URL: https://github.com/apache/bval/pull/191#discussion_r2096042675
##########
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:
hmm, both part kind of contradict themselves if I'm reading it right and in
CDI integration it is worse since all the inheritance is in Annotated* model,
no more in plain reflection at all so think we should exclude and challenge (if
desired) this test and just not loop to parents. (likely with a comment
explaining the model is not reflection based + liskov principle is the opposite
of this)
##########
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:
hmm, both parts kind of contradict themselves if I'm reading it right and in
CDI integration it is worse since all the inheritance is in Annotated* model,
no more in plain reflection at all so think we should exclude and challenge (if
desired) this test and just not loop to parents. (likely with a comment
explaining the model is not reflection based + liskov principle is the opposite
of this)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
