Feature Partial Diff (#1581)

With this pull request, diffs for Git are loaded in chunks. This means, that for diffs with a lot of files only a part of them are loaded. In the UI a button will be displayed to load more. In the REST API, the number of files can be specified. This only works for diffs, that are delivered as "parsed" diffs. Currently, this is only available for Git. Co-authored-by: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
2026-07-05 22:18:11 +02:00 · 2021-03-12 13:52:17 +01:00
parent e66553705f
commit 84c8e02bf1
37 changed files with 900 additions and 223 deletions
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java
@@ -63,6 +63,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.Silent.class)
@@ -153,6 +154,38 @@ public class DiffResourceTest extends RepositoryTestBase {
      .contains("\"self\":{\"href\":\"http://self\"}");
  }

+  @Test
+  public void shouldGetParsedDiffsWithOffset() throws Exception {
+    DiffResult diffResult = mock(DiffResult.class);
+    when(diffResultCommandBuilder.getDiffResult()).thenReturn(diffResult);
+    when(diffResultToDiffResultDtoMapper.mapForRevision(REPOSITORY, diffResult, "revision"))
+      .thenReturn(new DiffResultDto(Links.linkingTo().self("http://self").build()));
+    MockHttpRequest request = MockHttpRequest
+      .get(DIFF_URL + "revision/parsed?offset=42")
+      .accept(VndMediaType.DIFF_PARSED);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    verify(diffResultCommandBuilder).setOffset(42);
+  }
+
+  @Test
+  public void shouldGetParsedDiffsWithLimit() throws Exception {
+    DiffResult diffResult = mock(DiffResult.class);
+    when(diffResultCommandBuilder.getDiffResult()).thenReturn(diffResult);
+    when(diffResultToDiffResultDtoMapper.mapForRevision(REPOSITORY, diffResult, "revision"))
+      .thenReturn(new DiffResultDto(Links.linkingTo().self("http://self").build()));
+    MockHttpRequest request = MockHttpRequest
+      .get(DIFF_URL + "revision/parsed?limit=42")
+      .accept(VndMediaType.DIFF_PARSED);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    verify(diffResultCommandBuilder).setLimit(42);
+  }
+
  @Test
  public void shouldGet404OnMissingRepository() throws URISyntaxException {
    when(serviceFactory.create(any(NamespaceAndName.class))).thenThrow(new NotFoundException("Text", "x"));
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapperTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapperTest.java
@@ -35,13 +35,13 @@ import sonia.scm.repository.api.DiffResult;
 import sonia.scm.repository.api.Hunk;

 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalInt;

 import static java.net.URI.create;
 import static java.util.Collections.emptyIterator;
+import static java.util.Optional.of;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -86,6 +86,21 @@ class DiffResultToDiffResultDtoMapperTest {
      .isEqualTo("/scm/api/v2/repositories/space/X/diff/123/parsed");
  }

+  @Test
+  void shouldCreateNextLinkForRevision() {
+    DiffResult result = createResult();
+    mockPartialResult(result);
+
+    DiffResultDto dto = mapper.mapForRevision(REPOSITORY, result, "123");
+
+    Optional<Link> nextLink = dto.getLinks().getLinkBy("next");
+    assertThat(nextLink)
+      .isPresent()
+      .get()
+      .extracting("href")
+      .isEqualTo("/scm/api/v2/repositories/space/X/diff/123/parsed?offset=30&limit=10");
+  }
+
  @Test
  void shouldCreateLinkToLoadMoreLinesForFilesWithHunks() {
    DiffResultDto dto = mapper.mapForRevision(REPOSITORY, createResult(), "123");
@@ -111,6 +126,55 @@ class DiffResultToDiffResultDtoMapperTest {
      .isEqualTo("/scm/api/v2/repositories/space/X/incoming/feature%2Fsome/master/diff/parsed");
  }

+  @Test
+  void shouldCreateSelfLinkForIncomingWithOffset() {
+    DiffResult result = createResult();
+    when(result.getOffset()).thenReturn(25);
+    DiffResultDto dto = mapper.mapForIncoming(REPOSITORY, result, "feature/some", "master");
+
+    Optional<Link> selfLink = dto.getLinks().getLinkBy("self");
+    assertThat(selfLink)
+      .isPresent()
+      .get()
+      .extracting("href")
+      .isEqualTo("/scm/api/v2/repositories/space/X/incoming/feature%2Fsome/master/diff/parsed?offset=25");
+  }
+
+  @Test
+  void shouldCreateSelfLinkForIncomingWithLimit() {
+    DiffResult result = createResult();
+    when(result.getLimit()).thenReturn(of(25));
+    DiffResultDto dto = mapper.mapForIncoming(REPOSITORY, result, "feature/some", "master");
+
+    Optional<Link> selfLink = dto.getLinks().getLinkBy("self");
+    assertThat(selfLink)
+      .isPresent()
+      .get()
+      .extracting("href")
+      .isEqualTo("/scm/api/v2/repositories/space/X/incoming/feature%2Fsome/master/diff/parsed?offset=0&limit=25");
+  }
+
+  @Test
+  void shouldCreateNextLinkForIncoming() {
+    DiffResult result = createResult();
+    mockPartialResult(result);
+
+    DiffResultDto dto = mapper.mapForIncoming(REPOSITORY, result, "feature/some", "master");
+
+    Optional<Link> nextLink = dto.getLinks().getLinkBy("next");
+    assertThat(nextLink)
+      .isPresent()
+      .get()
+      .extracting("href")
+      .isEqualTo("/scm/api/v2/repositories/space/X/incoming/feature%2Fsome/master/diff/parsed?offset=30&limit=10");
+  }
+
+  private void mockPartialResult(DiffResult result) {
+    when(result.getLimit()).thenReturn(of(10));
+    when(result.getOffset()).thenReturn(20);
+    when(result.isPartial()).thenReturn(true);
+  }
+
  private DiffResult createResult() {
    return result(
      addedFile("A.java", "abc"),
--- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IncomingRootResourceTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IncomingRootResourceTest.java
@@ -39,6 +39,7 @@ import org.junit.After;
 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;
@@ -71,6 +72,7 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static sonia.scm.repository.api.DiffFormat.NATIVE;

@@ -97,9 +99,9 @@ public class IncomingRootResourceTest extends RepositoryTestBase {
  @Mock
  private LogCommandBuilder logCommandBuilder;

-  @Mock
+  @Mock(answer = Answers.RETURNS_SELF)
  private DiffCommandBuilder diffCommandBuilder;
-  @Mock
+  @Mock(answer = Answers.RETURNS_SELF)
  private DiffResultCommandBuilder diffResultCommandBuilder;

  @Mock
@@ -199,9 +201,6 @@ public class IncomingRootResourceTest extends RepositoryTestBase {

  @Test
  public void shouldGetDiffs() throws Exception {
-    when(diffCommandBuilder.setRevision("src_changeset_id")).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setAncestorChangeset("target_changeset_id")).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setFormat(NATIVE)).thenReturn(diffCommandBuilder);
    when(diffCommandBuilder.retrieveContent()).thenReturn(output -> {});
    MockHttpRequest request = MockHttpRequest
      .get(INCOMING_DIFF_URL + "src_changeset_id/target_changeset_id/diff")
@@ -217,12 +216,13 @@ public class IncomingRootResourceTest extends RepositoryTestBase {
    assertThat(response.getOutputHeaders().containsKey(expectedHeader)).isTrue();
    assertThat((String) response.getOutputHeaders().get("Content-Disposition").get(0))
      .contains(expectedValue);
+    verify(diffCommandBuilder).setRevision("src_changeset_id");
+    verify(diffCommandBuilder).setAncestorChangeset("target_changeset_id");
+    verify(diffCommandBuilder).setFormat(NATIVE);
  }

  @Test
  public void shouldGetParsedDiffs() throws Exception {
-    when(diffResultCommandBuilder.setRevision("src_changeset_id")).thenReturn(diffResultCommandBuilder);
-    when(diffResultCommandBuilder.setAncestorChangeset("target_changeset_id")).thenReturn(diffResultCommandBuilder);
    DiffResult diffResult = mock(DiffResult.class);
    when(diffResultCommandBuilder.getDiffResult()).thenReturn(diffResult);
    when(diffResultToDiffResultDtoMapper.mapForIncoming(REPOSITORY, diffResult, "src_changeset_id", "target_changeset_id"))
@@ -239,6 +239,42 @@ public class IncomingRootResourceTest extends RepositoryTestBase {
      .isEqualTo(200);
    assertThat(response.getContentAsString())
      .contains("\"self\":{\"href\":\"http://self\"}");
+    verify(diffResultCommandBuilder).setRevision("src_changeset_id");
+    verify(diffResultCommandBuilder).setAncestorChangeset("target_changeset_id");
+  }
+
+  @Test
+  public void shouldGetParsedDiffsWithLimit() throws Exception {
+    DiffResult diffResult = mock(DiffResult.class);
+    when(diffResultCommandBuilder.getDiffResult()).thenReturn(diffResult);
+    when(diffResultToDiffResultDtoMapper.mapForIncoming(REPOSITORY, diffResult, "src_changeset_id", "target_changeset_id"))
+    .thenReturn(new DiffResultDto(Links.linkingTo().self("http://self").build()));
+
+    MockHttpRequest request = MockHttpRequest
+      .get(INCOMING_DIFF_URL + "src_changeset_id/target_changeset_id/diff/parsed?limit=42")
+      .accept(VndMediaType.DIFF_PARSED);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    verify(diffResultCommandBuilder).setLimit(42);
+  }
+
+  @Test
+  public void shouldGetParsedDiffsWithOffset() throws Exception {
+    DiffResult diffResult = mock(DiffResult.class);
+    when(diffResultCommandBuilder.getDiffResult()).thenReturn(diffResult);
+    when(diffResultToDiffResultDtoMapper.mapForIncoming(REPOSITORY, diffResult, "src_changeset_id", "target_changeset_id"))
+    .thenReturn(new DiffResultDto(Links.linkingTo().self("http://self").build()));
+
+    MockHttpRequest request = MockHttpRequest
+      .get(INCOMING_DIFF_URL + "src_changeset_id/target_changeset_id/diff/parsed?offset=42")
+      .accept(VndMediaType.DIFF_PARSED);
+    MockHttpResponse response = new MockHttpResponse();
+
+    dispatcher.invoke(request, response);
+
+    verify(diffResultCommandBuilder).setOffset(42);
  }

  @Test
@@ -256,9 +292,6 @@ public class IncomingRootResourceTest extends RepositoryTestBase {

  @Test
  public void shouldGet404OnMissingRevision() throws Exception {
-    when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setAncestorChangeset(anyString())).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setFormat(any())).thenReturn(diffCommandBuilder);
    when(diffCommandBuilder.retrieveContent()).thenThrow(new NotFoundException("Text", "x"));

    MockHttpRequest request = MockHttpRequest
@@ -273,9 +306,6 @@ public class IncomingRootResourceTest extends RepositoryTestBase {

  @Test
  public void shouldGet400OnCrlfInjection() throws Exception {
-    when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setAncestorChangeset(anyString())).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setFormat(any())).thenReturn(diffCommandBuilder);
    when(diffCommandBuilder.retrieveContent()).thenThrow(new NotFoundException("Text", "x"));
    MockHttpRequest request = MockHttpRequest
      .get(INCOMING_DIFF_URL + "ny%0D%0ASet-cookie:%20Tamper=3079675143472450634/ny%0D%0ASet-cookie:%20Tamper=3079675143472450634/diff")
@@ -290,9 +320,6 @@ public class IncomingRootResourceTest extends RepositoryTestBase {

  @Test
  public void shouldGet400OnUnknownFormat() throws Exception {
-    when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setAncestorChangeset(anyString())).thenReturn(diffCommandBuilder);
-    when(diffCommandBuilder.setFormat(any())).thenReturn(diffCommandBuilder);
    when(diffCommandBuilder.retrieveContent()).thenThrow(new NotFoundException("Test", "test"));
    MockHttpRequest request = MockHttpRequest
      .get(INCOMING_DIFF_URL + "src_changeset_id/target_changeset_id/diff?format=Unknown")