On second thought... using threadlocals weren't such a hack after all.
The thing is that you can have many instances of handlers on a page,
so I can't work with simple instance variables. Can't work with normal
static variables either, as that would be over all applications and
current requests in the context.
Other options would be to introduce a variable holder mechanism in
Page, or like you proposed, using request attributes. Imo both are
hacks too, that would leak what really should be internals out of the
scope of ajax handler. Furthermore, request attributes are specific
for normal web applications; don't know whether they may be used for
portlets at all.
I think I solved it. I committed it in head, and attached the patch to
this email. If some can come up with something that works without
needing thread locals /and/ without breaking encapsulation, that's
welcome. Furthermore, as I don't have time to write a unit test for it
now, that would be a welcome contribution too :)
Eelco
On 9/6/05, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
> I guess just checking on the actual contribution should be enough...
> I'll fix it tommorrow.
>
> Eelco
>
>
> On 9/6/05, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
> > Good catch, you're right! I guess using a thread local is quite a
> > hack. Anyone a better idea how we should do this?
> >
> > Eelco
> >
> > On 9/6/05, Arto Arffman <[EMAIL PROTECTED]> wrote:
> > > Hi,
> > > I was browsing through ajax implementation (to learn Wicket and ajax), and
> > > found out something odd. It seems that it is not possible to have two
> > > different AjaxHandlers on the same page. AjaxHandler base class uses
> > > static
> > > ThreadLocal to prevent including javascript resources more than once. If
> > > one
> > > would like to use dojo and scriptaculous on the sama page, then only the
> > > first one would get its script reference rendered (because they both
> > > extend
> > > AjaxHandler).
> > >
> > > private static final ThreadLocal headContribHolder = new ThreadLocal();
> > > ...
> > > public final void renderHead(HtmlHeaderContainer container)
> > > {
> > > if (headContribHolder.get() == null)
> > > {
> > > headContribHolder.set(dummy);
> > > renderHeadInitContribution(container);
> > > }
> > > renderHeadContribution(container);
> > > }
> > >
> > >
> > > I did not test this, though. I'm not fluent with Wicket and ajax, so I
> > > found
> > > it too time consuming. But I did a simple code fragment that I think
> > > proves
> > > my case:
> > >
> > > public class B {
> > > public static void main(String[] args) {
> > > System.out.println("start");
> > > X x = new X();
> > > Y y = new Y();
> > > x.start();
> > > System.out.println(y.isRunning());
> > > y.end();
> > > System.out.println(x.isRunning());
> > > }
> > > }
> > > class BaseTL {
> > > private static final ThreadLocal flag = new ThreadLocal();
> > > public void start() {
> > > flag.set(new Object());
> > > }
> > > public void end() {
> > > flag.set(null);
> > > }
> > >
> > > public boolean isRunning() {
> > > return flag.get() != null;
> > > }
> > >
> > > }
> > > class X extends BaseTL {
> > > }
> > > class Y extends BaseTL {
> > > }
> > >
> > > When only x is started y.isRunning() returns also true.
> > >
> > > Anyway, I think that the framework should provide a way to do "things"
> > > only
> > > once per request cycle. Using ThreadLocal seems a little bit like a hack
> > > to
> > > me.
> > >
> > > /Arto
> > >
> >
>
Index: src/java/wicket/AjaxHandler.java
===================================================================
RCS file: /cvsroot/wicket/wicket/src/java/wicket/AjaxHandler.java,v
retrieving revision 1.2
diff -u -r1.2 AjaxHandler.java
--- src/java/wicket/AjaxHandler.java 4 Sep 2005 23:47:04 -0000 1.2
+++ src/java/wicket/AjaxHandler.java 7 Sep 2005 04:34:09 -0000
@@ -20,6 +20,8 @@
import java.io.OutputStream;
import java.io.Serializable;
+import java.util.HashSet;
+import java.util.Set;
import wicket.markup.ComponentTag;
import wicket.markup.html.HtmlHeaderContainer;
@@ -50,9 +52,6 @@
/** thread local for head contributions. */
private static final ThreadLocal headContribHolder = new ThreadLocal();
- /** we just need one simple indicator object to put in our thread
locals. */
- private static final Object dummy = new Object();
-
/**
* Construct.
*/
@@ -66,11 +65,26 @@
public final String getBodyOnload()
{
String staticContrib = null;
- if (bodyOnloadContribHolder.get() == null)
+ Set contributors = (Set)bodyOnloadContribHolder.get();
+
+ // were any contributors set?
+ if (contributors == null)
+ {
+ contributors = new HashSet(1);
+ bodyOnloadContribHolder.set(contributors);
+ }
+
+ // get the id of the implementation; we need this trick to be
+ // able to support multiple implementations
+ String implementationId = getImplementationId();
+
+ // was a contribution for this specific implementation done yet?
+ if(!contributors.contains(implementationId))
{
- bodyOnloadContribHolder.set(dummy);
staticContrib = getBodyOnloadInitContribution();
+ contributors.add(implementationId);
}
+
String contrib = getBodyOnloadContribution();
if (staticContrib != null)
{
@@ -84,11 +98,26 @@
*/
public final void renderHead(HtmlHeaderContainer container)
{
- if (headContribHolder.get() == null)
+ Set contributors = (Set)headContribHolder.get();
+
+ // were any contributors set?
+ if (contributors == null)
+ {
+ contributors = new HashSet(1);
+ headContribHolder.set(contributors);
+ }
+
+ // get the id of the implementation; we need this trick to be
+ // able to support multiple implementations
+ String implementationId = getImplementationId();
+
+ // was a contribution for this specific implementation done yet?
+ if(!contributors.contains(implementationId))
{
- headContribHolder.set(dummy);
renderHeadInitContribution(container);
+ contributors.add(implementationId);
}
+
renderHeadContribution(container);
}
@@ -111,6 +140,16 @@
}
/**
+ * Gets the unique id of an ajax implementation. This should be
implemented by
+ * base classes only - like the dojo or scriptaculous implementation -
to provide
+ * a means to differentiate between implementations while not going to
the level
+ * of concrete implementations. It is used to ensure 'static' header
contributions
+ * are done only once per implementation.
+ * @return unique id of an ajax implementation
+ */
+ protected abstract String getImplementationId();
+
+ /**
* Gets the response to render to the requester.
* @return the response to render to the requester
*/
Index: src/java/wicket/markup/html/ajax/dojo/DojoAjaxHandler.java
===================================================================
RCS file:
/cvsroot/wicket/wicket/src/java/wicket/markup/html/ajax/dojo/DojoAjaxHandler.java,v
retrieving revision 1.3
diff -u -r1.3 DojoAjaxHandler.java
--- src/java/wicket/markup/html/ajax/dojo/DojoAjaxHandler.java 4 Sep 2005
23:41:20 -0000 1.3
+++ src/java/wicket/markup/html/ajax/dojo/DojoAjaxHandler.java 7 Sep 2005
04:34:09 -0000
@@ -70,4 +70,12 @@
addJsReference(container, new PackageResourceReference(
Application.get(), DojoAjaxHandler.class,
"dojo.js"));
}
+
+ /**
+ * @see AjaxHandler#getImplementationId()
+ */
+ protected final String getImplementationId()
+ {
+ return "DojoImpl";
+ }
}
Index:
src/java/wicket/markup/html/ajax/scriptaculous/ScriptaculousAjaxHandler.java
===================================================================
RCS file:
/cvsroot/wicket/wicket/src/java/wicket/markup/html/ajax/scriptaculous/ScriptaculousAjaxHandler.java,v
retrieving revision 1.3
diff -u -r1.3 ScriptaculousAjaxHandler.java
---
src/java/wicket/markup/html/ajax/scriptaculous/ScriptaculousAjaxHandler.java
4 Sep 2005 23:41:20 -0000 1.3
+++
src/java/wicket/markup/html/ajax/scriptaculous/ScriptaculousAjaxHandler.java
7 Sep 2005 04:34:09 -0000
@@ -86,4 +86,12 @@
addJsReference(container, new
PackageResourceReference(application,
ScriptaculousAjaxHandler.class, "util.js"));
}
+
+ /**
+ * @see AjaxHandler#getImplementationId()
+ */
+ protected final String getImplementationId()
+ {
+ return "ScriptaculousImpl";
+ }
}
