[opengarage] Move to jetty httpclient and other small improvements (#20980)

* Move to jetty httpclient

Signed-off-by: Paul Smedley <paul@smedley.id.au>

bundles/org.openhab.binding.opengarage/src/main/java/org/openhab/binding/opengarage/internal/OpenGarageHandler.java

@@ -21,6 +21,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jetty.client.HttpClient;
 import org.openhab.binding.opengarage.internal.api.ControllerVariables;
 import org.openhab.binding.opengarage.internal.api.Enums.OpenGarageCommand;
 import org.openhab.core.library.types.DecimalType;
@@ -57,20 +58,22 @@ public class OpenGarageHandler extends BaseThingHandler {
     private final Logger logger = LoggerFactory.getLogger(OpenGarageHandler.class);
 
     private @NonNullByDefault({}) OpenGarageWebTargets webTargets;
+    private final HttpClient httpClient;
 
     // reference to periodically scheduled poll task
     private Future<?> pollScheduledFuture = CompletableFuture.completedFuture(null);
 
     // reference to one-shot poll task which gets scheduled after a garage state change command
     private Future<?> pollScheduledFutureTransition = CompletableFuture.completedFuture(null);
-    private Instant lastTransition;
+    private volatile Instant lastTransition;
-    private String lastTransitionText;
+    private volatile String lastTransitionText;
 
     private OpenGarageConfiguration config = new OpenGarageConfiguration();
     private Gson gson = new Gson();
 
-    public OpenGarageHandler(Thing thing) {
+    public OpenGarageHandler(Thing thing, HttpClient httpClient) {
         super(thing);
+        this.httpClient = httpClient;
         this.lastTransition = Instant.MIN;
         this.lastTransitionText = "";
     }
@@ -81,10 +84,10 @@ public class OpenGarageHandler extends BaseThingHandler {
             logger.debug("Received command {} for thing '{}' on channel {}", command, thing.getUID().getAsString(),
                     channelUID.getId());
             Function<Boolean, Boolean> maybeInvert = getInverter(channelUID.getId());
+
             switch (channelUID.getId()) {
-                case OpenGarageBindingConstants.CHANNEL_OG_STATUS:
+                case OpenGarageBindingConstants.CHANNEL_OG_STATUS, OpenGarageBindingConstants.CHANNEL_OG_STATUS_SWITCH,
-                case OpenGarageBindingConstants.CHANNEL_OG_STATUS_SWITCH:
+                        OpenGarageBindingConstants.CHANNEL_OG_STATUS_ROLLERSHUTTER -> {
-                case OpenGarageBindingConstants.CHANNEL_OG_STATUS_ROLLERSHUTTER:
                     if (command.equals(StopMoveType.STOP) || command.equals(StopMoveType.MOVE)) {
                         changeStatus(OpenGarageCommand.CLICK);
                     } else {
@@ -99,8 +102,9 @@ public class OpenGarageHandler extends BaseThingHandler {
                         this.pollScheduledFutureTransition = this.scheduler.schedule(this::poll,
                                 this.config.doorTransitionTimeSeconds, TimeUnit.SECONDS);
                     }
-                    break;
+                }
-                default:
+                default -> {
+                }
             }
         } catch (OpenGarageCommunicationException ex) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, ex.getMessage());
@@ -116,7 +120,8 @@ public class OpenGarageHandler extends BaseThingHandler {
         } else {
             updateStatus(ThingStatus.UNKNOWN);
             int requestTimeout = Math.max(OpenGarageWebTargets.DEFAULT_TIMEOUT_MS, config.refresh * 1000);
-            webTargets = new OpenGarageWebTargets(config.hostname, config.port, config.password, requestTimeout);
+            webTargets = new OpenGarageWebTargets(httpClient, config.hostname, config.port, config.password,
+                    requestTimeout);
             this.pollScheduledFuture = this.scheduler.scheduleWithFixedDelay(this::poll, 1, config.refresh,
                     TimeUnit.SECONDS);
         }
@@ -172,24 +177,15 @@ public class OpenGarageHandler extends BaseThingHandler {
                 }
 
                 switch (controllerVariables.vehicle) {
-                    case 0:
+                    case 0 -> updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE,
-                        updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE,
+                            new StringType("No vehicle detected"));
-                                new StringType("No vehicle detected"));
+                    case 1 ->
-                        break;
-                    case 1:
                         updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE, new StringType("Vehicle detected"));
-                        break;
+                    case 2 -> updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE,
-                    case 2:
+                            new StringType("Vehicle status unknown"));
-                        updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE,
+                    case 3 -> updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE,
-                                new StringType("Vehicle status unknown"));
+                            new StringType("Vehicle status not available"));
-                        break;
+                    default -> logger.debug("Received unknown vehicle value: {}", controllerVariables.vehicle);
-                    case 3:
-                        updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE,
-                                new StringType("Vehicle status not available"));
-                        break;
-
-                    default:
-                        logger.debug("Received unknown vehicle value: {}", controllerVariables.vehicle);
                 }
                 updateState(OpenGarageBindingConstants.CHANNEL_OG_VEHICLE_STATUS,
                         new DecimalType(controllerVariables.vehicle));

