From f3925fa3116c50044b649244928d430445e214a6 Mon Sep 17 00:00:00 2001
From: Philipp Czora <philipp.czora@cloudogu.com>
Date: Tue, 14 Aug 2018 17:16:10 +0200
Subject: [PATCH] Added links to sources ressource / refactored code and fixed
 issues

---
 .../api/v2/resources/BrowserResultDto.java    |  7 ++
 .../api/v2/resources/BrowserResultMapper.java | 29 ++++++--
 .../api/v2/resources/FileObjectMapper.java    | 17 +++++
 .../scm/api/v2/resources/MapperModule.java    |  2 +-
 .../RepositoryToRepositoryDtoMapper.java      |  2 +-
 .../scm/api/v2/resources/ResourceLinks.java   | 10 ++-
 .../api/v2/resources/SourceRootResource.java  | 26 ++++---
 .../v2/resources/BrowserResultMapperTest.java | 72 ++++++++++---------
 .../api/v2/resources/ResourceLinksTest.java   |  9 ++-
 9 files changed, 120 insertions(+), 54 deletions(-)

diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultDto.java
index fe0f98930c..12c227b65c 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultDto.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultDto.java
@@ -1,6 +1,7 @@
 package sonia.scm.api.v2.resources;
 
 import de.otto.edison.hal.HalRepresentation;
+import de.otto.edison.hal.Links;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
@@ -17,6 +18,12 @@ public class BrowserResultDto extends HalRepresentation implements Iterable<File
   private String branch;
   private List<FileObjectDto> files;
 
