Skip to content

Commit

Permalink
Fixes ClassCastException when using assertion on SortedSet and extrac…
Browse files Browse the repository at this point in the history 
…ting method (#1242)
  • Loading branch information
@joel-costigliola
joel-costigliola committed Jun 2, 2018
1 parent 6bdbbb5 commit 1bd10d9
Show file tree
Hide file tree
Showing 5 changed files with 1,081 additions and 29 deletions.
47 changes: 31 additions & 16 deletions src/main/java/org/assertj/core/api/AbstractIterableAssert.java
View file
Expand Up @@ -38,6 +38,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
Expand All @@ -53,7 +54,6 @@
import org.assertj.core.api.iterable.ThrowingExtractor;
import org.assertj.core.condition.Not;
import org.assertj.core.description.Description;
import org.assertj.core.extractor.Extractors;
import org.assertj.core.groups.FieldsOrPropertiesExtractor;
import org.assertj.core.groups.Tuple;
import org.assertj.core.internal.CommonErrors;
Expand Down Expand Up @@ -835,7 +835,7 @@ public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object
List<Object> values = FieldsOrPropertiesExtractor.extract(actual, byName(propertyOrField));
String extractedDescription = extractedDescriptionOf(propertyOrField);
String description = mostRelevantDescription(info.description(), extractedDescription);
return newListAssertInstance(values).withAssertionState(myself).as(description);
return newListAssertInstanceForMethodsChangingElementType(values).as(description);
}

/**
Expand Down Expand Up @@ -883,7 +883,7 @@ public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object
List<Object> values = FieldsOrPropertiesExtractor.extract(actual, resultOf(method));
String extractedDescription = extractedDescriptionOfMethod(method);
String description = mostRelevantDescription(info.description(), extractedDescription);
return newListAssertInstance(values).withAssertionState(myself).as(description);
return newListAssertInstanceForMethodsChangingElementType(values).as(description);
}

/**
Expand Down Expand Up @@ -934,7 +934,7 @@ public <P> AbstractListAssert<?, List<? extends P>, P, ObjectAssert<P>> extracti
List<P> values = (List<P>) FieldsOrPropertiesExtractor.extract(actual, resultOf(method));
String extractedDescription = extractedDescriptionOfMethod(method);
String description = mostRelevantDescription(info.description(), extractedDescription);
return newListAssertInstance(values).withAssertionState(myself).as(description);
return newListAssertInstanceForMethodsChangingElementType(values).as(description);
}

/**
Expand Down Expand Up @@ -1025,7 +1025,7 @@ public <P> AbstractListAssert<?, List<? extends P>, P, ObjectAssert<P>> extracti
List<P> values = (List<P>) FieldsOrPropertiesExtractor.extract(actual, byName(propertyOrField));
String extractedDescription = extractedDescriptionOf(propertyOrField);
String description = mostRelevantDescription(info.description(), extractedDescription);
return newListAssertInstance(values).withAssertionState(myself).as(description);
return newListAssertInstanceForMethodsChangingElementType(values).as(description);
}

/**
Expand Down Expand Up @@ -1117,7 +1117,7 @@ public AbstractListAssert<?, List<? extends Tuple>, Tuple, ObjectAssert<Tuple>>
List<Tuple> values = FieldsOrPropertiesExtractor.extract(actual, byName(propertiesOrFields));
String extractedDescription = extractedDescriptionOf(propertiesOrFields);
String description = mostRelevantDescription(info.description(), extractedDescription);
return newListAssertInstance(values).withAssertionState(myself).as(description);
return newListAssertInstanceForMethodsChangingElementType(values).as(description);
}

/**
Expand Down Expand Up @@ -1163,7 +1163,7 @@ public AbstractListAssert<?, List<? extends Tuple>, Tuple, ObjectAssert<Tuple>>
@CheckReturnValue
public <V> AbstractListAssert<?, List<? extends V>, V, ObjectAssert<V>> extracting(Extractor<? super ELEMENT, V> extractor) {
List<V> values = FieldsOrPropertiesExtractor.extract(actual, extractor);
return newListAssertInstance(values).withAssertionState(myself);
return newListAssertInstanceForMethodsChangingElementType(values);
}

/**
Expand Down Expand Up @@ -1209,6 +1209,20 @@ public <V> AbstractListAssert<?, List<? extends V>, V, ObjectAssert<V>> extracti
@CheckReturnValue
public <V, EXCEPTION extends Exception> AbstractListAssert<?, List<? extends V>, V, ObjectAssert<V>> extracting(ThrowingExtractor<? super ELEMENT, V, EXCEPTION> extractor) {
List<V> values = FieldsOrPropertiesExtractor.extract(actual, extractor);
return newListAssertInstanceForMethodsChangingElementType(values);
}

/**
* Should be used after any methods changing the elements type like {@link #extracting(Extractor)} as it will propagate the correct
* assertions state, that is everyting but the element comparator (since the element type has changed).
*/
private <V> AbstractListAssert<?, List<? extends V>, V, ObjectAssert<V>> newListAssertInstanceForMethodsChangingElementType(List<V> values) {
if (actual instanceof SortedSet) {
// Reset the natural element comparator set when building an iterable assert instance for a SortedSet as it is likely not
// compatible with extracted values type, example with a SortedSet<Person> using a comparator on the Person's age, after
// extracting names we get a a List<String> which is mot suitable for the age comparator
usingDefaultElementComparator();
}
return newListAssertInstance(values).withAssertionState(myself);
}

