Adds test of HttpProxy

9 files changed, 209 insertions, 97 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java
index 83950d7ce03..78cf762671a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/HttpProxy.java
@@ -40,10 +40,10 @@ public class HttpProxy {
 
         // "http" and "state" seems to uniquely identify an interesting HTTP port on each service
         PortInfo port = service.getPorts().stream()
-                .filter(portInfo -> !portInfo.getTags().stream().collect(Collectors.toSet()).containsAll(
+                .filter(portInfo -> portInfo.getTags().stream().collect(Collectors.toSet()).containsAll(
                         Stream.of("http", "state").collect(Collectors.toSet())))
                 .findFirst()
-                .orElseThrow(() -> new InternalServerException("Failed to find HTTP status port"));
+                .orElseThrow(() -> new InternalServerException("Failed to find HTTP state port"));
 
         return internalGet(host.getHostname(), port.getPort(), relativePath);
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java
index 5fe61d5dc8d..1ff1581816e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SimpleHttpFetcher.java
@@ -21,10 +21,9 @@ public class SimpleHttpFetcher implements HttpFetcher {
             connection.setReadTimeout(params.readTimeoutMs);
             int code = connection.getResponseCode();
             String contentType = connection.getContentType();
-            try (InputStream inputStream = connection.getInputStream()) {
-                ProxyResponse response = new ProxyResponse(code, contentType, inputStream);
-                return response;
-            }
+            InputStream inputStream = connection.getInputStream();
+            StaticResponse response = new StaticResponse(code, contentType, inputStream);
+            return response;
         } catch (SocketTimeoutException e) {
             String message = "Timed out after " + params.readTimeoutMs + " ms reading response from " + url;
             logger.log(LogLevel.WARNING, message, e);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ProxyResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/StaticResponse.java
index 114c6c08bb6..74cd0a94e79 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/ProxyResponse.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/StaticResponse.java
@@ -4,30 +4,33 @@ package com.yahoo.vespa.config.server.http;
 import com.yahoo.container.jdisc.HttpResponse;
 import org.apache.commons.io.IOUtils;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
 
-public class ProxyResponse extends HttpResponse {
+public class StaticResponse extends HttpResponse {
     private final String contentType;
-    private final InputStream inputStream;
+    private final InputStream body;
 
     /**
-     *
-     * @param status
-     * @param contentType
-     * @param inputStream    Ownership is passed to ProxyResponse (responsible for closing it)
+     * @param body    Ownership is passed to StaticResponse (is responsible for closing it)
      */
-    public ProxyResponse(int status, String contentType, InputStream inputStream) {
+    public StaticResponse(int status, String contentType, InputStream body) {
         super(status);
         this.contentType = contentType;
-        this.inputStream = inputStream;
+        this.body = body;
+    }
+
+    public StaticResponse(int status, String contentType, String body) {
+        this(status, contentType, new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)));
     }
 
     @Override
     public void render(OutputStream outputStream) throws IOException {
-        IOUtils.copy(inputStream, outputStream);
-        inputStream.close();
+        IOUtils.copy(body, outputStream);
+        body.close();
     }
 
     @Override
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
index 62df83e5916..a1c7f211765 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
@@ -72,7 +72,6 @@ public class ApplicationHandler extends HttpHandler {
             return applicationRepository.clusterControllerStatusPage(tenant, applicationId, hostName, pathSuffix);
         }
 
-        // This matches if group count > 7!? It should claim a namespace... We will have to mangle the namespace above.
         if (isContentRequest(request)) {
             long sessionId = applicationRepository.getSessionIdForApplication(tenant, applicationId);
             String contentPath = ApplicationContentRequest.getContentPath(request);
@@ -178,7 +177,8 @@ public class ApplicationHandler extends HttpHandler {
     }
 
     private static boolean isContentRequest(HttpRequest request) {
-        return getBindingMatch(request).groupCount() > 7;
+        return getBindingMatch(request).groupCount() > 7 &&
+                request.getUri().getPath().contains("/content/");
     }
 
     private static String getHostFromRequest(HttpRequest req) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java
index 73f68023266..0f11118d49d 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java
@@ -3,23 +3,13 @@ package com.yahoo.vespa.config.server.application;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.yahoo.config.codegen.InnerCNode;
-import com.yahoo.config.model.api.FileDistribution;
-import com.yahoo.config.model.api.HostInfo;
 import com.yahoo.config.model.api.Model;
-import com.yahoo.config.model.api.PortInfo;
-import com.yahoo.config.model.api.ServiceInfo;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.InstanceName;
-import com.yahoo.config.provision.ProvisionInfo;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.Version;
 import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.vespa.config.ConfigKey;
-import com.yahoo.vespa.config.ConfigPayload;
-import com.yahoo.vespa.config.buildergen.ConfigDefinition;
 import com.yahoo.vespa.config.server.ServerCache;
 import com.yahoo.vespa.config.server.http.SessionHandlerTest;
 import com.yahoo.vespa.config.server.monitoring.MetricUpdater;
@@ -31,13 +21,6 @@ import org.xml.sax.SAXException;
 
 import java.io.IOException;
 import java.net.URI;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
@@ -59,7 +42,7 @@ public class ApplicationConvergenceCheckerTest {
 
     @Before
     public void setup() throws IOException, SAXException, InterruptedException {
-        Model mockModel = new MockModel("localhost", 1337);
+        Model mockModel = MockModel.createContainer("localhost", 1337);
         application = new Application(mockModel, new ServerCache(), 3, Version.fromIntValues(0, 0, 0), MetricUpdater.createTestUpdater(), appId);
     }
 
@@ -112,61 +95,4 @@ public class ApplicationConvergenceCheckerTest {
         }
     }
 
-    // Model with two services, one that does not have a state port
-    private static class MockModel implements Model {
-        private final String hostname;
-        private final int statePort;
-
-        MockModel(String hostname, int statePort) {
-            this.hostname = hostname;
-            this.statePort = statePort;
-        }
-
-        @Override
-        public ConfigPayload getConfig(ConfigKey<?> configKey, ConfigDefinition targetDef, ConfigPayload override) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public ConfigPayload getConfig(ConfigKey<?> configKey, InnerCNode targetDef, ConfigPayload override) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Set<ConfigKey<?>> allConfigsProduced() {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Collection<HostInfo> getHosts() {
-            ServiceInfo container = createServiceInfo(hostname, "container", "container",
-                                                      ClusterSpec.Type.container, statePort, "state");
-            ServiceInfo serviceNoStatePort = createServiceInfo(hostname, "logserver", "logserver",
-                                                               ClusterSpec.Type.admin, 1234, "logtp");
-            return Collections.singleton(new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort)));
-        }
-
-        ServiceInfo createServiceInfo(String hostname, String name, String type, ClusterSpec.Type clusterType, int port, String portTags) {
-            PortInfo portInfo = new PortInfo(port, Collections.singleton(portTags));
-            Map<String, String> properties = new HashMap<>();
-            properties.put("clustername", "default");
-            properties.put("clustertype", clusterType.name());
-            return new ServiceInfo(name, type, Collections.singleton(portInfo), properties, "", hostname);
-        }
-
-        @Override
-        public Set<String> allConfigIds() {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public void distributeFiles(FileDistribution fileDistribution) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Optional<ProvisionInfo> getProvisionInfo() {
-            throw new UnsupportedOperationException();
-        }
-    }
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java
new file mode 100644
index 00000000000..67dfe1048f3
--- /dev/null
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/HttpProxyTest.java
@@ -0,0 +1,64 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.application;
+
+import com.yahoo.config.model.api.Model;
+import com.yahoo.container.jdisc.HttpResponse;
+import com.yahoo.vespa.config.server.http.HttpFetcher;
+import com.yahoo.vespa.config.server.http.StaticResponse;
+import com.yahoo.vespa.config.server.http.RequestTimeoutException;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import java.net.URL;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class HttpProxyTest {
+    private final HttpFetcher fetcher = mock(HttpFetcher.class);
+    private final HttpProxy proxy = new HttpProxy(fetcher);
+
+    private static final String hostname = "foo.yahoo.com";
+    private static final int port = 19050;
+    private final Application applicationMock = mock(Application.class);
+
+    @Before
+    public void setup() {
+        Model modelMock = MockModel.createClusterController(hostname, port);
+        when(applicationMock.getModel()).thenReturn(modelMock);
+    }
+
+    @Test
+    public void testNormalGet() throws Exception {
+        ArgumentCaptor<HttpFetcher.Params> actualParams = ArgumentCaptor.forClass(HttpFetcher.Params.class);
+        ArgumentCaptor<URL> actualUrl = ArgumentCaptor.forClass(URL.class);
+        HttpResponse response = new StaticResponse(200, "application/json", "body");
+        when(fetcher.get(actualParams.capture(), actualUrl.capture())).thenReturn(response);
+
+        HttpResponse actualResponse = proxy.get(
+                applicationMock, hostname, "container-clustercontroller", "clustercontroller-status/v1/clusterName");
+
+        assertEquals(1, actualParams.getAllValues().size());
+        assertEquals(2000, actualParams.getValue().readTimeoutMs);
+
+        assertEquals(1, actualUrl.getAllValues().size());
+        assertEquals(new URL("http://" + hostname + ":" + port + "/clustercontroller-status/v1/clusterName"),
+                actualUrl.getValue());
+
+        // The HttpResponse returned by the fetcher IS the same object as the one returned by the proxy,
+        // when everything goes well.
+        assertTrue(actualResponse == response);
+    }
+
+    @Test(expected = RequestTimeoutException.class)
+    public void testFetchException() {
+        when(fetcher.get(any(), any())).thenThrow(new RequestTimeoutException("timed out"));
+
+        HttpResponse actualResponse = proxy.get(
+                applicationMock, hostname, "container-clustercontroller", "clustercontroller-status/v1/clusterName");
+    }
+}
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java
new file mode 100644
index 00000000000..b22e8648eae
--- /dev/null
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java
@@ -0,0 +1,105 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.application;
+
+import com.yahoo.config.codegen.InnerCNode;
+import com.yahoo.config.model.api.FileDistribution;
+import com.yahoo.config.model.api.HostInfo;
+import com.yahoo.config.model.api.Model;
+import com.yahoo.config.model.api.PortInfo;
+import com.yahoo.config.model.api.ServiceInfo;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.ProvisionInfo;
+import com.yahoo.vespa.config.ConfigKey;
+import com.yahoo.vespa.config.ConfigPayload;
+import com.yahoo.vespa.config.buildergen.ConfigDefinition;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+// Model with two services, one that does not have a state port
+class MockModel implements Model {
+    private final Collection<HostInfo> hosts;
+
+    static MockModel createContainer(String hostname, int statePort) {
+        ServiceInfo container = createServiceInfo(hostname, "container", "container",
+                ClusterSpec.Type.container, statePort, "state");
+        ServiceInfo serviceNoStatePort = createServiceInfo(hostname, "logserver", "logserver",
+                ClusterSpec.Type.admin, 1234, "logtp");
+        HostInfo hostInfo = new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort));
+        return new MockModel(Collections.singleton(hostInfo));
+    }
+
+    static MockModel createClusterController(String hostname, int statePort) {
+        ServiceInfo container = createServiceInfo(
+                hostname,
+                "foo", // name
+                "container-clustercontroller", // type
+                ClusterSpec.Type.container,
+                statePort,
+                "state http external query");
+        ServiceInfo serviceNoStatePort = createServiceInfo(hostname, "storagenode", "storagenode",
+                ClusterSpec.Type.content, 1234, "rpc");
+        HostInfo hostInfo = new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort));
+
+        return new MockModel(Collections.singleton(hostInfo));
+    }
+
+    static private ServiceInfo createServiceInfo(
+            String hostname,
+            String name,
+            String type,
+            ClusterSpec.Type clusterType,
+            int port,
+            String portTags) {
+        PortInfo portInfo = new PortInfo(port, Arrays.stream(portTags.split(" ")).collect(Collectors.toSet()));
+        Map<String, String> properties = new HashMap<>();
+        properties.put("clustername", "default");
+        properties.put("clustertype", clusterType.name());
+        return new ServiceInfo(name, type, Collections.singleton(portInfo), properties, "", hostname);
+    }
+
+    MockModel(Collection<HostInfo> hosts) {
+        this.hosts = hosts;
+    }
+
+    @Override
+    public ConfigPayload getConfig(ConfigKey<?> configKey, ConfigDefinition targetDef, ConfigPayload override) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public ConfigPayload getConfig(ConfigKey<?> configKey, InnerCNode targetDef, ConfigPayload override) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Set<ConfigKey<?>> allConfigsProduced() {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Collection<HostInfo> getHosts() {
+        return hosts;
+    }
+
+    @Override
+    public Set<String> allConfigIds() {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void distributeFiles(FileDistribution fileDistribution) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Optional<ProvisionInfo> getProvisionInfo() {
+        throw new UnsupportedOperationException();
+    }
+}
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java
index 6f69b135972..23b30cd2502 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HandlerTest.java
@@ -17,7 +17,12 @@ import static org.junit.Assert.*;
  */
 public class HandlerTest {
 
-    public static void assertHttpStatusCodeErrorCodeAndMessage(HttpResponse response, int statusCode, HttpErrorResponse.errorCodes errorCode, String message) throws IOException {
+    public static void assertHttpStatusCodeErrorCodeAndMessage(
+            HttpResponse response,
+            int statusCode,
+            HttpErrorResponse.errorCodes errorCode,
+            String contentType,
+            String message) throws IOException {
         assertNotNull(response);
         String renderedString = SessionHandlerTest.getRenderedString(response);
         if (renderedString == null) {
@@ -27,11 +32,21 @@ public class HandlerTest {
         if (errorCode != null) {
             assertThat(renderedString, containsString(errorCode.name()));
         }
+        if (contentType != null) {
+            assertThat(renderedString, response.getContentType(), is(contentType));
+        }
         assertThat(renderedString, containsString(message));
     }
 
+    public static void assertHttpStatusCodeErrorCodeAndMessage(HttpResponse response, int statusCode, HttpErrorResponse.errorCodes errorCode, String message) throws IOException {
+        assertHttpStatusCodeErrorCodeAndMessage(response, statusCode, errorCode, null, message);
+    }
+
     public static void assertHttpStatusCodeAndMessage(HttpResponse response, int statusCode, String message) throws IOException {
         assertHttpStatusCodeErrorCodeAndMessage(response, statusCode, null, message);
     }
 
+    public static void assertHttpStatusCodeAndMessage(HttpResponse response, int statusCode, String contentType, String message) throws IOException {
+        assertHttpStatusCodeErrorCodeAndMessage(response, statusCode, null, contentType, message);
+    }
 }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
index f695c533b0f..b091ec29b75 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
@@ -12,7 +12,6 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
-import com.yahoo.container.jdisc.LiteralResponse;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.jdisc.Response;
 import com.yahoo.path.Path;
@@ -28,6 +27,7 @@ import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.application.ZKTenantApplications;
 import com.yahoo.vespa.config.server.http.HandlerTest;
 import com.yahoo.vespa.config.server.http.HttpErrorResponse;
+import com.yahoo.vespa.config.server.http.StaticResponse;
 import com.yahoo.vespa.config.server.http.SessionHandlerTest;
 import com.yahoo.vespa.config.server.http.SimpleHttpFetcher;
 import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry;
@@ -227,10 +227,10 @@ public class ApplicationHandlerTest {
         String url = toUrlPath(application, Zone.defaultZone(), true) + "/clustercontroller/" + host + "/status/v1/clusterName1";
 
         when(mockHttpProxy.get(any(), eq(host), eq("container-clustercontroller"), eq("clustercontroller-status/v1/clusterName1")))
-                .thenReturn(new LiteralResponse(200, "<html>...</html>"));
+                .thenReturn(new StaticResponse(200, "text/html", "<html>...</html>"));
 
         HttpResponse response = mockHandler.handle(HttpRequest.createTestRequest(url, com.yahoo.jdisc.http.HttpRequest.Method.GET));
-        HandlerTest.assertHttpStatusCodeAndMessage(response, 200, "<html>...</html>");
+        HandlerTest.assertHttpStatusCodeAndMessage(response, 200, "text/html", "<html>...</html>");
     }
 
     @Test