Merge with 2.0.0-m3

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

@@ -1,4 +1,11 @@
 package sonia.scm;
 
 public class AlreadyExistsException extends Exception {
+
+  public AlreadyExistsException(String message) {
+    super(message);
+  }
+
+  public AlreadyExistsException() {
+  }
 }

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

@@ -7,4 +7,9 @@ public class NotFoundException extends Exception {
 
   public NotFoundException() {
   }
+
+
+  public NotFoundException(String message) {
+    super(message);
+  }
 }

scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java

@@ -3,6 +3,8 @@ package sonia.scm.it;
 import com.google.common.base.Charsets;
 import com.google.common.io.Files;
 import org.apache.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import sonia.scm.repository.Changeset;
 import sonia.scm.repository.Person;
 import sonia.scm.repository.client.api.ClientCommand;
@@ -15,6 +17,8 @@ import java.util.UUID;
 
 public class RepositoryUtil {
 
+  private static final Logger LOG = LoggerFactory.getLogger(RepositoryUtil.class);
+
   private static final RepositoryClientFactory REPOSITORY_CLIENT_FACTORY = new RepositoryClientFactory();
 
   static RepositoryClient createRepositoryClient(String repositoryType, File folder) throws IOException {
@@ -58,6 +62,7 @@ public class RepositoryUtil {
   }
 
   static Changeset commit(RepositoryClient repositoryClient, String username, String message) throws IOException {
+    LOG.info("user: {} try to commit with message:  {}", username, message);
     Changeset changeset = repositoryClient.getCommitCommand().commit(new Person(username, username + "@scm-manager.org"), message);
     if (repositoryClient.isCommandSupported(ClientCommand.PUSH)) {
       repositoryClient.getPushCommand().push();

scm-it/src/test/java/sonia/scm/it/TestData.java

@@ -33,6 +33,7 @@ public class TestData {
   }
 
   public static void cleanup() {
+    LOG.info("start to clean up to integration tests");
     cleanupRepositories();
     cleanupGroups();
     cleanupUsers();
@@ -43,6 +44,7 @@ public class TestData {
   }
 
   public static void createUser(String username, String password) {
+    LOG.info("create user with username: {}", username);
     given(VndMediaType.USER)
       .when()
       .content(" {\n" +
@@ -64,6 +66,8 @@ public class TestData {
 
 
   public static void createUserPermission(String name, PermissionType permissionType, String repositoryType) {
+    String defaultPermissionUrl = TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType);
+    LOG.info("create permission with name {} and type: {} using the endpoint: {}", name, permissionType, defaultPermissionUrl);
     given(VndMediaType.PERMISSION)
       .when()
       .content("{\n" +
@@ -72,7 +76,7 @@ public class TestData {
         "\t\"groupPermission\": false\n" +
         "\t\n" +
         "}")
-      .post(TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType))
+      .post(defaultPermissionUrl)
       .then()
       .statusCode(HttpStatus.SC_CREATED)
     ;
@@ -114,6 +118,7 @@ public class TestData {
 
 
   private static void cleanupRepositories() {
+    LOG.info("clean up repository");
     List<String> repositories = given(VndMediaType.REPOSITORY_COLLECTION)
       .when()
       .get(createResourceUrl("repositories"))
@@ -160,6 +165,7 @@ public class TestData {
   }
 
   private static void createDefaultRepositories() {
+    LOG.info("create default repositories");
     for (String repositoryType : availableScmTypes()) {
       String url = given(VndMediaType.REPOSITORY)
         .body(repositoryJson(repositoryType))
@@ -171,6 +177,7 @@ public class TestData {
         .statusCode(HttpStatus.SC_CREATED)
         .extract()
         .header("location");
+      LOG.info("a {} repository is created: {}", repositoryType, url);
       DEFAULT_REPOSITORIES.put(repositoryType, url);
     }
   }

scm-webapp/src/main/java/sonia/scm/api/rest/AlreadyExistsExceptionMapper.java

@@ -11,6 +11,8 @@ import javax.ws.rs.ext.Provider;
 public class AlreadyExistsExceptionMapper implements ExceptionMapper<AlreadyExistsException> {
   @Override
   public Response toResponse(AlreadyExistsException exception) {
-    return Response.status(Status.CONFLICT).build();
+    return Response.status(Status.CONFLICT)
+      .entity(exception.getMessage())
+      .build();
   }
 }

scm-webapp/src/main/java/sonia/scm/api/rest/IllegalArgumentExceptionMapper.java

@@ -33,6 +33,8 @@ package sonia.scm.api.rest;
 
 //~--- JDK imports ------------------------------------------------------------
 
+import lombok.extern.slf4j.Slf4j;
+
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 import javax.ws.rs.ext.ExceptionMapper;
@@ -43,7 +45,7 @@ import javax.ws.rs.ext.Provider;
  * @author Sebastian Sdorra
  * @since 1.36
  */
-@Provider
+@Provider @Slf4j
 public class IllegalArgumentExceptionMapper
   implements ExceptionMapper<IllegalArgumentException>
 {
@@ -59,6 +61,7 @@ public class IllegalArgumentExceptionMapper
   @Override
   public Response toResponse(IllegalArgumentException exception)
   {
+    log.info("caught IllegalArgumentException -- mapping to bad request", exception);
     return Response.status(Status.BAD_REQUEST).build();
   }
 }

scm-webapp/src/main/java/sonia/scm/api/rest/StatusExceptionMapper.java

@@ -90,6 +90,8 @@ public class StatusExceptionMapper<E extends Throwable>
       logger.debug(msg.toString());
     }
 
-    return Response.status(status).build();
+    return Response.status(status)
+      .entity(exception.getMessage())
+      .build();
   }
 }

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

@@ -1,32 +0,0 @@
-package sonia.scm.api.v2.resources;
-
-import de.otto.edison.hal.HalRepresentation;
-
-import java.util.Collection;
-import java.util.List;
-import java.util.stream.Collectors;
-
-import static de.otto.edison.hal.Embedded.embeddedBuilder;
-import static de.otto.edison.hal.Links.linkingTo;
-
-abstract class CollectionToDtoMapper<E, D extends HalRepresentation> {
-
-  private final String collectionName;
-  private final BaseMapper<E, D> mapper;
-
-  protected CollectionToDtoMapper(String collectionName, BaseMapper<E, D> mapper) {
-    this.collectionName = collectionName;
-    this.mapper = mapper;
-  }
-
-  public HalRepresentation map(Collection<E> collection) {
-    List<D> dtos = collection.stream().map(mapper::map).collect(Collectors.toList());
-    return new HalRepresentation(
-      linkingTo().self(createSelfLink()).build(),
-      embeddedBuilder().with(collectionName, dtos).build()
-    );
-  }
-
-  protected abstract String createSelfLink();
-
-}

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

@@ -10,6 +10,8 @@ import lombok.ToString;
 @Getter @Setter @ToString
 public class PermissionDto extends HalRepresentation {
 
+  public static final String GROUP_PREFIX = "@";
+
   @JsonInclude(JsonInclude.Include.NON_NULL)
   private String name;
 
@@ -26,6 +28,14 @@ public class PermissionDto extends HalRepresentation {
 
   private boolean groupPermission = false;
 
+  public PermissionDto() {
+  }
+
+  public PermissionDto(String permissionName, boolean groupPermission) {
+    name = permissionName;
+    this.groupPermission = groupPermission;
+  }
+
 
   @Override
   @SuppressWarnings("squid:S1185") // We want to have this method available in this package

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

@@ -5,7 +5,6 @@ import com.webcohesion.enunciate.metadata.rs.ResponseHeader;
 import com.webcohesion.enunciate.metadata.rs.StatusCodes;
 import com.webcohesion.enunciate.metadata.rs.TypeHint;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.commons.lang.StringUtils;
 import sonia.scm.AlreadyExistsException;
 import sonia.scm.NotFoundException;
 import sonia.scm.repository.NamespaceAndName;
@@ -28,10 +27,14 @@ import javax.ws.rs.Produces;
 import javax.ws.rs.core.Response;
 import java.net.URI;
 import java.util.Optional;
+import java.util.function.Predicate;
+
+import static sonia.scm.api.v2.resources.PermissionDto.GROUP_PREFIX;
 
 @Slf4j
 public class PermissionRootResource {
 
+
   private PermissionDtoToPermissionMapper dtoToModelMapper;
   private PermissionToPermissionDtoMapper modelToDtoMapper;
   private PermissionCollectionToDtoMapper permissionCollectionToDtoMapper;
@@ -101,7 +104,7 @@ public class PermissionRootResource {
     return Response.ok(
       repository.getPermissions()
         .stream()
-        .filter(permission -> permissionName.equals(permission.getName()))
+        .filter(filterPermission(permissionName))
         .map(permission -> modelToDtoMapper.map(permission, repository))
         .findFirst()
         .orElseThrow(NotFoundException::new)
@@ -135,6 +138,7 @@ public class PermissionRootResource {
 
   /**
    * Update a permission to the user or group managed by the repository
+   * ignore the user input for groupPermission and take it from the path parameter (if the group prefix (@) exists it is a group permission)
    *
    * @param permission     permission to modify
    * @param permissionName permission to modify
@@ -152,15 +156,23 @@ public class PermissionRootResource {
   public Response update(@PathParam("namespace") String namespace,
                          @PathParam("name") String name,
                          @PathParam("permission-name") String permissionName,
-                         PermissionDto permission) throws NotFoundException {
+                         PermissionDto permission) throws NotFoundException, AlreadyExistsException {
     log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission);
     Repository repository = load(namespace, name);
     RepositoryPermissions.permissionWrite(repository).check();
+    String extractedPermissionName = getPermissionName(permissionName);
+    if (!isPermissionExist(new PermissionDto(extractedPermissionName, isGroupPermission(permissionName)), repository)) {
+      throw new NotFoundException("the permission " + extractedPermissionName + " does not exist");
+    }
+    permission.setGroupPermission(isGroupPermission(permissionName));
+    if (!extractedPermissionName.equals(permission.getName())) {
+      checkPermissionAlreadyExists(permission, repository, "target permission " + permission.getName() + " already exists");
+    }
     Permission existingPermission = repository.getPermissions()
       .stream()
-      .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName))
+      .filter(filterPermission(permissionName))
       .findFirst()
-      .orElseThrow(() -> new NotFoundException());
+      .orElseThrow(NotFoundException::new);
     dtoToModelMapper.modify(existingPermission, permission);
     manager.modify(repository);
     log.info("the permission with name: {} is updated.", permissionName);
@@ -190,7 +202,7 @@ public class PermissionRootResource {
     RepositoryPermissions.modify(repository).check();
     repository.getPermissions()
       .stream()
-      .filter(perm -> StringUtils.isNotBlank(perm.getName()) && perm.getName().equals(permissionName))
+      .filter(filterPermission(permissionName))
       .findFirst()
       .ifPresent(p -> repository.getPermissions().remove(p))
     ;
@@ -199,6 +211,22 @@ public class PermissionRootResource {
     return Response.noContent().build();
   }
 
+  Predicate<Permission> filterPermission(String permissionName) {
+    return permission -> getPermissionName(permissionName).equals(permission.getName())
+      &&
+      permission.isGroupPermission() == isGroupPermission(permissionName);
+  }
+
+  private String getPermissionName(String permissionName) {
+    return Optional.of(permissionName)
+      .filter(p -> !isGroupPermission(permissionName))
+      .orElse(permissionName.substring(1));
+  }
+
+  private boolean isGroupPermission(String permissionName) {
+    return permissionName.startsWith(GROUP_PREFIX);
+  }
+
 
   /**
    * check if the actual user is permitted to manage the repository permissions
@@ -220,15 +248,23 @@ public class PermissionRootResource {
    *
    * @param permission the searched permission
    * @param repository the repository to be inspected
+   * @param errorMessage error message
    * @throws AlreadyExistsException if the permission already exists in the repository
    */
-  private void checkPermissionAlreadyExists(PermissionDto permission, Repository repository) throws AlreadyExistsException {
-    boolean isPermissionAlreadyExist = repository.getPermissions()
+  private void checkPermissionAlreadyExists(PermissionDto permission, Repository repository, String errorMessage) throws AlreadyExistsException {
+    if (isPermissionExist(permission, repository)) {
+      throw new AlreadyExistsException(errorMessage);
+    }
+  }
+
+  private boolean isPermissionExist(PermissionDto permission, Repository repository) {
+    return repository.getPermissions()
       .stream()
-      .anyMatch(p -> p.getName().equals(permission.getName()));
-    if (isPermissionAlreadyExist) {
-      throw new AlreadyExistsException();
+      .anyMatch(p -> p.getName().equals(permission.getName()) && p.isGroupPermission() == permission.isGroupPermission());
   }
+
+  private void checkPermissionAlreadyExists(PermissionDto permission, Repository repository) throws AlreadyExistsException {
+    checkPermissionAlreadyExists(permission, repository, "the permission " + permission.getName() + " already exist.");
   }
 }
 

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

@@ -12,9 +12,11 @@ import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryPermissions;
 
 import javax.inject.Inject;
+import java.util.Optional;
 
 import static de.otto.edison.hal.Link.link;
 import static de.otto.edison.hal.Links.linkingTo;
+import static sonia.scm.api.v2.resources.PermissionDto.GROUP_PREFIX;
 
 @Mapper
 public abstract class PermissionToPermissionDtoMapper {
@@ -39,11 +41,14 @@ public abstract class PermissionToPermissionDtoMapper {
    */
   @AfterMapping
   void appendLinks(@MappingTarget PermissionDto target, @Context Repository repository) {
+    String permissionName = Optional.of(target.getName())
+      .filter(p -> !target.isGroupPermission())
+      .orElse(GROUP_PREFIX + target.getName());
     Links.Builder linksBuilder = linkingTo()
-      .self(resourceLinks.permission().self(repository.getNamespace(), repository.getName(), target.getName()));
+      .self(resourceLinks.permission().self(repository.getNamespace(), repository.getName(), permissionName));
     if (RepositoryPermissions.permissionWrite(repository).isPermitted()) {
-      linksBuilder.single(link("update", resourceLinks.permission().update(repository.getNamespace(), repository.getName(), target.getName())));
-      linksBuilder.single(link("delete", resourceLinks.permission().delete(repository.getNamespace(), repository.getName(), target.getName())));
+      linksBuilder.single(link("update", resourceLinks.permission().update(repository.getNamespace(), repository.getName(), permissionName)));
+      linksBuilder.single(link("delete", resourceLinks.permission().delete(repository.getNamespace(), repository.getName(), permissionName)));
     }
     target.add(linksBuilder.build());
   }

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

@@ -1,5 +1,6 @@
 package sonia.scm.api.v2.resources;
 
+import sonia.scm.NotFoundException;
 import sonia.scm.repository.BrowserResult;
 import sonia.scm.repository.NamespaceAndName;
 import sonia.scm.repository.RepositoryNotFoundException;
@@ -46,7 +47,7 @@ public class SourceRootResource {
   @GET
   @Produces(VndMediaType.SOURCE)
   @Path("{revision}/{path: .*}")
-  public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) throws RevisionNotFoundException, RepositoryNotFoundException, IOException {
+  public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) throws NotFoundException, IOException {
     return getSource(namespace, name, path, revision);
   }
 

scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java

@@ -39,6 +39,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -55,6 +56,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 import static sonia.scm.api.v2.resources.DispatcherMock.createDispatcher;
+import static sonia.scm.api.v2.resources.PermissionDto.GROUP_PREFIX;
 
 @Slf4j
 @SubjectAware(
@@ -253,7 +255,7 @@ public class PermissionRootResourceTest {
     createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE);
     Permission newPermission = TEST_PERMISSIONS.get(0);
     assertExpectedRequest(requestPOSTPermission
-      .content("{\"name\" : \"" + newPermission.getName() + "\" , \"type\" : \"WRITE\" , \"groupPermission\" : true}")
+      .content("{\"name\" : \"" + newPermission.getName() + "\" , \"type\" : \"WRITE\" , \"groupPermission\" : false}")
       .expectedResponseStatus(409)
     );
   }
@@ -358,7 +360,10 @@ public class PermissionRootResourceTest {
     result.setName(permission.getName());
     result.setGroupPermission(permission.isGroupPermission());
     result.setType(permission.getType().name());
-    String permissionHref = "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + PATH_OF_ALL_PERMISSIONS + permission.getName();
+    String permissionName = Optional.of(permission.getName())
+      .filter(p -> !permission.isGroupPermission())
+      .orElse(GROUP_PREFIX + permission.getName());
+    String permissionHref = "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + PATH_OF_ALL_PERMISSIONS + permissionName;
     if (PERMISSION_READ.equals(userPermission)) {
       result.add(linkingTo()
         .self(permissionHref)

scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionToPermissionDtoMapperTest.java

@@ -0,0 +1,63 @@
+package sonia.scm.api.v2.resources;
+
+import com.github.sdorra.shiro.ShiroRule;
+import com.github.sdorra.shiro.SubjectAware;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.junit.MockitoJUnitRunner;
+import sonia.scm.repository.Permission;
+import sonia.scm.repository.PermissionType;
+import sonia.scm.repository.Repository;
+
+import java.net.URI;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@RunWith(MockitoJUnitRunner.Silent.class)
+@SubjectAware(
+  configuration = "classpath:sonia/scm/repository/shiro.ini"
+)
+public class PermissionToPermissionDtoMapperTest {
+
+  @Rule
+  public ShiroRule shiro = new ShiroRule();
+
+  private final URI baseUri = URI.create("http://example.com/base/");
+
+  @SuppressWarnings("unused") // Is injected
+  private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
+
+  @InjectMocks
+  PermissionToPermissionDtoMapperImpl mapper;
+
+  @Test
+  @SubjectAware(username = "trillian", password = "secret")
+  public void shouldMapGroupPermissionCorrectly() {
+    Repository repository = getDummyRepository();
+    Permission permission = new Permission("42", PermissionType.OWNER, true);
+
+    PermissionDto permissionDto = mapper.map(permission, repository);
+
+    assertThat(permissionDto.getLinks().getLinkBy("self").isPresent()).isTrue();
+    assertThat(permissionDto.getLinks().getLinkBy("self").get().getHref()).contains("@42");
+  }
+
+  @Test
+  @SubjectAware(username = "trillian", password = "secret")
+  public void shouldMapNonGroupPermissionCorrectly() {
+    Repository repository = getDummyRepository();
+    Permission permission = new Permission("42", PermissionType.OWNER, false);
+
+    PermissionDto permissionDto = mapper.map(permission, repository);
+
+    assertThat(permissionDto.getLinks().getLinkBy("self").isPresent()).isTrue();
+    assertThat(permissionDto.getLinks().getLinkBy("self").get().getHref()).contains("42");
+    assertThat(permissionDto.getLinks().getLinkBy("self").get().getHref()).doesNotContain("@");
+  }
+
+  private Repository getDummyRepository() {
+    return new Repository("repo", "git", "foo", "bar");
+  }
+}

scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java

@@ -29,12 +29,13 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.when;
+import static sonia.scm.api.v2.resources.DispatcherMock.createDispatcher;
 
 
 @RunWith(MockitoJUnitRunner.Silent.class)
 public class SourceRootResourceTest {
 
-  private final Dispatcher dispatcher = MockDispatcherFactory.createDispatcher();
+  private Dispatcher dispatcher;
   private final URI baseUri = URI.create("/");
   private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
 
@@ -75,9 +76,7 @@ public class SourceRootResourceTest {
         null,
         null)),
         null);
-
-    dispatcher.getRegistry().addSingletonResource(repositoryRootResource);
-    dispatcher.getProviderFactory().registerProvider(NotFoundExceptionMapper.class);
+    dispatcher = createDispatcher(repositoryRootResource);
   }
 
   @Test