Clarify when to invoke MBR and MBW pipeline

[NicoNes]

Hi guys,

AFAIK the current spec does not clearly state when to invoke MBR and MBW pipeline and this leads to situation like following:

Consider the given resource and providers below registered in a JAX-RS application:

@Path("test")
public class TestResource {
	@GET
	public Response getMethodWithEntity(Object entity) {
		return Response.ok().build();
	}
}

public class TestFilter implements ContainerRequestFilter {

	public static final String HAS_ENTITY = "Has entity";

	@Override
	public void filter(ContainerRequestContext containerRequestContext ) throws IOException {
		requestContext.setProperty(HAS_ENTITY, containerRequestContext .hasEntity());
	}
}

public class TestReaderInterceptor implements ReaderInterceptor {

	@Override
	public Object aroundReadFrom(ReaderInterceptorContext context) throws
 IOException, WebApplicationException {
		boolean hasEntity = (boolean) context.getProperty(TestFilter.HAS_ENTITY);
		if (!hasEntity) {
			System.out.println("Invocation of ReaderInterceptor even if there is 
no request entity to read");
		}
		return context.proceed();
	}
}

public class TestMBR implements MessageBodyReader<Object> {
	@Override
	public boolean isReadable(Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType) {
		return true;
	}

	@Override
	public Object readFrom(Class<Object> type, Type genericType, Annotation[] annotations, MediaType mediaType,
			MultivaluedMap<String, String> httpHeaders, InputStream entityStream)
			throws IOException, WebApplicationException {
		return new Object();
	}
}

When calling the GET method of TestResource from my browser without sending no request body I was
not expecting the TestReaderInterceptor.aroundReadFrom() neither TestMBR.readFrom() to be invoked, but they are although there is no request body to process.

In contrast on the response side, the MBW pipeline is not invoked when there is no response body.

This behavior is the same whatever you are doing a GET, POST, PUT. I did it in both the RI and RESTEASY

So what I suggest is to clearly state that both MBR and MBW pipeline MUST be invoked only when there is a request/response body to process.
In other words when no entity is identified in previous filter chain step, containerRequestContext.hasEntity()/containerResponseContext.hasEntity() returns false, MBR/MBW MUST not be invoked.

WDYT? Do you think it is acceptable ?

This PR follows up dicussion started here #659 (comment)

[chkal]

Sorry for the delayed response.

Not invoking the MBW pipeline if there is no response entity sounds reasonable to me. Especially because the responsible MBW can only be determined if you have an entity class. I'm +1 for making this more explicit by mentioning it in the MBW API docs.

The MBR case is more difficult. For the MBR one could decide whether to invoke the pipeline either by the presence of a request body sent by the client OR by the presence of an entity parameter (or @FormParam) on the matched resource method. I'm not sure if the spec should enforce any specific handling here, especially because I see good arguments for both ways.

Other thoughts?

[NicoNes]

Hi @chkal,

either by the presence of a request body sent by the client

In the case where a request body is sent by the client and there is no entityParemeter (or @FormParam) on the matched resource as follow, why should we invoke the MBR ? What will we do with the read entity ?

@Path("test")
public class TestResource {
	@GET (or @PUT whatever)
	public Response getMethodWithEntity() {
		return Response.ok().build();
	}
}

OR by the presence of an entity parameter (or @FormParam) on the matched resource method

So according to me MBR, should only be invoked in this case. I mean when user explicitly express that he cares about the request body by adding entity parameter or @FormParam on the matched resource method.
Now in this case the question is : is this condition enougth to invoke MBR ? or should we also be sure that a request body exist before invoking MBR ?
At first sight I would say that a first check should be performed to check if the request body exists (containerRequestContext.hasEntity()) before invoking MBR. Because for me if there is no stream to read no need to read it.
In this situation the entity parameter or @FormParam on the resource method they should be injected with null except if a default value is specified using @DefaultValue in case of @FormParam.

Which uses cases are you thinkng about ?

[chkal]

either by the presence of a request body sent by the client

In the case where a request body is sent by the client and there is no entityParemeter (or @FormParam) on the matched resource as follow, why should we invoke the MBR ? What will we do with the read entity ?

Agreed. Actually the JAX-RS implementation knows if the matched resource method declares an entity parameter or if there is any @FormParam involved. And if neither is the case, the MBR isn't really required as the resulting entity isn't used any anyway. The only thing that COULD happen is that there is a ReaderInterceptor which wants to intercept the incoming entity stream (for logging, auditing or whatever) which wouldn't be invoked in this case.

Now in this case the question is : is this condition enougth to invoke MBR ? or should we also be sure that a request body exist before invoking MBR ?

IMO it would make sense to invoke the MBR even if there is no request body, because a ReaderInterceptor could use ReaderInterceptorContext.setInputStream() to "create" an entity stream. So you don't really know if the MBR will get a stream to read from before invoking the pipeline.

[NicoNes]

IMO it would make sense to invoke the MBR even if there is no request body, because a ReaderInterceptor could use ReaderInterceptorContext.setInputStream() to "create" an entity stream

I get you point.

But in the case where I'm not interested in "creating" entity stream when there is no request body, the MBR will be invoked and will just add "extra treament" for nothing right ?

IMO the best option for user to "create" entity stream when there is no request body is to do it from the filter chain using "ContainerRequestContext.setEntityStream()". Then the MBR will be invoked after that normally with something to process.
This way both MBW and MBR pipeline are consistents and only invoked if there is something to process and not everytime.

WDYT ?

[chkal]

But in the case where I'm not interested in "creating" entity stream when there is no request body, the MBR will be invoked and will just add "extra treament" for nothing right ?

Correct.

IMO the best option for user to "create" entity stream when there is no request body is to do it from the filter chain using "ContainerRequestContext.setEntityStream()". Then the MBR will be invoked after that normally with something to process.

Agreed. This would certainly be a better option. I just wanted to mention that in theory people could use the ReaderInterceptor API this way.

@eclipse-ee4j/eclipse-jaxrs Any other thoughts?

[spericas]

I think a MBR should be called if there is an entity param (or @FormParam), regardless of how the request looks like. If a resource expects an entity, its corresponding MBR (and filter chain) should be given the chance to produce one, whether it's read from an inputstream or defaulted in some other way.

If there isn't an entity param, then I would understand why calling a MBR does not make much sense. I understand that this behavior is different for requests and responses, but that asymmetry is inherent to a client-server architecture and the reason why we have different types of filters, different APIs, etc.

[NicoNes]

If a resource expects an entity, its corresponding MBR (and filter chain) should be given the chance to produce one, whether it's read from an inputstream or defaulted in some other way.

As I was saying to @chkal, I understand this point of giving a chance to user to produce one.
But in the case where I'm not interested in "creating" entity when there is no request body, the MBR will be invoked and will just add "extra treament" for nothing: MBR selection process, execution...

So in order not to pay this extra cost for nothing, the best solution will be to let user to create/modify entity at filtering step, since this step will always be invoked whether there is a request entity or not. And then if the entity exists invoke MBR as expected.

WDYT ? Do you see any case where entity creation should/could not be done at filtering step and sould only be done in MBR ?

[spericas]

@NicoNes I don't believe the two cases are equivalent. MBRs and their interceptors are typed while filters operate on byte streams: providing a default would be a lot easier using the MBR/interceptor chain.
Frankly, this whole discussion seems a bit pointless to me, the use case we are trying to optimize is very narrow in practice; at this point the discussion seems more academic than practical.

[NicoNes]

@spericas Yes for sure it's more academic than practical I agree.
Can we please at least be more specific in the spec about when MBR are invoked saying that its is invoked no matter of the HTTP method or the request body availability. Only the enty param or @FormParam presence matter.

[spericas]

@NicoNes Yes, we can certainly do that. If you have a suggestion as how/where to do this based on your analysis, please provide it here. We can target this for 2.2.

[chkal]

I was thinking about simply adding some statements like this to the MBW/MBR API docs:

The MessageBodyReader pipeline is executed if the matching resource method declares an entity parameter or uses at least one @FormParam.

The MessageBodyWriter pipeline is only invoked if there is a response entity.

[spericas]

@chkal LGTM

[NicoNes]

I was looking at the Http RFCs trying to see if there is something preventing us from handling request body depending on the HTTP method we are using.
It appears that request body is only forbidden for TRACE method. So my question is: should we enforce a check in JAX-RS for that case given what we said before ? Or is it more the responsability of the underlying HTTP server to perform this check ?
I personnaly thinks it's should be checked by the HTTP server.

Then if we all agree about that, @chkal statements looks good to me 👍

[chkal]

So my question is: should we enforce a check in JAX-RS for that case given what we said before ? Or is it more the responsability of the underlying HTTP server to perform this check ?

I don't think that this should be enforced on our side.

BTW: I just open #744 for adjusting the MBW/MBR API docs. Please let me know what you think.

[spericas]

I agree with @chkal. No need to enforce that.

[mkarg]

@spericas PR is merged. Can we close this issue?

[mkarg]

Shall be backport this clarification into 2.1.x?

[chkal]

@mkarg +1 for closing. And IMO we could backport it.

[spericas]

@mkarg Yes on closing and backporting.

[mkarg]

Ok, so I put it on my backport stack.