[neeo] fix ClassNotFoundException and switch to constructor injection

bundles/org.openhab.binding.neeo/src/main/java/org/openhab/binding/neeo/internal/NeeoBrainApi.java

@@ -17,6 +17,8 @@
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicReference;
 
+import javax.ws.rs.client.ClientBuilder;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jetty.http.HttpStatus;
 import org.openhab.binding.neeo.internal.models.ExecuteResult;
@@ -44,7 +46,10 @@ public class NeeoBrainApi implements AutoCloseable {
     private final Gson gson = NeeoUtil.getGson();
 
     /** The {@link HttpRequest} used for making requests */
-    private final AtomicReference<HttpRequest> request = new AtomicReference<>(new HttpRequest());
+    private final AtomicReference<HttpRequest> request;
+
+    /** The {@link ClientBuilder} to use */
+    private final ClientBuilder clientBuilder;
 
     /** The IP address of the neeo brain */
     private final NeeoUrlBuilder urlBuilder;
@@ -54,11 +59,14 @@ public class NeeoBrainApi implements AutoCloseable {
      *
      * @param ipAddress the non-empty ip address
      */
-    public NeeoBrainApi(String ipAddress) {
+    public NeeoBrainApi(String ipAddress, ClientBuilder clientBuilder) {
         NeeoUtil.requireNotEmpty(ipAddress, "ipAddress cannot be empty");
 
         this.urlBuilder = new NeeoUrlBuilder(
                 NeeoConstants.PROTOCOL + ipAddress + ":" + NeeoConstants.DEFAULT_BRAIN_PORT);
+        this.clientBuilder = clientBuilder;
+
+        request = new AtomicReference<>(new HttpRequest(clientBuilder));
     }
 
     /**
@@ -232,7 +240,7 @@ public void deregisterForwardActions() throws IOException {
 
     @Override
     public void close() throws Exception {
-        NeeoUtil.close(request.getAndSet(new HttpRequest()));
+        NeeoUtil.close(request.getAndSet(new HttpRequest(clientBuilder)));
     }
 
     /**

bundles/org.openhab.binding.neeo/src/main/java/org/openhab/binding/neeo/internal/handler/NeeoBrainHandler.java

@@ -29,6 +29,7 @@
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.servlet.ServletException;
+import javax.ws.rs.client.ClientBuilder;
 
 import org.apache.commons.lang.StringUtils;
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -73,6 +74,9 @@ public class NeeoBrainHandler extends BaseBridgeHandler {
     /** The {@link NetworkAddressService} to use */
     private final NetworkAddressService networkAddressService;
 
+    /** The {@link ClientBuilder} to use */
+    private final ClientBuilder clientBuilder;
+
     /** GSON implementation - only used to deserialize {@link NeeoAction} */
     private final Gson gson = new Gson();
 
@@ -112,7 +116,7 @@ public class NeeoBrainHandler extends BaseBridgeHandler {
      * @param networkAddressService the non-null {@link NetworkAddressService}
      */
     NeeoBrainHandler(Bridge bridge, int servicePort, HttpService httpService,
-            NetworkAddressService networkAddressService) {
+            NetworkAddressService networkAddressService, ClientBuilder clientBuilder) {
         super(bridge);
 
         Objects.requireNonNull(bridge, "bridge cannot be null");
@@ -122,6 +126,7 @@ public class NeeoBrainHandler extends BaseBridgeHandler {
         this.servicePort = servicePort;
         this.httpService = httpService;
         this.networkAddressService = networkAddressService;
+        this.clientBuilder = clientBuilder;
     }
 
     /**
@@ -164,7 +169,7 @@ private void initializeTask() {
                         "Brain IP Address must be specified");
                 return;
             }
-            final NeeoBrainApi api = new NeeoBrainApi(ipAddress);
+            final NeeoBrainApi api = new NeeoBrainApi(ipAddress, clientBuilder);
             final NeeoBrain brain = api.getBrain();
             final String brainId = getNeeoBrainId();
 
@@ -184,22 +189,18 @@ private void initializeTask() {
             if (config.isEnableForwardActions()) {
                 NeeoUtil.checkInterrupt();
 
-                forwardActionServlet = new NeeoForwardActionsServlet(scheduler,
-                        new NeeoForwardActionsServlet.Callback() {
-                            @Override
-                            public void post(String json) {
-                                triggerChannel(NeeoConstants.CHANNEL_BRAIN_FOWARDACTIONS, json);
-
-                                final NeeoAction action = Objects.requireNonNull(gson.fromJson(json, NeeoAction.class));
-
-                                for (final Thing child : getThing().getThings()) {
-                                    final ThingHandler th = child.getHandler();
-                                    if (th instanceof NeeoRoomHandler) {
-                                        ((NeeoRoomHandler) th).processAction(action);
-                                    }
-                                }
-                            }
-                        }, config.getForwardChain());
+                forwardActionServlet = new NeeoForwardActionsServlet(scheduler, json -> {
+                    triggerChannel(NeeoConstants.CHANNEL_BRAIN_FOWARDACTIONS, json);
+
+                    final NeeoAction action = Objects.requireNonNull(gson.fromJson(json, NeeoAction.class));
+
+                    for (final Thing child : getThing().getThings()) {
+                        final ThingHandler th = child.getHandler();
+                        if (th instanceof NeeoRoomHandler) {
+                            ((NeeoRoomHandler) th).processAction(action);
+                        }
+                    }
+                }, config.getForwardChain(), clientBuilder);
 
                 NeeoUtil.checkInterrupt();
                 try {

bundles/org.openhab.binding.neeo/src/main/java/org/openhab/binding/neeo/internal/handler/NeeoForwardActionsServlet.java

@@ -16,13 +16,12 @@
 import java.util.Objects;
 import java.util.concurrent.ScheduledExecutorService;
 
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.client.ClientBuilder;
 
 import org.apache.commons.io.IOUtils;
-import org.apache.commons.lang.StringUtils;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.jetty.http.HttpStatus;
@@ -52,6 +51,9 @@ public class NeeoForwardActionsServlet extends HttpServlet {
     @Nullable
     private final String forwardChain;
 
+    /** The {@link ClientBuilder} to use */
+    private final ClientBuilder clientBuilder;
+
     /** The scheduler to use to schedule recipe execution */
     private final ScheduledExecutorService scheduler;
 
@@ -62,7 +64,8 @@ public class NeeoForwardActionsServlet extends HttpServlet {
      * @param callback a non-null {@link Callback}
      * @param forwardChain a possibly null, possibly empty forwarding chain
      */
-    NeeoForwardActionsServlet(ScheduledExecutorService scheduler, Callback callback, @Nullable String forwardChain) {
+    NeeoForwardActionsServlet(ScheduledExecutorService scheduler, Callback callback, @Nullable String forwardChain,
+            ClientBuilder clientBuilder) {
         super();
 
         Objects.requireNonNull(scheduler, "scheduler cannot be null");
@@ -71,6 +74,7 @@ public class NeeoForwardActionsServlet extends HttpServlet {
         this.scheduler = scheduler;
         this.callback = callback;
         this.forwardChain = forwardChain;
+        this.clientBuilder = clientBuilder;
     }
 
     /**
@@ -81,22 +85,23 @@ public class NeeoForwardActionsServlet extends HttpServlet {
      * @param resp the non-null response
      */
     @Override
-    protected void doPost(@Nullable HttpServletRequest req, @Nullable HttpServletResponse resp)
-            throws ServletException, IOException {
-        Objects.requireNonNull(req, "req cannot be null");
-        Objects.requireNonNull(resp, "resp cannot be null");
+    protected void doPost(@Nullable HttpServletRequest req, @Nullable HttpServletResponse resp) throws IOException {
+        if (req == null || resp == null) {
+            logger.warn("doPost called with req={}, resp={}, non-null required.", req, resp);
+            return;
+        }
 
         final String json = IOUtils.toString(req.getReader());
         logger.debug("handleForwardActions {}", json);
 
         callback.post(json);
 
         final String fc = forwardChain;
-        if (fc != null && StringUtils.isNotEmpty(fc)) {
+        if (fc != null && !fc.isEmpty()) {
             scheduler.execute(() -> {
-                try (final HttpRequest request = new HttpRequest()) {
+                try (final HttpRequest request = new HttpRequest(clientBuilder)) {
                     for (final String forwardUrl : fc.split(",")) {
-                        if (StringUtils.isNotEmpty(forwardUrl)) {
+                        if (forwardUrl != null && !forwardUrl.isEmpty()) {
                             final HttpResponse httpResponse = request.sendPostJsonCommand(forwardUrl, json);
                             if (httpResponse.getHttpCode() != HttpStatus.OK_200) {
                                 logger.debug("Cannot forward event {} to {}: {}", json, forwardUrl,

bundles/org.openhab.binding.neeo/src/main/java/org/openhab/binding/neeo/internal/handler/NeeoHandlerFactory.java

@@ -17,6 +17,8 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
+import javax.ws.rs.client.ClientBuilder;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.neeo.internal.NeeoConstants;
@@ -33,6 +35,7 @@
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerFactory;
 import org.osgi.framework.ServiceRegistration;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.http.HttpService;
@@ -48,54 +51,23 @@
 public class NeeoHandlerFactory extends BaseThingHandlerFactory {
 
     /** The {@link HttpService} used to register callbacks */
-    @NonNullByDefault({})
-    private HttpService httpService;
+    private final HttpService httpService;
 
     /** The {@link NetworkAddressService} used for ip lookup */
-    @NonNullByDefault({})
-    private NetworkAddressService networkAddressService;
+    private final NetworkAddressService networkAddressService;
+
+    /** The {@link ClientBuilder} used in HttpRequest */
+    private final ClientBuilder clientBuilder;
 
     /** The discovery services created by this class (one per room and one for each device) */
     private final ConcurrentMap<ThingUID, ServiceRegistration<?>> discoveryServiceRegs = new ConcurrentHashMap<>();
 
-    /**
-     * Sets the {@link HttpService}.
-     *
-     * @param httpService the non-null {@link HttpService} to use
-     */
-    @Reference
-    protected void setHttpService(HttpService httpService) {
-        Objects.requireNonNull(httpService, "httpService cannot be null");
+    @Activate
+    public NeeoHandlerFactory(@Reference HttpService httpService,
+            @Reference NetworkAddressService networkAddressService, @Reference ClientBuilder clientBuilder) {
         this.httpService = httpService;
-    }
-
-    /**
-     * Unsets the {@link HttpService}
-     *
-     * @param httpService the {@link HttpService} (not used in this implementation)
-     */
-    protected void unsetHttpService(HttpService httpService) {
-        this.httpService = null;
-    }
-
-    /**
-     * Sets the {@link NetworkAddressService}.
-     *
-     * @param networkAddressService the non-null {@link NetworkAddressService} to use
-     */
-    @Reference
-    protected void setNetworkAddressService(NetworkAddressService networkAddressService) {
-        Objects.requireNonNull(networkAddressService, "networkAddressService cannot be null");
         this.networkAddressService = networkAddressService;
-    }
-
-    /**
-     * Unsets the {@link NetworkAddressService}
-     *
-     * @param networkAddressService the {@link NetworkAddressService} (not used in this implementation)
-     */
-    protected void unsetNetworkAddressService(NetworkAddressService networkAddressService) {
-        this.networkAddressService = null;
+        this.clientBuilder = clientBuilder;
     }
 
     @Override
@@ -112,17 +84,10 @@ protected ThingHandler createHandler(Thing thing) {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (thingTypeUID.equals(NeeoConstants.BRIDGE_TYPE_BRAIN)) {
-            final HttpService localHttpService = httpService;
-            final NetworkAddressService localNetworkAddressService = networkAddressService;
-
-            Objects.requireNonNull(localHttpService, "HttpService cannot be null");
-            Objects.requireNonNull(localNetworkAddressService, "networkAddressService cannot be null");
-
             final int port = HttpServiceUtil.getHttpServicePort(this.bundleContext);
-
             final NeeoBrainHandler handler = new NeeoBrainHandler((Bridge) thing,
-                    port < 0 ? NeeoConstants.DEFAULT_BRAIN_HTTP_PORT : port, localHttpService,
-                    localNetworkAddressService);
+                    port < 0 ? NeeoConstants.DEFAULT_BRAIN_HTTP_PORT : port, httpService, networkAddressService,
+                    clientBuilder);
             registerRoomDiscoveryService(handler);
             return handler;
         } else if (thingTypeUID.equals(NeeoConstants.BRIDGE_TYPE_ROOM)) {

bundles/org.openhab.binding.neeo/src/main/java/org/openhab/binding/neeo/internal/net/HttpRequest.java

@@ -13,6 +13,8 @@
 package org.openhab.binding.neeo.internal.net;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.Objects;
 
 import javax.ws.rs.ProcessingException;
@@ -47,8 +49,8 @@ public class HttpRequest implements AutoCloseable {
     /**
      * Instantiates a new request
      */
-    public HttpRequest() {
-        client = ClientBuilder.newClient();
+    public HttpRequest(ClientBuilder clientBuilder) {
+        client = clientBuilder.build();
 
         if (logger.isDebugEnabled()) {
             client.register(new LoggingFilter(new Slf4LoggingAdapter(logger), true));
@@ -81,16 +83,23 @@ public HttpResponse sendGetCommand(String uri) {
     /**
      * Send post JSON command using the body
      *
-     * @param uri the non empty uri
+     * @param uriString the non empty uri
      * @param body the non-null, possibly empty body
      * @return the {@link HttpResponse}
      */
-    public HttpResponse sendPostJsonCommand(String uri, String body) {
-        NeeoUtil.requireNotEmpty(uri, "uri cannot be empty");
+    public HttpResponse sendPostJsonCommand(String uriString, String body) {
+        NeeoUtil.requireNotEmpty(uriString, "uri cannot be empty");
         Objects.requireNonNull(body, "body cannot be null");
 
+        logger.trace("sendPostJsonCommand: target={}, body={}", uriString, body);
+
         try {
-            final Builder request = client.target(uri).request(MediaType.APPLICATION_JSON);
+            URI targetUri = new URI(uriString);
+            if (!targetUri.isAbsolute()) {
+                logger.warn("Absolute URI required but provided URI '{}' is non-absolute. ", uriString);
+                return new HttpResponse(HttpStatus.NOT_ACCEPTABLE_406, "Absolute URI required");
+            }
+            final Builder request = client.target(targetUri).request(MediaType.APPLICATION_JSON);
 
             final Response content = request.post(Entity.entity(body, MediaType.APPLICATION_JSON));
 
@@ -103,6 +112,8 @@ public HttpResponse sendPostJsonCommand(String uri, String body) {
             // as well
         } catch (IOException | IllegalStateException | IllegalArgumentException | ProcessingException e) {
             return new HttpResponse(HttpStatus.SERVICE_UNAVAILABLE_503, e.getMessage());
+        } catch (URISyntaxException e) {
+            return new HttpResponse(HttpStatus.NOT_ACCEPTABLE_406, e.getMessage());
         }
     }