From 7ae9e39c7226aa0282a13058c4b81b1b7fc09502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20L=27hopital?= Date: Sat, 7 Mar 2026 09:07:09 +0100 Subject: [PATCH] [netatmo] Avoid rushing server APIs (#20314) * Stabilize activity peaks Signed-off-by: clinique Signed-off-by: gael@lhopital.org --- .../netatmo/internal/api/dto/HomeEvent.java | 18 ++-- .../deserialization/NADeserializer.java | 5 +- .../internal/handler/ApiBridgeHandler.java | 85 ++++++++++++++----- .../internal/handler/CommonInterface.java | 17 ++-- .../internal/api/dto/HomeEventTest.java | 75 ++++++++++++++++ 5 files changed, 166 insertions(+), 34 deletions(-) create mode 100644 bundles/org.openhab.binding.netatmo/src/test/java/org/openhab/binding/netatmo/internal/api/dto/HomeEventTest.java diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/dto/HomeEvent.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/dto/HomeEvent.java index 9cfa3e6b4c..6f2d415185 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/dto/HomeEvent.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/dto/HomeEvent.java @@ -12,6 +12,7 @@ */ package org.openhab.binding.netatmo.internal.api.dto; +import java.time.Instant; import java.time.ZonedDateTime; import java.util.List; import java.util.Optional; @@ -37,15 +38,15 @@ public class HomeEvent extends Event { public class NAEventsDataResponse extends ApiResponse> { } - private record Snapshot(@Nullable String url, @Nullable ZonedDateTime expiresAt) { + private record Snapshot(@Nullable String url, @Nullable Instant expiresAt) { public @Nullable String url() { - ZonedDateTime expires = expiresAt; - // If no expiration data provided, lets consider it is available - if (expires == null) { + Instant expires = expiresAt; + // If no expiration data provided or later than now: it is available + if (expires == null || expires.isAfter(Instant.now())) { return url; } - // If the snapshot is expired we consider it as not available, so do not provide the url - return expires.isAfter(ZonedDateTime.now().withZoneSameInstant(expires.getZone())) ? url : null; + // Consider it as not available, so do not provide the url + return null; } } @@ -119,4 +120,9 @@ public class HomeEvent extends Event { private @Nullable String internalGetUrl(@Nullable Snapshot image) { return image == null ? null : image.url(); } + + @Override + public boolean isIgnoredForThingUpdate() { + return true; + } } diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/deserialization/NADeserializer.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/deserialization/NADeserializer.java index e29835966b..9d324ae038 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/deserialization/NADeserializer.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/deserialization/NADeserializer.java @@ -54,7 +54,10 @@ public class NADeserializer { (JsonDeserializer) (json, type, context) -> context.deserialize(json, json.getAsJsonObject().has("therm_mode") ? HomeData.Energy.class : HomeData.Security.class)) - .registerTypeAdapter(ZonedDateTime.class, (JsonDeserializer) (json, type, context) -> { + .registerTypeAdapter(Instant.class, (JsonDeserializer) (json, type, context) -> { + long netatmoTS = json.getAsJsonPrimitive().getAsLong(); + return Instant.ofEpochSecond(netatmoTS); + }).registerTypeAdapter(ZonedDateTime.class, (JsonDeserializer) (json, type, context) -> { long netatmoTS = json.getAsJsonPrimitive().getAsLong(); Instant i = Instant.ofEpochSecond(netatmoTS); return ZonedDateTime.ofInstant(i, timeZoneProvider.getTimeZone()); 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 8a158008e7..0bbc5e97ba 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 @@ -104,11 +104,14 @@ import com.google.gson.GsonBuilder; public class ApiBridgeHandler extends BaseBridgeHandler { private static final int TIMEOUT_S = 20; private static final int API_LIMIT_INTERVAL_S = 3600; + private static final int MAX_REQUESTS_PER_SECOND = 5; + private static final long ONE_SECOND_IN_NANOS = TimeUnit.SECONDS.toNanos(1); 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 requestsWindowInNanos = new ArrayDeque<>(MAX_REQUESTS_PER_SECOND); private final BindingConfiguration bindingConf; private final HttpClient httpClient; private final OAuthFactory oAuthFactory; @@ -328,28 +331,13 @@ public class ApiBridgeHandler extends BaseBridgeHandler { } connectApi.getAuthorization().ifPresent(auth -> request.header(HttpHeader.AUTHORIZATION, auth)); - if (payload != null && contentType != null - && (HttpMethod.POST.equals(method) || HttpMethod.PUT.equals(method))) { - InputStream stream = new ByteArrayInputStream(payload.getBytes(StandardCharsets.UTF_8)); - try (InputStreamContentProvider inputStreamContentProvider = new InputStreamContentProvider(stream)) { - request.content(inputStreamContentProvider, contentType); - request.header(HttpHeader.ACCEPT, MediaType.APPLICATION_JSON); - } - logger.trace(" -with payload: {} ", payload); - } - - if (isLinked(requestCountChannelUID)) { - 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())); - } + handlePayload(method, payload, contentType, request); + handleRequestCounter(); logger.trace(" -with headers: {} ", String.join(", ", request.getHeaders().stream().map(HttpField::toString).toList())); + + throttleApiRequestRate(); ContentResponse response = request.send(); Code statusCode = HttpStatus.getCode(response.getStatus()); @@ -377,8 +365,13 @@ public class ApiBridgeHandler extends BaseBridgeHandler { String delayStr = response.getHeaders().get(HttpHeader.RETRY_AFTER); int delay = delayStr != null ? Integer.valueOf(delayStr) : Integer.MAX_VALUE; if (exception.getStatusCode() == ServiceError.CONCURRENCY_LIMIT_TIMED_OUT) { - delay = Math.min(delay, TIMEOUT_S); - message = "@text/concurrency-limit-timed-out [ \"%d\" ]"; + if (retryCount > 0) { + logger.debug("Concurrency limited section, retry counter: {}", retryCount); + return executeUri(uri, method, clazz, payload, contentType, retryCount - 1); + } else { + delay = Math.min(delay, TIMEOUT_S); + message = "@text/concurrency-limit-timed-out [ \"%d\" ]"; + } } else { // ServiceError.MAXIMUM_USAGE_REACHED delay = Math.min(delay, API_LIMIT_INTERVAL_S); message = "@text/maximum-usage-reached [ \"%d\" ]"; @@ -400,6 +393,56 @@ public class ApiBridgeHandler extends BaseBridgeHandler { } } + private void handleRequestCounter() { + if (!isLinked(requestCountChannelUID)) { + return; + } + + Instant now = Instant.now(); + requestsTimestamps.addLast(now); + Instant oneHourAgo = now.minus(1, ChronoUnit.HOURS); + requestsTimestamps.removeIf(t -> t.isBefore(oneHourAgo)); + updateState(requestCountChannelUID, new DecimalType(requestsTimestamps.size())); + } + + private void handlePayload(HttpMethod method, @Nullable String payload, @Nullable String contentType, + Request request) { + if (payload == null || contentType == null + || !(HttpMethod.POST.equals(method) || HttpMethod.PUT.equals(method))) { + return; + } + + InputStream stream = new ByteArrayInputStream(payload.getBytes(StandardCharsets.UTF_8)); + try (InputStreamContentProvider inputStreamContentProvider = new InputStreamContentProvider(stream)) { + request.content(inputStreamContentProvider, contentType); + request.header(HttpHeader.ACCEPT, MediaType.APPLICATION_JSON); + } + logger.trace(" -with payload: {} ", payload); + } + + private void throttleApiRequestRate() throws InterruptedException { + while (true) { + long now = System.nanoTime(); + long threshold = now - ONE_SECOND_IN_NANOS; + + while (!requestsWindowInNanos.isEmpty() && requestsWindowInNanos.getFirst() <= threshold) { + requestsWindowInNanos.removeFirst(); + } + + if (requestsWindowInNanos.size() < MAX_REQUESTS_PER_SECOND) { + requestsWindowInNanos.addLast(now); + return; + } + + long waitNanos = requestsWindowInNanos.getFirst() + ONE_SECOND_IN_NANOS - now; + if (waitNanos > 0) { + logger.trace("Rate limit reached ({} req/s), waiting {} ms", MAX_REQUESTS_PER_SECOND, + TimeUnit.NANOSECONDS.toMillis(waitNanos)); + TimeUnit.NANOSECONDS.sleep(waitNanos); + } + } + } + public void identifyAllModulesAndApplyAction(BiFunction> action) { ThingUID accountUID = getThing().getUID(); try { diff --git a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/CommonInterface.java b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/CommonInterface.java index c41bf0f7e9..fe1b094ab5 100644 --- a/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/CommonInterface.java +++ b/bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/CommonInterface.java @@ -184,17 +184,22 @@ public interface CommonInterface { return; } } + String finalReason = null; for (Capability cap : getCapabilities().values()) { - String thingStatusReason = cap.setNewData(newData); - if (thingStatusReason != null) { - finalReason = thingStatusReason; + String statusReason = cap.setNewData(newData); + if (statusReason != null) { + finalReason = statusReason; } } + + if (newData.isIgnoredForThingUpdate()) { + return; + } + // Prevent turning ONLINE myself if in the meantime something turned account OFFLINE - ApiBridgeHandler accountHandler = getAccountHandler(); - if (accountHandler != null && accountHandler.isConnected() && !newData.isIgnoredForThingUpdate()) { - setThingStatus(finalReason == null ? ThingStatus.ONLINE : ThingStatus.OFFLINE, ThingStatusDetail.NONE, + if (getAccountHandler() instanceof ApiBridgeHandler accountHandler && accountHandler.isConnected()) { + setThingStatus(finalReason != null ? ThingStatus.OFFLINE : ThingStatus.ONLINE, ThingStatusDetail.NONE, finalReason); } } diff --git a/bundles/org.openhab.binding.netatmo/src/test/java/org/openhab/binding/netatmo/internal/api/dto/HomeEventTest.java b/bundles/org.openhab.binding.netatmo/src/test/java/org/openhab/binding/netatmo/internal/api/dto/HomeEventTest.java new file mode 100644 index 0000000000..3b1e295298 --- /dev/null +++ b/bundles/org.openhab.binding.netatmo/src/test/java/org/openhab/binding/netatmo/internal/api/dto/HomeEventTest.java @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2010-2026 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.binding.netatmo.internal.api.dto; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.time.ZoneId; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.openhab.binding.netatmo.internal.api.NetatmoException; +import org.openhab.binding.netatmo.internal.deserialization.NADeserializer; +import org.openhab.core.i18n.TimeZoneProvider; + +/** + * @author Gaël L'hopital - Initial contribution + */ +public class HomeEventTest { + private static final long PAST_EPOCH_SECONDS = 946684800L; // 2000-01-01T00:00:00Z + private static final long FUTURE_EPOCH_SECONDS = 4102444800L; // 2100-01-01T00:00:00Z + + private static NADeserializer gson; + + @BeforeAll + public static void init() { + TimeZoneProvider timeZoneProvider = mock(TimeZoneProvider.class); + when(timeZoneProvider.getTimeZone()).thenReturn(ZoneId.systemDefault()); + gson = new NADeserializer(timeZoneProvider); + } + + @Test + public void testSnapshotUrlDeserializationWithFutureExpiration() throws NetatmoException { + String event = """ + {\ + "snapshot": {\ + "url": "https://example.netatmo/snapshot.jpg",\ + "expires_at": %d\ + }\ + }\ + """.formatted(FUTURE_EPOCH_SECONDS); + + HomeEvent object = gson.deserialize(HomeEvent.class, event); + + assertEquals("https://example.netatmo/snapshot.jpg", object.getSnapshotUrl()); + } + + @Test + public void testVignetteUrlDeserializationWithPastExpiration() throws NetatmoException { + String event = """ + {\ + "vignette": {\ + "url": "https://example.netatmo/vignette.jpg",\ + "expires_at": %d\ + }\ + }\ + """.formatted(PAST_EPOCH_SECONDS); + + HomeEvent object = gson.deserialize(HomeEvent.class, event); + + assertNull(object.getVignetteUrl()); + } +}