[xwiki-devs] [proposal] Fix XWIKI-4265 by changing the way the previous version of a document is found.
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded. doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private. everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first. What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion() #2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive() #3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive() #4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment. WDYT? Caleb
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment.
What would you have to use instead of XWikiDocument.getPreviousVersion() ?
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
Thomas Mortagne wrote:
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment.
What would you have to use instead of XWikiDocument.getPreviousVersion() ?
api.Document.getPreviousVersion() My rationale is that we should use public api in internal code as often as possible because public api is supposed to always stay the same and internal code will thus become stronger. Also it will mean that an accidental api break will be more likely to cause test failures.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On Mon, Apr 12, 2010 at 16:58, Caleb James DeLisle <[email protected]> wrote:
Thomas Mortagne wrote:
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment.
What would you have to use instead of XWikiDocument.getPreviousVersion() ?
api.Document.getPreviousVersion() My rationale is that we should use public api in internal code as often as possible because public api is supposed to always stay the same and internal code will thus become stronger. Also it will mean that an accidental api break will be more likely to cause test failures.
Maybe but we are far from it. You don't always have the public api when you are manipulating documents and having to create a Document just for it would be a pain...
WDYT?
Caleb
_______________________________________________ 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
On 04/12/2010 04:58 PM, Caleb James DeLisle wrote:
Thomas Mortagne wrote:
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment.
What would you have to use instead of XWikiDocument.getPreviousVersion() ?
api.Document.getPreviousVersion()
-0 First, the API is not usually available inside the core, and it shouldn't since actually this is not a service API, but a public scripting API. Second, going to the API is an unneeded roundtrip, since the API should not contain any logic, it should just forward the call to the internal method, which is again XWikiDocument.getPreviousVersion. Third, the API might actually disappear in the future, once we move to the 2.0 model and add @Security and @Access annotations to it to mark public (scriptable) API methods and their associated access rights requirements.
My rationale is that we should use public api in internal code as often as possible because public api is supposed to always stay the same and internal code will thus become stronger. Also it will mean that an accidental api break will be more likely to cause test failures.
WDYT?
Caleb
-- Sergiu Dumitriu http://purl.org/net/sergiu/
Sergiu Dumitriu wrote:
On 04/12/2010 04:58 PM, Caleb James DeLisle wrote:
Thomas Mortagne wrote:
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment. What would you have to use instead of XWikiDocument.getPreviousVersion() ? api.Document.getPreviousVersion()
-0
First, the API is not usually available inside the core, and it shouldn't since actually this is not a service API, but a public scripting API.
Second, going to the API is an unneeded roundtrip, since the API should not contain any logic, it should just forward the call to the internal method, which is again XWikiDocument.getPreviousVersion.
Third, the API might actually disappear in the future, once we move to the 2.0 model and add @Security and @Access annotations to it to mark public (scriptable) API methods and their associated access rights requirements.
I thought the API would outlast the rest of the old core since removing the API would break every application and snippet. I did some find-grepping and it seems that getPreviousVersion is not used anywhere in platform, watch, xoffice, xeclipse, or workspaces. I don't see the reasoning for having no logic in the API and I hate to add methods to XWikiDocument but another option is to add a method XWikiDocument.getPreviousVersion(XWikiContext)
My rationale is that we should use public api in internal code as often as possible because public api is supposed to always stay the same and internal code will thus become stronger. Also it will mean that an accidental api break will be more likely to cause test failures.
WDYT?
Caleb
Based on the response, I am amending the proposal to add doc.XWikiDocument.getPreviousVersion(XWikiContext) in case future development in the old core finds it useful. api.Document.getPreviousVersion() will be redirected to the new method. WDYT? Caleb Caleb James DeLisle wrote:
Sergiu Dumitriu wrote:
On 04/12/2010 04:58 PM, Caleb James DeLisle wrote:
Thomas Mortagne wrote:
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment. What would you have to use instead of XWikiDocument.getPreviousVersion() ? api.Document.getPreviousVersion() -0
First, the API is not usually available inside the core, and it shouldn't since actually this is not a service API, but a public scripting API.
Second, going to the API is an unneeded roundtrip, since the API should not contain any logic, it should just forward the call to the internal method, which is again XWikiDocument.getPreviousVersion.
Third, the API might actually disappear in the future, once we move to the 2.0 model and add @Security and @Access annotations to it to mark public (scriptable) API methods and their associated access rights requirements.
I thought the API would outlast the rest of the old core since removing the API would break every application and snippet. I did some find-grepping and it seems that getPreviousVersion is not used anywhere in platform, watch, xoffice, xeclipse, or workspaces. I don't see the reasoning for having no logic in the API and I hate to add methods to XWikiDocument but another option is to add a method XWikiDocument.getPreviousVersion(XWikiContext)
My rationale is that we should use public api in internal code as often as possible because public api is supposed to always stay the same and internal code will thus become stronger. Also it will mean that an accidental api break will be more likely to cause test failures.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
I have found an easier way! XWikiDocument#getDocumentArchive() can be set to call loadDocumentArchive(XWikiContext) with an XWikiContext from the ExecutionContext, gotten by way of either XWikiDocument#getContext() or XWikiDocument#getXWikiContext() WDYT? Caleb Caleb James DeLisle wrote:
Based on the response, I am amending the proposal to add doc.XWikiDocument.getPreviousVersion(XWikiContext) in case future development in the old core finds it useful. api.Document.getPreviousVersion() will be redirected to the new method.
WDYT?
Caleb
Caleb James DeLisle wrote:
Sergiu Dumitriu wrote:
On 04/12/2010 04:58 PM, Caleb James DeLisle wrote:
Thomas Mortagne wrote:
On Mon, Apr 12, 2010 at 15:12, Caleb James DeLisle <[email protected]> wrote:
Currently, api.Document.getPreviousVersion() calls doc.XWikiDocument.getPreviousVersion() which calls doc.XWikiDocument.getDocumentArchive() which will return null if the document archive is not currently loaded.
doc.XWikiDocument.getPreviousVersion() is inherently dangerous and should be deprecated then removed. doc.XWikiDocument.getDocumentArchive() sometimes returns null and should be deprecated then made private.
everywhere doc.XWikiDocument.getDocumentArchive() is used it should be replaced with doc.XWikiDocument.getDocumentArchive(XWikiContext) which calls loadDocumentArchive first.
What I propose we do now (for 2.3) #1 Change api.Document.getPreviousVersion() to call getDocumentArchive(getXWikiContext()) and move the logic from doc.XWikiDocument.getPreviousVersion() into api.Document.getPreviousVersion()
#2 change doc.XWikiDocument.copyDocument(DocumentReference newDocumentReference, XWikiContext context) to call getDocumentArchive(XWikiContext) instead of getDocumentArchive()
#3 Add warnings in javadoc for: clone(XWikiDocument document) cloneInternal(DocumentReference newDocumentReference, boolean keepsIdentity) to say that they may not copy the archive since they use getDocumentArchive()
#4 mark doc.XWikiDocument.getPreviousVersion() and doc.XWikiDocument.getDocumentArchive() as depricated and explain why in a comment. What would you have to use instead of XWikiDocument.getPreviousVersion() ? api.Document.getPreviousVersion() -0
First, the API is not usually available inside the core, and it shouldn't since actually this is not a service API, but a public scripting API.
Second, going to the API is an unneeded roundtrip, since the API should not contain any logic, it should just forward the call to the internal method, which is again XWikiDocument.getPreviousVersion.
Third, the API might actually disappear in the future, once we move to the 2.0 model and add @Security and @Access annotations to it to mark public (scriptable) API methods and their associated access rights requirements. I thought the API would outlast the rest of the old core since removing the API would break every application and snippet. I did some find-grepping and it seems that getPreviousVersion is not used anywhere in platform, watch, xoffice, xeclipse, or workspaces. I don't see the reasoning for having no logic in the API and I hate to add methods to XWikiDocument but another option is to add a method XWikiDocument.getPreviousVersion(XWikiContext)
My rationale is that we should use public api in internal code as often as possible because public api is supposed to always stay the same and internal code will thus become stronger. Also it will mean that an accidental api break will be more likely to cause test failures.
WDYT?
Caleb
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
participants (3)
-
Caleb James DeLisle -
Sergiu Dumitriu -
Thomas Mortagne