You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Martin Dietze <di...@fh-wedel.de> on 2010/08/24 10:21:21 UTC

Cache key collisions (IMarkupCacheKeyProvider)

We've recently run into a situation where in an application
the wrong markup was provided for a component implementing
IMarkupCacheKeyProvider. It is pretty obvious how this happened,
see below, but I'd like to ask whether this behaviour is
actually what we want or a bug.

We have two components, both rendering their cache keys in the
same way:

| public String getCacheKey( final MarkupContainer container,
|                            final Class<?> containerClass ) {
|   return getId();
| }

The cache key was implemented in this simplistic way as the
component would always render markup of which the only dynamic
bit is the ID (an inner component is created with that ID and
then added to the component itself which derives from Panel).

In their respective pages both components were used in a
RepeatingView, thus their IDs were integer numbers as provided
by newChildId().

After having successfully loaded the page containing the first
of the two components loading the page containing the second
component fails. Further analysis revealed the following:

In MarkupCache#getMarkup() the respective components are 
returned as cache key providers; then getMarkupFromCache() is
called on the cache key. The getMarkupFromCache() method has an
argument `MarkupContainer container', but it is not used in the
default implementation.

Consequently, the markup cache returns the markup for the wrong
component (since both happened to be created with the same IDs).

This means that for the worst case one has to take care that
the cache keys created in components such as the above need to
be collision-free across the whole application.

I can only speculate on why the respective component's container
is ignored when looking up the markup from the cache. One could
argue that this approach is simpler, also one could actually
make use of this behaviour by reusing markup across components.

The drawback is that this is error-prone (as in this case). Also
if I use third-party components implementing IMarkupCacheKeyProvider 
the way they create their cache keys is beyond my control which
makes keeping cache keys collision-free potentially problematic.

As an immediate fix I think one should add a warning to the
IMarkupCacheKeyProvider's JavaDoc pointing out that no checking
of the components' hierarchy is performed, so that cache keys
need to be application-wide unique. From my experience adding 
the fully qualified class name to the cache key is usually a
good idea (unfortunately I had considered this unnecessary for
such simplistic components)...

Apart from that I'd like to ask whether this way of dealing with
the markup cache is really what we want?

Cheers,

M'bert

-- 
----------- / http://herbert.the-little-red-haired-girl.org / -------------
=+= 
Ich trink kein Wasser. Da ficken Fische drin...

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org


Re: Cache key collisions (IMarkupCacheKeyProvider)

Posted by Martin Dietze <di...@fh-wedel.de>.
On Tue, August 24, 2010, Igor Vaynberg wrote:

> IMarkupCacheKeyProvider is for advanced users. you have to know what
> you are doing when you implement the interface.

OK, that's fine with me. Why not add a comment to the interface
stating just this plus a hint that in general adding the fully
qualified class name to the cache key is a good idea?

Cheers,

M'bert

-- 
----------- / http://herbert.the-little-red-haired-girl.org / -------------
=+= 
Es genuegt nicht, keine Gedanken zu haben, man muss auch 
unfaehig sein, sie auszudruecken.  -- Stanislav Jerzy Lec

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org


Re: Cache key collisions (IMarkupCacheKeyProvider)

Posted by Igor Vaynberg <ig...@gmail.com>.
IMarkupCacheKeyProvider is for advanced users. you have to know what
you are doing when you implement the interface.

it makes sense to me that the key is application-scoped since there is
really no other scope that makes sense, and yes, like you said, its a
good idea to prefix the key with the fqn unless you want to share the
markup with other componets.

-igor



On Tue, Aug 24, 2010 at 1:21 AM, Martin Dietze <di...@fh-wedel.de> wrote:
> We've recently run into a situation where in an application
> the wrong markup was provided for a component implementing
> IMarkupCacheKeyProvider. It is pretty obvious how this happened,
> see below, but I'd like to ask whether this behaviour is
> actually what we want or a bug.
>
> We have two components, both rendering their cache keys in the
> same way:
>
> | public String getCacheKey( final MarkupContainer container,
> |                            final Class<?> containerClass ) {
> |   return getId();
> | }
>
> The cache key was implemented in this simplistic way as the
> component would always render markup of which the only dynamic
> bit is the ID (an inner component is created with that ID and
> then added to the component itself which derives from Panel).
>
> In their respective pages both components were used in a
> RepeatingView, thus their IDs were integer numbers as provided
> by newChildId().
>
> After having successfully loaded the page containing the first
> of the two components loading the page containing the second
> component fails. Further analysis revealed the following:
>
> In MarkupCache#getMarkup() the respective components are
> returned as cache key providers; then getMarkupFromCache() is
> called on the cache key. The getMarkupFromCache() method has an
> argument `MarkupContainer container', but it is not used in the
> default implementation.
>
> Consequently, the markup cache returns the markup for the wrong
> component (since both happened to be created with the same IDs).
>
> This means that for the worst case one has to take care that
> the cache keys created in components such as the above need to
> be collision-free across the whole application.
>
> I can only speculate on why the respective component's container
> is ignored when looking up the markup from the cache. One could
> argue that this approach is simpler, also one could actually
> make use of this behaviour by reusing markup across components.
>
> The drawback is that this is error-prone (as in this case). Also
> if I use third-party components implementing IMarkupCacheKeyProvider
> the way they create their cache keys is beyond my control which
> makes keeping cache keys collision-free potentially problematic.
>
> As an immediate fix I think one should add a warning to the
> IMarkupCacheKeyProvider's JavaDoc pointing out that no checking
> of the components' hierarchy is performed, so that cache keys
> need to be application-wide unique. From my experience adding
> the fully qualified class name to the cache key is usually a
> good idea (unfortunately I had considered this unnecessary for
> such simplistic components)...
>
> Apart from that I'd like to ask whether this way of dealing with
> the markup cache is really what we want?
>
> Cheers,
>
> M'bert
>
> --
> ----------- / http://herbert.the-little-red-haired-girl.org / -------------
> =+=
> Ich trink kein Wasser. Da ficken Fische drin...
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org