Problems with attempting to add an existing object (and a fix)

[Michael Smith <ms...@xn.com.au>]

Hi,

I'd noticed some database (JDBCDescriptorStore being used) corruption
(well, things being there that shouldn't) recently in the children
table, but had attributed them to local testing/debugging changes I'd
made. However, today I got a clean version of slide and did some more
extensive testing.

It turns out that the following happens when you try to add an object
that already exists:
  In StructureImpl.java, slide goes through the path to the uri you're
trying to add. If the object already exists, it:
  Adds the object as a child of its parent (is this needed? Comments in
the source indicate that it might be to avoid problems at
initialisation, so it may be).
  Stores the object anyway.
  Throws an ObjectAlreadyExistsException.

When the store happens, the JDBCDescriptorStore deletes the existing
children, and re-creates them. However, just above, we've added the
child - even though the ObjectNode quite possibly already HAS this as a
child.

So, we end up entering two copies into the children table. That's not
good. 

For example, I have a front-end for some admin tasks, one of which is
adding a user (with control over roles, etc. Customised for my
purposes). If the user already exists, an ObjectAlreadyExistsException
is thrown and an error message given to the user - fine. However,
because of the problem above, /users has multiple identical children.
Not a good thing. I probably should check for existence before trying to
create it anyway, because it's neater, but doing it this way should not
be a problem.

The following patch corrects this, by ensuring children are unique.

Michael

--- src/share/org/apache/slide/structure/ObjectNode.java	2000/12/01
07:17:38	1.4
+++ src/share/org/apache/slide/structure/ObjectNode.java	2001/02/14
06:34:41
@@ -181,7 +181,7 @@
      * @param object Child
      */
     void addChild(ObjectNode child) {
-        if ((child != null)) {
+        if (child != null && !hasChild(child)) {
             children.addElement(child.getUri());
         }
     }
@@ -193,7 +193,7 @@
      * @param uri Child's uri
      */
     void addChild(String uri) {
-        if (uri != null) {
+        if (uri != null && !hasChild(uri)) {
             // We put a dummy object in the hashtable
             children.addElement(uri);
         }

Re: Problems with attempting to add an existing object (and a fix)

[Remy Maucherat <re...@apache.org>]

> Hi,
>
> I'd noticed some database (JDBCDescriptorStore being used) corruption
> (well, things being there that shouldn't) recently in the children
> table, but had attributed them to local testing/debugging changes I'd
> made. However, today I got a clean version of slide and did some more
> extensive testing.
>
> It turns out that the following happens when you try to add an object
> that already exists:
>   In StructureImpl.java, slide goes through the path to the uri you're
> trying to add. If the object already exists, it:
>   Adds the object as a child of its parent (is this needed? Comments in
> the source indicate that it might be to avoid problems at
> initialisation, so it may be).

I don't remember the exact problem I fixed when I put that in. I wonder if
it's still needed.

>   Stores the object anyway.
>   Throws an ObjectAlreadyExistsException.
>
> When the store happens, the JDBCDescriptorStore deletes the existing
> children, and re-creates them. However, just above, we've added the
> child - even though the ObjectNode quite possibly already HAS this as a
> child.
>
> So, we end up entering two copies into the children table. That's not
> good.

Indeed.

> For example, I have a front-end for some admin tasks, one of which is
> adding a user (with control over roles, etc. Customised for my
> purposes). If the user already exists, an ObjectAlreadyExistsException
> is thrown and an error message given to the user - fine. However,
> because of the problem above, /users has multiple identical children.
> Not a good thing. I probably should check for existence before trying to
> create it anyway, because it's neater, but doing it this way should not
> be a problem.
>
> The following patch corrects this, by ensuring children are unique.

Even if the above wasn't done (perhaps the algorithm in Structure should be
tweaked to address this - although it's not really a bug), it seems to me
that checking for the existence of the child in ObjectNode is necessary to
improve robustness.

Thanks a lot for the patch.

Remy