Merge base branch into feature

scm-core/src/main/java/sonia/scm/HandlerBase.java

@@ -55,7 +55,7 @@ public interface HandlerBase<T extends TypedObject, E extends Exception>
    *
    * @return  The persisted object.
    */
-  public T create(T object) throws E, IOException;
+  public T create(T object) throws E;
 
   /**
    * Removes a persistent object.
@@ -66,7 +66,7 @@ public interface HandlerBase<T extends TypedObject, E extends Exception>
    * @throws E
    * @throws IOException
    */
-  public void delete(T object) throws E, IOException;
+  public void delete(T object) throws E;
 
   /**
    * Modifies a persistent object.
@@ -77,5 +77,5 @@ public interface HandlerBase<T extends TypedObject, E extends Exception>
    * @throws E
    * @throws IOException
    */
-  public void modify(T object) throws E, IOException;
+  public void modify(T object) throws E;
 }

scm-core/src/main/java/sonia/scm/Manager.java

@@ -33,7 +33,6 @@
 
 package sonia.scm;
 
-import java.io.IOException;
 import java.util.Collection;
 import java.util.Comparator;
 
@@ -56,9 +55,8 @@ public interface Manager<T extends ModelObject, E extends Exception>
    * @param object to refresh
    *
    * @throws E
-   * @throws IOException
    */
-  void refresh(T object) throws E, IOException;
+  void refresh(T object) throws E;
 
   //~--- get methods ----------------------------------------------------------
 

scm-core/src/main/java/sonia/scm/ManagerDecorator.java

@@ -77,7 +77,7 @@ public class ManagerDecorator<T extends ModelObject, E extends Exception>
    * {@inheritDoc}
    */
   @Override
-  public T create(T object) throws E, IOException
+  public T create(T object) throws E
   {
     return decorated.create(object);
   }
@@ -86,7 +86,7 @@ public class ManagerDecorator<T extends ModelObject, E extends Exception>
    * {@inheritDoc}
    */
   @Override
-  public void delete(T object) throws E, IOException
+  public void delete(T object) throws E
   {
     decorated.delete(object);
   }
@@ -104,7 +104,7 @@ public class ManagerDecorator<T extends ModelObject, E extends Exception>
    * {@inheritDoc}
    */
   @Override
-  public void modify(T object) throws E, IOException
+  public void modify(T object) throws E
   {
     decorated.modify(object);
   }
@@ -113,7 +113,7 @@ public class ManagerDecorator<T extends ModelObject, E extends Exception>
    * {@inheritDoc}
    */
   @Override
-  public void refresh(T object) throws E, IOException
+  public void refresh(T object) throws E
   {
     decorated.refresh(object);
   }

scm-core/src/main/java/sonia/scm/group/GroupAlreadyExistsException.java

@@ -48,9 +48,9 @@ public class GroupAlreadyExistsException extends GroupException
   /**
    * Constructs a new instance.
    * 
-   * @param message exception message
+   * @param name The name (aka id) of the group
    */
-  public GroupAlreadyExistsException(String message) {
+  public GroupAlreadyExistsException(String name) {
-    super(message);
+    super(name + " group already exists");
   }
 }

scm-core/src/main/java/sonia/scm/group/GroupNotFoundException.java

@@ -52,39 +52,7 @@ public class GroupNotFoundException extends GroupException
    * Constructs a new GroupNotFoundException.
    *
    */
-  public GroupNotFoundException() {}
+  public GroupNotFoundException() {
-
+    super("group does not exists");
-  /**
-   * Constructs a new GroupNotFoundException.
-   *
-   *
-   * @param message message for the exception
-   */
-  public GroupNotFoundException(String message)
-  {
-    super(message);
-  }
-
-  /**
-   * Constructs a new GroupNotFoundException.
-   *
-   *
-   * @param throwable root cause
-   */
-  public GroupNotFoundException(Throwable throwable)
-  {
-    super(throwable);
-  }
-
-  /**
-   * Constructs a new GroupNotFoundException.
-   *
-   *
-   * @param message message for the exception
-   * @param throwable root cause
-   */
-  public GroupNotFoundException(String message, Throwable throwable)
-  {
-    super(message, throwable);
   }
 }

scm-core/src/main/java/sonia/scm/repository/AbstractSimpleRepositoryHandler.java

@@ -106,7 +106,7 @@ public abstract class AbstractSimpleRepositoryHandler<C extends SimpleRepository
    */
   @Override
   public Repository create(Repository repository)
-          throws RepositoryException, IOException
+          throws RepositoryException
   {
     File directory = getDirectory(repository);
 
@@ -135,11 +135,14 @@ public abstract class AbstractSimpleRepositoryHandler<C extends SimpleRepository
               directory);
         }
 
