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
AssertFactory with the ability to convert the value upfront #3368
Comments
Could you help me understand your example better by adding which method returns (or would return) what? E.g.: assertThat(mvc.perform(get("/family"))) // dedicated Assert
.body().jsonPath()
.extractingPath("$.familyMembers[0]").convertTo(Member.class) // AbstractObjectAssert
.satisfies(member -> assertThat(member.name).isEqualTo("Homer")); or feel free to point me to an example in Spring MVC, if there is any.
I kept it package-private because I couldn't imagine a use case for having it exposed, but happy to evaluate it again. Could you elaborate on how |
You can also see a (different) example in the branch I am working on.
If |
That's exactly why I introduced an Do I understand correctly that you would like |
Looking at your branch, specifically: public <T> AbstractObjectAssert<?, T> convertTo(Class<T> target) {
isNotNull();
T value = convertToTargetType(target);
return Assertions.assertThat(value);
} due to type erasure, the compiler cannot identify a "better" candidate than To get type-specific assertions, the client code would need to specify the desired assertion in one way or another, similar to the Or did I understand something totally different? |
Now I think I understood what you mean. If public <ASSERT extends AbstractAssert<?, ?>> ASSERT convertTo(InstanceOfAssertFactory<?, ASSERT> factory) {
// `isNotNull()` implicitly done by `extracting`
return extracting(__ -> convertToTargetType(targetType), factory); // where to get `targetType`?
} as of today, there is no way to get Is that what you're looking for? |
@snicoll my 2cts here is: since you are dealing with JSON would a JSON assertion library be a better fit than AssertJ? I'm thinking of https://github.com/lukas-krecan/JsonUnit for example |
If I got @snicoll correctly, the pain point is actually after JSON handling and I can imagine it would circle back to AssertJ even with JsonUnit. Also, I guess it's related to spring-projects/spring-framework#21178. |
@scordio I've been going back and forth on my branch, but here is something that looks a bit like what I was after. It's far from perfect but I believe it should help understand the context.
In Spring Framework we have As for checking if I was wondering if I could get away with a default method on default ASSERT createAssert(Object actual, Function<Type, T> conversionFactory) {
T value = conversionFactory.apply(theType);
return createAssert(value);
} The problem now being, of course, that the existing implementations in @joel-costigliola we already have JSON Path and JSON Assert support in the Spring Framework that I am trying to integrate with AssertJ. One request was to be able to run assertions on a high-level DTO, rather that the raw JSON value. |
@snicoll I'm going back and forth through your changes, trying to understand how AssertJ might help. If you would put aside backward compatibility, what change or feature would you like to see in AssertJ to support your use case better? Is it summarized by what you initially wrote?
Also, about:
I'm still unsure if we should add conversion utilities to Should we maybe explore this direction only if we cannot come up with something better in the |
I don't think we should, but offering a way using an opt-in method would certainly be very much appreciated. In my example, AssertJ does not add support for conversion, but rather allow a callback so that something else can. To summarize my ask. AssertJ already has a number of out-ot-the-box features to represent various types that provides a dedicated assert object. If the feature is declined then, I have two choices:
|
I don't see why we should decline this request 🙂 Let me digest it a bit better and come back with a proposal. |
@snicoll could you point me to a test case that cannot work today because of the raw type usage in |
Sure, I should have done that from the very beginning: The example is contrived to show the status quo and what I would like to achieve. With The test case does not use any generics type but I can add another test if needs to be. I am not aware that assertJ has a way to refer to a type with generic information, such as the one from Spring Framework I am using there so it's more than just the extra method I've shared above. That said, with I should mention that I am happy to contribute based on your guidance if that can help. |
Very much appreciated 🙂 I keep digging to get a clearer idea of where we want to go and then we can discuss who will take up the changes! |
This is indeed a direction I'd like to investigate. I always had the impression that |
Nope. You need to be able to build a |
@snicoll I'm sketching a possible solution so it would help a lot to see a more complex case in your repo, if you have time for it. |
What do you need? A conversion for a type with generics? |
Yes, just to make sure I'm not composing something incompatible with what you would expect from users. |
I've added a commit (that does not compile) as providing the type and then a function to get the assertObject is not straightforward. However, relying on the existing built-in instances would be already very good so I don't know if that's a blocker. |
The direction I'm evaluating is without an opt-in method for value conversion, but making sure that the consumer code can apply arbitrary conversions before the Assuming that @Test
void convertToUsingInstanceOfAssertFactory() {
assertThat(forValue(123)).convertTo(InstanceOfAssertFactories.STRING).startsWith("1");
} and @Test
void convertToListOfObject() {
Customer[] customers = new Customer[] { new Customer("123", "John", "Smith", 42) };
assertThat(forValue(customers)).convertTo(InstanceOfAssertFactories.list(Customer.class))
.hasSize(1).contains(new Customer("123", "John", "Smith", 42));
}
public class CustomAssert extends AbstractObjectAssert<CustomAssert, Object> {
public CustomAssert(Object actual) {
super(actual, CustomAssert.class);
}
public <T, ASSERT extends AbstractAssert<?, ?>> ASSERT convertTo(InstanceOfAssertFactory<T, ASSERT> factory) {
return new ConversionServiceBasedAssertFactory<>(factory).createAssert(this.actual);
}
private static class ConversionServiceBasedAssertFactory<T, ASSERT extends AbstractAssert<?, ?>> implements AssertFactory<Object, ASSERT> {
// FIXME should be an interface instead of InstanceOfAssertFactory
private final InstanceOfAssertFactory<T, ASSERT> delegate;
private final ResolvableType resolvableType;
private ConversionServiceBasedAssertFactory(InstanceOfAssertFactory<T, ASSERT> delegate) {
this.delegate = delegate;
this.resolvableType = ResolvableType.forType(List.class); // FIXME should be delegate.getType()
}
@Override
public ASSERT createAssert(Object actual) {
return delegate.createAssert(convert(actual));
}
@SuppressWarnings("unchecked")
private T convert(Object actual) {
Object convert = DefaultConversionService.getSharedInstance().convert(
actual, new TypeDescriptor(resolvableType, null, null));
return (T) convert;
}
}
} This obviously is not enough for the third case you introduced: @Test
void convertToListOfObjectUsingGenericType() {
Customer[] customers = new Customer[] { new Customer("123", "John", "Smith", 42) };
assertThat(forValue(customers)).convertTo(/* something coming from AssertJ that supports parameterized type references? */)
.hasSize(1).contains(new Customer("123", "John", "Smith", 42));
} If it can solve this use case, I'm definitely keen to introduce something similar to |
Having that would help with the use case we're trying to implement.
It does not matter, I think. I was trying to show how to use |
My wish would be to at least enhance factories like As you also pointed out, today these factories know the raw type only. Ideally, However, would it make sense to have it public and also add new factories like This would allow users to write things like: Object actual = List.of(Set.of("a", "b"), Set.of("c", "d"));
ParameterizedTypeReference<Set<String>> elementTypeReference = new ParameterizedTypeReference<>() {};
assertThat(actual).asInstanceOf(list(elementTypeReference)).hasSize(2); where the a |
BTW over the weekend I'll polish what I have and push it so you can have a better idea of where I'm headed. |
# Conflicts: # assertj-core/src/test/java/org/assertj/core/api/InstanceOfAssertFactoriesTest.java
# Conflicts: # assertj-core/src/main/java/org/assertj/core/api/InstanceOfAssertFactories.java # assertj-core/src/test/java/org/assertj/core/api/InstanceOfAssertFactoriesTest.java
Feature summary
While working on the AssertJ support for Spring MVC, I've realized that assertJ does not provide a lot of conversion utilities for the actual value, which is more than fair, but can be helpful higher in the stack.
One of the support we have is for json path where you can craft an expression against the body result of an HTTP request and further assert the content. Our json path feature an "extractingPath" that is similar to AssertJ's extracting feature. But that operates on the raw value and we want to be able to assert that the raw JSON can be deserialized to a DTO.
Example
Let's take a silly little example with a family API that returns this:
Ignoring the setup, this is the pseudo code I am talking about:
This works but I am returning an
AbstractObjectAssert
against my DTO that represent a family member. What's I'd like to do is to express a type and, at the same time, provide a dedicatedAssert
implementation for it.InstanceOfAssertFactory
does what I need butgetType
is package private so I can't access it. Also I like the idea of having an interface for this concept and expose aType
so that generic types can be taken into account perhaps?Also
InstanceOfAssertFactory
carry the fact that the actual is expected to be of that type which, again, is totally fair but custom APIs may benefit from conversion as well.The text was updated successfully, but these errors were encountered: