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

Reply via email to