[netatmo] API limit reached handling (#16489)

Signed-off-by: Gaël L'hopital <gael@lhopital.org>

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/NetatmoBindingConstants.java

@@ -25,7 +25,6 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
  */
 @NonNullByDefault
 public class NetatmoBindingConstants {
-
     public static final String BINDING_ID = "netatmo";
     public static final String VENDOR = "Netatmo";
 

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/config/ApiHandlerConfiguration.java

@@ -29,4 +29,13 @@ public class ApiHandlerConfiguration {
     public String webHookUrl = "";
     public String webHookPostfix = "";
     public int reconnectInterval = 300;
+
+    public ConfigurationLevel check() {
+        if (clientId.isBlank()) {
+            return ConfigurationLevel.EMPTY_CLIENT_ID;
+        } else if (clientSecret.isBlank()) {
+            return ConfigurationLevel.EMPTY_CLIENT_SECRET;
+        }
+        return ConfigurationLevel.COMPLETED;
+    }
 }

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/config/ConfigurationLevel.java

@@ -23,7 +23,6 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
 public enum ConfigurationLevel {
     EMPTY_CLIENT_ID("@text/conf-error-no-client-id"),
     EMPTY_CLIENT_SECRET("@text/conf-error-no-client-secret"),
-    REFRESH_TOKEN_NEEDED("@text/conf-error-grant-needed [ \"%s\" ]"),
     COMPLETED("");
 
     public String message;

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java

@@ -18,7 +18,6 @@ import static org.openhab.binding.netatmo.internal.NetatmoBindingConstants.*;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.reflect.Constructor;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
@@ -103,6 +102,7 @@ import com.google.gson.GsonBuilder;
 @NonNullByDefault
 public class ApiBridgeHandler extends BaseBridgeHandler {
     private static final int TIMEOUT_S = 20;
+    private static final int API_LIMIT_INTERVAL_S = 3600;
 
     private final Logger logger = LoggerFactory.getLogger(ApiBridgeHandler.class);
     private final AuthenticationApi connectApi = new AuthenticationApi(this);
@@ -128,7 +128,6 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         this.deserializer = deserializer;
         this.httpService = httpService;
         this.oAuthFactory = oAuthFactory;
-
         requestCountChannelUID = new ChannelUID(thing.getUID(), GROUP_MONITORING, CHANNEL_REQUEST_COUNT);
     }
 
@@ -138,15 +137,9 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
 
         ApiHandlerConfiguration configuration = getConfiguration();
 
-        if (configuration.clientId.isBlank()) {
+        ConfigurationLevel confLevel = configuration.check();
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+        if (!ConfigurationLevel.COMPLETED.equals(confLevel)) {
-                    ConfigurationLevel.EMPTY_CLIENT_ID.message);
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, confLevel.message);
-            return;
-        }
-
-        if (configuration.clientSecret.isBlank()) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                    ConfigurationLevel.EMPTY_CLIENT_SECRET.message);
             return;
         }
 
@@ -170,15 +163,13 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         logger.debug("Connected to Netatmo API.");
 
         ApiHandlerConfiguration configuration = getConfiguration();
-        if (!configuration.webHookUrl.isBlank()) {
+        if (!configuration.webHookUrl.isBlank()
-            SecurityApi securityApi = getRestManager(SecurityApi.class);
+                && getRestManager(SecurityApi.class) instanceof SecurityApi securityApi) {
-            if (securityApi != null) {
+            webHookServlet.ifPresent(servlet -> servlet.dispose());
-                webHookServlet.ifPresent(servlet -> servlet.dispose());
+            WebhookServlet servlet = new WebhookServlet(this, httpService, deserializer, securityApi,
-                WebhookServlet servlet = new WebhookServlet(this, httpService, deserializer, securityApi,
+                    configuration.webHookUrl, configuration.webHookPostfix);
-                        configuration.webHookUrl, configuration.webHookPostfix);
+            servlet.startListening();
-                servlet.startListening();
+            this.webHookServlet = Optional.of(servlet);
-                this.webHookServlet = Optional.of(servlet);
-            }
         }
 
         updateStatus(ThingStatus.ONLINE);
@@ -197,8 +188,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
                 accessTokenResponse = oAuthClientService.getAccessTokenResponseByAuthorizationCode(code, redirectUri);
 
                 // Dispose grant servlet upon completion of authorization flow.
-                grantServlet.ifPresent(servlet -> servlet.dispose());
+                freeGrantServlet();
-                grantServlet = Optional.empty();
             } else {
                 accessTokenResponse = oAuthClientService.getAccessTokenResponse();
             }
