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-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
