Sitemap SSE: check that the subscription is linked to a page (#5556)

* Sitemap SSE: check that the subscription is linked to a page
* Sitemap SSE: eventOutputs and broadcaster cleaned when the SSE connection is properly closed
* Sitemap SSE: enhanced debug logs to show how many subscriptions are active

Fix #5211

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

bundles/io/org.eclipse.smarthome.io.rest.sitemap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/SitemapSubscriptionService.java

@@ -122,7 +122,7 @@ protected void removeSitemapProvider(SitemapProvider provider) {
     public String createSubscription(SitemapSubscriptionCallback callback) {
         String subscriptionId = UUID.randomUUID().toString();
         callbacks.put(subscriptionId, callback);
-        logger.debug("Created new subscription with id {}", subscriptionId);
+        logger.debug("Created new subscription with id {} ({} active subscriptions)", subscriptionId, callbacks.size());
         return subscriptionId;
     }
 
@@ -141,7 +141,7 @@ public void removeSubscription(String subscriptionId) {
                 listener.dispose();
             }
         }
-        logger.debug("Removed subscription with id {}", subscriptionId);
+        logger.debug("Removed subscription with id {} ({} active subscriptions)", subscriptionId, callbacks.size());
     }
 
     /**
@@ -158,20 +158,22 @@ public boolean exists(String subscriptionId) {
      * Retrieves the current page id for a subscription.
      *
      * @param subscriptionId the subscription to get the page id for
-     * @return the id of the currently active page
+     * @return the id of the currently active page or null if no page is currently set for the subscription
      */
     public String getPageId(String subscriptionId) {
-        return extractPageId(pageOfSubscription.get(subscriptionId));
+        String sitemapWithPageId = pageOfSubscription.get(subscriptionId);
+        return (sitemapWithPageId == null) ? null : extractPageId(sitemapWithPageId);
     }
 
     /**
      * Retrieves the current sitemap name for a subscription.
      *
      * @param subscriptionId the subscription to get the sitemap name for
-     * @return the name of the current sitemap
+     * @return the name of the current sitemap or null if no sitemap is currently set for the subscription
      */
     public String getSitemapName(String subscriptionId) {
-        return extractSitemapName(pageOfSubscription.get(subscriptionId));
+        String sitemapWithPageId = pageOfSubscription.get(subscriptionId);
+        return (sitemapWithPageId == null) ? null : extractSitemapName(sitemapWithPageId);
     }
 
     private String extractSitemapName(String sitemapWithPageId) {
@@ -199,8 +201,8 @@ public void setPageId(String subscriptionId, String sitemapName, String pageId)
             addCallbackToListener(sitemapName, pageId, callback);
             pageOfSubscription.put(subscriptionId, getValue(sitemapName, pageId));
 
-            logger.debug("Subscription {} changed to page {} of sitemap {}",
-                    new Object[] { subscriptionId, pageId, sitemapName });
+            logger.debug("Subscription {} changed to page {} of sitemap {} ({} active subscriptions}",
+                    new Object[] { subscriptionId, pageId, sitemapName, callbacks.size() });
         } else {
             throw new IllegalArgumentException("Subscription " + subscriptionId + " does not exist!");
         }

bundles/io/org.eclipse.smarthome.io.rest.sitemap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/internal/SitemapResource.java

@@ -281,6 +281,10 @@ public Object getSitemapEvents(
         if (sitemapname != null && pageId != null) {
             subscriptions.setPageId(subscriptionId, sitemapname, pageId);
         }
+        if (subscriptions.getSitemapName(subscriptionId) == null || subscriptions.getPageId(subscriptionId) == null) {
+            return JSONResponse.createResponse(Status.BAD_REQUEST, null,
+                    "Subscription id " + subscriptionId + " is not yet linked to a sitemap/page.");
+        }
         logger.debug("Client requested sitemap event stream for subscription {}.", subscriptionId);
 
         // Disables proxy buffering when using an nginx http server proxy for this response.
@@ -745,6 +749,10 @@ public void onClose(ChunkedOutput<OutboundEvent> event) {
             SitemapEventOutput sitemapEvent = (SitemapEventOutput) event;
             logger.debug("SSE connection for subscription {} has been closed.", sitemapEvent.getSubscriptionId());
             subscriptions.removeSubscription(sitemapEvent.getSubscriptionId());
+            EventOutput eventOutput = eventOutputs.remove(sitemapEvent.getSubscriptionId());
+            if (eventOutput != null) {
+                broadcaster.remove(eventOutput);
+            }
         }
     }