From a3d57fd73ac3dc4288cc93b4e2677975d0820a98 Mon Sep 17 00:00:00 2001 From: Rafael Winterhalter Date: Thu, 19 May 2022 23:46:43 +0200 Subject: [PATCH] Reintroduce inheriting type annotations from interfaces if only one interface is mocked, including additional interfaces. Without this restriction, the first presented interface might determine the interfaces that are inherited by a subsequent mock that presents the interfaces in a different order. Also, it does not make semantic sense to decide on a particular interface to inherit annotations from. Fixes #2640. --- .../bytebuddy/SubclassBytecodeGenerator.java | 24 ++++-- .../SubclassByteBuddyMockMakerTest.java | 86 +++++++++++++++++-- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java b/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java index c9029924e0..717de9de39 100644 --- a/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java +++ b/src/main/java/org/mockito/internal/creation/bytebuddy/SubclassBytecodeGenerator.java @@ -222,24 +222,32 @@ public Class mockClass(MockFeatures features) { } } // Graal requires that the byte code of classes is identical what requires that interfaces - // are always - // defined in the exact same order. Therefore, we add an interface to the interface set if - // not mocking - // a class when Graal is active. + // are always defined in the exact same order. Therefore, we add an interface to the + // interface set if not mocking a class when Graal is active. @SuppressWarnings("unchecked") Class target = GraalImageCode.getCurrent().isDefined() && features.mockedType.isInterface() ? (Class) Object.class : features.mockedType; + // If we create a mock for an interface with additional interfaces implemented, we do not + // want to preserve the annotations of either interface. The caching mechanism does not + // consider the order of these interfaces and the same mock class might be reused for + // either order. Also, it does not have clean semantics as annotations are not normally + // preserved for interfaces in Java. + Annotation[] annotationsOnType; + if (features.stripAnnotations) { + annotationsOnType = new Annotation[0]; + } else if (!features.mockedType.isInterface() || features.interfaces.isEmpty()) { + annotationsOnType = features.mockedType.getAnnotations(); + } else { + annotationsOnType = new Annotation[0]; + } DynamicType.Builder builder = byteBuddy .subclass(target) .name(name) .ignoreAlso(BytecodeGenerator.isGroovyMethod(false)) - .annotateType( - features.stripAnnotations || features.mockedType.isInterface() - ? new Annotation[0] - : features.mockedType.getAnnotations()) + .annotateType(annotationsOnType) .implement( new ArrayList<>( GraalImageCode.getCurrent().isDefined() diff --git a/src/test/java/org/mockito/internal/creation/bytebuddy/SubclassByteBuddyMockMakerTest.java b/src/test/java/org/mockito/internal/creation/bytebuddy/SubclassByteBuddyMockMakerTest.java index 6e082287d1..e6a0086c9f 100644 --- a/src/test/java/org/mockito/internal/creation/bytebuddy/SubclassByteBuddyMockMakerTest.java +++ b/src/test/java/org/mockito/internal/creation/bytebuddy/SubclassByteBuddyMockMakerTest.java @@ -12,6 +12,7 @@ import org.mockito.internal.creation.MockSettingsImpl; import org.mockito.plugins.MockMaker; +import java.io.Serializable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.Observable; @@ -79,9 +80,8 @@ public void is_type_mockable_give_empty_reason_if_type_is_mockable() { } @Test - public void mock_type_with_annotations() throws Exception { - MockSettingsImpl mockSettings = - new MockSettingsImpl(); + public void mock_class_with_annotations() throws Exception { + MockSettingsImpl mockSettings = new MockSettingsImpl<>(); mockSettings.setTypeToMock(ClassWithAnnotation.class); ClassWithAnnotation proxy = mockMaker.createMock(mockSettings, dummyHandler()); @@ -102,10 +102,79 @@ public void mock_type_with_annotations() throws Exception { .isEqualTo("bar"); } + @Test + public void mock_class_with_annotations_with_additional_interface() throws Exception { + MockSettingsImpl mockSettings = new MockSettingsImpl<>(); + mockSettings.setTypeToMock(ClassWithAnnotation.class); + mockSettings.extraInterfaces(Serializable.class); + + ClassWithAnnotation proxy = mockMaker.createMock(mockSettings, dummyHandler()); + + assertThat(proxy.getClass().isAnnotationPresent(SampleAnnotation.class)).isTrue(); + assertThat(proxy.getClass().getAnnotation(SampleAnnotation.class).value()).isEqualTo("foo"); + + assertThat( + proxy.getClass() + .getMethod("sampleMethod") + .isAnnotationPresent(SampleAnnotation.class)) + .isTrue(); + assertThat( + proxy.getClass() + .getMethod("sampleMethod") + .getAnnotation(SampleAnnotation.class) + .value()) + .isEqualTo("bar"); + } + + @Test + public void mock_interface_with_annotations() throws Exception { + MockSettingsImpl mockSettings = new MockSettingsImpl<>(); + mockSettings.setTypeToMock(InterfaceWithAnnotation.class); + + InterfaceWithAnnotation proxy = mockMaker.createMock(mockSettings, dummyHandler()); + + assertThat(proxy.getClass().isAnnotationPresent(SampleAnnotation.class)).isTrue(); + assertThat(proxy.getClass().getAnnotation(SampleAnnotation.class).value()).isEqualTo("foo"); + + assertThat( + proxy.getClass() + .getMethod("sampleMethod") + .isAnnotationPresent(SampleAnnotation.class)) + .isTrue(); + assertThat( + proxy.getClass() + .getMethod("sampleMethod") + .getAnnotation(SampleAnnotation.class) + .value()) + .isEqualTo("bar"); + } + + @Test + public void mock_interface_with_annotations_with_additional_interface() throws Exception { + MockSettingsImpl mockSettings = new MockSettingsImpl<>(); + mockSettings.setTypeToMock(InterfaceWithAnnotation.class); + mockSettings.extraInterfaces(Serializable.class); + + InterfaceWithAnnotation proxy = mockMaker.createMock(mockSettings, dummyHandler()); + + assertThat(proxy.getClass().isAnnotationPresent(SampleAnnotation.class)).isFalse(); + + assertThat( + proxy.getClass() + .getMethod("sampleMethod") + .isAnnotationPresent(SampleAnnotation.class)) + .isTrue(); + assertThat( + proxy.getClass() + .getMethod("sampleMethod") + .getAnnotation(SampleAnnotation.class) + .value()) + .isEqualTo("bar"); + } + @Test public void mock_type_without_annotations() throws Exception { - MockSettingsImpl mockSettings = - new MockSettingsImpl(); + MockSettingsImpl mockSettings = new MockSettingsImpl<>(); mockSettings.setTypeToMock(ClassWithAnnotation.class); mockSettings.withoutAnnotations(); @@ -138,4 +207,11 @@ public void sampleMethod() { throw new UnsupportedOperationException(); } } + + @SampleAnnotation("foo") + public interface InterfaceWithAnnotation { + + @SampleAnnotation("bar") + void sampleMethod(); + } }