Issue 119: WebContext exposes ServletContext explicitly
Status:  Fixed
Owner: ----
Closed:  Jul 2008
Reported by bgo...@e1b.org, Jul 17, 2008
WebContext is presumably an abstraction away from a specific
implementation.  However, the method getServletContext() return a
ServletContext object.

I would suggest refactoring this method to be public Object
getContextObject() instead.  My use case is that I am writing a portlet
implementation of WebContext, and the portlet object heirarchy does not
extend the servlet object heirarchy.  While PortletContext has almost all
of the same methods as ServletContext, it is not actually implemented.

I looks like the only place that the library uses getServletContext() is
SpringMessages.java.  The constructor of that class could easily use
instanceof() to determine whether it is a portlet context or servlet
context, and use PortletApplicationContextUtils or
WebApplicationContextUtils accordingly.

I can complete the PortletWebRequestContext implementation without this
refactoring using an adapter, but I think that the JMesa API would be
cleaner if refactored.
Jul 17, 2008
#1 extremec...@gmail.com
The getServletContext() method was just put in recently (for the last release) and
now I would just prefer to pull it before it gets used. As you pointed out it is not
a method that cuts across enough implementations.

This issue surfaced a better interface. So now I put in a SpringWebContext interface
that extends the WebContext and adds a getApplicationContext() method.

How do you want to do this? Should I send you a jar file with the changes or are you
running off the trunk code? I did check everything into the trunk right now. If you
want a new jar sent send me an email at extremecomponents@gmail.com.


Jul 17, 2008
#2 extremec...@gmail.com
I guess we will also need a new TableFacade.createSpringPortletTableFacade() and a
SpringPortletTableFacadeTag. I can help you with those changes or just follow what I
did for the Spring code.
Jul 19, 2008
#3 bgo...@e1b.org
thanks... I'm working with trunk so I can make the change that you described.  In
order to make the change for TableFacade the portlet 2.0 api should be added as a
dependency (jcp.org/en/jsr/detail?id=286).  I think there is a separate JAR for
Spring Portlet MVC too (spring-portlet.jar or something like that) that is probably
needed.
Jul 21, 2008
#4 bgo...@e1b.org
When JSPs are included in a portlet, the portlet objects get wrapped with adapters so
that they implement the JSP API.  Thus, I think the tag should not have to be
reimplemented.  I'll have a patch with the PortletRequestWebContext and the
associated factory methods soon.
Jul 21, 2008
#5 bgo...@e1b.org
I'm wondering if it would be a little more DRY to make the SpringWebContext
implementation into a decorator for any old WebContext, and when instantiated it
could auto-detect using reflection.  For example:

public class SpringWebContextImpl extends WebContextWrapper 
        implements SpringWebContext {

    // Use reflection here so that there is not a runtime dependency on 
    // the portlet API.  We'll assume that the runtime environment already has
    // the servlet API and the JSP API though.
    private final static Class<?> portletRequest;

    private ApplicationContext ctx;
    
    static {
        Class c;
        try {
            c = Class.forName("javax.portlet.PortletRequest");
        } catch (ClassNotFoundException e) {
            c = null;
        }
        portletRequest = c;
    }

    public SpringWebContextImpl(WebContext ctx) {
        super(ctx);
    }

    /**
     * <p>If <code>setApplicationContext</code> has not been called, we will
     * do our best to return an <code>ApplicationContext</code> based on the
     * wrapped WebContext.  If an appropriate context can not be discovered,
     * an exception is thrown.</p>
     * <p>If <code>setApplicationContext()</code> has been called and is not 
     * null, we simply return that instance.</p>
     * 
     * @return The Spring <code>ApplicationContext</code>.
     * @throws UnsupportedOperationException if the application context has not
     * been set and one cannot be discovered.
     */
    public ApplicationContext getApplicationContext() {
        if (ctx != null) {
            Object it = getBackingObject();
            if (it instanceof HttpServletRequest) {
                ctx = WebApplicationContextUtils.getWebApplicationContext(
                    ((HttpServletRequest) it).getSession().getServletContext()
                );
            } else if (it instanceof PageContext) {
                ctx = WebApplicationContextUtils.getWebApplicationContext(
                    ((PageContext) it).getServletContext()
                );
            } else if (portletRequest != null && 
                        portletRequest.isAssignableFrom(it.getClass())) {
                // TODO: Need spring-portlet.jar for this
                throw new UnsupportedOperationException();
            } else {
                // TODO: Could add some support for JSF too...
                throw new UnsupportedOperationException();
            }
        }
        return ctx;
    }

    public void setApplicationContext(ApplicationContext ctx) {
        this.ctx = ctx;
    }
    
}
Jul 21, 2008
#6 extremec...@gmail.com
The tag isn't actually reimplemented though. It just requires the TableFacadeTag to
be extended and the proper methods to be overridden. If anything I should consider
making the TableFacadeTag abstract and making the required methods abstract.

protected WebContext getWebContext()
protected TableFacade createTableFacade()

It is unfortunate that the tld declarations have to be copied but thats a small price
to pay for the clarity when using the tags.

I appreciate your suggestion for the SpringWebContextImpl but I think just
implementing the interface is still better. There is no cut and paste going on so
there is no problem keeping things DRY.


Status: Accepted
Labels: -Type-Defect Type-Enhancement
Jul 26, 2008
#7 extremec...@gmail.com
incorporated the new portlet code
Jul 26, 2008
#8 extremec...@gmail.com
(No comment was entered for this change.)
Status: Fixed