[netatmo] Avoid endless loop when Security claims event history (#17484)

* Avoid looping in events

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

diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/AuthenticationApi.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/AuthenticationApi.java
index 638c8c9882449834c25a9d3184e2cc0d312506f9..a32e371c0d935c6b301d973921dbce6c32948525 100644 (file)
--- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/AuthenticationApi.java
+++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/AuthenticationApi.java
@@ -15,7 +15,6 @@ package org.openhab.binding.netatmo.internal.api;
 import static org.openhab.binding.netatmo.internal.api.data.NetatmoConstants.*;
 import static org.openhab.core.auth.oauth2client.internal.Keyword.*;
 
-import java.net.URI;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -37,8 +36,8 @@ import org.openhab.binding.netatmo.internal.handler.ApiBridgeHandler;
  */
 @NonNullByDefault
 public class AuthenticationApi extends RestManager {
-    public static final URI TOKEN_URI = getApiBaseBuilder(PATH_OAUTH, SUB_PATH_TOKEN).build();
-    public static final URI AUTH_URI = getApiBaseBuilder(PATH_OAUTH, SUB_PATH_AUTHORIZE).build();
+    public static final String TOKEN_URI = getApiBaseBuilder(PATH_OAUTH, SUB_PATH_TOKEN).build().toString();
+    public static final String AUTH_URI = getApiBaseBuilder(PATH_OAUTH, SUB_PATH_AUTHORIZE).build().toString();
 
     private List<Scope> grantedScope = List.of();
     private @Nullable String authorization;
@@ -47,16 +46,9 @@ public class AuthenticationApi extends RestManager {
         super(bridge, FeatureArea.NONE);
     }
 
-    public void setAccessToken(@Nullable String accessToken) {
-        if (accessToken != null) {
-            authorization = "Bearer " + accessToken;
-        } else {
-            authorization = null;
-        }
-    }
-
-    public void setScope(String scope) {
-        grantedScope = Stream.of(scope.split(" ")).map(s -> Scope.valueOf(s.toUpperCase())).toList();
+    public void setAccessToken(@Nullable String accessToken, String scope) {
+        authorization = accessToken != null ? "Bearer " + accessToken : null;
+        grantedScope = Stream.of(scope.toUpperCase().split(" ")).map(Scope::valueOf).toList();
     }
 
     public void dispose() {

diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/SecurityApi.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/SecurityApi.java
index 28825bf3ef66c4a3fa4b320467cd0103a2e34497..8a28bb8696d917a616a4947227e64cea3cf52eb2 100644 (file)
--- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/SecurityApi.java
+++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/SecurityApi.java
@@ -18,6 +18,8 @@ import java.net.URI;
 import java.time.ZonedDateTime;
 import java.util.Comparator;
 import java.util.List;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import javax.ws.rs.core.UriBuilder;
 
@@ -70,12 +72,9 @@ public class SecurityApi extends RestManager {
 
     private List<HomeEvent> getEvents(@Nullable Object... params) throws NetatmoException {
         UriBuilder uriBuilder = getApiUriBuilder(SUB_PATH_GET_EVENTS, params);
-        BodyResponse<Home> body = get(uriBuilder, NAEventsDataResponse.class).getBody();
-        if (body != null) {
-            Home home = body.getElement();
-            if (home != null) {
-                return home.getEvents();
-            }
+        if (get(uriBuilder, NAEventsDataResponse.class).getBody() instanceof BodyResponse<Home> body
+                && body.getElement() instanceof Home home) {
+            return home.getEvents();
         }
         throw new NetatmoException("home should not be null");
     }
@@ -84,17 +83,20 @@ public class SecurityApi extends RestManager {
         List<HomeEvent> events = getEvents(PARAM_HOME_ID, homeId);
 
         // we have to rewind to the latest event just after freshestEventTime
-        if (events.size() > 0) {
+        if (!events.isEmpty()) {
+            String oldestId = "";
             HomeEvent oldestRetrieved = events.get(events.size() - 1);
-            while (oldestRetrieved.getTime().isAfter(freshestEventTime)) {
-                events.addAll(getEvents(PARAM_HOME_ID, homeId, PARAM_EVENT_ID, oldestRetrieved.getId()));
+            while (oldestRetrieved.getTime().isAfter(freshestEventTime) && !oldestId.equals(oldestRetrieved.getId())) {
+                oldestId = oldestRetrieved.getId();
+                events.addAll(getEvents(PARAM_HOME_ID, homeId, PARAM_EVENT_ID, oldestId, PARAM_SIZE, 300));
                 oldestRetrieved = events.get(events.size() - 1);
             }
         }
 
-        // Remove unneeded events being before freshestEventTime
+        // Remove potential duplicates then unneeded events being before freshestEventTime
         return events.stream().filter(event -> event.getTime().isAfter(freshestEventTime))
-                .sorted(Comparator.comparing(HomeEvent::getTime).reversed()).toList();
+                .collect(Collectors.toConcurrentMap(HomeEvent::getId, Function.identity(), (p, q) -> p)).values()
+                .stream().sorted(Comparator.comparing(HomeEvent::getTime).reversed()).toList();
     }
 
     public List<HomeEvent> getPersonEvents(String homeId, String personId) throws NetatmoException {

diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/data/NetatmoConstants.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/data/NetatmoConstants.java
index c2e325361dcb2ebc6085d42d20244034e5c173da..184571e02084a390c42db6b031922976b3a68391 100644 (file)
--- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/data/NetatmoConstants.java
+++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/data/NetatmoConstants.java
@@ -149,6 +149,7 @@ public class NetatmoConstants {
     public static final String PARAM_EVENT_ID = "event_id";
     public static final String PARAM_SCHEDULE_ID = "schedule_id";
     public static final String PARAM_OFFSET = "offset";
+    public static final String PARAM_SIZE = "size";
     public static final String PARAM_GATEWAY_TYPE = "gateway_types";
     public static final String PARAM_MODE = "mode";
     public static final String PARAM_URL = "url";

diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java
index cec105c56712e823ab3b2f86100dfb419a4d6679..5e9f8f1a830f060248e905c01d26f8b387f95bb7 100644 (file)
--- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java
+++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java
@@ -21,7 +21,8 @@ import java.io.InputStream;
 import java.lang.reflect.Constructor;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
-import java.time.LocalDateTime;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayDeque;
 import java.util.Collection;
 import java.util.Deque;
@@ -35,6 +36,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.function.BiFunction;
 
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.UriBuilder;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -80,7 +82,6 @@ import org.openhab.core.auth.client.oauth2.OAuthResponseException;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.ChannelUID;
-import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.thing.ThingUID;
@@ -106,7 +107,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
     private final Logger logger = LoggerFactory.getLogger(ApiBridgeHandler.class);
     private final AuthenticationApi connectApi = new AuthenticationApi(this);
     private final Map<Class<? extends RestManager>, RestManager> managers = new HashMap<>();
-    private final Deque<LocalDateTime> requestsTimestamps = new ArrayDeque<>(200);
+    private final Deque<Instant> requestsTimestamps = new ArrayDeque<>(200);
     private final BindingConfiguration bindingConf;
     private final HttpClient httpClient;
     private final OAuthFactory oAuthFactory;
@@ -150,9 +151,9 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         }
 
         oAuthClientService = oAuthFactory
-                .createOAuthClientService(this.getThing().getUID().getAsString(),
-                        AuthenticationApi.TOKEN_URI.toString(), AuthenticationApi.AUTH_URI.toString(),
-                        configuration.clientId, configuration.clientSecret, FeatureArea.ALL_SCOPES, false)
+                .createOAuthClientService(this.getThing().getUID().getAsString(), AuthenticationApi.TOKEN_URI,
+                        AuthenticationApi.AUTH_URI, configuration.clientId, configuration.clientSecret,
+                        FeatureArea.ALL_SCOPES, false)
                 .withGsonBuilder(new GsonBuilder().registerTypeAdapter(AccessTokenResponse.class,
                         new AccessTokenResponseDeserializer()));
 
@@ -166,7 +167,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             return;
         }
 
-        logger.debug("Connecting to Netatmo API.");
+        logger.debug("Connected to Netatmo API.");
 
         ApiHandlerConfiguration configuration = getConfiguration();
         if (!configuration.webHookUrl.isBlank()) {
@@ -181,10 +182,6 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         }
 
         updateStatus(ThingStatus.ONLINE);
-
-        getThing().getThings().stream().filter(Thing::isEnabled).map(Thing::getHandler)
-                .filter(CommonInterface.class::isInstance).map(CommonInterface.class::cast)
-                .forEach(CommonInterface::expireData);
     }
 
     private boolean authenticate(@Nullable String code, @Nullable String redirectUri) {
@@ -221,9 +218,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             return false;
         }
 
-        connectApi.setAccessToken(accessTokenResponse.getAccessToken());
-        connectApi.setScope(accessTokenResponse.getScope());
-
+        connectApi.setAccessToken(accessTokenResponse.getAccessToken(), accessTokenResponse.getScope());
         return true;
     }
 
@@ -233,6 +228,7 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
         grantServlet = Optional.of(servlet);
         updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
                 ConfigurationLevel.REFRESH_TOKEN_NEEDED.message.formatted(servlet.getPath()));
+        connectApi.dispose();
     }
 
     public ApiHandlerConfiguration getConfiguration() {
@@ -317,20 +313,21 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
                 InputStream stream = new ByteArrayInputStream(payload.getBytes(StandardCharsets.UTF_8));
                 try (InputStreamContentProvider inputStreamContentProvider = new InputStreamContentProvider(stream)) {
                     request.content(inputStreamContentProvider, contentType);
-                    request.header(HttpHeader.ACCEPT, "application/json");
+                    request.header(HttpHeader.ACCEPT, MediaType.APPLICATION_JSON);
                 }
                 logger.trace(" -with payload: {} ", payload);
             }
 
             if (isLinked(requestCountChannelUID)) {
-                LocalDateTime now = LocalDateTime.now();
-                LocalDateTime oneHourAgo = now.minusHours(1);
+                Instant now = Instant.now();
                 requestsTimestamps.addLast(now);
+                Instant oneHourAgo = now.minus(1, ChronoUnit.HOURS);
                 while (requestsTimestamps.getFirst().isBefore(oneHourAgo)) {
                     requestsTimestamps.removeFirst();
                 }
                 updateState(requestCountChannelUID, new DecimalType(requestsTimestamps.size()));
             }
+
             logger.trace(" -with headers: {} ",
                     String.join(", ", request.getHeaders().stream().map(HttpField::toString).toList()));
             ContentResponse response = request.send();
@@ -355,6 +352,8 @@ public class ApiBridgeHandler extends BaseBridgeHandler {
             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) {

diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/SecurityCapability.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/SecurityCapability.java
index f3dca854c85d0fe68544b25ad66b9d439f64b7dc..255c7754f97a5f79d631553d8611d33f63d04330 100644 (file)
--- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/SecurityCapability.java
+++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/capability/SecurityCapability.java
@@ -68,7 +68,6 @@ class SecurityCapability extends RestCapability<SecurityApi> {
     @Override
     public void initialize() {
         super.initialize();
-        freshestEventTime = ZDT_REFERENCE;
         securityId = handler.getThingConfigAs(HomeConfiguration.class).getIdForArea(FeatureArea.SECURITY);
     }
 
@@ -124,28 +123,28 @@ class SecurityCapability extends RestCapability<SecurityApi> {
     protected List<NAObject> updateReadings(SecurityApi api) {
         List<NAObject> result = new ArrayList<>();
         try {
-            for (HomeEvent event : api.getHomeEvents(securityId, freshestEventTime)) {
-                HomeEvent previousEvent = eventBuffer.get(event.getCameraId());
-                if (previousEvent == null || previousEvent.getTime().isBefore(event.getTime())) {
-                    eventBuffer.put(event.getCameraId(), event);
-                }
-                String personId = event.getPersonId();
-                if (personId != null) {
-                    previousEvent = eventBuffer.get(personId);
-                    if (previousEvent == null || previousEvent.getTime().isBefore(event.getTime())) {
-                        eventBuffer.put(personId, event);
-                    }
+            api.getHomeEvents(securityId, freshestEventTime).stream().forEach(event -> {
+                bufferIfNewer(event.getCameraId(), event);
+                if (event.getPersonId() instanceof String personId) {
+                    bufferIfNewer(personId, event);
                 }
                 if (event.getTime().isAfter(freshestEventTime)) {
                     freshestEventTime = event.getTime();
                 }
-            }
+            });
         } catch (NetatmoException e) {
             logger.warn("Error retrieving last events for home '{}' : {}", securityId, e.getMessage());
         }
         return result;
     }
 
+    private void bufferIfNewer(String id, HomeEvent event) {
+        HomeEvent previousEvent = eventBuffer.get(id);
+        if (previousEvent == null || previousEvent.getTime().isBefore(event.getTime())) {
+            eventBuffer.put(id, event);
+        }
+    }
+
     public NAObjectMap<HomeDataPerson> getPersons() {
         return persons;
     }