jsr369-experts@servlet-spec.java.net

[jsr369-experts] Re: [servlet-spec users] Re: [73-getMapping] PROPOSAL

From: Edward Burns <edward.burns_at_oracle.com>
Date: Thu, 16 Feb 2017 14:41:00 -0800

Hello Volunteers,

Thanks for your patience on this. This reply is culled from reviewing
the discussion on users_at_servlet-spec as well as on this list. Comments inline.

>>>>> On Wed, 30 Mar 2016 01:04:43 +0200, arjan tijms <arjan.tijms_at_gmail.com> said:

AT> @WebServlet({"/exact", "/path/*", "*.ext"})
AT> public class Servlet extends HttpServlet {
AT> private static final long serialVersionUID = 1L;

AT> protected void doGet(HttpServletRequest request, HttpServletResponse
AT> response) throws ServletException, IOException {
AT> ServletMapping mapping = request.getServletMapping();
AT> response.getWriter()
AT> .append("Mapping match:")
AT> .append(mapping.getMatchType().name())
AT> .append("\n")
AT> .append("Match value:")
AT> .append(mapping.getMatchValue())
AT> .append("\n")
AT> .append("Pattern:")
AT> .append(mapping.getPattern());
AT> }

AT> }

AT> Calling with http://localhost:8080/servlet4/bla.ext

AT> Prints:

AT> Mapping match:EXTENSION
AT> Match value:/bla

In my implementation in GlassFish I have this returning /bla.ext.

AT> Pattern:*.ext

AT> Calling with http://localhost:8080/servlet4/path/bla

AT> Prints:

AT> Mapping match:PATH
AT> Match value:/bla

GlassFish: /path/bla

