Skip to content

Commit

Permalink
Fixed bug where the constructor strategy would prepend its intercepti…
Browse files Browse the repository at this point in the history
…on instead of appending it such that user-specified instrumentations would be applied first. Fixed a bug in the method call instrumentation where calling a constructor would underflow the operand stack.
  • Loading branch information
Rafael Winterhalter committed Feb 24, 2015
1 parent 94e5a91 commit 03a2df7
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 43 deletions.
Expand Up @@ -122,7 +122,7 @@ public MethodRegistry inject(MethodRegistry methodRegistry,
case DEFAULT_CONSTRUCTOR:
case IMITATE_SUPER_TYPE:
case IMITATE_SUPER_TYPE_PUBLIC:
return methodRegistry.prepend(new MethodRegistry.LatentMethodMatcher.Simple(isConstructor()),
return methodRegistry.append(new MethodRegistry.LatentMethodMatcher.Simple(isConstructor()),
SuperMethodCall.INSTANCE,
defaultMethodAttributeAppenderFactory);
default:
Expand Down
Expand Up @@ -529,9 +529,14 @@ static enum ForSelfOrStaticInvocation implements TargetHandler {

@Override
public StackManipulation resolve(MethodDescription methodDescription, TypeDescription instrumentedType) {
return methodDescription.isStatic()
? StackManipulation.LegalTrivial.INSTANCE
: MethodVariableAccess.REFERENCE.loadFromIndex(0);
return new StackManipulation.Compound(
methodDescription.isStatic()
? StackManipulation.LegalTrivial.INSTANCE
: MethodVariableAccess.REFERENCE.loadFromIndex(0),
methodDescription.isConstructor()
? Duplication.SINGLE
: StackManipulation.LegalTrivial.INSTANCE
);
}

@Override
Expand Down
Expand Up @@ -6,6 +6,7 @@
import net.bytebuddy.dynamic.ClassLoadingStrategy;
import net.bytebuddy.dynamic.DynamicType;
import net.bytebuddy.dynamic.loading.ClassReloadingStrategy;
import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy;
import net.bytebuddy.instrumentation.*;
import net.bytebuddy.instrumentation.attribute.annotation.AnnotationDescription;
import net.bytebuddy.instrumentation.method.MethodDescription;
Expand All @@ -21,6 +22,7 @@
import net.bytebuddy.instrumentation.method.bytecode.stack.member.MethodReturn;
import net.bytebuddy.instrumentation.type.InstrumentedType;
import net.bytebuddy.instrumentation.type.TypeDescription;
import net.bytebuddy.modifier.Visibility;
import net.bytebuddy.pool.TypePool;
import net.bytebuddy.test.utility.JavaVersionRule;
import net.bytebuddy.test.utility.PrecompiledTypeClassLoader;
Expand All @@ -45,6 +47,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertNotEquals;
import static org.mockito.Mockito.mock;

public class ByteBuddyTutorialExamplesTest {
Expand Down Expand Up @@ -249,7 +252,7 @@ public void testFieldsAndMethodsMethodSuperCallExplicit() throws Exception {

@Test
@JavaVersionRule.Enforce(8)
public void testFieldsAndMethodMethodDefaultCall() throws Exception {
public void testFieldsAndMethodsMethodDefaultCall() throws Exception {
// This test differs from the tutorial by only conditionally expressing the Java 8 types.
ClassLoader classLoader = new PrecompiledTypeClassLoader(getClass().getClassLoader());
Object instance = new ByteBuddy(ClassFileVersion.JAVA_V8)
Expand All @@ -265,6 +268,20 @@ public void testFieldsAndMethodMethodDefaultCall() throws Exception {
assertThat(method.invoke(instance), is((Object) "foo"));
}

@Test
public void testFieldsAndMethodsExplicitMethodCall() throws Exception {
Object object = new ByteBuddy()
.subclass(Object.class, ConstructorStrategy.Default.NO_CONSTRUCTORS)
.defineConstructor(Arrays.<Class<?>>asList(int.class), Visibility.PUBLIC)
.intercept(MethodCall.invoke(Object.class.getDeclaredConstructor()))
.make()
.load(getClass().getClassLoader(), ClassLoadingStrategy.Default.WRAPPER)
.getLoaded()
.getDeclaredConstructor(int.class)
.newInstance(42);
assertNotEquals(Object.class, object.getClass());
}

@Test
public void testFieldsAndMethodsSuper() throws Exception {
MemoryDatabase loggingDatabase = new ByteBuddy()
Expand Down
Expand Up @@ -35,7 +35,7 @@ public class ConstructorStrategyDefaultTest {

@Before
public void setUp() throws Exception {
when(methodRegistry.prepend(any(MethodRegistry.LatentMethodMatcher.class),
when(methodRegistry.append(any(MethodRegistry.LatentMethodMatcher.class),
any(Instrumentation.class),
any(MethodAttributeAppender.Factory.class))).thenReturn(methodRegistry);
when(instrumentedType.getSupertype()).thenReturn(superType);
Expand All @@ -55,7 +55,7 @@ public void testImitateSuperTypeStrategy() throws Exception {
when(methodList.filter(isConstructor().<MethodDescription>and(isVisibleTo(instrumentedType)))).thenReturn(filteredMethodList);
assertThat(ConstructorStrategy.Default.IMITATE_SUPER_TYPE.extractConstructors(instrumentedType), is(filteredMethodList));
assertThat(ConstructorStrategy.Default.IMITATE_SUPER_TYPE.inject(methodRegistry, methodAttributeAppenderFactory), is(methodRegistry));
verify(methodRegistry).prepend(any(MethodRegistry.LatentMethodMatcher.class), any(Instrumentation.class), eq(methodAttributeAppenderFactory));
verify(methodRegistry).append(any(MethodRegistry.LatentMethodMatcher.class), any(Instrumentation.class), eq(methodAttributeAppenderFactory));
verifyNoMoreInteractions(methodRegistry);
verify(instrumentedType, atLeastOnce()).getSupertype();
verifyNoMoreInteractions(instrumentedType);
Expand All @@ -66,7 +66,7 @@ public void testImitateSuperTypePublicStrategy() throws Exception {
when(methodList.filter(isPublic().and(isConstructor()))).thenReturn(filteredMethodList);
assertThat(ConstructorStrategy.Default.IMITATE_SUPER_TYPE_PUBLIC.extractConstructors(instrumentedType), is(filteredMethodList));
assertThat(ConstructorStrategy.Default.IMITATE_SUPER_TYPE_PUBLIC.inject(methodRegistry, methodAttributeAppenderFactory), is(methodRegistry));
verify(methodRegistry).prepend(any(MethodRegistry.LatentMethodMatcher.class), any(Instrumentation.class), eq(methodAttributeAppenderFactory));
verify(methodRegistry).append(any(MethodRegistry.LatentMethodMatcher.class), any(Instrumentation.class), eq(methodAttributeAppenderFactory));
verifyNoMoreInteractions(methodRegistry);
verify(instrumentedType, atLeastOnce()).getSupertype();
verifyNoMoreInteractions(instrumentedType);
Expand All @@ -78,7 +78,7 @@ public void testDefaultConstructorStrategy() throws Exception {
when(filteredMethodList.size()).thenReturn(1);
assertThat(ConstructorStrategy.Default.DEFAULT_CONSTRUCTOR.extractConstructors(instrumentedType), is(filteredMethodList));
assertThat(ConstructorStrategy.Default.DEFAULT_CONSTRUCTOR.inject(methodRegistry, methodAttributeAppenderFactory), is(methodRegistry));
verify(methodRegistry).prepend(any(MethodRegistry.LatentMethodMatcher.class), any(Instrumentation.class), eq(methodAttributeAppenderFactory));
verify(methodRegistry).append(any(MethodRegistry.LatentMethodMatcher.class), any(Instrumentation.class), eq(methodAttributeAppenderFactory));
verifyNoMoreInteractions(methodRegistry);
verify(instrumentedType, atLeastOnce()).getSupertype();
verifyNoMoreInteractions(instrumentedType);
Expand Down
Expand Up @@ -61,7 +61,7 @@ protected <T> DynamicType.Loaded<T> instrument(Class<T> target,
FieldAttributeAppender.NoOp.INSTANCE,
MethodAttributeAppender.NoOp.INSTANCE,
ConstructorStrategy.Default.IMITATE_SUPER_TYPE)
.method(targetMethods).intercept(instrumentation)
.invokable(targetMethods).intercept(instrumentation)
.make()
.load(classLoader, ClassLoadingStrategy.Default.WRAPPER);
}
Expand Down
Expand Up @@ -15,8 +15,7 @@
import java.util.Arrays;
import java.util.List;

import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.*;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -47,7 +46,7 @@ public void testUnambiguousDefaultMethod() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
DefaultMethodCall.unambiguousOnly(),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD));
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(1));
Method method = loaded.getLoaded().getDeclaredMethod(FOO);
Expand Down Expand Up @@ -85,7 +84,7 @@ private void assertConflictChoice(Class<?> preferredInterface,
DynamicType.Loaded<?> loaded = instrument(Object.class,
DefaultMethodCall.prioritize(preferredInterfaces),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
preferredInterface, secondInterface);
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(1));
Method method = loaded.getLoaded().getDeclaredMethod(FOO);
Expand Down Expand Up @@ -127,7 +126,7 @@ public void testDeclaredAndImplementedMethod() throws Exception {
DynamicType.Loaded<?> loaded = instrument(classLoader.loadClass(SINGLE_DEFAULT_METHOD_CLASS),
DefaultMethodCall.unambiguousOnly(),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD));
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(1));
Method method = loaded.getLoaded().getDeclaredMethod(FOO);
Expand All @@ -151,7 +150,7 @@ public void testDeclaredAndImplementedAmbiguousMethodWithPreference() throws Exc
DynamicType.Loaded<?> loaded = instrument(classLoader.loadClass(SINGLE_DEFAULT_METHOD_CLASS),
DefaultMethodCall.prioritize(classLoader.loadClass(SINGLE_DEFAULT_METHOD)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD), classLoader.loadClass(CONFLICTING_INTERFACE));
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(1));
Method method = loaded.getLoaded().getDeclaredMethod(FOO);
Expand Down
Expand Up @@ -102,17 +102,17 @@ public void testInstanceMethodInvocationWithoutArguments() throws Exception {

@Test
public void testSuperConstructorInvocationWithoutArguments() throws Exception {
DynamicType.Loaded<SuperConstructorCall> loaded = instrument(SuperConstructorCall.class,
MethodCall.invoke(Object.class.getDeclaredConstructor()),
SuperConstructorCall.class.getClassLoader(),
DynamicType.Loaded<Object> loaded = instrument(Object.class,
MethodCall.invoke(Object.class.getDeclaredConstructor()).onSuper(),
Object.class.getClassLoader(),
isConstructor());
assertThat(loaded.getLoadedAuxiliaryTypes().size(), is(0));
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(0));
assertThat(loaded.getLoaded().getDeclaredConstructors().length, is(1));
assertThat(loaded.getLoaded().getDeclaredFields().length, is(0));
SuperConstructorCall instance = loaded.getLoaded().newInstance();
assertNotEquals(SuperConstructorCall.class, instance.getClass());
assertThat(instance, instanceOf(SuperConstructorCall.class));
Object instance = loaded.getLoaded().newInstance();
assertNotEquals(Object.class, instance.getClass());
assertThat(instance, instanceOf(Object.class));
}

@Test
Expand Down Expand Up @@ -365,7 +365,7 @@ public void testDefaultMethod() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodCall.invoke(classLoader.loadClass(SINGLE_DEFAULT_METHOD).getDeclaredMethod(FOO)).onDefault(),
classLoader,
ElementMatchers.not(isDeclaredBy(Object.class)),
ElementMatchers.isMethod().and(ElementMatchers.not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD));
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(1));
Method method = loaded.getLoaded().getDeclaredMethod(FOO);
Expand Down
Expand Up @@ -14,8 +14,7 @@
import java.lang.reflect.Method;
import java.util.concurrent.Callable;

import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.*;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -45,7 +44,7 @@ public void testRunnableDefaultCall() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(RunnableClass.class),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD));
Object instance = loaded.getLoaded().newInstance();
Method method = loaded.getLoaded().getMethod(FOO);
Expand All @@ -58,7 +57,7 @@ public void testCallableDefaultCall() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(CallableClass.class),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD));
Object instance = loaded.getLoaded().newInstance();
Method method = loaded.getLoaded().getMethod(FOO);
Expand All @@ -81,7 +80,7 @@ public void testExplicitDefaultCall() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(classLoader.loadClass(PREFERRING_INTERCEPTOR)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD), classLoader.loadClass(CONFLICTING_INTERFACE));
Object instance = loaded.getLoaded().newInstance();
Method method = loaded.getLoaded().getMethod(FOO);
Expand All @@ -94,7 +93,7 @@ public void testExplicitDefaultCallToOtherInterface() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(classLoader.loadClass(CONFLICTING_PREFERRING_INTERCEPTOR)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD), classLoader.loadClass(CONFLICTING_INTERFACE));
Object instance = loaded.getLoaded().newInstance();
Method method = loaded.getLoaded().getMethod(FOO);
Expand All @@ -117,7 +116,7 @@ public void testSerializableProxy() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(SerializationCheck.class),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(SINGLE_DEFAULT_METHOD));
Object instance = loaded.getLoaded().newInstance();
Method method = loaded.getLoaded().getMethod(FOO);
Expand Down
Expand Up @@ -9,8 +9,7 @@
import org.junit.Test;
import org.junit.rules.MethodRule;

import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.*;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

Expand Down Expand Up @@ -40,7 +39,7 @@ public void testDefaultInterface() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(classLoader.loadClass(DELEGATION_TARGET)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(DEFAULT_INTERFACE));
Object instance = loaded.getLoaded().newInstance();
assertThat(instance.getClass().getDeclaredMethod(FOO).invoke(instance), is((Object) (FOO + BAR)));
Expand All @@ -52,7 +51,7 @@ public void testNoDefaultInterface() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(DelegationNoDefaultInterfaceInterceptor.class),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
DelegationNoDefaultInterface.class);
DelegationNoDefaultInterface instance = (DelegationNoDefaultInterface) loaded.getLoaded().newInstance();
instance.foo();
Expand All @@ -64,7 +63,7 @@ public void testDefaultInterfaceSerializableProxy() throws Exception {
DynamicType.Loaded<?> loaded = instrument(Object.class,
MethodDelegation.to(classLoader.loadClass(DELEGATION_TARGET_SERIALIZABLE)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(DEFAULT_INTERFACE));
Object instance = loaded.getLoaded().newInstance();
assertThat(instance.getClass().getDeclaredMethod(FOO).invoke(instance), is((Object) (FOO + BAR)));
Expand Down
Expand Up @@ -13,8 +13,7 @@

import java.io.Serializable;

import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.not;
import static net.bytebuddy.matcher.ElementMatchers.*;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -122,7 +121,7 @@ public void testDefaultMethodExplicit() throws Exception {
MethodDelegation.to(classLoader.loadClass(DEFAULT_INTERFACE_TARGET_EXPLICIT))
.appendParameterBinder(Morph.Binder.install(Morphing.class)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(DEFAULT_INTERFACE));
Object instance = loaded.getLoaded().newInstance();
assertThat(instance.getClass().getDeclaredMethod(FOO, String.class)
Expand All @@ -136,7 +135,7 @@ public void testDefaultMethodImplicit() throws Exception {
MethodDelegation.to(classLoader.loadClass(DEFAULT_INTERFACE_TARGET_IMPLICIT))
.appendParameterBinder(Morph.Binder.install(Morphing.class)),
classLoader,
not(isDeclaredBy(Object.class)),
isMethod().and(not(isDeclaredBy(Object.class))),
classLoader.loadClass(DEFAULT_INTERFACE));
Object instance = loaded.getLoaded().newInstance();
assertThat(instance.getClass().getDeclaredMethod(FOO, String.class)
Expand Down

0 comments on commit 03a2df7

Please sign in to comment.