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

<replaced-method /> unnecessarily requires explicit arg-type since 6.0 #31826

Closed
sammyhk opened this issue Dec 13, 2023 · 5 comments
Closed

<replaced-method /> unnecessarily requires explicit arg-type since 6.0 #31826

sammyhk opened this issue Dec 13, 2023 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@sammyhk
Copy link

sammyhk commented Dec 13, 2023

Affects: 6.1.1


We are migrating from Spring 5.3.x to 6.1.1 and found seems not working in the XML definition. Consider the following example:

applicationContext.xml

<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

	<bean id="testReplaceMethodBean" class="test.spring.TestReplaceMethodBean">
		<replaced-method name="doSomething"
			replacer="doSomethingMethodReplacer" />
	</bean>

	<bean id="doSomethingMethodReplacer"
		class="test.spring.DoSomethingMethodReplacer" />
</beans>

TestReplaceMethodBean.java

package test.spring;

public interface TestReplaceMethodBean {
    String doSomething(Object... parameters);
}

DoSomethingMethodReplacer.java

package test.spring;

import java.lang.reflect.Method;
import java.util.Arrays;
import org.springframework.beans.factory.support.MethodReplacer;

public class DoSomethingMethodReplacer implements MethodReplacer {
    @Override
    public Object reimplement(Object obj, Method method, Object[] args) throws Throwable {
        return Arrays.toString((Object[]) args[0]);
    }
}

TestReplacedMethod.java

package test.spring;

import org.springframework.context.support.AbstractApplicationContext;
import org.springframework.context.support.ClassPathXmlApplicationContext;

public class TestReplacedMethod {

    public static void main(String[] args) {
        try (AbstractApplicationContext applicationContext = new ClassPathXmlApplicationContext(
                "classpath*:path/to/applicationContext.xml")) {
            TestReplaceMethodBean testBean = applicationContext.getBean(TestReplaceMethodBean.class);
            // expecting to print "[value1, value2]"
            System.out.println(testBean.doSomething(new Object[] { "value1", "value2" }));
        }
    }
}

It is expected to print [value1, value2] when run TestReplacedMethod but got AbstractMethodError:

Exception in thread "main" java.lang.AbstractMethodError: Receiver class test.spring.TestReplaceMethodBean$$SpringCGLIB$$0 does not define or inherit an implementation of the resolved method 'abstract java.lang.String doSomething(java.lang.Object[])' of interface test.spring.TestReplaceMethodBean.
	at test.spring.TestReplacedMethod.main(TestReplacedMethod.java:12)
@sammyhk sammyhk changed the title <replaced-method /> not working properly in Spring 6 <replaced-method /> not working properly in Spring 6 Dec 13, 2023
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 13, 2023
@sammyhk
Copy link
Author

sammyhk commented Dec 13, 2023

Further tested with <arg-type match="java.lang.Object[]" /> still not working:

Updated but still not working applicationContext.xml

<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

	<bean id="testReplaceMethodBean" class="test.spring.TestReplaceMethodBean">
		<replaced-method name="doSomething"
			replacer="doSomethingMethodReplacer">
			<arg-type match="java.lang.Object[]" />
		</replaced-method>
	</bean>

	<bean id="doSomethingMethodReplacer"
		class="test.spring.DoSomethingMethodReplacer" />
</beans>

While adding <arg-type match="java.lang.Object" /> works, maybe this is a hint to debug:

Workaround working applicationContext.xml

<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
	xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

	<bean id="testReplaceMethodBean" class="test.spring.TestReplaceMethodBean">
		<replaced-method name="doSomething"
			replacer="doSomethingMethodReplacer">
			<arg-type match="java.lang.Object" />
		</replaced-method>
	</bean>

	<bean id="doSomethingMethodReplacer"
		class="test.spring.DoSomethingMethodReplacer" />
</beans>

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Dec 13, 2023
@sbrannen sbrannen self-assigned this Dec 13, 2023
@sbrannen
Copy link
Member

Hi @sammyhk,

