From 2e1ea32e8fd0608588aaeef70cdd8c3acd175cb8 Mon Sep 17 00:00:00 2001 From: lsiepel Date: Tue, 22 Feb 2022 21:00:43 +0100 Subject: [PATCH] [plugwiseha] Improve cache and timeout handling (#12345) * Rework caching and timeout handling * Fix another caching related issue Signed-off-by: Leo Siepel --- .../api/model/PlugwiseHAController.java | 43 ++++++++++++------- .../model/PlugwiseHAControllerRequest.java | 28 ++++++------ .../internal/api/model/dto/Locations.java | 5 --- .../handler/PlugwiseHAApplianceHandler.java | 7 ++- .../handler/PlugwiseHABaseHandler.java | 8 ++-- .../handler/PlugwiseHABridgeHandler.java | 4 +- .../handler/PlugwiseHAZoneHandler.java | 7 ++- 7 files changed, 54 insertions(+), 48 deletions(-) diff --git a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAController.java b/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAController.java index 16c3e3ff001..2b6e4b72108 100644 --- a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAController.java +++ b/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 = 30; + private static final int MAX_AGE_MINUTES_FULL_REFRESH = 15; 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) - throws PlugwiseHAException { + public PlugwiseHAController(HttpClient httpClient, String host, int port, String username, String smileId, + 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 { - Appliances appliances = this.getAppliances(forceRefresh); + public @Nullable Appliance getAppliance(String id) throws PlugwiseHAException { + 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 { - Locations locations = this.getLocations(forceRefresh); + public @Nullable Location getLocation(String id) throws PlugwiseHAException { + 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(); diff --git a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAControllerRequest.java b/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAControllerRequest.java index 3d1e35fa04d..b559f64cd0d 100644 --- a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/PlugwiseHAControllerRequest.java +++ b/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 { 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 { private String getContent() throws PlugwiseHAException { String content; - ContentResponse response; - - try { - response = getContentResponse(); - } catch (PlugwiseHATimeoutException e) { - // Retry - response = getContentResponse(); - } + ContentResponse response = getContentResponse(REQUEST_MAX_RETRY_COUNT); int status = response.getStatus(); switch (status) { @@ -224,18 +219,25 @@ public class PlugwiseHAControllerRequest { return content; } - private ContentResponse getContentResponse() throws PlugwiseHAException { + private ContentResponse getContentResponse(int retries) throws PlugwiseHAException { Request request = newRequest(); ContentResponse response; - if (logger.isTraceEnabled()) { - logger.trace(">> {} {}", request.getMethod(), request.getURI()); - } + this.logger.debug("Performing API request: {} {}", 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(); diff --git a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/dto/Locations.java b/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/dto/Locations.java index 5aacbc40264..5f3ed1b921f 100644 --- a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/api/model/dto/Locations.java +++ b/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 { updatedPointLogs.merge(originalLocation.getPointLogs()); } - ActuatorFunctionalities updatedActuatorFunctionalities = location.getActuatorFunctionalities(); - if (updatedActuatorFunctionalities != null) { - updatedActuatorFunctionalities.merge(originalLocation.getActuatorFunctionalities()); - } - this.put(id, location); } } diff --git a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAApplianceHandler.java b/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAApplianceHandler.java index b3a1ed174c1..0773ff46274 100644 --- a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAApplianceHandler.java +++ b/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 * 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) - throws PlugwiseHAException; + protected abstract @Nullable E getEntity(PlugwiseHAController controller) throws PlugwiseHAException; /** * Handles a {@link RefreshType} command for a given channel. @@ -139,7 +137,7 @@ public abstract class PlugwiseHABaseHandler 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 @Nullable E entity = null; try { - entity = getEntity(controller, false); + entity = getEntity(controller); } catch (PlugwiseHAException e) { updateStatus(OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); setLinkedChannelsUndef(); diff --git a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHABridgeHandler.java b/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHABridgeHandler.java index 1e7a0b61f9c..0be22adccaa 100644 --- a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHABridgeHandler.java +++ b/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) { diff --git a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAZoneHandler.java b/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAZoneHandler.java index d5e9250b339..182debb1b0b 100644 --- a/bundles/org.openhab.binding.plugwiseha/src/main/java/org/openhab/binding/plugwiseha/internal/handler/PlugwiseHAZoneHandler.java +++ b/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