-        fileSystem.destroy(directory);
+        try {
+          fileSystem.destroy(directory);
+        } catch (IOException e) {
+          logger.error("could not delete directory after failed repository creation: {}", directory, e);
+        }
       }
 
-      Throwables.propagateIfPossible(ex, RepositoryException.class,
+      Throwables.propagateIfPossible(ex, RepositoryException.class);
-                                     IOException.class);
       return null;
     }
   }
@@ -172,14 +175,17 @@ public abstract class AbstractSimpleRepositoryHandler<C extends SimpleRepository
    * @throws RepositoryException
    */
   @Override
-  public void delete(Repository repository)
+  public void delete(Repository repository) throws RepositoryException
-          throws RepositoryException, IOException
   {
     File directory = getDirectory(repository);
 
     if (directory.exists())
     {
-      fileSystem.destroy(directory);
+      try {
+        fileSystem.destroy(directory);
+      } catch (IOException e) {
+        throw new RepositoryException("could not delete repository", e);
+      }
       cleanupEmptyDirectories(config.getRepositoryDirectory(),
                               directory.getParentFile());
     }
@@ -231,8 +237,7 @@ public abstract class AbstractSimpleRepositoryHandler<C extends SimpleRepository
    * @throws RepositoryException
    */
   @Override
-  public void modify(Repository repository)
+  public void modify(Repository repository) throws RepositoryException
-          throws RepositoryException, IOException
   {
 
     // nothing todo

scm-core/src/main/java/sonia/scm/security/SyncingRealmHelper.java

@@ -30,12 +30,10 @@ package sonia.scm.security;
 
 import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
-
 import org.apache.shiro.authc.AuthenticationException;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.SimpleAuthenticationInfo;
 import org.apache.shiro.subject.SimplePrincipalCollection;
-
 import sonia.scm.group.Group;
 import sonia.scm.group.GroupException;
 import sonia.scm.group.GroupManager;
@@ -46,9 +44,6 @@ import sonia.scm.user.UserException;
 import sonia.scm.user.UserManager;
 import sonia.scm.web.security.AdministrationContext;
 
-
-import java.io.IOException;
-
 import java.util.Collection;
 
 /**
@@ -134,7 +129,7 @@ public final class SyncingRealmHelper {
           groupManager.create(group);
         }
       }
-      catch (GroupException | IOException ex) {
+      catch (GroupException ex) {
         throw new AuthenticationException("could not store group", ex);
       }
     });
@@ -155,7 +150,7 @@ public final class SyncingRealmHelper {
           userManager.create(user);
         }
       }
-      catch (UserException | IOException ex) {
+      catch (UserException ex) {
         throw new AuthenticationException("could not store user", ex);
       }
     });

scm-core/src/main/java/sonia/scm/user/UserAlreadyExistsException.java

@@ -49,11 +49,11 @@ public class UserAlreadyExistsException extends UserException
   /**
    * Constructs a new instance.
    *
-   * @param message message of exception
+   * @param name The name (aka id) of the user
    * @since 1.5
    */
-  public UserAlreadyExistsException(String message)
+  public UserAlreadyExistsException(String name)
   {
-    super(message);
+    super(name + " user already exists");
   }
 }

scm-core/src/main/java/sonia/scm/user/UserNotFoundException.java

@@ -51,39 +51,7 @@ public class UserNotFoundException extends UserException
    * Constructs a new UserNotFoundException.
    *
    */
-  public UserNotFoundException() {}
+  public UserNotFoundException() {
-
+    super("user does not exists");
-  /**
-   * Constructs a new UserNotFoundException.
-   *
-   *
-   * @param message message for the exception
-   */
-  public UserNotFoundException(String message)
-  {
-    super(message);
-  }
-
-  /**
-   * Constructs a new UserNotFoundException.
-   *
-   *
-   * @param throwable root cause
-   */
-  public UserNotFoundException(Throwable throwable)
-  {
-    super(throwable);
-  }
-
-  /**
-   * Constructs a new UserNotFoundException.
-   *
-   *
-   * @param message message for the exception
-   * @param throwable root cause
-   */
-  public UserNotFoundException(String message, Throwable throwable)
-  {
-    super(message, throwable);
   }
 }

scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java

@@ -55,6 +55,11 @@ import javax.ws.rs.core.Request;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 import javax.ws.rs.core.UriInfo;
+import java.beans.BeanInfo;
+import java.beans.IntrospectionException;
+import java.beans.Introspector;
+import java.beans.PropertyDescriptor;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.Date;
@@ -76,17 +81,15 @@ public abstract class AbstractManagerResource<T extends ModelObject,
   private static final Logger logger =
     LoggerFactory.getLogger(AbstractManagerResource.class);
 
-  //~--- constructors ---------------------------------------------------------
+  protected final Manager<T, E> manager;
+  private final Class<T> type;
 
-  /**
+  protected int cacheMaxAge = 0;
-   * Constructs ...
+  protected boolean disableCache = false;
-   *
+
-   *
+  public AbstractManagerResource(Manager<T, E> manager, Class<T> type) {
-   * @param manager
-   */
-  public AbstractManagerResource(Manager<T, E> manager)
-  {
     this.manager = manager;
+    this.type = type;
   }
 
   //~--- methods --------------------------------------------------------------
@@ -526,45 +529,25 @@ public abstract class AbstractManagerResource<T extends ModelObject,
     return builder.build();
   }
 
