[hueemulation] Fix warnings/stacktraces at startup (#5709)

* Properly annotates optional references with nullable to fix NPEs
* Reworked HueEmulationConfigWithRuntime.startNow to prevent IllegalStateException

Fixes #5622

Signed-off-by: Wouter Born <github@maindrain.net>

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/HueEmulationService.java

@@ -117,7 +117,7 @@ public void filter(ContainerRequestContext requestContext, ContainerResponseCont
  // Don't fail the service if the upnp server does not come up
  // That part is required for discovery only but does not affect already configured hue applications
  @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption = ReferencePolicyOption.GREEDY)
- protected @NonNullByDefault({}) UpnpServer discovery;
+ protected @Nullable UpnpServer discovery;
  @Reference
  protected @NonNullByDefault({}) ConfigStore cs;
  @Reference
@@ -201,10 +201,11 @@ public void handleEvent(@Nullable Event event) {
  initParams.put("com.sun.jersey.api.json.POJOMappingFeature", "false");
  initParams.put(ServletProperties.PROVIDER_WEB_APP, "false");
  httpService.registerServlet(RESTAPI_PATH, new ServletContainer(resourceConfig), initParams, null);
- if (discovery == null) {
+ UpnpServer localDiscovery = discovery;
+ if (localDiscovery == null) {
  logger.warn("The UPnP Server service has not been started!");
- } else if (!discovery.upnpAnnouncementThreadRunning()) {
- discovery.handleEvent(null);
+ } else if (!localDiscovery.upnpAnnouncementThreadRunning()) {
+ localDiscovery.handleEvent(null);
  }
  statusResource.startUpnpSelfTest();
  logger.info("Hue Emulation service available under {}", RESTAPI_PATH);

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/rest/LightsAndGroups.java

@@ -32,6 +32,7 @@
 import javax.ws.rs.core.UriInfo;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.smarthome.core.common.registry.RegistryChangeListener;
 import org.eclipse.smarthome.core.events.EventPublisher;
 import org.eclipse.smarthome.core.items.GenericItem;
@@ -111,7 +112,7 @@ public class LightsAndGroups implements RegistryChangeListener<Item> {
  @Reference
  protected @NonNullByDefault({}) ItemRegistry itemRegistry;
  @Reference(policy = ReferencePolicy.DYNAMIC, cardinality = ReferenceCardinality.OPTIONAL)
- protected volatile @NonNullByDefault({}) EventPublisher eventPublisher;
+ protected volatile @Nullable EventPublisher eventPublisher;
 
  /**
  * Registers to the {@link ItemRegistry} and enumerates currently existing items.
@@ -373,9 +374,10 @@ public Response setLightStateApi(@Context UriInfo uri, //
 
  // If a command could be created, post it to the framework now
  if (command != null) {
- if (eventPublisher != null) {
+ EventPublisher localEventPublisher = eventPublisher;
+ if (localEventPublisher != null) {
  logger.debug("sending {} to {}", command, itemUID);
- eventPublisher.post(ItemEventFactory.createCommandEvent(itemUID, command, "hueemulation"));
+ localEventPublisher.post(ItemEventFactory.createCommandEvent(itemUID, command, "hueemulation"));
  } else {
  logger.warn("No event publisher. Cannot post item '{}' command!", itemUID);
  }
@@ -418,8 +420,10 @@ public Response setGroupActionApi(@Context UriInfo uri, //
  // If a command could be created, post it to the framework now
  if (command != null) {
  logger.debug("sending {} to {}", command, id);
- if (eventPublisher != null) {
- eventPublisher.post(ItemEventFactory.createCommandEvent(groupItem.getUID(), command, "hueemulation"));
+ EventPublisher localEventPublisher = eventPublisher;
+ if (localEventPublisher != null) {
+ localEventPublisher
+ .post(ItemEventFactory.createCommandEvent(groupItem.getUID(), command, "hueemulation"));
  } else {
  logger.warn("No event publisher. Cannot post item '{}' command!", groupItem.getUID());
  }

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/rest/StatusResource.java

@@ -54,7 +54,7 @@
 @NonNullByDefault
 public class StatusResource implements RegistryListener {
  @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption = ReferencePolicyOption.GREEDY)
- protected @NonNullByDefault({}) UpnpServer discovery;
+ protected @Nullable UpnpServer discovery;
  @Reference
  protected @NonNullByDefault({}) ConfigStore cs;
  @Reference
@@ -125,12 +125,11 @@ private static String TR(String s) {
  @Path("status")
  @Produces("text/html")
  public String getStatus() {
- if (discovery == null) { // Optional service wiring
+ UpnpServer localDiscovery = discovery;
+ if (localDiscovery == null) { // Optional service wiring
  return "UPnP Server service not started!";
  }
 
- int httpPort = discovery.getDefaultport();
-
  String format = "<html><body><h1>Self test</h1>" + //
  "<p>To access any links you need be in pairing mode!</p>" + //
  "<p>Pairing mode: %s (%s) <a href='%s/api/status/link'>Enable</a> | <a href='%s/api/status/link?V1=true'>Enable with bridge V1 emulation</a></p>"
@@ -145,18 +144,18 @@ public String getStatus() {
  + //
  "<h2>Users</h2><ul>%s</ul></body></html>";
 
- final String users = cs.ds.config.whitelist.entrySet().stream().map(user -> "<li>" + user.getKey() + " <b>"
+ String users = cs.ds.config.whitelist.entrySet().stream().map(user -> "<li>" + user.getKey() + " <b>"
  + user.getValue().name + "</b> <small>" + user.getValue().lastUseDate + "</small>")
  .collect(Collectors.joining("\n"));
 
- final String url = "http://" + cs.ds.config.ipaddress + ":" + String.valueOf(httpPort);
+ String url = "http://" + cs.ds.config.ipaddress + ":" + String.valueOf(localDiscovery.getDefaultport());
 
- final String reachable = discovery.selfTests().stream()
+ String reachable = localDiscovery.selfTests().stream()
  .map(entry -> TR(TD(entry.address) + TD(toYesNo(entry.reachable)) + TD(toYesNo(entry.isOurs))))
  .collect(Collectors.joining("\n"));
 
  Registry registry = upnpService.getRegistry();
- final String upnps;
+ String upnps;
  if (registry != null) {
  upnps = registry.getRemoteDevices().stream().map(device -> getDetails(device))
  .map(details -> TR(TD(details.getSerialNumber()) + TD(details.getFriendlyName())))
@@ -165,7 +164,7 @@ public String getStatus() {
  upnps = TR(TD("service not available") + TD(""));
  }
 
- if (!discovery.upnpAnnouncementThreadRunning()) {
+ if (!localDiscovery.upnpAnnouncementThreadRunning()) {
  selfTestUpnpFound = upnpStatus.upnp_announcement_thread_not_running;
  }
 
@@ -224,7 +223,9 @@ public void remoteDeviceUpdated(Registry registry, RemoteDevice device) {
  @NonNullByDefault({})
  @Override
  public void remoteDeviceRemoved(Registry registry, RemoteDevice device) {
- if (selfTestUpnpFound != upnpStatus.success || discovery.upnpAnnouncementThreadRunning()) {
+ UpnpServer localDiscovery = discovery;
+ if (selfTestUpnpFound != upnpStatus.success || localDiscovery == null
+ || localDiscovery.upnpAnnouncementThreadRunning()) {
  return;
  }
  DeviceDetails details = getDetails(device);
@@ -237,7 +238,9 @@ public void remoteDeviceRemoved(Registry registry, RemoteDevice device) {
  @NonNullByDefault({})
  @Override
  public void localDeviceAdded(Registry registry, LocalDevice device) {
- if (selfTestUpnpFound == upnpStatus.success || discovery.upnpAnnouncementThreadRunning()) {
+ UpnpServer localDiscovery = discovery;
+ if (selfTestUpnpFound == upnpStatus.success || localDiscovery == null
+ || localDiscovery.upnpAnnouncementThreadRunning()) {
  return;
  }
  checkForDevice(getDetails(device));

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/upnp/HueEmulationConfigWithRuntime.java

@@ -23,6 +23,8 @@
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.io.hueemulation.internal.HueEmulationConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * The upnp server runtime configuration. Based on a {@link HueEmulationConfig} and determined ip address and port.
@@ -33,6 +35,9 @@
  */
 @NonNullByDefault
 class HueEmulationConfigWithRuntime extends Thread implements Runnable {
+
+ private final Logger logger = LoggerFactory.getLogger(HueEmulationConfigWithRuntime.class);
+
  final @NonNullByDefault({}) HueEmulationConfig config;
  final InetAddress address;
  final String addressString;
@@ -93,7 +98,8 @@ String getMulticastAddress() {
  public synchronized CompletableFuture<@Nullable HueEmulationConfigWithRuntime> startNow(
  @Nullable HueEmulationConfigWithRuntime ignored) {
  if (hasAlreadyBeenStarted) {
- throw new IllegalStateException("Cannot restart thread");
+ logger.debug("Cannot restart thread");
+ return future;
  }
  hasAlreadyBeenStarted = true;
  super.start();

bundles/org.openhab.io.hueemulation/src/main/java/org/openhab/io/hueemulation/internal/upnp/UpnpServer.java

@@ -339,7 +339,7 @@ private void useAddressPort(HueEmulationConfigWithRuntime r) {
  * configuration ready event and depending on service start order we are also called by our own activate() method
  * when the configuration is already ready at that time.
  * <p>
- * Therefore this method is "synchronized" and chains a completeable future for each call to re-evaluate the config
+ * Therefore this method is "synchronized" and chains a completable future for each call to re-evaluate the config
  * after the former future has finished.
  */
  @Override