On Fri, Jul 27, 2012 at 10:12 AM, Ludovic Dubost <ludo...@xwiki.com> 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.

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

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.

-Fabio
_______________________________________________
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to