-  /**
-   * Method description
-   *
-   *
-   * @param sortby
-   * @param desc
-   *
-   * @return
-   */
   @SuppressWarnings("unchecked")
-  private Comparator<T> createComparator(String sortby, boolean desc)
+  private Comparator<T> createComparator(String sortBy, boolean desc)
   {
+    checkSortByField(sortBy);
     Comparator comparator;
 
     if (desc)
     {
-      comparator = new BeanReverseComparator(sortby);
+      comparator = new BeanReverseComparator(sortBy);
     }
     else
     {
-      comparator = new BeanComparator(sortby);
+      comparator = new BeanComparator(sortBy);
     }
 
     return comparator;
   }
 
-  /**
+  private Collection<T> fetchItems(String sortBy, boolean desc, int start,
-   * Method description
-   *
-   *
-   *
-   * @param sortby
-   * @param desc
-   * @param start
-   * @param limit
-   *
-   * @return
-   */
-  private Collection<T> fetchItems(String sortby, boolean desc, int start,
     int limit)
   {
     AssertUtil.assertPositive(start);
@@ -573,18 +556,18 @@ public abstract class AbstractManagerResource<T extends ModelObject,
 
     if (limit > 0)
     {
-      if (Util.isEmpty(sortby))
+      if (Util.isEmpty(sortBy))
       {
 
         // replace with something useful
-        sortby = "id";
+        sortBy = "id";
       }
 
-      items = manager.getAll(createComparator(sortby, desc), start, limit);
+      items = manager.getAll(createComparator(sortBy, desc), start, limit);
     }
-    else if (Util.isNotEmpty(sortby))
+    else if (Util.isNotEmpty(sortBy))
     {
-      items = manager.getAll(createComparator(sortby, desc));
+      items = manager.getAll(createComparator(sortBy, desc));
     }
     else
     {
@@ -594,17 +577,32 @@ public abstract class AbstractManagerResource<T extends ModelObject,
     return items;
   }
 
-  protected PageResult<T> fetchPage(String sortby, boolean desc, int pageNumber,
+  // We have to handle IntrospectionException here, because it's a checked exception
+  // It shouldn't occur really - so creating a new unchecked exception would be over-engineered here
+  @SuppressWarnings("squid:S00112")
+  private void checkSortByField(String sortBy) {
+    try {
+      BeanInfo info = Introspector.getBeanInfo(type);
+      PropertyDescriptor[] pds = info.getPropertyDescriptors();
+      if (Arrays.stream(pds).noneMatch(p -> p.getName().equals(sortBy))) {
+        throw new IllegalArgumentException("sortBy");
+      }
+    } catch (IntrospectionException e) {
+      throw new RuntimeException("error introspecting model type " + type.getName(), e);
+    }
+  }
+
+  protected PageResult<T> fetchPage(String sortBy, boolean desc, int pageNumber,
     int pageSize) {
     AssertUtil.assertPositive(pageNumber);
     AssertUtil.assertPositive(pageSize);
 
-    if (Util.isEmpty(sortby)) {
+    if (Util.isEmpty(sortBy)) {
       // replace with something useful
-      sortby = "id";
+      sortBy = "id";
     }
 
-    return manager.getPage(createComparator(sortby, desc), pageNumber, pageSize);
+    return manager.getPage(createComparator(sortBy, desc), pageNumber, pageSize);
   }
 
   //~--- get methods ----------------------------------------------------------
@@ -676,16 +674,4 @@ public abstract class AbstractManagerResource<T extends ModelObject,
       return super.compare(o1, o2) * -1;
     }
   }
-
-
-  //~--- fields ---------------------------------------------------------------
-
-  /** Field description */
-  protected int cacheMaxAge = 0;
-
-  /** Field description */
-  protected boolean disableCache = false;
-
-  /** Field description */
-  protected Manager<T, E> manager;
 }

scm-webapp/src/main/java/sonia/scm/api/rest/resources/GroupResource.java

@@ -41,18 +41,12 @@ import com.webcohesion.enunciate.metadata.rs.ResponseCode;
 import com.webcohesion.enunciate.metadata.rs.ResponseHeader;
 import com.webcohesion.enunciate.metadata.rs.StatusCodes;
 import com.webcohesion.enunciate.metadata.rs.TypeHint;
