From 7e31498fa4d41a4116d3e09445c158f71cd2e479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 10 Aug 2018 16:31:22 +0200 Subject: [PATCH 01/20] Introduce content resource with content type detection --- scm-webapp/pom.xml | 13 ++- .../scm/api/v2/resources/ContentResource.java | 96 +++++++++++++++++++ .../api/v2/resources/RepositoryResource.java | 9 +- .../v2/resources/BranchRootResourceTest.java | 2 +- .../resources/RepositoryRootResourceTest.java | 2 +- 5 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index d0003c1f7c..965bddf2b0 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -226,7 +226,18 @@ compiler ${mustache.version} - + + + com.github.sdorra + spotter-core + 1.1.0 + + + + org.apache.tika + tika-core + 1.18 + diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java new file mode 100644 index 0000000000..dc14d00e54 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -0,0 +1,96 @@ +package sonia.scm.api.v2.resources; + +import com.github.sdorra.spotter.ContentType; +import com.github.sdorra.spotter.ContentTypes; +import com.github.sdorra.spotter.Language; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.PathNotFoundException; +import sonia.scm.repository.RepositoryException; +import sonia.scm.repository.RepositoryNotFoundException; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.HEAD; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.core.Response; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Optional; + +public class ContentResource { + + private final RepositoryServiceFactory servicefactory; + + @Inject + public ContentResource(RepositoryServiceFactory servicefactory) { + this.servicefactory = servicefactory; + } + + @GET + @Path("{revision}/{path: .*}") + public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { + try (RepositoryService repositoryService = servicefactory.create(new NamespaceAndName(namespace, name))) { + try { + byte[] content = getContent(revision, path, repositoryService); + Response.ResponseBuilder responseBuilder = Response.ok(content); + appendContentType(path, content, responseBuilder); + return responseBuilder.build(); + } catch (PathNotFoundException e) { + return Response.status(404).build(); + } catch (IOException e) { + e.printStackTrace(); + return Response.status(500).entity(e.getMessage()).build(); + } catch (RepositoryException e) { + e.printStackTrace(); + return Response.status(500).entity(e.getMessage()).build(); + } + } catch (RepositoryNotFoundException e) { + return Response.status(404).build(); + } + } + + @HEAD + @Path("{revision}/{path: .*}") + public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { + try (RepositoryService repositoryService = servicefactory.create(new NamespaceAndName(namespace, name))) { + try { + byte[] content = getContent(revision, path, repositoryService); + + Response.ResponseBuilder responseBuilder = Response.ok(); + + appendContentType(path, content, responseBuilder); + return responseBuilder.build(); + } catch (PathNotFoundException e) { + return Response.status(404).build(); + } catch (IOException e) { + e.printStackTrace(); + return Response.status(500).entity(e.getMessage()).build(); + } catch (RepositoryException e) { + e.printStackTrace(); + return Response.status(500).entity(e.getMessage()).build(); + } + } catch (RepositoryNotFoundException e) { + return Response.status(404).build(); + } + } + + private void appendContentType(String path, byte[] content, Response.ResponseBuilder responseBuilder) { + ContentType contentType = ContentTypes.detect(path, content); + System.out.println("Content-Type: " + contentType); + + Optional language = contentType.getLanguage(); + if (language.isPresent()) { + responseBuilder.header("Content-Type", contentType); + } + responseBuilder.header("Content-Length", content.length); + } + + private byte[] getContent(@PathParam("revision") String revision, @PathParam("path") String path, RepositoryService repositoryService) throws IOException, RepositoryException { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + repositoryService.getCatCommand().setRevision(revision).retriveContent(outputStream, path); + return outputStream.toByteArray(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java index 43aa6de608..817eb29f11 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java @@ -35,6 +35,7 @@ public class RepositoryResource { private final Provider branchRootResource; private final Provider changesetRootResource; private final Provider sourceRootResource; + private final Provider contentResource; private final Provider permissionRootResource; @Inject @@ -44,7 +45,7 @@ public class RepositoryResource { Provider tagRootResource, Provider branchRootResource, Provider changesetRootResource, - Provider sourceRootResource, Provider permissionRootResource) { + Provider sourceRootResource, Provider contentResource, Provider permissionRootResource) { this.dtoToRepositoryMapper = dtoToRepositoryMapper; this.manager = manager; this.repositoryToDtoMapper = repositoryToDtoMapper; @@ -53,6 +54,7 @@ public class RepositoryResource { this.branchRootResource = branchRootResource; this.changesetRootResource = changesetRootResource; this.sourceRootResource = sourceRootResource; + this.contentResource = contentResource; this.permissionRootResource = permissionRootResource; } @@ -151,6 +153,11 @@ public class RepositoryResource { return sourceRootResource.get(); } + @Path("content/") + public ContentResource content() { + return contentResource.get(); + } + @Path("permissions/") public PermissionRootResource permissions() { return permissionRootResource.get(); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java index 711a5cf196..1b23b18f27 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java @@ -45,7 +45,7 @@ public class BranchRootResourceTest { public void prepareEnvironment() throws Exception { BranchCollectionToDtoMapper branchCollectionToDtoMapper = new BranchCollectionToDtoMapper(branchToDtoMapper, resourceLinks); BranchRootResource branchRootResource = new BranchRootResource(serviceFactory, branchToDtoMapper, branchCollectionToDtoMapper); - RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider.of(new RepositoryResource(null, null, null, null, MockProvider.of(branchRootResource), null, null, null)), null); + RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider.of(new RepositoryResource(null, null, null, null, MockProvider.of(branchRootResource), null, null, null, null)), null); dispatcher.getRegistry().addSingletonResource(repositoryRootResource); when(serviceFactory.create(new NamespaceAndName("space", "repo"))).thenReturn(service); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java index 75d09a2414..d47b26e5eb 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java @@ -74,7 +74,7 @@ public class RepositoryRootResourceTest { @Before public void prepareEnvironment() { initMocks(this); - RepositoryResource repositoryResource = new RepositoryResource(repositoryToDtoMapper, dtoToRepositoryMapper, repositoryManager, null, null, null, null, null); + RepositoryResource repositoryResource = new RepositoryResource(repositoryToDtoMapper, dtoToRepositoryMapper, repositoryManager, null, null, null, null, null, null); RepositoryCollectionToDtoMapper repositoryCollectionToDtoMapper = new RepositoryCollectionToDtoMapper(repositoryToDtoMapper, resourceLinks); RepositoryCollectionResource repositoryCollectionResource = new RepositoryCollectionResource(repositoryManager, repositoryCollectionToDtoMapper, dtoToRepositoryMapper, resourceLinks); RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider.of(repositoryResource), MockProvider.of(repositoryCollectionResource)); From b197f92945853a1a010ee255db0ad865a93672a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 10 Aug 2018 16:32:56 +0200 Subject: [PATCH 02/20] Remove wrong annotations --- .../main/java/sonia/scm/api/v2/resources/ContentResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index dc14d00e54..9d15f14f50 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -88,7 +88,7 @@ public class ContentResource { responseBuilder.header("Content-Length", content.length); } - private byte[] getContent(@PathParam("revision") String revision, @PathParam("path") String path, RepositoryService repositoryService) throws IOException, RepositoryException { + private byte[] getContent(String revision, String path, RepositoryService repositoryService) throws IOException, RepositoryException { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); repositoryService.getCatCommand().setRevision(revision).retriveContent(outputStream, path); return outputStream.toByteArray(); From 815cee156933a7747f4ea4c0ff26e6d4939c138e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 13 Aug 2018 15:52:50 +0200 Subject: [PATCH 03/20] Return content as stream --- .../scm/api/v2/resources/ContentResource.java | 50 ++++--- .../api/v2/resources/ContentResourceTest.java | 131 ++++++++++++++++++ scm-webapp/src/test/resources/Dockerfile | 8 ++ scm-webapp/src/test/resources/JustBytes | Bin 0 -> 1024 bytes scm-webapp/src/test/resources/SomeGoCode.go | 1 + 5 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java create mode 100644 scm-webapp/src/test/resources/Dockerfile create mode 100644 scm-webapp/src/test/resources/JustBytes create mode 100644 scm-webapp/src/test/resources/SomeGoCode.go diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 9d15f14f50..06acec1409 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -2,7 +2,8 @@ package sonia.scm.api.v2.resources; import com.github.sdorra.spotter.ContentType; import com.github.sdorra.spotter.ContentTypes; -import com.github.sdorra.spotter.Language; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; @@ -16,12 +17,14 @@ import javax.ws.rs.HEAD; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.util.Optional; public class ContentResource { + private static final Logger LOG = LoggerFactory.getLogger(ContentResource.class); + private final RepositoryServiceFactory servicefactory; @Inject @@ -34,17 +37,27 @@ public class ContentResource { public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = servicefactory.create(new NamespaceAndName(namespace, name))) { try { - byte[] content = getContent(revision, path, repositoryService); - Response.ResponseBuilder responseBuilder = Response.ok(content); - appendContentType(path, content, responseBuilder); + + StreamingOutput stream = os -> { + try { + repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); + } catch (RepositoryException e) { + e.printStackTrace(); + } + os.close(); + }; + + Response.ResponseBuilder responseBuilder = Response.ok(stream); + appendContentType(path, getHead(revision, path, repositoryService), responseBuilder); + return responseBuilder.build(); } catch (PathNotFoundException e) { return Response.status(404).build(); } catch (IOException e) { - e.printStackTrace(); + LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); return Response.status(500).entity(e.getMessage()).build(); } catch (RepositoryException e) { - e.printStackTrace(); + LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); return Response.status(500).entity(e.getMessage()).build(); } } catch (RepositoryNotFoundException e) { @@ -57,19 +70,18 @@ public class ContentResource { public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = servicefactory.create(new NamespaceAndName(namespace, name))) { try { - byte[] content = getContent(revision, path, repositoryService); Response.ResponseBuilder responseBuilder = Response.ok(); - appendContentType(path, content, responseBuilder); + appendContentType(path, getHead(revision, path, repositoryService), responseBuilder); return responseBuilder.build(); } catch (PathNotFoundException e) { return Response.status(404).build(); } catch (IOException e) { - e.printStackTrace(); + LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); return Response.status(500).entity(e.getMessage()).build(); } catch (RepositoryException e) { - e.printStackTrace(); + LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); return Response.status(500).entity(e.getMessage()).build(); } } catch (RepositoryNotFoundException e) { @@ -77,18 +89,14 @@ public class ContentResource { } } - private void appendContentType(String path, byte[] content, Response.ResponseBuilder responseBuilder) { - ContentType contentType = ContentTypes.detect(path, content); - System.out.println("Content-Type: " + contentType); - - Optional language = contentType.getLanguage(); - if (language.isPresent()) { - responseBuilder.header("Content-Type", contentType); - } - responseBuilder.header("Content-Length", content.length); + private void appendContentType(String path, byte[] head, Response.ResponseBuilder responseBuilder) { + ContentType contentType = ContentTypes.detect(path, head); + responseBuilder.header("Content-Type", contentType.getRaw()); + contentType.getLanguage().ifPresent(language -> responseBuilder.header("Language", language)); + responseBuilder.header("Content-Length", head.length); } - private byte[] getContent(String revision, String path, RepositoryService repositoryService) throws IOException, RepositoryException { + private byte[] getHead(String revision, String path, RepositoryService repositoryService) throws IOException, RepositoryException { ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); repositoryService.getCatCommand().setRevision(revision).retriveContent(outputStream, path); return outputStream.toByteArray(); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java new file mode 100644 index 0000000000..0493ef5289 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java @@ -0,0 +1,131 @@ +package sonia.scm.api.v2.resources; + +import com.google.common.io.Resources; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Answers; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.PathNotFoundException; +import sonia.scm.repository.RepositoryNotFoundException; +import sonia.scm.repository.api.CatCommandBuilder; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; + +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; +import java.net.URL; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.AdditionalMatchers.not; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class ContentResourceTest { + + private static final String NAMESPACE = "space"; + private static final String REPO_NAME = "name"; + private static final String REV = "rev"; + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private RepositoryServiceFactory repositoryServiceFactory; + + @InjectMocks + private ContentResource contentResource; + + private CatCommandBuilder catCommand; + + @Before + public void initService() throws Exception { + NamespaceAndName existingNamespaceAndName = new NamespaceAndName(NAMESPACE, REPO_NAME); + RepositoryService repositoryService = repositoryServiceFactory.create(existingNamespaceAndName); + catCommand = repositoryService.getCatCommand(); + when(catCommand.setRevision(REV)).thenReturn(catCommand); + + // defaults for unknown things + doThrow(new RepositoryNotFoundException("x")).when(repositoryServiceFactory).create(not(eq(existingNamespaceAndName))); + doThrow(new PathNotFoundException("x")).when(catCommand).retriveContent(any(), any()); + } + + @Test + public void shouldReadSimpleFile() throws Exception { + mockContent("file", "Hello".getBytes()); + + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "file"); + assertEquals(200, response.getStatus()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ((StreamingOutput) response.getEntity()).write(baos); + + assertEquals("Hello", baos.toString()); + } + + @Test + public void shouldHandleMissingFile() { + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "doesNotExist"); + assertEquals(404, response.getStatus()); + } + + @Test + public void shouldHandleMissingRepository() { + Response response = contentResource.get("no", "repo", REV, "anything"); + assertEquals(404, response.getStatus()); + } + + @Test + public void shouldRecognizeTikaSourceCode() throws Exception { + mockContentFromResource("SomeGoCode.go"); + + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "SomeGoCode.go"); + assertEquals(200, response.getStatus()); + + assertEquals("GO", response.getHeaderString("Language")); + assertEquals("text/x-go", response.getHeaderString("Content-Type")); + } + + @Test + public void shouldRecognizeSpecialSourceCode() throws Exception { + mockContentFromResource("Dockerfile"); + + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "Dockerfile"); + assertEquals(200, response.getStatus()); + + assertEquals("DOCKERFILE", response.getHeaderString("Language")); + assertEquals("text/plain", response.getHeaderString("Content-Type")); + } + + @Test + public void shouldRandomByteFile() throws Exception { + mockContentFromResource("JustBytes"); + + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "JustBytes"); + assertEquals(200, response.getStatus()); + + assertFalse(response.getHeaders().containsKey("Language")); + assertEquals("application/octet-stream", response.getHeaderString("Content-Type")); + } + + private void mockContentFromResource(String fileName) throws Exception { + URL url = Resources.getResource(fileName); + mockContent(fileName, Resources.toByteArray(url)); + } + + private void mockContent(String path, byte[] content) throws Exception { + doAnswer(invocation -> { + OutputStream outputStream = (OutputStream) invocation.getArguments()[0]; + outputStream.write(content); + outputStream.close(); + return null; + }).when(catCommand).retriveContent(any(), eq(path)); + } +} diff --git a/scm-webapp/src/test/resources/Dockerfile b/scm-webapp/src/test/resources/Dockerfile new file mode 100644 index 0000000000..d4b48dfd45 --- /dev/null +++ b/scm-webapp/src/test/resources/Dockerfile @@ -0,0 +1,8 @@ +FROM ubuntu + +COPY nothing /nowhere + +RUN apt-get update + +EXEC shutdhown -h now + diff --git a/scm-webapp/src/test/resources/JustBytes b/scm-webapp/src/test/resources/JustBytes new file mode 100644 index 0000000000000000000000000000000000000000..f455a92b4710106feb2670ffef3d213ea01a5958 GIT binary patch literal 1024 zcmV+b1poWV;w+C0=6yeghc-S^vF^!TEtD=Y13BL(*yv5Z!`(7r z+>&z#<>A(;$E&wSEqsx2>%`XkXQo?U=d7v7Fpag(zz+1l;|e29ZJkUh5Q?M#a~y?A zSm%VxBJhvmoOCDqspgwr)R3`L*xqAA(`|xpuyNIpA$aKE_}Q5~a!v?)gOv-R zKH-%0!R?fqv6F&pL|f-%b;;sH@oXg)Rf;PW_?nc^(uEhH2j|#@u3o#R7V_iQx7*=EGt2JaV1|TbG*r@%EcNUKIv`rFGR;Ib>G7WuK3941i>(V%PqRvMb zb{x0#mTfkc(8$AOXGhq=wd8I)yvG)cm|C>O0)e&r@&L47A&GE`3^x3-rJ^DNTp&+# z=s?FFfwFJ>hdEqBy70to{N;40Es1ZjjX6s5KA_G|nX!Uk2aJGXvy;8Aqs`{U`6|`D z4J^#p&d+u*VE}&?Ya}bJ&3o&jNg@n!9PSV=-Mh1Ur~&5iLQRM=YpI{6+H(3<>%|0b>f&G z+S_1QH53r3jzT{Q`iz1WVkfiOI$LmmBW8X{OsaM=hZnsQB_&IP3dwlQh7v=5_(h4| zLnSC6Pf4FiM%tvVV-=j_BAtZjN;ki~ES>9K=(6jj0ft-Pi~yZl3*g}7OW@Xe*y25j zget~FiTwwHsI7wELm?@jyHJSbiOrpC2n8YGd5ISj*JhF zQjkP)MoommIoqGvKS>2@&XM1*^D+ME|3OO??a4AL^VfjaEhgs08oEiBempOr%fsW; zf}voCvmfGaw`vf`c63aVG2Sb#GBB1_VEkm)gtmhb0a1jF1smA)&zxqA=n1jc-0Kh= uY Date: Tue, 14 Aug 2018 07:52:24 +0200 Subject: [PATCH 04/20] Name test method --- .../java/sonia/scm/api/v2/resources/ContentResourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java index 0493ef5289..b2434905e7 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java @@ -105,7 +105,7 @@ public class ContentResourceTest { } @Test - public void shouldRandomByteFile() throws Exception { + public void shouldHandleRandomByteFile() throws Exception { mockContentFromResource("JustBytes"); Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "JustBytes"); From fdaff02e01a364a67986885b5236353b4872ab6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 14 Aug 2018 12:58:03 +0200 Subject: [PATCH 05/20] Handle exception in stream processing --- .../scm/api/v2/resources/ContentResource.java | 22 +++++++++++-------- .../api/v2/resources/ContentResourceTest.java | 19 ++++++++++++++-- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 06acec1409..4108a7634f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -16,7 +16,9 @@ import javax.ws.rs.GET; import javax.ws.rs.HEAD; import javax.ws.rs.Path; import javax.ws.rs.PathParam; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.StreamingOutput; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -41,8 +43,10 @@ public class ContentResource { StreamingOutput stream = os -> { try { repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); + } catch (PathNotFoundException e) { + throw new WebApplicationException(Status.NOT_FOUND); } catch (RepositoryException e) { - e.printStackTrace(); + throw new WebApplicationException(Status.INTERNAL_SERVER_ERROR); } os.close(); }; @@ -52,16 +56,16 @@ public class ContentResource { return responseBuilder.build(); } catch (PathNotFoundException e) { - return Response.status(404).build(); + return Response.status(Status.NOT_FOUND).build(); } catch (IOException e) { LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(500).entity(e.getMessage()).build(); + return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); } catch (RepositoryException e) { LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(500).entity(e.getMessage()).build(); + return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); } } catch (RepositoryNotFoundException e) { - return Response.status(404).build(); + return Response.status(Status.NOT_FOUND).build(); } } @@ -76,16 +80,16 @@ public class ContentResource { appendContentType(path, getHead(revision, path, repositoryService), responseBuilder); return responseBuilder.build(); } catch (PathNotFoundException e) { - return Response.status(404).build(); + return Response.status(Status.NOT_FOUND).build(); } catch (IOException e) { LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(500).entity(e.getMessage()).build(); + return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); } catch (RepositoryException e) { LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(500).entity(e.getMessage()).build(); + return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); } } catch (RepositoryNotFoundException e) { - return Response.status(404).build(); + return Response.status(Status.NOT_FOUND).build(); } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java index b2434905e7..23544e8cf6 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java @@ -18,6 +18,7 @@ import sonia.scm.repository.api.RepositoryServiceFactory; import javax.ws.rs.core.Response; import javax.ws.rs.core.StreamingOutput; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.OutputStream; import java.net.URL; @@ -64,8 +65,7 @@ public class ContentResourceTest { Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "file"); assertEquals(200, response.getStatus()); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - ((StreamingOutput) response.getEntity()).write(baos); + ByteArrayOutputStream baos = readOutputStream(response); assertEquals("Hello", baos.toString()); } @@ -115,6 +115,15 @@ public class ContentResourceTest { assertEquals("application/octet-stream", response.getHeaderString("Content-Type")); } + @Test + public void shouldHandleExceptionsInStreamProcessing() throws Exception { + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "MissingFile"); + + ByteArrayOutputStream baos = readOutputStream(response); + + assertEquals(404, response.getStatus()); + } + private void mockContentFromResource(String fileName) throws Exception { URL url = Resources.getResource(fileName); mockContent(fileName, Resources.toByteArray(url)); @@ -128,4 +137,10 @@ public class ContentResourceTest { return null; }).when(catCommand).retriveContent(any(), eq(path)); } + + private ByteArrayOutputStream readOutputStream(Response response) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ((StreamingOutput) response.getEntity()).write(baos); + return baos; + } } From dcdab9a00e26ca71da0a7508367b6b0c30819936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 14 Aug 2018 13:09:43 +0200 Subject: [PATCH 06/20] Fix typo --- .../sonia/scm/api/v2/resources/ContentResource.java | 10 +++++----- .../scm/api/v2/resources/ContentResourceTest.java | 9 --------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 4108a7634f..82b75a15ea 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -27,17 +27,17 @@ public class ContentResource { private static final Logger LOG = LoggerFactory.getLogger(ContentResource.class); - private final RepositoryServiceFactory servicefactory; + private final RepositoryServiceFactory serviceFactory; @Inject - public ContentResource(RepositoryServiceFactory servicefactory) { - this.servicefactory = servicefactory; + public ContentResource(RepositoryServiceFactory serviceFactory) { + this.serviceFactory = serviceFactory; } @GET @Path("{revision}/{path: .*}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { - try (RepositoryService repositoryService = servicefactory.create(new NamespaceAndName(namespace, name))) { + try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { try { StreamingOutput stream = os -> { @@ -72,7 +72,7 @@ public class ContentResource { @HEAD @Path("{revision}/{path: .*}") public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { - try (RepositoryService repositoryService = servicefactory.create(new NamespaceAndName(namespace, name))) { + try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { try { Response.ResponseBuilder responseBuilder = Response.ok(); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java index 23544e8cf6..3c926f7986 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java @@ -115,15 +115,6 @@ public class ContentResourceTest { assertEquals("application/octet-stream", response.getHeaderString("Content-Type")); } - @Test - public void shouldHandleExceptionsInStreamProcessing() throws Exception { - Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "MissingFile"); - - ByteArrayOutputStream baos = readOutputStream(response); - - assertEquals(404, response.getStatus()); - } - private void mockContentFromResource(String fileName) throws Exception { URL url = Resources.getResource(fileName); mockContent(fileName, Resources.toByteArray(url)); From 99a17c51031cf1e7fd8d995d4f847e2adffcf4f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 14 Aug 2018 13:58:22 +0200 Subject: [PATCH 07/20] Log error cases --- .../scm/api/v2/resources/ContentResource.java | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 82b75a15ea..59aa720955 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -38,33 +38,23 @@ public class ContentResource { @Path("{revision}/{path: .*}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - try { - StreamingOutput stream = os -> { try { repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); } catch (PathNotFoundException e) { + LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); throw new WebApplicationException(Status.NOT_FOUND); } catch (RepositoryException e) { + LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); throw new WebApplicationException(Status.INTERNAL_SERVER_ERROR); } os.close(); }; - Response.ResponseBuilder responseBuilder = Response.ok(stream); - appendContentType(path, getHead(revision, path, repositoryService), responseBuilder); - - return responseBuilder.build(); - } catch (PathNotFoundException e) { - return Response.status(Status.NOT_FOUND).build(); - } catch (IOException e) { - LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); - } catch (RepositoryException e) { - LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); - } + Response.ResponseBuilder responseBuilder = Response.ok(stream); + return createContentHeader(namespace, name, revision, path, repositoryService, responseBuilder); } catch (RepositoryNotFoundException e) { + LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); return Response.status(Status.NOT_FOUND).build(); } } @@ -73,27 +63,31 @@ public class ContentResource { @Path("{revision}/{path: .*}") public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - try { - Response.ResponseBuilder responseBuilder = Response.ok(); - - appendContentType(path, getHead(revision, path, repositoryService), responseBuilder); - return responseBuilder.build(); - } catch (PathNotFoundException e) { - return Response.status(Status.NOT_FOUND).build(); - } catch (IOException e) { - LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); - } catch (RepositoryException e) { - LOG.error("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); - } + return createContentHeader(namespace, name, revision, path, repositoryService, responseBuilder); } catch (RepositoryNotFoundException e) { + LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); return Response.status(Status.NOT_FOUND).build(); } } - private void appendContentType(String path, byte[] head, Response.ResponseBuilder responseBuilder) { + private Response createContentHeader(String namespace, String name, String revision, String path, RepositoryService repositoryService, Response.ResponseBuilder responseBuilder) { + try { + appendContentHeader(path, getHead(revision, path, repositoryService), responseBuilder); + } catch (PathNotFoundException e) { + LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); + return Response.status(Status.NOT_FOUND).build(); + } catch (IOException e) { + LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); + return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); + } catch (RepositoryException e) { + LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); + return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); + } + return responseBuilder.build(); + } + + private void appendContentHeader(String path, byte[] head, Response.ResponseBuilder responseBuilder) { ContentType contentType = ContentTypes.detect(path, head); responseBuilder.header("Content-Type", contentType.getRaw()); contentType.getLanguage().ifPresent(language -> responseBuilder.header("Language", language)); From 2266b971a9c7716232d1ec6a38ccd2c06ea847df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 14 Aug 2018 14:13:26 +0200 Subject: [PATCH 08/20] Make smaller functions --- .../scm/api/v2/resources/ContentResource.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 59aa720955..014b9b90ef 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -38,19 +38,7 @@ public class ContentResource { @Path("{revision}/{path: .*}") public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - StreamingOutput stream = os -> { - try { - repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); - } catch (PathNotFoundException e) { - LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); - throw new WebApplicationException(Status.NOT_FOUND); - } catch (RepositoryException e) { - LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); - throw new WebApplicationException(Status.INTERNAL_SERVER_ERROR); - } - os.close(); - }; - + StreamingOutput stream = createStreamingOutput(namespace, name, revision, path, repositoryService); Response.ResponseBuilder responseBuilder = Response.ok(stream); return createContentHeader(namespace, name, revision, path, repositoryService, responseBuilder); } catch (RepositoryNotFoundException e) { @@ -59,6 +47,21 @@ public class ContentResource { } } + private StreamingOutput createStreamingOutput(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path, RepositoryService repositoryService) { + return os -> { + try { + repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); + os.close(); + } catch (PathNotFoundException e) { + LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); + throw new WebApplicationException(Status.NOT_FOUND); + } catch (RepositoryException e) { + LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); + throw new WebApplicationException(Status.INTERNAL_SERVER_ERROR); + } + }; + } + @HEAD @Path("{revision}/{path: .*}") public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { From c8c1cad67fd5ba9e5fc333851a1a61373e61937a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 14 Aug 2018 17:22:30 +0200 Subject: [PATCH 09/20] Add raw stream result to cat command --- .../scm/repository/api/CatCommandBuilder.java | 39 +++--- .../sonia/scm/repository/spi/CatCommand.java | 20 +--- .../scm/repository/spi/GitCatCommand.java | 112 ++++++++++++------ .../scm/repository/spi/HgCatCommand.java | 24 ++-- .../scm/repository/spi/SvnCatCommand.java | 19 ++- 5 files changed, 134 insertions(+), 80 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java index f7b4d75fbd..8a08663f25 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java @@ -37,22 +37,21 @@ package sonia.scm.repository.api; import com.google.common.base.Preconditions; import com.google.common.base.Strings; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.spi.CatCommand; import sonia.scm.repository.spi.CatCommandRequest; import sonia.scm.util.IOUtil; -//~--- JDK imports ------------------------------------------------------------ - import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +//~--- JDK imports ------------------------------------------------------------ + /** * Shows the content of a file in the {@link Repository}.
*
@@ -106,23 +105,31 @@ public final class CatCommandBuilder } /** - * Passes the content of the given file to the outputstream. + * Passes the content of the given file to the output stream. * - * @param outputStream outputstream for the content + * @param outputStream output stream for the content * @param path file path - * - * @return {@code this} - * - * @throws IOException - * @throws RepositoryException */ - public CatCommandBuilder retriveContent(OutputStream outputStream, - String path) - throws IOException, RepositoryException - { + public void retriveContent(OutputStream outputStream, String path) throws IOException, RepositoryException { getCatResult(outputStream, path); + } - return this; + /** + * Returns an output stream with the file content. + * + * @param path file path + */ + public InputStream getStream(String path) throws IOException, RepositoryException { + Preconditions.checkArgument(!Strings.isNullOrEmpty(path), + "path is required"); + + CatCommandRequest requestClone = request.clone(); + + requestClone.setPath(path); + + logger.debug("create cat stream for {}", requestClone); + + return catCommand.getCatResultStream(requestClone); } //~--- get methods ---------------------------------------------------------- diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java index b9a6f52ab0..cbf1866298 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java @@ -37,11 +37,12 @@ package sonia.scm.repository.spi; import sonia.scm.repository.RepositoryException; -//~--- JDK imports ------------------------------------------------------------ - import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +//~--- JDK imports ------------------------------------------------------------ + /** * * @author Sebastian Sdorra @@ -50,16 +51,7 @@ import java.io.OutputStream; public interface CatCommand { - /** - * Method description - * - * - * @param request - * @param output - * - * @throws IOException - * @throws RepositoryException - */ - public void getCatResult(CatCommandRequest request, OutputStream output) - throws IOException, RepositoryException; + void getCatResult(CatCommandRequest request, OutputStream output) throws IOException, RepositoryException; + + InputStream getCatResultStream(CatCommandRequest request) throws IOException, RepositoryException; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java index 4420b9a22a..fdf8042c57 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java @@ -34,29 +34,29 @@ package sonia.scm.repository.spi; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.base.Strings; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.filter.PathFilter; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import sonia.scm.repository.GitUtil; import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; import sonia.scm.util.Util; -//~--- JDK imports ------------------------------------------------------------ - +import java.io.Closeable; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +//~--- JDK imports ------------------------------------------------------------ + /** * * @author Sebastian Sdorra @@ -110,22 +110,31 @@ public class GitCatCommand extends AbstractGitCommand implements CatCommand getContent(repo, revId, request.getPath(), output); } - /** - * Method description - * - * - * - * @param repo - * @param revId - * @param path - * @param output - * - * - * @throws IOException - * @throws RepositoryException - */ + @Override + public InputStream getCatResultStream(CatCommandRequest request) throws IOException, RepositoryException { + logger.debug("try to read content for {}", request); + + org.eclipse.jgit.lib.Repository repo = open(); + + ObjectId revId = getCommitOrDefault(repo, request.getRevision()); + ClosableObjectLoaderContainer closableObjectLoaderContainer = getLoader(repo, revId, request.getPath()); + return new InputStreamWrapper(closableObjectLoaderContainer); + } + void getContent(org.eclipse.jgit.lib.Repository repo, ObjectId revId, - String path, OutputStream output) + String path, OutputStream output) + throws IOException, RepositoryException + { + ClosableObjectLoaderContainer closableObjectLoaderContainer = getLoader(repo, revId, path); + try { + closableObjectLoaderContainer.objectLoader.copyTo(output); + } finally { + closableObjectLoaderContainer.close(); + } + } + + private ClosableObjectLoaderContainer getLoader(Repository repo, ObjectId revId, + String path) throws IOException, RepositoryException { TreeWalk treeWalk = null; @@ -157,23 +166,12 @@ public class GitCatCommand extends AbstractGitCommand implements CatCommand treeWalk.setFilter(PathFilter.create(path)); - if (treeWalk.next()) + if (treeWalk.next() && treeWalk.getFileMode(0).getObjectType() == Constants.OBJ_BLOB) { + ObjectId blobId = treeWalk.getObjectId(0); + ObjectLoader loader = repo.open(blobId); - // Path exists - if (treeWalk.getFileMode(0).getObjectType() == Constants.OBJ_BLOB) - { - ObjectId blobId = treeWalk.getObjectId(0); - ObjectLoader loader = repo.open(blobId); - - loader.copyTo(output); - } - else - { - - // Not a blob, its something else (tree, gitlink) - throw new PathNotFoundException(path); - } + return new ClosableObjectLoaderContainer(loader, treeWalk, revWalk); } else { @@ -182,8 +180,52 @@ public class GitCatCommand extends AbstractGitCommand implements CatCommand } finally { + } + } + + private static class ClosableObjectLoaderContainer implements Closeable { + private final ObjectLoader objectLoader; + private final TreeWalk treeWalk; + private final RevWalk revWalk; + + private ClosableObjectLoaderContainer(ObjectLoader objectLoader, TreeWalk treeWalk, RevWalk revWalk) { + this.objectLoader = objectLoader; + this.treeWalk = treeWalk; + this.revWalk = revWalk; + } + + @Override + public void close() throws IOException { GitUtil.release(revWalk); GitUtil.release(treeWalk); } } + + private static class InputStreamWrapper extends InputStream { + + private final InputStream delegate; + private final TreeWalk treeWalk; + private final RevWalk revWalk; + + private InputStreamWrapper(ClosableObjectLoaderContainer container) throws IOException { + this(container.objectLoader.openStream(), container.treeWalk, container.revWalk); + } + + private InputStreamWrapper(InputStream delegate, TreeWalk treeWalk, RevWalk revWalk) { + this.delegate = delegate; + this.treeWalk = treeWalk; + this.revWalk = revWalk; + } + + @Override + public int read () throws IOException { + return delegate.read(); + } + + @Override + public void close () throws IOException { + GitUtil.release(revWalk); + GitUtil.release(treeWalk); + } + } } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java index 76438c1f1e..a8e585915e 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java @@ -37,17 +37,16 @@ package sonia.scm.repository.spi; import com.google.common.io.ByteStreams; import com.google.common.io.Closeables; - import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; import sonia.scm.web.HgUtil; -//~--- JDK imports ------------------------------------------------------------ - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +//~--- JDK imports ------------------------------------------------------------ + /** * * @author Sebastian Sdorra @@ -83,16 +82,9 @@ public class HgCatCommand extends AbstractCommand implements CatCommand public void getCatResult(CatCommandRequest request, OutputStream output) throws IOException, RepositoryException { - com.aragost.javahg.commands.CatCommand cmd = - com.aragost.javahg.commands.CatCommand.on(open()); - - cmd.rev(HgUtil.getRevision(request.getRevision())); - - InputStream input = null; - + InputStream input = getCatResultStream(request); try { - input = cmd.execute(request.getPath()); ByteStreams.copy(input, output); } finally @@ -100,4 +92,14 @@ public class HgCatCommand extends AbstractCommand implements CatCommand Closeables.close(input, true); } } + + @Override + public InputStream getCatResultStream(CatCommandRequest request) throws IOException, RepositoryException { + com.aragost.javahg.commands.CatCommand cmd = + com.aragost.javahg.commands.CatCommand.on(open()); + + cmd.rev(HgUtil.getRevision(request.getRevision())); + + return cmd.execute(request.getPath()); + } } diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java index af08ea929e..5bd3c54b1b 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java @@ -37,22 +37,23 @@ package sonia.scm.repository.spi; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.tmatesoft.svn.core.SVNException; import org.tmatesoft.svn.core.SVNProperties; import org.tmatesoft.svn.core.io.SVNRepository; import org.tmatesoft.svn.core.wc.SVNClientManager; import org.tmatesoft.svn.core.wc.admin.SVNLookClient; - import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.SvnUtil; -//~--- JDK imports ------------------------------------------------------------ - +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; +//~--- JDK imports ------------------------------------------------------------ + /** * * @author Sebastian Sdorra @@ -120,6 +121,16 @@ public class SvnCatCommand extends AbstractSvnCommand implements CatCommand } } + @Override + public InputStream getCatResultStream(CatCommandRequest request) throws IOException, RepositoryException { + // There seems to be no method creating an input stream as a result, so + // we have no other possibility then to copy the content into a buffer and + // stream it from there. + ByteArrayOutputStream output = new ByteArrayOutputStream(); + getCatResult(request, output); + return new ByteArrayInputStream(output.toByteArray()); + } + /** * Method description * From c3a455145ab5f12a7decdd17a843450e8ee226ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 14 Aug 2018 17:31:27 +0200 Subject: [PATCH 10/20] Read header using direct stream --- .../scm/api/v2/resources/ContentResource.java | 20 +++++++++++++++---- .../api/v2/resources/ContentResourceTest.java | 15 +++++++++++++- scm-webapp/src/test/resources/someScript.sh | 6 ++++++ 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 scm-webapp/src/test/resources/someScript.sh diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 014b9b90ef..4182fb9343 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -10,6 +10,7 @@ import sonia.scm.repository.RepositoryException; import sonia.scm.repository.RepositoryNotFoundException; import sonia.scm.repository.api.RepositoryService; import sonia.scm.repository.api.RepositoryServiceFactory; +import sonia.scm.util.IOUtil; import javax.inject.Inject; import javax.ws.rs.GET; @@ -20,12 +21,14 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.StreamingOutput; -import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; public class ContentResource { private static final Logger LOG = LoggerFactory.getLogger(ContentResource.class); + private static final int HEAD_BUFFER_SIZE = 1024; private final RepositoryServiceFactory serviceFactory; @@ -98,8 +101,17 @@ public class ContentResource { } private byte[] getHead(String revision, String path, RepositoryService repositoryService) throws IOException, RepositoryException { - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - repositoryService.getCatCommand().setRevision(revision).retriveContent(outputStream, path); - return outputStream.toByteArray(); + InputStream stream = repositoryService.getCatCommand().setRevision(revision).getStream(path); + try { + byte[] buffer = new byte[HEAD_BUFFER_SIZE]; + int length = stream.read(buffer); + if (length < buffer.length) { + return Arrays.copyOf(buffer, length); + } else { + return buffer; + } + } finally { + IOUtil.close(stream); + } } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java index 3c926f7986..36f6c92c47 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java @@ -17,6 +17,7 @@ import sonia.scm.repository.api.RepositoryServiceFactory; import javax.ws.rs.core.Response; import javax.ws.rs.core.StreamingOutput; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -55,7 +56,7 @@ public class ContentResourceTest { // defaults for unknown things doThrow(new RepositoryNotFoundException("x")).when(repositoryServiceFactory).create(not(eq(existingNamespaceAndName))); - doThrow(new PathNotFoundException("x")).when(catCommand).retriveContent(any(), any()); + doThrow(new PathNotFoundException("x")).when(catCommand).getStream(any()); } @Test @@ -104,6 +105,17 @@ public class ContentResourceTest { assertEquals("text/plain", response.getHeaderString("Content-Type")); } + @Test + public void shouldRecognizeShebangSourceCode() throws Exception { + mockContentFromResource("someScript.sh"); + + Response response = contentResource.get(NAMESPACE, REPO_NAME, REV, "someScript.sh"); + assertEquals(200, response.getStatus()); + + assertEquals("PYTHON", response.getHeaderString("Language")); + assertEquals("application/x-sh", response.getHeaderString("Content-Type")); + } + @Test public void shouldHandleRandomByteFile() throws Exception { mockContentFromResource("JustBytes"); @@ -127,6 +139,7 @@ public class ContentResourceTest { outputStream.close(); return null; }).when(catCommand).retriveContent(any(), eq(path)); + doAnswer(invocation -> new ByteArrayInputStream(content)).when(catCommand).getStream(eq(path)); } private ByteArrayOutputStream readOutputStream(Response response) throws IOException { diff --git a/scm-webapp/src/test/resources/someScript.sh b/scm-webapp/src/test/resources/someScript.sh new file mode 100644 index 0000000000..0bcecb189d --- /dev/null +++ b/scm-webapp/src/test/resources/someScript.sh @@ -0,0 +1,6 @@ +#!/usr/bin/python + +for f in * ; do + ls $f +done + From 3038d6460c7ee5dd53fc3c40999a9fa670f491d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 15 Aug 2018 08:24:11 +0200 Subject: [PATCH 11/20] Verify header will not read complete file --- .../api/v2/resources/ContentResourceTest.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java index 36f6c92c47..9302a4fcfd 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ContentResourceTest.java @@ -20,11 +20,14 @@ import javax.ws.rs.core.StreamingOutput; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.net.URL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.AdditionalMatchers.not; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -127,6 +130,18 @@ public class ContentResourceTest { assertEquals("application/octet-stream", response.getHeaderString("Content-Type")); } + @Test + public void shouldNotReadCompleteFileForHead() throws Exception { + FailingAfterSomeBytesStream stream = new FailingAfterSomeBytesStream(); + doAnswer(invocation -> stream).when(catCommand).getStream(eq("readHeadOnly")); + + Response response = contentResource.metadata(NAMESPACE, REPO_NAME, REV, "readHeadOnly"); + assertEquals(200, response.getStatus()); + + assertEquals("application/octet-stream", response.getHeaderString("Content-Type")); + assertTrue("stream has to be closed after reading head", stream.isClosed()); + } + private void mockContentFromResource(String fileName) throws Exception { URL url = Resources.getResource(fileName); mockContent(fileName, Resources.toByteArray(url)); @@ -147,4 +162,25 @@ public class ContentResourceTest { ((StreamingOutput) response.getEntity()).write(baos); return baos; } + + private static class FailingAfterSomeBytesStream extends InputStream { + private int bytesRead = 0; + private boolean closed = false; + @Override + public int read() { + if (++bytesRead > 1024) { + fail("read too many bytes"); + } + return 0; + } + + @Override + public void close() throws IOException { + closed = true; + } + + public boolean isClosed() { + return closed; + } + } } From 267ef50e511d293ddedbc7a1cc41ff986d965830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 15 Aug 2018 09:24:21 +0200 Subject: [PATCH 12/20] Clean up git cat command --- .../scm/repository/spi/GitCatCommand.java | 172 +++++------------- .../scm/repository/spi/GitCatCommandTest.java | 79 ++++---- 2 files changed, 80 insertions(+), 171 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java index fdf8042c57..0748a42803 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java @@ -32,8 +32,6 @@ package sonia.scm.repository.spi; -//~--- non-JDK imports -------------------------------------------------------- - import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; @@ -51,135 +49,72 @@ import sonia.scm.repository.RepositoryException; import sonia.scm.util.Util; import java.io.Closeable; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -//~--- JDK imports ------------------------------------------------------------ -/** - * - * @author Sebastian Sdorra - */ -public class GitCatCommand extends AbstractGitCommand implements CatCommand -{ +public class GitCatCommand extends AbstractGitCommand implements CatCommand { - /** - * the logger for GitCatCommand - */ - private static final Logger logger = - LoggerFactory.getLogger(GitCatCommand.class); + private static final Logger logger = LoggerFactory.getLogger(GitCatCommand.class); - //~--- constructors --------------------------------------------------------- - - /** - * Constructs ... - * - * - * - * @param context - * @param repository - */ - public GitCatCommand(GitContext context, - sonia.scm.repository.Repository repository) - { + public GitCatCommand(GitContext context, sonia.scm.repository.Repository repository) { super(context, repository); } - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param request - * @param output - * - * @throws IOException - * @throws RepositoryException - */ @Override - public void getCatResult(CatCommandRequest request, OutputStream output) - throws IOException, RepositoryException - { + public void getCatResult(CatCommandRequest request, OutputStream output) throws IOException, RepositoryException { logger.debug("try to read content for {}", request); - - org.eclipse.jgit.lib.Repository repo = open(); - - ObjectId revId = getCommitOrDefault(repo, request.getRevision()); - getContent(repo, revId, request.getPath(), output); + try (ClosableObjectLoaderContainer closableObjectLoaderContainer = getLoader(request)) { + closableObjectLoaderContainer.objectLoader.copyTo(output); + } } @Override public InputStream getCatResultStream(CatCommandRequest request) throws IOException, RepositoryException { logger.debug("try to read content for {}", request); - - org.eclipse.jgit.lib.Repository repo = open(); - - ObjectId revId = getCommitOrDefault(repo, request.getRevision()); - ClosableObjectLoaderContainer closableObjectLoaderContainer = getLoader(repo, revId, request.getPath()); - return new InputStreamWrapper(closableObjectLoaderContainer); + return new InputStreamWrapper(getLoader(request)); } - void getContent(org.eclipse.jgit.lib.Repository repo, ObjectId revId, - String path, OutputStream output) - throws IOException, RepositoryException - { - ClosableObjectLoaderContainer closableObjectLoaderContainer = getLoader(repo, revId, path); - try { + void getContent(org.eclipse.jgit.lib.Repository repo, ObjectId revId, String path, OutputStream output) throws IOException, RepositoryException { + try (ClosableObjectLoaderContainer closableObjectLoaderContainer = getLoader(repo, revId, path)) { closableObjectLoaderContainer.objectLoader.copyTo(output); - } finally { - closableObjectLoaderContainer.close(); } } - private ClosableObjectLoaderContainer getLoader(Repository repo, ObjectId revId, - String path) - throws IOException, RepositoryException - { - TreeWalk treeWalk = null; - RevWalk revWalk = null; + private ClosableObjectLoaderContainer getLoader(CatCommandRequest request) throws IOException, RepositoryException { + org.eclipse.jgit.lib.Repository repo = open(); + ObjectId revId = getCommitOrDefault(repo, request.getRevision()); + return getLoader(repo, revId, request.getPath()); + } - try - { - treeWalk = new TreeWalk(repo); - treeWalk.setRecursive(Util.nonNull(path).contains("/")); + private ClosableObjectLoaderContainer getLoader(Repository repo, ObjectId revId, String path) throws IOException, RepositoryException { + TreeWalk treeWalk = new TreeWalk(repo); + treeWalk.setRecursive(Util.nonNull(path).contains("/")); - if (logger.isDebugEnabled()) - { - logger.debug("load content for {} at {}", path, revId.name()); - } + logger.debug("load content for {} at {}", path, revId.name()); - revWalk = new RevWalk(repo); + RevWalk revWalk = new RevWalk(repo); - RevCommit entry = revWalk.parseCommit(revId); - RevTree revTree = entry.getTree(); + RevCommit entry = revWalk.parseCommit(revId); + RevTree revTree = entry.getTree(); - if (revTree != null) - { - treeWalk.addTree(revTree); - } - else - { - logger.error("could not find tree for {}", revId.name()); - } - - treeWalk.setFilter(PathFilter.create(path)); - - if (treeWalk.next() && treeWalk.getFileMode(0).getObjectType() == Constants.OBJ_BLOB) - { - ObjectId blobId = treeWalk.getObjectId(0); - ObjectLoader loader = repo.open(blobId); - - return new ClosableObjectLoaderContainer(loader, treeWalk, revWalk); - } - else - { - throw new PathNotFoundException(path); - } + if (revTree != null) { + treeWalk.addTree(revTree); + } else { + logger.error("could not find tree for {}", revId.name()); } - finally - { + + treeWalk.setFilter(PathFilter.create(path)); + + if (treeWalk.next() && treeWalk.getFileMode(0).getObjectType() == Constants.OBJ_BLOB) { + ObjectId blobId = treeWalk.getObjectId(0); + ObjectLoader loader = repo.open(blobId); + + return new ClosableObjectLoaderContainer(loader, treeWalk, revWalk); + } else { + throw new PathNotFoundException(path); } } @@ -195,37 +130,28 @@ public class GitCatCommand extends AbstractGitCommand implements CatCommand } @Override - public void close() throws IOException { + public void close() { GitUtil.release(revWalk); GitUtil.release(treeWalk); } } - private static class InputStreamWrapper extends InputStream { + private static class InputStreamWrapper extends FilterInputStream { - private final InputStream delegate; - private final TreeWalk treeWalk; - private final RevWalk revWalk; + private final ClosableObjectLoaderContainer container; private InputStreamWrapper(ClosableObjectLoaderContainer container) throws IOException { - this(container.objectLoader.openStream(), container.treeWalk, container.revWalk); + super(container.objectLoader.openStream()); + this.container = container; } - private InputStreamWrapper(InputStream delegate, TreeWalk treeWalk, RevWalk revWalk) { - this.delegate = delegate; - this.treeWalk = treeWalk; - this.revWalk = revWalk; - } - - @Override - public int read () throws IOException { - return delegate.read(); - } - - @Override - public void close () throws IOException { - GitUtil.release(revWalk); - GitUtil.release(treeWalk); + @Override + public void close() throws IOException { + try { + super.close(); + } finally { + container.close(); } } + } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java index ede6a53429..2452d779d7 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java @@ -32,19 +32,16 @@ package sonia.scm.repository.spi; -//~--- non-JDK imports -------------------------------------------------------- - import org.junit.Test; - +import sonia.scm.repository.GitConstants; +import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; -import static org.junit.Assert.*; - -//~--- JDK imports ------------------------------------------------------------ - import java.io.ByteArrayOutputStream; import java.io.IOException; -import sonia.scm.repository.GitConstants; +import java.io.InputStream; + +import static org.junit.Assert.assertEquals; /** * Unit tests for {@link GitCatCommand}. @@ -53,15 +50,8 @@ import sonia.scm.repository.GitConstants; * * @author Sebastian Sdorra */ -public class GitCatCommandTest extends AbstractGitCommandTestBase -{ +public class GitCatCommandTest extends AbstractGitCommandTestBase { - /** - * Tests cat command with default branch. - * - * @throws IOException - * @throws RepositoryException - */ @Test public void testDefaultBranch() throws IOException, RepositoryException { // without default branch, the repository head should be used @@ -75,16 +65,8 @@ public class GitCatCommandTest extends AbstractGitCommandTestBase assertEquals("a and b", execute(request)); } - /** - * Method description - * - * - * @throws IOException - * @throws RepositoryException - */ @Test - public void testCat() throws IOException, RepositoryException - { + public void testCat() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); request.setPath("a.txt"); @@ -92,36 +74,37 @@ public class GitCatCommandTest extends AbstractGitCommandTestBase assertEquals("a and b", execute(request)); } - /** - * Method description - * - * - * @throws IOException - * @throws RepositoryException - */ @Test - public void testSimpleCat() throws IOException, RepositoryException - { + public void testSimpleCat() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); request.setPath("b.txt"); assertEquals("b", execute(request)); } - /** - * Method description - * - * - * @param request - * - * @return - * - * @throws IOException - * @throws RepositoryException - */ - private String execute(CatCommandRequest request) - throws IOException, RepositoryException - { + @Test(expected = PathNotFoundException.class) + public void testUnknownFile() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + + request.setPath("unknown"); + assertEquals("b", execute(request)); + } + + @Test + public void testSimpleStream() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + request.setPath("b.txt"); + + InputStream catResultStream = new GitCatCommand(createContext(), repository).getCatResultStream(request); + + assertEquals('b', catResultStream.read()); + assertEquals('\n', catResultStream.read()); + assertEquals(-1, catResultStream.read()); + + catResultStream.close(); + } + + private String execute(CatCommandRequest request) throws IOException, RepositoryException { String content = null; ByteArrayOutputStream baos = new ByteArrayOutputStream(); From 5d6fcffe5e312b74ffe913e6f5d940eb3241a453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 15 Aug 2018 09:59:35 +0200 Subject: [PATCH 13/20] Clean up hg cat command --- .../scm/repository/spi/HgCatCommand.java | 51 +++-------- .../scm/repository/spi/HgCatCommandTest.java | 90 +++++++------------ 2 files changed, 44 insertions(+), 97 deletions(-) diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java index a8e585915e..c0ae0b51b5 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgCatCommand.java @@ -33,8 +33,7 @@ package sonia.scm.repository.spi; -//~--- non-JDK imports -------------------------------------------------------- - +import com.aragost.javahg.commands.ExecutionException; import com.google.common.io.ByteStreams; import com.google.common.io.Closeables; import sonia.scm.repository.Repository; @@ -45,50 +44,18 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -//~--- JDK imports ------------------------------------------------------------ +public class HgCatCommand extends AbstractCommand implements CatCommand { -/** - * - * @author Sebastian Sdorra - */ -public class HgCatCommand extends AbstractCommand implements CatCommand -{ - - /** - * Constructs ... - * - * - * @param context - * @param repository - */ - HgCatCommand(HgCommandContext context, Repository repository) - { + HgCatCommand(HgCommandContext context, Repository repository) { super(context, repository); } - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param request - * @param output - * - * @throws IOException - * @throws RepositoryException - */ @Override - public void getCatResult(CatCommandRequest request, OutputStream output) - throws IOException, RepositoryException - { + public void getCatResult(CatCommandRequest request, OutputStream output) throws IOException, RepositoryException { InputStream input = getCatResultStream(request); - try - { + try { ByteStreams.copy(input, output); - } - finally - { + } finally { Closeables.close(input, true); } } @@ -100,6 +67,10 @@ public class HgCatCommand extends AbstractCommand implements CatCommand cmd.rev(HgUtil.getRevision(request.getRevision())); - return cmd.execute(request.getPath()); + try { + return cmd.execute(request.getPath()); + } catch (ExecutionException e) { + throw new RepositoryException(e); + } } } diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java index 49ef283c1a..c297182b82 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java @@ -33,36 +33,21 @@ package sonia.scm.repository.spi; -//~--- non-JDK imports -------------------------------------------------------- - import org.junit.Test; - import sonia.scm.repository.RepositoryException; -import static org.junit.Assert.*; - -//~--- JDK imports ------------------------------------------------------------ - import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; -/** - * - * @author Sebastian Sdorra - */ -public class HgCatCommandTest extends AbstractHgCommandTestBase -{ +import static org.junit.Assert.assertEquals; + +//~--- JDK imports ------------------------------------------------------------ + +public class HgCatCommandTest extends AbstractHgCommandTestBase { - /** - * Method description - * - * - * @throws IOException - * @throws RepositoryException - */ @Test - public void testCat() throws IOException, RepositoryException - { + public void testCat() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); request.setPath("a.txt"); @@ -70,48 +55,39 @@ public class HgCatCommandTest extends AbstractHgCommandTestBase assertEquals("a", execute(request)); } - /** - * Method description - * - * - * @throws IOException - * @throws RepositoryException - */ @Test - public void testSimpleCat() throws IOException, RepositoryException - { + public void testSimpleCat() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); request.setPath("b.txt"); assertEquals("b", execute(request)); } - /** - * Method description - * - * - * @param request - * - * @return - * - * @throws IOException - * @throws RepositoryException - */ - private String execute(CatCommandRequest request) - throws IOException, RepositoryException - { - String content = null; + @Test(expected = RepositoryException.class) + public void testUnknownFile() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + + request.setPath("unknown"); + execute(request); + } + + @Test + public void testSimpleStream() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + request.setPath("b.txt"); + + InputStream catResultStream = new HgCatCommand(cmdContext, repository).getCatResultStream(request); + + assertEquals('b', catResultStream.read()); + assertEquals('\n', catResultStream.read()); + assertEquals(-1, catResultStream.read()); + + catResultStream.close(); + } + + private String execute(CatCommandRequest request) throws IOException, RepositoryException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); - - try - { - new HgCatCommand(cmdContext, repository).getCatResult(request, baos); - } - finally - { - content = baos.toString().trim(); - } - - return content; + new HgCatCommand(cmdContext, repository).getCatResult(request, baos); + return baos.toString().trim(); } } From 78278b0dc8fe33e8d11c9204bd7f11b5edd1e93d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 15 Aug 2018 10:06:35 +0200 Subject: [PATCH 14/20] Remove irritating assert --- .../test/java/sonia/scm/repository/spi/GitCatCommandTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java index 2452d779d7..875d47c15b 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java @@ -87,7 +87,7 @@ public class GitCatCommandTest extends AbstractGitCommandTestBase { CatCommandRequest request = new CatCommandRequest(); request.setPath("unknown"); - assertEquals("b", execute(request)); + execute(request); } @Test From 90316d92acde30b9d21b244cd9739631f8673489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 15 Aug 2018 10:32:08 +0200 Subject: [PATCH 15/20] Clean up svn cat command --- .../scm/repository/spi/SvnCatCommand.java | 14 +++ .../scm/repository/spi/SvnCatCommandTest.java | 91 ++++++++++--------- 2 files changed, 60 insertions(+), 45 deletions(-) diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java index 5bd3c54b1b..4a8b907c29 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnCatCommand.java @@ -37,13 +37,16 @@ package sonia.scm.repository.spi; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.tmatesoft.svn.core.SVNErrorCode; import org.tmatesoft.svn.core.SVNException; import org.tmatesoft.svn.core.SVNProperties; import org.tmatesoft.svn.core.io.SVNRepository; import org.tmatesoft.svn.core.wc.SVNClientManager; import org.tmatesoft.svn.core.wc.admin.SVNLookClient; +import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryException; +import sonia.scm.repository.RevisionNotFoundException; import sonia.scm.repository.SvnUtil; import java.io.ByteArrayInputStream; @@ -157,6 +160,17 @@ public class SvnCatCommand extends AbstractSvnCommand implements CatCommand } catch (SVNException ex) { + handleSvnException(request, ex); + } + } + + private void handleSvnException(CatCommandRequest request, SVNException ex) throws RepositoryException { + int svnErrorCode = ex.getErrorMessage().getErrorCode().getCode(); + if (SVNErrorCode.FS_NOT_FOUND.getCode() == svnErrorCode) { + throw new PathNotFoundException(request.getPath()); + } else if (SVNErrorCode.FS_NO_SUCH_REVISION.getCode() == svnErrorCode) { + throw new RevisionNotFoundException(request.getRevision()); + } else { throw new RepositoryException("could not get content from revision", ex); } } diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnCatCommandTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnCatCommandTest.java index 198a55edf3..f7e2123bad 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnCatCommandTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnCatCommandTest.java @@ -32,36 +32,23 @@ package sonia.scm.repository.spi; -//~--- non-JDK imports -------------------------------------------------------- - import org.junit.Test; - +import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; - -import static org.junit.Assert.*; - -//~--- JDK imports ------------------------------------------------------------ +import sonia.scm.repository.RevisionNotFoundException; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; -/** - * - * @author Sebastian Sdorra - */ -public class SvnCatCommandTest extends AbstractSvnCommandTestBase -{ +import static org.junit.Assert.assertEquals; + +//~--- JDK imports ------------------------------------------------------------ + +public class SvnCatCommandTest extends AbstractSvnCommandTestBase { - /** - * Method description - * - * - * @throws IOException - * @throws RepositoryException - */ @Test - public void testCat() throws IOException, RepositoryException - { + public void testCat() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); request.setPath("a.txt"); @@ -69,36 +56,50 @@ public class SvnCatCommandTest extends AbstractSvnCommandTestBase assertEquals("a", execute(request)); } - /** - * Method description - * - * - * @throws IOException - * @throws RepositoryException - */ @Test - public void testSimpleCat() throws IOException, RepositoryException - { + public void testSimpleCat() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); request.setPath("c/d.txt"); assertEquals("d", execute(request)); } - /** - * Method description - * - * - * @param request - * - * @return - * - * @throws IOException - * @throws RepositoryException - */ - private String execute(CatCommandRequest request) - throws IOException, RepositoryException - { + @Test(expected = PathNotFoundException.class) + public void testUnknownFile() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + + request.setPath("unknown"); + request.setRevision("1"); + + execute(request); + } + + @Test(expected = RevisionNotFoundException.class) + public void testUnknownRevision() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + + request.setPath("a.txt"); + request.setRevision("42"); + + execute(request); + } + + @Test + public void testSimpleStream() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + request.setPath("a.txt"); + request.setRevision("1"); + + InputStream catResultStream = new SvnCatCommand(createContext(), repository).getCatResultStream(request); + + assertEquals('a', catResultStream.read()); + assertEquals('\n', catResultStream.read()); + assertEquals(-1, catResultStream.read()); + + catResultStream.close(); + } + + private String execute(CatCommandRequest request) throws IOException, RepositoryException { String content = null; ByteArrayOutputStream baos = new ByteArrayOutputStream(); From 6e858ea305648051d697b593bf66b55c4d4a380f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 15 Aug 2018 11:58:34 +0200 Subject: [PATCH 16/20] Add documentation --- .../scm/api/v2/resources/ContentResource.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 4182fb9343..15899923d0 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -2,6 +2,8 @@ package sonia.scm.api.v2.resources; import com.github.sdorra.spotter.ContentType; import com.github.sdorra.spotter.ContentTypes; +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.NamespaceAndName; @@ -37,8 +39,26 @@ public class ContentResource { this.serviceFactory = serviceFactory; } + /** + * Returns the content of a file for the given revision in the repository. The content type depends on the file + * content and can be discovered calling HEAD on the same URL. If a programming languge could be + * recognized, this will be given in the header Language. + * + * @param namespace the namespace of the repository + * @param name the name of the repository + * @param revision the revision + * @param path The path of the file + * + */ @GET @Path("{revision}/{path: .*}") + @StatusCodes({ + @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to read the repository"), + @ResponseCode(code = 404, condition = "not found, no repository with the specified name available in the namespace"), + @ResponseCode(code = 500, condition = "internal server error") + }) public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { StreamingOutput stream = createStreamingOutput(namespace, name, revision, path, repositoryService); @@ -65,8 +85,25 @@ public class ContentResource { }; } + /** + * Returns the content type and the programming language (if it can be detected) of a file for the given revision in + * the repository. The programming language will be given in the header Language. + * + * @param namespace the namespace of the repository + * @param name the name of the repository + * @param revision the revision + * @param path The path of the file + * + */ @HEAD @Path("{revision}/{path: .*}") + @StatusCodes({ + @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to read the repository"), + @ResponseCode(code = 404, condition = "not found, no repository with the specified name available in the namespace"), + @ResponseCode(code = 500, condition = "internal server error") + }) public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { Response.ResponseBuilder responseBuilder = Response.ok(); From 70039ad54089c482b423a6728c2294b4e1e6f159 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Wed, 15 Aug 2018 16:51:50 +0200 Subject: [PATCH 17/20] Removed setting content length to header length in ContentResource Header is always 1024 Bytes long --- .../sonia/scm/repository/api/CatCommandBuilder.java | 4 ---- .../java/sonia/scm/repository/spi/CatCommand.java | 7 +------ .../api/rest/resources/BrowserStreamingOutput.java | 12 +++--------- .../sonia/scm/api/v2/resources/ContentResource.java | 7 +------ 4 files changed, 5 insertions(+), 25 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java index 8a08663f25..270b996635 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/CatCommandBuilder.java @@ -33,8 +33,6 @@ package sonia.scm.repository.api; -//~--- non-JDK imports -------------------------------------------------------- - import com.google.common.base.Preconditions; import com.google.common.base.Strings; import org.slf4j.Logger; @@ -50,8 +48,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -//~--- JDK imports ------------------------------------------------------------ - /** * Shows the content of a file in the {@link Repository}.
*
diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java index cbf1866298..e0e336f2ff 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/CatCommand.java @@ -33,23 +33,18 @@ package sonia.scm.repository.spi; -//~--- non-JDK imports -------------------------------------------------------- - import sonia.scm.repository.RepositoryException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -//~--- JDK imports ------------------------------------------------------------ - /** * * @author Sebastian Sdorra * @since 1.17 */ -public interface CatCommand -{ +public interface CatCommand { void getCatResult(CatCommandRequest request, OutputStream output) throws IOException, RepositoryException; diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/BrowserStreamingOutput.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/BrowserStreamingOutput.java index 234aae4015..73f59f14da 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/BrowserStreamingOutput.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/BrowserStreamingOutput.java @@ -35,26 +35,20 @@ package sonia.scm.api.rest.resources; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.io.Closeables; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.RevisionNotFoundException; import sonia.scm.repository.api.CatCommandBuilder; import sonia.scm.repository.api.RepositoryService; - -//~--- JDK imports ------------------------------------------------------------ - -import java.io.IOException; -import java.io.OutputStream; +import sonia.scm.util.IOUtil; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import javax.ws.rs.core.StreamingOutput; -import sonia.scm.util.IOUtil; +import java.io.IOException; +import java.io.OutputStream; /** * diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 15899923d0..62486df287 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -15,11 +15,7 @@ import sonia.scm.repository.api.RepositoryServiceFactory; import sonia.scm.util.IOUtil; import javax.inject.Inject; -import javax.ws.rs.GET; -import javax.ws.rs.HEAD; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.WebApplicationException; +import javax.ws.rs.*; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.StreamingOutput; @@ -134,7 +130,6 @@ public class ContentResource { ContentType contentType = ContentTypes.detect(path, head); responseBuilder.header("Content-Type", contentType.getRaw()); contentType.getLanguage().ifPresent(language -> responseBuilder.header("Language", language)); - responseBuilder.header("Content-Length", head.length); } private byte[] getHead(String revision, String path, RepositoryService repositoryService) throws IOException, RepositoryException { From 9babeecea6fe32d2738d58a7311b49ee13bd027b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 16 Aug 2018 10:24:47 +0200 Subject: [PATCH 18/20] Create and handle RevisionNotFoundException --- .../java/sonia/scm/repository/spi/GitCatCommand.java | 9 ++++++++- .../sonia/scm/repository/spi/GitCatCommandTest.java | 10 ++++++++++ .../sonia/scm/repository/spi/HgCatCommandTest.java | 9 +++++++++ .../sonia/scm/api/v2/resources/ContentResource.java | 10 +++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java index 0748a42803..c451dd8614 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitCatCommand.java @@ -32,6 +32,7 @@ package sonia.scm.repository.spi; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectLoader; @@ -46,6 +47,7 @@ import org.slf4j.LoggerFactory; import sonia.scm.repository.GitUtil; import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; +import sonia.scm.repository.RevisionNotFoundException; import sonia.scm.util.Util; import java.io.Closeable; @@ -97,7 +99,12 @@ public class GitCatCommand extends AbstractGitCommand implements CatCommand { RevWalk revWalk = new RevWalk(repo); - RevCommit entry = revWalk.parseCommit(revId); + RevCommit entry = null; + try { + entry = revWalk.parseCommit(revId); + } catch (MissingObjectException e) { + throw new RevisionNotFoundException(revId.getName()); + } RevTree revTree = entry.getTree(); if (revTree != null) { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java index 875d47c15b..c23db0873b 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitCatCommandTest.java @@ -36,6 +36,7 @@ import org.junit.Test; import sonia.scm.repository.GitConstants; import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; +import sonia.scm.repository.RevisionNotFoundException; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -90,6 +91,15 @@ public class GitCatCommandTest extends AbstractGitCommandTestBase { execute(request); } + @Test(expected = RevisionNotFoundException.class) + public void testUnknownRevision() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + + request.setRevision("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + request.setPath("a.txt"); + execute(request); + } + @Test public void testSimpleStream() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java index c297182b82..00dd747bdf 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java @@ -71,6 +71,15 @@ public class HgCatCommandTest extends AbstractHgCommandTestBase { execute(request); } + @Test(expected = RepositoryException.class) + public void testUnknownRevision() throws IOException, RepositoryException { + CatCommandRequest request = new CatCommandRequest(); + + request.setRevision("abc"); + request.setPath("a.txt"); + execute(request); + } + @Test public void testSimpleStream() throws IOException, RepositoryException { CatCommandRequest request = new CatCommandRequest(); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 62486df287..ae209b4238 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -10,12 +10,17 @@ import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.PathNotFoundException; import sonia.scm.repository.RepositoryException; import sonia.scm.repository.RepositoryNotFoundException; +import sonia.scm.repository.RevisionNotFoundException; import sonia.scm.repository.api.RepositoryService; import sonia.scm.repository.api.RepositoryServiceFactory; import sonia.scm.util.IOUtil; import javax.inject.Inject; -import javax.ws.rs.*; +import javax.ws.rs.GET; +import javax.ws.rs.HEAD; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.StreamingOutput; @@ -116,6 +121,9 @@ public class ContentResource { } catch (PathNotFoundException e) { LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); return Response.status(Status.NOT_FOUND).build(); + } catch (RevisionNotFoundException e) { + LOG.debug("revision '{}' not found in repository {}/{}", revision, namespace, name, e); + return Response.status(Status.NOT_FOUND).build(); } catch (IOException e) { LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); From a2c7fb22dd02ca936645594c2973504c16380a9c Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 16 Aug 2018 10:47:47 +0200 Subject: [PATCH 19/20] Merged heads --- pom.xml | 2 +- .../scm/repository/spi/HgCatCommandTest.java | 2 -- .../scm/api/v2/resources/ContentResource.java | 29 +++++++++---------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index 5e67c117db..3295e732db 100644 --- a/pom.xml +++ b/pom.xml @@ -305,7 +305,7 @@ org.mockito - mockito-all + mockito-core ${mockito.version} test diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java index 00dd747bdf..afb5249b25 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgCatCommandTest.java @@ -42,8 +42,6 @@ import java.io.InputStream; import static org.junit.Assert.assertEquals; -//~--- JDK imports ------------------------------------------------------------ - public class HgCatCommandTest extends AbstractHgCommandTestBase { @Test diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index ae209b4238..0936a5b4a0 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -73,17 +73,17 @@ public class ContentResource { private StreamingOutput createStreamingOutput(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path, RepositoryService repositoryService) { return os -> { - try { - repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); - os.close(); - } catch (PathNotFoundException e) { - LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); - throw new WebApplicationException(Status.NOT_FOUND); - } catch (RepositoryException e) { - LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); - throw new WebApplicationException(Status.INTERNAL_SERVER_ERROR); - } - }; + try { + repositoryService.getCatCommand().setRevision(revision).retriveContent(os, path); + os.close(); + } catch (PathNotFoundException e) { + LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); + throw new WebApplicationException(Status.NOT_FOUND); + } catch (RepositoryException e) { + LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); + throw new WebApplicationException(Status.INTERNAL_SERVER_ERROR); + } + }; } /** @@ -107,7 +107,7 @@ public class ContentResource { }) public Response metadata(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision, @PathParam("path") String path) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - Response.ResponseBuilder responseBuilder = Response.ok(); + Response.ResponseBuilder responseBuilder = Response.ok(); return createContentHeader(namespace, name, revision, path, repositoryService, responseBuilder); } catch (RepositoryNotFoundException e) { LOG.debug("path '{}' not found in repository {}/{}", path, namespace, name, e); @@ -124,10 +124,7 @@ public class ContentResource { } catch (RevisionNotFoundException e) { LOG.debug("revision '{}' not found in repository {}/{}", revision, namespace, name, e); return Response.status(Status.NOT_FOUND).build(); - } catch (IOException e) { - LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); - return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); - } catch (RepositoryException e) { + } catch (IOException | RepositoryException e) { LOG.info("error reading repository resource {} from {}/{}", path, namespace, name, e); return Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()).build(); } From 35ce7f9828262200ebc6ba2cc9922e2eda40baf7 Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Thu, 16 Aug 2018 11:09:53 +0000 Subject: [PATCH 20/20] Close branch feature/content_endpoint_v2