[Yanel-dev] Little correction for VirtualFilesystemNode

[Balz Schreier]

Hi Michael,
yes I think my suggestion to correct the method implementation was a bad
one, because some realms might rely on the behaviour as it is now.

Regarding the javadoc, it not clear to me what this means:
"     * Creates a new node and adds it as a direct child to this node.

     * @param name name of the child node
"

because my node with path "/vouchers/node.xml" is a direct child to the ROOT
node, so I would expect that it gets stored as child of root, but it doesn't
(--> "//" ).

So I would update the javadoc with something like:

        * Creates a new node and adds it as a direct relative child to the
current node.

     * @param name Relative path of the child node (e.g. if current node has
a path like /data/images , the relative path could be "1970/abc.xml" -> The
method will create a new node with path /data/images/1970/abc.xml

Cheers
Balz

On Tue, Apr 12, 2011 at 11:18 AM, Michael Wechner <michael.wechner at wyona.com
> wrote:

>  Hi Balz
>
> It seems to me that the Javadoc of
>
> src/core/java/org/wyona/yarep/core/Node.java#addNode(String, int)
>
> is pretty clear or please feel free to suggest a better wording.
>
> Re the Virtual Filesystem implementation I would suggest that we check the
> name for slashes
> and throw an exception such that it is clear that one should not use the
> method (1) like this.
>
> WDYT?
>
> Thanks
>
> Michael
>
>
> On 4/11/11 9:32 AM, Balz Schreier wrote:
>
> Hi,
>
>  I have detected the following (at least for me) unexpected behaviour and
> it should get corrected, the fix is very easy, so I don't send a patch for
> this.
>
>  *Use Case:*
> You have prepared a Yarep path (e.g. "/vouchers/thisisanidstring.xml"), and
> you want to add this node now to a repository.
>
>  *Solution:*
> there are two ways in Yanel how to do that:
> 1) repo.getRootNode().addNode(yarepPath, NodeType.RESOURCE);
>
>  2) YarepUtil.addNodes(repo, yarepPath, NodeType.RESOURCE);
>
>  of course, yarepPath = "/vouchers/thisisanidstring.xml" in both
> scenarios.
>
>  Unexpected result:
> 1) Does store the new node under "//vouchers/thisisanidstring.xml",
> starting with a double slash!! --> Index also contains then that path
> 2) Correct, stores node under "/vouchers/thisisanidstring.xml"
>
>
>  *How to fix it:*
>
>  Class VirtualFileSystemNode:
>
>  Currently this code looks like this:
>
>     /**
>
>      * @see org.wyona.yarep.core.Node#addNode(java.lang.String, int)
>
>      */
>
>     public Node addNode(String name, int type) throws RepositoryException
> {
>
>         String newPath = getPath() + "/" + name;
>
>         if (getPath().endsWith("/")) {
>
>             newPath = getPath() + name;
>
>         }
>
>  Corrected code would be:
>
>         String newPath = null;
>
>         if (name.startsWith("/")) {
>
>             name = name.substring(1);
>
>         }
>
>         if (getPath().endsWith("/")) {
>
>             newPath = getPath() + name;
>
>         } else {
>
>             newPath = getPath() + "/" + name;
>
>         }
>
>
>
>  Alternative:
> If you think that this should not get fixed, I would add a clear Javadoc
> saying that the "String name" parameter MUST BE RELATIVE.
>
>  What do you think?
> Cheers
> Balz
>
>
>
> --
> Yanel-development mailing list Yanel-development at wyona.com
> http://lists.wyona.org/cgi-bin/mailman/listinfo/yanel-development
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wyona.org/pipermail/yanel-development/attachments/20110412/16aa08a4/attachment-0001.html>