Make `MvcResultAssert.getResponse()` public about spring-framework

[snicoll]

Shouldn’t you use satisfies for this?

[odrotbohm]

I need to return the result from a method.

[snicoll]

Ok. An assert object is not meant to expose anything. You can assign the result of perform and get the response from there. Would that work?

[odrotbohm]

Yes, that works and is what I do as a workaround. I assume that other trying to migrate their existing assertion pipelines will run into this as well.

Furthermore, getResponse() is already present, but protected. So I wonder why it should be fine to call that method on the object returned from ….andExpect(…) but not the assertion object.

[snicoll]

It doesn’t matter that the method already exists. As I’ve tried to explain, we’re following what AssertJ does and they don’t expose their internal state with a getter like that (and they have them protected just because the hierarchy needs access to it).

Mixing AssetJ with returning an element of the result and not willing to assign the result doesn’t look like a common case to me. But perhaps I am missing something.

[odrotbohm]

We’re following what AssertJ does and they don’t expose their internal state with a getter like that (and they have them protected just because the hierarchy needs access to it).

Well, we're extending AssertJ. Which naturally means adding stuff to the thing that's extended, isn't it? So, AssertJ not doing something we do is by definition, isn't it? Furthermore, that internal state already is exposed on AssertableMvcResult. There's nothing to be hidden, really. What do we gain from asking developers to write boilerplate code in this particular case? That we differentiate what an AbstractAssert is allowed to expose VS. an AssertProvider?

Mixing AssertJ with returning an element of the result and not willing to assign the result doesn’t look like a common case to me.

Not willing sounds stubborn. I just think it would be nice to return the result of such an operation immediately, without having to capture some state initially solely to obtain the thing I want to return from it later. It's already possible on MockMvc: define the request, chain expectations, return an aspect of the result. I don't see how anything in AssertJ is deeming that problematic non-idiomatic.

Using the response of a request is pretty common, as soon as you move beyond one-line examples and have test methods that execute slightly more complex testing flows that span multiple requests. Find an example of that in Spring RESTBucks.

[snicoll]

Well, we're extending AssertJ. Which naturally means adding stuff to the thing that's extended, isn't it?

Yes, just like any custom Assert, including the ones provided by AssertJ itself. The point I was trying to make is that we want the user experience to be natural and consistent with other Assert classes. And an Assert either returns SELF or a more dedicated assert instance based on an extracting function of some kind. It usually doesn't have a termination method that returns actual or some state of actual.

Furthermore, that internal state already is exposed on AssertableMvcResult

Again, that doesn't matter. And to be clear, I am not denying the use case here, just that it has to be fixed by making a method public, because it "is already present". I've already noticed a use case with async request where you want to first make a request and then get the response to trigger another perform. Your sample takes that to the next level with chain of processing that each take the previous response and contribute the one used by the next. I wonder if we shouldn't make that use case first class by providing a dedicated method. We'd need to hack a bit more to see how we'd name that though.

WDYT?

[odrotbohm]

In the end, I, personally, am more concerned about our users' ability to write boilerplate-free code. IMO, what is or is not deemed idiomatic should be of lower priority. Especially, as in this case, the absence of a certain style of method is equated with being non-idiomatic, which, I think, is not the same thing. But I've already argued that in a multitude of ways.

Find the translation of the Hamcrest-based definition of matchers to the AssertJ ones in Spring RESTBucks here. Note how, on multiple occasions (more here and here), this causes what could have been expressed in a single, fluent statement now has to be broken up into multiple expressions.

[snicoll]

Thanks for sharing the migration.

this causes what could have been expressed in a single, fluent statement now has to be broken up into multiple expressions.

I am sorry but I don't see how this has to be a must have. Doing so would mean you'd have to write something like return assertThat(mvc.perform(...) and that would read very strangely indeed.

I think you've done that by accident actually, calling result what is actually an assert object.

I am going to close this now, but I am keeping the use case of producing another request based on a previous response in mind as I've shared in my previous comment.