@@ -207,8 +197,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             startAuthorizationFlow();
             return false;
         } catch (IOException e) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
+            prepareReconnection(getConfiguration().reconnectInterval, e.getMessage(), code, redirectUri);
-            prepareReconnection(code, redirectUri);
             return false;
         }
 
@@ -227,7 +216,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         servlet.startListening();
         grantServlet = Optional.of(servlet);
         updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                ConfigurationLevel.REFRESH_TOKEN_NEEDED.message.formatted(servlet.getPath()));
+                "@text/conf-error-grant-needed [ \"%s\" ]".formatted(servlet.getPath()));
         connectApi.dispose();
     }
 
@@ -235,11 +224,15 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         return getConfigAs(ApiHandlerConfiguration.class);
     }
 
-    private void prepareReconnection(@Nullable String code, @Nullable String redirectUri) {
+    private void prepareReconnection(int delay, @Nullable String message, @Nullable String code,
+            @Nullable String redirectUri) {
+        if (!ThingStatus.OFFLINE.equals(thing.getStatus())) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, message);
+        }
         connectApi.dispose();
         freeConnectJob();
-        connectJob = Optional.of(scheduler.schedule(() -> openConnection(code, redirectUri),
+        connectJob = Optional.of(scheduler.schedule(() -> openConnection(code, redirectUri), delay, TimeUnit.SECONDS));
-                getConfiguration().reconnectInterval, TimeUnit.SECONDS));
+        logger.debug("Reconnection scheduled in {} seconds", delay);
     }
 
     private void freeConnectJob() {
@@ -247,6 +240,11 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         connectJob = Optional.empty();
     }
 
