Create error response for concurrent modifications

This commit is contained in:
René Pfeuffer
2018-10-25 13:48:00 +02:00
parent bda5b41271
commit d185743ef0
10 changed files with 57 additions and 17 deletions

View File

@@ -1,4 +1,34 @@
package sonia.scm; package sonia.scm;
public class ConcurrentModificationException extends Exception { import java.util.Collections;
import java.util.List;
import static java.util.Collections.unmodifiableList;
import static java.util.stream.Collectors.joining;
public class ConcurrentModificationException extends RuntimeException {
private final List<ContextEntry> context;
public ConcurrentModificationException(Class type, String id) {
this(Collections.singletonList(new ContextEntry(type, id)));
}
public ConcurrentModificationException(String type, String id) {
this(Collections.singletonList(new ContextEntry(type, id)));
}
private ConcurrentModificationException(List<ContextEntry> context) {
super(createMessage(context));
this.context = context;
}
public List<ContextEntry> getContext() {
return unmodifiableList(context);
}
private static String createMessage(List<ContextEntry> context) {
return context.stream()
.map(c -> c.getType().toLowerCase() + " with id " + c.getId())
.collect(joining(" in ", "", " has been modified concurrently"));
}
} }

View File

@@ -15,14 +15,15 @@ public class NotFoundException extends RuntimeException {
private final List<ContextEntry> context; private final List<ContextEntry> context;
public NotFoundException(Class type, String id) { public NotFoundException(Class type, String id) {
this.context = Collections.singletonList(new ContextEntry(type, id)); this(Collections.singletonList(new ContextEntry(type, id)));
} }
public NotFoundException(String type, String id) { public NotFoundException(String type, String id) {
this.context = Collections.singletonList(new ContextEntry(type, id)); this(Collections.singletonList(new ContextEntry(type, id)));
} }
private NotFoundException(List<ContextEntry> context) { private NotFoundException(List<ContextEntry> context) {
super(createMessage(context));
this.context = context; this.context = context;
} }
@@ -46,8 +47,7 @@ public class NotFoundException extends RuntimeException {
return unmodifiableList(context); return unmodifiableList(context);
} }
@Override private static String createMessage(List<ContextEntry> context) {
public String getMessage() {
return context.stream() return context.stream()
.map(c -> c.getType().toLowerCase() + " with id " + c.getId()) .map(c -> c.getType().toLowerCase() + " with id " + c.getId())
.collect(joining(" in ", "could not find ", "")); .collect(joining(" in ", "could not find ", ""));

View File

@@ -97,7 +97,7 @@ public abstract class UserManagerTestBase extends ManagerTestBase<User> {
} }
@Test(expected = NotFoundException.class) @Test(expected = NotFoundException.class)
public void testDeleteNotFound() throws Exception { public void testDeleteNotFound() {
manager.delete(UserTestData.createDent()); manager.delete(UserTestData.createDent());
} }
@@ -181,7 +181,7 @@ public abstract class UserManagerTestBase extends ManagerTestBase<User> {
} }
@Test @Test
public void testModify() throws AlreadyExistsException, NotFoundException, ConcurrentModificationException { public void testModify() throws AlreadyExistsException {
User zaphod = UserTestData.createZaphod(); User zaphod = UserTestData.createZaphod();
manager.create(zaphod); manager.create(zaphod);
@@ -238,7 +238,7 @@ public abstract class UserManagerTestBase extends ManagerTestBase<User> {
} }
@Test @Test
public void testRefresh() throws AlreadyExistsException, NotFoundException { public void testRefresh() throws AlreadyExistsException {
User zaphod = UserTestData.createZaphod(); User zaphod = UserTestData.createZaphod();
manager.create(zaphod); manager.create(zaphod);
@@ -299,7 +299,7 @@ public abstract class UserManagerTestBase extends ManagerTestBase<User> {
return user; return user;
} }
private void modifyAndDeleteUser(User user) throws IOException, NotFoundException, ConcurrentModificationException { private void modifyAndDeleteUser(User user) {
String name = user.getName(); String name = user.getName();
String nd = name.concat(" new displayname"); String nd = name.concat(" new displayname");

View File

@@ -1,6 +1,8 @@
package sonia.scm.api.rest; package sonia.scm.api.rest;
import sonia.scm.ConcurrentModificationException; import sonia.scm.ConcurrentModificationException;
import sonia.scm.api.v2.resources.ErrorDto;
import sonia.scm.web.VndMediaType;
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.ExceptionMapper;
@@ -10,6 +12,6 @@ import javax.ws.rs.ext.Provider;
public class ConcurrentModificationExceptionMapper implements ExceptionMapper<ConcurrentModificationException> { public class ConcurrentModificationExceptionMapper implements ExceptionMapper<ConcurrentModificationException> {
@Override @Override
public Response toResponse(ConcurrentModificationException exception) { public Response toResponse(ConcurrentModificationException exception) {
return Response.status(Response.Status.CONFLICT).build(); return Response.status(Response.Status.CONFLICT).entity(ErrorDto.from(exception)).type(VndMediaType.ERROR_TYPE).build();
} }
} }

View File

@@ -3,6 +3,7 @@ package sonia.scm.api.v2.resources;
import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude;
import lombok.Getter; import lombok.Getter;
import org.slf4j.MDC; import org.slf4j.MDC;
import sonia.scm.ConcurrentModificationException;
import sonia.scm.ContextEntry; import sonia.scm.ContextEntry;
import sonia.scm.NotFoundException; import sonia.scm.NotFoundException;
@@ -32,4 +33,8 @@ public class ErrorDto {
static ErrorDto from(NotFoundException notFoundException) { static ErrorDto from(NotFoundException notFoundException) {
return new ErrorDto(MDC.get("transaction_id"), "todo", notFoundException.getContext(), notFoundException.getMessage()); return new ErrorDto(MDC.get("transaction_id"), "todo", notFoundException.getContext(), notFoundException.getMessage());
} }
public static ErrorDto from(ConcurrentModificationException concurrentModificationException) {
return new ErrorDto(MDC.get("transaction_id"), "todo", concurrentModificationException.getContext(), concurrentModificationException.getMessage());
}
} }

View File

@@ -98,7 +98,7 @@ public class GroupResource {
@ResponseCode(code = 500, condition = "internal server error") @ResponseCode(code = 500, condition = "internal server error")
}) })
@TypeHint(TypeHint.NO_CONTENT.class) @TypeHint(TypeHint.NO_CONTENT.class)
public Response update(@PathParam("id") String name, @Valid GroupDto groupDto) throws ConcurrentModificationException { public Response update(@PathParam("id") String name, @Valid GroupDto groupDto) {
return adapter.update(name, existing -> dtoToGroupMapper.map(groupDto)); return adapter.update(name, existing -> dtoToGroupMapper.map(groupDto));
} }
} }

