CHE-6072: fix bug when 'null' is used as a server path

Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
6.19.x
Oleksandr Garagatyi 2017-08-23 16:06:14 +03:00
parent c7df77ccf7
commit 7cb8a224fc
6 changed files with 95 additions and 20 deletions

View File

@ -10,6 +10,8 @@
*/
package org.eclipse.che.api.core.model.workspace.config;
import org.eclipse.che.commons.annotation.Nullable;
/**
* Configuration of server that can be started inside of machine.
*
@ -47,5 +49,6 @@ public interface ServerConfig {
String getProtocol();
/** Path used by server. */
@Nullable
String getPath();
}

View File

@ -94,7 +94,9 @@ public final class Labels {
public Serializer server(String ref, ServerConfig server) {
labels.put(String.format(SERVER_PORT_LABEL_FMT, ref), server.getPort());
labels.put(String.format(SERVER_PROTOCOL_LABEL_FMT, ref), server.getProtocol());
labels.put(String.format(SERVER_PATH_LABEL_FMT, ref), server.getPath());
if (server.getPath() != null) {
labels.put(String.format(SERVER_PATH_LABEL_FMT, ref), server.getPath());
}
return this;
}

View File

@ -11,9 +11,9 @@
package org.eclipse.che.workspace.infrastructure.docker;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import com.google.common.collect.ImmutableMap;
import java.util.HashMap;
import java.util.Map;
import org.eclipse.che.api.core.model.workspace.config.ServerConfig;
import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
@ -36,6 +36,7 @@ public class LabelsTest {
.runtimeId(new RuntimeIdentityImpl("workspace123", "my-env", "owner"))
.server("my-server1/http", new ServerConfigImpl("8000/tcp", "http", "/api/info"))
.server("my-server2", new ServerConfigImpl("8080/tcp", "ws", "/connect"))
.server("my-server3", new ServerConfigImpl("7070/tcp", "http", null))
.labels();
Map<String, String> expected =
ImmutableMap.<String, String>builder()
@ -49,6 +50,8 @@ public class LabelsTest {
.put("org.eclipse.che.server.my-server2.port", "8080/tcp")
.put("org.eclipse.che.server.my-server2.protocol", "ws")
.put("org.eclipse.che.server.my-server2.path", "/connect")
.put("org.eclipse.che.server.my-server3.port", "7070/tcp")
.put("org.eclipse.che.server.my-server3.protocol", "http")
.build();
assertEquals(serialized, expected);
@ -70,9 +73,15 @@ public class LabelsTest {
.put("org.eclipse.che.server.my-server2.port", "8080/tcp")
.put("org.eclipse.che.server.my-server2.protocol", "ws")
.put("org.eclipse.che.server.my-server2.path", "/connect")
.put("org.eclipse.che.server.my-server3.port", "7070/tcp")
.put("org.eclipse.che.server.my-server3.protocol", "http")
.build();
Labels.Deserializer deserializer = Labels.newDeserializer(labels);
Map<String, ServerConfig> expectedServers = new HashMap<>();
expectedServers.put("my-server1/http", new ServerConfigImpl("8000/tcp", "http", "/api/info"));
expectedServers.put("my-server2", new ServerConfigImpl("8080/tcp", "ws", "/connect"));
expectedServers.put("my-server3", new ServerConfigImpl("7070/tcp", "http", null));
assertEquals(deserializer.machineName(), "dev-machine");
@ -82,16 +91,6 @@ public class LabelsTest {
assertEquals(runtimeId.getOwner(), "owner", "workspace owner");
Map<String, ServerConfig> servers = deserializer.servers();
ServerConfig server1 = servers.get("my-server1/http");
assertNotNull(server1, "first server");
assertEquals(server1.getPort(), "8000/tcp");
assertEquals(server1.getProtocol(), "http");
assertEquals(server1.getPath(), "/api/info");
ServerConfig server2 = servers.get("my-server2");
assertNotNull(server2, "second server");
assertEquals(server2.getPort(), "8080/tcp");
assertEquals(server2.getProtocol(), "ws");
assertEquals(server2.getPath(), "/connect");
assertEquals(servers, expectedServers);
}
}

View File

@ -70,17 +70,30 @@ public class ServerResolver {
(name, config) ->
servers.put(
name,
new ServerImpl(
config.getProtocol()
+ "://"
+ route.getSpec().getHost()
+ config.getPath(),
ServerStatus.UNKNOWN)));
newServer(
config.getProtocol(), route.getSpec().getHost(), config.getPath())));
}
}
return servers;
}
private ServerImpl newServer(String protocol, String host, String path) {
StringBuilder ub = new StringBuilder();
if (protocol != null) {
ub.append(protocol).append("://");
} else {
ub.append("tcp://");
}
ub.append(host);
if (path != null) {
if (!path.isEmpty() && !path.startsWith("/")) {
ub.append("/");
}
ub.append(path);
}
return new ServerImpl(ub.toString(), ServerStatus.UNKNOWN);
}
private List<Service> getMatchedServices(Pod pod, Container container) {
return services
.stream()

View File

@ -11,6 +11,7 @@
package org.eclipse.che.workspace.infrastructure.openshift;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
@ -105,6 +106,63 @@ public class ServerResolverTest {
assertEquals(resolved.get("ws-server"), new ServerImpl("ws://localhost/connect"));
}
@Test
public void testResolvingServersWhenThereIsMatchedServiceForContainerAndServerPathIsNull() {
Container container = createContainer();
Pod pod = createPod(singletonMap("kind", "web-app"));
Service nonMatchedByPodService =
createService("matched", CONTAINER_PORT, singletonMap("kind", "web-app"));
Route route =
createRoute(
"matched", singletonMap("http-server", new ServerConfigImpl("3054", "http", null)));
ServerResolver serverResolver =
ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route));
Map<String, ServerImpl> resolved = serverResolver.resolve(pod, container);
assertEquals(resolved.size(), 1);
assertEquals(resolved.get("http-server"), new ServerImpl("http://localhost"));
}
@Test
public void testResolvingServersWhenThereIsMatchedServiceForContainerAndServerPathIsEmpty() {
Container container = createContainer();
Pod pod = createPod(singletonMap("kind", "web-app"));
Service nonMatchedByPodService =
createService("matched", CONTAINER_PORT, singletonMap("kind", "web-app"));
Route route =
createRoute(
"matched", singletonMap("http-server", new ServerConfigImpl("3054", "http", "")));
ServerResolver serverResolver =
ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route));
Map<String, ServerImpl> resolved = serverResolver.resolve(pod, container);
assertEquals(resolved.size(), 1);
assertEquals(resolved.get("http-server"), new ServerImpl("http://localhost"));
}
@Test
public void testResolvingServersWhenThereIsMatchedServiceForContainerAndServerPathIsRelative() {
Container container = createContainer();
Pod pod = createPod(singletonMap("kind", "web-app"));
Service nonMatchedByPodService =
createService("matched", CONTAINER_PORT, singletonMap("kind", "web-app"));
Route route =
createRoute(
"matched", singletonMap("http-server", new ServerConfigImpl("3054", "http", "api")));
ServerResolver serverResolver =
ServerResolver.of(singletonList(nonMatchedByPodService), singletonList(route));
Map<String, ServerImpl> resolved = serverResolver.resolve(pod, container);
assertEquals(resolved.size(), 1);
assertEquals(resolved.get("http-server"), new ServerImpl("http://localhost/api"));
}
private Pod createPod(Map<String, String> labels) {
return new PodBuilder().withNewMetadata().withLabels(labels).endMetadata().build();
}

View File

@ -105,7 +105,7 @@ public class ServerConfigImpl implements ServerConfig {
return Objects.equals(id, that.id)
&& Objects.equals(port, that.port)
&& Objects.equals(protocol, that.protocol)
&& getPath().equals(that.getPath());
&& Objects.equals(path, that.path);
}
@Override