Skip to content

Commit

Permalink
The Workspace validator should allow the 'ssh' machine type (#1055)
Browse files Browse the repository at this point in the history
* The Workspace validator should allow the 'ssh' machine type


Signed-off-by: Kaloyan Raev <kaloyan.r@zend.com>

* Avoid hardcoding machine types in workspace validation

* getProviderTypes() should return immutable collection

* Fix indentation in DefaultWorkspaceValidatorTest

* Fix @link Javadoc annotation in MachineInstanceProviders
  • Loading branch information
kaloyan-raev authored and skabashnyuk committed Apr 26, 2016
1 parent f348a8e commit 97a17fd
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@

import javax.inject.Inject;
import javax.inject.Singleton;

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

/**
* Provides machines {link InstanceProvider} implementation by machine type
* Provides machines {@link InstanceProvider} implementation by machine type
*
* @author Alexander Garagatyi
*/
Expand All @@ -37,16 +40,35 @@ public MachineInstanceProviders(Set<InstanceProvider> instanceProviders) {
}

/**
* Returns {link InstanceProvider} implementation by machine type
* Returns {@link InstanceProvider} implementation by machine type
*
* @param machineType type of machine implementation
* @return implementation of the machine {code InstanceProvider}
* @throws NotFoundException if no implementation found for provided machine type
*/
public InstanceProvider getProvider(String machineType) throws NotFoundException {
if (instanceProviders.containsKey(machineType)) {
if (hasProvider(machineType)) {
return instanceProviders.get(machineType);
}
throw new NotFoundException(String.format("Can't find machine provider for unsupported machine type '%s'", machineType));
}

/**
* Checks if an {@link InstanceProvider} implementation of the given machine type exists
*
* @param machineType type of machine implementation
* @return <code>true</code> if such implementation exists, <code>false</code> otherwise
*/
public boolean hasProvider(String machineType) {
return instanceProviders.containsKey(machineType);
}

/**
* Returns the machine types of all available {@link InstanceProvider} implementations.
*
* @return a collection of machine types
*/
public Collection<String> getProviderTypes() {
return Collections.unmodifiableSet(instanceProviders.keySet());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
import org.eclipse.che.api.core.model.workspace.Environment;
import org.eclipse.che.api.core.model.workspace.Workspace;
import org.eclipse.che.api.core.model.workspace.WorkspaceConfig;
import org.eclipse.che.api.machine.server.MachineInstanceProviders;

import com.google.common.base.Joiner;

import javax.inject.Inject;
import javax.inject.Singleton;
import java.util.Map;
import java.util.regex.Pattern;
Expand All @@ -36,6 +40,13 @@ public class DefaultWorkspaceValidator implements WorkspaceValidator {
private static final Pattern WS_NAME = Pattern.compile("[a-zA-Z0-9][-_.a-zA-Z0-9]{1,18}[a-zA-Z0-9]");
private static final Pattern SERVER_PORT = Pattern.compile("[1-9]+[0-9]*/(?:tcp|udp)");
private static final Pattern SERVER_PROTOCOL = Pattern.compile("[a-z][a-z0-9-+.]*");

private final MachineInstanceProviders machineInstanceProviders;

@Inject
public DefaultWorkspaceValidator(MachineInstanceProviders machineInstanceProviders) {
this.machineInstanceProviders = machineInstanceProviders;
}

@Override
public void validateWorkspace(Workspace workspace) throws BadRequestException {
Expand Down Expand Up @@ -112,10 +123,12 @@ private void validateEnv(Environment environment, String workspaceName) throws B
private void validateMachine(MachineConfig machineCfg, String envName) throws BadRequestException {
checkArgument(!isNullOrEmpty(machineCfg.getName()), "Environment %s contains machine with null or empty name", envName);
checkNotNull(machineCfg.getSource(), "Environment " + envName + " contains machine without source");
checkArgument("docker".equals(machineCfg.getType()),
"Type of machine %s in environment %s is not supported. Supported value is 'docker'.",
checkArgument(machineInstanceProviders.hasProvider(machineCfg.getType()),
"Type %s of machine %s in environment %s is not supported. Supported values: %s.",
machineCfg.getType(),
machineCfg.getName(),
envName);
envName,
Joiner.on(", ").join(machineInstanceProviders.getProviderTypes()));

for (ServerConf serverConf : machineCfg.getServers()) {
checkArgument(serverConf.getPort() != null && SERVER_PORT.matcher(serverConf.getPort()).matches(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
package org.eclipse.che.api.workspace.server;

import org.eclipse.che.api.core.BadRequestException;
import org.eclipse.che.api.machine.server.MachineInstanceProviders;
import org.eclipse.che.api.machine.server.spi.InstanceProvider;
import org.eclipse.che.api.machine.shared.dto.CommandDto;
import org.eclipse.che.api.machine.shared.dto.MachineConfigDto;
import org.eclipse.che.api.machine.shared.dto.MachineSourceDto;
Expand All @@ -19,8 +21,14 @@
import org.eclipse.che.api.workspace.shared.dto.EnvironmentDto;
import org.eclipse.che.api.workspace.shared.dto.RecipeDto;
import org.eclipse.che.api.workspace.shared.dto.WorkspaceConfigDto;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.testng.MockitoTestNGListener;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Listeners;
import org.testng.annotations.Test;

import java.util.ArrayList;
Expand All @@ -32,19 +40,27 @@
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

/**
* Tests for {@link WorkspaceValidator} and {@link DefaultWorkspaceValidator}
*
* @author Alexander Reshetnyak
*/
@Listeners(MockitoTestNGListener.class)
public class DefaultWorkspaceValidatorTest {

private WorkspaceValidator wsValidator;
@Mock
private MachineInstanceProviders machineInstanceProviders;
@InjectMocks
private DefaultWorkspaceValidator wsValidator;

@BeforeClass
@BeforeMethod
public void prepare() throws Exception {
wsValidator = new DefaultWorkspaceValidator();
when(machineInstanceProviders.hasProvider("docker")).thenReturn(true);
when(machineInstanceProviders.getProviderTypes()).thenReturn(Arrays.asList(new String[] { "docker", "ssh" }));
}

@Test
Expand Down Expand Up @@ -301,7 +317,7 @@ public void shouldFailValidationIfMachineSourceIsNull() throws Exception {
}

@Test(expectedExceptions = BadRequestException.class,
expectedExceptionsMessageRegExp = "Type of machine .* in environment .* is not supported. Supported value is 'docker'.")
expectedExceptionsMessageRegExp = "Type .* of machine .* in environment .* is not supported. Supported values: docker, ssh.")
public void shouldFailValidationIfMachineTypeIsNull() throws Exception {
final WorkspaceConfigDto config = createConfig();
config.getEnvironments()
Expand All @@ -315,7 +331,7 @@ public void shouldFailValidationIfMachineTypeIsNull() throws Exception {
}

@Test(expectedExceptions = BadRequestException.class,
expectedExceptionsMessageRegExp = "Type of machine .* in environment .* is not supported. Supported value is 'docker'.")
expectedExceptionsMessageRegExp = "Type .* of machine .* in environment .* is not supported. Supported values: docker, ssh.")
public void shouldFailValidationIfMachineTypeIsNotDocker() throws Exception {
final WorkspaceConfigDto config = createConfig();
config.getEnvironments()
Expand Down

0 comments on commit 97a17fd

Please sign in to comment.