Merge pull request #143 from andylintner/unregister-service-fix

Fix for unregistering service when re-advertising

Author: andylintner

CHANGES.md

@@ -15,6 +15,10 @@
 * Supported valid states for Thermostat, SecuritySystem, HeaterCooler and HumidifierDehumidifier [#108] [#120](https://github.com/hap-java/HAP-Java/pull/120)
 * Support for FilterMaintenance. Can be used as a linked service for an Air Purifier  [#124](https://github.com/hap-java/HAP-Java/pull/124)
 
+## Fixes
+
+* Fix for re-advertising service when using alternative jMDNS implementations.
+
 # HAP-Java 1.1.5
 
 ## Fixes

pom.xml

@@ -148,8 +148,15 @@
 
 		<dependency>
 			<groupId>org.mockito</groupId>
-			<artifactId>mockito-all</artifactId>
-			<version>1.10.19</version>
+			<artifactId>mockito-core</artifactId>
+			<version>3.8.0</version>
+			<scope>test</scope>
+		</dependency>
+		
+		<dependency>
+			<groupId>org.assertj</groupId>
+			<artifactId>assertj-core</artifactId>
+			<version>3.19.0</version>
 			<scope>test</scope>
 		</dependency>
 

src/main/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiser.java

@@ -26,6 +26,7 @@ public class JmdnsHomekitAdvertiser {
   private String setupId;
   private int port;
   private int configurationIndex;
+  private ServiceInfo serviceInfo;
 
   public JmdnsHomekitAdvertiser(JmDNS jmdns) {
     this.jmdns = jmdns;
@@ -87,12 +88,20 @@ public synchronized void setConfigurationIndex(int revision) throws IOException
   }
 
   private void unregisterService() {
-    jmdns.unregisterService(buildServiceInfo());
+    if (serviceInfo != null) {
+      jmdns.unregisterService(serviceInfo);
+      serviceInfo = null;
+    }
   }
 
   private void registerService() throws IOException {
     logger.info("Registering " + SERVICE_TYPE + " on port " + port);
-    jmdns.registerService(buildServiceInfo());
+    if (this.serviceInfo != null) {
+      throw new AssertionError(
+          "Registering an already registered service without unregistering first is not allowed");
+    }
+    serviceInfo = buildServiceInfo();
+    jmdns.registerService(serviceInfo);
   }
 
   private ServiceInfo buildServiceInfo() {

src/test/java/io/github/hapjava/server/impl/jmdns/JmdnsHomekitAdvertiserTest.java

@@ -0,0 +1,66 @@
+package io.github.hapjava.server.impl.jmdns;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.io.IOException;
+import java.net.UnknownHostException;
+import javax.jmdns.JmDNS;
+import javax.jmdns.ServiceInfo;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+public class JmdnsHomekitAdvertiserTest {
+
+  JmdnsHomekitAdvertiser subject;
+  JmDNS jmdns;
+
+  @Before
+  public void setup() throws UnknownHostException, IOException {
+    jmdns = mock(JmDNS.class);
+    subject = new JmdnsHomekitAdvertiser(jmdns);
+  }
+
+  @Test
+  public void testAdvertiseTwiceFails() throws Exception {
+    advertise();
+    assertThatThrownBy(() -> advertise()).isNotNull();
+  }
+
+  /*
+   * Verify that the unregister call is for the initial registered service
+   * when changing discoverability causes advertising to be toggled.
+   */
+  @Test
+  public void testSetDiscoverableAfterAdvertise() throws Exception {
+    subject.setDiscoverable(false);
+    advertise();
+    subject.setDiscoverable(true);
+    assertThat(getArgumentFromUnregister().getPropertyString("sf")).isEqualTo("0");
+  }
+
+  /*
+   * Verify that the unregister call is for the initial registered service
+   * when changing the config index causes advertising to be toggled.
+   */
+  @Test
+  public void testSetConfigurationIndex() throws Exception {
+    subject.setConfigurationIndex(1);
+    advertise();
+    subject.setConfigurationIndex(2);
+    assertThat(getArgumentFromUnregister().getPropertyString("c#")).isEqualTo("1");
+  }
+
+  private ServiceInfo getArgumentFromUnregister() {
+    ArgumentCaptor<ServiceInfo> serviceInfoCaptor = ArgumentCaptor.forClass(ServiceInfo.class);
+    verify(jmdns).unregisterService(serviceInfoCaptor.capture());
+    return serviceInfoCaptor.getValue();
+  }
+
+  private void advertise() throws Exception {
+    subject.advertise("test", "00:00:00:00:00:00", 1234, 1, "1");
+  }
+}