Wicket
  1. Wicket
  2. WICKET-1501

MarkupCache.putIntoCache doesn't behave correctly!!

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.3.3
    • Fix Version/s: 1.3.4, 1.4-M1
    • Component/s: wicket
    • Labels:
      None

      Description

      'putIntoCache' checks for the locationString not to be null instead of the cacheKey!

      This way you always get old markup returned instead of the freshly supplied markup which shouldn't be cached at all.

        Activity

        Hide
        Johan Compagner added a comment -

        ahh ok so loadMarkup is the method that is incorrect.

        I will fix that that putInCache is not called when cache key == null

        Show
        Johan Compagner added a comment - ahh ok so loadMarkup is the method that is incorrect. I will fix that that putInCache is not called when cache key == null
        Hide
        Jan Kriesten added a comment -

        That's what I was trying to say:

        Even though cacheKey is 'null' - the markup is still cached under locationString.

        Since the resulting Markup is always returned from 'putIntoCache' - the originally updated markup in the parameter to 'putIntoCache' is thrown away and replaced by the old cached one...

        Regards, — Jan.

        Show
        Jan Kriesten added a comment - That's what I was trying to say: Even though cacheKey is 'null' - the markup is still cached under locationString. Since the resulting Markup is always returned from 'putIntoCache' - the originally updated markup in the parameter to 'putIntoCache' is thrown away and replaced by the old cached one... Regards, — Jan.
        Hide
        Johan Compagner added a comment -

        if you have a null cache key then markup cache should ignore it completely
        Where is i still put into the cache even if the cache key is null?

        Show
        Johan Compagner added a comment - if you have a null cache key then markup cache should ignore it completely Where is i still put into the cache even if the cache key is null?
        Hide
        Jan Kriesten added a comment -

        hi johan,

        huh?? i have no cache key for the template - it just shall not be handled by wicket's cache system (it's already cached by freemarker and changes every request)....

        so - not having a cache key means i can't lookup the locationString in the first map, right?

        regards, — jan.

        Show
        Jan Kriesten added a comment - hi johan, huh?? i have no cache key for the template - it just shall not be handled by wicket's cache system (it's already cached by freemarker and changes every request).... so - not having a cache key means i can't lookup the locationString in the first map, right? regards, — jan.
        Hide
        Johan Compagner added a comment -

        cache keys shouldn't be the keys that go into the markpuCache
        they should only go into the markupKeyCache map

        Where is a cache key going into the markupCache?

        it is now 2 fold you have:

        Map<CacheKey,LocationString>
        Map<LocationString,Markup>

        that is how it should work now.

        Show
        Johan Compagner added a comment - cache keys shouldn't be the keys that go into the markpuCache they should only go into the markupKeyCache map Where is a cache key going into the markupCache? it is now 2 fold you have: Map<CacheKey,LocationString> Map<LocationString,Markup> that is how it should work now.
        Hide
        Jan Kriesten added a comment -

        Johan -

        I return 'null' for the cacheKey of my template. That's fine. The Markup isn't cached under_this key.

        The problem arises because wicket caches the Markup in putIntoCache also under the locationString - which has nothing to do with the cacheKey. So, markup is cached under two keys by wicket, which leads to the problem.

        Regards, — Jan.

        Show
        Jan Kriesten added a comment - Johan - I return 'null' for the cacheKey of my template. That's fine. The Markup isn't cached under_this key. The problem arises because wicket caches the Markup in putIntoCache also under the locationString - which has nothing to do with the cacheKey. So, markup is cached under two keys by wicket, which leads to the problem. Regards, — Jan.
        Hide
        Johan Compagner added a comment -

        i dont get it, the method putIntoCache SHOULD check for the location string. Because that is the one that goes into the markupCache
        the cacheKey doesn't go into that markupCache at all anymore that only goes into the markupKeyCache

        So as far as i can see the putIntoCache method is fine.
        And why it also can return it is this:

        // We don't lock the cache while loading a markup. Thus it may
        // happen that the very same markup gets loaded twice (the first
        // markup being loaded, but not yet in the cache, and another
        // request requesting the very same markup). Since markup
        // loading in avg takes less than 100ms, it is not really an
        // issue. For consistency reasons however, we should always use
        // the markup loaded first which is why it gets returned.
        markup = (Markup)markupCache.get(locationString);

        So thats threadding.
        putIntoCache puts the markup into the cache if not there or if it is there returns the one that is in there (the first one parsed)
        If you have new stuff then that is not the place to overwrite markup. You first have to call
        public final Markup removeMarkup(final String cacheKey)

        Show
        Johan Compagner added a comment - i dont get it, the method putIntoCache SHOULD check for the location string. Because that is the one that goes into the markupCache the cacheKey doesn't go into that markupCache at all anymore that only goes into the markupKeyCache So as far as i can see the putIntoCache method is fine. And why it also can return it is this: // We don't lock the cache while loading a markup. Thus it may // happen that the very same markup gets loaded twice (the first // markup being loaded, but not yet in the cache, and another // request requesting the very same markup). Since markup // loading in avg takes less than 100ms, it is not really an // issue. For consistency reasons however, we should always use // the markup loaded first which is why it gets returned. markup = (Markup)markupCache.get(locationString); So thats threadding. putIntoCache puts the markup into the cache if not there or if it is there returns the one that is in there (the first one parsed) If you have new stuff then that is not the place to overwrite markup. You first have to call public final Markup removeMarkup(final String cacheKey)
        Hide
        Jan Kriesten added a comment -

        I don't actually understand why the markup is loaded from the markupCache in putIntoCache at all - it's already provided as a method argument...

        Show
        Jan Kriesten added a comment - I don't actually understand why the markup is loaded from the markupCache in putIntoCache at all - it's already provided as a method argument...

          People

          • Assignee:
            Johan Compagner
            Reporter:
            Jan Kriesten
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development