Expand Down Expand Up @@ -1297,8 +1311,10 @@ public <V, EXCEPTION extends Exception> AbstractListAssert<?, List<? extends V>,
}

private <V> AbstractListAssert<?, List<? extends V>, V, ObjectAssert<V>> doFlatExtracting(Extractor<? super ELEMENT, ? extends Collection<V>> extractor) {
List<V> result = FieldsOrPropertiesExtractor.extract(actual, extractor).stream().flatMap(Collection::stream).collect(toList());
return newListAssertInstance(result).withAssertionState(myself);
List<V> result = FieldsOrPropertiesExtractor.extract(actual, extractor).stream()
.flatMap(Collection::stream)
.collect(toList());
return newListAssertInstanceForMethodsChangingElementType(result);
}

/**
Expand Down Expand Up @@ -1332,7 +1348,7 @@ public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object
List<Object> result = actualStream.flatMap(element -> Stream.of(extractors)
.map(extractor -> extractor.extract(element)))
.collect(Collectors.toList());
return newListAssertInstance(result).withAssertionState(myself);
return newListAssertInstanceForMethodsChangingElementType(result);
}

/**
Expand Down Expand Up @@ -1376,7 +1392,7 @@ public <EXCEPTION extends Exception> AbstractListAssert<?, List<? extends Object
List<Object> result = actualStream.flatMap(element -> Stream.of(extractors)
.map(extractor -> extractor.extract(element)))
.collect(Collectors.toList());
return newListAssertInstance(result).withAssertionState(myself);
return newListAssertInstanceForMethodsChangingElementType(result);
}

/**
Expand Down Expand Up @@ -1428,7 +1444,7 @@ public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object
CommonErrors.wrongElementTypeForFlatExtracting(group);
}
}
return newListAssertInstance(extractedValues).withAssertionState(myself);
return newListAssertInstanceForMethodsChangingElementType(extractedValues);
}

/**
Expand Down Expand Up @@ -1488,7 +1504,7 @@ public AbstractListAssert<?, List<? extends Tuple>, Tuple, ObjectAssert<Tuple>>
.toArray());
List<Tuple> tuples = stream(actual.spliterator(), false).map(tupleExtractor)
.collect(toList());
return newListAssertInstance(tuples).withAssertionState(myself);
return newListAssertInstanceForMethodsChangingElementType(tuples);
}

/**
Expand Down Expand Up @@ -1516,11 +1532,10 @@ public AbstractListAssert<?, List<? extends Tuple>, Tuple, ObjectAssert<Tuple>>
*/
@CheckReturnValue
public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object>> flatExtracting(String... fieldOrPropertyNames) {
List<Object> extractedValues = FieldsOrPropertiesExtractor.extract(actual, Extractors.byName(fieldOrPropertyNames))
.stream()
List<Object> extractedValues = FieldsOrPropertiesExtractor.extract(actual, byName(fieldOrPropertyNames)).stream()
.flatMap(tuple -> tuple.toList().stream())
.collect(toList());
return newListAssertInstance(extractedValues).withAssertionState(myself);
return newListAssertInstanceForMethodsChangingElementType(extractedValues);
}

