Bug 184053 - deleting dstore file doesn't remove it from view
Summary: deleting dstore file doesn't remove it from view
Status: VERIFIED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 2.0   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-25 12:37 EDT by David McKnight CLA
Modified: 2008-08-13 13:18 EDT (History)
1 user (show)

See Also:
mober.at+eclipse: review-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David McKnight CLA 2007-04-25 12:37:22 EDT
when you try to delete a dstore file, the file is deleted, however the view doesn't remove it.  This is because a dstore marks the associated DataElement as deleted, changing it's name which means the element can't be found via it's hashcode.
Comment 1 David McKnight CLA 2007-04-25 12:39:59 EDT
Since the mapped lookup fails, and the new name won't match the original name (causing the original recursive lookup to fail), I've added a method, recursiveFindExactMatch() to find the underlying object when the mapped lookup fails.
Comment 2 Martin Oberhuber CLA 2007-04-25 16:14:15 EDT
Data elements, or remote resources in general, changing their properties and thus changing their hashCode is a common problem.

I think the right solution for this is using the IRemoteObjectIdentifier's getAbsoluteName() method for identifying remote resources, rather than the hashCode. The absolute name of an object must remain the same even if it is deleted or properties change in some other manner -- then the elementMap will also find the element and be able to remove it.

As per bug 182454, IRemoteObjectIdentifier's Javadoc documents all this. Please review the Javadoc and try using it accordingly. We should move to using the elementMap in all cases, and get rid of recursive lookups wherever possible.

Or, let me know if my thinking is wrong.
Comment 3 David McKnight CLA 2007-04-25 16:49:38 EDT
I'm not sure how using IRemoteObjectIdentifier is going to help this since that's just the adapter using the underlying model object to get at the absolute name.  Right now for DStoreFile, we simply get the information from the DataElement to produce it's absolute name.  However, when a DataElement is deleted (like in the case of deleting a file), it's attributes change such that we can no longer get the absolute name from it.  I suppose one way to deal with this might be by storing the absolute path separately after creating the DStoreHostFile from the DataElement so that even when the properties change in the DataElement, we still have access to the original path.  What do you think?         
Comment 4 Martin Oberhuber CLA 2007-04-25 17:00:04 EDT
Yes, this should be done.

Changing the absoluteName of an element breaks an important contract in my 
opinion. The absoluteNameis in identifier of the object, and it must remain
valid over the entire lifetime of the object in memory.

I agree that having the absoluteName logic in the UI-adapter only is a problem. We should consider moving this logic into non-UI. Taking into account that ISubSystem should go into non-UI at some time, I could imagine having these as ISubSystem methods:

  String ISubSystem#getAbsoluteName(Object remoteElement)
  Object ISubSystem#getObjectByAbsoluteName(String absoluteName)

Once this is done, the adapters would no longer need getAbsoluteName() and the subsystem would be consulted for getting it. What do you think? Would it be too problematic to try getting an absolute name without knowing the kind of object in the subsystem? Should we go with another layer of non-UI adapters instead?
Comment 5 David McKnight CLA 2007-04-26 07:45:28 EDT
Not all objects are under subsystems.  The adapters are currently used to get the absolute names of hosts and filters as well.  I suppose we could explicitly add getAbsoluteName() to those constructs or do you have something else in mind?
Comment 6 David McKnight CLA 2007-04-26 10:25:26 EDT
I've added the absolute path as a field of DSToreHostFile so that we can still find the mapped file after deletion.   As for the subsystem API stuff, we should put that in a separate defect.
Comment 7 Martin Oberhuber CLA 2007-04-27 03:35:32 EDT
Thanks Dave, this seems to be the right approach.

Did you check what happens when
  - a file is renamed
  - a file is moved to a different folder
  - a folder is renamed
  - a folder is moved to a different folder
on the same dstore connection?

In each of these cases, the absoluteName needs to be re-computed OR a new IHostFile with a new absoluteName created. I'm not sure how dstore works, but in case the DataElement and the IHostFile remains the same in these cases, there would be a problem.

Remember that the API contract of IHostFile is being a handle for a particular path on the remote side.
Comment 8 Martin Oberhuber CLA 2008-08-13 13:18:35 EDT
[target cleanup] 2.0 M7 was the original target milestone for this bug