On 04/17/2010 10:47 AM, Caleb James DeLisle 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?
Speaking of memory leaks, here's something I noticed:
Start a standalone wiki, visit some pages, then leave it open for a
while (1-2 days), then try to access the wiki. Out of heap space.
This makes me think that the memory leak is somewhere in a background
thread, Lucene, Scheduler, distributed events...
>> + private Execution execution = Utils.getComponent(Execution.class);
>> +
>> /**
>> * @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).
>> + 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");
>>
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs