On Dec 31, 2009, at 3:26 PM, Vincent Massol wrote:

On Dec 31, 2009, at 1:20 PM, Sergiu Dumitriu wrote:

...

On 12/31/2009 12:39 PM, Vincent Massol wrote:

...

Hi devs,

I'm almost done with my entity reference refactoring and I've just realized I have missed something I think. So far the implementation only supports Absolute references (i.e the entity reference factory always return a reference with all parts filled - you choose to use a default factory or a current entity depending on how you wish to resolve the names when they have not been provided in the passed reference string).

I now think we must also support relative references (i.e. when some parts can be null) and that it's up to the user of the api to decide if they want to convert a relative reference to an absolute one or not.

Here's a use case: renaming of documents. For exemple documents have links specified as a string representing the target doc name. If we don't have relative references then we need to decide if we want to use the default serializer (all parts printed including wiki name) or the compact serializer (only parts different from context reference printed). This doesn't support printing only what the user had decided to fill. For ex a user might have specified voluntarily the space and page name and right now with my implementation he'll get only the page name specified if the new space is the same as the space for the current doc.

So here's my proposal:

* Entity Reference Factory leaves parts to null when not specified in the string representation. * We add a EntityReference.getAbsoluteReference(EntityReference base) method to return an absolute reference. It's resolved against the passed base reference (i.e. parts not specified are taken from it)

What happens when the passed base is not what should be? Should the implementation fall back to the current document?

We could throw an IllegalArgumentException for example.

That said, I'm still hesitating between having this getAbsoluteReference() API and adding a EntityReferenceResolver interface:

EntityReferenceResolver |_ EntityReference resolve(EntityReference)

and have several implementations: CurrentEntityReferenceResolver, DefaultEntityReferenceResolver, etc (as I have now for EntityReferenceFactory).

I'm inclined to create a EntityReferenceNormalizer interface with a normalize() method and 2 implementations: CurrentEntityReferenceNormalizer, DefaultEntityReferenceNormalizer My current issue is to know what to do when the serializer is used with an "invalid" relative reference. For ex with: new EntityReference("page", EntityType.DOCUMENT, new EntityReference("wiki", EntityType.WIKI)) (no space specified) One option is to make serialize throw an InvalidReference exception. hmmm since this is not supposed to happen (you cannot get an invalid reference if you use the reference factory) we could probably throw a Runtime exception. I'll go with this option unless someone suggest something better. Thanks -Vincent

Thanks -Vincent

...

...

WDYT?

I'm going to start refactoring my code to do this later today so please let me know if you see any pb with it.

Looks good, +1.

On Sun, Jan 3, 2010 at 13:22, Vincent Massol <[email protected]> wrote:

Note that using relative references means that all code that accepts an EntityReference must check that it's valid and throw some exception if not, whereas before there was no need for any check and no exception throwing....

In most cases it's not possible for the code to run the normalizer since it wouldn't know which normalizer to use (the default one, the current one, etc?). It's the calling code that would know which normalizer to use...

We could decide of an arbitrary normalizer like saying when we don't know we use "current" normalizer same way it's done for a File for example. We could have a EntityReference#getAbsoluteReference() that make sure to get a valid full reference (and return this if it's already a full reference). IMO the code should not throw an exception but instead document what normalizer it's using when the reference is not absolute exactly like file system APIs in general.

My status: * I have implemented the normalizer code (both Default normalizer and current normalizer) * I'm now going to start re-implementing the entity reference factories and serializers * I'm looking at existing code to see how the code would need to be adapted and I'm finding that in several cases it's going to be hard. Just to illustrate this take this code:

    public String getAttachmentURL(AttachmentReference attachmentReference, boolean isFullURL)     {         String url;         if (isFullURL) {             XWikiContext xcontext = getContext();             url = xcontext .getURLFactory().createAttachmentURL(attachmentReference.getName(),

attachmentReference .getDocumentReference().getLastSpaceReference().getName(),                 attachmentReference.getDocumentReference().getName(),                 "download", null, attachmentReference.getDocumentReference().getWikiReference().getName(),                 xcontext).toString();         } else {             url = getAttachmentURL (this .entityReferenceSerializer .serialize(attachmentReference.getDocumentReference()),                 attachmentReference.getName());         }         return url;     }

There's now no guarantee that the passed reference has a non null attachment name, space name or wiki name. The code would need to verify this (or createAttachmentURL() would need to check it but that's the same). What should it do if it finds a null name? It cannot guess if it should use the current normalizer or the default normalizer.

The only solution I can see would be to check if the passed reference is absolute (using AttachmentReference.isAbsolute() for ex) and throw an InvalidEntityReference exception if it's not absolute.

But that's going to be a pain probably...

I'm still hesitating...

WDYT?

Thanks -Vincent

On Dec 31, 2009, at 12:39 PM, Vincent Massol wrote:

...

Hi devs,

I'm almost done with my entity reference refactoring and I've just realized I have missed something I think. So far the implementation only supports Absolute references (i.e the entity reference factory always return a reference with all parts filled - you choose to use a default factory or a current entity depending on how you wish to resolve the names when they have not been provided in the passed reference string).

I now think we must also support relative references (i.e. when some parts can be null) and that it's up to the user of the api to decide if they want to convert a relative reference to an absolute one or not.

Here's a use case: renaming of documents. For exemple documents have links specified as a string representing the target doc name. If we don't have relative references then we need to decide if we want to use the default serializer (all parts printed including wiki name) or the compact serializer (only parts different from context reference printed). This doesn't support printing only what the user had decided to fill. For ex a user might have specified voluntarily the space and page name and right now with my implementation he'll get only the page name specified if the new space is the same as the space for the current doc.

So here's my proposal:

* Entity Reference Factory leaves parts to null when not specified in the string representation. * We add a EntityReference.getAbsoluteReference(EntityReference base) method to return an absolute reference. It's resolved against the passed base reference (i.e. parts not specified are taken from it)

WDYT?

I'm going to start refactoring my code to do this later today so please let me know if you see any pb with it.

Thanks -Vincent

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

-- Thomas Mortagne

Actually I'm no longer sure the cons of having relative reference overcomes the pros of having only absolute references. Re the rename use case I had defined, it's probably not that bad to use the compact entity reference serializer (which I have already written) to use the minimal syntax for referencing the parent or a link. It would mean that a link as [[CurrentSpace.Page]] would get transformed to [[NewPage]] if the Page document is renamed to NewPage in the *same* space. It's not bad and could actually be considered a feature to rewrite the link to the smallest possible representation that works. I don't see any other important use case and there are lots of pros of having only absolute references internally. Thus I'd like to withdraw this idea of adding support for relative references. I'll go on without relative references for now. Let me know if you see a real need that overcomes the cons it brings. Thanks -Vincent On Jan 3, 2010, at 2:43 PM, Vincent Massol wrote:

On Jan 3, 2010, at 1:49 PM, Thomas Mortagne wrote:

...

On Sun, Jan 3, 2010 at 13:22, Vincent Massol <[email protected]> wrote:

...

Note that using relative references means that all code that accepts an EntityReference must check that it's valid and throw some exception if not, whereas before there was no need for any check and no exception throwing....

In most cases it's not possible for the code to run the normalizer since it wouldn't know which normalizer to use (the default one, the current one, etc?). It's the calling code that would know which normalizer to use...

We could decide of an arbitrary normalizer like saying when we don't know we use "current" normalizer same way it's done for a File for example. We could have a EntityReference#getAbsoluteReference() that make sure to get a valid full reference (and return this if it's already a full reference). IMO the code should not throw an exception but instead document what normalizer it's using when the reference is not absolute exactly like file system APIs in general.

I don't think we can compare with the filesystem. There's only one normalizing reference for the file system. This is not our case.

Also I don't agree that API code should default to the current normalizer. This would mean the code will only work when there's a context doc and this puts some unnecessary restriction on the code. It's really the calling code that should pass the correct reference.

I think (haven't thought enough about it though) I'd prefer to separate absolute reference from relative references so that apis that need an absolute ref can specify it in their signature. That would remove the need for checking + the need to throw an exception or use an arbitrary normalizer.

We could also have a reference factory that takes an absolute ref and generate a relative reference (w/ default + current impl) for the user cases when you have an abs ref and need a relative one. Since our ref factory are typed we wouldn't even need a new interface.

I need to think more about it but my feeling is that the absolute ref stuff we had before wasn't so bad after all. Maybe we just need to add a new notion of relative ref somehow for those few use cases requiring it but keep using absolute ref in most places.

Thanks -Vincent

...

...

My status: * I have implemented the normalizer code (both Default normalizer and current normalizer) * I'm now going to start re-implementing the entity reference factories and serializers * I'm looking at existing code to see how the code would need to be adapted and I'm finding that in several cases it's going to be hard. Just to illustrate this take this code:

public String getAttachmentURL(AttachmentReference attachmentReference, boolean isFullURL) { String url; if (isFullURL) { XWikiContext xcontext = getContext(); url = xcontext .getURLFactory().createAttachmentURL(attachmentReference.getName(),

attachmentReference .getDocumentReference().getLastSpaceReference().getName(), attachmentReference.getDocumentReference().getName(), "download", null, attachmentReference.getDocumentReference().getWikiReference().getName(), xcontext).toString(); } else { url = getAttachmentURL (this .entityReferenceSerializer .serialize(attachmentReference.getDocumentReference()), attachmentReference.getName()); } return url; }

There's now no guarantee that the passed reference has a non null attachment name, space name or wiki name. The code would need to verify this (or createAttachmentURL() would need to check it but that's the same). What should it do if it finds a null name? It cannot guess if it should use the current normalizer or the default normalizer.

The only solution I can see would be to check if the passed reference is absolute (using AttachmentReference.isAbsolute() for ex) and throw an InvalidEntityReference exception if it's not absolute.

But that's going to be a pain probably...

I'm still hesitating...

WDYT?

Thanks -Vincent

On Dec 31, 2009, at 12:39 PM, Vincent Massol wrote:

...

Hi devs,

I'm almost done with my entity reference refactoring and I've just realized I have missed something I think. So far the implementation only supports Absolute references (i.e the entity reference factory always return a reference with all parts filled - you choose to use a default factory or a current entity depending on how you wish to resolve the names when they have not been provided in the passed reference string).

I now think we must also support relative references (i.e. when some parts can be null) and that it's up to the user of the api to decide if they want to convert a relative reference to an absolute one or not.

Here's a use case: renaming of documents. For exemple documents have links specified as a string representing the target doc name. If we don't have relative references then we need to decide if we want to use the default serializer (all parts printed including wiki name) or the compact serializer (only parts different from context reference printed). This doesn't support printing only what the user had decided to fill. For ex a user might have specified voluntarily the space and page name and right now with my implementation he'll get only the page name specified if the new space is the same as the space for the current doc.

So here's my proposal:

* Entity Reference Factory leaves parts to null when not specified in the string representation. * We add a EntityReference.getAbsoluteReference(EntityReference base) method to return an absolute reference. It's resolved against the passed base reference (i.e. parts not specified are taken from it)

WDYT?

I'm going to start refactoring my code to do this later today so please let me know if you see any pb with it.

Thanks -Vincent

On Mon, Jan 4, 2010 at 09:31, Vincent Massol <[email protected]> wrote:

On Jan 4, 2010, at 9:26 AM, Vincent Massol wrote:

...

Actually I'm no longer sure the cons of having relative reference overcomes the pros of having only absolute references.

Re the rename use case I had defined, it's probably not that bad to use the compact entity reference serializer (which I have already written) to use the minimal syntax for referencing the parent or a link. It would mean that a link as [[CurrentSpace.Page]] would get transformed to [[NewPage]] if the Page document is renamed to NewPage in the *same* space. It's not bad and could actually be considered a feature to rewrite the link to the smallest possible representation that works.

+1

...

I don't see any other important use case and there are lots of pros of having only absolute references internally.

Thus I'd like to withdraw this idea of adding support for relative references.

I'll go on without relative references for now. Let me know if you see a real need that overcomes the cons it brings.

Note that I'll keep the normalizer as a helper component if you want to normalize an EntityReference without going through a string representation.

+1

Thanks -Vincent

...

Thanks -Vincent

On Jan 3, 2010, at 2:43 PM, Vincent Massol wrote:

...

On Jan 3, 2010, at 1:49 PM, Thomas Mortagne wrote:

...

On Sun, Jan 3, 2010 at 13:22, Vincent Massol <[email protected]> wrote:

...

Note that using relative references means that all code that accepts an EntityReference must check that it's valid and throw some exception if not, whereas before there was no need for any check and no exception throwing....

In most cases it's not possible for the code to run the normalizer since it wouldn't know which normalizer to use (the default one, the current one, etc?). It's the calling code that would know which normalizer to use...

We could decide of an arbitrary normalizer like saying when we don't know we use "current" normalizer same way it's done for a File for example. We could have a EntityReference#getAbsoluteReference() that make sure to get a valid full reference (and return this if it's already a full reference). IMO the code should not throw an exception but instead document what normalizer it's using when the reference is not absolute exactly like file system APIs in general.

I don't think we can compare with the filesystem. There's only one normalizing reference for the file system. This is not our case.

Also I don't agree that API code should default to the current normalizer. This would mean the code will only work when there's a context doc and this puts some unnecessary restriction on the code. It's really the calling code that should pass the correct reference.

I think (haven't thought enough about it though) I'd prefer to separate absolute reference from relative references so that apis that need an absolute ref can specify it in their signature.  That would remove the need for checking + the need to throw an exception or use an arbitrary normalizer.

We could also have a reference factory that takes an absolute ref and generate a relative reference (w/ default + current impl) for the user cases when you have an abs ref and need a relative one. Since our ref factory are typed we wouldn't even need a new interface.

I need to think more about it but my feeling is that the absolute ref stuff we had before wasn't so bad after all. Maybe we just need to add a new notion of relative ref somehow for those few use cases requiring it but keep using absolute ref in most places.

Thanks -Vincent

...

...

My status: * I have implemented the normalizer code (both Default normalizer and current normalizer) * I'm now going to start re-implementing the entity reference factories and serializers * I'm looking at existing code to see how the code would need to be adapted and I'm finding that in several cases it's going to be hard. Just to illustrate this take this code:

  public String getAttachmentURL(AttachmentReference attachmentReference, boolean isFullURL)   {       String url;       if (isFullURL) {           XWikiContext xcontext = getContext();           url = xcontext .getURLFactory().createAttachmentURL(attachmentReference.getName(),

attachmentReference .getDocumentReference().getLastSpaceReference().getName(),               attachmentReference.getDocumentReference().getName(),               "download", null, attachmentReference.getDocumentReference().getWikiReference().getName(),               xcontext).toString();       } else {           url = getAttachmentURL (this .entityReferenceSerializer .serialize(attachmentReference.getDocumentReference()),               attachmentReference.getName());       }       return url;   }

There's now no guarantee that the passed reference has a non null attachment name, space name or wiki name. The code would need to verify this (or createAttachmentURL() would need to check it but that's the same). What should it do if it finds a null name? It cannot guess if it should use the current normalizer or the default normalizer.

The only solution I can see would be to check if the passed reference is absolute (using AttachmentReference.isAbsolute() for ex) and throw an InvalidEntityReference exception if it's not absolute.

But that's going to be a pain probably...

I'm still hesitating...

WDYT?

Thanks -Vincent

On Dec 31, 2009, at 12:39 PM, Vincent Massol wrote:

...

Hi devs,

I'm almost done with my entity reference refactoring and I've just realized I have missed something I think. So far the implementation only supports Absolute references (i.e the entity reference factory always return a reference with all parts filled - you choose to use a default factory or a current entity depending on how you wish to resolve the names when they have not been provided in the passed reference string).

I now think we must also support relative references (i.e. when some parts can be null) and that it's up to the user of the api to decide if they want to convert a relative reference to an absolute one or not.

Here's a use case: renaming of documents. For exemple documents have links specified as a string representing the target doc name. If we don't have relative references then we need to decide if we want to use the default serializer (all parts printed including wiki name) or the compact serializer (only parts different from context reference printed). This doesn't support printing only what the user had decided to fill. For ex a user might have specified voluntarily the space and page name and right now with my implementation he'll get only the page name specified if the new space is the same as the space for the current doc.

So here's my proposal:

* Entity Reference Factory leaves parts to null when not specified in the string representation. * We add a EntityReference.getAbsoluteReference(EntityReference base) method to return an absolute reference. It's resolved against the passed base reference (i.e. parts not specified are taken from it)

WDYT?

I'm going to start refactoring my code to do this later today so please let me know if you see any pb with it.

Thanks -Vincent

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

-- Thomas Mortagne