From 15f69b901118a468ecbad4194cb0e02f235d8736 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Sun, 4 Apr 2021 18:58:16 +0200 Subject: [PATCH] [pushover] Added exception handling and synchronized (#10437) Signed-off-by: Christoph Weitkamp --- .../org.openhab.binding.pushover/README.md | 2 ++ .../connection/PushoverAPIConnection.java | 14 ++++------ .../factory/PushoverHandlerFactory.java | 3 +- .../handler/PushoverAccountHandler.java | 28 +++++++++++++++++-- .../resources/OH-INF/i18n/pushover.properties | 1 + .../OH-INF/i18n/pushover_de.properties | 1 + 6 files changed, 36 insertions(+), 13 deletions(-) diff --git a/bundles/org.openhab.binding.pushover/README.md b/bundles/org.openhab.binding.pushover/README.md index f19e70bdfe..1719689689 100644 --- a/bundles/org.openhab.binding.pushover/README.md +++ b/bundles/org.openhab.binding.pushover/README.md @@ -31,6 +31,8 @@ Currently the binding does not support any Channels. ## Thing Actions All actions return a `Boolean` value to indicate if the message was sent successfully or not. +If the communication to Pushover servers fails the binding does not try to send the message again. +One has to take care of that on its own if it is important. The parameter `message` is **mandatory**, the `title` parameter defaults to whatever value you defined in the `title` related configuration parameter. Parameters declared as `@Nullable` are not optional. One has to pass a `null` value if it should be skipped or the default value for it should be used. diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java index a7c7fef07e..8e8f6e820c 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java @@ -37,7 +37,7 @@ import org.openhab.core.cache.ExpiringCacheMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParser; @@ -130,7 +130,7 @@ public class PushoverAPIConnection { return executeRequest(HttpMethod.POST, url, body); } - private String executeRequest(HttpMethod httpMethod, String url, @Nullable ContentProvider body) + private synchronized String executeRequest(HttpMethod httpMethod, String url, @Nullable ContentProvider body) throws PushoverCommunicationException, PushoverConfigurationException { logger.trace("Pushover request: {} - URL = '{}'", httpMethod, url); try { @@ -169,13 +169,11 @@ public class PushoverAPIConnection { private String getMessageError(String content) { final JsonObject json = JsonParser.parseString(content).getAsJsonObject(); - if (json.has("errors")) { - final JsonArray errors = json.get("errors").getAsJsonArray(); - if (errors != null) { - return errors.toString(); - } + final JsonElement errorsElement = json.get("errors"); + if (errorsElement != null && errorsElement.isJsonArray()) { + return errorsElement.getAsJsonArray().toString(); } - return "Unknown error occured."; + return "@text/offline.conf-error-unknown"; } private boolean getMessageStatus(String content) { diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/factory/PushoverHandlerFactory.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/factory/PushoverHandlerFactory.java index a08d95242f..2ce4f3cfa0 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/factory/PushoverHandlerFactory.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/factory/PushoverHandlerFactory.java @@ -31,8 +31,7 @@ import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; /** - * The {@link PushoverHandlerFactory} is responsible for creating things and thing - * handlers. + * The {@link PushoverHandlerFactory} is responsible for creating things and thing handlers. * * @author Christoph Weitkamp - Initial contribution */ diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java index ae71790763..9840bc23ae 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java @@ -134,7 +134,14 @@ public class PushoverAccountHandler extends BaseThingHandler { public boolean sendMessage(PushoverMessageBuilder messageBuilder) { if (connection != null) { - return connection.sendMessage(messageBuilder); + try { + return connection.sendMessage(messageBuilder); + } catch (PushoverCommunicationException e) { + // do nothing, causing exception is already logged + } catch (PushoverConfigurationException e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage()); + } + return false; } else { throw new IllegalArgumentException("PushoverAPIConnection is null!"); } @@ -142,7 +149,14 @@ public class PushoverAccountHandler extends BaseThingHandler { public String sendPriorityMessage(PushoverMessageBuilder messageBuilder) { if (connection != null) { - return connection.sendPriorityMessage(messageBuilder); + try { + return connection.sendPriorityMessage(messageBuilder); + } catch (PushoverCommunicationException e) { + // do nothing, causing exception is already logged + } catch (PushoverConfigurationException e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage()); + } + return ""; } else { throw new IllegalArgumentException("PushoverAPIConnection is null!"); } @@ -150,12 +164,20 @@ public class PushoverAccountHandler extends BaseThingHandler { public boolean cancelPriorityMessage(String receipt) { if (connection != null) { - return connection.cancelPriorityMessage(receipt); + try { + return connection.cancelPriorityMessage(receipt); + } catch (PushoverCommunicationException e) { + // do nothing, causing exception is already logged + } catch (PushoverConfigurationException e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage()); + } + return false; } else { throw new IllegalArgumentException("PushoverAPIConnection is null!"); } } + @SuppressWarnings("null") private void asyncValidateUser() { try { connection.validateUser(); diff --git a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover.properties b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover.properties index 5e1bad241d..85e66740a8 100644 --- a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover.properties +++ b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover.properties @@ -1,6 +1,7 @@ # user defined messages offline.conf-error-missing-apikey = The 'apikey' parameter must be configured. offline.conf-error-missing-user = The 'user' parameter must be configured. +offline.conf-error-unknown = An unknown error occurred. # actions sendMessageActionLabel = send a plain text message diff --git a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties index f340464ea0..9f094e31e8 100644 --- a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties +++ b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties @@ -24,6 +24,7 @@ thing-type.config.pushover.pushover-account.expire.description = Dieser Paramete # user defined messages offline.conf-error-missing-apikey = Der Parameter 'apikey' muss konfiguriert werden. offline.conf-error-missing-user = Der Parameter 'user' muss konfiguriert werden. +offline.conf-error-unknown = Ein unbekannter Fehler ist aufgetreten. # actions sendMessageActionLabel = eine Textnachricht senden -- 2.47.3