From 405ed71f25dda6f78074f4cf58b533dbf02ddc77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20L=27hopital?= Date: Sat, 5 Oct 2024 21:44:07 +0200 Subject: [PATCH] [netatmo] Avoid endless loop when Security claims event history (#17484) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Avoid looping in events Signed-off-by: Gaël L'hopital Signed-off-by: Ciprian Pascu --- .../internal/api/AuthenticationApi.java | 18 +++------- .../netatmo/internal/api/SecurityApi.java | 24 +++++++------- .../internal/api/data/NetatmoConstants.java | 1 + .../internal/handler/ApiBridgeHandler.java | 33 +++++++++---------- .../capability/SecurityCapability.java | 25 +++++++------- 5 files changed, 47 insertions(+), 54 deletions(-) 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 638c8c98824..a32e371c0d9 100644 --- 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 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 28825bf3ef6..8a28bb8696d 100644 --- 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 getEvents(@Nullable Object... params) throws NetatmoException { UriBuilder uriBuilder = getApiUriBuilder(SUB_PATH_GET_EVENTS, params); - BodyResponse 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 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 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 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 c2e325361dc..184571e0208 100644 --- 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 cec105c5671..5e9f8f1a830 100644 --- 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, RestManager> managers = new HashMap<>(); - private final Deque requestsTimestamps = new ArrayDeque<>(200); + private final Deque 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 f3dca854c85..255c7754f97 100644 --- 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 { @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 { protected List updateReadings(SecurityApi api) { List 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 getPersons() { return persons; }