diff --git a/gradle/changelog/partial_diff.yaml b/gradle/changelog/partial_diff.yaml new file mode 100644 index 0000000000..1fe2b5ad2b --- /dev/null +++ b/gradle/changelog/partial_diff.yaml @@ -0,0 +1,2 @@ +- type: added + description: Partial diff ([#1581](https://github.com/scm-manager/scm-manager/issues/1581)) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java index 174dc42021..728c156a84 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.api; import sonia.scm.FeatureNotSupportedException; @@ -30,11 +30,11 @@ import sonia.scm.repository.spi.DiffCommandRequest; import java.util.Set; -abstract class AbstractDiffCommandBuilder { +abstract class AbstractDiffCommandBuilder { /** request for the diff command implementation */ - final DiffCommandRequest request = new DiffCommandRequest(); + final R request = createRequest(); private final Set supportedFeatures; @@ -89,4 +89,6 @@ abstract class AbstractDiffCommandBuilder } abstract T self(); + + abstract R createRequest(); } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java index 8055fd72d5..9c43cf8737 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.api; //~--- non-JDK imports -------------------------------------------------------- @@ -31,6 +31,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.Feature; import sonia.scm.repository.spi.DiffCommand; +import sonia.scm.repository.spi.DiffCommandRequest; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -54,13 +55,10 @@ import java.util.Set; * System.out.println(content); * * - * - * TODO check current behavior. - * * @author Sebastian Sdorra * @since 1.17 */ -public final class DiffCommandBuilder extends AbstractDiffCommandBuilder +public final class DiffCommandBuilder extends AbstractDiffCommandBuilder { /** @@ -161,6 +159,11 @@ public final class DiffCommandBuilder extends AbstractDiffCommandBuilder { String getOldRevision(); String getNewRevision(); + + default boolean isPartial() { + return false; + } + + default int getOffset() { + return 0; + } + + default Optional getLimit() { + return empty(); + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java index 686135feb9..160a37d208 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.api; import com.google.common.base.Preconditions; @@ -29,11 +29,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.Feature; import sonia.scm.repository.spi.DiffResultCommand; +import sonia.scm.repository.spi.DiffResultCommandRequest; import java.io.IOException; import java.util.Set; -public class DiffResultCommandBuilder extends AbstractDiffCommandBuilder { +public class DiffResultCommandBuilder extends AbstractDiffCommandBuilder { private static final Logger LOG = LoggerFactory.getLogger(DiffResultCommandBuilder.class); @@ -44,6 +45,31 @@ public class DiffResultCommandBuilder extends AbstractDiffCommandBuilder diffEntries; - private GitDiffResult(org.eclipse.jgit.lib.Repository repository, Differ.Diff diff) { + private final int offset; + private final Integer limit; + + private GitDiffResult(org.eclipse.jgit.lib.Repository repository, Differ.Diff diff, int offset, Integer limit) { this.repository = repository; this.diff = diff; + this.offset = offset; + this.limit = limit; + this.diffEntries = diff.getEntries(); } @Override @@ -70,12 +88,32 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu return GitUtil.getId(diff.getCommit().getId()); } + @Override + public boolean isPartial() { + return limit != null && limit + offset < diffEntries.size(); + } + + @Override + public int getOffset() { + return offset; + } + + @Override + public Optional getLimit() { + return ofNullable(limit); + } + @Override public Iterator iterator() { - return diff.getEntries() + Stream diffEntryStream = diffEntries .stream() + .skip(offset); + if (limit != null) { + diffEntryStream = diffEntryStream.limit(limit); + } + return diffEntryStream .map(diffEntry -> new GitDiffFile(repository, diffEntry)) - .collect(Collectors.toList()) + .map(DiffFile.class::cast) .iterator(); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java index e2722b78ab..7f4c5c20d4 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java @@ -55,6 +55,8 @@ public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { DiffFile b = iterator.next(); assertThat(b.getOldPath()).isEqualTo("b.txt"); assertThat(b.getNewPath()).isEqualTo("/dev/null"); + + assertThat(diffResult.isPartial()).isFalse(); } @Test @@ -69,6 +71,8 @@ public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { DiffFile b = iterator.next(); assertThat(b.getOldRevision()).isEqualTo("61780798228d17af2d34fce4cfbdf35556832472"); assertThat(b.getNewRevision()).isEqualTo("0000000000000000000000000000000000000000"); + + assertThat(iterator.hasNext()).isFalse(); } @Test @@ -119,10 +123,55 @@ public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { assertThat(renameB.iterator().hasNext()).isFalse(); } + @Test + public void shouldLimitResult() throws IOException { + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4", null, 1); + Iterator iterator = diffResult.iterator(); + + DiffFile a = iterator.next(); + assertThat(a.getNewPath()).isEqualTo("a.txt"); + assertThat(a.getOldPath()).isEqualTo("a.txt"); + + assertThat(iterator.hasNext()).isFalse(); + + assertThat(diffResult.isPartial()).isTrue(); + assertThat(diffResult.getLimit()).get().isEqualTo(1); + assertThat(diffResult.getOffset()).isZero(); + } + + @Test + public void shouldSetOffsetForResult() throws IOException { + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4", 1, null); + Iterator iterator = diffResult.iterator(); + + DiffFile b = iterator.next(); + assertThat(b.getOldPath()).isEqualTo("b.txt"); + assertThat(b.getNewPath()).isEqualTo("/dev/null"); + + assertThat(iterator.hasNext()).isFalse(); + + assertThat(diffResult.isPartial()).isFalse(); + } + + @Test + public void shouldNotBePartialWhenResultCountMatchesLimit() throws IOException { + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4", 0, 2); + + assertThat(diffResult.isPartial()).isFalse(); + assertThat(diffResult.getLimit()).get().isEqualTo(2); + assertThat(diffResult.getOffset()).isZero(); + } + private DiffResult createDiffResult(String s) throws IOException { + return createDiffResult(s, null, null); + } + + private DiffResult createDiffResult(String s, Integer offset, Integer limit) throws IOException { GitDiffResultCommand gitDiffResultCommand = new GitDiffResultCommand(createContext()); - DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); + DiffResultCommandRequest diffCommandRequest = new DiffResultCommandRequest(); diffCommandRequest.setRevision(s); + diffCommandRequest.setOffset(offset); + diffCommandRequest.setLimit(limit); return gitDiffResultCommand.getDiffResult(diffCommandRequest); } diff --git a/scm-ui/ui-api/package.json b/scm-ui/ui-api/package.json index efc8229f4c..c3d9b13638 100644 --- a/scm-ui/ui-api/package.json +++ b/scm-ui/ui-api/package.json @@ -29,7 +29,8 @@ "fetch-mock-jest": "^1.5.1", "query-string": "5", "react": "^17.0.1", - "react-query": "^3.5.16" + "react-query": "^3.5.16", + "gitdiff-parser": "^0.1.2" }, "babel": { "presets": [ diff --git a/scm-ui/ui-api/src/diff.test.ts b/scm-ui/ui-api/src/diff.test.ts new file mode 100644 index 0000000000..ecdc316eb3 --- /dev/null +++ b/scm-ui/ui-api/src/diff.test.ts @@ -0,0 +1,225 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import fetchMock from "fetch-mock-jest"; +import { Diff } from "@scm-manager/ui-types"; +import { act, renderHook } from "@testing-library/react-hooks"; +import { useDiff } from "./diff"; +import createWrapper from "./tests/createWrapper"; + +describe("Test diff", () => { + const simpleDiff: Diff = { + files: [ + { + oldPath: "/dev/null", + newPath: "0.txt", + oldEndingNewLine: true, + newEndingNewLine: true, + oldRevision: "0000000000000000000000000000000000000000", + newRevision: "573541ac9702dd3969c9bc859d2b91ec1f7e6e56", + type: "add", + language: "text", + hunks: [ + { + content: "@@ -0,0 +1 @@", + newStart: 1, + newLines: 1, + changes: [ + { + content: "0", + type: "insert", + lineNumber: 1, + isInsert: true + } + ] + } + ], + _links: { + lines: { + href: + "/api/v2/repositories/scmadmin/HeartOfGold-git/content/one_to_onehundred/0.txt?start={start}&end={end}", + templated: true + } + } + } + ], + partial: false, + _links: { + self: { + href: "/api/v2/diff" + } + } + }; + + const partialDiff1: Diff = { + files: [ + { + oldPath: "/dev/null", + newPath: "0.txt", + oldEndingNewLine: true, + newEndingNewLine: true, + oldRevision: "0000000000000000000000000000000000000000", + newRevision: "573541ac9702dd3969c9bc859d2b91ec1f7e6e56", + type: "add", + language: "text", + hunks: [ + { + content: "@@ -0,0 +1 @@", + newStart: 1, + newLines: 1, + changes: [ + { + content: "0", + type: "insert", + lineNumber: 1, + isInsert: true + } + ] + } + ], + _links: { + lines: { + href: + "/api/v2/repositories/scmadmin/HeartOfGold-git/content/one_to_onehundred/0.txt?start={start}&end={end}", + templated: true + } + } + } + ], + partial: true, + _links: { + self: { + href: "/diff" + }, + next: { + href: "/diff?offset=1&limit=1" + } + } + }; + + const partialDiff2: Diff = { + files: [ + { + oldPath: "/dev/null", + newPath: "1.txt", + oldEndingNewLine: true, + newEndingNewLine: true, + oldRevision: "0000000000000000000000000000000000000000", + newRevision: "573541ac9702dd3969c9bc859d2b91ec1f7e6e56", + type: "add", + language: "text", + hunks: [ + { + content: "@@ -0,0 +1 @@", + newStart: 1, + newLines: 1, + changes: [ + { + content: "1", + type: "insert", + lineNumber: 1, + isInsert: true + } + ] + } + ], + _links: { + lines: { + href: + "/api/v2/repositories/scmadmin/HeartOfGold-git/content/one_to_onehundred/1.txt?start={start}&end={end}", + templated: true + } + } + } + ], + partial: false, + _links: { + self: { + href: "/diff" + } + } + }; + + beforeEach(() => { + fetchMock.reset(); + }); + + it("should return simple parsed diff", async () => { + fetchMock.getOnce("/api/v2/diff", { + body: simpleDiff, + headers: { "Content-Type": "application/vnd.scmm-diffparsed+json;v=2" } + }); + const { result, waitFor } = renderHook(() => useDiff("/diff"), { + wrapper: createWrapper() + }); + await waitFor(() => !!result.current.data); + expect(result.current.data).toEqual(simpleDiff); + }); + + it("should parse and return textual diff", async () => { + fetchMock.getOnce("/api/v2/diff", { + body: `diff --git a/new.txt b/new.txt +--- a/new.txt ++++ b/new.txt +@@ -1,1 +1,1 @@ +-i am old! +\\ No newline at end of file ++i am new! +\\ No newline at end of file +`, + headers: { "Content-Type": "text/plain" } + }); + const { result, waitFor } = renderHook(() => useDiff("/diff"), { + wrapper: createWrapper() + }); + await waitFor(() => !!result.current.data); + expect(result.current.data?.files).toHaveLength(1); + }); + + it("should return parsed diff in multiple chunks", async () => { + fetchMock.getOnce("/api/v2/diff?limit=1", { + body: partialDiff1, + headers: { "Content-Type": "application/vnd.scmm-diffparsed+json;v=2" } + }); + fetchMock.getOnce("/api/v2/diff?offset=1&limit=1", { + body: partialDiff2, + headers: { "Content-Type": "application/vnd.scmm-diffparsed+json;v=2" } + }); + const { result, waitFor, waitForNextUpdate } = renderHook(() => useDiff("/diff?limit=1"), { + wrapper: createWrapper() + }); + await waitFor(() => !!result.current.data); + expect(result.current.data).toEqual(partialDiff1); + + await act(() => { + const { fetchNextPage } = result.current; + fetchNextPage(); + return waitForNextUpdate(); + }); + + await waitFor(() => !result.current.isFetchingNextPage); + + expect(result.current.data).toEqual({ ...partialDiff2, files: [partialDiff1.files[0], partialDiff2.files[0]] }); + }); +}); diff --git a/scm-ui/ui-api/src/diff.ts b/scm-ui/ui-api/src/diff.ts new file mode 100644 index 0000000000..3d3b9b2de6 --- /dev/null +++ b/scm-ui/ui-api/src/diff.ts @@ -0,0 +1,86 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import parser from "gitdiff-parser"; + +import { useInfiniteQuery } from "react-query"; +import { apiClient } from "./apiclient"; +import { Diff, Link } from "@scm-manager/ui-types"; + +type UseDiffOptions = { + limit?: number; +}; + +export const useDiff = (link: string, options: UseDiffOptions = {}) => { + let initialLink = link; + if (options.limit) { + initialLink = `${initialLink}?limit=${options.limit}`; + } + const { isLoading, error, data, isFetchingNextPage, fetchNextPage } = useInfiniteQuery( + ["link", link], + ({ pageParam }) => { + return apiClient.get(pageParam || initialLink).then(response => { + const contentType = response.headers.get("Content-Type"); + if (contentType && contentType.toLowerCase() === "application/vnd.scmm-diffparsed+json;v=2") { + return response.json(); + } else { + return response + .text() + .then(parser.parse) + .then(data => { + return { + files: data, + partial: false, + _links: {} + }; + }); + } + }); + }, + { + getNextPageParam: lastPage => (lastPage._links.next as Link)?.href + } + ); + + return { + isLoading, + error, + isFetchingNextPage, + fetchNextPage: () => { + fetchNextPage(); + }, + data: merge(data?.pages) + }; +}; + +const merge = (diffs?: Diff[]): Diff | undefined => { + if (!diffs || diffs.length === 0) { + return; + } + const joinedFiles = diffs.flatMap(diff => diff.files); + return { + ...diffs[diffs.length - 1], + files: joinedFiles + }; +}; diff --git a/scm-ui/ui-api/src/index.ts b/scm-ui/ui-api/src/index.ts index 69ede48603..2434d1a977 100644 --- a/scm-ui/ui-api/src/index.ts +++ b/scm-ui/ui-api/src/index.ts @@ -44,6 +44,7 @@ export * from "./repository-roles"; export * from "./permissions"; export * from "./sources"; export * from "./import"; +export * from "./diff"; export { default as ApiProvider } from "./ApiProvider"; export * from "./ApiProvider"; diff --git a/scm-ui/ui-components/package.json b/scm-ui/ui-components/package.json index f8fbf5f485..4c7842aec7 100644 --- a/scm-ui/ui-components/package.json +++ b/scm-ui/ui-components/package.json @@ -52,7 +52,8 @@ "react-test-renderer": "^17.0.1", "storybook-addon-i18next": "^1.3.0", "to-camel-case": "^1.0.0", - "worker-plugin": "^3.2.0" + "worker-plugin": "^3.2.0", + "gitdiff-parser": "^0.1.2" }, "dependencies": { "@scm-manager/ui-api": "^2.14.2-SNAPSHOT", @@ -60,7 +61,6 @@ "@scm-manager/ui-types": "^2.14.2-SNAPSHOT", "classnames": "^2.2.6", "date-fns": "^2.4.1", - "gitdiff-parser": "^0.1.2", "lowlight": "^1.13.0", "prism-themes": "^1.4.0", "query-string": "5", diff --git a/scm-ui/ui-components/src/repos/Diff.stories.tsx b/scm-ui/ui-components/src/repos/Diff.stories.tsx index 9649d97a84..65e135a6ea 100644 --- a/scm-ui/ui-components/src/repos/Diff.stories.tsx +++ b/scm-ui/ui-components/src/repos/Diff.stories.tsx @@ -30,14 +30,14 @@ import simpleDiff from "../__resources__/Diff.simple"; import hunksDiff from "../__resources__/Diff.hunks"; import binaryDiff from "../__resources__/Diff.binary"; import markdownDiff from "../__resources__/Diff.markdown"; -import { DiffEventContext, File, FileControlFactory } from "./DiffTypes"; +import { DiffEventContext, FileControlFactory } from "./DiffTypes"; import Toast from "../toast/Toast"; import { getPath } from "./diffs"; import DiffButton from "./DiffButton"; import styled from "styled-components"; import { MemoryRouter } from "react-router-dom"; import { one, two } from "../__resources__/changesets"; -import { Changeset } from "@scm-manager/ui-types"; +import { Changeset, FileDiff } from "@scm-manager/ui-types"; import JumpToFileButton from "./JumpToFileButton"; const diffFiles = parser.parse(simpleDiff); @@ -141,7 +141,7 @@ storiesOf("Diff", module) return ; }) .add("SyntaxHighlighting", () => { - const filesWithLanguage = diffFiles.map((file: File) => { + const filesWithLanguage = diffFiles.map((file: FileDiff) => { const ext = getPath(file).split(".")[1]; if (ext === "tsx") { file.language = "typescript"; @@ -160,7 +160,7 @@ storiesOf("Diff", module) oldPath.endsWith(".java")} /> )) .add("Expandable", () => { - const filesWithLanguage = diffFiles.map((file: File) => { + const filesWithLanguage = diffFiles.map((file: FileDiff) => { file._links = { lines: { href: "http://example.com/" } }; return file; }); diff --git a/scm-ui/ui-components/src/repos/Diff.tsx b/scm-ui/ui-components/src/repos/Diff.tsx index 1cec03ab37..da01994085 100644 --- a/scm-ui/ui-components/src/repos/Diff.tsx +++ b/scm-ui/ui-components/src/repos/Diff.tsx @@ -23,7 +23,8 @@ */ import React from "react"; import DiffFile from "./DiffFile"; -import { DiffObjectProps, File, FileControlFactory } from "./DiffTypes"; +import { DiffObjectProps, FileControlFactory } from "./DiffTypes"; +import { FileDiff } from "@scm-manager/ui-types"; import { escapeWhitespace } from "./diffs"; import Notification from "../Notification"; import { WithTranslation, withTranslation } from "react-i18next"; @@ -32,7 +33,7 @@ import { RouteComponentProps, withRouter } from "react-router-dom"; type Props = RouteComponentProps & WithTranslation & DiffObjectProps & { - diff: File[]; + diff: FileDiff[]; fileControlFactory?: FileControlFactory; }; diff --git a/scm-ui/ui-components/src/repos/DiffExpander.test.ts b/scm-ui/ui-components/src/repos/DiffExpander.test.ts index 3534cf77d2..7e4b2e73d6 100644 --- a/scm-ui/ui-components/src/repos/DiffExpander.test.ts +++ b/scm-ui/ui-components/src/repos/DiffExpander.test.ts @@ -23,7 +23,7 @@ */ import fetchMock from "fetch-mock"; import DiffExpander from "./DiffExpander"; -import { File, Hunk } from "./DiffTypes"; +import { FileDiff, Hunk } from "@scm-manager/ui-types"; const HUNK_0: Hunk = { content: "@@ -1,8 +1,8 @@", @@ -94,7 +94,7 @@ const HUNK_3: Hunk = { { content: "line", type: "normal", oldLineNumber: 38, newLineNumber: 40, isNormal: true } ] }; -const TEST_CONTENT_WITH_HUNKS: File = { +const TEST_CONTENT_WITH_HUNKS: FileDiff = { hunks: [HUNK_0, HUNK_1, HUNK_2, HUNK_3], newEndingNewLine: true, newPath: "src/main/js/CommitMessage.js", @@ -112,7 +112,7 @@ const TEST_CONTENT_WITH_HUNKS: File = { } }; -const TEST_CONTENT_WITH_NEW_BINARY_FILE: File = { +const TEST_CONTENT_WITH_NEW_BINARY_FILE: FileDiff = { oldPath: "/dev/null", newPath: "src/main/fileUploadV2.png", oldEndingNewLine: true, @@ -122,7 +122,7 @@ const TEST_CONTENT_WITH_NEW_BINARY_FILE: File = { type: "add" }; -const TEST_CONTENT_WITH_NEW_TEXT_FILE: File = { +const TEST_CONTENT_WITH_NEW_TEXT_FILE: FileDiff = { oldPath: "/dev/null", newPath: "src/main/markdown/README.md", oldEndingNewLine: true, @@ -151,7 +151,7 @@ const TEST_CONTENT_WITH_NEW_TEXT_FILE: File = { } }; -const TEST_CONTENT_WITH_DELETED_TEXT_FILE: File = { +const TEST_CONTENT_WITH_DELETED_TEXT_FILE: FileDiff = { oldPath: "README.md", newPath: "/dev/null", oldEndingNewLine: true, @@ -171,7 +171,7 @@ const TEST_CONTENT_WITH_DELETED_TEXT_FILE: File = { _links: { lines: { href: "http://localhost:8081/dev/null?start={start}&end={end}", templated: true } } }; -const TEST_CONTENT_WITH_DELETED_LINES_AT_END: File = { +const TEST_CONTENT_WITH_DELETED_LINES_AT_END: FileDiff = { oldPath: "pom.xml", newPath: "pom.xml", oldEndingNewLine: true, @@ -214,7 +214,7 @@ const TEST_CONTENT_WITH_DELETED_LINES_AT_END: File = { } }; -const TEST_CONTENT_WITH_ALL_LINES_REMOVED_FROM_FILE: File = { +const TEST_CONTENT_WITH_ALL_LINES_REMOVED_FROM_FILE: FileDiff = { oldPath: "pom.xml", newPath: "pom.xml", oldEndingNewLine: true, @@ -281,7 +281,7 @@ describe("with hunks the diff expander", () => { const expandedHunk = diffExpander.getHunk(1).hunk; const subsequentHunk = diffExpander.getHunk(2).hunk; fetchMock.get("http://localhost:8081/scm/api/v2/content/abc/CommitMessage.js?start=20&end=21", "new line 1"); - let newFile: File; + let newFile: FileDiff; await diffExpander .getHunk(1) .expandBottom(1) @@ -306,7 +306,7 @@ describe("with hunks the diff expander", () => { "http://localhost:8081/scm/api/v2/content/abc/CommitMessage.js?start=8&end=13", "new line 9\nnew line 10\nnew line 11\nnew line 12\nnew line 13" ); - let newFile: File; + let newFile: FileDiff; await diffExpander .getHunk(1) .expandHead(5) @@ -335,7 +335,7 @@ describe("with hunks the diff expander", () => { "http://localhost:8081/scm/api/v2/content/abc/CommitMessage.js?start=40&end=50", "new line 40\nnew line 41\nnew line 42" ); - let newFile: File; + let newFile: FileDiff; await diffExpander .getHunk(3) .expandBottom(10) @@ -348,7 +348,7 @@ describe("with hunks the diff expander", () => { "http://localhost:8081/scm/api/v2/content/abc/CommitMessage.js?start=40&end=-1", "new line 40\nnew line 41\nnew line 42" ); - let newFile: File; + let newFile: FileDiff; await diffExpander .getHunk(3) .expandBottom(-1) diff --git a/scm-ui/ui-components/src/repos/DiffExpander.ts b/scm-ui/ui-components/src/repos/DiffExpander.ts index fea69316f5..7e468eb819 100644 --- a/scm-ui/ui-components/src/repos/DiffExpander.ts +++ b/scm-ui/ui-components/src/repos/DiffExpander.ts @@ -23,13 +23,12 @@ */ import { apiClient } from "@scm-manager/ui-components"; -import { Change, File, Hunk } from "./DiffTypes"; -import { Link } from "@scm-manager/ui-types"; +import { Change, FileDiff, Hunk, Link } from "@scm-manager/ui-types"; class DiffExpander { - file: File; + file: FileDiff; - constructor(file: File) { + constructor(file: FileDiff) { this.file = file; } @@ -73,7 +72,7 @@ class DiffExpander { } }; - expandHead: (n: number, count: number) => Promise = (n, count) => { + expandHead: (n: number, count: number) => Promise = (n, count) => { const start = this.minLineNumber(n) - Math.min(count, this.computeMaxExpandHeadRange(n)) - 1; const end = this.minLineNumber(n) - 1; return this.loadLines(start, end).then(lines => { @@ -90,7 +89,7 @@ class DiffExpander { }); }; - expandBottom: (n: number, count: number) => Promise = (n, count) => { + expandBottom: (n: number, count: number) => Promise = (n, count) => { const maxExpandBottomRange = this.computeMaxExpandBottomRange(n); const start = this.maxLineNumber(n); const end = @@ -191,8 +190,8 @@ export type ExpandableHunk = { hunk: Hunk; maxExpandHeadRange: number; maxExpandBottomRange: number; - expandHead: (count: number) => Promise; - expandBottom: (count: number) => Promise; + expandHead: (count: number) => Promise; + expandBottom: (count: number) => Promise; }; export default DiffExpander; diff --git a/scm-ui/ui-components/src/repos/DiffFile.tsx b/scm-ui/ui-components/src/repos/DiffFile.tsx index 32e47c1105..7cb1d7df10 100644 --- a/scm-ui/ui-components/src/repos/DiffFile.tsx +++ b/scm-ui/ui-components/src/repos/DiffFile.tsx @@ -30,7 +30,8 @@ import { Decoration, getChangeKey, Hunk } from "react-diff-view"; import { ButtonGroup } from "../buttons"; import Tag from "../Tag"; import Icon from "../Icon"; -import { Change, ChangeEvent, DiffObjectProps, File, Hunk as HunkType } from "./DiffTypes"; +import { Change, FileDiff, Hunk as HunkType } from "@scm-manager/ui-types"; +import { ChangeEvent, DiffObjectProps } from "./DiffTypes"; import TokenizedDiffView from "./TokenizedDiffView"; import DiffButton from "./DiffButton"; import { MenuContext, OpenInFullscreenButton } from "@scm-manager/ui-components"; @@ -45,7 +46,7 @@ const EMPTY_ANNOTATION_FACTORY = {}; type Props = DiffObjectProps & WithTranslation & { - file: File; + file: FileDiff; }; type Collapsible = { @@ -53,7 +54,7 @@ type Collapsible = { }; type State = Collapsible & { - file: File; + file: FileDiff; sideBySide?: boolean; diffExpander: DiffExpander; expansionError?: any; @@ -257,7 +258,7 @@ class DiffFile extends React.Component { }; }; - diffExpanded = (newFile: File) => { + diffExpanded = (newFile: FileDiff) => { this.setState({ file: newFile, diffExpander: new DiffExpander(newFile) }); }; @@ -303,7 +304,7 @@ class DiffFile extends React.Component { } }; - renderHunk = (file: File, expandableHunk: ExpandableHunk, i: number) => { + renderHunk = (file: FileDiff, expandableHunk: ExpandableHunk, i: number) => { const hunk = expandableHunk.hunk; if (this.props.markConflicts && hunk.changes) { this.markConflicts(hunk); @@ -353,7 +354,7 @@ class DiffFile extends React.Component { } }; - getAnchorId(file: File) { + getAnchorId(file: FileDiff) { let path: string; if (file.type === "delete") { path = file.oldPath; @@ -363,7 +364,7 @@ class DiffFile extends React.Component { return escapeWhitespace(path); } - renderFileTitle = (file: File) => { + renderFileTitle = (file: FileDiff) => { if (file.oldPath !== file.newPath && (file.type === "copy" || file.type === "rename")) { return ( <> @@ -376,7 +377,7 @@ class DiffFile extends React.Component { return file.newPath; }; - hoverFileTitle = (file: File): string => { + hoverFileTitle = (file: FileDiff): string => { if (file.oldPath !== file.newPath && (file.type === "copy" || file.type === "rename")) { return `${file.oldPath} > ${file.newPath}`; } else if (file.type === "delete") { @@ -385,7 +386,7 @@ class DiffFile extends React.Component { return file.newPath; }; - renderChangeTag = (file: File) => { + renderChangeTag = (file: FileDiff) => { const { t } = this.props; if (!file.type) { return; @@ -401,7 +402,7 @@ class DiffFile extends React.Component { return ; }; - hasContent = (file: File) => file && !file.isBinary && file.hunks && file.hunks.length > 0; + hasContent = (file: FileDiff) => file && !file.isBinary && file.hunks && file.hunks.length > 0; render() { const { fileControlFactory, fileAnnotationFactory, t } = this.props; diff --git a/scm-ui/ui-components/src/repos/DiffTypes.ts b/scm-ui/ui-components/src/repos/DiffTypes.ts index c128f1ed33..53a651af99 100644 --- a/scm-ui/ui-components/src/repos/DiffTypes.ts +++ b/scm-ui/ui-components/src/repos/DiffTypes.ts @@ -24,55 +24,7 @@ import { ReactNode } from "react"; import { DefaultCollapsed } from "./defaultCollapsed"; -import { Links } from "@scm-manager/ui-types"; - -// We place the types here and not in @scm-manager/ui-types, -// because they represent not a real scm-manager related type. -// This types represents only the required types for the Diff related components, -// such as every other component does with its Props. - -export type FileChangeType = "add" | "modify" | "delete" | "copy" | "rename"; - -export type File = { - hunks?: Hunk[]; - newEndingNewLine: boolean; - newMode?: string; - newPath: string; - newRevision?: string; - oldEndingNewLine: boolean; - oldMode?: string; - oldPath: string; - oldRevision?: string; - type: FileChangeType; - language?: string; - // TODO does this property exists? - isBinary?: boolean; - _links?: Links; -}; - -export type Hunk = { - changes: Change[]; - content: string; - oldStart?: number; - newStart?: number; - oldLines?: number; - newLines?: number; - fullyExpanded?: boolean; - expansion?: boolean; -}; - -export type ChangeType = "insert" | "delete" | "normal" | "conflict"; - -export type Change = { - content: string; - isNormal?: boolean; - isInsert?: boolean; - isDelete?: boolean; - lineNumber?: number; - newLineNumber?: number; - oldLineNumber?: number; - type: ChangeType; -}; +import { Change, Hunk, FileDiff as File } from "@scm-manager/ui-types"; export type ChangeEvent = { change: Change; diff --git a/scm-ui/ui-components/src/repos/LoadingDiff.tsx b/scm-ui/ui-components/src/repos/LoadingDiff.tsx index e7983520b0..a95b975e47 100644 --- a/scm-ui/ui-components/src/repos/LoadingDiff.tsx +++ b/scm-ui/ui-components/src/repos/LoadingDiff.tsx @@ -21,93 +21,72 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ -import React from "react"; -import { apiClient, NotFoundError } from "@scm-manager/ui-api"; +import React, { FC } from "react"; +import { NotFoundError, useDiff } from "@scm-manager/ui-api"; import ErrorNotification from "../ErrorNotification"; -// @ts-ignore -import parser from "gitdiff-parser"; +import Notification from "../Notification"; import Loading from "../Loading"; import Diff from "./Diff"; -import { DiffObjectProps, File } from "./DiffTypes"; -import { Notification } from "../index"; -import { withTranslation, WithTranslation } from "react-i18next"; +import { DiffObjectProps } from "./DiffTypes"; +import { useTranslation } from "react-i18next"; +import Button from "../buttons/Button"; +import styled from "styled-components"; -type Props = WithTranslation & - DiffObjectProps & { - url: string; - }; - -type State = { - diff?: File[]; - loading: boolean; - error?: Error; +type Props = DiffObjectProps & { + url: string; + limit?: number; }; -class LoadingDiff extends React.Component { - static defaultProps = { - sideBySide: false - }; +type NotificationProps = { + fetchNextPage: () => void; + isFetchingNextPage: boolean; +}; - constructor(props: Props) { - super(props); - this.state = { - loading: true - }; - } +const StyledNotification = styled(Notification)` + margin-top: 1.5rem; +`; - componentDidMount() { - this.fetchDiff(); - } +const PartialNotification: FC = ({ fetchNextPage, isFetchingNextPage }) => { + const [t] = useTranslation("repos"); + return ( + +
+
{t("changesets.moreDiffsAvailable")}
+
+ + ); +}; - componentDidUpdate(prevProps: Props) { - if (prevProps.url !== this.props.url) { - this.fetchDiff(); +const LoadingDiff: FC = ({ url, limit, ...props }) => { + const { error, isLoading, data, fetchNextPage, isFetchingNextPage } = useDiff(url, { limit }); + const [t] = useTranslation("repos"); + + if (error) { + if (error instanceof NotFoundError) { + return {t("changesets.noChangesets")}; } + return ; + } else if (isLoading) { + return ; + } else if (!data?.files) { + return null; + } else { + return ( + <> + + {data.partial ? ( + + ) : null} + + ); } +}; - fetchDiff = () => { - const { url } = this.props; - this.setState({ loading: true }); - apiClient - .get(url) - .then(response => { - const contentType = response.headers.get("Content-Type"); - if (contentType && contentType.toLowerCase() === "application/vnd.scmm-diffparsed+json;v=2") { - return response.json().then(data => data.files); - } else { - return response.text().then(parser.parse); - } - }) - .then((diff: File[]) => { - this.setState({ - loading: false, - diff: diff - }); - }) - .catch((error: Error) => { - this.setState({ - loading: false, - error - }); - }); - }; +LoadingDiff.defaultProps = { + limit: 25, + sideBySide: false +}; - render() { - const { diff, loading, error } = this.state; - if (error) { - if (error instanceof NotFoundError) { - return {this.props.t("changesets.noChangesets")}; - } - return ; - } else if (loading) { - return ; - } else if (!diff) { - return null; - } else { - return ; - } - } -} - -export default withTranslation("repos")(LoadingDiff); +export default LoadingDiff; diff --git a/scm-ui/ui-components/src/repos/TokenizedDiffView.tsx b/scm-ui/ui-components/src/repos/TokenizedDiffView.tsx index 82242af187..19e9290852 100644 --- a/scm-ui/ui-components/src/repos/TokenizedDiffView.tsx +++ b/scm-ui/ui-components/src/repos/TokenizedDiffView.tsx @@ -25,7 +25,7 @@ import React, { FC } from "react"; import styled from "styled-components"; // @ts-ignore we have no typings for react-diff-view import { Diff, useTokenizeWorker } from "react-diff-view"; -import { File } from "./DiffTypes"; +import { FileDiff } from "@scm-manager/ui-types"; import { determineLanguage } from "../languages"; // @ts-ignore no types for css modules @@ -65,7 +65,7 @@ const tokenize = new Worker("./Tokenize.worker.ts", { name: "tokenizer", type: " tokenize.postMessage({ theme }); type Props = { - file: File; + file: FileDiff; viewType: "split" | "unified"; className?: string; }; diff --git a/scm-ui/ui-components/src/repos/diffs.test.ts b/scm-ui/ui-components/src/repos/diffs.test.ts index 58c5caca5a..98f9b954cb 100644 --- a/scm-ui/ui-components/src/repos/diffs.test.ts +++ b/scm-ui/ui-components/src/repos/diffs.test.ts @@ -22,11 +22,11 @@ * SOFTWARE. */ -import { File, FileChangeType, Hunk } from "./DiffTypes"; +import { FileChangeType, Hunk, FileDiff } from "@scm-manager/ui-types"; import { getPath, createHunkIdentifier, createHunkIdentifierFromContext, escapeWhitespace } from "./diffs"; describe("tests for diff util functions", () => { - const file = (type: FileChangeType, oldPath: string, newPath: string): File => { + const file = (type: FileChangeType, oldPath: string, newPath: string): FileDiff => { return { hunks: [], type: type, diff --git a/scm-ui/ui-components/src/repos/diffs.ts b/scm-ui/ui-components/src/repos/diffs.ts index 0f39f3a4dc..a692bdb8dd 100644 --- a/scm-ui/ui-components/src/repos/diffs.ts +++ b/scm-ui/ui-components/src/repos/diffs.ts @@ -22,16 +22,17 @@ * SOFTWARE. */ -import { BaseContext, File, Hunk } from "./DiffTypes"; +import { BaseContext } from "./DiffTypes"; +import { FileDiff, Hunk } from "@scm-manager/ui-types"; -export function getPath(file: File) { +export function getPath(file: FileDiff) { if (file.type === "delete") { return file.oldPath; } return file.newPath; } -export function createHunkIdentifier(file: File, hunk: Hunk) { +export function createHunkIdentifier(file: FileDiff, hunk: Hunk) { const path = getPath(file); return `${file.type}_${path}_${hunk.content}`; } diff --git a/scm-ui/ui-components/src/repos/index.ts b/scm-ui/ui-components/src/repos/index.ts index f33ffbc4cb..d7a444d2de 100644 --- a/scm-ui/ui-components/src/repos/index.ts +++ b/scm-ui/ui-components/src/repos/index.ts @@ -25,11 +25,6 @@ import * as diffs from "./diffs"; import { - File, - FileChangeType, - Hunk, - Change, - ChangeType, BaseContext, AnnotationFactory, AnnotationFactoryContext, @@ -37,6 +32,7 @@ import { DiffEventContext } from "./DiffTypes"; +import { FileDiff as File, FileChangeType, Hunk, Change, ChangeType } from "@scm-manager/ui-types"; export { diffs }; export * from "./annotate"; diff --git a/scm-ui/ui-types/src/Diff.ts b/scm-ui/ui-types/src/Diff.ts new file mode 100644 index 0000000000..98b7a48d25 --- /dev/null +++ b/scm-ui/ui-types/src/Diff.ts @@ -0,0 +1,73 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { HalRepresentation, Links } from "./hal"; + +export type FileChangeType = "add" | "modify" | "delete" | "copy" | "rename"; + +export type Diff = HalRepresentation & { + files: FileDiff[]; + partial: boolean; +}; + +export type FileDiff = { + hunks?: Hunk[]; + newEndingNewLine: boolean; + newMode?: string; + newPath: string; + newRevision?: string; + oldEndingNewLine: boolean; + oldMode?: string; + oldPath: string; + oldRevision?: string; + type: FileChangeType; + language?: string; + // TODO does this property exists? + isBinary?: boolean; + _links?: Links; +}; + +export type Hunk = { + changes: Change[]; + content: string; + oldStart?: number; + newStart?: number; + oldLines?: number; + newLines?: number; + fullyExpanded?: boolean; + expansion?: boolean; +}; + +export type ChangeType = "insert" | "delete" | "normal" | "conflict"; + +export type Change = { + content: string; + isNormal?: boolean; + isInsert?: boolean; + isDelete?: boolean; + lineNumber?: number; + newLineNumber?: number; + oldLineNumber?: number; + type: ChangeType; +}; diff --git a/scm-ui/ui-types/src/index.ts b/scm-ui/ui-types/src/index.ts index 1073dc5725..d3441e6779 100644 --- a/scm-ui/ui-types/src/index.ts +++ b/scm-ui/ui-types/src/index.ts @@ -64,3 +64,5 @@ export * from "./NamespaceStrategies"; export * from "./LoginInfo"; export * from "./Admin"; + +export * from "./Diff"; diff --git a/scm-ui/ui-webapp/public/locales/de/repos.json b/scm-ui/ui-webapp/public/locales/de/repos.json index 3f13f6dee8..bd9e7bac41 100644 --- a/scm-ui/ui-webapp/public/locales/de/repos.json +++ b/scm-ui/ui-webapp/public/locales/de/repos.json @@ -203,7 +203,9 @@ "errorSubtitle": "Changesets konnten nicht abgerufen werden", "noChangesets": "Keine Changesets in diesem Branch gefunden. Die Commits könnten gelöscht worden sein.", "branchSelectorLabel": "Branches", - "collapseDiffs": "Auf-/Zuklappen" + "collapseDiffs": "Auf-/Zuklappen", + "moreDiffsAvailable": "Es sind weitere Diffs verfügbar", + "loadMore": "Weitere laden" }, "changeset": { "label": "Changeset", diff --git a/scm-ui/ui-webapp/public/locales/en/repos.json b/scm-ui/ui-webapp/public/locales/en/repos.json index c29286e353..7b34ffcdd1 100644 --- a/scm-ui/ui-webapp/public/locales/en/repos.json +++ b/scm-ui/ui-webapp/public/locales/en/repos.json @@ -203,7 +203,9 @@ "errorSubtitle": "Could not fetch changesets", "noChangesets": "No changesets found for this branch. The commits could have been removed.", "branchSelectorLabel": "Branches", - "collapseDiffs": "Collapse" + "collapseDiffs": "Collapse", + "moreDiffsAvailable": "There are more diffs available", + "loadMore": "Load more" }, "changeset": { "label": "Changeset", diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultDto.java index f081a3d173..4b08251053 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultDto.java @@ -42,6 +42,7 @@ public class DiffResultDto extends HalRepresentation { } private List files; + private boolean partial; @Data @EqualsAndHashCode(callSuper = false) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java index 955f11ae12..883ba8f398 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java @@ -39,6 +39,7 @@ import java.util.List; import java.util.Optional; import java.util.OptionalInt; +import static de.otto.edison.hal.Link.link; import static de.otto.edison.hal.Link.linkBuilder; import static de.otto.edison.hal.Links.linkingTo; @@ -55,23 +56,57 @@ class DiffResultToDiffResultDtoMapper { } public DiffResultDto mapForIncoming(Repository repository, DiffResult result, String source, String target) { - DiffResultDto dto = new DiffResultDto(linkingTo().self(resourceLinks.incoming().diffParsed(repository.getNamespace(), repository.getName(), source, target)).build()); + String baseLink = resourceLinks.incoming().diffParsed(repository.getNamespace(), repository.getName(), source, target); + Links.Builder links = linkingTo().self(createSelfLink(result, baseLink)); + appendNextChunkLinkIfNeeded(links, result, baseLink); + DiffResultDto dto = new DiffResultDto(links.build()); setFiles(result, dto, repository, source); return dto; } public DiffResultDto mapForRevision(Repository repository, DiffResult result, String revision) { - DiffResultDto dto = new DiffResultDto(linkingTo().self(resourceLinks.diff().parsed(repository.getNamespace(), repository.getName(), revision)).build()); + String baseLink = resourceLinks.diff().parsed(repository.getNamespace(), repository.getName(), revision); + Links.Builder links = linkingTo().self(createSelfLink(result, baseLink)); + appendNextChunkLinkIfNeeded(links, result, baseLink); + DiffResultDto dto = new DiffResultDto(links.build()); setFiles(result, dto, repository, revision); return dto; } + private String createSelfLink(DiffResult result, String baseLink) { + if (result.getOffset() > 0 || result.getLimit().isPresent()) { + return createLinkWithLimitAndOffset(baseLink, result.getOffset(), result.getLimit().orElse(null)); + } else { + return baseLink; + } + } + + private void appendNextChunkLinkIfNeeded(Links.Builder links, DiffResult result, String baseLink) { + if (result.isPartial()) { + Optional limit = result.getLimit(); + if (limit.isPresent()) { + links.single(link("next", createLinkWithLimitAndOffset(baseLink, result.getOffset() + limit.get(), limit.get()))); + } else { + throw new IllegalStateException("a result cannot be partial without a limit"); + } + } + } + + private String createLinkWithLimitAndOffset(String baseLink, int offset, Integer limit) { + if (limit == null) { + return String.format("%s?offset=%s", baseLink, offset); + } else { + return String.format("%s?offset=%s&limit=%s", baseLink, offset, limit); + } + } + private void setFiles(DiffResult result, DiffResultDto dto, Repository repository, String revision) { List files = new ArrayList<>(); for (DiffFile file : result) { files.add(mapFile(file, repository, revision)); } dto.setFiles(files); + dto.setPartial(result.isPartial()); } private DiffResultDto.FileDto mapFile(DiffFile file, Repository repository, String revision) { @@ -119,7 +154,6 @@ class DiffResultToDiffResultDtoMapper { dto.setOldPath(oldPath); dto.setOldRevision(file.getOldRevision()); - Optional language = ContentTypeResolver.resolve(path).getLanguage(); language.ifPresent(value -> dto.setLanguage(ProgrammingLanguages.getValue(value))); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java index f0644c6dfb..24fc55436f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import io.swagger.v3.oas.annotations.Operation; @@ -39,6 +39,7 @@ import sonia.scm.util.HttpUtil; import sonia.scm.web.VndMediaType; import javax.inject.Inject; +import javax.validation.constraints.Min; import javax.validation.constraints.Pattern; import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; @@ -142,10 +143,18 @@ public class DiffRootResource { schema = @Schema(implementation = ErrorDto.class) ) ) - public DiffResultDto getParsed(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision) throws IOException { + public DiffResultDto getParsed(@PathParam("namespace") String namespace, + @PathParam("name") String name, + @PathParam("revision") String revision, + @QueryParam("limit") @Min(1) Integer limit, + @QueryParam("offset") @Min(0) Integer offset) throws IOException { HttpUtil.checkForCRLFInjection(revision); try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - DiffResult diffResult = repositoryService.getDiffResultCommand().setRevision(revision).getDiffResult(); + DiffResult diffResult = repositoryService.getDiffResultCommand() + .setRevision(revision) + .setLimit(limit) + .setOffset(offset) + .getDiffResult(); return parsedDiffMapper.mapForRevision(repositoryService.getRepository(), diffResult, revision); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IncomingRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IncomingRootResource.java index 2695a048a4..f1b5e07566 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IncomingRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IncomingRootResource.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import com.google.inject.Inject; @@ -43,6 +43,7 @@ import sonia.scm.repository.api.RepositoryServiceFactory; import sonia.scm.util.HttpUtil; import sonia.scm.web.VndMediaType; +import javax.validation.constraints.Min; import javax.validation.constraints.Pattern; import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; @@ -240,15 +241,19 @@ public class IncomingRootResource { schema = @Schema(implementation = ErrorDto.class) )) public Response incomingDiffParsed(@PathParam("namespace") String namespace, - @PathParam("name") String name, - @PathParam("source") String source, - @PathParam("target") String target) throws IOException { + @PathParam("name") String name, + @PathParam("source") String source, + @PathParam("target") String target, + @QueryParam("limit") @Min(1) Integer limit, + @QueryParam("offset") @Min(0) Integer offset) throws IOException { HttpUtil.checkForCRLFInjection(source); HttpUtil.checkForCRLFInjection(target); try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { DiffResult diffResult = repositoryService.getDiffResultCommand() .setRevision(source) .setAncestorChangeset(target) + .setLimit(limit) + .setOffset(offset) .getDiffResult(); return Response.ok(parsedDiffMapper.mapForIncoming(repositoryService.getRepository(), diffResult, source, target)).build(); } diff --git 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 index 1e351f0510..da22f511a8 100644 --- 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")); diff --git 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 index 8fca3ddff4..65a931f2c1 100644 --- 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 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 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 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 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"), diff --git 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 index f2a5fe02b0..ca5f8b79e1 100644 --- 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")