Community
Participate
Working Groups
Currently, when a delete is undone, the session and persistent properties associated with the resource are lost, and this is the subject of bug 160729. In order to implement the fix for that, the UI needs an API on IResource that will get all of the session and persistent properties. Something like this (on IResource): // K<QualifiedName> V<String> public Map getSessionProperties() // K<QualifeedName> V<String> public Map setPersistentProperties() Implementation of each of this is straightforward and I would be happy to provide a patch if people agree that we should have this. I think these methods could also be generally useful for applications that provide ways to store all of the properties of a in a separate area or file for things like source control.
Right. The implementation is straightforward. Could you please attach the patch for a review?
This is no longer needed for the persistent properties portion of the delete undo support (bug 160729). A new enhancement request, bug 216884 replaces this one. However, this is needed for the session properties portion of delete undo. And it might be useful for to have the persistent properties API in any case, so I will submit a patch that adds these APIs.
Thanks for the patch. However I would suggest to unify the API for persistent and session properties. I like your first proposal for adding these methods in IResource... public Map getSessionProperties() public Map getPersistentProperties() public void setSessionProperties(Map) public void setPersistentProperties(Map) I've been thinking about adding somthing like public QualifiedName[] getSessionPropertiesKeys() public QualifiedName[] getPersistentPropertiesKeys() however the issue is that, even if we got this list we would have to call getters couple times, what would be less efficient than just getting whole map of properties.
I think we should go with creating a map for the reasons you outline. Especially for the persistent properties, since they are stored on disk, forcing a double access if you want to read the values can get expensive. Please note however that the patch submitted was bug 216884, which is necessary to support the delete undo without keeping the properties in memory. I still think that's the direction we should go; if we take the approach outlined in this bug report, we would be keeping an unbounded amount of persistent properties in memory which could be quite a problem. If you disagree with this, can you say why? I would be happy to submit a patch to provide these APIs as well, as they might be useful for other purposes (and are required to support delete undo for the session properties). (In reply to comment #3) > Thanks for the patch. > However I would suggest to unify the API for persistent and session properties. > I like your first proposal for adding these methods in IResource... > > public Map getSessionProperties() > public Map getPersistentProperties() > public void setSessionProperties(Map) > public void setPersistentProperties(Map) > > I've been thinking about adding somthing like > > public QualifiedName[] getSessionPropertiesKeys() > public QualifiedName[] getPersistentPropertiesKeys() > > however the issue is that, even if we got this list we would have to call > getters couple times, what would be less efficient than just getting whole map > of properties. >
Created attachment 88441 [details] Patch for I20071213-1500 Here is a patch for adding these methods: public Map getSessionProperties() public Map getPersistentProperties() I did not see any point in adding the set methods since there are already adequate set methods. I also added some test for properties (there did not appear to be any). They test for: 1) The new APIS 2) Properties persist across copy (session properties don't) 3) Properties persist across close (session properties don't) This also includes a minor refactoring in Resource which eliminated a few lines of duplicate code that appears in many places.
Szymon, can we try to get this into M6? This should be pretty straightforward, and I think will be very useful to people.
Created attachment 93593 [details] Reviewed patch We should be very careful about new introduced API. If we introduced it now, we would not be able to remove it, since M6 is almost done. Besides I think that better API would be methods that get array of properties' keys. Then if people wanted to get properties' values, they could do it using the existing methods. Anyway I made some changes in your patch since this new getters should return copies of properties map.
Created attachment 93609 [details] Reviewed patch with since tags
Patch released. Francis, thanks for your contribution. How do you want to appear in the copyright attribution? Under your name or a company name?
I have added the following to the copyright of the changed files: * Oakland Software Incorporated - added getSessionProperties and getPersistentProperties
(In reply to comment #10) > I have added the following to the copyright of the changed files: > > * Oakland Software Incorporated - added getSessionProperties and > getPersistentProperties > That's fine; here is what I typically have been using: * Oakland Software (Francis Upton) <francisu@ieee.org> - bug xxxxx In the future I will make sure my patches have the property copyright notice. Thanks for taking this!
Verified in I20080327-0100.