+    private void freeGrantServlet() {
+        grantServlet.ifPresent(servlet -> servlet.dispose());
+        grantServlet = Optional.empty();
+    }
+
     @Override
     public void dispose() {
         logger.debug("Shutting down Netatmo API bridge handler.");
@@ -254,8 +252,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         webHookServlet.ifPresent(servlet -> servlet.dispose());
         webHookServlet = Optional.empty();
 
-        grantServlet.ifPresent(servlet -> servlet.dispose());
+        freeGrantServlet();
-        grantServlet = Optional.empty();
 
         connectApi.dispose();
         freeConnectJob();
@@ -280,13 +277,12 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
     public <T extends RestManager> @Nullable T getRestManager(Class<T> clazz) {
         if (!managers.containsKey(clazz)) {
             try {
-                Constructor<T> constructor = clazz.getConstructor(getClass());
+                T instance = clazz.getConstructor(getClass()).newInstance(this);
-                T instance = constructor.newInstance(this);
                 Set<Scope> expected = instance.getRequiredScopes();
                 if (connectApi.matchesScopes(expected)) {
                     managers.put(clazz, instance);
                 } else {
-                    logger.info("Unable to instantiate {}, expected scope {} is not active", clazz, expected);
+                    logger.warn("Unable to instantiate {}, expected scope {} is not active", clazz, expected);
                 }
             } catch (SecurityException | ReflectiveOperationException e) {
                 logger.warn("Error invoking RestManager constructor for class {}: {}", clazz, e.getMessage());
@@ -303,7 +299,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             Request request = httpClient.newRequest(uri).method(method).timeout(TIMEOUT_S, TimeUnit.SECONDS);
 
             if (!authenticate(null, null)) {
-                prepareReconnection(null, null);
+                prepareReconnection(getConfiguration().reconnectInterval, "@text/status-bridge-offline", null, null);
                 throw new NetatmoException("Not authenticated");
             }
             connectApi.getAuthorization().ifPresent(auth -> request.header(HttpHeader.AUTHORIZATION, auth));
@@ -345,29 +341,30 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             try {
                 exception = new NetatmoException(deserializer.deserialize(ApiError.class, responseBody));
             } catch (NetatmoException e) {
-                exception = new NetatmoException("Error deserializing error: %s".formatted(statusCode.getMessage()));
+                if (statusCode == Code.TOO_MANY_REQUESTS) {
+                    exception = new NetatmoException(statusCode.getMessage());
+                } else {
+                    exception = new NetatmoException(
+                            "Error deserializing error: %s".formatted(statusCode.getMessage()));
+                }
+            }
+            if (statusCode == Code.TOO_MANY_REQUESTS
+                    || exception.getStatusCode() == ServiceError.MAXIMUM_USAGE_REACHED) {
+                prepareReconnection(API_LIMIT_INTERVAL_S,
+                        "@text/maximum-usage-reached [ \"%d\" ]".formatted(API_LIMIT_INTERVAL_S), null, null);
             }
             throw exception;
-        } catch (NetatmoException e) {
-            if (e.getStatusCode() == ServiceError.MAXIMUM_USAGE_REACHED) {
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/maximum-usage-reached");
-                prepareReconnection(null, null);
-            } else if (e.getStatusCode() == ServiceError.INVALID_TOKEN_MISSING) {
-                startAuthorizationFlow();
-            }
-            throw e;
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
-            throw new NetatmoException("Request interrupted");
+            throw new NetatmoException(e, "Request interrupted");
         } catch (TimeoutException | ExecutionException e) {
             if (retryCount > 0) {
-                logger.debug("Request timedout, retry counter: {}", retryCount);
+                logger.debug("Request error, retry counter: {}", retryCount);
                 return executeUri(uri, method, clazz, payload, contentType, retryCount - 1);
             }
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "@text/request-time-out");
+            prepareReconnection(getConfiguration().reconnectInterval, "@text/request-time-out", null, e.getMessage());
-            prepareReconnection(null, null);
+            throw new NetatmoException("%s: \"%s\"".formatted(e.getClass().getName(), e.getMessage()));
-            throw new NetatmoException(String.format("%s: \"%s\"", e.getClass().getName(), e.getMessage()));
         }
     }
 

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/AlarmEventCapability.java

@@ -49,6 +49,7 @@ public class AlarmEventCapability extends HomeSecurityThingCapability {
         handler.updateState(GROUP_LAST_EVENT, CHANNEL_EVENT_TIME, toDateTimeType(event.getTime()));
         handler.updateState(GROUP_LAST_EVENT, CHANNEL_EVENT_SUBTYPE, Objects.requireNonNull(
                 event.getSubTypeDescription().map(ChannelTypeUtils::toStringType).orElse(UnDefType.NULL)));
+
         final String message = event.getName();
         handler.updateState(GROUP_LAST_EVENT, CHANNEL_EVENT_MESSAGE,
                 message == null || message.isBlank() ? UnDefType.NULL : toStringType(message));

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CameraCapability.java

@@ -73,7 +73,7 @@ public class CameraCapability extends HomeSecurityThingCapability {
             List<ChannelHelper> channelHelpers) {
         super(handler, descriptionProvider, channelHelpers);
         this.personChannelUID = new ChannelUID(thingUID, GROUP_LAST_EVENT, CHANNEL_EVENT_PERSON_ID);
-        this.cameraHelper = (CameraChannelHelper) channelHelpers.stream().filter(c -> c instanceof CameraChannelHelper)
+        this.cameraHelper = (CameraChannelHelper) channelHelpers.stream().filter(CameraChannelHelper.class::isInstance)
                 .findFirst().orElseThrow(() -> new IllegalArgumentException(
                         "CameraCapability must find a CameraChannelHelper, please file a bug report."));
     }

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties

@@ -465,7 +465,7 @@ device-not-connected = Thing is not reachable
 data-over-limit = Data seems quite old
 request-time-out = Request timed out - will attempt reconnection later
 deserialization-unknown = Deserialization lead to an unknown code
-maximum-usage-reached = Maximum usage reached. Will try reconnection after `reconnectInterval` seconds.
+maximum-usage-reached = Maximum usage reached, will reconnect in {0} seconds.
 
 homestatus-unknown-error = Unknown error
 homestatus-internal-error = Internal error