Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 359874 - Subsystem could check only connection state on connect
Summary: Subsystem could check only connection state on connect
Status: NEW
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.2.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: dsdp.tm.rse-inbox CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-04 12:44 EDT by Simon Bernard CLA
Modified: 2011-10-05 05:59 EDT (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Bernard CLA 2011-10-04 12:44:36 EDT 
In SubSystem#connect(org.eclipse.core.runtime.IProgressMonitor, boolean), we check if the subsystem is connected. If it's not, we connect to the connection service linked to this subsystem.

I think, it could be better to test if the connection service is connected instead of testing the subsystem connection state.

Because, if a user extends the SubSystem class and override the isConnect method. We could be in the case where the subsystem is considered as not connected but the connectionService is connected.

I describe a case while this problem could happens in Bug 359873
Comment 1 Martin Oberhuber CLA 2011-10-04 18:15:13 EDT 
I think this is invalid, see my explanation in bug 359873 .

When you override isConnected() to add constraints you also have to override connect() -- our connect method can't foresee any modified behavior of isConnected().
Comment 2 Simon Bernard CLA 2011-10-05 05:59:16 EDT 
I agree if I override isConnect, I surely must override connect too.

I must do something like that : 

if (supportsConnecting)
{
    if (getConnectorService() != null && !getConnectorService().isConnected()) {
        super.connect(monitor, forcePrompt);
    }
    
    if (!isOtherConnectionConnected()) {
	connectOtherConnection();
    }
}
It's not really elegant and the test of connection state is execute twice (One in the overrided method and one in the super method)


If connection state test is done only on the connectorService instead of subsystem state, the overriding of connect will be more elegant.

if (supportsConnecting)
{
    super.connect(monitor, forcePrompt);

    if (!isOtherConnectionConnected()) {
	connectOtherConnection();
    }
}