From 84c8e02bf1c05a3e9e913a0a183127e4f96e9b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 12 Mar 2021 13:52:17 +0100 Subject: [PATCH] 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 --- gradle/changelog/partial_diff.yaml | 2 + .../api/AbstractDiffCommandBuilder.java | 8 +- .../repository/api/DiffCommandBuilder.java | 13 +- .../sonia/scm/repository/api/DiffResult.java | 18 +- .../api/DiffResultCommandBuilder.java | 35 ++- .../repository/spi/DiffCommandRequest.java | 2 +- .../scm/repository/spi/DiffResultCommand.java | 6 +- .../spi/DiffResultCommandRequest.java | 38 +++ .../repository/spi/GitDiffResultCommand.java | 48 +++- .../spi/GitDiffResultCommandTest.java | 51 +++- scm-ui/ui-api/package.json | 3 +- scm-ui/ui-api/src/diff.test.ts | 225 ++++++++++++++++++ scm-ui/ui-api/src/diff.ts | 86 +++++++ scm-ui/ui-api/src/index.ts | 1 + scm-ui/ui-components/package.json | 4 +- .../ui-components/src/repos/Diff.stories.tsx | 8 +- scm-ui/ui-components/src/repos/Diff.tsx | 5 +- .../src/repos/DiffExpander.test.ts | 22 +- .../ui-components/src/repos/DiffExpander.ts | 15 +- scm-ui/ui-components/src/repos/DiffFile.tsx | 21 +- scm-ui/ui-components/src/repos/DiffTypes.ts | 50 +--- .../ui-components/src/repos/LoadingDiff.tsx | 131 +++++----- .../src/repos/TokenizedDiffView.tsx | 4 +- scm-ui/ui-components/src/repos/diffs.test.ts | 4 +- scm-ui/ui-components/src/repos/diffs.ts | 7 +- scm-ui/ui-components/src/repos/index.ts | 6 +- scm-ui/ui-types/src/Diff.ts | 73 ++++++ scm-ui/ui-types/src/index.ts | 2 + scm-ui/ui-webapp/public/locales/de/repos.json | 4 +- scm-ui/ui-webapp/public/locales/en/repos.json | 4 +- .../scm/api/v2/resources/DiffResultDto.java | 1 + .../DiffResultToDiffResultDtoMapper.java | 40 +++- .../api/v2/resources/DiffRootResource.java | 15 +- .../v2/resources/IncomingRootResource.java | 13 +- .../api/v2/resources/DiffResourceTest.java | 33 +++ .../DiffResultToDiffResultDtoMapperTest.java | 66 ++++- .../resources/IncomingRootResourceTest.java | 59 +++-- 37 files changed, 900 insertions(+), 223 deletions(-) create mode 100644 gradle/changelog/partial_diff.yaml create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/DiffResultCommandRequest.java create mode 100644 scm-ui/ui-api/src/diff.test.ts create mode 100644 scm-ui/ui-api/src/diff.ts create mode 100644 scm-ui/ui-types/src/Diff.ts 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")