JSPWiki
  1. JSPWiki
  2. JSPWIKI-665

Page View Plugin and page renames and deletions

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.8.4, 2.9
    • Fix Version/s: 2.9.1
    • Component/s: Core & storage
    • Labels:
      None

      Description

      When a wiki page is renamed or deleted, the counters used by the PageViewPlugin (http://www.jspwiki.org/wiki/PageViewPlugin) are not adapted accordingly. As a results of this, the overview of counters can contains pages that do not exist anymore. The counters of these pages will not be incremented anymore, so they will disappear of the top list, but they still remain in the full list. It would make sense to remove the counters for deleted pages. It would defnitely make sense to update the counters when a page is renamed.

      A visit to a non existing page is also stored by the PageViewPlugin, eg by clicking on a link to a page showing the message "This page does not exist. Why don't you go and create it?" is stored as a valid visit. Counting visits to non existing pages seems something of very low value.

      1. PVPTpatch.diff
        3 kB
        Glen Mazza
      2. JSPWiki665.patch
        6 kB
        Glen Mazza

        Issue Links

          Activity

          Hide
          Harry Metske added a comment -

          fixed in 2.9.1-svn-1.

          Show
          Harry Metske added a comment - fixed in 2.9.1-svn-1.
          Hide
          Harry Metske added a comment -

          Glen, just to let you know, I hope to get me some time this weekend to get this patch in.

          kind regards,
          Harry

          Show
          Harry Metske added a comment - Glen, just to let you know, I hope to get me some time this weekend to get this patch in. kind regards, Harry
          Hide
          Glen Mazza added a comment -

          Enclosed patch does not handle renames (yet), but does appear to handle deletes fine. With a delete, the count in the PageViewPlugin for that page resets to zero. The patch also fully fixes JSPWIKI-750 so a specialized test suite is no longer necessary.

          In this patch, I created a temporary nonStaticDeleteTestPage in TestEngine (needed because a "delete page" event needs to be thrown) that sits alongside the current static deleteTestPage. If this patch is accepted, I plan on soon sending another patch getting rid of the latter and renaming the former to the latter. I'm holding back on that now because it's a nontrivial amount of work and I don't want to go too far into the patch if there's a problem with it for other reasons.

          I also noticed the current trunk doesn't seem to do a good job in incrementing page views (perhaps it's just because my IP address viewing it and the page view plugin is careful not to increment multiple times under those circumstances--I'm unsure.) AFAICT that's unrelated to this patch.

          Test case: Create a list of most frequently accessed pages (show=list), as described here: http://www.jspwiki.org/wiki/PageViewPlugin, along with a few pages that have the PageViewPlugin defined within them. Then delete a page--you'll note the deleted page is still in the PVP's count. After applying this patch, a page delete will result in that page being removed from the PVP's list.

          Show
          Glen Mazza added a comment - Enclosed patch does not handle renames (yet), but does appear to handle deletes fine. With a delete, the count in the PageViewPlugin for that page resets to zero. The patch also fully fixes JSPWIKI-750 so a specialized test suite is no longer necessary. In this patch, I created a temporary nonStaticDeleteTestPage in TestEngine (needed because a "delete page" event needs to be thrown) that sits alongside the current static deleteTestPage. If this patch is accepted, I plan on soon sending another patch getting rid of the latter and renaming the former to the latter. I'm holding back on that now because it's a nontrivial amount of work and I don't want to go too far into the patch if there's a problem with it for other reasons. I also noticed the current trunk doesn't seem to do a good job in incrementing page views (perhaps it's just because my IP address viewing it and the page view plugin is careful not to increment multiple times under those circumstances--I'm unsure.) AFAICT that's unrelated to this patch. Test case: Create a list of most frequently accessed pages (show=list), as described here: http://www.jspwiki.org/wiki/PageViewPlugin , along with a few pages that have the PageViewPlugin defined within them. Then delete a page--you'll note the deleted page is still in the PVP's count. After applying this patch, a page delete will result in that page being removed from the PVP's list.
          Hide
          Florian Holeczek added a comment -

          Linking this issue to JSPWIKI-750

          Show
          Florian Holeczek added a comment - Linking this issue to JSPWIKI-750
          Hide
          Glen Mazza added a comment -

          This problem is making testing via the PageViewPluginTest difficult. Enclosed patch fixes the problem with PVPT's testShowCountsBasic() by using new pages not referenced by any other test. However, it introduces a problem in testShowCountsSorted() because it ends up seeing new pages (already deleted in testShowCountsBasic()) that end up conflicting with what it is trying to test. Along with attached patch, I'd recommend disabling (ignoring) the testShowCountsSorted() just to allow JSPWiki to build until this issue is fixed.

          Show
          Glen Mazza added a comment - This problem is making testing via the PageViewPluginTest difficult. Enclosed patch fixes the problem with PVPT's testShowCountsBasic() by using new pages not referenced by any other test. However, it introduces a problem in testShowCountsSorted() because it ends up seeing new pages (already deleted in testShowCountsBasic()) that end up conflicting with what it is trying to test. Along with attached patch, I'd recommend disabling (ignoring) the testShowCountsSorted() just to allow JSPWiki to build until this issue is fixed.

            People

            • Assignee:
              Harry Metske
              Reporter:
              Bruno Peeters
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development