OD.associate() throws exception when object already in cache

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Call ObjectDatastore.associate(object) twice with the same object.

What is the expected output? 

I expect associate() to ignore the 2nd attempt to associate the object with the 
cache. Optionally, it could log this at a fine or finer level.

What do you see instead?

associate() throws IllegalArgumentException("Cannot store associated instance. 
Use update instead.");

The exception comes from StandardSingleStoreCommand's constructor, the 
following code is called:

        if (!command.update && command.datastore.associatedKey(instance) != null)
        {
            throw new IllegalArgumentException("Cannot store associated instance. Use update instead.");
        }
        else if (command.update && command.datastore.associatedKey(instance) == null)
        {
            throw new IllegalArgumentException("Cannot update non-associated instance. Use store instead.");
        }

What version of the product are you using? On what operating system?

Source code build from sources downloaded on Mar 27, 2011. Ubuntu 10.10.

Please provide any additional information below.

Original issue reported on code.google.com by rickh...@gmail.com on 29 Apr 2011 at 10:50

[GoogleCodeExporter]

Certainly, the error message is misleading.

I'm not sure ignoring the problem is the best solution.  Surely it signifies a 
programming error if you try to associate an instance that is already 
associated?

Can you give more details about when you do not know if an instance should be 
associated or not?  Perhaps it is better to structure your code so that when 
you receive instances from an external source you associate them at the point 
of entry?

I am open to being persuaded otherwise though, if you can provide an example of 
when it is not practical to do this.

Original comment by jdpatterson on 30 Apr 2011 at 4:14

• Changed state: Accepted

[GoogleCodeExporter]

I have since rewritten the code that caused me not to know if an object was 
already associated. I'm not sure, now, exactly how that code worked. But I 
think what happened was that I was reusing a method that was used in both 
cases, of an object already having been associated, and with an object that was 
not yet associated. At the time, the object in question did not have an getId() 
method, and given that GaeKey is not being initialized correctly, I was unable 
to check for the presence of a non-null key. I have since added a getId() 
method, so now I can check for the non-null key (id). 

So, given that in some cases, one doesn't want to use @Id or @GaeKey, I would 
suggest that allowing a repeat associate() call without throwing an exception, 
would allow for code structured like mine was, when there is no other way to 
check if the object is already associated. I would, however, definitely log it.

Original comment by rickh...@gmail.com on 30 Apr 2011 at 6:52

[GoogleCodeExporter]

Glad you got your code working. I still do not see the when you would not know 
if an external instance was going to be used e.g. from GWT or stored in http 
Session.  I would not recommend calling associate() for every instance in a DAO 
method just in case it was not associated.  Its not an expensive datastore rpc 
call... but it feels wrong to so such unneeded work for every instance.

Original comment by jdpatterson on 1 May 2011 at 1:14

[GoogleCodeExporter]

I'm OK with your solution of throwing an exception for subsequent calls to 
associate(). Given that I restructured my code and added @Id with accessor 
methods, I can readily tell if it has been associated. So, it's fine with me if 
you leave the code as written, except for changing the error message to a more 
appropriate message. Thank you for such fast attention to issues. Really 
appreciate it.

Original comment by rickh...@gmail.com on 1 May 2011 at 1:19

[GoogleCodeExporter]

For now I have updated the error message to "Cannot associate an already 
associated instance."

Original comment by jdpatterson on 1 May 2011 at 1:20

[GoogleCodeExporter]

Pushed to main repo.  Thanks!

Original comment by jdpatterson on 1 May 2011 at 1:37

• Changed state: Fixed