Quinton, all,
I did review your patch and kept the basic idea. However, I did rework
it to add the tools no longer directly to the perm/tem storage of the
user and the session but to a Map object which then is put in. This has
the charme that I was actually able to consolidate _all_ tool scopes
into a nice, streamlined method and remove lots of code from the Pull
Service. ATM it just compiles (Bayern - Rostock in 30 minutes. Gotta go
now. ;-) ) and isn't tested beyond that but it should work.
I found a bug with RunDataApplicationTools in global scope which I will
tackle later.
Please review. If there are no objections, I'd like to check that in,
because it makes the tool handling for all the scopes IMHO cleaner and
more understandable.
Did you know, that Global Tools were not instantiated through the pool?
;-)
Regards
Henning
On Fri, 2003-03-21 at 16:22, Quinton McCombs wrote:
> > -----Original Message-----
> > From: Henning P. Schmiedehausen [mailto:[EMAIL PROTECTED]
> > Sent: Friday, March 21, 2003 4:43 AM
> > To: [EMAIL PROTECTED]
> > Subject: Re: [RFC] changes to the pull service
> >
> >
> > "Quinton McCombs" <[EMAIL PROTECTED]> writes:
> >
> > >I have noticed that a NullpointerException is being through in
> > >TurbinePullService.populateWithSessionTools(). This happens
> > when the
> > >first request for the session is the login action (or the logout
> > >action). It is because the method does not expect the user
> > object to
> > >be null. Since the session validator has not run yet, the user is
> > >null.
> >
> > This first happened after your change. Before that,
> > populateWithSessionTools would have checked, whether the
> > current user is the anonymous user and if yes, would add
> > tools to the user.
> >
> > >There is also the issue of session scope tools being lost when the
> > >login action is executed (TTWS22).
> >
> > >Here is what I came up with to address these two issues.
> >
> > >populateWithSessionTools() will only process session and authorized
> > >scope tools. These tools will be stored in the session
> > instead of the
> > >user. The synchronized block will use the session instead
> > of the user
> > >object. Note: I am not sure if this needs to be synchronized. We
> > >should not have a case where multiple threads modifing the same
> > >session.
> >
> > I think, the correct solution would be, to pull the session
> > tools away from the user object. Adding a "temp storage"
> > object to the session context should be fine.
>
> I will go ahead and check in the code then. Take a look at how I am
> storing the data. I was going to attach a pacth to my message but the
> patch made it difficult to see where the changes really were...
>
> > But see no difference between Session and Global tools after
> > that change.
>
> For global tools, there is only one instance created and must be thread
> safe. For session tools, there is on per session and does not need to
> be thread safe.
>
> Session and authorized are perfect for maintaining state related data
> for the user.
>
> Global would be more for utility type tools.
>
> > Regards
> > Henning
> >
> > --
> > Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
> > [EMAIL PROTECTED] +49 9131 50 654 0 http://www.intermeta.de/
> >
> > Java, perl, Solaris, Linux, xSP Consulting, Web Services
> > freelance consultant -- Jakarta Turbine Development -- hero for hire
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> >
> >
> >
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH [EMAIL PROTECTED]
Am Schwabachgrund 22 Fon.: 09131 / 50654-0 [EMAIL PROTECTED]
D-91054 Buckenhof Fax.: 09131 / 50654-20
Index: pull/PullService.java
===================================================================
RCS file: /home/cvs/jakarta-turbine-2/src/java/org/apache/turbine/services/pull/PullService.java,v
retrieving revision 1.8
diff -u -r1.8 PullService.java
--- pull/PullService.java 21 Mar 2003 15:28:20 -0000 1.8
+++ pull/PullService.java 22 Mar 2003 13:57:26 -0000
@@ -127,8 +127,8 @@
/** Default value for per request tool refreshing */
boolean TOOLS_PER_REQUEST_REFRESH_DEFAULT = false;
- /** prefix for key used in the session to store session scope pull tools */
- String SESSION_TOOLS_ATTRIBUTE_PREFIX = "turbine.sessiontools.";
+ /** Key under which the Tools Table is stored */
+ String TOOLBOX_KEY = "_toolbox_";
/**
* Get the context containing global tools that will be
Index: pull/TurbinePullService.java
===================================================================
RCS file: /home/cvs/jakarta-turbine-2/src/java/org/apache/turbine/services/pull/TurbinePullService.java,v
retrieving revision 1.24
diff -u -r1.24 TurbinePullService.java
--- pull/TurbinePullService.java 21 Mar 2003 22:44:54 -0000 1.24
+++ pull/TurbinePullService.java 22 Mar 2003 13:57:27 -0000
@@ -55,8 +55,12 @@
*/
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
+
+import javax.servlet.http.HttpSession;
import org.apache.commons.configuration.Configuration;
@@ -190,21 +194,25 @@
}
}
- /** Internal list of global tools */
+ /** Internal list of global ToolData instances */
private List globalTools;
- /** Internal list of request tools */
+ /** Internal list of request ToolData instances */
private List requestTools;
- /** Internal list of session tools */
+ /** Internal list of session ToolData instances */
private List sessionTools;
- /** Internal list of authorized tools */
+ /** Internal list of authorized ToolData instances */
private List authorizedTools;
- /** Internal list of persistent tools */
+ /** Internal list of persistent ToolData instances */
private List persistentTools;
+ /** Internal list of instantiated global tools */
+ private Map globalToolInstances = new HashMap(10);
+
+
/** Directory where application tool resources are stored.*/
private String resourcesDirectory;
@@ -319,7 +327,8 @@
// Create and populate the global context right now
globalContext = new VelocityContext();
- populateWithGlobalTools(globalContext);
+ populateWithTools(globalTools, globalToolInstances,
+ globalContext, null, null);
}
/**
@@ -386,7 +395,7 @@
*/
public void populateContext(Context context, RunData data)
{
- populateWithRequestTools(context, data);
+ populateWithTools(requestTools, null, context, data, data);
// session tools (whether session-only or persistent are
// very similar, so the same method is used - the
@@ -405,91 +414,33 @@
// We should either store the session pull tools in the session or
// make Turbine.loginAction() copy the session pull tools into the
// new user object.
- populateWithSessionTools(sessionTools, context, data, user);
+
+ populateWithTools(sessionTools, getSessionToolBox(data, "session"), context, data, null);
if (!TurbineSecurity.isAnonymousUser(user))
{
if (user.hasLoggedIn())
{
- populateWithSessionTools(authorizedTools, context, data, user);
- populateWithPermTools(persistentTools, context, data, user);
- }
- }
- }
-
- /**
- * Populate the given context with the global tools
- *
- * @param context a Velocity Context to populate
- */
- private void populateWithGlobalTools(Context context)
- {
- for (Iterator it = globalTools.iterator(); it.hasNext();)
- {
- ToolData toolData = (ToolData) it.next();
- try
- {
- Object tool = toolData.toolClass.newInstance();
-
- // global tools are init'd with a null data parameter
- initTool(tool, null);
-
- // put the tool in the context
- context.put(toolData.toolName, tool);
- }
- catch (Exception e)
- {
- log.error("Could not instantiate global tool "
- + toolData.toolName + " from a "
- + toolData.toolClassName + " object", e);
- }
- }
- }
-
- /**
- * Populate the given context with the request-scope tools
- *
- * @param context a Velocity Context to populate
- * @param data a RunData instance
- */
- private void populateWithRequestTools(Context context, RunData data)
- {
- // Iterate the tools
- for (Iterator it = requestTools.iterator(); it.hasNext();)
- {
- ToolData toolData = (ToolData) it.next();
- try
- {
- // Fetch Object through the Pool.
- Object tool = pool.getInstance(toolData.toolClass);
-
- // request tools are init'd with a RunData object
- initTool(tool, data);
-
- // put the tool in the context
- context.put(toolData.toolName, tool);
- }
- catch (Exception e)
- {
- log.error("Could not instantiate request tool "
- + toolData.toolName + " from a "
- + toolData.toolClassName + " object", e);
+ populateWithTools(authorizedTools, getUserTempToolBox(user, "authorized"), context, data, user);
+ populateWithTools(persistentTools, getUserPermToolBox(user, "persistent"), context, data, user);
}
}
}
/**
- * Populate the given context with the session-scoped tools.
+ * Populate the given context with the tools from a tool
+ * scope list. The tool instances are pulled from a given
+ * tool box or (if they don't exist) instantiated.
*
* @param tools The list of tools with which to populate the
* session.
+ * @param toolbox The Toolbox from which the tools should be added.
+ * This parameter might be null.
* @param context The context to populate.
* @param data The current RunData object
- * @param user The <code>User</code> object whose storage to
- * retrieve the tool from.
*/
- private void populateWithSessionTools(List tools, Context context,
- RunData data, User user)
+ private void populateWithTools(List tools, Map toolbox, Context context,
+ RunData data, Object initObj)
{
// Iterate the tools
for (Iterator it = tools.iterator(); it.hasNext();)
@@ -497,16 +448,15 @@
ToolData toolData = (ToolData) it.next();
try
{
- // ensure that tool is created only once for a user
- // by synchronizing against the user object
- synchronized (data.getSession())
+ // Make sure that not another task tries to
+ // add a tool to the box in this very moment.
+ synchronized (toolbox)
{
// first try and fetch the tool from the user's
// hashtable
- Object tool = data.getSession().getAttribute(
- SESSION_TOOLS_ATTRIBUTE_PREFIX
- + toolData.toolClassName);
-
+ Object tool = (toolbox != null)
+ ? toolbox.get(toolData.toolClassName) : null;
+
if (tool == null)
{
// if not there, an instance must be fetched from
@@ -514,94 +464,14 @@
tool = pool.getInstance(toolData.toolClass);
// session tools are init'd with the User object
- initTool(tool, user);
+ initTool(tool, initObj);
// store the newly created tool in the session
- data.getSession().setAttribute(
- SESSION_TOOLS_ATTRIBUTE_PREFIX
- + tool.getClass().getName(), tool);
- }
-
- // *NOT* else
- if(tool != null)
- {
- // This is a semantics change. In the old
- // Turbine, Session tools were initialized and
- // then refreshed every time they were pulled
- // into the context if "refreshToolsPerRequest"
- // was wanted.
- //
- // RunDataApplicationTools now have a parameter
- // for refresh. If it is not refreshed immediately
- // after init(), the parameter value will be undefined
- // until the 2nd run. So we refresh all the session
- // tools on every run, even if we just init'ed it.
- //
- if (refreshToolsPerRequest)
+ if (toolbox != null)
{
- refreshTool(tool, data);
+ toolbox.put(toolData.toolClassName, tool);
}
-
- // put the tool in the context
- log.debug("Adding " + tool + " to ctx as "
- + toolData.toolName);
- context.put(toolData.toolName, tool);
- }
- else
- {
- log.info("Tool " + toolData.toolName
- + " was null, skipping it.");
- }
- }
- }
- catch (Exception e)
- {
- log.error("Could not instantiate session tool "
- + toolData.toolName + " from a "
- + toolData.toolClassName + " object", e);
- }
- }
- }
-
- /**
- * Populate the given context with the perm-scoped tools.
- *
- * @param tools The list of tools with which to populate the
- * session.
- * @param context The context to populate.
- * @param data The current RunData object
- * @param user The <code>User</code> object whose storage to
- * retrieve the tool from.
- */
- private void populateWithPermTools(List tools, Context context,
- RunData data, User user)
- {
- // Iterate the tools
- for (Iterator it = tools.iterator(); it.hasNext();)
- {
- ToolData toolData = (ToolData) it.next();
- try
- {
- // ensure that tool is created only once for a user
- // by synchronizing against the user object
- synchronized (user)
- {
- // first try and fetch the tool from the user's
- // hashtable
- Object tool = user.getPerm(toolData.toolClassName);
-
- if (tool == null)
- {
- // if not there, an instance must be fetched from
- // the pool
- tool = pool.getInstance(toolData.toolClass);
-
- // session tools are init'd with the User object
- initTool(tool, user);
-
- // store the newly created tool in the user's hashtable
- user.setPerm(toolData.toolClassName, tool);
}
// *NOT* else
@@ -628,7 +498,6 @@
// put the tool in the context
log.debug("Adding " + tool + " to ctx as "
+ toolData.toolName);
- log.warn("Persistent scope tools are deprecated.");
context.put(toolData.toolName, tool);
}
else
@@ -640,7 +509,7 @@
}
catch (Exception e)
{
- log.error("Could not instantiate perm tool "
+ log.error("Could not instantiate tool "
+ toolData.toolName + " from a "
+ toolData.toolClassName + " object", e);
}
@@ -675,14 +544,14 @@
* ApplicationTool interface because we
* know those types of tools have a refresh
* method.
+ *
+ * @param data The current RunData Object
*/
public void refreshGlobalTools()
{
- for (Iterator it = globalTools.iterator(); it.hasNext();)
+ for (Iterator it = globalToolInstances.values().iterator(); it.hasNext();)
{
- ToolData toolData = (ToolData) it.next();
- Object tool = globalContext.get(toolData.toolName);
- refreshTool(tool, null);
+ refreshTool(it.next(), null);
}
}
@@ -767,4 +636,99 @@
((RunDataApplicationTool) tool).refresh(data);
}
}
+
+ /**
+ * Fetch the session based tool box. If it does not exist,
+ * create one.
+ *
+ * @param data The Rundata Object
+ * @param attrName Used to distinguish multiple tool boxes
+ *
+ * @return A Map Object for the session based toolbox.
+ */
+ private Map getSessionToolBox(RunData data, String attrName)
+ {
+ HttpSession session = data.getSession();
+ String toolboxKey = PullService.TOOLBOX_KEY + attrName;
+
+ synchronized (session)
+ {
+ Map toolbox = (Map) session.getAttribute(toolboxKey);
+
+ if (toolbox == null)
+ {
+ toolbox = new HashMap(10);
+ session.setAttribute(toolboxKey, toolbox);
+ }
+ return toolbox;
+ }
+ }
+
+ /**
+ * Fetch the user based temporary toolbox. If it does not
+ * exist, create one
+ *
+ * @param user The current User object.
+ * @param attrName Used to distinguish multiple tool boxes
+ *
+ * @return A Map Object for the user based temporary toolbox
+ * or null.
+ */
+ private Map getUserTempToolBox(User user, String attrName)
+ {
+ if (TurbineSecurity.isAnonymousUser(user))
+ {
+ // Anonymous User never gets any tools, else
+ // information might leak from one session to the next
+ return null;
+ }
+
+ return getToolBox(user.getTempStorage(), attrName);
+ }
+
+ /**
+ * Fetch the user based permanent toolbox. If it does not
+ * exist, create one
+ *
+ * @param user The current User object.
+ * @param attrName Used to distinguish multiple tool boxes
+ *
+ * @return A Map Object for the user based permanent toolbox
+ * or null.
+ */
+ private Map getUserPermToolBox(User user, String attrName)
+ {
+ if (TurbineSecurity.isAnonymousUser(user))
+ {
+ // Anonymous User never gets any tools, else
+ // information might leak from one session to the next
+ return null;
+ }
+ return getToolBox(user.getPermStorage(), attrName);
+ }
+
+ /**
+ * Fetch the named toolbox from the storage container.
+ * If it does not exist, add it.
+ *
+ * @param storage A Storage map
+ * @param attrName The Attribute Name
+ */
+ private Map getToolBox(Map storage, String attrName)
+ {
+ String toolboxKey = PullService.TOOLBOX_KEY + attrName;
+
+ synchronized (storage)
+ {
+ Map toolbox = (Map) storage.get(toolboxKey);
+
+ if (toolbox == null)
+ {
+ toolbox = new HashMap(10);
+ storage.put(toolboxKey, toolbox);
+ }
+ return toolbox;
+ }
+ }
}
+
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