/**
Expand Down
36 changes: 23 additions & 13 deletions src/main/java/org/assertj/core/api/AbstractObjectAssert.java
View file
Expand Up @@ -13,6 +13,7 @@
package org.assertj.core.api;

import static java.util.Objects.requireNonNull;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.description.Description.mostRelevantDescription;
import static org.assertj.core.extractor.Extractors.byName;
import static org.assertj.core.extractor.Extractors.extractedDescriptionOf;
Expand All @@ -26,6 +27,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.assertj.core.api.iterable.Extractor;
import org.assertj.core.description.Description;
import org.assertj.core.groups.Tuple;
import org.assertj.core.internal.TypeComparators;
Expand Down Expand Up @@ -218,7 +220,7 @@ public SELF isEqualToIgnoringGivenFields(Object other, String... propertiesOrFie
* @return {@code this} assertion object.
* @throws AssertionError if the actual object is {@code null}.
* @throws AssertionError if some fields or properties of the actual object are null.
*
*
* @since 2.5.0 / 3.5.0
*/
public SELF hasNoNullFieldsOrProperties() {
Expand Down Expand Up @@ -249,7 +251,7 @@ public SELF hasNoNullFieldsOrProperties() {
* @return {@code this} assertion object.
* @throws AssertionError if the actual object is {@code null}.
* @throws AssertionError if some (non ignored) fields or properties of the actual object are null.
*
*
* @since 2.5.0 / 3.5.0
*/
public SELF hasNoNullFieldsOrPropertiesExcept(String... propertiesOrFieldsToIgnore) {
Expand Down Expand Up @@ -310,8 +312,8 @@ protected TypeComparators getComparatorsByType() {
* <p>
* The comparators specified by this method are only used for field by field comparison like {@link #isEqualToComparingFieldByField(Object)}.
* <p>
* When used with {@link #isEqualToComparingFieldByFieldRecursively(Object)}, the fields/properties must be specified from the root object,
* for example if Foo class as a Bar field and Bar class has an id, to set a comparator for Bar's id use {@code "bar.id"}.
* When used with {@link #isEqualToComparingFieldByFieldRecursively(Object)}, the fields/properties must be specified from the root object,
* for example if Foo class as a Bar field and Bar class has an id, to set a comparator for Bar's id use {@code "bar.id"}.
* <p>
* Example:
* <pre><code class='java'> public class TolkienCharacter {
Expand Down Expand Up @@ -402,7 +404,7 @@ public <T> SELF usingComparatorForFields(Comparator<T> comparator, String... pro
* assertThat(frodo).usingComparatorForType(closeEnough, Double.class)
* .isEqualToComparingFieldByField(reallyTallFrodo);</code></pre>
*
* If multiple compatible comparators have been registered for a given {@code type}, the closest in the inheritance
* If multiple compatible comparators have been registered for a given {@code type}, the closest in the inheritance
* chain to the given {@code type} is chosen in the following order:
* <ol>
* <li>The comparator for the exact given {@code type}</li>
Expand Down Expand Up @@ -524,7 +526,7 @@ public SELF hasFieldOrPropertyWithValue(String name, Object value) {
* <p>
* Private fields can be extracted unless you call {@link Assertions#setAllowExtractingPrivateFields(boolean) Assertions.setAllowExtractingPrivateFields(false)}.
* <p>
* If the object under test is a {@link Map} with {@link String} keys, extracting will extract values matching the given fields/properties.
* If the object under test is a {@link Map} with {@link String} keys, extracting will extract values matching the given fields/properties.
* <p>
* Example:
* <pre><code class='java'> // Create frodo, setting its name, age and Race (Race having a name property)
Expand Down Expand Up @@ -552,18 +554,18 @@ public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object
}

/**
* Use the given {@link Function}s to extract the values from the object under test into a list, this new list becoming
* the object under test.
* Use the given {@link Function}s to extract the values from the object under test into a list, this new list becoming
* the object under test.
* <p>
* If the given {@link Function}s extract the id, name and email values then the list will contain the id, name and email values
* If the given {@link Function}s extract the id, name and email values then the list will contain the id, name and email values
* of the object under test, you can then perform list assertions on the extracted values.
* <p>
* Example:
* <pre><code class='java'> // Create frodo, setting its name, age and Race (Race having a name property)
* TolkienCharacter frodo = new TolkienCharacter(&quot;Frodo&quot;, 33, HOBBIT);
*
*
* // let's verify Frodo's name, age and race name:
* assertThat(frodo).extracting(TolkienCharacter::getName,
* assertThat(frodo).extracting(TolkienCharacter::getName,
* character -&gt; character.age, // public field
* character -&gt; character.getRace().getName())
* .containsExactly(&quot;Frodo&quot;, 33, "Hobbit");</code></pre>
Expand All @@ -581,6 +583,14 @@ public AbstractListAssert<?, List<? extends Object>, Object, ObjectAssert<Object
return newListAssertInstance(values).as(info.description());
}

public AbstractObjectAssert<?, ?> extracting(Extractor<? super ACTUAL, Object> extractor) {
return null;
}

public void should_testName() {
assertThat(new Object()).extracting(s -> s);
}

/**
* Assert that the object under test (actual) is equal to the given object based on recursive a property/field by property/field comparison (including
* inherited ones). This can be useful if actual's {@code equals} implementation does not suit you.
Expand Down Expand Up @@ -657,13 +667,13 @@ public SELF isEqualToComparingFieldByFieldRecursively(Object other) {
}

/**
* Verify that the object under test returns the given expected value from the given {@link Function},
* Verify that the object under test returns the given expected value from the given {@link Function},
* a typical usage is to pass a method reference to assert object's property.
* <p>
* Wrapping the given {@link Function} with {@link Assertions#from(Function)} makes the assertion more readable.
* <p>
* Example:
* <pre><code class="java"> // from is not mandatory but it makes the assertions more readable
* <pre><code class="java"> // from is not mandatory but it makes the assertions more readable
* assertThat(frodo).returns("Frodo", from(TolkienCharacter::getName))
* .returns("Frodo", TolkienCharacter::getName) // no from :(
* .returns(HOBBIT, from(TolkienCharacter::getRace));</code></pre>
Expand Down

0 comments on commit 1bd10d9

Please sign in to comment.