Use optionals for get by name and namespace

This commit is contained in:
René Pfeuffer
2018-07-11 13:42:09 +02:00
parent de3848b662
commit 617b98c6f3
5 changed files with 31 additions and 22 deletions

View File

@@ -41,6 +41,7 @@ import sonia.scm.TypeManager;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import java.io.IOException; import java.io.IOException;
import java.util.Collection; import java.util.Collection;
import java.util.Optional;
//~--- JDK imports ------------------------------------------------------------ //~--- JDK imports ------------------------------------------------------------
@@ -148,11 +149,10 @@ public interface RepositoryManager
@Override @Override
public RepositoryHandler getHandler(String type); public RepositoryHandler getHandler(String type);
default Repository getByNamespace(String namespace, String name) { default Optional<Repository> getByNamespace(String namespace, String name) {
return getAll() return getAll()
.stream() .stream()
.filter(r -> r.getName().equals(name) && r.getNamespace().equals(namespace)) .filter(r -> r.getName().equals(name) && r.getNamespace().equals(namespace))
.findFirst() .findFirst();
.orElse(null);
} }
} }

View File

@@ -7,6 +7,7 @@ import sonia.scm.PageResult;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import java.io.IOException; import java.io.IOException;
import java.util.Optional;
import java.util.function.Function; import java.util.function.Function;
import java.util.function.Predicate; import java.util.function.Predicate;
import java.util.function.Supplier; import java.util.function.Supplier;
@@ -55,8 +56,8 @@ class IdResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
return singleAdapter.delete(id); return singleAdapter.delete(id);
} }
private Supplier<MODEL_OBJECT> loadBy(String id) { private Supplier<Optional<MODEL_OBJECT>> loadBy(String id) {
return () -> manager.get(id); return () -> Optional.ofNullable(manager.get(id));
} }
private Predicate<MODEL_OBJECT> idStaysTheSame(String id) { private Predicate<MODEL_OBJECT> idStaysTheSame(String id) {

View File

@@ -18,6 +18,7 @@ import javax.ws.rs.Path;
import javax.ws.rs.PathParam; import javax.ws.rs.PathParam;
import javax.ws.rs.Produces; import javax.ws.rs.Produces;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import java.util.Optional;
import java.util.function.Predicate; import java.util.function.Predicate;
import java.util.function.Supplier; import java.util.function.Supplier;
@@ -153,7 +154,7 @@ public class RepositoryResource {
return permissionRootResource.get(); return permissionRootResource.get();
} }
private Supplier<Repository> loadBy(String namespace, String name) { private Supplier<Optional<Repository>> loadBy(String namespace, String name) {
return () -> manager.getByNamespace(namespace, name); return () -> manager.getByNamespace(namespace, name);
} }

View File

@@ -8,6 +8,7 @@ import sonia.scm.api.rest.resources.AbstractManagerResource;
import javax.ws.rs.core.GenericEntity; import javax.ws.rs.core.GenericEntity;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import java.util.Collection; import java.util.Collection;
import java.util.Optional;
import java.util.function.Function; import java.util.function.Function;
import java.util.function.Predicate; import java.util.function.Predicate;
import java.util.function.Supplier; import java.util.function.Supplier;
@@ -38,34 +39,35 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
* Reads the model object for the given id, transforms it to a dto and returns a corresponding http response. * Reads the model object for the given id, transforms it to a dto and returns a corresponding http response.
* This handles all corner cases, eg. no matching object for the id or missing privileges. * This handles all corner cases, eg. no matching object for the id or missing privileges.
*/ */
Response get(Supplier<MODEL_OBJECT> reader, Function<MODEL_OBJECT, DTO> mapToDto) { Response get(Supplier<Optional<MODEL_OBJECT>> reader, Function<MODEL_OBJECT, DTO> mapToDto) {
MODEL_OBJECT modelObject = reader.get(); return reader.get()
if (modelObject == null) { .map(mapToDto)
return Response.status(Response.Status.NOT_FOUND).build(); .map(Response::ok)
} .map(Response.ResponseBuilder::build)
DTO dto = mapToDto.apply(modelObject); .orElse(Response.status(Response.Status.NOT_FOUND).build());
return Response.ok(dto).build();
} }
/** /**
* Update the model object for the given id according to the given function and returns a corresponding http response. * Update the model object for the given id according to the given function and returns a corresponding http response.
* This handles all corner cases, eg. no matching object for the id or missing privileges. * This handles all corner cases, eg. no matching object for the id or missing privileges.
*/ */
public Response update(Supplier<MODEL_OBJECT> reader, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges, Predicate<MODEL_OBJECT> hasSameKey) { public Response update(Supplier<Optional<MODEL_OBJECT>> reader, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges, Predicate<MODEL_OBJECT> hasSameKey) {
MODEL_OBJECT existingModelObject = reader.get(); Optional<MODEL_OBJECT> existingModelObject = reader.get();
if (existingModelObject == null) { if (!existingModelObject.isPresent()) {
return Response.status(Response.Status.NOT_FOUND).build(); return Response.status(Response.Status.NOT_FOUND).build();
} }
MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject); MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject.get());
if (!hasSameKey.test(changedModelObject)) { if (!hasSameKey.test(changedModelObject)) {
return Response.status(BAD_REQUEST).entity("illegal change of id").build(); return Response.status(BAD_REQUEST).entity("illegal change of id").build();
} }
return update(getId(existingModelObject), changedModelObject); return update(getId(existingModelObject.get()), changedModelObject);
} }
public Response delete(Supplier<MODEL_OBJECT> reader) { public Response delete(Supplier<Optional<MODEL_OBJECT>> reader) {
MODEL_OBJECT existingModelObject = reader.get(); return reader.get()
return delete(existingModelObject.getId()); .map(MODEL_OBJECT::getId)
.map(this::delete)
.orElse(null);
} }
@Override @Override