AT> Pattern:/path/*

>>>>> On Mon, 4 Apr 2016 14:19:51 +0200, arjan tijms <arjan.tijms_at_gmail.com> said:

AT> I think we need to discuss the "implicit" mapping. As by my
AT> experiments,

[...]

On Wed, 6 Apr 2016 17:24:17 +0200 Arjan Tijms wrote:

AT> Maybe there's no need for it indeed. The Servlet spec mentioned the
AT> concept, so for completeness I added it to the API. But I wouldn't
AT> know of a practical use case for it to be honest.

I received the same feedback from our implementation expert Ryan Lubke.
I never added isImplicit() on the topic branch, and I have dropped
IMPLICIT from the MappingMatch enum.

AT> I also got some feedback regarding the getPattern() method. People
AT> (also) seem to want the pattern in such a way that they can use it
AT> right away in their code. E.g. for path mapping the pattern would
AT> now be e.g. "/path/*", but it would also be desirable to have a
AT> "/path/" returned.

On Mon, 4 Apr 2016 20:10:44 +0100 Mark Thomas wrote:

MT> I see where you are going with this but getPattern() is meant to return
MT> the pattern specified in web.xml.

Yes, that is what I have as well.

MT> At the moment, it feels more like the use case is meet by
MT> getMatchType() and a combination of getServletPath() and
MT> getPathInfo() depending on the match type and use case. At this
MT> point I'm not sure there is a single return value that meets all use
MT> cases.

Yes, I agree with that.

On Wed, 6 Apr 2016 15:31:14 +1000 Stuart Douglas wrote:

SD> I was also not clear what the behavior of getMapping() should be
SD> when using forward()/include(), in this case should it reflect the
SD> mapping of the forwarded/included Servlet or the original?

This is the current proposal from [1]:

  The mappings for any applicable Filters are not indicated in the
  result. If the currently active Servlet invocation was obtained by a
  call to ServletRequest.getRequestDispatcher() followed by a call to
  RequestDispatcher.forward(), the returned ServletMapping is the one
  corresponding to the path used to obtain the RequestDispatcher. If the
  currently active Servlet invocation was obtained by a call to
  ServletRequest.getRequestDispatcher() followed by a call to
  RequestDispatcher.include(), the returned ServletMapping is the one
  corresponding path that caused the first Servlet in the invocation
  sequence to be invoked. If the currently active Servlet invocation was
  obtained by a call to AsyncContext.dispatch(), the returned
  ServletMapping is the one corresponding path that caused the first
  Servlet in the invocation sequence to be invoked. See sections 9.3.1,
  9.4.2 and 9.7.2 of the specification document for additional request
  attributes related to ServletMapping.


>>>>> On Fri, 10 Feb 2017 09:11:43 +0000, Mark Thomas <markt_at_apache.org> said:

MT> Yes. Specifically this part of the thread:
MT> https://java.net/projects/servlet-spec/lists/users/archive/2016-04/message/7

Right, that's important:

MT> I think we need to follow the same rules as for the path of the
MT> original request. That means we'll need some new request
MT> attributes. Something like:

MT> javax.servlet.forward.mapping

MT> javax.servlet.include.mapping

MT> If folks think this is sensible, I can add implementing this to my
MT> TODO list for the next Tomcat milestone so folks can try it out.

Yes, and the javax.servlet.async.mapping as well. I will update spec
sections 9.3.1, 9.4.2 and 9.7.2 to define these attributes, when they
are set, and what is their value. In the meantime, you can look at [2].

Paul Penedict wrote:

PB> Question on the mapping. If a request comes into a servlet and the
PB> servlet includes/forwards to a JSP, what does the JSP see? I presume
PB> it will be the Mapping of the implicit *.jsp servlet, correct? Will
PB> there be any special request attributes to expose the servlet's --
PB> something to the effect like the javax.servlet.include/forward
PB> attributes?

See my response to "Wed, 6 Apr 2016 15:31:14 +1000 Stuart Douglas wrote:".

Arjan Tijms wrote:

AT> Btw, I noticed a typo or small oversight went into the proposed
AT> Mapping interface. There's now a method called getMappingType(),
AT> which originally returned the enum called MappingType. The spec lead
AT> changed this before committing it to MappingMatch, but my guess is
AT> he forgot to adjust the getter as well. That probably should become
AT> getMappingMatch(), if that makes sense?

I've updated the branch at r64536 to have these changes.

On Thursday 7 April 2016 Greg Wilkins wrote:

GW> What is the mapping for file mapped JSPs?

SD> At the moment I am not treating JSP specially, so we just return
SD> whatever the mapping type is for the original Servlet mapping.

SD> I don't really see any advantage in treating JSP's as a special
SD> case.

SD> I was just thinking about this a bit more and it might be useful to
SD> include the target Servlet name in the mapping result, so a filter
SD> could tell exactly what Servlet the request is targeted at (we could
SD> even take it one step further and include a list of the filter names
SD> that will process the request, although I don't know what the use
SD> case would be).

GW> You are suggesting that we add getServletName to the Mapping, which
GW> in this case would return "home" I see value in that.

I like this idea. I've added it.

GW> There may even be value in a getDescriptor() method that would give
GW> a text description of what descriptor the mapping was from: web.xml,
GW> fragment web.xml from a particular jar, annotation on a particular
GW> class etc.

SD> I am not sure about getDescriptor(), I can't really see a use case
SD> for it.

Yes, I don't like this idea.

On Mon 11 April Mark Thomas wrote:

MT> I've updated Tomcat's implementation as follows:

MT> - added RequestDispatcher.FORWARD_MAPPING
MT> - added RequestDispatcher.INCLUDE_MAPPING

MT> The behaviour for forward/include is the same as for the other
MT> forward/include attributes.

I've made these changes.

MT> - s/getMatchType/getMappingMatch/

MT> - added Mapping.getServletName()

MT> - removed MappingMatch.IMPLICIT

I've made those changes.

>>>>> On Thu, 9 Feb 2017 11:15:38 +1100, Greg Wilkins <gregw_at_webtide.com> said:

GW> We are looking at implementing this in the next few days.
GW> Firstly it is unclear what mapping should be returned for all dispatch
GW> types:

GW> - REQUEST is ok
GW> - FORWARDED, we guess it is the forwarded mapping, just like
GW> getServletPath has changed

MT> Returns the forward mapping. Original mapping available via
MT> RequestDispatcher.FORWARD_MAPPING / javax.servlet.forward.mapping

GW> - INCLUDED, we guess the original mapping is returned, but should the
GW> included mapping be available as a request attribute like the other
GW> included path info?

MT> Returns the original mapping. The mapping for the include is available via
MT> RequestDispatcher.INCLUDE_MAPPING / javax.servlet.include.mapping

GW> - ASYNC, assume it is the dispatched path?

MT> Yes. The original is available via
MT> ASYNC_MAPPING / javax.servlet.async.mapping

GW> - ERROR, no idea

MT> Returns the error mapping. Original mapping not available

GW> - what about between dispatches? I assume undefined, but it would be
GW> good to say as much
GW> - what about named dispatchers?

MT> Original mapping. No attributes set.

GW> Also why does the getMapping() method javadoc say:

EB> Each invocation of this method must return a fresh instance of Mapping.

GW> Why is this needed for an immutable object that contains 3 references to
GW> other immutable objects?

I can remove it. I was doing it out of an abundance of caution

GW> It also says:

EB> The implementation must retain no reference to the returned Mapping.

GW> which is a bit overkill for an immutable object. We don't say that for
GW> every immutable string or Cookie instance returned by the API?

I have removed these two statements and replaced them with "The returned
object is immutable".

I have updated the API in the branch and published the new artifact.

ACTION: Please review the API at [1] and comment by close of business
GMT-0800 Tuesday 21 February.

Thanks,

Ed


-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 13 business days until planned start of JSF 2.3 Final Approval Ballot
|  3 business days until DevNexus 2017
| 28 business days until JavaLand 2017
[1] https://maven.java.net/service/local/repositories/snapshots/archive/javax/servlet/javax.servlet-api/4.0.0-b03-SERVLET_SPEC-73-SNAPSHOT/javax.servlet-api-4.0.0-b03-SERVLET_SPEC-73-20170216.222658-3-javadoc.jar/!/javax/servlet/http/HttpServletRequest.html#getServletMapping--
[2] https://maven.java.net/service/local/repositories/snapshots/archive/javax/servlet/javax.servlet-api/4.0.0-b03-SERVLET_SPEC-73-SNAPSHOT/javax.servlet-api-4.0.0-b03-SERVLET_SPEC-73-20170216.222658-3-javadoc.jar/!/constant-values.html#javax.servlet.RequestDispatcher.FORWARD_MAPPING
https://maven.java.net/service/local/repositories/snapshots/archive/javax/servlet/javax.servlet-api/4.0.0-b03-SERVLET_SPEC-73-SNAPSHOT/javax.servlet-api-4.0.0-b03-SERVLET_SPEC-73-20170216.222658-3-javadoc.jar/!/constant-values.html#javax.servlet.AsyncContext.ASYNC_MAPPING