Skip to content

Commit

Permalink
Code Review Changes and More Test Cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@anki2189 @steve-todorov
anki2189 authored and steve-todorov committed Feb 11, 2020
1 parent dc35268 commit 9bb9026
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 28 deletions.
2 changes: 1 addition & 1 deletion ...a/org/carlspring/strongbox/controllers/configuration/StoragesConfigurationController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public ResponseEntity getRepositoryResponseEntity(@RepositoryMapping Repository
@PreAuthorize("hasAuthority('CONFIGURATION_DELETE_REPOSITORY')")
@DeleteMapping(value = "/{storageId}/{repositoryId}", produces = { MediaType.TEXT_PLAIN_VALUE,
MediaType.APPLICATION_JSON_VALUE })
public ResponseEntity removeRepository(@RepositoryMapping Repository repository,
public ResponseEntity removeRepository(@RepositoryMapping(allowOutOfServiceRepository = true) Repository repository,
@ApiParam(value = "Whether to force delete the repository from the file system")
@RequestParam(name = "force", defaultValue = "false") final boolean force,
@RequestHeader(HttpHeaders.ACCEPT) String accept)
Expand Down
2 changes: 1 addition & 1 deletion strongbox-web-core/src/main/java/org/carlspring/strongbox/web/RepositoryMapping.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@

String repositoryVariableName() default "repositoryId";

boolean inServiceRepository() default true;
boolean allowOutOfServiceRepository() default false;
}
20 changes: 10 additions & 10 deletions ...web-core/src/main/java/org/carlspring/strongbox/web/RepositoryMethodArgumentResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ public class RepositoryMethodArgumentResolver
{

public static final String NOT_FOUND_STORAGE_MESSAGE = "Could not find requested storage %s.";

public static final String NOT_FOUND_REPOSITORY_MESSAGE = "Could not find requested repository %s:%s.";

public static final String NOT_IN_SERVICE_REPOSITORY_MESSAGE = "Requested repository %s:%s is out of service.";

@Inject
Expand All @@ -58,7 +56,7 @@ public Object resolveArgument(final MethodParameter parameter,
final ModelAndViewContainer modelAndViewContainer,
final NativeWebRequest nativeWebRequest,
final WebDataBinderFactory webDataBinderFactory)
throws MissingPathVariableException
throws MissingPathVariableException
{
final RepositoryMapping repositoryMapping = parameter.getParameterAnnotation(RepositoryMapping.class);
final String storageVariableName = repositoryMapping.storageVariableName();
Expand All @@ -71,7 +69,7 @@ public Object resolveArgument(final MethodParameter parameter,
RequestAttributes.SCOPE_REQUEST);

if (repository != null && Objects.equals(repository.getId(), repositoryId) &&
Objects.equals(repository.getStorage().getId(), storageId))
Objects.equals(repository.getStorage().getId(), storageId))
{
return repository;
}
Expand All @@ -90,8 +88,10 @@ public Object resolveArgument(final MethodParameter parameter,
throw new RepositoryNotFoundException(message);
}

final boolean inService = repositoryMapping.inServiceRepository();
if (!inService)
// This annotation is used in a lot of controllers - some of which are related to the configuration management.
// It is necessary to allow requests to pass when the repository status is `out of service` (i.e. `/api/configuration/**`),
// but still return `ServiceUnavailableException` when people are accessing `/storages/**`.
if (!repository.isInService() && !repositoryMapping.allowOutOfServiceRepository())
{
final String message = String.format(NOT_IN_SERVICE_REPOSITORY_MESSAGE, storageId, repositoryId);
throw new ServiceUnavailableException(message);
Expand All @@ -103,12 +103,12 @@ public Object resolveArgument(final MethodParameter parameter,
private String getRequiredPathVariable(final MethodParameter parameter,
final NativeWebRequest nativeWebRequest,
final String variableName)
throws MissingPathVariableException
throws MissingPathVariableException
{
// Check @PathVariable parameter.
@SuppressWarnings("unchecked")
final Map<String, String> uriTemplateVars = (Map<String, String>) nativeWebRequest.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE,
RequestAttributes.SCOPE_REQUEST);
final Map<String, String> uriTemplateVars = (Map<String, String>) nativeWebRequest.getAttribute(
HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST);
if (MapUtils.isNotEmpty(uriTemplateVars))
{
final String pathVariable = uriTemplateVars.get(variableName);
Expand Down Expand Up @@ -149,4 +149,4 @@ private Repository getRepository(final String storageId,
return storage.getRepository(repositoryId);
}

}
}
114 changes: 98 additions & 16 deletions ...core/src/test/java/org/carlspring/strongbox/web/RepositoryMethodArgumentResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.carlspring.strongbox.storage.repository.Repository;
import org.carlspring.strongbox.storage.repository.RepositoryData;
import org.carlspring.strongbox.storage.repository.RepositoryDto;
import org.carlspring.strongbox.storage.repository.RepositoryStatusEnum;

import javax.inject.Inject;

Expand Down Expand Up @@ -75,9 +76,12 @@ public void testSupportsParameter()
boolean supportsParameterTrueCase1 = repositoryMethodArgumentResolver.supportsParameter(getMethodParameter("uploadArtifactWithRepositoryMapping",
Repository.class));
assertTrue(supportsParameterTrueCase1);
boolean supportsParameterTrueCase2 = repositoryMethodArgumentResolver.supportsParameter(getMethodParameter("uploadArtifactForRepositoryNotInService",
boolean supportsParameterTrueCase2 = repositoryMethodArgumentResolver.supportsParameter(getMethodParameter("uploadArtifactWithOutOfServiceRepositoryAllowed",
Repository.class));
assertTrue(supportsParameterTrueCase2);
boolean supportsParameterTrueCase3 = repositoryMethodArgumentResolver.supportsParameter(getMethodParameter("uploadArtifactWithOutOfServiceRepositoryNotAllowed",
Repository.class));
assertTrue(supportsParameterTrueCase3);
boolean supportsParameterFalseCase1 = repositoryMethodArgumentResolver.supportsParameter(getMethodParameter("uploadArtifactWithoutRepositoryMapping",
Repository.class));
assertFalse(supportsParameterFalseCase1);
Expand All @@ -91,71 +95,144 @@ public void testResolveArgument()
throws MissingPathVariableException
{

mockStoragesAndRepositories();

Map<String, String> uriTemplateVars = new HashMap<>();
uriTemplateVars.put("storageId", "storage-pypi");
uriTemplateVars.put("repositoryId", "pypi-releases");
nativeWebRequest.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars,
RequestAttributes.SCOPE_REQUEST);

MethodParameter uploadArtifactWithRepositoryMappingMethod = getMethodParameter("uploadArtifactWithRepositoryMapping",
Repository.class);
Repository repository = (Repository) repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithRepositoryMappingMethod,
mockStoragesAndRepositories(RepositoryStatusEnum.OUT_OF_SERVICE);

MethodParameter uploadArtifactWithOutOfServiceRepositoryAllowedMethod = getMethodParameter("uploadArtifactWithOutOfServiceRepositoryAllowed",
Repository.class);
Repository repository = (Repository) repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory);
assertNotNull(repository);

mockStoragesAndRepositories(RepositoryStatusEnum.IN_SERVICE);
repository = (Repository) repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory);
assertNotNull(repository);

mockStoragesAndRepositories(RepositoryStatusEnum.IN_SERVICE);
MethodParameter uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod = getMethodParameter("uploadArtifactWithOutOfServiceRepositoryNotAllowed",
Repository.class);
repository = (Repository) repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory);
assertNotNull(repository);

}

@Test
public void testResolveArgumentThrowingServiceUnavailableException()
throws MissingPathVariableException
{
Map<String, String> uriTemplateVars = new HashMap<>();
uriTemplateVars.put("storageId", "storage-pypi");
uriTemplateVars.put("repositoryId", "pypi-releases");
nativeWebRequest.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars,
RequestAttributes.SCOPE_REQUEST);

mockStoragesAndRepositories(RepositoryStatusEnum.OUT_OF_SERVICE);

// ServiceUnavailableException Test Case
MethodParameter uploadArtifactForRepositoryNotInServiceMethod = getMethodParameter("uploadArtifactForRepositoryNotInService",
Repository.class);
MethodParameter uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod = getMethodParameter("uploadArtifactWithOutOfServiceRepositoryNotAllowed",
Repository.class);
ServiceUnavailableException serviceUnavailableException = Assertions.assertThrows(ServiceUnavailableException.class,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactForRepositoryNotInServiceMethod,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory));
assertEquals(String.format(RepositoryMethodArgumentResolver.NOT_IN_SERVICE_REPOSITORY_MESSAGE,
uriTemplateVars.get("storageId"), uriTemplateVars.get("repositoryId")),
serviceUnavailableException.getMessage());

