AssertFactory with the ability to convert the value upfront

[snicoll]

AssertFactory with the ability to convert the value upfront

[snicoll]

@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.

due to type erasure, the compiler cannot identify a "better" candidate than assertThat(T), which is used as a fallback since T is unbounded.

In Spring Framework we have ParameterizedTypeReference that lets you provide T with generic information that works ultimately on a java.lang.reflect.Type. ValueType in this commit is already an InstanceOfAssertFactory but keeps the Type reference for doing the conversion. Later I've removed that class altogether as I wanted to get some feedback from you about the feature first.

As for checking if actual is indeed of type Type, there's a "shortcut" on doing an instanceOf on the raw type so this is definitely far from perfect. The actual conversion is responsible to figure out if it can produce the actual requested type. All that happens outside of AssertJ so I was trying to figure out something that is as minimal as possible that lets me reuse standard constructs.

I was wondering if I could get away with a default method on AssertFactory that I could call, something along the lines of:

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 InstanceOfAssertFactories only have the raw type...

@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.

[scordio]

I don't see why we should decline this request 🙂

Let me digest it a bit better and come back with a proposal.

[snicoll]

Sure, I should have done that from the very beginning:
https://github.com/snicoll-scratches/assertj-conversion

The example is contrived to show the status quo and what I would like to achieve. With InstanceOfAssertFactory, it can only be used when actual is of the requested type. ValueType in the project shows how it can be augmented to offer opt-in conversion.

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 InstanceOfAssertFactories providing shortcut for a number of cases, it could be easier to build that internally.

I should mention that I am happy to contribute based on your guidance if that can help.

[snicoll]

I assume it should be somehow compatible with what Spring already offers.

Nope. You need to be able to build a java.lang.reflect.Type programmatically with the relevant generic information. That's what Spring's ResolvableType does but we only need to Type information of the JDK. If you look at ParameterizedTypeReference in Spring, you can see another "cheap" way of doing this and get a Type.

[snicoll]

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.

[scordio]

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.

InstanceOfAssertFactory does what I need but getType is package private so I can't access it.

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 getType() would be consumed in the use case you have in mind?

[snicoll]

Could you help me understand your example better by adding which method returns (or would return) what? E.g.:

extractingPath returns an AbstractObjectAssert extension whose actual is the raw json value resolved by the given path. In the case above, this is a Map with a name attribute equals to Homer. We can use that as a source to rebuild the actual message.

convertTo(Class<T>) does the conversion and returns an AbstractObjectAssert<?, T>.

You can also see a (different) example in the branch I am working on.

Could you elaborate on how getType() would be consumed in the use case you have in mind?

If getType or anything that lets me know the type would be public I could then ask Spring's converter "please convert the input into that type. And once that is done, I can use the assertion object to get a dedicated object. The problem currently is that even if you provide a type that assertJ supports natively you always get an ObjectAssert of T which forces you to use satisfies if you want to get back dedicated assertions.

[scordio]

The problem currently is that even if you provide a type that assertJ supports natively you always get an ObjectAssert of T which forces you to use satisfies if you want to get back dedicated assertions.

That's exactly why I introduced an InstanceOfAssertFactory parameter instead of a Class one for asInstanceOf and the extracting variants, basically due to type erasure. Conceptually, the InstanceOfAssertFactory instance maps the expected type to the corresponding Assert subtype, information that the compiler can use to allow type-specific assertions.

Do I understand correctly that you would like convertTo(Class<T>) to return type-specific assertions depending on its Class parameter? How would you solve the type erasure issue?

[scordio]

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 assertThat(T), which is used as a fallback since T is unbounded.

To get type-specific assertions, the client code would need to specify the desired assertion in one way or another, similar to the asInstanceOf examples.

Or did I understand something totally different?

[scordio]

Now I think I understood what you mean. If convertTo would evolve as:

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 targetType without an additional parameter.

Is that what you're looking for?

[joel-costigliola]

@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

[scordio]

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]

@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?

I like the idea of having an interface for this concept and expose a Type so that generic types can be taken into account perhaps?

Also, about:

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.

I'm still unsure if we should add conversion utilities to AssertFactory, like the example in your previous message.

Should we maybe explore this direction only if we cannot come up with something better in the InstanceOfAssertFactory area? Or is it something that you would need anyway?

[snicoll]

I'm still unsure if we should add conversion utilities to AssertFactory, like the example in your previous message.

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. InstanceOfAssertFactories is the main one I have in mind. But this class works on the assumption that actual is of that type. It's ok and the current createAssert is totally fine. I am asking about another version where someone else could get a chance to convert the value before the assert object is created.

If the feature is declined then, I have two choices:

1. Do nothing and then users get a generic assert object (so in the case you ask to convert to a List<Member> you get an AbstractObjectAssert<List<Member>>)
2. Creating my own type, which means that I have to redo what InstanceOfAssertFactories does.

[scordio]

@snicoll could you point me to a test case that cannot work today because of the raw type usage in InstanceOfAssertFactories?

[scordio]

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!

[scordio]

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.

This is indeed a direction I'd like to investigate. I always had the impression that InstanceOfAssertFactory instances like list(Class) were a bit "weak"... maybe adding some support to types with generic information could be a way to go but I assume it should be somehow compatible with what Spring already offers.

[scordio]

The test case does not use any generics type but I can add another test if needs to be.

@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.

[snicoll]

What do you need? A conversion for a type with generics?

[scordio]

Yes, just to make sure I'm not composing something incompatible with what you would expect from users.

[scordio]

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 InstanceOfAssertFactory instance is invoked.

Assuming that InstanceOfAssertFactory offers a new Type getType() method (or, even better, there is an interface for it), to address:

@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));
}

CustomAssert could become:

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 ParameterizedTypeReference in AssertJ. I'll continue digging and keep you updated.

[snicoll]

Assuming that InstanceOfAssertFactory offers a new Type getType() method (or, even better, there is an interface for it), to address:

Having that would help with the use case we're trying to implement.

This obviously is not enough for the third case you introduced:

It does not matter, I think. I was trying to show how to use ParameterizedType but if you need to do something that InstanceOfAssertFactories does not provide already, then you'd probably better off creating a method anyway.

[scordio]

My wish would be to at least enhance factories like list(Class).

As you also pointed out, today these factories know the raw type only. Ideally, getType() should return the corresponding ParameterizedType instance instead. So far, it seems feasible in my draft but I'm currently keeping the AssertJ version of ParameterizedTypeReference as a package-private detail.

However, would it make sense to have it public and also add new factories like list(ParameterizedTypeReference)?

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 getType() call on list(elementTypeReference) would return the proper List<Set<String>> instance of ParameterizedType.

[scordio]

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.