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.

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

@annotation pointcut is not matched with complex hierarchy and generics [SPR-17310] #21843

Open
spring-projects-issues opened this issue Sep 28, 2018 · 6 comments
Labels
in: core status: waiting-for-triage

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Sep 28, 2018

Yanming Zhou opened SPR-17310 and commented

Persistable   <-  BaseUser   <-  User
BaseManager<T extends Persistable>   <-  BaseUserManager<T extends BaseUser>  <-     UserManager<User> 
 
BaseManagerImpl<T extends Persistable>  <-  BaseUserManagerImpl<T extends BaseUser>  <-     UserManagerImpl<User>

The mostSpecificMethod should return method UserManagerImpl.save(User), but return BaseUserManagerImpl.save(BaseUser) which doesn't annotated, it will cause @annotation pointcut broken.


Affects: 5.1 GA

Attachments:

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 28, 2018

Yanming Zhou commented

It seems eclipse compiler bug. I'm not sure it has relation with https://bugs.eclipse.org/bugs/show_bug.cgi?id=495396

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 30, 2018

Yanming Zhou commented

I have reported https://bugs.eclipse.org/bugs/show_bug.cgi?id=539651

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 30, 2018

Yanming Zhou commented

Finally I figure it out, method.getDeclaringClass() return wrong class with eclipse compiler, it cause spring ClassUtils.getMostSpecificMethod() return wrong method.

@Test
public void test() throws Exception {
	Class<?> targetClass = UserManagerImpl.class;
	Method method = targetClass.getMethod("save", Persistable.class);
	assertEquals(targetClass, method.getDeclaringClass()); // eclipse compiler fail
}

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 12, 2018

Yanming Zhou commented

javac create two bridge methods:

public void com.example.UserManagerImpl.save(com.example.Persistable)
public void com.example.UserManagerImpl.save(com.example.BaseUser)

ecj create only one bridge method:

public void com.example.UserManagerImpl.save(com.example.BaseUser)

It seems a corner case that JLS not covered, maybe it's not a bug of eclipse compiler, Juergen Hoeller could you make some changes to keep compatibility with both of them?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 17, 2018

Yanming Zhou commented

Workaround:

add two lines before return statement

resolvedMethod = BridgeMethodResolver.findBridgedMethod(resolvedMethod);
resolvedMethod = ClassUtils.getMostSpecificMethod(resolvedMethod, specificTargetClass);

https://github.com/spring-projects/spring-framework/blob/master/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java#L198

@spring-projects-issues spring-projects-issues added type: bug status: waiting-for-triage in: core and removed type: bug labels Jan 11, 2019
@quaff
Copy link
Contributor

@quaff quaff commented Mar 1, 2019

Minimized unit test, It will fail in eclipse.

import static org.junit.Assert.assertEquals;

import java.lang.reflect.Method;

import org.junit.Test;
import org.springframework.aop.support.AopUtils;

public class AopUtilsTest {

	@Test
	public void testGetMostSpecificMethod() throws Exception {
		String methodName = "feed";
		Class<?> targetClass = DogService.class;
		Method targetMethod = targetClass.getDeclaredMethod(methodName, Dog.class);
		Method originalMethod = AnimalService.class.getDeclaredMethod(methodName, Animal.class);
		Method actualMethod = AopUtils.getMostSpecificMethod(originalMethod, targetClass);
		assertEquals("Please ensure class compiled with javac not eclipse", targetMethod, actualMethod);
	}

	public static class Animal {

	}

	public static abstract class Mammal extends Animal {

	}

	public static class Dog extends Mammal {

	}

	public static class AnimalService<T extends Animal> {
		public void feed(T obj) {
		}
	}

	public static class MammalService<T extends Mammal> extends AnimalService<T> {
		@Override
		public void feed(T obj) {
		}
	}

	public static class DogService extends MammalService<Dog> {
		@Override
		public void feed(Dog obj) {
		}
	}

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: waiting-for-triage
Projects
None yet
Development

No branches or pull requests

2 participants