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
