Following a discussion with Vincent, I've created the feature-restimprovements feature branch on the xwiki xwiki-platform github and passed it to fabio who will:
- integrate his own patches for wiki creation and xar upload - move internal code to internal I've create a new pull request for this branch https://github.com/xwiki/xwiki-platform/pull/71 The older pull request (https://github.com/xwiki/xwiki-platform/pull/56) has been closed. Ludovic 2012/10/8 Ludovic Dubost <[email protected]> > Hi, > > All votes told to move the code to internal packages. > > This is something I can do, however I'm not sure exactly which code > should go into the internal package and which one should stay where it > is. > > Right now here is the list of classes failing or not the CLIRR tests > > Not failing: > > org.xwiki.rest.ComponentsObjectFactory > org.xwiki.rest.Constants > org.xwiki.rest.RangeIterable > org.xwiki.rest.Relations > org.xwiki.rest.Utils > org.xwiki.rest.XWikiAuthentication > org.xwiki.rest.XWikiJaxRsApplication > org.xwiki.rest.XWikiResource > org.xwiki.rest.XWikiRestComponent > org.xwiki.rest.XWikiRestletJaxRsApplication > org.xwiki.rest.XWikiRestletServlet > org.xwiki.rest.XWikiSecretVerifier > org.xwiki.rest.XWikiSetupCleanupFilter > org.xwiki.rest.exceptions.QueryExceptionMapper > org.xwiki.rest.exceptions.XWikiExceptionMapper > org.xwiki.rest.representations.TextPlainReader > org.xwiki.rest.representations.comments.FormUrlEncodedCommentReader > org.xwiki.rest.representations.comments.TextPlainCommentReader > org.xwiki.rest.representations.objects.FormUrlEncodedObjectReader > org.xwiki.rest.representations.objects.FormUrlEncodedPropertyReader > org.xwiki.rest.representations.objects.TextPlainPropertyReader > org.xwiki.rest.representations.pages.FormUrlEncodedPageReader > org.xwiki.rest.representations.pages.TextPlainPageReader > org.xwiki.rest.representations.tags.FormUrlEncodedTagsReader > org.xwiki.rest.representations.tags.TextPlainTagsReader > org.xwiki.rest.resources.BrowserAuthenticationResource > org.xwiki.rest.resources.RootResource > org.xwiki.rest.resources.SyntaxesResource > org.xwiki.rest.resources.attachments.AttachmentAtPageVersionResource > org.xwiki.rest.resources.attachments.AttachmentHistoryResource > org.xwiki.rest.resources.attachments.AttachmentResource > org.xwiki.rest.resources.attachments.AttachmentVersionResource > org.xwiki.rest.resources.classes.ClassPropertiesResource > org.xwiki.rest.resources.classes.ClassPropertyResource > org.xwiki.rest.resources.classes.ClassResource > org.xwiki.rest.resources.classes.ClassesResource > org.xwiki.rest.resources.objects.BaseObjectsResource > org.xwiki.rest.resources.pages.ModifiablePageResource > org.xwiki.rest.resources.pages.PageTagsResource > org.xwiki.rest.resources.pages.PageTranslationsResource > org.xwiki.rest.resources.spaces.SpaceResource > org.xwiki.rest.resources.spaces.SpacesResource > org.xwiki.rest.resources.tags.TagsResource > org.xwiki.rest.resources.wikis.WikiPagesResource > org.xwiki.rest.resources.wikis.WikiSearchQueryResource > org.xwiki.rest.resources.wikis.WikisResource > > Failing: > > org.xwiki.rest.DomainObjectFactory > org.xwiki.rest.resources.BaseAttachmentsResource > org.xwiki.rest.resources.BaseSearchResult > org.xwiki.rest.resources.ModificationsResource > org.xwiki.rest.resources.attachments.AttachmentsAtPageVersionResource > org.xwiki.rest.resources.attachments.AttachmentsResource > org.xwiki.rest.resources.comments.CommentResource > org.xwiki.rest.resources.comments.CommentVersionResource > org.xwiki.rest.resources.comments.CommentsResource > org.xwiki.rest.resources.comments.CommentsVersionResource > org.xwiki.rest.resources.objects.AllObjectsForClassNameResource > org.xwiki.rest.resources.objects.ObjectAtPageVersionResource > org.xwiki.rest.resources.objects.ObjectPropertiesAtPageVersionResource > org.xwiki.rest.resources.objects.ObjectPropertiesResource > org.xwiki.rest.resources.objects.ObjectPropertyAtPageVersionResource > org.xwiki.rest.resources.objects.ObjectPropertyResource > org.xwiki.rest.resources.objects.ObjectResource > org.xwiki.rest.resources.objects.ObjectsAtPageVersionResource > org.xwiki.rest.resources.objects.ObjectsForClassNameResource > org.xwiki.rest.resources.objects.ObjectsResource > org.xwiki.rest.resources.pages.PageChildrenResource > org.xwiki.rest.resources.pages.PageHistoryResource > org.xwiki.rest.resources.pages.PageResource > org.xwiki.rest.resources.pages.PageTranslationHistoryResource > org.xwiki.rest.resources.pages.PageTranslationResource > org.xwiki.rest.resources.pages.PageTranslationVersionResource > org.xwiki.rest.resources.pages.PageVersionResource > org.xwiki.rest.resources.pages.PagesResource > org.xwiki.rest.resources.spaces.SpaceAttachmentsResource > org.xwiki.rest.resources.spaces.SpaceSearchResource > org.xwiki.rest.resources.tags.PagesForTagsResource > org.xwiki.rest.resources.wikis.WikiAttachmentsResource > org.xwiki.rest.resources.wikis.WikiSearchResource > > > So which one of these are not internal ? > > Ludovic > > 2012/8/25 Thomas Mortagne <[email protected]>: > > On Tue, Aug 21, 2012 at 10:18 PM, Vincent Massol <[email protected]> > wrote: > >> > >> On Aug 21, 2012, at 2:18 PM, Fabio Mancinelli wrote: > >> > >>> On Fri, Jul 27, 2012 at 10:12 AM, Ludovic Dubost <[email protected]> > wrote: > >>>> As part of rest improvements to display pretty names of users and > >>>> other improvements, I'm getting CLIRR errors because of API changes of > >>>> the model and of public class: > >>>> > >>>> > >>>> 1/ Model CLIRR error because the version field has been moved to > >>>> PageSummary from Page. Page extends PageSummary. I need the version > >>>> field also in representations sending back only PageSummaries. > >>>> Unfortunately CLIRR does not realize that the version field is still > >>>> there when moved to the super class. I believe it's safe to ignore > >>>> this error. Howerver I've put ignore all errors on the Page class as I > >>>> don't have a way to ignore this specific error > >>>> > >>> Yep, I think it's safe. We're adding stuff in a representation (page > >>> summary) and keeping it also in the other, so API-wise it's ok. > >> > >> Note that CLIRR doesn't have false positives so if it complains it > means there's a breakage. The only decision to take is whether it's an > "acceptable" breakage, i.e. we voluntarily break assuming that nobody was > using it and accept that a few users might get broken. > >> > >>>> 2/ CLIRR errors because of parameter additions to objects that are > >>>> used (I think) only internally by the REST server API. Here are the > >>>> errors: > >>>> > >>>> [ERROR] org.xwiki.rest.DomainObjectFactory: In method 'public > >>>> org.xwiki.rest.model.jaxb.Attachment > >>>> createAttachment(org.xwiki.rest.model.jaxb.ObjectFactory, > >>>> java.net.URI, com.xpn.xwiki.api.Attachment, java.lang.String, > >>>> java.lang.String)' the number of arguments has changed > >>> > >>> The DomainObjectFactory is actually a utility class that is used to > >>> build REST-model objects from XWiki-model objects. > >>> It has been created just to prevent code duplication in resource > >>> implementations. > >> > >> The question is whether this is supposed to be a SPI or not. > >> > >>> Now I think it's unlikely that somebody uses it outside the REST > >>> module (a quick grep confirmed this for platform). > >>> > >>> The only use case for a developer of a module to use this class is if > >>> she wants to return a REST-model object and build it using the utility > >>> methods. > >>> I think this is quite unlikely. > >>> > >>> AFAIU all parameters additions are about "pretty names" > >>> ( > https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3L81 > ) > >>> > >>> If we want to be conservative we might do the following: we can add > >>> the new methods and preserve the old ones making them call the new > >>> ones with default parameters. > >>> > >>> * false in methods like this > >>> > https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3L407 > >>> * null, false in methods like this > >>> > https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3L494 > >>> . This implies that in the new implementation the if statement should > >>> also check for null values (like in this case: > >>> > https://github.com/ldubost/xwiki-platform/compare/master...bd49bcc84e1dec3d20b57e970c293a459524d2e4#L3R629 > ) > >>> > >>> We could also think about whether continuing to keep this class in the > >>> public API. It could make sense but I think that nobody will ever use > >>> it so we can start to @deprecate it and eventually move it in internal > >>> packages. > >> > >> Based on what you say I'd say that these classes/methods were put > public by mistake and should all be moved to the internal package without > going through deprecation. > > > > +1 for internal, if it's not supposed to be used outside of the module > > that's the definition of internal > > > >> > >> Of course this means no other other XWiki modules should use them > either since they're internal. > >> > >> Also this means that we don't provide any SPI either since SPI are > user-public. Is that what we want? > >> > >> Thanks > >> -Vincent > >> _______________________________________________ > >> 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 > > > > -- > Ludovic Dubost > Founder and CEO > Blog: http://blog.ludovic.org/ > XWiki: http://www.xwiki.com > Skype: ldubost GTalk: ldubost > -- Ludovic Dubost Founder and CEO Blog: http://blog.ludovic.org/ XWiki: http://www.xwiki.com Skype: ldubost GTalk: ldubost _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