-
 import org.apache.shiro.SecurityUtils;
-
 import sonia.scm.group.Group;
 import sonia.scm.group.GroupException;
 import sonia.scm.group.GroupManager;
 import sonia.scm.security.Role;
 
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.Collection;
-
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.DefaultValue;
@@ -69,6 +63,9 @@ import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Request;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
+import java.util.Collection;
+
+//~--- JDK imports ------------------------------------------------------------
 
 /**
  * RESTful Web Service Resource to manage groups and their members.
@@ -97,7 +94,7 @@ public class GroupResource
   @Inject
   public GroupResource(GroupManager groupManager)
   {
-    super(groupManager);
+    super(groupManager, Group.class);
   }
 
   //~--- methods --------------------------------------------------------------

scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryImportResource.java

@@ -40,12 +40,13 @@ import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.Files;
 import com.google.inject.Inject;
-
+import com.webcohesion.enunciate.metadata.rs.ResponseCode;
+import com.webcohesion.enunciate.metadata.rs.ResponseHeader;
+import com.webcohesion.enunciate.metadata.rs.StatusCodes;
+import com.webcohesion.enunciate.metadata.rs.TypeHint;
 import org.apache.shiro.SecurityUtils;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import sonia.scm.NotSupportedFeatuerException;
 import sonia.scm.Type;
 import sonia.scm.api.rest.RestActionUploadResult;
@@ -65,26 +66,6 @@ import sonia.scm.repository.api.UnbundleCommandBuilder;
 import sonia.scm.security.Role;
 import sonia.scm.util.IOUtil;
 
-import static com.google.common.base.Preconditions.*;
-
-//~--- JDK imports ------------------------------------------------------------
-
-import com.webcohesion.enunciate.metadata.rs.ResponseCode;
-import com.webcohesion.enunciate.metadata.rs.ResponseHeader;
-import com.webcohesion.enunciate.metadata.rs.StatusCodes;
-import com.webcohesion.enunciate.metadata.rs.TypeHint;
-
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-
-import java.net.URI;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-import java.util.Set;
-
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DefaultValue;
 import javax.ws.rs.FormParam;
@@ -100,10 +81,22 @@ import javax.ws.rs.core.GenericEntity;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
-
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlRootElement;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+//~--- JDK imports ------------------------------------------------------------
 
 /**
  * Rest resource for importing repositories.
@@ -564,10 +557,6 @@ public class RepositoryImportResource
     {
       handleGenericCreationFailure(ex, type, name);
     }
-    catch (IOException ex)
-    {
-      handleGenericCreationFailure(ex, type, name);
-    }
 
     return repository;
   }
@@ -716,10 +705,6 @@ public class RepositoryImportResource
     {
       manager.delete(repository);
     }
-    catch (IOException e)
-    {
-      logger.error("can not delete repository", e);
-    }
     catch (RepositoryException e)
     {
       logger.error("can not delete repository", e);

scm-webapp/src/main/java/sonia/scm/api/rest/resources/RepositoryResource.java

@@ -127,7 +127,7 @@ public class RepositoryResource extends AbstractManagerResource<Repository, Repo
   public RepositoryResource(RepositoryManager repositoryManager,
     RepositoryServiceFactory servicefactory, HealthChecker healthChecker)
   {
-    super(repositoryManager);
+    super(repositoryManager, Repository.class);
     this.repositoryManager = repositoryManager;
     this.servicefactory = servicefactory;
     this.healthChecker = healthChecker;
@@ -205,9 +205,9 @@ public class RepositoryResource extends AbstractManagerResource<Repository, Repo
         logger.warn("delete not allowed", ex);
         response = Response.status(Response.Status.FORBIDDEN).build();
       }
-      catch (RepositoryException | IOException ex)
+      catch (RepositoryException ex)
       {
-        logger.error("error during create", ex);
+        logger.error("error during delete", ex);
         response = createErrorResponse(ex);
       }
     }

scm-webapp/src/main/java/sonia/scm/api/rest/resources/UserResource.java

@@ -41,10 +41,8 @@ import com.webcohesion.enunciate.metadata.rs.ResponseCode;
 import com.webcohesion.enunciate.metadata.rs.ResponseHeader;
 import com.webcohesion.enunciate.metadata.rs.StatusCodes;
 import com.webcohesion.enunciate.metadata.rs.TypeHint;
-
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.authc.credential.PasswordService;
-
 import sonia.scm.security.Role;
 import sonia.scm.user.User;
 import sonia.scm.user.UserException;
@@ -52,11 +50,6 @@ import sonia.scm.user.UserManager;
 import sonia.scm.util.AssertUtil;
 import sonia.scm.util.Util;
 
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.Collection;
-
-import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
@@ -72,6 +65,9 @@ import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Request;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
+import java.util.Collection;
+
+//~--- JDK imports ------------------------------------------------------------
 
 /**
  * RESTful Web Service Resource to manage users.
@@ -101,7 +97,7 @@ public class UserResource extends AbstractManagerResource<User, UserException>
   @Inject
   public UserResource(UserManager userManager, PasswordService passwordService)
   {
-    super(userManager);
+    super(userManager, User.class);
     this.passwordService = passwordService;
   }
 

scm-webapp/src/main/java/sonia/scm/api/v2/resources/CollectionResourceManagerAdapter.java

@@ -30,8 +30,8 @@ class CollectionResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
                              DTO extends HalRepresentation,
                              EXCEPTION extends Exception> extends AbstractManagerResource<MODEL_OBJECT, EXCEPTION> {
 
-  CollectionResourceManagerAdapter(Manager<MODEL_OBJECT, EXCEPTION> manager) {
+  CollectionResourceManagerAdapter(Manager<MODEL_OBJECT, EXCEPTION> manager, Class<MODEL_OBJECT> type) {
-    super(manager);
+    super(manager, type);
   }
 
   /**
@@ -47,7 +47,7 @@ class CollectionResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
    * Creates a model object for the given dto and returns a corresponding http response.
    * This handles all corner cases, eg. no conflicts or missing privileges.
    */
