[plugwiseha] Improve cache and timeout handling (#12345)

* Rework caching and timeout handling
* Fix another caching related issue

Signed-off-by: Leo Siepel <leosiepel@gmail.com>

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAController.java

@@ -53,8 +53,7 @@ public class PlugwiseHAController {
 
     // Private member variables/constants
 
-    private static final int MAX_AGE_MINUTES_REFRESH = 10;
+    private static final int MAX_AGE_MINUTES_FULL_REFRESH = 15;
-    private static final int MAX_AGE_MINUTES_FULL_REFRESH = 30;
     private static final DateTimeFormatter FORMAT = DateTimeFormatter.RFC_1123_DATE_TIME; // default Date format that
                                                                                           // will be used in conversion
 
@@ -68,18 +67,20 @@ public class PlugwiseHAController {
     private final int port;
     private final String username;
     private final String smileId;
+    private final int maxAgeSecondsRefresh;
 
     private @Nullable ZonedDateTime gatewayUpdateDateTime;
     private @Nullable ZonedDateTime gatewayFullUpdateDateTime;
     private @Nullable DomainObjects domainObjects;
 
-    public PlugwiseHAController(HttpClient httpClient, String host, int port, String username, String smileId)
+    public PlugwiseHAController(HttpClient httpClient, String host, int port, String username, String smileId,
-            throws PlugwiseHAException {
+            int maxAgeSecondsRefresh) throws PlugwiseHAException {
         this.httpClient = httpClient;
         this.host = host;
         this.port = port;
         this.username = username;
         this.smileId = smileId;
+        this.maxAgeSecondsRefresh = maxAgeSecondsRefresh;
 
         this.xStream = new PlugwiseHAXStream();
 
@@ -165,8 +166,12 @@ public class PlugwiseHAController {
         }
     }
 
-    public @Nullable Appliance getAppliance(String id, Boolean forceRefresh) throws PlugwiseHAException {
+    public @Nullable Appliance getAppliance(String id) throws PlugwiseHAException {
-        Appliances appliances = this.getAppliances(forceRefresh);
+        Appliances appliances = this.getAppliances(false);
+        if (!appliances.containsKey(id)) {
+            appliances = this.getAppliances(true);
+        }
+
         if (!appliances.containsKey(id)) {
             this.logger.debug("Plugwise Home Automation Appliance with id {} is not known", id);
             return null;
@@ -203,8 +208,11 @@ public class PlugwiseHAController {
         }
     }
 
-    public @Nullable Location getLocation(String id, Boolean forceRefresh) throws PlugwiseHAException {
+    public @Nullable Location getLocation(String id) throws PlugwiseHAException {
-        Locations locations = this.getLocations(forceRefresh);
+        Locations locations = this.getLocations(false);
+        if (!locations.containsKey(id)) {
+            locations = this.getLocations(true);
+        }
 
         if (!locations.containsKey(id)) {
             this.logger.debug("Plugwise Home Automation Zone with {} is not known", id);
@@ -221,10 +229,11 @@ public class PlugwiseHAController {
 
         request.setPath("/core/domain_objects");
         request.addPathParameter("@locale", "en-US");
-
         DomainObjects domainObjects = executeRequest(request);
-        this.gatewayUpdateDateTime = ZonedDateTime.parse(request.getServerDateTime(), PlugwiseHAController.FORMAT);
+
-        this.gatewayFullUpdateDateTime = this.gatewayUpdateDateTime;
+        ZonedDateTime serverTime = ZonedDateTime.parse(request.getServerDateTime(), PlugwiseHAController.FORMAT);
+        this.gatewayUpdateDateTime = serverTime;
+        this.gatewayFullUpdateDateTime = serverTime;
 
         return mergeDomainObjects(domainObjects);
     }
@@ -232,14 +241,16 @@ public class PlugwiseHAController {
     public @Nullable DomainObjects getUpdatedDomainObjects() throws PlugwiseHAException {
         ZonedDateTime localGatewayUpdateDateTime = this.gatewayUpdateDateTime;
         ZonedDateTime localGatewayFullUpdateDateTime = this.gatewayFullUpdateDateTime;
-        if (localGatewayUpdateDateTime == null
+
-                || localGatewayUpdateDateTime.isBefore(ZonedDateTime.now().minusMinutes(MAX_AGE_MINUTES_REFRESH))) {
+        if (localGatewayUpdateDateTime == null || localGatewayFullUpdateDateTime == null) {
             return getDomainObjects();
-        } else if (localGatewayFullUpdateDateTime == null || localGatewayFullUpdateDateTime
+        } else if (localGatewayUpdateDateTime.isBefore(ZonedDateTime.now().minusSeconds(maxAgeSecondsRefresh))) {
+            return getUpdatedDomainObjects(localGatewayUpdateDateTime);
+        } else if (localGatewayFullUpdateDateTime
                 .isBefore(ZonedDateTime.now().minusMinutes(MAX_AGE_MINUTES_FULL_REFRESH))) {
             return getDomainObjects();
         } else {
-            return getUpdatedDomainObjects(localGatewayUpdateDateTime);
+            return null;
         }
     }
 
@@ -428,6 +439,7 @@ public class PlugwiseHAController {
     private DomainObjects mergeDomainObjects(@Nullable DomainObjects updatedDomainObjects) {
         DomainObjects localDomainObjects = this.domainObjects;
         if (localDomainObjects == null && updatedDomainObjects != null) {
+            this.domainObjects = updatedDomainObjects;
             return updatedDomainObjects;
         } else if (localDomainObjects != null && updatedDomainObjects == null) {
             return localDomainObjects;
@@ -442,6 +454,7 @@ public class PlugwiseHAController {
             if (locations != null) {
                 localDomainObjects.mergeLocations(locations);
             }
+            this.domainObjects = localDomainObjects;
             return localDomainObjects;
         } else {
             return new DomainObjects();

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAControllerRequest.java

@@ -62,12 +62,14 @@ import com.thoughtworks.xstream.XStream;
  * PlugwiseHAController}.
  * 
  * @author B. van Wetten - Initial contribution
+ * @author Leo Siepel - Adjustments to timeout logic
  */
 @NonNullByDefault
 public class PlugwiseHAControllerRequest<T> {
 
     private static final String CONTENT_TYPE_TEXT_XML = MimeTypes.Type.TEXT_XML_8859_1.toString();
     private static final long TIMEOUT_SECONDS = 5;
+    private static final int REQUEST_MAX_RETRY_COUNT = 3;
 
     private final Logger logger = LoggerFactory.getLogger(PlugwiseHAControllerRequest.class);
     private final XStream xStream;
@@ -191,14 +193,7 @@ public class PlugwiseHAControllerRequest<T> {
 
     private String getContent() throws PlugwiseHAException {
         String content;
-        ContentResponse response;
+        ContentResponse response = getContentResponse(REQUEST_MAX_RETRY_COUNT);
-
-        try {
-            response = getContentResponse();
-        } catch (PlugwiseHATimeoutException e) {
-            // Retry
-            response = getContentResponse();
-        }
 
         int status = response.getStatus();
         switch (status) {
@@ -224,18 +219,25 @@ public class PlugwiseHAControllerRequest<T> {
         return content;
     }
 
-    private ContentResponse getContentResponse() throws PlugwiseHAException {
+    private ContentResponse getContentResponse(int retries) throws PlugwiseHAException {
         Request request = newRequest();
         ContentResponse response;
 
-        if (logger.isTraceEnabled()) {
+        this.logger.debug("Performing API request: {} {}", request.getMethod(), request.getURI());
-            logger.trace(">> {} {}", request.getMethod(), request.getURI());
-        }
 
         try {
             response = request.send();
-        } catch (TimeoutException | InterruptedException e) {
+        } catch (InterruptedException e) {
+            this.logger.trace("InterruptedException occured {} {}", e.getMessage(), e.getStackTrace());
+            Thread.currentThread().interrupt();
             throw new PlugwiseHATimeoutException(e);
+        } catch (TimeoutException e) {
+            if (retries > 0) {
+                this.logger.debug("TimeoutException occured, remaining retries {}", retries - 1);
+                return getContentResponse(retries - 1);
+            } else {
+                throw new PlugwiseHATimeoutException(e);
+            }
         } catch (ExecutionException e) {
             // Unwrap the cause and try to cleanly handle it
             Throwable cause = e.getCause();

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/dto/Locations.java

@@ -42,11 +42,6 @@ public class Locations extends PlugwiseHACollection<Location> {
                         updatedPointLogs.merge(originalLocation.getPointLogs());
                     }
 
-                    ActuatorFunctionalities updatedActuatorFunctionalities = location.getActuatorFunctionalities();
-                    if (updatedActuatorFunctionalities != null) {
-                        updatedActuatorFunctionalities.merge(originalLocation.getActuatorFunctionalities());
-                    }
-
                     this.put(id, location);
                 }
             }

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAApplianceHandler.java

@@ -96,7 +96,7 @@ public class PlugwiseHAApplianceHandler extends PlugwiseHABaseHandler<Appliance,
             try {
                 PlugwiseHAController controller = bridgeHandler.getController();
                 if (controller != null) {
-                    this.appliance = getEntity(controller, true);
+                    this.appliance = getEntity(controller);
                     Appliance localAppliance = this.appliance;
                     if (localAppliance != null) {
                         if (localAppliance.isBatteryOperated()) {
@@ -117,10 +117,9 @@ public class PlugwiseHAApplianceHandler extends PlugwiseHABaseHandler<Appliance,
     }
 
     @Override
-    protected @Nullable Appliance getEntity(PlugwiseHAController controller, Boolean forceRefresh)
+    protected @Nullable Appliance getEntity(PlugwiseHAController controller) throws PlugwiseHAException {
-            throws PlugwiseHAException {
         PlugwiseHAThingConfig config = getPlugwiseThingConfig();
-        Appliance appliance = controller.getAppliance(config.getId(), forceRefresh);
+        Appliance appliance = controller.getAppliance(config.getId());
 
         return appliance;
     }

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHABaseHandler.java

@@ -79,10 +79,8 @@ public abstract class PlugwiseHABaseHandler<E, C extends PlugwiseHAThingConfig>
      * Get the Plugwise Entity that belongs to this ThingHandler
      *
      * @param controller the controller for this ThingHandler
-     * @param forceRefresh indicated if the entity should be refreshed from the Plugwise API
      */
-    protected abstract @Nullable E getEntity(PlugwiseHAController controller, Boolean forceRefresh)
+    protected abstract @Nullable E getEntity(PlugwiseHAController controller) throws PlugwiseHAException;
-            throws PlugwiseHAException;
 
     /**
      * Handles a {@link RefreshType} command for a given channel.
@@ -139,7 +137,7 @@ public abstract class PlugwiseHABaseHandler<E, C extends PlugwiseHAThingConfig>
             if (controller != null) {
                 try {
                     @Nullable
-                    E entity = getEntity(controller, false);
+                    E entity = getEntity(controller);
                     if (entity != null) {
                         if (this.isLinked(channelUID)) {
                             if (command instanceof RefreshType) {
@@ -225,7 +223,7 @@ public abstract class PlugwiseHABaseHandler<E, C extends PlugwiseHAThingConfig>
                     @Nullable
                     E entity = null;
                     try {
-                        entity = getEntity(controller, false);
+                        entity = getEntity(controller);
                     } catch (PlugwiseHAException e) {
                         updateStatus(OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
                         setLinkedChannelsUndef();

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHABridgeHandler.java

@@ -95,7 +95,7 @@ public class PlugwiseHABridgeHandler extends BaseBridgeHandler {
             logger.debug("Initializing the Plugwise Home Automation bridge handler with config = {}", bridgeConfig);
             try {
                 this.controller = new PlugwiseHAController(httpClient, bridgeConfig.getHost(), bridgeConfig.getPort(),
-                        bridgeConfig.getUsername(), bridgeConfig.getsmileId());
+                        bridgeConfig.getUsername(), bridgeConfig.getsmileId(), bridgeConfig.getRefresh());
                 scheduleRefreshJob(bridgeConfig);
             } catch (PlugwiseHAException e) {
                 updateStatus(OFFLINE, CONFIGURATION_ERROR, e.getMessage());
@@ -163,7 +163,7 @@ public class PlugwiseHABridgeHandler extends BaseBridgeHandler {
 
     private void run() {
         try {
-            logger.trace("Executing refresh job");
+            this.logger.trace("Executing refresh job");
             refresh();
 
             if (super.thing.getStatus() == ThingStatus.INITIALIZING) {

bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAZoneHandler.java

@@ -86,7 +86,7 @@ public class PlugwiseHAZoneHandler extends PlugwiseHABaseHandler<Location, Plugw
             try {
                 PlugwiseHAController controller = bridgeHandler.getController();
                 if (controller != null) {
-                    this.location = getEntity(controller, true);
+                    this.location = getEntity(controller);
                     if (this.location != null) {
                         setLocationProperties();
                         updateStatus(ONLINE);
@@ -103,10 +103,9 @@ public class PlugwiseHAZoneHandler extends PlugwiseHABaseHandler<Location, Plugw
     }
 
     @Override
-    protected @Nullable Location getEntity(PlugwiseHAController controller, Boolean forceRefresh)
+    protected @Nullable Location getEntity(PlugwiseHAController controller) throws PlugwiseHAException {
-            throws PlugwiseHAException {
         PlugwiseHAThingConfig config = getPlugwiseThingConfig();
-        Location location = controller.getLocation(config.getId(), forceRefresh);
+        Location location = controller.getLocation(config.getId());
 
         return location;
     }