You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ja...@apache.org on 2014/09/01 16:26:54 UTC
svn commit: r1621804 - in /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp:
control/RequestHandler.java event/EventFactory.java
event/RomeEventHandler.java view/ViewFactory.java
Author: jacopoc
Date: Mon Sep 1 14:26:54 2014
New Revision: 1621804
URL: http://svn.apache.org/r1621804
Log:
A series of clean-ups and modifications to RequestHandler, EventFactory and View Factory to make then (mostly) immutable and eliminate some potential concurrency issues that could happen at initialization. This is just a first pass.
Modified:
ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java
ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java
ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java
Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1621804&r1=1621803&r2=1621804&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Mon Sep 1 14:26:54 2014
@@ -69,29 +69,29 @@ import org.owasp.esapi.errors.EncodingEx
public class RequestHandler {
public static final String module = RequestHandler.class.getName();
- private boolean throwRequestHandlerExceptionOnMissingLocalRequest = UtilProperties.propertyValueEqualsIgnoreCase(
+ private static final boolean throwRequestHandlerExceptionOnMissingLocalRequest = UtilProperties.propertyValueEqualsIgnoreCase(
"requestHandler.properties", "throwRequestHandlerExceptionOnMissingLocalRequest", "Y");
- private String statusCodeString = UtilProperties.getPropertyValue("requestHandler.properties", "status-code", "302");
+ private final String defaultStatusCodeString = UtilProperties.getPropertyValue("requestHandler.properties", "status-code", "302");
+ private final ViewFactory viewFactory;
+ private final EventFactory eventFactory;
+ private final URL controllerConfigURL;
+ private final boolean forceHttpSession;
+ private final boolean trackServerHit;
+ private final boolean trackVisit;
+ private final boolean cookies;
+ private final String charset;
+
public static RequestHandler getRequestHandler(ServletContext servletContext) {
RequestHandler rh = (RequestHandler) servletContext.getAttribute("_REQUEST_HANDLER_");
if (rh == null) {
- rh = new RequestHandler();
+ rh = new RequestHandler(servletContext);
servletContext.setAttribute("_REQUEST_HANDLER_", rh);
- rh.init(servletContext);
}
return rh;
}
- protected ServletContext context = null;
- protected ViewFactory viewFactory = null;
- protected EventFactory eventFactory = null;
- protected URL controllerConfigURL = null;
-
- public void init(ServletContext context) {
- if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler Loading...]", module);
- this.context = context;
-
- // init the ControllerConfig, but don't save it anywhere
+ private RequestHandler(ServletContext context) {
+ // init the ControllerConfig, but don't save it anywhere, just load it into the cache
this.controllerConfigURL = ConfigXMLReader.getControllerConfigURL(context);
try {
ConfigXMLReader.getControllerConfig(this.controllerConfigURL);
@@ -99,8 +99,14 @@ public class RequestHandler {
// FIXME: controller.xml errors should throw an exception.
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
}
- this.viewFactory = new ViewFactory(this);
- this.eventFactory = new EventFactory(this);
+ this.viewFactory = new ViewFactory(context, this.controllerConfigURL);
+ this.eventFactory = new EventFactory(context, this.controllerConfigURL);
+
+ this.forceHttpSession = "true".equalsIgnoreCase(context.getInitParameter("forceHttpSession"));
+ this.trackServerHit = !"false".equalsIgnoreCase(context.getInitParameter("track-serverhit"));
+ this.trackVisit = !"false".equalsIgnoreCase(context.getInitParameter("track-visit"));
+ this.cookies = !"false".equalsIgnoreCase(context.getInitParameter("cookies"));
+ this.charset = context.getInitParameter("charset");
}
public ConfigXMLReader.ControllerConfig getControllerConfig() {
@@ -129,16 +135,17 @@ public class RequestHandler {
// get the controllerConfig once for this method so we don't have to get it over and over inside the method
ConfigXMLReader.ControllerConfig controllerConfig = this.getControllerConfig();
Map<String, ConfigXMLReader.RequestMap> requestMapMap = null;
- String controllerStatusCodeString = null;
+ String statusCodeString = null;
try {
requestMapMap = controllerConfig.getRequestMapMap();
- controllerStatusCodeString = controllerConfig.getStatusCode();
+ statusCodeString = controllerConfig.getStatusCode();
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
throw new RequestHandlerException(e);
}
- if(UtilValidate.isNotEmpty(controllerStatusCodeString != null))
- statusCodeString = controllerStatusCodeString;
+ if (UtilValidate.isEmpty(statusCodeString)) {
+ statusCodeString = defaultStatusCodeString;
+ }
// workaround if we are in the root webapp
String cname = UtilHttp.getApplicationName(request);
@@ -248,7 +255,6 @@ public class RequestHandler {
requestMap = requestMapMap.get(defaultRequest);
}
}
- boolean forceHttpSession = "true".equals(context.getInitParameter("forceHttpSession"));
// Check if we SHOULD be secure and are not.
String forwardedProto = request.getHeader("X-Forwarded-Proto");
boolean isForwardedSecure = UtilValidate.isNotEmpty(forwardedProto) && "HTTPS".equals(forwardedProto.toUpperCase());
@@ -775,11 +781,6 @@ public class RequestHandler {
return null;
}
- /** Returns the ServletContext Object. */
- public ServletContext getServletContext() {
- return context;
- }
-
/** Returns the ViewFactory Object. */
public ViewFactory getViewFactory() {
return viewFactory;
@@ -942,7 +943,7 @@ public class RequestHandler {
long viewStartTime = System.currentTimeMillis();
// setup character encoding and content type
- String charset = UtilFormatOut.checkEmpty(getServletContext().getInitParameter("charset"), req.getCharacterEncoding(), "UTF-8");
+ String charset = UtilFormatOut.checkEmpty(this.charset, req.getCharacterEncoding(), "UTF-8");
String viewCharset = viewMap.encoding;
//NOTE: if the viewCharset is "none" then no charset will be used
@@ -1198,7 +1199,7 @@ public class RequestHandler {
String encodedUrl;
if (encode) {
- boolean forceManualJsessionid = "false".equals(getServletContext().getInitParameter("cookies")) ? true : false;
+ boolean forceManualJsessionid = !cookies;
boolean isSpider = false;
// if the current request comes from a spider, we will not add the jsessionid to the link
@@ -1293,7 +1294,7 @@ public class RequestHandler {
}
public boolean trackStats(HttpServletRequest request) {
- if (!"false".equalsIgnoreCase(context.getInitParameter("track-serverhit"))) {
+ if (trackServerHit) {
String uriString = RequestHandler.getRequestUri(request.getPathInfo());
if (uriString == null) {
uriString="";
@@ -1312,7 +1313,7 @@ public class RequestHandler {
}
public boolean trackVisit(HttpServletRequest request) {
- if (!"false".equalsIgnoreCase(context.getInitParameter("track-visit"))) {
+ if (trackVisit) {
String uriString = RequestHandler.getRequestUri(request.getPathInfo());
if (uriString == null) {
uriString="";
Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java?rev=1621804&r1=1621803&r2=1621804&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java Mon Sep 1 14:26:54 2014
@@ -18,12 +18,11 @@
*******************************************************************************/
package org.ofbiz.webapp.event;
+import java.net.URL;
import java.util.Map;
import java.util.Set;
import javax.servlet.ServletContext;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
import javolution.util.FastMap;
@@ -31,7 +30,6 @@ import org.ofbiz.base.util.Debug;
import org.ofbiz.base.util.GeneralRuntimeException;
import org.ofbiz.base.util.ObjectType;
import org.ofbiz.webapp.control.ConfigXMLReader;
-import org.ofbiz.webapp.control.RequestHandler;
import org.ofbiz.webapp.control.WebAppConfigurationException;
/**
@@ -41,14 +39,14 @@ public class EventFactory {
public static final String module = EventFactory.class.getName();
- protected RequestHandler requestHandler = null;
- protected ServletContext context = null;
+ private final URL controllerConfigURL;
+ private final ServletContext context;
protected Map<String, EventHandler> handlers = null;
- public EventFactory(RequestHandler requestHandler) {
+ public EventFactory(ServletContext context, URL controllerConfigURL) {
handlers = FastMap.newInstance();
- this.requestHandler = requestHandler;
- this.context = requestHandler.getServletContext();
+ this.controllerConfigURL = controllerConfigURL;
+ this.context = context;
// pre-load all event handlers
try {
@@ -62,7 +60,7 @@ public class EventFactory {
private void preLoadAll() throws EventHandlerException {
Set<String> handlers = null;
try {
- handlers = this.requestHandler.getControllerConfig().getEventHandlerMap().keySet();
+ handlers = ConfigXMLReader.getControllerConfig(this.controllerConfigURL).getEventHandlerMap().keySet();
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
}
@@ -104,7 +102,7 @@ public class EventFactory {
EventHandler handler = null;
String handlerClass = null;
try {
- handlerClass = this.requestHandler.getControllerConfig().getEventHandlerMap().get(type);
+ handlerClass = ConfigXMLReader.getControllerConfig(this.controllerConfigURL).getEventHandlerMap().get(type);
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
}
@@ -126,19 +124,4 @@ public class EventFactory {
}
return handler;
}
-
- public static String runRequestEvent(HttpServletRequest request, HttpServletResponse response, String requestUri)
- throws EventHandlerException {
- ServletContext application = ((ServletContext) request.getAttribute("servletContext"));
- RequestHandler handler = (RequestHandler) application.getAttribute("_REQUEST_HANDLER_");
- ConfigXMLReader.ControllerConfig controllerConfig = handler.getControllerConfig();
- ConfigXMLReader.RequestMap requestMap;
- try {
- requestMap = controllerConfig.getRequestMapMap().get(requestUri);
- } catch (WebAppConfigurationException e) {
- Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
- throw new EventHandlerException(e);
- }
- return handler.runEvent(request, response, requestMap.event, requestMap, "unknown");
- }
}
Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java?rev=1621804&r1=1621803&r2=1621804&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java Mon Sep 1 14:26:54 2014
@@ -43,18 +43,10 @@ public class RomeEventHandler implements
public static final String mime = "application/xml; charset=UTF-8";
public static final String defaultFeedType = "rss_2.0";
- protected RequestHandler handler;
- protected ServletContext context;
protected EventHandler service;
protected WireFeedOutput out;
public void init(ServletContext context) throws EventHandlerException {
- this.context = context;
- this.handler = (RequestHandler) context.getAttribute("_REQUEST_HANDLER_");
- if (this.handler == null) {
- throw new EventHandlerException("No request handler found in servlet context!");
- }
-
// get the service event handler
this.service = new ServiceEventHandler();
this.service.init(context);
@@ -65,6 +57,10 @@ public class RomeEventHandler implements
* @see org.ofbiz.webapp.event.EventHandler#invoke(ConfigXMLReader.Event, ConfigXMLReader.RequestMap, javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
*/
public String invoke(Event event, RequestMap requestMap, HttpServletRequest request, HttpServletResponse response) throws EventHandlerException {
+ RequestHandler handler = (RequestHandler) request.getSession().getServletContext().getAttribute("_REQUEST_HANDLER_");
+ if (handler == null) {
+ throw new EventHandlerException("No request handler found in servlet context!");
+ }
// generate the main and entry links
String entryLinkReq = request.getParameter("entryLinkReq");
String mainLinkReq = request.getParameter("mainLinkReq");
Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java?rev=1621804&r1=1621803&r2=1621804&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java Mon Sep 1 14:26:54 2014
@@ -18,6 +18,7 @@
*******************************************************************************/
package org.ofbiz.webapp.view;
+import java.net.URL;
import java.util.Map;
import java.util.Set;
@@ -29,8 +30,7 @@ import org.ofbiz.base.util.Debug;
import org.ofbiz.base.util.GeneralRuntimeException;
import org.ofbiz.base.util.ObjectType;
import org.ofbiz.base.util.UtilValidate;
-import org.ofbiz.webapp.control.RequestHandler;
-import org.ofbiz.webapp.control.RequestHandlerException;
+import org.ofbiz.webapp.control.ConfigXMLReader;
import org.ofbiz.webapp.control.WebAppConfigurationException;
/**
@@ -40,14 +40,14 @@ public class ViewFactory {
public static final String module = ViewFactory.class.getName();
- protected RequestHandler requestHandler = null;
- protected ServletContext context = null;
+ private final URL controllerConfigURL;
+ private final ServletContext context;
protected Map<String, ViewHandler> handlers = null;
- public ViewFactory(RequestHandler requestHandler) {
+ public ViewFactory(ServletContext context, URL controllerConfigURL) {
this.handlers = FastMap.newInstance();
- this.requestHandler = requestHandler;
- this.context = requestHandler.getServletContext();
+ this.controllerConfigURL = controllerConfigURL;
+ this.context = context;
// pre-load all the view handlers
try {
@@ -61,7 +61,7 @@ public class ViewFactory {
private void preLoadAll() throws ViewHandlerException {
Set<String> handlers = null;
try {
- handlers = this.requestHandler.getControllerConfig().getViewHandlerMap().keySet();
+ handlers = ConfigXMLReader.getControllerConfig(this.controllerConfigURL).getViewHandlerMap().keySet();
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
}
@@ -120,7 +120,7 @@ public class ViewFactory {
ViewHandler handler = null;
String handlerClass = null;
try {
- handlerClass = this.requestHandler.getControllerConfig().getViewHandlerMap().get(type);
+ handlerClass = ConfigXMLReader.getControllerConfig(this.controllerConfigURL).getViewHandlerMap().get(type);
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
}
Re: svn commit: r1621804 - in
/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp:
control/RequestHandler.java event/EventFactory.java
event/RomeEventHandler.java view/ViewFactory.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 02/09/2014 11:53, Jacopo Cappellato a écrit :
> On Sep 2, 2014, at 11:43 AM, Jacques Le Roux <ja...@les7arts.com> wrote:
>> I don't see a need to make those finals. They are cached and cost at maximum hundreds of bytes.
>> Having them cached allows to dynamically change them, which can be handy sometimes (general remark for other possible cases)
>>
>> Jacques
> These fields were not modifiable even before my changes; the final keyword I have added didn't change the external behavior of them. However in general, if some field can change and the class can be used by different threads then you have to make sure the class is thread safe (and this is expensive): this is the main advantage of having (when possible) immutable classes, they are thread safe at no cost. This is not a matter of memory usage.
Indeed, forgot the _REQUEST_HANDLER_ servletContext attribute
Jacques
>
> Jacopo
Re: svn commit: r1621804 - in /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp: control/RequestHandler.java event/EventFactory.java event/RomeEventHandler.java view/ViewFactory.java
Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Sep 2, 2014, at 11:43 AM, Jacques Le Roux <ja...@les7arts.com> wrote:
>
> I don't see a need to make those finals. They are cached and cost at maximum hundreds of bytes.
> Having them cached allows to dynamically change them, which can be handy sometimes (general remark for other possible cases)
>
> Jacques
These fields were not modifiable even before my changes; the final keyword I have added didn't change the external behavior of them. However in general, if some field can change and the class can be used by different threads then you have to make sure the class is thread safe (and this is expensive): this is the main advantage of having (when possible) immutable classes, they are thread safe at no cost. This is not a matter of memory usage.
Jacopo
Re: svn commit: r1621804 - in
/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp:
control/RequestHandler.java event/EventFactory.java
event/RomeEventHandler.java view/ViewFactory.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
Inline
Le 01/09/2014 16:26, jacopoc@apache.org a écrit :
> Author: jacopoc
> Date: Mon Sep 1 14:26:54 2014
> New Revision: 1621804
>
> URL: http://svn.apache.org/r1621804
> Log:
> A series of clean-ups and modifications to RequestHandler, EventFactory and View Factory to make then (mostly) immutable and eliminate some potential concurrency issues that could happen at initialization. This is just a first pass.
>
> Modified:
> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/EventFactory.java
> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/RomeEventHandler.java
> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/ViewFactory.java
>
> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java?rev=1621804&r1=1621803&r2=1621804&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java (original)
> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/RequestHandler.java Mon Sep 1 14:26:54 2014
> @@ -69,29 +69,29 @@ import org.owasp.esapi.errors.EncodingEx
> public class RequestHandler {
>
> public static final String module = RequestHandler.class.getName();
> - private boolean throwRequestHandlerExceptionOnMissingLocalRequest = UtilProperties.propertyValueEqualsIgnoreCase(
> + private static final boolean throwRequestHandlerExceptionOnMissingLocalRequest = UtilProperties.propertyValueEqualsIgnoreCase(
> "requestHandler.properties", "throwRequestHandlerExceptionOnMissingLocalRequest", "Y");
> - private String statusCodeString = UtilProperties.getPropertyValue("requestHandler.properties", "status-code", "302");
I don't see a need to make those finals. They are cached and cost at maximum hundreds of bytes.
Having them cached allows to dynamically change them, which can be handy sometimes (general remark for other possible cases)
Jacques