+  @Override
+  @SuppressWarnings("squid:S1185") // We want to have this method available in this package
+  protected HalRepresentation add(Links links) {
+    return super.add(links);
+  }
+
   @Override
   public Iterator<FileObjectDto> iterator() {
     Iterator<FileObjectDto> it = null;
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultMapper.java
index 1690225a72..6ab054bd74 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultMapper.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultMapper.java
@@ -1,18 +1,27 @@
 package sonia.scm.api.v2.resources;
 
-import org.mapstruct.Mapper;
+import de.otto.edison.hal.Links;
 import sonia.scm.repository.BrowserResult;
 import sonia.scm.repository.FileObject;
+import sonia.scm.repository.NamespaceAndName;
 
+import javax.inject.Inject;
 import java.util.ArrayList;
 import java.util.List;
 
-@Mapper
-public abstract class BrowserResultMapper extends BaseMapper<BrowserResult, BrowserResultDto> {
+public class BrowserResultMapper {
 
-  abstract FileObjectDto mapFileObject(FileObject fileObject);
+  @Inject
+  private FileObjectMapper fileObjectMapper;
 
-  public BrowserResultDto map(BrowserResult browserResult) {
+  @Inject
+  private ResourceLinks resourceLinks;
+
+  private FileObjectDto mapFileObject(FileObject fileObject, NamespaceAndName namespaceAndName, String revision) {
+    return fileObjectMapper.map(fileObject, namespaceAndName, revision);
+  }
+
+  public BrowserResultDto map(BrowserResult browserResult, NamespaceAndName namespaceAndName) {
     BrowserResultDto browserResultDto = new BrowserResultDto();
 
     browserResultDto.setTag(browserResult.getTag());
@@ -21,11 +30,17 @@ public abstract class BrowserResultMapper extends BaseMapper<BrowserResult, Brow
 
     List<FileObjectDto> fileObjectDtoList = new ArrayList<>();
     for (FileObject fileObject : browserResult.getFiles()) {
-      fileObjectDtoList.add(mapFileObject(fileObject));
+      fileObjectDtoList.add(mapFileObject(fileObject, namespaceAndName, browserResult.getRevision()));
     }
 
     browserResultDto.setFiles(fileObjectDtoList);
-
+    this.addLinks(browserResult, browserResultDto, namespaceAndName);
     return browserResultDto;
   }
+
+  private void addLinks(BrowserResult browserResult, BrowserResultDto dto, NamespaceAndName namespaceAndName) {
+    dto.add(Links.linkingTo().self(resourceLinks.source().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), browserResult.getRevision())).build());
+  }
+
+
 }
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectMapper.java
index 87abc22380..013f7e1125 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectMapper.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectMapper.java
@@ -1,8 +1,25 @@
 package sonia.scm.api.v2.resources;
 
+import de.otto.edison.hal.Links;
+import org.mapstruct.AfterMapping;
+import org.mapstruct.Context;
 import org.mapstruct.Mapper;
+import org.mapstruct.MappingTarget;
 import sonia.scm.repository.FileObject;
+import sonia.scm.repository.NamespaceAndName;
+
+import javax.inject.Inject;
 
 @Mapper
 public abstract class FileObjectMapper extends BaseMapper<FileObject, FileObjectDto> {
+
+  @Inject
+  private ResourceLinks resourceLinks;
+
+  protected abstract FileObjectDto map(FileObject fileObject, @Context NamespaceAndName namespaceAndName, @Context String revision);
+
+  @AfterMapping
+  void addLinks(FileObject fileObject, @MappingTarget FileObjectDto dto, @Context NamespaceAndName namespaceAndName, @Context String revision) {
+      dto.add(Links.linkingTo().self(resourceLinks.source().withPath(namespaceAndName.getNamespace(), namespaceAndName.getName(), revision, fileObject.getName())).build());
+  }
 }
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java
index 5247a17ac8..5ecdaee23d 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java
@@ -26,7 +26,7 @@ public class MapperModule extends AbstractModule {
 
     bind(BranchToBranchDtoMapper.class).to(Mappers.getMapper(BranchToBranchDtoMapper.class).getClass());
 
-    bind(BrowserResultMapper.class).to(Mappers.getMapper(BrowserResultMapper.class).getClass());
+    bind(FileObjectMapper.class).to(Mappers.getMapper(FileObjectMapper.class).getClass());
 
     bind(UriInfoStore.class).in(ServletScopes.REQUEST);
   }
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java
index ca30a09286..d20e2f66dc 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryToRepositoryDtoMapper.java
@@ -36,7 +36,7 @@ public abstract class RepositoryToRepositoryDtoMapper extends BaseMapper<Reposit
     linksBuilder.single(link("tags", resourceLinks.tagCollection().self(target.getNamespace(), target.getName())));
     linksBuilder.single(link("branches", resourceLinks.branchCollection().self(target.getNamespace(), target.getName())));
     linksBuilder.single(link("changesets", resourceLinks.changeset().self(target.getNamespace(), target.getName())));
-    linksBuilder.single(link("sources", resourceLinks.source().self(target.getNamespace(), target.getName())));
+    linksBuilder.single(link("sources", resourceLinks.source().withoutRevision(target.getNamespace(), target.getName())));
     target.add(linksBuilder.build());
   }
 }
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java
index 319bbec32b..e323faf6f7 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java
@@ -290,13 +290,21 @@ class ResourceLinks {
       sourceLinkBuilder = new LinkBuilder(uriInfo, RepositoryRootResource.class, RepositoryResource.class, SourceRootResource.class);
     }
 