// StorageNotFoundException Test Case
}

@Test
public void testResolveArgumentThrowingStorageNotFoundException()
throws MissingPathVariableException
{
Map<String, String> uriTemplateVars = new HashMap<>();
uriTemplateVars.put("storageId", "storage-nuget");
uriTemplateVars.put("repositoryId", "pypi-releases");
nativeWebRequest.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars,
RequestAttributes.SCOPE_REQUEST);

mockStoragesAndRepositories(RepositoryStatusEnum.IN_SERVICE);

// StorageNotFoundException Test Case
MethodParameter uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod = getMethodParameter("uploadArtifactWithOutOfServiceRepositoryNotAllowed",
Repository.class);
StorageNotFoundException storageNotFoundException = Assertions.assertThrows(StorageNotFoundException.class,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithRepositoryMappingMethod,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory));
assertEquals(String.format(RepositoryMethodArgumentResolver.NOT_FOUND_STORAGE_MESSAGE,
uriTemplateVars.get("storageId")),
storageNotFoundException.getMessage());

// RepositoryNotFoundException Test Case
mockStoragesAndRepositories(RepositoryStatusEnum.OUT_OF_SERVICE);
storageNotFoundException = Assertions.assertThrows(StorageNotFoundException.class,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory));
assertEquals(String.format(RepositoryMethodArgumentResolver.NOT_FOUND_STORAGE_MESSAGE,
uriTemplateVars.get("storageId")),
storageNotFoundException.getMessage());
}

