[MAGNOLIA-547] Workaround for non-synchronized sessions and unnecessary invalidation of session removed Created: 10/Sep/05  Updated: 28/Sep/05  Resolved: 27/Sep/05

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 2.1 Final
Fix Version/s: 2.1.1, 3.0 Beta 1

Type: Bug Priority: Critical
Reporter: Michael Aemisegger Assignee: Fabrizio Giustina
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 1h
Time Spent: Not Specified
Original Estimate: 1h
Environment:

all


Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

---------- History start ----------
I hadtrouble with non synchronized hierarchy managers.

I create a page and rename it to "parentPage". Then, I create another page as a child of the first page and rename it to "childPage". That's when I run into trouble:

hm.getContent("/parentPage/untitled") does still return the old content, which should not exist anymore.

hm.getContent("/parentPage").getContent("/untitled") instead, throws a PathNotFoundException as expected.

hm.getContent("/parentPage/childPage") in turn also returns the newly renamed content.

The two return values have same id, session, startPage etc. I couldn't find any difference.

Also hm.save() or parentPage.save() didn't help, which is very strange, since http://incubator.apache.org/jackrabbit/apidocs/org/apache/jackrabbit/core/package-summary.html#package_description says that jackrabbit uses copy-on-write strategy. A save shouldn't be needed at all according to http://www.day.com/maven/jsr170/javadocs/jcr-0.16.4.1/javax/jcr/Workspace.html#move(java.lang.String, java.lang.String)

I tried to reproduce this error on the demo instance of magnolia. But everything went well. Then I found the difference between my code and the original code:

SessionAccessControl.invalidateUser(this.getRequest());

in Tree.rename().
---------- History end ----------

Further researching revealed
http://issues.apache.org/jira/browse/JCR-155

So, I finally changed the HierarchyManager to perform the move on the session and then save it, rather than performing the move on the workspace itself.

This allowed me to remove the malicious SessionAccessControl.invalidateUser(this.getRequest()) calls in Tree.java. It's not very pleasant to have the session invalidated just because you perform CRUD operations on the repository. A quick test suggests that also magnolia does not have problems anymore without these calls.

patch for revision 1437 of tagged version /magnolia2.1
Note that the patch for Tree.java might already have been applied partly :

diff -uBbPr originals/magnolia-2.1/src/main/info/magnolia/cms/core/HierarchyManager.java patched/magnolia-2.1/src/main/info/magnolia/cms/core/HierarchyManager.java
— originals/magnolia-2.1/src/main/info/magnolia/cms/core/HierarchyManager.java 2005-08-31 11:29:25.000000000 +0200
+++ patched/magnolia-2.1/src/main/info/magnolia/cms/core/HierarchyManager.java 2005-09-10 14:22:39.000000000 +0200
@@ -476,7 +476,12 @@
AccessDeniedException

{ Access.isGranted(this.accessManager, source, Permission.REMOVE); Access.isGranted(this.accessManager, destination, Permission.WRITE); - this.workSpace.move(source, destination); + /* + * maem: rather use session because of caching bug + * see http://issues.apache.org/jira/browse/JCR-155 + */ + this.workSpace.getSession().move(source, destination); + this.workSpace.getSession().save(); }

/**

diff -uBbPr originals/magnolia-2.1/src/main/info/magnolia/cms/gui/control/Tree.java patchedForSendingPatches/magnolia-2.1/src/main/info/magnolia/cms/gui/control/Tree.java
— originals/magnolia-2.1/src/main/info/magnolia/cms/gui/control/Tree.java 2005-08-31 11:29:16.000000000 +0200
+++ patchedForSendingPatches/magnolia-2.1/src/main/info/magnolia/cms/gui/control/Tree.java 2005-09-10 14:45:37.000000000 +0200
@@ -785,7 +785,6 @@
// copy
hm.copyTo(source, destination);
}

  • SessionAccessControl.invalidateUser(this.getRequest());
    Content newContent = hm.getContent(destination);
    try { newContent.updateMetaData(this.getRequest()); @@ -860,7 +859,6 @@ parent.orderBefore(newLabel, placedBefore); }

    }

  • SessionAccessControl.invalidateUser(this.getRequest());
    Content newPage = hm.getContent(dest);
    returnValue = newLabel;
    newPage.updateMetaData(this.getRequest());
    @@ -1090,9 +1088,7 @@
    html.append("</script>"); //$NON-NLS-1$

// contextmenu

  • if (menu.getMenuItems().size() != 0) { html.append(menu.getHtml()); - }

// register menu
html.append("<script>" + this.getJavascriptTree() + ".menu = " + menu.getName() + "</script>"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$



 Comments   
Comment by Fabrizio Giustina [ 27/Sep/05 ]

committed to svn, thanks
seems to work properly: Sameer, do you think the SessionAccessControl.invalidateUser() should be needed for some reason?

Comment by Sameer Charles [ 27/Sep/05 ]

I do not see any reason of invalidating user after copy or move operation, although I remember in very early jcr implementation
you needed to log in again to the repository after these operations.

Comment by Michael Aemisegger [ 27/Sep/05 ]

The reason for the invalidation most probably was due to the jackrabbit bug http://issues.apache.org/jira/browse/JCR-155 .

I guess, acquiring a new http session forced the workspace session to be built from scratch which circumvented above problems.

This bug is claimed to be fixed, however for some reason I still suffered the same problem. I'm not sure whether it was because the jackrabbit build retrieved by maven was not yet updated or the fix did not improve the situation. So I still need to remove some session attributes where the session invalidation took place:

/*

private HierarchyManager refreshHierarchyManager( String repository )

{ HierarchyManager hierarchyManager; this.getRequest().getSession().removeAttribute( "mgnlHMgr_" + repository + "_" + ContentRepository.DEFAULT_WORKSPACE ); this.getRequest().getSession().removeAttribute( "mgnlAccessMgr_" + repository + "_default" ); this.getRequest().getSession().removeAttribute( "mgnlRepositorySession_" + repository + "_default" ); hierarchyManager = SessionAccessControl.getHierarchyManager( this.getRequest(), this.getRepository() ); return hierarchyManager; }

Maybe with a truly new jackrabbit snapshot, this code will not be necessary anymore.

This issue should carefully be tested, e.g. with different users logging into the system and modifying content sequentially and concurrently.

Comment by Philipp Bracher [ 28/Sep/05 ]

I commited the new jackrabbit version (r292180) to the 2.1 branch and made the berkeley db persistence manager to the default choice.

Hope this helps (at least some strange bugs I had in the dms are gone now)

Generated at Mon Feb 12 03:18:30 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.