Bug 210534

Summary: [api][breaking][persistence] Remove ISystemHostPool.getHostList() and setName() methods
Product: [Tools] Target Management Reporter: Martin Oberhuber <mober.at+eclipse>
Component: RSEAssignee: Martin Oberhuber <mober.at+eclipse>
Status: RESOLVED FIXED QA Contact: Martin Oberhuber <mober.at+eclipse>
Severity: enhancement    
Priority: P3 CC: ddykstal.eclipse
Version: 2.0Keywords: api
Target Milestone: 3.0 M4Flags: ddykstal.eclipse: review+
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch removing the methods and updating Javadoc none

Description Martin Oberhuber CLA 2007-11-21 10:06:13 EST 
The ISystemHostPool.getHostList() method exposes a list of connection objects, thus giving clients access to an internal structure that cannot easily be worked on in a thread-safe way.

The method should be removed from the public API, since clients can call
   IHost[] ISystemHostPool.getHosts()
instead which returns a copy of the internal list as an array and is thus thread-safe.

The getHostList() method should be reserved for internal use in subclasses of the SystemHostPool implementation (and thus be made protected).
Comment 1 Martin Oberhuber CLA 2007-11-21 10:17:33 EST 
For a similar reason,
   ISystemHostPool.setName(String)
should also be removed from the public API, because calling it could lead to inconsistencies. Host pools are never created by clients directly; therefore, clients can always call
   ISystemHostPool.renameHostPool(String)
instead.
Comment 2 Martin Oberhuber CLA 2007-11-21 10:20:23 EST 
Created attachment 83434 [details]
Patch removing the methods and updating Javadoc
Comment 3 Martin Oberhuber CLA 2007-11-21 10:24:34 EST 
Patch committed to ISystemHostPool v1.7

DaveD please review my JavaDoc changes for correctness. Please also look at bug 210537 to investigate the "throws" clauses in ISystemHostPool.
Comment 4 Martin Oberhuber CLA 2007-11-21 10:49:26 EST 
Especially please review if this is what we really want in orderHosts():

* Existing connections in the internal connection list that do not match any
* alias name in the given name list, will be deleted from this host pool!
Comment 5 David Dykstal CLA 2007-12-18 13:34:59 EST 
The changes to the javadoc look good.

Regarding Bug 210537 - I think throwing "Exception" is weak engineering. If we throw anything it should be something RSEException or SystemException that either translates or wraps the reason - perhaps as an IStatus.