@Test
public void testResolveArgumentThrowingRepositoryNotFoundException()
throws MissingPathVariableException
{
Map<String, String> uriTemplateVars = new HashMap<>();
uriTemplateVars.put("storageId", "storage-pypi");
uriTemplateVars.put("repositoryId", "maven-releases");
nativeWebRequest.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, uriTemplateVars,
RequestAttributes.SCOPE_REQUEST);

mockStoragesAndRepositories(RepositoryStatusEnum.IN_SERVICE);

// RepositoryNotFoundException Test Case
MethodParameter uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod = getMethodParameter("uploadArtifactWithOutOfServiceRepositoryNotAllowed",
Repository.class);
RepositoryNotFoundException repositoryNotFoundException = Assertions.assertThrows(RepositoryNotFoundException.class,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithRepositoryMappingMethod,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory));
assertEquals(String.format(RepositoryMethodArgumentResolver.NOT_FOUND_REPOSITORY_MESSAGE,
uriTemplateVars.get("storageId"), uriTemplateVars.get("repositoryId")),
repositoryNotFoundException.getMessage());

mockStoragesAndRepositories(RepositoryStatusEnum.OUT_OF_SERVICE);
repositoryNotFoundException = Assertions.assertThrows(RepositoryNotFoundException.class,
() -> repositoryMethodArgumentResolver.resolveArgument(uploadArtifactWithOutOfServiceRepositoryNotAllowedMethod,
modelAndViewContainer,
nativeWebRequest,
webDataBinderFactory));
assertEquals(String.format(RepositoryMethodArgumentResolver.NOT_FOUND_REPOSITORY_MESSAGE,
uriTemplateVars.get("storageId"), uriTemplateVars.get("repositoryId")),
repositoryNotFoundException.getMessage());
}

private void mockStoragesAndRepositories()
private void mockStoragesAndRepositories(RepositoryStatusEnum repositoryStatus)
{
Map<String, RepositoryDto> repositories = new HashMap<>();
Map<String, StorageDto> storages = new HashMap<>();

RepositoryDto repositoryDto = new RepositoryDto();
repositoryDto.setId("pypi-releases");
repositoryDto.setStatus(repositoryStatus.getStatus());
repositories.put(repositoryDto.getId(), repositoryDto);

StorageDto storageDto = new StorageDto();
Expand Down Expand Up @@ -198,8 +275,13 @@ void uploadArtifactWithRepositoryMappingAndInvalidRepository(@RepositoryMapping
{
}

@PutMapping(path = "/{storageId}/{repositoryId}/uploadArtifactForRepositoryNotInService")
void uploadArtifactForRepositoryNotInService(@RepositoryMapping(inServiceRepository = false) Repository repository)
@PutMapping(path = "/{storageId}/{repositoryId}/uploadArtifactWithOutOfServiceRepositoryAllowed")
void uploadArtifactWithOutOfServiceRepositoryAllowed(@RepositoryMapping(allowOutOfServiceRepository = true) Repository repository)
{
}

@PutMapping(path = "/{storageId}/{repositoryId}/uploadArtifactWithOutOfServiceRepositoryNotAllowed")
void uploadArtifactWithOutOfServiceRepositoryNotAllowed(@RepositoryMapping(allowOutOfServiceRepository = false) Repository repository)
{
}

Expand Down

0 comments on commit 9bb9026

Please sign in to comment.