-  public Response create(DTO dto, Supplier<MODEL_OBJECT> modelObjectSupplier, Function<MODEL_OBJECT, String> uriCreator) throws IOException, EXCEPTION {
+  public Response create(DTO dto, Supplier<MODEL_OBJECT> modelObjectSupplier, Function<MODEL_OBJECT, String> uriCreator) throws EXCEPTION {
     if (dto == null) {
       return Response.status(BAD_REQUEST).build();
     }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupCollectionResource.java

@@ -36,16 +36,17 @@ public class GroupCollectionResource {
     this.dtoToGroupMapper = dtoToGroupMapper;
     this.groupCollectionToDtoMapper = groupCollectionToDtoMapper;
     this.resourceLinks = resourceLinks;
-    this.adapter = new IdResourceManagerAdapter<>(manager);
+    this.adapter = new IdResourceManagerAdapter<>(manager, Group.class);
   }
 
   /**
    * Returns all groups for a given page number with a given page size (default page size is {@value DEFAULT_PAGE_SIZE}).
-   * 
+   *
    * <strong>Note:</strong> This method requires "group" privilege.
-   *  @param page     the number of the requested page
+   *
+   * @param page     the number of the requested page
    * @param pageSize the page size (default page size is {@value DEFAULT_PAGE_SIZE})
-   * @param sortBy   sort parameter
+   * @param sortBy   sort parameter (if empty - undefined sorting)
    * @param desc     sort direction desc or aesc
    */
   @GET
@@ -54,6 +55,7 @@ public class GroupCollectionResource {
   @TypeHint(GroupDto[].class)
   @StatusCodes({
     @ResponseCode(code = 200, condition = "success"),
+    @ResponseCode(code = 400, condition = "\"sortBy\" field unknown"),
     @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"),
     @ResponseCode(code = 403, condition = "not authorized, the current user does not have the \"group\" privilege"),
     @ResponseCode(code = 500, condition = "internal server error")

scm-webapp/src/main/java/sonia/scm/api/v2/resources/GroupResource.java

@@ -29,7 +29,7 @@ public class GroupResource {
                        GroupDtoToGroupMapper groupDtoToGroupMapper) {
     this.groupToGroupDtoMapper = groupToGroupDtoMapper;
     this.dtoToGroupMapper = groupDtoToGroupMapper;
-    this.adapter = new IdResourceManagerAdapter<>(manager);
+    this.adapter = new IdResourceManagerAdapter<>(manager, Group.class);
   }
 
   /**

scm-webapp/src/main/java/sonia/scm/api/v2/resources/IdResourceManagerAdapter.java

@@ -23,10 +23,10 @@ class IdResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
   private final SingleResourceManagerAdapter<MODEL_OBJECT, DTO, EXCEPTION> singleAdapter;
   private final CollectionResourceManagerAdapter<MODEL_OBJECT, DTO, EXCEPTION> collectionAdapter;
 
-  IdResourceManagerAdapter(Manager<MODEL_OBJECT, EXCEPTION> manager) {
+  IdResourceManagerAdapter(Manager<MODEL_OBJECT, EXCEPTION> manager, Class<MODEL_OBJECT> type) {
     this.manager = manager;
-    singleAdapter = new SingleResourceManagerAdapter<>(manager);
+    singleAdapter = new SingleResourceManagerAdapter<>(manager, type);
-    collectionAdapter = new CollectionResourceManagerAdapter<>(manager);
+    collectionAdapter = new CollectionResourceManagerAdapter<>(manager, type);
   }
 
   Response get(String id, Function<MODEL_OBJECT, DTO> mapToDto) {

scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryCollectionResource.java

@@ -30,7 +30,7 @@ public class RepositoryCollectionResource {
 
   @Inject
   public RepositoryCollectionResource(RepositoryManager manager, RepositoryCollectionToDtoMapper repositoryCollectionToDtoMapper, RepositoryDtoToRepositoryMapper dtoToRepositoryMapper, ResourceLinks resourceLinks) {
-    this.adapter = new CollectionResourceManagerAdapter<>(manager);
+    this.adapter = new CollectionResourceManagerAdapter<>(manager, Repository.class);
     this.repositoryCollectionToDtoMapper = repositoryCollectionToDtoMapper;
     this.dtoToRepositoryMapper = dtoToRepositoryMapper;
     this.resourceLinks = resourceLinks;

scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java

@@ -43,7 +43,7 @@ public class RepositoryResource {
     this.dtoToRepositoryMapper = dtoToRepositoryMapper;
     this.manager = manager;
     this.repositoryToDtoMapper = repositoryToDtoMapper;
-    this.adapter = new SingleResourceManagerAdapter<>(manager);
+    this.adapter = new SingleResourceManagerAdapter<>(manager, Repository.class);
     this.tagRootResource = tagRootResource;
     this.branchRootResource = branchRootResource;
     this.changesetRootResource = changesetRootResource;

scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java

@@ -28,8 +28,8 @@ class SingleResourceManagerAdapter<MODEL_OBJECT extends ModelObject,
                              DTO extends HalRepresentation,
                              EXCEPTION extends Exception> extends AbstractManagerResource<MODEL_OBJECT, EXCEPTION> {
 
-  SingleResourceManagerAdapter(Manager<MODEL_OBJECT, EXCEPTION> manager) {
+  SingleResourceManagerAdapter(Manager<MODEL_OBJECT, EXCEPTION> manager, Class<MODEL_OBJECT> type) {
-    super(manager);
+    super(manager, type);
   }
 
   /**

scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java

@@ -35,7 +35,7 @@ public class UserCollectionResource {
     UserCollectionToDtoMapper userCollectionToDtoMapper, ResourceLinks resourceLinks) {
     this.dtoToUserMapper = dtoToUserMapper;
     this.userCollectionToDtoMapper = userCollectionToDtoMapper;
-    this.adapter = new IdResourceManagerAdapter<>(manager);
+    this.adapter = new IdResourceManagerAdapter<>(manager, User.class);
     this.resourceLinks = resourceLinks;
   }
 
@@ -43,9 +43,10 @@ public class UserCollectionResource {
    * Returns all users for a given page number with a given page size (default page size is {@value DEFAULT_PAGE_SIZE}).
    *
    * <strong>Note:</strong> This method requires "user" privilege.
-   *  @param page     the number of the requested page
+   *
+   * @param page     the number of the requested page
    * @param pageSize the page size (default page size is {@value DEFAULT_PAGE_SIZE})
-   * @param sortBy   sort parameter
+   * @param sortBy   sort parameter (if empty - undefined sorting)
    * @param desc     sort direction desc or asc
    */
   @GET
@@ -54,6 +55,7 @@ public class UserCollectionResource {
   @TypeHint(UserDto[].class)
   @StatusCodes({
     @ResponseCode(code = 200, condition = "success"),
+    @ResponseCode(code = 400, condition = "\"sortBy\" field unknown"),
     @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"),
     @ResponseCode(code = 403, condition = "not authorized, the current user does not have the \"user\" privilege"),
     @ResponseCode(code = 500, condition = "internal server error")

scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java

@@ -29,7 +29,7 @@ public class UserResource {
   public UserResource(UserDtoToUserMapper dtoToUserMapper, UserToUserDtoMapper userToDtoMapper, UserManager manager) {
     this.dtoToUserMapper = dtoToUserMapper;
     this.userToDtoMapper = userToDtoMapper;
-    this.adapter = new IdResourceManagerAdapter<>(manager);
+    this.adapter = new IdResourceManagerAdapter<>(manager, User.class);
   }
 
   /**

scm-webapp/src/main/java/sonia/scm/group/DefaultGroupManager.java

@@ -111,7 +111,7 @@ public class DefaultGroupManager extends AbstractGroupManager
    * @throws IOException
    */
   @Override
-  public Group create(Group group) throws GroupException, IOException
+  public Group create(Group group) throws GroupException
   {
     String type = group.getType();
 
@@ -132,7 +132,7 @@ public class DefaultGroupManager extends AbstractGroupManager
     
     if (groupDAO.contains(name))
     {
-      throw new GroupAlreadyExistsException(name.concat(" group already exists"));
+      throw new GroupAlreadyExistsException(name);
     }
 
     removeDuplicateMembers(group);
@@ -153,7 +153,7 @@ public class DefaultGroupManager extends AbstractGroupManager
    * @throws IOException
    */
   @Override
-  public void delete(Group group) throws GroupException, IOException
+  public void delete(Group group) throws GroupException
   {
     if (logger.isInfoEnabled())
     {
@@ -172,7 +172,7 @@ public class DefaultGroupManager extends AbstractGroupManager
     }
     else
     {
-      throw new GroupNotFoundException("user does not exists");
+      throw new GroupNotFoundException();
     }
   }
 
@@ -195,7 +195,7 @@ public class DefaultGroupManager extends AbstractGroupManager
    * @throws IOException
    */
   @Override
-  public void modify(Group group) throws GroupException, IOException
+  public void modify(Group group) throws GroupException
   {
     if (logger.isInfoEnabled())
     {
@@ -218,7 +218,7 @@ public class DefaultGroupManager extends AbstractGroupManager
     }
     else
     {
-      throw new GroupNotFoundException("group does not exists");
+      throw new GroupNotFoundException();
     }
   }
 
@@ -232,7 +232,7 @@ public class DefaultGroupManager extends AbstractGroupManager
    * @throws IOException
    */
   @Override
-  public void refresh(Group group) throws GroupException, IOException
+  public void refresh(Group group) throws GroupException
   {
     String name = group.getName();
     if (logger.isInfoEnabled())
@@ -245,7 +245,7 @@ public class DefaultGroupManager extends AbstractGroupManager
 
     if (fresh == null)
     {
-      throw new GroupNotFoundException("group does not exists");
+      throw new GroupNotFoundException();
     }
 
     fresh.copyProperties(group);

scm-webapp/src/main/java/sonia/scm/repository/DefaultRepositoryManager.java

@@ -139,7 +139,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
    * @throws RepositoryException
    */
   public Repository create(Repository repository, boolean initRepository)
-    throws RepositoryException, IOException {
+    throws RepositoryException {
     logger.info("create repository {} of type {}", repository.getName(),
       repository.getType());
 
@@ -175,7 +175,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
    */
   @Override
   public Repository create(Repository repository)
-    throws RepositoryException, IOException {
+    throws RepositoryException {
     return create(repository, true);
   }
 
@@ -190,7 +190,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
    */
   @Override
   public void delete(Repository repository)
-    throws RepositoryException, IOException {
+    throws RepositoryException {
     if (logger.isInfoEnabled()) {
       logger.info("delete repository {} of type {}", repository.getName(),
         repository.getType());
@@ -250,7 +250,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
    */
   @Override
   public void modify(Repository repository)
-    throws RepositoryException, IOException {
+    throws RepositoryException {
     if (logger.isInfoEnabled()) {
       logger.info("modify repository {} of type {}", repository.getName(),
         repository.getType());
@@ -285,7 +285,7 @@ public class DefaultRepositoryManager extends AbstractRepositoryManager {
    */
   @Override
   public void refresh(Repository repository)
-    throws RepositoryException, IOException {
+    throws RepositoryException {
     AssertUtil.assertIsNotNull(repository);
     RepositoryPermissions.read(repository).check();
 

scm-webapp/src/main/java/sonia/scm/repository/LastModifiedUpdateListener.java

@@ -35,10 +35,8 @@ package sonia.scm.repository;
 
 import com.google.common.eventbus.Subscribe;
 import com.google.inject.Inject;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import sonia.scm.EagerSingleton;
 import sonia.scm.plugin.Extension;
 import sonia.scm.web.security.AdministrationContext;
@@ -46,8 +44,6 @@ import sonia.scm.web.security.PrivilegedAction;
 
 //~--- JDK imports ------------------------------------------------------------
 
-import java.io.IOException;
-
 /**
  *
  * @author Sebastian Sdorra
@@ -154,7 +150,7 @@ public final class LastModifiedUpdateListener
         {
           repositoryManager.modify(dbr);
         }
-        catch (RepositoryException | IOException ex)
+        catch (RepositoryException ex)
         {
           logger.error("could not modify repository", ex);
         }

scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java

@@ -137,7 +137,7 @@ public class DefaultUserManager extends AbstractUserManager
    * @throws UserException
    */
   @Override
-  public User create(User user) throws UserException, IOException
+  public User create(User user) throws UserException
   {
     String type = user.getType();
 
@@ -155,7 +155,7 @@ public class DefaultUserManager extends AbstractUserManager
 
     if (userDAO.contains(user.getName()))
     {
-      throw new UserAlreadyExistsException(user.getName().concat(" user already exists"));
+      throw new UserAlreadyExistsException(user.getName());
     }
 
     AssertUtil.assertIsValid(user);
@@ -176,7 +176,7 @@ public class DefaultUserManager extends AbstractUserManager
    * @throws UserException
    */
   @Override
-  public void delete(User user) throws UserException, IOException
+  public void delete(User user) throws UserException
   {
     if (logger.isInfoEnabled())
     {
@@ -194,7 +194,7 @@ public class DefaultUserManager extends AbstractUserManager
     }
     else
     {
-      throw new UserNotFoundException("user does not exists");
+      throw new UserNotFoundException();
     }
   }
 
@@ -225,7 +225,7 @@ public class DefaultUserManager extends AbstractUserManager
    * @throws UserException
    */
   @Override
-  public void modify(User user) throws UserException, IOException
+  public void modify(User user) throws UserException
   {
     String name = user.getName();
     if (logger.isInfoEnabled())
@@ -246,7 +246,7 @@ public class DefaultUserManager extends AbstractUserManager
     }
     else
     {
-      throw new UserNotFoundException("user does not exists");
+      throw new UserNotFoundException();
     }
   }
 
@@ -260,7 +260,7 @@ public class DefaultUserManager extends AbstractUserManager
    * @throws UserException
    */
   @Override
-  public void refresh(User user) throws UserException, IOException
+  public void refresh(User user) throws UserException
   {
     if (logger.isInfoEnabled())
     {
@@ -272,7 +272,7 @@ public class DefaultUserManager extends AbstractUserManager
 
     if (fresh == null)
     {
-      throw new UserNotFoundException("user does not exists");
+      throw new UserNotFoundException();
     }
 
     fresh.copyProperties(user);

scm-webapp/src/test/java/sonia/scm/api/rest/resources/AbstractManagerResourceTest.java

@@ -0,0 +1,122 @@
+package sonia.scm.api.rest.resources;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import sonia.scm.Manager;
+import sonia.scm.ModelObject;
+
+import javax.ws.rs.core.GenericEntity;
+import javax.ws.rs.core.Request;
+import java.util.Collection;
+import java.util.Comparator;
+
+import static java.util.Collections.emptyList;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class AbstractManagerResourceTest {
+
+  @Mock
+  private Manager<Simple, Exception> manager;
+  @Mock
+  private Request request;
+  @Captor
+  private ArgumentCaptor<Comparator<Simple>> comparatorCaptor;
+
+  private AbstractManagerResource<Simple, Exception> abstractManagerResource;
+
+  @Before
+  public void captureComparator() {
+    when(manager.getAll(comparatorCaptor.capture(), eq(0), eq(1))).thenReturn(emptyList());
+    abstractManagerResource = new SimpleManagerResource();
+  }
+
+  @Test
+  public void shouldAcceptDefaultSortByParameter() {
+    abstractManagerResource.getAll(request, 0, 1, null, true);
+
+    Comparator<Simple> comparator = comparatorCaptor.getValue();
+    assertTrue(comparator.compare(new Simple("1", null), new Simple("2", null)) > 0);
+  }
+
+  @Test
+  public void shouldAcceptValidSortByParameter() {
+    abstractManagerResource.getAll(request, 0, 1, "data", true);
+
+    Comparator<Simple> comparator = comparatorCaptor.getValue();
+    assertTrue(comparator.compare(new Simple("", "1"), new Simple("", "2")) > 0);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void shouldFailForIllegalSortByParameter() {
+    abstractManagerResource.getAll(request, 0, 1, "x", true);
+  }
+
+
+  private class SimpleManagerResource extends AbstractManagerResource<Simple, Exception> {
+
+    {
+      disableCache = true;
+    }
+
+    private SimpleManagerResource() {
+      super(AbstractManagerResourceTest.this.manager, Simple.class);
+    }
+
+    @Override
+    protected GenericEntity<Collection<Simple>> createGenericEntity(Collection<Simple> items) {
+      return null;
+    }
+
+    @Override
+    protected String getId(Simple item) {
+      return null;
+    }
+
+    @Override
+    protected String getPathPart() {
+      return null;
+    }
+  }
+
+  public static class Simple implements ModelObject {
+
+    private String id;
+    private String data;
+
+    Simple(String id, String data) {
+      this.id = id;
+      this.data = data;
+    }
+
+    public String getData() {
+      return data;
+    }
+
+    @Override
+    public String getId() {
+      return id;
+    }
+
+    @Override
+    public Long getLastModified() {
+      return null;
+    }
+
+    @Override
+    public String getType() {
+      return null;
+    }
+    @Override
+    public boolean isValid() {
+      return false;
+    }
+  }
+}