Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback validation should blow up if the fallback method isn't from … #123

Merged
merged 1 commit into from
Oct 11, 2019
Merged

Fallback validation should blow up if the fallback method isn't from … #123

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion implementation/src/main/java/io/smallrye/faulttolerance/config/SecurityActions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;

// this must be kept identical to io.smallrye.faulttolerance.SecurityActions
Expand Down Expand Up @@ -98,10 +100,20 @@ private static Method doGetMethod(final Class<?> beanClass, final Class<?> decla
ParametrizedParamsTranslator expectedParamsTranslator = prepareParamsTranslator(beanClass, declaringClass);

ParametrizedParamsTranslator actualParamsTranslator = new ParametrizedParamsTranslator();

// gather all interface methods based on declaring class
Set<String> allInterfaceMethodNames = new HashSet<>();
for (Class<?> clazz : declaringClass.getInterfaces()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this perhaps include super-interfaces of declaringClass.getInterfaces()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think spec allows that. Quoting:

The named fallback method must be on the same class, a superclass or an implemented interface of the class which declares the annotated method

From implemented interface of the class... I assume it has to be on an interface that directly implemented by given class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's probably yet another vague formulation in the spec, but fair enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be the case, to be fair I am learning about FT spec "on the fly" so take what I say with a pinch of salt ;)

for (Method m : clazz.getMethods()) {
allInterfaceMethodNames.add(m.getName());
}
}
while (true) {
method = getMethodFromClass(beanClass, current, name, expectedParameters, actualParamsTranslator,
expectedParamsTranslator);
if (method != null) {
// fault tolerance spec is in 8.1.2; fallback method must be from same class, superclass, implemented interface
if (method != null
&& (current.isAssignableFrom(declaringClass) || allInterfaceMethodNames.contains(method.getName()))) {
return method;
} else {
if (current.getSuperclass() == null) {
Expand Down