sane-city / wot-servient

W3C Web of Things implementation for Java
MIT License
29 stars 5 forks source link

Wot::destory() call won't reset MqttProtocolSettings #2

Closed Dnnd closed 4 years ago

Dnnd commented 4 years ago

Expected behavior

Calling DefaultWot::serverOnly(Config config) after Wot::destory is independent from previous calls of DefaultWot::serverOnly(Config config)

Observed behavior

Calling DefaultWot::serverOnly(Config config) after Wot::destory will create the instance of Wot with config from the first invocation of DefaultWot::serverOnly(Config config)

Example

public class TestWotRestart {
    private static final Logger logger = LoggerFactory.getLogger(TestWotRestart.class);

    public static Config getConfigV1() {
        String configText = "wot.servient.mqtt.broker = \"tcp://localhost:1883\", wot.servient.servers = [\"city.sane.wot.binding.mqtt.MqttProtocolServer\"]";
        Config wotConfig = ConfigFactory.parseString(configText)
                .withFallback(ConfigFactory.load());
        logger.debug("Expected broker V1: {}",  wotConfig.getString("wot.servient.mqtt.broker"));
        return wotConfig;
    }

    public static Config getConfigV2() {
        String configText = "wot.servient.mqtt.broker = \"tcp://localhost:1884\", wot.servient.servers = [\"city.sane.wot.binding.mqtt.MqttProtocolServer\"]";
        Config wotConfig = ConfigFactory.parseString(configText)
                .withFallback(ConfigFactory.load());
        logger.debug("Expected broker V2: {}", wotConfig.getString("wot.servient.mqtt.broker"));
        return wotConfig;
    }

    public static void main(String[] args) throws WotException, ExecutionException, InterruptedException, IOException {

        Wot wot = DefaultWot.serverOnly(getConfigV1());
        wot.destroy().get();
        wot = DefaultWot.serverOnly(getConfigV2());
        System.in.read();
    }
}

Output:

17:03:55.820 [main] DEBUG ru.mipt.wot.cloud.example.TestWotRestart - Expected broker V1: tcp://localhost:1883
17:03:55.943 [main] DEBUG city.sane.wot.ServientConfig - Servient storing credentials for '[]'
17:03:55.943 [main] INFO city.sane.wot.Servient - Start Servient
17:03:55.946 [main] INFO city.sane.wot.binding.mqtt.MqttProtocolServer - Start MqttServer
17:03:55.953 [ForkJoinPool.commonPool-worker-3] DEBUG city.sane.RefCountResource - First Reference. Create Resource.
17:03:56.065 [ForkJoinPool.commonPool-worker-3] INFO city.sane.wot.binding.mqtt.MqttProtocolSettings - MqttClient trying to connect to broker at 'tcp://localhost:1883' with client ID 'wot20859372974799'
17:03:56.405 [ForkJoinPool.commonPool-worker-3] INFO city.sane.wot.binding.mqtt.MqttProtocolSettings - MqttClient connected to broker at 'tcp://localhost:1883'
17:03:56.405 [main] INFO city.sane.wot.Servient - Stop Servient
17:03:56.406 [main] INFO city.sane.wot.binding.mqtt.MqttProtocolServer - Stop MqttServer
...
17:03:56.431 [main] DEBUG ru.mipt.wot.cloud.example.TestWotRestart - Expected broker V2: tcp://localhost:1884
...
17:03:56.433 [ForkJoinPool.commonPool-worker-3] INFO city.sane.wot.binding.mqtt.MqttProtocolSettings - MqttClient trying to connect to broker at 'tcp://localhost:1883' with client ID 'wot20859843970165'

As you can see, despite different config parameters, the actual configuration is remains unchanged after first call to DefaultWot::serverOnly(Config config).

I suppose, the root cause of this behavior is interaction between SharedMqttClientProvider and MqttProtocolServer classes. The MqttProtocolServer::stop method https://github.com/sane-city/wot-servient/blob/35f8e899e7f22548fc98aaafd1494f26952aa9ef/wot-servient-binding-mqtt/src/main/java/city/sane/wot/binding/mqtt/MqttProtocolServer.java#L70 won't set value of SharedMqttClientProvider::singleton field to null so it's reinitialization https://github.com/sane-city/wot-servient/blob/35f8e899e7f22548fc98aaafd1494f26952aa9ef/wot-servient-binding-mqtt/src/main/java/city/sane/wot/binding/mqtt/SharedMqttClientProvider.java#L21-L22 inside singleton == null clause will never happen.

HeikoBornholdt commented 4 years ago

Thank you for the issue!

Commit https://github.com/sane-city/wot-servient/commit/24918ef49c3963304a4e8000b1f9fb9b52a15e97 should fix the bug. Can you please confirm this bugfix?

Dnnd commented 4 years ago

Yes, https://github.com/sane-city/wot-servient/commit/24918ef49c3963304a4e8000b1f9fb9b52a15e97 fixes this issue. Thank you for fixing this so quickly!