bundles/org.openhab.binding.opengarage/src/main/java/org/openhab/binding/opengarage/internal/OpenGarageHandlerFactory.java

@@ -14,12 +14,15 @@ package org.openhab.binding.opengarage.internal;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.core.io.net.http.HttpClientFactory;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingTypeUID;
 import org.openhab.core.thing.binding.BaseThingHandlerFactory;
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerFactory;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
 
 /**
  * The {@link OpenGarageHandlerFactory} is responsible for creating things and thing
@@ -31,6 +34,13 @@ import org.osgi.service.component.annotations.Component;
 @NonNullByDefault
 public class OpenGarageHandlerFactory extends BaseThingHandlerFactory {
 
+    private final HttpClientFactory httpClientFactory;
+
+    @Activate
+    public OpenGarageHandlerFactory(@Reference HttpClientFactory httpClientFactory) {
+        this.httpClientFactory = httpClientFactory;
+    }
+
     @Override
     public boolean supportsThingType(ThingTypeUID thingTypeUID) {
         return OpenGarageBindingConstants.SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID);
@@ -41,7 +51,7 @@ public class OpenGarageHandlerFactory extends BaseThingHandlerFactory {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (thingTypeUID.equals(OpenGarageBindingConstants.OPENGARAGE_THING)) {
-            return new OpenGarageHandler(thing);
+            return new OpenGarageHandler(thing, httpClientFactory.getCommonHttpClient());
         }
 
         return null;

bundles/org.openhab.binding.opengarage/src/main/java/org/openhab/binding/opengarage/internal/OpenGarageWebTargets.java

@@ -12,11 +12,16 @@
  */
 package org.openhab.binding.opengarage.internal;
 
-import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.http.HttpMethod;
+import org.eclipse.jetty.http.HttpStatus;
 import org.openhab.binding.opengarage.internal.api.Enums.OpenGarageCommand;
-import org.openhab.core.io.net.http.HttpUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -34,8 +39,10 @@ public class OpenGarageWebTargets {
     private String changeControllerVariablesUri;
     private final Logger logger = LoggerFactory.getLogger(OpenGarageWebTargets.class);
     private int timeoutMs;
+    private final HttpClient httpClient;
 
-    public OpenGarageWebTargets(String ipAddress, long port, String password, int timeoutMs) {
+    public OpenGarageWebTargets(HttpClient httpClient, String ipAddress, long port, String password, int timeoutMs) {
+        this.httpClient = httpClient;
         String baseUri = "http://" + ipAddress + ":" + port + "/";
         this.timeoutMs = timeoutMs;
         this.getControllerVariablesUri = baseUri + "jc";
@@ -48,21 +55,14 @@ public class OpenGarageWebTargets {
 
     public void setControllerVariables(OpenGarageCommand request) throws OpenGarageCommunicationException {
         logger.debug("Received request: {}", request);
-        String queryParams = null;
+
-        switch (request) {
+        String queryParams = switch (request) {
-            case OPEN:
+            case OPEN -> "&open=1";
-                queryParams = "&open=1";
+            case CLOSE -> "&close=1";
-                break;
+            case CLICK -> "&click=1";
-            case CLOSE:
+        };
-                queryParams = "&close=1";
+
-                break;
+        invoke(changeControllerVariablesUri, queryParams);
-            case CLICK:
-                queryParams = "&click=1";
-                break;
-        }
-        if (queryParams != null) {
-            invoke(changeControllerVariablesUri, queryParams);
-        }
     }
 
     private String invoke(String uri) throws OpenGarageCommunicationException {
@@ -73,21 +73,30 @@ public class OpenGarageWebTargets {
         String uriWithParams = uri + params;
         logger.debug("Calling url: {}", uriWithParams);
         String response;
+
         synchronized (this) {
             try {
-                response = HttpUtil.executeUrl("GET", uriWithParams, this.timeoutMs);
+                ContentResponse contentResponse = httpClient.newRequest(uriWithParams).method(HttpMethod.GET)
-            } catch (IOException ex) {
+                        .timeout(this.timeoutMs, TimeUnit.MILLISECONDS).send();
+
+                if (contentResponse.getStatus() != HttpStatus.OK_200) {
+                    throw new OpenGarageCommunicationException(
+                            String.format("OpenGarage controller returned HTTP %d while invoking %s",
+                                    contentResponse.getStatus(), uriWithParams));
+                }
+
+                response = contentResponse.getContentAsString();
+
+            } catch (InterruptedException | ExecutionException | TimeoutException ex) {
                 logger.debug("{}", ex.getLocalizedMessage(), ex);
-                // Response will also be set to null if parsing in executeUrl fails so we use null here to make the
+                if (ex instanceof InterruptedException) {
-                // error check below consistent.
+                    Thread.currentThread().interrupt();
-                response = null;
+                }
+                throw new OpenGarageCommunicationException(
+                        String.format("OpenGarage controller returned error while invoking %s", uriWithParams), ex);
             }
         }
 
-        if (response == null) {
-            throw new OpenGarageCommunicationException(
-                    String.format("OpenGaragecontroller returned error while invoking %s", uriWithParams));
-        }
         return response;
     }
 }