seg_yinzy
/
che-server
mirror of https://github.com/eclipse-che/che-server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

CHE-2022; fixed validation behavior when field value matches default value for its type (#2023)

6.19.x
Max Shaposhnik 2016-08-04 10:56:29 +03:00 committed by GitHub
parent fb82bf80b9
commit 19df9badeb
5 changed files with 72 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/FactoryService.java
View File

@ -245,6 +245,7 @@ public class FactoryService extends Service {
if (factory == null) {
throw new BadRequestException("Not null factory required");
}
factoryBuilder.checkValid(factory);
processDefaults(factory);
createValidator.validateOnCreate(factory);
final Factory storedFactory = factoryStore.getFactory(factoryStore.saveFactory(factory, null));
@ -345,6 +346,7 @@ public class FactoryService extends Service {
newFactory.setId(existingFactory.getId());
// validate the new content
factoryBuilder.checkValid(newFactory, true);
createValidator.validateOnCreate(newFactory);
// access granted, user can update the factory

14
wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/ValueHelper.java
View File

@ -13,6 +13,8 @@ package org.eclipse.che.api.factory.server;
import java.util.Collection;
import java.util.Map;
import static com.google.common.base.Strings.isNullOrEmpty;
/**
* @author Sergii Kabashniuk
*/
@ -22,19 +24,13 @@ public class ValueHelper {
*
* @param value
* - value to check
* @return - true if value is useless for factory (0 for primitives or empty collection or map), false otherwise
* @return - true if value is useless for factory (empty string, collection or map), false otherwise
*/
public static boolean isEmpty(Object value) {
return (null == value) ||
(value.getClass().equals(Boolean.class) && !((Boolean)value)) ||
(value.getClass().equals(Integer.class) && (Integer)value == 0) ||
(value.getClass().equals(Long.class) && (Long)value == 0) ||
(value.getClass().equals(String.class) && isNullOrEmpty((String)value) ||
(Collection.class.isAssignableFrom(value.getClass()) && ((Collection)value).isEmpty()) ||
(Map.class.isAssignableFrom(value.getClass()) && ((Map)value).isEmpty()) ||
(value.getClass().equals(Byte.class) && (Byte)value == 0) ||
(value.getClass().equals(Short.class) && (Short)value == 0) ||
(value.getClass().equals(Double.class) && (Double)value == 0) ||
(value.getClass().equals(Float.class) && (Float)value == 0);
(Map.class.isAssignableFrom(value.getClass()) && ((Map)value).isEmpty()));
}
}

40
wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/builder/FactoryBuilder.java
View File

@ -116,13 +116,27 @@ public class FactoryBuilder {
}
/**
* Validate factory compatibility.
* Validate factory compatibility at creation time.
*
* @param factory
* - factory object to validate
* @throws ConflictException
*/
public void checkValid(Factory factory) throws ConflictException {
checkValid(factory, false);
}
/**
* Validate factory compatibility.
*
* @param factory
* - factory object to validate
* @param isUpdate
* - indicates is validation performed on update time.
* Set-by-server variables are allowed during update.
* @throws ConflictException
*/
public void checkValid(Factory factory, boolean isUpdate) throws ConflictException {
if (null == factory) {
throw new ConflictException(FactoryConstants.UNPARSABLE_FACTORY_MESSAGE);
}
@ -145,7 +159,7 @@ public class FactoryBuilder {
default:
throw new ConflictException(FactoryConstants.INVALID_VERSION_MESSAGE);
}
validateCompatibility(factory, null, Factory.class, usedFactoryVersionMethodProvider, v, "");
validateCompatibility(factory, null, Factory.class, usedFactoryVersionMethodProvider, v, "", isUpdate);
}
/**
@ -170,7 +184,7 @@ public class FactoryBuilder {
*
* @param object
* - object to validate factory parameters
* @param object
* @param parent
* - parent object
* @param methodsProvider
* - class that provides methods with {@link org.eclipse.che.api.core.factory.FactoryParameter}
@ -188,7 +202,8 @@ public class FactoryBuilder {
Class methodsProvider,
Class allowedMethodsProvider,
Version version,
String parentName) throws ConflictException {
String parentName,
boolean isUpdate) throws ConflictException {
// validate source
if (SourceStorageDto.class.equals(methodsProvider) && !hasSubprojectInPath(parent)) {
sourceStorageParametersValidator.validate((SourceStorage)object, version);
@ -203,9 +218,12 @@ public class FactoryBuilder {
continue;
}
String fullName = (parentName.isEmpty() ? "" : (parentName + ".")) + CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_CAMEL,
method.getName()
.substring(3)
.toLowerCase());
method.getName().startsWith("is")
? method.getName()
.substring(2)
: method.getName()
.substring(3)
.toLowerCase());
// check that field is set
Object parameterValue;
try {
@ -235,7 +253,7 @@ public class FactoryBuilder {
throw new ConflictException(String.format(FactoryConstants.PARAMETRIZED_INVALID_PARAMETER_MESSAGE, fullName, version));
} else {
// is parameter deprecated
if (factoryParameter.deprecatedSince().compareTo(version) <= 0 || factoryParameter.setByServer()) {
if (factoryParameter.deprecatedSince().compareTo(version) <= 0 || (!isUpdate && factoryParameter.setByServer())) {
throw new ConflictException(
String.format(FactoryConstants.PARAMETRIZED_INVALID_PARAMETER_MESSAGE, fullName, version));
}
@ -243,7 +261,7 @@ public class FactoryBuilder {
// use recursion if parameter is DTO object
if (method.getReturnType().isAnnotationPresent(DTO.class)) {
// validate inner objects such Git ot ProjectAttributes
validateCompatibility(parameterValue, object, method.getReturnType(), method.getReturnType(), version, fullName);
validateCompatibility(parameterValue, object, method.getReturnType(), method.getReturnType(), version, fullName, isUpdate);
} else if (Map.class.isAssignableFrom(method.getReturnType())) {
Type tp = ((ParameterizedType)method.getGenericReturnType()).getActualTypeArguments()[1];
@ -253,7 +271,7 @@ public class FactoryBuilder {
Map<Object, Object> map = (Map)parameterValue;
for (Map.Entry<Object, Object> entry : map.entrySet()) {
validateCompatibility(entry.getValue(), object, secMapParamClass, secMapParamClass, version,
fullName + "." + entry.getKey());
fullName + "." + entry.getKey(), isUpdate);
}
} else {
throw new RuntimeException("This type of fields is not supported by factory.");
@ -267,7 +285,7 @@ public class FactoryBuilder {
if (secListParamClass.isAnnotationPresent(DTO.class)) {
List<Object> list = (List)parameterValue;
for (Object entry : list) {
validateCompatibility(entry, object, secListParamClass, secListParamClass, version, fullName);
validateCompatibility(entry, object, secListParamClass, secListParamClass, version, fullName, isUpdate);
}
} else {
throw new RuntimeException("This type of fields is not supported by factory.");

2
wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/FactoryServiceTest.java
View File

@ -89,6 +89,7 @@ import static org.eclipse.che.api.factory.server.FactoryService.VALIDATE_QUERY_P
import static org.eclipse.che.api.workspace.server.DtoConverter.asDto;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyMap;
import static org.mockito.Matchers.anySetOf;
@ -160,6 +161,7 @@ public class FactoryServiceTest {
dto = DtoFactory.getInstance();
factoryBuilder = spy(new FactoryBuilder(new SourceStorageParametersValidator()));
doNothing().when(factoryBuilder).checkValid(any(Factory.class));
doNothing().when(factoryBuilder).checkValid(any(Factory.class), anyBoolean());
when(factoryParametersResolverHolder.getFactoryParametersResolvers()).thenReturn(factoryParametersResolvers);
when(userDao.getById(anyString())).thenReturn(new UserImpl(null,
null,

35
wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/builder/FactoryBuilderTest.java
View File

@ -14,6 +14,8 @@ import com.google.common.collect.ImmutableMap;
import org.eclipse.che.api.core.ApiException;
import org.eclipse.che.api.core.factory.FactoryParameter;
import org.eclipse.che.api.core.model.machine.Limits;
import org.eclipse.che.api.core.model.workspace.Environment;
import org.eclipse.che.api.factory.server.impl.SourceStorageParametersValidator;
import org.eclipse.che.api.factory.shared.dto.Action;
import org.eclipse.che.api.factory.shared.dto.Author;
@ -26,6 +28,7 @@ import org.eclipse.che.api.factory.shared.dto.OnAppLoaded;
import org.eclipse.che.api.factory.shared.dto.OnProjectsLoaded;
import org.eclipse.che.api.factory.shared.dto.Policies;
import org.eclipse.che.api.machine.shared.dto.CommandDto;
import org.eclipse.che.api.machine.shared.dto.LimitsDto;
import org.eclipse.che.api.machine.shared.dto.MachineConfigDto;
import org.eclipse.che.api.machine.shared.dto.MachineSourceDto;
import org.eclipse.che.api.machine.shared.dto.ServerConfDto;
@ -45,6 +48,7 @@ import org.testng.annotations.Test;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.net.URISyntaxException;
import java.util.Collections;
import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
@ -102,6 +106,12 @@ public class FactoryBuilderTest {
factoryBuilder.checkValid(factory);
}
@Test(dataProvider = "setByServerParamsProvider")
public void shouldAllowUsingParamsThatCanBeSetOnlyByServerDuringUpdate(Factory factory)
throws InvocationTargetException, IllegalAccessException, ApiException, NoSuchMethodException {
factoryBuilder.checkValid(factory, true);
}
@DataProvider(name = "setByServerParamsProvider")
public static Object[][] setByServerParamsProvider() throws URISyntaxException, IOException, NoSuchMethodException {
Factory factory = prepareFactory();
@ -122,7 +132,30 @@ public class FactoryBuilderTest {
@DataProvider(name = "notValidParamsProvider")
public static Object[][] notValidParamsProvider() throws URISyntaxException, IOException, NoSuchMethodException {
return new Object[][] {};
Factory factory = prepareFactory();
EnvironmentDto environmentDto = factory.getWorkspace().getEnvironments().get(0);
environmentDto.getMachineConfigs().add(dto.createDto(MachineConfigDto.class).withType(null));
return new Object[][] {
{dto.clone(factory).withWorkspace(factory.getWorkspace().withDefaultEnv(null)) },
{dto.clone(factory).withWorkspace(factory.getWorkspace().withEnvironments(Collections.singletonList(environmentDto))) }
};
}
@Test(dataProvider = "sameAsDefaultParamsProvider")
public void shouldAllowUsingParamsWIthValueLikeDefaultForType(Factory factory)
throws InvocationTargetException, IllegalAccessException, ApiException, NoSuchMethodException {
factoryBuilder.checkValid(factory);
}
@DataProvider(name = "sameAsDefaultParamsProvider")
public static Object[][] sameAsDefaultParamsProvider() throws URISyntaxException, IOException, NoSuchMethodException {
Factory factory = prepareFactory();
EnvironmentDto environmentDto = factory.getWorkspace().getEnvironments().get(0);
environmentDto.getMachineConfigs().get(0).withDev(false)
.withLimits(dto.newDto(LimitsDto.class).withRam(0));
return new Object[][] {
{dto.clone(factory).withWorkspace(factory.getWorkspace().withEnvironments(Collections.singletonList(environmentDto))) }
};
}