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.

>> +
>>      /**
>>       * @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).

>> +    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
>



-- 
Thomas Mortagne
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to