Bug 338510

Comment 1 Martin Oberhuber 2011-03-01 04:24:43 EST

What version is this with? Could this be a duplicate of but 301075 ?

Comment 2 Martin Oberhuber 2011-03-01 04:25:08 EST

dup of bug 301075 ?

Comment 3 David McKnight 2011-03-01 15:37:40 EST

Created attachment 190087 [details]
potential patch

I don't have a test case for this at the moment but I suspect this isn't quite the same bug as bug 301075.  In fact, it's likely that there are several other places where we aren't cloning the property sets.  As such, I'm thinking it would make sense to add API to IPropertySetContainer for property set as I've done in the patch.

Martin, do you have any thoughts on this?

Comment 4 David McKnight 2011-03-01 16:07:33 EST

Comment on attachment 190087 [details]
potential patch

Actually, the patch here might not help with this - I see that we already do copy property sets in SubSystemConfiguration.cloneSubSystem().

Comment 5 David McKnight 2011-03-01 16:38:47 EST

Created attachment 190094 [details]
patch to clone the subsystem property sets

The problem is that we pass the original subsystem's property set directly into the new subsystem, which results in this call to removePropertySet:

	public boolean addPropertySet(IPropertySet set) {
		IPropertySetContainer old = set.getContainer();
		if (old != null) {
			old.removePropertySet(set.getName());

The solution in the patch uses the clonePropertySets() method that was also used in bug 301075 rather than calling addPropertySet() on the original property set.  The closePropertySets() method has the potential to get copied a lot so it might be best to create a common API for this, as I put in the original patch.

Comment 6 David McKnight 2011-03-01 16:49:52 EST

Created attachment 190098 [details]
alternate patch with API change

Here's an alternative patch that involves the API change made in the obsolete patch rather.  This one is probably cleaner and more reusable but it does involve new API.

Comment 7 Martin Oberhuber 2011-03-01 17:47:08 EST

Introducing API for this seems right. I'm not sure why there has to be a #clonePropertySets() on several concrete classes like Host, ConnectorService, Subsystem. Why not do e.g.

   y.addPropertySets(PropertySetUtil.clone(x.getPropertySets())

On the other hand, with boilerplate code like this it's easy to forget the clone, having a direct #clonePropertySets() makes it clearer what needs to be done. I'll leave it to your imagination what's the cleaner fix.

Comment 8 Noriaki Takatsu 2011-03-01 21:08:53 EST

This bug is not a duplicate of bug 301075.
In the 301075, some property or property set is not cloned to the NEW one.
But in this problem the property set that was added to the original connection
was removed / deleted from the ORIGINAL connection definition after the copy connection operation is done.
For example:
- first create a new connection
- add some property to the connection
  for example, in the Shells subsystem. Environment Variables can be defined 
  as property. I confirmed that the following data was added in the node
  properties for the connection. 
06-child.00000.06-child.00003.00-name=Shells
06-child.00000.06-child.00003.01-type=SubSystem
06-child.00000.06-child.00003.03-attr.hidden=false
06-child.00000.06-child.00003.03-attr.type=dstore.shells
06-child.00000.06-child.00003.06-child.00000.00-name=EnvironmentVariables
06-child.00000.06-child.00003.06-child.00000.01-type=PropertySet
06-child.00000.06-child.00003.06-child.00000.03-attr.description=null
06-child.00000.06-child.00003.06-child.00000.06-child.00000.00-name=XXXXXXX
06-child.00000.06-child.00003.06-child.00000.06-child.00000.01-type=Property
06-child.00000.06-child.00003.06-child.00000.06-child.00000.03-attr.type=java.lang.String
06-child.00000.06-child.00003.06-child.00000.06-child.00000.03-attr.value=VVVVVVVV

- Then Copy the defined connection.  In copy connection, you have to select
  "Rename" checkbox in "Duplicate Name Collision" and specify the new name
  for the copy.

- After that, the above EnvironmentVariables data were deleted from 
  the ORIGINAL connection node properties file.

I tested the level of 3.2.2.

Comment 9 David McKnight 2011-03-01 21:11:30 EST

Noriaki, could you try this with the 2nd patch?

Comment 10 Noriaki Takatsu 2011-03-01 22:09:54 EST

I've confirmed that the 2nd patch could solve this problem.
Thank you.
And how can we get the new jars for our product ?

Comment 11 Noriaki Takatsu 2011-03-01 23:02:12 EST

We need a backport support for 3.0.x.

Comment 12 David McKnight 2011-03-02 07:52:26 EST

(In reply to comment #11)
> We need a backport support for 3.0.x.

For a backport, we'll need to do something like the first patch, since the second patch introduces new API.

Comment 13 David McKnight 2011-03-02 08:55:09 EST

I've committed the 2nd patch to cvs and opened bug 338665 for the backport.

Comment 14 John W Snyder 2011-08-23 11:36:49 EDT

We need a backport to 3.2.2+. I opened bug355530 for the backport.