[MGNLCMNT-97] Flushing cache after commenting is triggered too late Created: 18/Feb/14  Updated: 05/Mar/14  Resolved: 05/Mar/14

Status: Closed
Project: Commenting (closed)
Component/s: Cache, client-ui
Affects Version/s: None
Fix Version/s: 2.2

Type: Bug Priority: Critical
Reporter: Christoph Meier Assignee: Christoph Meier
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MGNLCMNT-94 make sure commenting works with anony... Closed
Template:
Acceptance criteria:
Empty
Date of First Response:

 Description   

Avoid sending cached response after a comment has been posted!

When a comment is posted, the page which "has" the comments must be flushed from the cache; this triggered by info.magnolia.module.commenting.cache.ReferencedPageFlushPolicy.
Unfortunatly, this happens mostly too late.
=> The user which posted a comment has to reload the page to see the comment.
Triggering the flushing of the page should occur earlier.

Note, that this is a follow-up-ticket of MGNLCMNT-94



 Comments   
Comment by Christoph Meier [ 04/Mar/14 ]

Using a query-param when redirecting the page to ensure its not server from cache.

Note: The last version which didn't have that problem and where MGNLCACHE-54 was not an issue was on Magnolia 4.2.9 / commenting-1.1.1.
There, PageComments#executeEarly() was not doing a redirect. If that redirect is not occuring, it works fine, the response showed after posting a comment is "fresh" (not serverd from cache).
But since MGNLCMNT-67 this redirect occurs (and should not be removed), and so, during redirect-request, cached-content will be served; to avoid exactly that, the redirection URI gets a query-param.

Comment by Roman Kovařík [ 05/Mar/14 ]
  1. Could you please add test? Something like:
    @Test
        public void test() throws Exception {
            // GIVEN
            comment = new ExtendsPageComments(nodeContent, renderable, model);
            // WHEN
            comment.executeEarly();
            // THEN
            verify(response).sendRedirect("....");
        }
    
  2. Why is the query like 1394005623740=1? I know that this is only because of caching, but wouldn't be something like parameterName=1394005623740 better?
  3. Use import import static org.mockito.Mockito.*;
Comment by Christoph Meier [ 05/Mar/14 ]

I added a test which checks that the redirec-links contains a query. => PageCommentsTest#testRedirectAfterPostContainsQuery

The already existing PageCommentsTest#testEnterWhitespaceCharIntoComment was not really meaningful. (I think it was never properly finished).
I extended it a little bit that it is testing at least some minimal behaviour.

Comment by Roman Kovařík [ 05/Mar/14 ]
  • These assertions always pass, because e.g first parameter would have value "forumId":
    verify(forumManager, never()).replyToThread("","","","","",false);
    verify(forumManager, never()).createThread("","","","",false);
    

    Better would be to do:

    • A)
      verify(forumManager).getForumId(...);
      verifyNoMoreInteractions(forumManager)
      
    • B)
      verify(forumManager, never()).replyToThread(anyString(), anyString(), anyString(), anyString(), anyString(), anyBoolean());
      ...
      
  • Could you also remove extra catch block on the line 202?
  • info.magnolia.module.commenting.frontend.action.PageCommentsTest.testRedirectAfterPostContainsQuery() looks good.
Generated at Mon Feb 12 00:03:07 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.