Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 359874

Summary:	Subsystem could check only connection state on connect
Product:	[Tools] Target Management	Reporter:	Simon Bernard <sbernard>
Component:	RSE	Assignee:	dsdp.tm.rse-inbox <tm.rse-inbox>
Status:	NEW ---	QA Contact:	Martin Oberhuber <mober.at+eclipse>
Severity:	normal
Priority:	P3	CC:	contact
Version:	3.2.1
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Whiteboard:

Description Simon Bernard

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

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

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();
    }
}