View File

@@ -39,7 +39,7 @@ class IdResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
return singleAdapter.get(loadBy(id), mapToDto); return singleAdapter.get(loadBy(id), mapToDto);
} }
public Response update(String id, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges) throws ConcurrentModificationException { public Response update(String id, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges) {
return singleAdapter.update( return singleAdapter.update(
loadBy(id), loadBy(id),
applyChanges, applyChanges,

View File

@@ -138,7 +138,7 @@ public class RepositoryResource {
@ResponseCode(code = 500, condition = "internal server error") @ResponseCode(code = 500, condition = "internal server error")
}) })
@TypeHint(TypeHint.NO_CONTENT.class) @TypeHint(TypeHint.NO_CONTENT.class)
public Response update(@PathParam("namespace") String namespace, @PathParam("name") String name, @Valid RepositoryDto repositoryDto) throws ConcurrentModificationException { public Response update(@PathParam("namespace") String namespace, @PathParam("name") String name, @Valid RepositoryDto repositoryDto) {
return adapter.update( return adapter.update(
loadBy(namespace, name), loadBy(namespace, name),
existing -> processUpdate(repositoryDto, existing), existing -> processUpdate(repositoryDto, existing),
@@ -204,7 +204,8 @@ public class RepositoryResource {
} }
private Supplier<Repository> loadBy(String namespace, String name) { private Supplier<Repository> loadBy(String namespace, String name) {
return () -> Optional.ofNullable(manager.get(new NamespaceAndName(namespace, name))).orElseThrow(() -> new NotFoundException(Repository.class, namespace + "/" + name)); NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name);
return () -> Optional.ofNullable(manager.get(namespaceAndName)).orElseThrow(() -> NotFoundException.notFound(namespaceAndName).build());
} }
private Predicate<Repository> nameAndNamespaceStaysTheSame(String namespace, String name) { private Predicate<Repository> nameAndNamespaceStaysTheSame(String namespace, String name) {

View File

@@ -33,6 +33,7 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
DTO extends HalRepresentation> extends AbstractManagerResource<MODEL_OBJECT> { DTO extends HalRepresentation> extends AbstractManagerResource<MODEL_OBJECT> {
private final Function<Throwable, Optional<Response>> errorHandler; private final Function<Throwable, Optional<Response>> errorHandler;
private final Class<MODEL_OBJECT> type;
SingleResourceManagerAdapter(Manager<MODEL_OBJECT> manager, Class<MODEL_OBJECT> type) { SingleResourceManagerAdapter(Manager<MODEL_OBJECT> manager, Class<MODEL_OBJECT> type) {
this(manager, type, e -> Optional.empty()); this(manager, type, e -> Optional.empty());
@@ -44,6 +45,7 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
Function<Throwable, Optional<Response>> errorHandler) { Function<Throwable, Optional<Response>> errorHandler) {
super(manager, type); super(manager, type);
this.errorHandler = errorHandler; this.errorHandler = errorHandler;
this.type = type;
} }
/** /**
@@ -58,14 +60,14 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
* 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.
*/ */
Response update(Supplier<MODEL_OBJECT> reader, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges, Predicate<MODEL_OBJECT> hasSameKey) throws ConcurrentModificationException { Response update(Supplier<MODEL_OBJECT> reader, Function<MODEL_OBJECT, MODEL_OBJECT> applyChanges, Predicate<MODEL_OBJECT> hasSameKey) {
MODEL_OBJECT existingModelObject = reader.get(); MODEL_OBJECT existingModelObject = reader.get();
MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject); MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject);
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();
} }
else if (modelObjectWasModifiedConcurrently(existingModelObject, changedModelObject)) { else if (modelObjectWasModifiedConcurrently(existingModelObject, changedModelObject)) {
throw new ConcurrentModificationException(); throw new ConcurrentModificationException(type, existingModelObject.getId());
} }
return update(getId(existingModelObject), changedModelObject); return update(getId(existingModelObject), changedModelObject);
} }

View File

@@ -101,7 +101,7 @@ public class UserResource {
@ResponseCode(code = 500, condition = "internal server error") @ResponseCode(code = 500, condition = "internal server error")
}) })
@TypeHint(TypeHint.NO_CONTENT.class) @TypeHint(TypeHint.NO_CONTENT.class)
public Response update(@PathParam("id") String name, @Valid UserDto userDto) throws ConcurrentModificationException { public Response update(@PathParam("id") String name, @Valid UserDto userDto) {
return adapter.update(name, existing -> dtoToUserMapper.map(userDto, existing.getPassword())); return adapter.update(name, existing -> dtoToUserMapper.map(userDto, existing.getPassword()));
} }