Community
Participate
Working Groups
SubSystemConfiguration.initializeSubSystem() default implementation currently sets the port of its associated ConnectorService to 0. The problem with this is, that the ConnectorService should know better than any associated SubSystem what a proper default value for its port property is. A second problem is that if any overrider wants to change the functionality and have a different default port value, the ConnectorService is marked dirty unnecessarily, because its port gets set to 0 and then back to the expected user value. I assume that the functionality of setting the port to 0 during initialization comes from the days where RSE did not have a separate ConnectorService. But now, since there is one, the only proper place for specifying a default of the ConnectorService's port is in the ConnectorService's constructor. Subsystems who want to do it differently can override initializeSubSystem() in their own implementation.
All current implementations of ConnectorService set the port to 0 in their constructor, except FTP (sets it to 21 -- the default) and daytime (sets it to 13). I thus expect the following change in functionality for FTP and Daytime only: * maybe improved performance sine ConnectorService is no longer marked dirty * Subsystem Properties of newly created connections for Daytime and FTP will show the actual port being used rather than "0 (First-available)". I consider this an improvement since it better shows users where the port can be actually changed. Note that existing subsystems which were created before this change will still have port "0" shown for Daytime and FTP since this port number (meaning "default port") is stored persistently.
Created attachment 73033 [details] Patch removing the setPort(0) call
Dave could you please review whether you'd consider attached patch appropriate for 2.0.1? It does slightly change the functionality that had been documented in the APIDocs, but IMHO that functionality was not appropriate and removing it just improves the overall situation.
This seems okay to me.
Patch committed - thanks.
