[mockito] push by brice.du...@gmail.com - issue 353 - @InjectMocks behaved differently between Java 6 and 7. Als... on 2012-07-01 22:30 GMT

[moc...@googlecode.com]

Revision: f419a03d6f0d
Author: Brice@HQ
Date: Sun Jul 1 15:30:48 2012
Log: issue 353 - @InjectMocks behaved differently between Java 6 and
7. Also tweaked the property/field injection with a second pass to allow
injection in some edge cases.
http://code.google.com/p/mockito/source/detail?r=f419a03d6f0d

Added:

/test/org/mockito/internal/configuration/injection/FieldTypeAndNameComparatorTest.java

/test/org/mockitousage/bugs/Issue353InjectionMightNotHappenInCertainConfigurationTest.java
Modified:
/src/org/mockito/InjectMocks.java

/src/org/mockito/internal/configuration/injection/PropertyAndSetterInjection.java

=======================================
--- /dev/null
+++
/test/org/mockito/internal/configuration/injection/FieldTypeAndNameComparatorTest.java
Sun Jul 1 15:30:48 2012
@@ -0,0 +1,79 @@
+package org.mockito.internal.configuration.injection;
+
+import org.junit.Test;
+
+import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+@SuppressWarnings("unused")
+public class FieldTypeAndNameComparatorTest {
+
+ private Object objectA;
+ private Object objectB;
+
+ private Number numberA;
+ private Number numberB;
+
+ private Integer integerA;
+ private Integer integerB;
+
+ @Test
+ public void when_same_type_the_order_is_based_on_field_name() throws
Exception {
+ assertThat(new
PropertyAndSetterInjection.FieldTypeAndNameComparator().compare(field("objectA"),
field("objectB"))).isEqualTo(-1);
+ assertThat(new
PropertyAndSetterInjection.FieldTypeAndNameComparator().compare(field("objectB"),
field("objectA"))).isEqualTo(1);
+ assertThat(new
PropertyAndSetterInjection.FieldTypeAndNameComparator().compare(field("objectB"),
field("objectB"))).isEqualTo(0);
+ }
+
+ @Test
+ public void when_type_is_different_the_supertype_comes_last() throws
Exception {
+ assertThat(new
PropertyAndSetterInjection.FieldTypeAndNameComparator().compare(field("numberA"),
field("objectB"))).isEqualTo(-1);
+ assertThat(new
PropertyAndSetterInjection.FieldTypeAndNameComparator().compare(field("objectB"),
field("numberA"))).isEqualTo(1);
+ }
+
+ @Test
+ public void using_Collections_dot_sort() throws Exception {
+ List<Field> unsortedFields = Arrays.asList(
+ field("objectB"),
+ field("integerB"),
+ field("numberA"),
+ field("numberB"),
+ field("objectA"),
+ field("integerA")
+ );
+
+ Collections.sort(unsortedFields, new
PropertyAndSetterInjection.FieldTypeAndNameComparator());
+
+ assertThat(unsortedFields).containsSequence(
+ field("integerA"),
+ field("integerB"),
+ field("numberA"),
+ field("numberB"),
+ field("objectA"),
+ field("objectB")
+ );
+ }
+
+
+ @Test
+ public void issue_352_order_was_different_between_JDK6_and_JDK7()
throws Exception {
+ List<Field> unsortedFields = Arrays.asList(
+ field("objectB"),
+ field("objectA")
+ );
+
+ Collections.sort(unsortedFields, new
PropertyAndSetterInjection.FieldTypeAndNameComparator());
+
+ assertThat(unsortedFields).containsSequence(
+ field("objectA"),
+ field("objectB")
+ );
+ }
+
+ private Field field(String field) throws NoSuchFieldException {
+ return getClass().getDeclaredField(field);
+ }
+}
=======================================
--- /dev/null
+++
/test/org/mockitousage/bugs/Issue353InjectionMightNotHappenInCertainConfigurationTest.java
Sun Jul 1 15:30:48 2012
@@ -0,0 +1,34 @@
+package org.mockitousage.bugs;
+
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static org.fest.assertions.Assertions.assertThat;
+import static org.junit.Assert.assertSame;
+
+import java.util.*;
+
+@RunWith(MockitoJUnitRunner.class)
+public class Issue353InjectionMightNotHappenInCertainConfigurationTest {
+ @Mock Map<String, String> stringString_that_matches_field;
+ @Mock Map<String, Integer> mockStringInteger_was_not_injected;
+ @InjectMocks FooService fooService;
+
+ @Test
+ public void
when_identical_types_and_the_correct_mock_name_is_greater_than_the_non_matching_name_then_injection_occurs_only_on_the_named_one()
{
+
assertThat("stringString_that_matches_field".compareTo("mockStringInteger_was_not_injected")).isGreaterThanOrEqualTo(1);
+
+ assertSame(stringString_that_matches_field,
fooService.stringString_that_matches_field);
+ assertSame(mockStringInteger_was_not_injected,
fooService.stringInteger_field);
+ }
+
+ public static class FooService {
+ Map<String, Integer> stringInteger_field = new HashMap<String,
Integer>();
+ Map<String, String> stringString_that_matches_field = new
HashMap<String, String>();
+ }
+
+}
=======================================
--- /src/org/mockito/InjectMocks.java Mon May 7 11:56:48 2012
+++ /src/org/mockito/InjectMocks.java Sun Jul 1 15:30:48 2012
@@ -32,12 +32,16 @@
*
* <li><strong>Property setter injection</strong>; mocks will first be
resolved by type,
* then, if there is several property of the same type, by the match
of the property name and the mock name.
- * <p><u>Note:</u> If &#064;InjectMocks instance wasn't initialized
before and have a no-arg constructor,
+ * <p><u>Note 1:</u> If you have properties with the same type (or
same erasure), it's better to name all &#064;Mock
+ * annotated fields with the matching properties, otherwise Mockito
might get confused and injection won't happen.</p>
+ * <p><u>Note 2:</u> If &#064;InjectMocks instance wasn't initialized
before and have a no-arg constructor,
* then it will be initialized with this constructor.</p></li>
*
* <li><strong>Field injection</strong>; mocks will first be resolved
by type,
* then, if there is several property of the same type, by the match
of the field name and the mock name.
- * <p><u>Note:</u> If &#064;InjectMocks instance wasn't initialized
before and have a no-arg constructor,
+ * <p><u>Note 1:</u> If you have fields with the same type (or same
erasure), it's better to name all &#064;Mock
+ * annotated fields with the matching fields, otherwise Mockito might
get confused and injection won't happen.</p>
+ * <p><u>Note 2:</u> If &#064;InjectMocks instance wasn't initialized
before and have a no-arg constructor,
* then it will be initialized with this constructor.</p></li>
* </ol>
* </p>
=======================================
---
/src/org/mockito/internal/configuration/injection/PropertyAndSetterInjection.java
Sat Mar 10 17:08:10 2012
+++
/src/org/mockito/internal/configuration/injection/PropertyAndSetterInjection.java
Sun Jul 1 15:30:48 2012
@@ -26,18 +26,23 @@
* <u>Algorithm :<br></u>
* for each field annotated by @InjectMocks
* <ul>
- * <li>copy mocks set
* <li>initialize field annotated by @InjectMocks
- * <li>for each field in @InjectMocks type ordered from sub-type to
super-type
+ * <li>for each fields of a class in @InjectMocks type hierarchy
* <ul>
- * <li>find mock candidate by type
- * <li>if more than <b>*one*</b> candidate find mock candidate on name
- * <li>if one mock candidate then
- * <ul>
- * <li>set mock by property setter if possible
- * <li>else set mock by field injection
- * </ul>
- * <li>remove mock from mocks copy (mocks are just injected once)
+ * <li>make a copy of mock candidates
+ * <li>order fields rom sub-type to super-type, then by field name
+ * <li>for the list of fields in a class try two passes of :
+ * <ul>
+ * <li>find mock candidate by type
+ * <li>if more than <b>*one*</b> candidate find mock candidate
on name
+ * <li>if one mock candidate then
+ * <ul>
+ * <li>set mock by property setter if possible
+ * <li>else set mock by field injection
+ * </ul>
+ * <li>remove mock from mocks copy (mocks are just injected
once in a class)
+ * <li>remove injected field from list of class fields
+ * </ul>
* <li>else don't fail, user will then provide dependencies
* </ul>
* </ul>
@@ -51,20 +56,7 @@
public class PropertyAndSetterInjection extends MockInjectionStrategy {

private final MockCandidateFilter mockCandidateFilter = new
TypeBasedCandidateFilter(new NameBasedCandidateFilter(new
FinalMockCandidateFilter()));
- private Comparator<Field> superTypesLast = new Comparator<Field>() {
- public int compare(Field field1, Field field2) {
- Class<?> field1Type = field1.getType();
- Class<?> field2Type = field2.getType();
-
- if(field1Type.isAssignableFrom(field2Type)) {
- return 1;
- }
- if(field2Type.isAssignableFrom(field1Type)) {
- return -1;
- }
- return 0;
- }
- };
+ private Comparator<Field> superTypesLast = new
FieldTypeAndNameComparator();

private ListUtil.Filter<Field> notFinalOrStatic = new
ListUtil.Filter<Field>() {
public boolean isOut(Field object) {
@@ -73,8 +65,22 @@
};


- public boolean processInjection(Field field, Object fieldOwner,
Set<Object> mockCandidates) {
+ public boolean processInjection(Field injectMocksField, Object
injectMocksFieldOwner, Set<Object> mockCandidates) {
// Set<Object> mocksToBeInjected = new
HashSet<Object>(mockCandidates);
+ FieldInitializationReport report =
initializeInjectMocksField(injectMocksField, injectMocksFieldOwner);
+
+ // for each field in the class hierarchy
+ boolean injectionOccurred = false;
+ Class<?> fieldClass = report.fieldClass();
+ Object fieldInstanceNeedingInjection = report.fieldInstance();
+ while (fieldClass != Object.class) {
+ injectionOccurred |= injectMockCandidates(fieldClass,
newMockSafeHashSet(mockCandidates), fieldInstanceNeedingInjection);
+ fieldClass = fieldClass.getSuperclass();
+ }
+ return injectionOccurred;
+ }
+
+ private FieldInitializationReport initializeInjectMocksField(Field
field, Object fieldOwner) {
FieldInitializationReport report = null;
try {
report = new FieldInitializer(fieldOwner, field).initialize();
@@ -85,28 +91,28 @@
}
new
Reporter().cannotInitializeForInjectMocksAnnotation(field.getName(), e);
}
+ return report; // never null
+ }


- // for each field in the class hierarchy
+ private boolean injectMockCandidates(Class<?> awaitingInjectionClazz,
Set<Object> mocks, Object instance) {
boolean injectionOccurred = false;
- Class<?> fieldClass = report.fieldClass();
- Object fieldInstanceNeedingInjection = report.fieldInstance();
- while (fieldClass != Object.class) {
- injectionOccurred |= injectMockCandidate(fieldClass,
newMockSafeHashSet(mockCandidates), fieldInstanceNeedingInjection);
- fieldClass = fieldClass.getSuperclass();
- }
+ List<Field> orderedInstanceFields =
orderedInstanceFieldsFrom(awaitingInjectionClazz);
+ // pass 1
+ injectionOccurred |= injectMockCandidatesOnFields(mocks, instance,
injectionOccurred, orderedInstanceFields);
+ // pass 2
+ injectionOccurred |= injectMockCandidatesOnFields(mocks, instance,
injectionOccurred, orderedInstanceFields);
return injectionOccurred;
}

-
-
- private boolean injectMockCandidate(Class<?> awaitingInjectionClazz,
Set<Object> mocks, Object instance) {
- boolean injectionOccurred = false;
- for(Field field :
orderedInstanceFieldsFrom(awaitingInjectionClazz)) {
+ private boolean injectMockCandidatesOnFields(Set<Object> mocks, Object
instance, boolean injectionOccurred, List<Field> orderedInstanceFields) {
+ for (Iterator<Field> it = orderedInstanceFields.iterator();
it.hasNext(); ) {
+ Field field = it.next();
Object injected = mockCandidateFilter.filterCandidate(mocks,
field, instance).thenInject();
- if(injected != null) {
+ if (injected != null) {
injectionOccurred |= true;
mocks.remove(injected);
+ it.remove();
}
}
return injectionOccurred;
@@ -121,4 +127,22 @@
return declaredFields;
}

-}
+ static class FieldTypeAndNameComparator implements Comparator<Field> {
+ public int compare(Field field1, Field field2) {
+ Class<?> field1Type = field1.getType();
+ Class<?> field2Type = field2.getType();
+
+ // if same type, compares on field name
+ if (field1Type == field2Type) {
+ return field1.getName().compareTo(field2.getName());
+ }
+ if(field1Type.isAssignableFrom(field2Type)) {
+ return 1;
+ }
+ if(field2Type.isAssignableFrom(field1Type)) {
+ return -1;
+ }
+ return 0;
+ }
+ }
+}