On Sat, Apr 17, 2010 at 11:35, Caleb James DeLisle <[email protected]> wrote:
Thomas Mortagne wrote:
On Sat, Apr 17, 2010 at 10:47, Caleb James DeLisle <[email protected]> wrote:
(resending, meant to send to the dev list.) Sorry to drag up old commits but I have a couple of questions.
vmassol (SVN) wrote:
Author: vmassol Date: 2010-01-07 23:02:51 +0100 (Thu, 07 Jan 2010) New Revision: 26074
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java Log: XWIKI-4677: Introduce new Model module
* Fixed infinite cycle (revert some previous change). We need to find a solution. * Refactored existing code to prevent manual parsing
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java 2010-01-07 16:55:03 UTC (rev 26073) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/XWikiDocument.java 2010-01-07 22:02:51 UTC (rev 26074) @@ -80,6 +80,7 @@ import org.xwiki.context.ExecutionContextManager; import org.xwiki.model.reference.DocumentReferenceResolver; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.model.reference.WikiReference; import org.xwiki.rendering.block.Block; import org.xwiki.rendering.block.HeaderBlock; import org.xwiki.rendering.block.LinkBlock; @@ -359,8 +360,10 @@ /** * Used to create proper {@link Syntax} objects. */ - SyntaxFactory syntaxFactory = Utils.getComponent(SyntaxFactory.class); + private SyntaxFactory syntaxFactory = Utils.getComponent(SyntaxFactory.class);
Why are we holding a hard reference on the Execution which was present when the document was first loaded or created? Is this not a memory leak? Cache holds Document holds Execution holds ExecutionContext holds XWikiContext holds DocumentArchive?
+ private Execution execution = Utils.getComponent(Execution.class);
Execution is not the context, it's the context manager, when you call Execution#getContext you get the context object dynamically from a threadlocal. In other words Execution is a singleton component. Ahh that makes me feel much better. I was beginning to see bugs everywhere.
+ /** * @since 2.2M1 */ @@ -375,7 +378,9 @@ @Deprecated public XWikiDocument() { - this(Utils.getComponent(DocumentReferenceResolver.class).resolve("")); + // TODO: Replace this with the following when we find a way to not generate a cycle: + // this(Utils.getComponent(DocumentReferenceResolver.class).resolve("")); + this(new DocumentReference("xwiki", "Main", "WebHome")); }
/** @@ -397,25 +402,18 @@ * * @param wiki The wiki this document belongs to. * @param space The space this document belongs to. - * @param name The name of the document. + * @param name The name of the document (can contain either the page name or the space and page name) * @deprecated since 2.2M1 use {@link #XWikiDocument(org.xwiki.model.reference.DocumentReference)} instead */ @Deprecated public XWikiDocument(String wiki, String space, String name) { - String normalizedPage; - String normalizedSpace; - int i1 = name.indexOf("."); - if (i1 == -1) { - normalizedPage = name; - normalizedSpace = space; - } else { - normalizedSpace = name.substring(0, i1); - normalizedPage = name.substring(i1 + 1); - } - - init(Utils.getComponent(DocumentReferenceResolver.class, "current/reference").resolve( - new DocumentReference(wiki, normalizedSpace, normalizedPage))); + // We allow to specify the space in the name (eg name = "space.page"). In this case the passed space is + // ignored. + DocumentReference reference = resolveReference(name, new DocumentReference(wiki, space, null)); + // Replace the resolved wiki by the passed wiki + reference.setWikiReference(new WikiReference(wiki)); + init(reference); }
public XWikiStoreInterface getStore(XWikiContext context) @@ -5846,4 +5844,21 @@
return syntaxId; } + + private DocumentReference resolveReference(String referenceAsString, DocumentReference defaultReference) + { + XWikiContext xcontext = getXWikiContext(); + XWikiDocument originalCurentDocument = xcontext.getDoc(); + try { + xcontext.setDoc(new XWikiDocument(defaultReference)); + return this.currentDocumentReferenceResolver.resolve(referenceAsString); + } finally { + xcontext.setDoc(originalCurentDocument); + } + } + How is this different from XWikiDocument#getContext() introduced in 23506? Is the only difference that the context is the one from when the document was created/loaded rather than the current context? If so this probably ought to be commented since it is a major trap for anyone who expects getXWikiContext().getUser() to yield the current user (for example).
Indeed getContext() and getXWikiContext() are doing exactly the same thing, one of them should be removed, probably getContext() which is less explicit (but getXWikiContext() should probably protect from uninitialized Execution like getContext() do).
Maybe keep the code of getContext but change the name to getXWikiContext and remove the reference to the Execution and let the component manager provide it as needed? WDYT?
I let you choose ;)
+ private XWikiContext getXWikiContext() + { + return (XWikiContext) this.execution.getContext().getProperty("xwikicontext"); + } }
Modified: platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java =================================================================== --- platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java 2010-01-07 16:55:03 UTC (rev 26073) +++ platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/doc/XWikiDocumentTest.java 2010-01-07 22:02:51 UTC (rev 26074) @@ -155,6 +155,29 @@ this.mockXWikiStoreInterface.stubs().method("search").will(returnValue(new ArrayList<XWikiDocument>())); }
+ public void testConstructor() + { + XWikiDocument doc = new XWikiDocument("notused", "space.page"); + assertEquals("space", doc.getSpaceName()); + assertEquals("page", doc.getPageName()); + assertEquals("xwiki", doc.getWikiName()); + + doc = new XWikiDocument("space", "page"); + assertEquals("space", doc.getSpaceName()); + assertEquals("page", doc.getPageName()); + assertEquals("xwiki", doc.getWikiName()); + + doc = new XWikiDocument("wiki", "space", "page"); + assertEquals("space", doc.getSpaceName()); + assertEquals("page", doc.getPageName()); + assertEquals("wiki", doc.getWikiName()); + + doc = new XWikiDocument("wiki", "notused", "notused:space.page"); + assertEquals("space", doc.getSpaceName()); + assertEquals("page", doc.getPageName()); + assertEquals("wiki", doc.getWikiName()); + } + public void testGetDisplayTitleWhenNoTitleAndNoContent() { this.document.setContent("Some content");
_______________________________________________ notifications mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/notifications
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne