From edaf9581c0e8a726e2ea6546205dbd098067f56a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20L=27hopital?= Date: Sun, 23 Apr 2023 11:14:10 +0200 Subject: [PATCH] [AirQuality] Enhance API error handling (#14602) * Enhancing API error handling --------- Signed-off-by: clinique --- .../internal/AirQualityException.java | 12 ++--- .../airquality/internal/api/ApiBridge.java | 23 +++++---- .../airquality/internal/api/Appreciation.java | 16 +++--- .../internal/api/dto/AirQualityData.java | 17 ++++--- .../internal/api/dto/AirQualityResponse.java | 51 ++++++++++++------- .../internal/api/dto/ResponseRoot.java | 34 +++++++++++++ .../discovery/AirQualityDiscoveryService.java | 4 +- .../handler/AirQualityBridgeHandler.java | 4 +- .../handler/AirQualityStationHandler.java | 34 +++++++------ 9 files changed, 123 insertions(+), 72 deletions(-) create mode 100644 bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/ResponseRoot.java diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/AirQualityException.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/AirQualityException.java index 480e0a1fd..03ce592d3 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/AirQualityException.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/AirQualityException.java @@ -23,24 +23,18 @@ import org.eclipse.jdt.annotation.Nullable; @NonNullByDefault public class AirQualityException extends Exception { private static final long serialVersionUID = -3398100220952729815L; - private int statusCode = -1; public AirQualityException(String message, Exception e) { super(message, e); } - public AirQualityException(String message) { - super(message); - } - - public int getStatusCode() { - return statusCode; + public AirQualityException(String message, Object... params) { + super(String.format(message, params)); } @Override public @Nullable String getMessage() { String message = super.getMessage(); - return message == null ? null - : String.format("Rest call failed: statusCode=%d, message=%s", statusCode, message); + return message == null ? null : String.format("Rest call failed: message=%s", message); } } diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/ApiBridge.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/ApiBridge.java index 884a453c1..fd5ae4b79 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/ApiBridge.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/ApiBridge.java @@ -13,13 +13,13 @@ package org.openhab.binding.airquality.internal.api; import java.io.IOException; +import java.util.Objects; import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.binding.airquality.internal.AirQualityException; import org.openhab.binding.airquality.internal.api.dto.AirQualityData; import org.openhab.binding.airquality.internal.api.dto.AirQualityResponse; -import org.openhab.binding.airquality.internal.api.dto.AirQualityResponse.ResponseStatus; import org.openhab.core.io.net.http.HttpUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,21 +66,24 @@ public class ApiBridge { * @return an air quality data object mapping the JSON response * @throws AirQualityException */ - public AirQualityData getData(int stationId, String location, int retryCounter) throws AirQualityException { + public AirQualityData getData(int stationId, String location) throws AirQualityException { String urlStr = buildRequestURL(apiKey, stationId, location); logger.debug("URL = {}", urlStr); try { String response = HttpUtil.executeUrl("GET", urlStr, null, null, null, REQUEST_TIMEOUT_MS); - logger.debug("aqiResponse = {}", response); - AirQualityResponse result = GSON.fromJson(response, AirQualityResponse.class); - if (result != null && result.getStatus() == ResponseStatus.OK) { - return result.getData(); - } else if (retryCounter == 0) { - logger.debug("Error in aqicn.org, retrying once"); - return getData(stationId, location, retryCounter + 1); + if (response != null) { + logger.debug("aqiResponse = {}", response); + AirQualityResponse result = GSON.fromJson(response, AirQualityResponse.class); + if (result != null) { + String error = result.getErrorMessage(); + if (error.isEmpty()) { + return Objects.requireNonNull(result.getData()); + } + throw new AirQualityException("Error raised : %s", error); + } } - throw new AirQualityException("Error in aqicn.org response: Missing data sub-object"); + throw new JsonSyntaxException("API response is null"); } catch (IOException | JsonSyntaxException e) { throw new AirQualityException("Communication error", e); } diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/Appreciation.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/Appreciation.java index 7a0d1c4db..2d9dcf01e 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/Appreciation.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/Appreciation.java @@ -24,17 +24,17 @@ import org.openhab.core.types.State; */ @NonNullByDefault public enum Appreciation { - GOOD(HSBType.fromRGB(0, 228, 0)), - MODERATE(HSBType.fromRGB(255, 255, 0)), - UNHEALTHY_FSG(HSBType.fromRGB(255, 126, 0)), - UNHEALTHY(HSBType.fromRGB(255, 0, 0)), - VERY_UNHEALTHY(HSBType.fromRGB(143, 63, 151)), - HAZARDOUS(HSBType.fromRGB(126, 0, 35)); + GOOD(0, 228, 0), + MODERATE(255, 255, 0), + UNHEALTHY_FSG(255, 126, 0), + UNHEALTHY(255, 0, 0), + VERY_UNHEALTHY(143, 63, 151), + HAZARDOUS(126, 0, 35); private HSBType color; - Appreciation(HSBType color) { - this.color = color; + Appreciation(int r, int g, int b) { + this.color = HSBType.fromRGB(r, g, b); } public State getColor() { diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityData.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityData.java index 82364c6e4..7b3f26f79 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityData.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityData.java @@ -14,9 +14,11 @@ package org.openhab.binding.airquality.internal.api.dto; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.airquality.internal.api.Pollutant; /** @@ -26,12 +28,13 @@ import org.openhab.binding.airquality.internal.api.Pollutant; * @author Kuba Wolanin - Initial contribution */ @NonNullByDefault -public class AirQualityData { +public class AirQualityData extends ResponseRoot { + private int aqi; private int idx; - private @NonNullByDefault({}) AirQualityTime time; - private @NonNullByDefault({}) AirQualityCity city; + private @Nullable AirQualityTime time; + private @Nullable AirQualityCity city; private List attributions = List.of(); private Map iaqi = Map.of(); private String dominentpol = ""; @@ -59,8 +62,8 @@ public class AirQualityData { * * @return {AirQualityJsonTime} */ - public AirQualityTime getTime() { - return time; + public Optional getTime() { + return Optional.ofNullable(time); } /** @@ -68,8 +71,8 @@ public class AirQualityData { * * @return {AirQualityJsonCity} */ - public AirQualityCity getCity() { - return city; + public Optional getCity() { + return Optional.ofNullable(city); } /** diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityResponse.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityResponse.java index e157a395b..5bfb8668c 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityResponse.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/AirQualityResponse.java @@ -13,8 +13,7 @@ package org.openhab.binding.airquality.internal.api.dto; import org.eclipse.jdt.annotation.NonNullByDefault; - -import com.google.gson.annotations.SerializedName; +import org.eclipse.jdt.annotation.Nullable; /** * The {@link AirQualityResponse} is the Java class used to map the JSON @@ -23,24 +22,40 @@ import com.google.gson.annotations.SerializedName; * @author Kuba Wolanin - Initial contribution */ @NonNullByDefault -public class AirQualityResponse { +public class AirQualityResponse extends ResponseRoot { - public static enum ResponseStatus { - NONE, - @SerializedName("error") - ERROR, - @SerializedName("ok") - OK; - } + private @Nullable AirQualityData data; - private ResponseStatus status = ResponseStatus.NONE; - private @NonNullByDefault({}) AirQualityData data; - - public ResponseStatus getStatus() { - return status; - } - - public AirQualityData getData() { + public @Nullable AirQualityData getData() { return data; } + + private ResponseStatus getStatus() { + AirQualityData localData = data; + return status == ResponseStatus.OK && localData != null && localData.status == ResponseStatus.OK + ? ResponseStatus.OK + : ResponseStatus.ERROR; + } + + public String getErrorMessage() { + if (getStatus() != ResponseStatus.OK) { + String localMsg = msg; + if (localMsg != null) { + return localMsg; + } else { + AirQualityData localData = data; + if (localData != null) { + localMsg = localData.msg; + if (localMsg != null) { + return localMsg; + } else { + return "Unknown error"; + } + } else { + return "No data provided"; + } + } + } + return ""; + } } diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/ResponseRoot.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/ResponseRoot.java new file mode 100644 index 000000000..09c665f7e --- /dev/null +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/api/dto/ResponseRoot.java @@ -0,0 +1,34 @@ +/** + * Copyright (c) 2010-2023 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.airquality.internal.api.dto; + +import org.eclipse.jdt.annotation.Nullable; + +import com.google.gson.annotations.SerializedName; + +/** + * The {@link ResponseRoot} is the common part of Air Quality API response objectss + * + * @author Gaël L'hopital - Initial contribution + */ +public class ResponseRoot { + public static enum ResponseStatus { + @SerializedName("error") + ERROR, + @SerializedName("ok") + OK; + } + + protected ResponseStatus status = ResponseStatus.OK; + protected @Nullable String msg; +} diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/discovery/AirQualityDiscoveryService.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/discovery/AirQualityDiscoveryService.java index ae334542d..0edc187dd 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/discovery/AirQualityDiscoveryService.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/discovery/AirQualityDiscoveryService.java @@ -84,14 +84,14 @@ public class AirQualityDiscoveryService extends AbstractDiscoveryService impleme PointType location = provider.getLocation(); AirQualityBridgeHandler bridge = this.bridgeHandler; if (location == null || bridge == null) { - logger.debug("LocationProvider.getLocation() is not set -> Will not provide any discovery results"); + logger.info("openHAB server location is not defined, will not provide any discovery results"); return; } createResults(location, bridge.getThing().getUID()); } } - public void createResults(PointType location, ThingUID bridgeUID) { + private void createResults(PointType location, ThingUID bridgeUID) { ThingUID localAirQualityThing = new ThingUID(THING_TYPE_STATION, bridgeUID, LOCAL); thingDiscovered(DiscoveryResultBuilder.create(localAirQualityThing).withLabel("Local Air Quality") .withProperty(LOCATION, String.format("%s,%s", location.getLatitude(), location.getLongitude())) diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityBridgeHandler.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityBridgeHandler.java index 90cd515f6..1ee591a2d 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityBridgeHandler.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityBridgeHandler.java @@ -13,7 +13,7 @@ package org.openhab.binding.airquality.internal.handler; import java.util.Collection; -import java.util.Collections; +import java.util.Set; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -67,7 +67,7 @@ public class AirQualityBridgeHandler extends BaseBridgeHandler { @Override public Collection> getServices() { - return Collections.singleton(AirQualityDiscoveryService.class); + return Set.of(AirQualityDiscoveryService.class); } public LocationProvider getLocationProvider() { diff --git a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityStationHandler.java b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityStationHandler.java index f95555465..83017784f 100644 --- a/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityStationHandler.java +++ b/bundles/org.openhab.binding.airquality/src/main/java/org/openhab/binding/airquality/internal/handler/AirQualityStationHandler.java @@ -110,13 +110,13 @@ public class AirQualityStationHandler extends BaseThingHandler { private void discoverAttributes() { getAirQualityData().ifPresent(data -> { // Update thing properties - Map properties = new HashMap<>(); - properties.put(ATTRIBUTIONS, data.getAttributions()); + Map properties = new HashMap<>(Map.of(ATTRIBUTIONS, data.getAttributions())); PointType serverLocation = locationProvider.getLocation(); if (serverLocation != null) { - PointType stationLocation = new PointType(data.getCity().getGeo()); - double distance = serverLocation.distanceFrom(stationLocation).doubleValue(); - properties.put(DISTANCE, new QuantityType<>(distance / 1000, KILO(SIUnits.METRE)).toString()); + data.getCity().ifPresent(city -> { + double distance = serverLocation.distanceFrom(new PointType(city.getGeo())).doubleValue(); + properties.put(DISTANCE, new QuantityType<>(distance / 1000, KILO(SIUnits.METRE)).toString()); + }); } // Search and remove missing pollutant channels @@ -132,8 +132,8 @@ public class AirQualityStationHandler extends BaseThingHandler { config.put(AirQualityConfiguration.STATION_ID, data.getStationId()); ThingBuilder thingBuilder = editThing(); - thingBuilder.withChannels(channels).withConfiguration(config).withProperties(properties) - .withLocation(data.getCity().getName()); + thingBuilder.withChannels(channels).withConfiguration(config).withProperties(properties); + data.getCity().map(city -> thingBuilder.withLocation(city.getName())); updateThing(thingBuilder.build()); }); } @@ -190,7 +190,7 @@ public class AirQualityStationHandler extends BaseThingHandler { if (apiBridge != null) { AirQualityConfiguration config = getConfigAs(AirQualityConfiguration.class); try { - result = apiBridge.getData(config.stationId, config.location, 0); + result = apiBridge.getData(config.stationId, config.location); updateStatus(ThingStatus.ONLINE); } catch (AirQualityException e) { updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR, e.getMessage()); @@ -253,24 +253,26 @@ public class AirQualityStationHandler extends BaseThingHandler { switch (channelId) { case TEMPERATURE: double temp = data.getIaqiValue("t"); - return temp != -1 ? new QuantityType<>(temp, SIUnits.CELSIUS) : UnDefType.UNDEF; + return temp != -1 ? new QuantityType<>(temp, SIUnits.CELSIUS) : UnDefType.NULL; case PRESSURE: double press = data.getIaqiValue("p"); - return press != -1 ? new QuantityType<>(press, HECTO(SIUnits.PASCAL)) : UnDefType.UNDEF; + return press != -1 ? new QuantityType<>(press, HECTO(SIUnits.PASCAL)) : UnDefType.NULL; case HUMIDITY: double hum = data.getIaqiValue("h"); - return hum != -1 ? new QuantityType<>(hum, Units.PERCENT) : UnDefType.UNDEF; + return hum != -1 ? new QuantityType<>(hum, Units.PERCENT) : UnDefType.NULL; case TIMESTAMP: - return new DateTimeType( - data.getTime().getObservationTime().withZoneSameLocal(timeZoneProvider.getTimeZone())); + return data.getTime() + .map(time -> (State) new DateTimeType( + time.getObservationTime().withZoneSameLocal(timeZoneProvider.getTimeZone()))) + .orElse(UnDefType.NULL); case DOMINENT: return new StringType(data.getDominentPol()); case DEW_POINT: double dp = data.getIaqiValue("dew"); - return dp != -1 ? new QuantityType<>(dp, SIUnits.CELSIUS) : UnDefType.UNDEF; + return dp != -1 ? new QuantityType<>(dp, SIUnits.CELSIUS) : UnDefType.NULL; case WIND_SPEED: double w = data.getIaqiValue("w"); - return w != -1 ? new QuantityType<>(w, Units.METRE_PER_SECOND) : UnDefType.UNDEF; + return w != -1 ? new QuantityType<>(w, Units.METRE_PER_SECOND) : UnDefType.NULL; default: if (groupId != null) { double idx = -1; @@ -283,7 +285,7 @@ public class AirQualityStationHandler extends BaseThingHandler { } return indexedValue(channelId, idx, pollutant); } - return UnDefType.UNDEF; + return UnDefType.NULL; } }