Skip to content
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

Add the ability to set <jsp-file> though the ServletRegistration interface #16

Closed
glassfishrobot opened this issue Oct 4, 2011 · 16 comments

Comments

@glassfishrobot
Copy link

Title says it all. This looks like an oversight in the original ServletRegistration API.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Reported by markt_asf

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Issue-Links:
is related to
GLASSFISH-21666

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
mode said:
Not everything defined in the web.xml can be done via ServletRegistration.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
markt_asf said:
Indeed, but is there a reason not to add jsp-file that I have missed?

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
mode said:
There was no specific discussion about jsp-file, however the main goal of the APIs for adding Servlet, Filters and Listeners was for pluggability of frameworks. Not sure how many frameworks will really need to the jsp-file for pluggability.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
markt_asf said:
Here is the Tomcat discussion that triggered this request:
http://tomcat.markmail.org/thread/pg65fgqhfra7czy5

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
@edburns said:
Here is a proposal that implements this.

@WebListener()
public class NewServletListener  implements ServletContextListener {

    @Override
    public void contextInitialized(ServletContextEvent sce) {

        final ServletContext servletContext = sce.getServletContext();
        final ServletRegistration.Dynamic dynamic = servletContext.addServlet("newjsp",
ServletContext.JSP_SERVLET_CLASS_REFERENCE);
        dynamic.setJspFile("/newjsp.jsp");
        dynamic.addMapping("/newjsp");

    }

}

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
@edburns said:
The use of the constant JSP_SERVLET_CLASS_REFERENCE enables the implementation to take the necessary actions to ensure this is equivalent to the implementation private JspServlet.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
@shingwaichan said:
is a deployment descriptor. If the API is to set , do we need to introduce the constant JSP_SERVLET_CLASS_REFERENCE?

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
@edburns said:
I chose adding the constant to minimize impact on the existing API for programmatic servlet registration, which is:

@WebListener()
public class NewServletListener  implements ServletContextListener {

    @Override
    public void contextInitialized(ServletContextEvent sce) {

        final ServletContext servletContext = sce.getServletContext();
        final ServletRegistration.Dynamic dynamic = servletContext.addServlet("newjsp",
[String, Class or Servlet]);
        dynamic.addMapping("/newjsp");

    }

}

As far as I know, and I could be wrong about this, but the existing API does require the use of one of the addServlet() variants on ServletContext. Given that assumption, neither the Class nor the Servlet variants are appropriate for a Servlet that is a JSP page. That leaves String. We have a private constant that is used internally for such things, but that is implementation specific. So, I judged the easiest and most minimal impact would be achieved by introducing a new constant whose meaning is as I've documented in the patch.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
markt_asf said:
I originally thought of using null. Looking at the addSevlet() Javadoc that looks legal. However, a specific constant strikes me as a better idea than giving null a special meaning.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
@shingwaichan said:
Revisions:

64485

Modified Paths:

trunk/api/javaee-api/javax.servlet/src/main/java/javax/servlet/ServletContext.java

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Marked as fixed on Thursday, February 9th 2017, 3:52:46 pm

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
@shingwaichan said:
add ServletContext#addJspFile()

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
This issue was imported from java.net JIRA SERVLET_SPEC-16

@glassfishrobot
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant