Deleting objects leaves dangling references from other objects

[Scott Ritchie]

When removing an object from the datastore, the corresponding keys in other classes are left there.

[Matt Giuca (mgiuca)]

For example, deleting achievements leaves the datastore in an inconsistent state. The achievement objects are removed, but the Game's achievements field (containing a list of achievements) still has references to those keys.

The achievements need to be removed from the game's achievement list.

[Scott Ritchie (sritchie)]

This is now the most critical bug I believe. If I get the change tonight I'll go through all the add/create/remove methods in the ServiceImpl classes and make sure these are being maintained properly

[Matt Giuca (mgiuca)]

Just waiting on this and one other bug. What is the status here? It would be good to have it, but it isn't entirely critical as we can add the delete functionality later.

[Scott Ritchie (sritchie)]

Well it kinda breaks things if an object is removed and then the key
is left elsewhere. I'm on the way home now so I'll do it when I get
there, I want to make sure that the addition methods are correctly
adding the relationships too

On 24/05/2011, at 8:23 PM, Matt Giuca <email address hidden> wrote:

> Just waiting on this and one other bug. What is the status here? It
> would be good to have it, but it isn't entirely critical as we can add
> the delete functionality later.
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/786418
>
> Title:
> Removal of an object doesnt manage entity keys
>
> Status in Melbourne University Game-based Learning Environment:
> Triaged
>
> Bug description:
> When removing an object from the datastore, the corresponding keys in
> other classes are left there.
>
> To unsubscribe from this bug, go to:
> https://bugs.launchpad.net/mugle/+bug/786418/+subscribe

[Prageeth Silva (prageethsilva)]

I think I know why this is happening. As far as I can remember, when
we introduced the keys, i remember rechecking the relationships.
However, what I missed was something along the lines:

Set<Key> list = ....

public void remove(ModelClass object) {

  this.list.remove(obj);

}

The above code will not work as Matt mentioned the other day. Key ==
Key will fail since the references are different. AFAIK list.remove()
removes an object by reference.

I think what's needed here is to loop through (not using for each
case) and check IDs and remove appropriately.

We can have a common method to handle this (parameters being a set of
keys and a key) since this would be used in many cases.

--
*Prageeth Silva*

[Scott Ritchie (sritchie)]

No the remove() methods in the ServiceImpl are missing those calls
altogether. It simply deletes the one in the datastore without maintaining
the relationship.

On Tue, May 24, 2011 at 9:14 PM, Prageeth Silva
<email address hidden>wrote:

> I think I know why this is happening. As far as I can remember, when
> we introduced the keys, i remember rechecking the relationships.
> However, what I missed was something along the lines:
>
> Set<Key> list = ....
>
> public void remove(ModelClass object) {
>
> this.list.remove(obj);
>
> }
>
> The above code will not work as Matt mentioned the other day. Key ==
> Key will fail since the references are different. AFAIK list.remove()
> removes an object by reference.
>
> I think what's needed here is to loop through (not using for each
> case) and check IDs and remove appropriately.
>
> We can have a common method to handle this (parameters being a set of
> keys and a key) since this would be used in many cases.
>
> --
> *Prageeth Silva*
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/786418
>
> Title:
> Removal of an object doesnt manage entity keys
>
> Status in Melbourne University Game-based Learning Environment:
> In Progress
>
> Bug description:
> When removing an object from the datastore, the corresponding keys in
> other classes are left there.
>
> To unsubscribe from this bug, go to:
> https://bugs.launchpad.net/mugle/+bug/786418/+subscribe
>

[Scott Ritchie (sritchie)]

I need more information about the expected behaviour of create/write - Given a model class object does it allow you to write the key back to the datastore when casting back through the ModelWrapper?
I currently am not coding around the cases where the client maliciously edits the key and sends it back to the server if this is the case.

[Prageeth Silva (prageethsilva)]

An object's primary key is never allowed to be overwritten. For example, when the client sends an object to be written (updated), the checkPermissions() should check if the client has permissions to update the object as a whole. The ModelWrapper.getDataClone() simply fetches that object from the datastore, updates the *non-reference lists* properties objects (only updates fields with write permissions and any other changes are discarded). This will NOT replace the server primary key ever.

At creation time, an object is created in the database (a new server primary key is generated by the datastore automatically). Then similar to the update method, this will update the fields that has correct permissions and simply discard the others. If an error occurs (this shouldn't be the case however), the whole object is immediately deleted from the database and an exception is thrown.

So answering your question, the model wrapper only checks permissions within an objects fields and will never replace the primary key (primary keys are always handled by the datastore). However, in case of a malicious client sends false primary keys, then the checkPermissions() should handle that and the ModelWrapper will not check this.

Hope this answered your question.

[Scott Ritchie (sritchie)]

Im unable to fix this in a short amount of time;

This is a much wider scope problem - we need to clearly define how deletions cascade down -
there are two types of methods that need to be fixed;

.remove() removes the object itself (currently does not cascade, does not maintain relationships)
.remove<ModelClass> (for example UserService.removeUserGameProfile) these remove the relationships of an object related to the one in the service, but do not remove that object itself.

[Matt Giuca (mgiuca)]

So can I de-milestone this bug?

[Scott Ritchie (sritchie)]

That might be an idea. If you do so, can you remove the delete
functionality on the UI for the time being?

On 25/05/2011, at 11:20 AM, Matt Giuca <email address hidden>
wrote:

> So can I de-milestone this bug?
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/786418
>
> Title:
> Removal of an object doesnt manage entity keys
>
> Status in Melbourne University Game-based Learning Environment:
> Triaged
>
> Bug description:
> When removing an object from the datastore, the corresponding keys in
> other classes are left there.
>
> To unsubscribe from this bug, go to:
> https://bugs.launchpad.net/mugle/+bug/786418/+subscribe

[Matt Giuca (mgiuca)]

I wasn't aware there was any in the UI. Where?

Oh, maybe in the devteam view there is a game delete button?