-    String self(String namespace, String name) {
+    String self(String namespace, String name, String revision) {
+      return sourceLinkBuilder.method("getRepositoryResource").parameters(namespace, name, revision).method("sources").parameters().method("getAll").parameters().href();
+    }
+
+    String withoutRevision(String namespace, String name) {
       return sourceLinkBuilder.method("getRepositoryResource").parameters(namespace, name).method("sources").parameters().method("getAll").parameters().href();
     }
 
     public String source(String namespace, String name, String revision) {
       return sourceLinkBuilder.method("getRepositoryResource").parameters(namespace, name).method("sources").parameters().method("get").parameters(revision).href();
     }
+
+    public String withPath(String namespace, String name, String revision, String path) {
+      return sourceLinkBuilder.method("getRepositoryResource").parameters(namespace, name).method("sources").parameters().method("get").parameters(revision, path).href();
+    }
   }
 
   public PermissionCollectionLinks permissionCollection() {
diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java
index 58586768e5..95df3e2b11 100644
--- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java
+++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java
@@ -23,7 +23,6 @@ public class SourceRootResource {
   private final BrowserResultMapper browserResultMapper;
 
 
-
   @Inject
   public SourceRootResource(RepositoryServiceFactory serviceFactory, BrowserResultMapper browserResultMapper) {
     this.serviceFactory = serviceFactory;
@@ -35,23 +34,30 @@ public class SourceRootResource {
   @Path("")
   public Response getAll(@PathParam("namespace") String namespace, @PathParam("name") String name) {
 
-    BrowserResult browserResult = null;
-    try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) {
+    BrowserResult browserResult;
+    Response response;
+    NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name);
+    try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
       BrowseCommandBuilder browseCommand = repositoryService.getBrowseCommand();
       browseCommand.setPath("/");
       browserResult = browseCommand.getBrowserResult();
+
+      if (browserResult != null) {
+        response = Response.ok(browserResultMapper.map(browserResult, namespaceAndName)).build();
+      } else {
+        response = Response.status(Response.Status.NOT_FOUND).build();
+      }
+
     } catch (RepositoryNotFoundException e) {
-      e.printStackTrace();
-    } catch (RepositoryException e) {
-      e.printStackTrace();
-    } catch (IOException e) {
-      e.printStackTrace();
+      response = Response.status(Response.Status.NOT_FOUND).build();
+    } catch (RepositoryException | IOException e) {
+      response = Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
     }
-    return Response.ok(browserResultMapper.map(browserResult)).build();
+    return response;
   }
 
   @GET
-  @Path("{revision}")
+  @Path("{revision}/{path: .*}")
   public Response get() {
     throw new UnsupportedOperationException();
   }
diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BrowserResultMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BrowserResultMapperTest.java
index c05f93ad7a..0f973d9713 100644
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BrowserResultMapperTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BrowserResultMapperTest.java
@@ -6,47 +6,79 @@ import org.apache.shiro.util.ThreadContext;
 import org.apache.shiro.util.ThreadState;
 import org.junit.Before;
 import org.junit.Test;
-import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.mockito.Mock;
 import sonia.scm.repository.BrowserResult;
 import sonia.scm.repository.FileObject;
+import sonia.scm.repository.NamespaceAndName;
 
-import java.time.Instant;
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.MockitoAnnotations.initMocks;
 
-@RunWith(MockitoJUnitRunner.class)
 public class BrowserResultMapperTest {
 
+  private final URI baseUri = URI.create("http://example.com/base/");
+  @SuppressWarnings("unused") // Is injected
+  private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
+
+  @Mock
+  private FileObjectMapper fileObjectMapper;
+
   @InjectMocks
-  private BrowserResultMapperImpl mapper;
+  private BrowserResultMapper mapper;
 
   private final Subject subject = mock(Subject.class);
   private final ThreadState subjectThreadState = new SubjectThreadState(subject);
 
+  private FileObject fileObject1 = new FileObject();
+  private FileObject fileObject2 = new FileObject();
+
 
   @Before
   public void init() {
     initMocks(this);
     subjectThreadState.bind();
     ThreadContext.bind(subject);
+
+    fileObject1.setName("FO 1");
+    fileObject1.setLength(100);
+    fileObject1.setLastModified(0L);
+    fileObject1.setPath("/path/object/1");
+    fileObject1.setDescription("description of file object 1");
+    fileObject1.setDirectory(false);
+
+    fileObject2.setName("FO 2");
+    fileObject2.setLength(100);
+    fileObject2.setLastModified(101L);
+    fileObject2.setPath("/path/object/2");
+    fileObject2.setDescription("description of file object 2");
+    fileObject2.setDirectory(true);
   }
 
   @Test
   public void shouldMapAttributesCorrectly() {
     BrowserResult browserResult = createBrowserResult();
 
-    BrowserResultDto dto = mapper.map(browserResult);
+    BrowserResultDto dto = mapper.map(browserResult, new NamespaceAndName("foo", "bar"));
 
     assertEqualAttributes(browserResult, dto);
+  }
 
-    assertEqualFileObjectAttributes(browserResult.getFiles().get(0), dto.getFiles().get(0));
-    assertEqualFileObjectAttributes(browserResult.getFiles().get(1), dto.getFiles().get(1));
+  @Test
+  public void shouldDelegateToFileObjectsMapper() {
+    BrowserResult browserResult = createBrowserResult();
+    NamespaceAndName namespaceAndName = new NamespaceAndName("foo", "bar");
+
+    BrowserResultDto dto = mapper.map(browserResult, namespaceAndName);
+
+    verify(fileObjectMapper).map(fileObject1, namespaceAndName, "Revision");
+    verify(fileObjectMapper).map(fileObject2, namespaceAndName, "Revision");
   }
 
   private BrowserResult createBrowserResult() {
@@ -62,22 +94,6 @@ public class BrowserResultMapperTest {
   private List<FileObject> createFileObjects() {
     List<FileObject> fileObjects = new ArrayList<>();
 
-    FileObject fileObject1 = new FileObject();
-    fileObject1.setName("FO 1");
-    fileObject1.setLength(100);
-    fileObject1.setLastModified(0L);
-    fileObject1.setPath("/path/object/1");
-    fileObject1.setDescription("description of file object 1");
-    fileObject1.setDirectory(false);
-
-    FileObject fileObject2 = new FileObject();
-    fileObject2.setName("FO 2");
-    fileObject2.setLength(100);
-    fileObject2.setLastModified(101L);
-    fileObject2.setPath("/path/object/2");
-    fileObject2.setDescription("description of file object 2");
-    fileObject2.setDirectory(true);
-
     fileObjects.add(fileObject1);
     fileObjects.add(fileObject2);
     return fileObjects;
@@ -89,12 +105,4 @@ public class BrowserResultMapperTest {
     assertThat(dto.getRevision()).isEqualTo(browserResult.getRevision());
   }
 
-  private void assertEqualFileObjectAttributes(FileObject fileObject, FileObjectDto dto) {
-    assertThat(dto.getName()).isEqualTo(fileObject.getName());
-    assertThat(dto.getLength()).isEqualTo(fileObject.getLength());
-    assertThat(dto.getLastModified()).isEqualTo(Instant.ofEpochMilli(fileObject.getLastModified()));
-    assertThat(dto.isDirectory()).isEqualTo(fileObject.isDirectory());
-    assertThat(dto.getDescription()).isEqualTo(fileObject.getDescription());
-    assertThat(dto.getPath()).isEqualTo(fileObject.getPath());
-  }
 }
diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksTest.java
index 000e628f26..430fea9748 100644
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksTest.java
@@ -147,13 +147,18 @@ public class ResourceLinksTest {
 
   @Test
   public void shouldCreateCorrectSourceCollectionUrl() {
-    String url = resourceLinks.source().self("space", "repo");
+    String url = resourceLinks.source().withoutRevision("space", "repo");
     assertEquals(BASE_URL + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo/sources/", url);
   }
 
+  @Test
+  public void shouldCreateCorrectSourceUrlWithFilename() {
+    String url = resourceLinks.source().withPath("foo", "bar", "rev", "file");
+    assertEquals(BASE_URL + RepositoryRootResource.REPOSITORIES_PATH_V2 + "foo/bar/sources/rev/file", url);
+  }
   @Test
   public void shouldCreateCorrectPermissionCollectionUrl() {
-    String url = resourceLinks.source().self("space", "repo");
+    String url = resourceLinks.source().withoutRevision("space", "repo");
     assertEquals(BASE_URL + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo/sources/", url);
   }