View File

@@ -25,6 +25,8 @@ import java.net.URISyntaxException;
import java.net.URL; import java.net.URL;
import static java.util.Collections.singletonList; import static java.util.Collections.singletonList;
import static java.util.Optional.empty;
import static java.util.Optional.of;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
@@ -33,6 +35,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyObject;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq; import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
@@ -74,6 +77,7 @@ public class RepositoryRootResourceTest {
@Test @Test
public void shouldFailForNotExistingRepository() throws URISyntaxException { public void shouldFailForNotExistingRepository() throws URISyntaxException {
when(repositoryManager.getByNamespace(anyString(), anyString())).thenReturn(empty());
mockRepository("space", "repo"); mockRepository("space", "repo");
MockHttpRequest request = MockHttpRequest.get("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/other"); MockHttpRequest request = MockHttpRequest.get("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/other");
@@ -115,6 +119,7 @@ public class RepositoryRootResourceTest {
public void shouldHandleUpdateForNotExistingRepository() throws URISyntaxException, IOException { public void shouldHandleUpdateForNotExistingRepository() throws URISyntaxException, IOException {
URL url = Resources.getResource("sonia/scm/api/v2/repository-test-update.json"); URL url = Resources.getResource("sonia/scm/api/v2/repository-test-update.json");
byte[] repository = Resources.toByteArray(url); byte[] repository = Resources.toByteArray(url);
when(repositoryManager.getByNamespace(anyString(), anyString())).thenReturn(empty());
MockHttpRequest request = MockHttpRequest MockHttpRequest request = MockHttpRequest
.put("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo") .put("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo")
@@ -212,7 +217,7 @@ public class RepositoryRootResourceTest {
repository.setName(name); repository.setName(name);
String id = namespace + "-" + name; String id = namespace + "-" + name;
repository.setId(id); repository.setId(id);
when(repositoryManager.getByNamespace(namespace, name)).thenReturn(repository); when(repositoryManager.getByNamespace(namespace, name)).thenReturn(of(repository));
when(repositoryManager.get(id)).thenReturn(repository); when(repositoryManager.get(id)).thenReturn(repository);
return repository; return repository;
} }