Thanks for raising the issue.

I've confirmed it behaves the way you've explained with Spring Framework 6.0.x using the following standalone test class that I based on your example.

package demo;

import java.lang.reflect.Method;
import java.util.Arrays;

import org.junit.jupiter.api.Test;

import org.springframework.beans.factory.support.MethodReplacer;
import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.io.ByteArrayResource;
import org.springframework.core.io.Resource;

import static org.assertj.core.api.Assertions.assertThat;

class ReplacedMethodTests {

	@Test
	void test() {
		String xmlConfig = """
				<?xml version="1.0" encoding="UTF-8"?>
				<beans xmlns="http://www.springframework.org/schema/beans"
					xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
					xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">

					<bean id="testReplaceMethodBean" class="demo.ReplacedMethodTests.TestReplaceMethodBean">
						<replaced-method name="doSomething" replacer="doSomethingMethodReplacer">
							<!--
							<arg-type match="java.lang.Object" />
							-->
						</replaced-method>
					</bean>

					<bean id="doSomethingMethodReplacer"
						class="demo.ReplacedMethodTests.DoSomethingMethodReplacer" />
				</beans>
				""";

		Resource xmlResource = new ByteArrayResource(xmlConfig.getBytes());

		try (GenericApplicationContext context = new GenericApplicationContext()) {
			new XmlBeanDefinitionReader(context).loadBeanDefinitions(xmlResource);
			context.refresh();

			TestReplaceMethodBean testBean = context.getBean(TestReplaceMethodBean.class);
			assertThat(testBean.doSomething("value1", "value2")).isEqualTo("[value1, value2]");
		}
	}


	interface TestReplaceMethodBean {

		String doSomething(Object... parameters);
	}

	static class DoSomethingMethodReplacer implements MethodReplacer {

		@Override
		public Object reimplement(Object obj, Method method, Object[] args) {
			return Arrays.toString((Object[]) args[0]);
		}
	}

}

I'll investigate whether this is a regression and report back here.

@sbrannen
Copy link
Member

I have confirmed that the above test class passes on 5.3.x unmodified.

So this is indeed a regression, and we are investigating the issue.

@sbrannen sbrannen added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 13, 2023
@sbrannen sbrannen changed the title <replaced-method /> not working properly in Spring 6 <replaced-method /> unnecessarily requires explicit arg-type since 6.0.x Dec 13, 2023
@sbrannen sbrannen added this to the 6.1.2 milestone Dec 13, 2023
@sbrannen sbrannen changed the title <replaced-method /> unnecessarily requires explicit arg-type since 6.0.x <replaced-method /> unnecessarily requires explicit arg-type since 6.0 Dec 13, 2023
@sbrannen
Copy link
Member

sbrannen commented Dec 13, 2023

While adding <arg-type match="java.lang.Object" /> works, maybe this is a hint to debug:

Yes, that was a very good tip. Thanks! 👍

After several rounds of debugging, I determined that ReplaceOverride.matches(Method) is now invoked before AbstractBeanDefinition.prepareMethodOverrides() is invoked. Consequently, the overloaded flag in ReplaceOverride has an incorrect (default) value of true which results in the exception you shared.

That's why <replaced-method /> now requires an explicit arg-type since 6.0, and I've updated the title of this issue to reflect that.

I believe that this is a regression that was introduced in commit b31a158.

I have a local prototype for a fix, so hopefully we'll be able to address this in time for tomorrow's 6.0.x and 6.1.x releases.

@sbrannen sbrannen added the for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x label Dec 13, 2023
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels Dec 13, 2023
sbrannen pushed a commit that referenced this issue Dec 13, 2023
sbrannen added a commit that referenced this issue Dec 13, 2023
This commit introduces an integration test for the regression fixed in
the previous commit (76bc9cf).

See gh-31826
Closes gh-31828

(cherry picked from commit 8d4deca)
@sbrannen
Copy link
Member

This has been fixed and backported for inclusion in 6.1.2 to 6.0.15, respectively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

4 participants