From 891e3587b3601d31e4d290e2e77a57e9f3e20ef4 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 27 Feb 2019 11:56:50 +0100 Subject: [PATCH 01/29] implemented api for diff annotations --- .../packages/ui-components/src/index.js | 13 ++ .../packages/ui-components/src/repos/Diff.js | 19 ++- .../ui-components/src/repos/DiffFile.js | 114 +++++++++++++----- .../ui-components/src/repos/DiffTypes.js | 61 ++++++++++ .../ui-components/src/repos/LoadingDiff.js | 8 +- .../packages/ui-components/src/repos/diffs.js | 18 +++ .../ui-components/src/repos/diffs.test.js | 76 ++++++++++++ .../packages/ui-components/src/repos/index.js | 15 +++ 8 files changed, 280 insertions(+), 44 deletions(-) create mode 100644 scm-ui-components/packages/ui-components/src/repos/DiffTypes.js create mode 100644 scm-ui-components/packages/ui-components/src/repos/diffs.js create mode 100644 scm-ui-components/packages/ui-components/src/repos/diffs.test.js diff --git a/scm-ui-components/packages/ui-components/src/index.js b/scm-ui-components/packages/ui-components/src/index.js index 5b3cdb4c95..7bd844f0a8 100644 --- a/scm-ui-components/packages/ui-components/src/index.js +++ b/scm-ui-components/packages/ui-components/src/index.js @@ -36,3 +36,16 @@ export * from "./layout"; export * from "./modals"; export * from "./navigation"; export * from "./repos"; + +// not sure if it is required +export type { + File, + FileChangeType, + Hunk, + Change, + BaseContext, + AnnotationFactory, + AnnotationFactoryContext, + DiffEventHandler, + DiffEventContext +} from "./repos"; diff --git a/scm-ui-components/packages/ui-components/src/repos/Diff.js b/scm-ui-components/packages/ui-components/src/repos/Diff.js index 61fd1ad814..369e812344 100644 --- a/scm-ui-components/packages/ui-components/src/repos/Diff.js +++ b/scm-ui-components/packages/ui-components/src/repos/Diff.js @@ -1,32 +1,27 @@ //@flow import React from "react"; import DiffFile from "./DiffFile"; +import type { DiffObjectProps } from "./DiffTypes"; -type Props = { - diff: any, - sideBySide: boolean +type Props = DiffObjectProps & { + diff: any }; class Diff extends React.Component { - static defaultProps = { sideBySide: false }; - renderFile = (file: any, i: number) => { - const { sideBySide } = this.props; - return ; - }; - render() { - const { diff } = this.props; + const { diff, ...fileProps } = this.props; return ( <> - {diff.map(this.renderFile)} + {diff.map((file, index) => ( + + ))} ); } - } export default Diff; diff --git a/scm-ui-components/packages/ui-components/src/repos/DiffFile.js b/scm-ui-components/packages/ui-components/src/repos/DiffFile.js index 71aa4e282c..d5b67184c6 100644 --- a/scm-ui-components/packages/ui-components/src/repos/DiffFile.js +++ b/scm-ui-components/packages/ui-components/src/repos/DiffFile.js @@ -1,9 +1,16 @@ //@flow import React from "react"; -import { Hunk, Diff as DiffComponent } from "react-diff-view"; +import { + Hunk, + Diff as DiffComponent, + getChangeKey, + Change, + DiffObjectProps, + File +} from "react-diff-view"; import injectSheets from "react-jss"; import classNames from "classnames"; -import {translate} from "react-i18next"; +import { translate } from "react-i18next"; const styles = { panel: { @@ -21,20 +28,18 @@ const styles = { } }; -type Props = { - file: any, - sideBySide: boolean, +type Props = DiffObjectProps & { + file: File, // context props classes: any, t: string => string -} +}; type State = { collapsed: boolean -} +}; class DiffFile extends React.Component { - constructor(props: Props) { super(props); this.state = { @@ -43,23 +48,77 @@ class DiffFile extends React.Component { } toggleCollapse = () => { - this.setState((state) => ({ - collapsed: ! state.collapsed + this.setState(state => ({ + collapsed: !state.collapsed })); }; - renderHunk = (hunk: any, i: number) => { + createHunkHeader = (hunk: Hunk, i: number) => { const { classes } = this.props; - let header = null; if (i > 0) { - header =
; + return
; } - return ; + return null; + }; + + collectHunkAnnotations = (hunk: Hunk) => { + const { annotationFactory, file } = this.props; + if (annotationFactory) { + return annotationFactory({ + hunk, + file + }); + } + }; + + handleClickEvent = (change: Change, hunk: Hunk) => { + const { file, onClick } = this.props; + const context = { + changeId: getChangeKey(change), + change, + hunk, + file + }; + if (onClick) { + onClick(context); + } + }; + + createCustomEvents = (hunk: Hunk) => { + const { onClick } = this.props; + if (onClick) { + return { + gutter: { + onClick: (change: Change) => { + this.handleClickEvent(change, hunk); + } + } + }; + } + }; + + renderHunk = (hunk: Hunk, i: number) => { + return ( + + ); }; renderFileTitle = (file: any) => { - if (file.oldPath !== file.newPath && (file.type === "copy" || file.type === "rename")) { - return (<>{file.oldPath} {file.newPath}); + if ( + file.oldPath !== file.newPath && + (file.type === "copy" || file.type === "rename") + ) { + return ( + <> + {file.oldPath} {file.newPath} + + ); } else if (file.type === "delete") { return file.oldPath; } @@ -73,11 +132,7 @@ class DiffFile extends React.Component { if (key === value) { value = file.type; } - return ( - - {value} - - ); + return {value}; }; render() { @@ -92,7 +147,7 @@ class DiffFile extends React.Component { body = (
- { file.hunks.map(this.renderHunk) } + {file.hunks.map(this.renderHunk)}
); @@ -100,21 +155,24 @@ class DiffFile extends React.Component { return (
-
+
- {this.renderFileTitle(file)} -
-
- {this.renderChangeTag(file)} + + + {this.renderFileTitle(file)} +
+
{this.renderChangeTag(file)}
{body}
); } - } export default injectSheets(styles)(translate("repos")(DiffFile)); diff --git a/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js b/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js new file mode 100644 index 0000000000..885a8f2e96 --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js @@ -0,0 +1,61 @@ +// @flow + +// 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 +}; + +export type Hunk = { + changes: Change[], + content: string +}; + +export type Change = { + content: string, + isNormal: boolean, + newLineNumber: number, + oldLineNumber: number, + type: string +}; + +export type BaseContext = { + hunk: Hunk, + file: File +}; + +export type AnnotationFactoryContext = BaseContext; + +// key = change id, value = react component +export type AnnotationFactory = ( + context: AnnotationFactoryContext +) => { + [string]: any +}; + +export type DiffEventContext = BaseContext & { + changeId: string, + change: Change +}; + +export type DiffEventHandler = (context: DiffEventContext) => void; + +export type DiffObjectProps = { + sideBySide: boolean, + onClick?: DiffEventHandler, + annotationFactory?: AnnotationFactory +}; diff --git a/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js b/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js index 5b51f790e4..ce1a074b41 100644 --- a/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js +++ b/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js @@ -6,10 +6,10 @@ import parser from "gitdiff-parser"; import Loading from "../Loading"; import Diff from "./Diff"; +import type {DiffObjectProps} from "./DiffTypes"; -type Props = { - url: string, - sideBySide: boolean +type Props = DiffObjectProps & { + url: string }; type State = { @@ -71,7 +71,7 @@ class LoadingDiff extends React.Component { return null; } else { - return ; + return ; } } diff --git a/scm-ui-components/packages/ui-components/src/repos/diffs.js b/scm-ui-components/packages/ui-components/src/repos/diffs.js new file mode 100644 index 0000000000..19d6a62b0d --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/repos/diffs.js @@ -0,0 +1,18 @@ +// @flow +import type { BaseContext, File, Hunk } from "./DiffTypes"; + +export function getPath(file: File) { + if (file.type === "delete") { + return file.oldPath; + } + return file.newPath; +} + +export function createHunkIdentifier(file: File, hunk: Hunk) { + const path = getPath(file); + return `${file.type}_${path}_${hunk.content}`; +} + +export function createHunkIdentifierFromContext(ctx: BaseContext) { + return createHunkIdentifier(ctx.file, ctx.hunk); +} diff --git a/scm-ui-components/packages/ui-components/src/repos/diffs.test.js b/scm-ui-components/packages/ui-components/src/repos/diffs.test.js new file mode 100644 index 0000000000..4aeaabe11b --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/repos/diffs.test.js @@ -0,0 +1,76 @@ +// @flow +import type { File, FileChangeType, Hunk } from "./DiffTypes"; +import { + getPath, + createHunkIdentifier, + createHunkIdentifierFromContext +} from "./diffs"; + +describe("tests for diff util functions", () => { + const file = ( + type: FileChangeType, + oldPath: string, + newPath: string + ): File => { + return { + hunks: [], + type: type, + oldPath, + newPath, + newEndingNewLine: true, + oldEndingNewLine: true + }; + }; + + const add = (path: string) => { + return file("add", "/dev/null", path); + }; + + const rm = (path: string) => { + return file("delete", path, "/dev/null"); + }; + + const modify = (path: string) => { + return file("modify", path, path); + }; + + const createHunk = (content: string): Hunk => { + return { + content, + changes: [] + }; + }; + + describe("getPath tests", () => { + it("should pick the new path, for type add", () => { + const file = add("/etc/passwd"); + const path = getPath(file); + expect(path).toBe("/etc/passwd"); + }); + + it("should pick the old path, for type delete", () => { + const file = rm("/etc/passwd"); + const path = getPath(file); + expect(path).toBe("/etc/passwd"); + }); + }); + + describe("createHunkIdentifier tests", () => { + it("should create identifier", () => { + const file = modify("/etc/passwd"); + const hunk = createHunk("@@ -1,18 +1,15 @@"); + const identifier = createHunkIdentifier(file, hunk); + expect(identifier).toBe("modify_/etc/passwd_@@ -1,18 +1,15 @@"); + }); + }); + + describe("createHunkIdentifierFromContext tests", () => { + it("should create identifier", () => { + const identifier = createHunkIdentifierFromContext({ + file: rm("/etc/passwd"), + hunk: createHunk("@@ -1,42 +1,39 @@") + }); + expect(identifier).toBe("delete_/etc/passwd_@@ -1,42 +1,39 @@"); + }); + }); +}); diff --git a/scm-ui-components/packages/ui-components/src/repos/index.js b/scm-ui-components/packages/ui-components/src/repos/index.js index 9ebd1e9c55..473bbb3efc 100644 --- a/scm-ui-components/packages/ui-components/src/repos/index.js +++ b/scm-ui-components/packages/ui-components/src/repos/index.js @@ -1,5 +1,20 @@ // @flow +import * as diffs from "./diffs"; +export { diffs }; export * from "./changesets"; + export { default as Diff } from "./Diff"; export { default as LoadingDiff } from "./LoadingDiff"; + +export type { + File, + FileChangeType, + Hunk, + Change, + BaseContext, + AnnotationFactory, + AnnotationFactoryContext, + DiffEventHandler, + DiffEventContext +} from "./DiffTypes"; From b537a2a2ba1bb6ca27d8885ce2e6d3d832784d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 1 Mar 2019 14:10:39 +0100 Subject: [PATCH 02/29] Fix deletion of permissions after modification Permissions could not be deleted, when they were modified (eg. change of role or of verbs). --- .../java/sonia/scm/repository/Repository.java | 4 +- .../scm/repository/RepositoryPermission.java | 50 +++++++++++++------ .../repository/RepositoryPermissionTest.java | 3 +- .../repos/permissions/modules/permissions.js | 5 +- ...issionDtoToRepositoryPermissionMapper.java | 12 +---- .../RepositoryPermissionRootResource.java | 8 ++- .../RepositoryPermissionRootResourceTest.java | 17 +++++-- 7 files changed, 62 insertions(+), 37 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/Repository.java b/scm-core/src/main/java/sonia/scm/repository/Repository.java index 18613c1a12..665f63487d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/Repository.java +++ b/scm-core/src/main/java/sonia/scm/repository/Repository.java @@ -307,8 +307,8 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per this.permissions.add(newPermission); } - public void removePermission(RepositoryPermission permission) { - this.permissions.remove(permission); + public boolean removePermission(RepositoryPermission permission) { + return this.permissions.remove(permission); } public void setPublicReadable(boolean publicReadable) { diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java index 54163e0393..4a458a6e6a 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java @@ -45,6 +45,7 @@ import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; import java.io.Serializable; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; @@ -55,6 +56,8 @@ import static java.util.Collections.unmodifiableSet; /** * Permissions controls the access to {@link Repository}. + * This object should be immutable, but could not be due to mapstruct. Do not modify instances of this because this + * would change the hash code and therefor make it undeletable in a repository. * * @author Sebastian Sdorra */ @@ -64,22 +67,26 @@ public class RepositoryPermission implements PermissionObject, Serializable { private static final long serialVersionUID = -2915175031430884040L; + public static final String REPOSITORY_MODIFIED_EXCEPTION_TEXT = "repository permission must not be modified"; - private boolean groupPermission = false; + private Boolean groupPermission; private String name; @XmlElement(name = "verb") private Set verbs; /** - * Constructs a new {@link RepositoryPermission}. - * This constructor is used by JAXB and mapstruct. + * This constructor exists for mapstruct and JAXB, only -- do not use this in "normal" code. + * + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public RepositoryPermission() {} public RepositoryPermission(String name, Collection verbs, boolean groupPermission) { this.name = name; - this.verbs = unmodifiableSet(new LinkedHashSet<>(verbs)); + this.verbs = new LinkedHashSet<>(verbs); this.groupPermission = groupPermission; } @@ -163,7 +170,7 @@ public class RepositoryPermission implements PermissionObject, Serializable */ public Collection getVerbs() { - return verbs == null? emptyList(): verbs; + return verbs == null ? emptyList() : Collections.unmodifiableSet(verbs); } /** @@ -181,35 +188,50 @@ public class RepositoryPermission implements PermissionObject, Serializable //~--- set methods ---------------------------------------------------------- /** - * Sets true if the permission is a group permission. + * Use this for creation only. This will throw an {@link IllegalStateException} when modified. + * @throws IllegalStateException when modified after the value has been set once. * - * - * @param groupPermission true if the permission is a group permission + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public void setGroupPermission(boolean groupPermission) { + if (this.groupPermission != null) { + throw new IllegalStateException(REPOSITORY_MODIFIED_EXCEPTION_TEXT); + } this.groupPermission = groupPermission; } /** - * The name of the user or group. + * Use this for creation only. This will throw an {@link IllegalStateException} when modified. + * @throws IllegalStateException when modified after the value has been set once. * - * - * @param name name of the user or group + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public void setName(String name) { + if (this.name != null) { + throw new IllegalStateException(REPOSITORY_MODIFIED_EXCEPTION_TEXT); + } this.name = name; } /** - * Sets the verb of the permission. + * Use this for creation only. This will throw an {@link IllegalStateException} when modified. + * @throws IllegalStateException when modified after the value has been set once. * - * - * @param verbs verbs of the permission + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public void setVerbs(Collection verbs) { + if (this.verbs != null) { + throw new IllegalStateException(REPOSITORY_MODIFIED_EXCEPTION_TEXT); + } this.verbs = unmodifiableSet(new LinkedHashSet<>(verbs)); } } diff --git a/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java b/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java index d65358a66e..c7f05c5f3c 100644 --- a/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java @@ -50,8 +50,7 @@ class RepositoryPermissionTest { @Test void shouldBeEqualWithRedundantVerbs() { RepositoryPermission permission1 = new RepositoryPermission("name1", asList("one", "two"), false); - RepositoryPermission permission2 = new RepositoryPermission("name1", asList("one", "two"), false); - permission2.setVerbs(asList("one", "two", "two")); + RepositoryPermission permission2 = new RepositoryPermission("name1", asList("one", "two", "two"), false); assertThat(permission1).isEqualTo(permission2); } diff --git a/scm-ui/src/repos/permissions/modules/permissions.js b/scm-ui/src/repos/permissions/modules/permissions.js index cb6b013c61..8c4161e907 100644 --- a/scm-ui/src/repos/permissions/modules/permissions.js +++ b/scm-ui/src/repos/permissions/modules/permissions.js @@ -451,7 +451,10 @@ function deletePermissionFromState( ) { let newPermission = []; for (let i = 0; i < oldPermissions.length; i++) { - if (oldPermissions[i] !== permission) { + if ( + oldPermissions[i].name !== permission.name || + oldPermissions[i].groupPermission !== permission.groupPermission + ) { newPermission.push(oldPermissions[i]); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java index 8e015e8b60..881e9251f0 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java @@ -5,18 +5,8 @@ import org.mapstruct.Mapper; import org.mapstruct.MappingTarget; import sonia.scm.repository.RepositoryPermission; -@Mapper(collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE) +@Mapper( collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE) public abstract class RepositoryPermissionDtoToRepositoryPermissionMapper { public abstract RepositoryPermission map(RepositoryPermissionDto permissionDto); - - /** - * this method is needed to modify an existing permission object - * - * @param target the target permission - * @param repositoryPermissionDto the source dto - * @return the mapped target permission object - */ - public abstract void modify(@MappingTarget RepositoryPermission target, RepositoryPermissionDto repositoryPermissionDto); - } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java index dd66e6e5f2..d149f55401 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java @@ -177,7 +177,11 @@ public class RepositoryPermissionRootResource { .filter(filterPermission(permissionName)) .findFirst() .orElseThrow(() -> notFound(entity(RepositoryPermission.class, namespace).in(Repository.class, namespace + "/" + name))); - dtoToModelMapper.modify(existingPermission, permission); + RepositoryPermission newPermission = dtoToModelMapper.map(permission); + if (!repository.removePermission(existingPermission)) { + throw new IllegalStateException(String.format("could not delete modified permission %s from repository %s/%s", existingPermission, namespace, name)); + } + repository.addPermission(newPermission); manager.modify(repository); log.info("the permission with name: {} is updated.", permissionName); return Response.noContent().build(); @@ -210,7 +214,7 @@ public class RepositoryPermissionRootResource { .findFirst() .ifPresent(repository::removePermission); manager.modify(repository); - log.info("the permission with name: {} is updated.", permissionName); + log.info("the permission with name: {} is deleted.", permissionName); return Response.noContent().build(); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java index 4472acb2c5..fb533ad280 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java @@ -301,11 +301,18 @@ public class RepositoryPermissionRootResourceTest extends RepositoryTestBase { @Test public void shouldGetUpdatedPermissions() throws URISyntaxException { - createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE); - RepositoryPermission modifiedPermission = TEST_PERMISSIONS.get(0); - // modify the type to owner - modifiedPermission.setVerbs(new ArrayList<>(singletonList("*"))); - ImmutableList expectedPermissions = ImmutableList.copyOf(TEST_PERMISSIONS); + ArrayList permissions = Lists + .newArrayList( + new RepositoryPermission("user_write", asList("*"), false), + new RepositoryPermission("user_read", singletonList("read"), false), + new RepositoryPermission("user_owner", singletonList("*"), false), + new RepositoryPermission("group_read", singletonList("read"), true), + new RepositoryPermission("group_write", asList("read", "modify"), true), + new RepositoryPermission("group_owner", singletonList("*"), true) + ); + createUserWithRepositoryAndPermissions(permissions, PERMISSION_WRITE); + RepositoryPermission modifiedPermission = permissions.get(0); + ImmutableList expectedPermissions = ImmutableList.copyOf(permissions); assertExpectedRequest(requestPUTPermission .content("{\"name\" : \"" + modifiedPermission.getName() + "\" , \"verbs\" : [\"*\"], \"groupPermission\" : false}") .path(PATH_OF_ALL_PERMISSIONS + modifiedPermission.getName()) From ffbf9e4fba049e89e55ebc47b8afe514bcf31b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 4 Mar 2019 13:22:40 +0100 Subject: [PATCH 03/29] Add independent classes from ssh plugin --- .../scm/protocolcommand/CommandContext.java | 20 ++++++++++++++++ .../scm/protocolcommand/CommandParser.java | 8 +++++++ .../protocolcommand/RepositoryContext.java | 23 +++++++++++++++++++ .../RepositoryContextResolver.java | 11 +++++++++ .../scm/protocolcommand/ScmSshProtocol.java | 9 ++++++++ 5 files changed, 71 insertions(+) create mode 100644 scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java create mode 100644 scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java create mode 100644 scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java create mode 100644 scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java create mode 100644 scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java new file mode 100644 index 0000000000..44a1cce95a --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java @@ -0,0 +1,20 @@ +package sonia.scm.protocolcommand; + +import lombok.AllArgsConstructor; +import lombok.Getter; + +import java.io.InputStream; +import java.io.OutputStream; + +@Getter +@AllArgsConstructor +public class CommandContext { + + private String command; + private String[] args; + + private InputStream inputStream; + private OutputStream outputStream; + private OutputStream errorStream; + +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java new file mode 100644 index 0000000000..86fa9554e9 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java @@ -0,0 +1,8 @@ +package sonia.scm.protocolcommand; + +@FunctionalInterface +public interface CommandParser { + + String[] parse(String command); + +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java new file mode 100644 index 0000000000..a64d5a6047 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java @@ -0,0 +1,23 @@ +package sonia.scm.protocolcommand; + +import sonia.scm.repository.Repository; + +import java.nio.file.Path; + +public class RepositoryContext { + private Repository repository; + private Path directory; + + public RepositoryContext(Repository repository, Path directory) { + this.repository = repository; + this.directory = directory; + } + + public Repository getRepository() { + return repository; + } + + public Path getDirectory() { + return directory; + } +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java new file mode 100644 index 0000000000..69d762109d --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java @@ -0,0 +1,11 @@ +package sonia.scm.protocolcommand; + +import sonia.scm.plugin.ExtensionPoint; + +@FunctionalInterface +@ExtensionPoint +public interface RepositoryContextResolver { + + RepositoryContext resolve(String[] args); + +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java new file mode 100644 index 0000000000..0a016c3806 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java @@ -0,0 +1,9 @@ +package sonia.scm.protocolcommand; + +import java.io.IOException; + +public interface ScmSshProtocol { + + void handle(CommandContext context, RepositoryContext repositoryContext) throws IOException; + +} From adcfb3ee5aa4a08269f7fcea0929b7f133b64595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 4 Mar 2019 14:27:07 +0100 Subject: [PATCH 04/29] Do not map not found exception manually --- .../git/GitProtocolModule.java | 15 ++++ .../git/GitRepositoryContextResolver.java | 44 ++++++++++++ .../protocolcommand/git/GitSshProtocol.java | 68 +++++++++++++++++++ .../git/SshReceivePackFactory.java | 44 ++++++++++++ .../git/SshUploadPackFactory.java | 13 ++++ 5 files changed, 184 insertions(+) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshUploadPackFactory.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java new file mode 100644 index 0000000000..897f71ad4b --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java @@ -0,0 +1,15 @@ +package sonia.scm.protocolcommand.git; + +import com.google.inject.servlet.ServletModule; +import sonia.scm.plugin.Extension; +import sonia.scm.protocolcommand.RepositoryContextResolver; +import sonia.scm.protocolcommand.ScmSshProtocol; + +@Extension +public class GitProtocolModule extends ServletModule { + @Override + protected void configureServlets() { + bind(RepositoryContextResolver.class).to(GitRepositoryContextResolver.class); + bind(ScmSshProtocol.class).to(GitSshProtocol.class); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java new file mode 100644 index 0000000000..8acfc68dce --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java @@ -0,0 +1,44 @@ +package sonia.scm.protocolcommand.git; + +import com.google.common.base.Splitter; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.protocolcommand.RepositoryContextResolver; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryLocationResolver; +import sonia.scm.repository.RepositoryManager; + +import javax.inject.Inject; +import java.nio.file.Path; +import java.util.Iterator; + +public class GitRepositoryContextResolver implements RepositoryContextResolver { + + private RepositoryManager repositoryManager; + private RepositoryLocationResolver locationResolver; + + @Inject + public GitRepositoryContextResolver(RepositoryManager repositoryManager, RepositoryLocationResolver locationResolver) { + this.repositoryManager = repositoryManager; + this.locationResolver = locationResolver; + } + + public RepositoryContext resolve(String[] args) { + NamespaceAndName namespaceAndName = extractNamespaceAndName(args); + Repository repository = repositoryManager.get(namespaceAndName); + Path path = locationResolver.getPath(repository.getId()).resolve("data"); + return new RepositoryContext(repository, path); + } + + private NamespaceAndName extractNamespaceAndName(String[] args) { + String path = args[args.length - 1]; + Iterator it = Splitter.on('/').omitEmptyStrings().split(path).iterator(); + String type = it.next(); + if ("repo".equals(type)) { + String ns = it.next(); + String name = it.next(); + return new NamespaceAndName(ns, name); + } + return null; + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java new file mode 100644 index 0000000000..c80127f1a7 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java @@ -0,0 +1,68 @@ +package sonia.scm.protocolcommand.git; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.RepositoryCache; +import org.eclipse.jgit.transport.ReceivePack; +import org.eclipse.jgit.transport.RemoteConfig; +import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.util.FS; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.protocolcommand.CommandContext; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.protocolcommand.ScmSshProtocol; +import sonia.scm.repository.RepositoryPermissions; + +import javax.inject.Inject; +import java.io.IOException; + +public class GitSshProtocol implements ScmSshProtocol { + + private static final Logger LOG = LoggerFactory.getLogger(GitSshProtocol.class); + + private SshUploadPackFactory uploadPackFactory; + private SshReceivePackFactory receivePackFactory; + + @Inject + public GitSshProtocol(SshUploadPackFactory uploadPackFactory, SshReceivePackFactory receivePackFactory) { + this.uploadPackFactory = uploadPackFactory; + this.receivePackFactory = receivePackFactory; + } + + @Override + public void handle(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { + String subCommand = commandContext.getArgs()[0]; + + if (RemoteConfig.DEFAULT_UPLOAD_PACK.equals(subCommand)) { + LOG.trace("got upload pack"); + upload(commandContext, repositoryContext); + } else if (RemoteConfig.DEFAULT_RECEIVE_PACK.equals(subCommand)) { + LOG.trace("got receive pack"); + receive(commandContext, repositoryContext); + } else { + throw new IllegalArgumentException("Unknown git command: " + commandContext.getCommand()); + } + } + + private void receive(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { + RepositoryPermissions.push(repositoryContext.getRepository()).check(); + try (Repository repository = open(repositoryContext)) { + ReceivePack receivePack = receivePackFactory.create(repositoryContext, repository); + receivePack.receive(commandContext.getInputStream(), commandContext.getOutputStream(), commandContext.getErrorStream()); + } + } + + private void upload(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { + RepositoryPermissions.pull(repositoryContext.getRepository()).check(); + try (Repository repository = open(repositoryContext)) { + UploadPack uploadPack = uploadPackFactory.create(repositoryContext, repository); + uploadPack.upload(commandContext.getInputStream(), commandContext.getOutputStream(), commandContext.getErrorStream()); + } + } + + private Repository open(RepositoryContext repositoryContext) throws IOException { + RepositoryCache.FileKey key = RepositoryCache.FileKey.lenient(repositoryContext.getDirectory().toFile(), FS.DETECTED); + return key.open(true); + } + +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java new file mode 100644 index 0000000000..819b92ca27 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java @@ -0,0 +1,44 @@ +package sonia.scm.protocolcommand.git; + +import com.google.inject.Inject; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.ReceivePack; +import org.eclipse.jgit.transport.resolver.ReceivePackFactory; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.repository.GitRepositoryHandler; +import sonia.scm.repository.spi.HookEventFacade; +import sonia.scm.web.CollectingPackParserListener; +import sonia.scm.web.GitReceiveHook; + +/** + * TODO we should have a single/abstract ReceivePackFactory for http and ssh. + */ +public class SshReceivePackFactory implements ReceivePackFactory { + + private final GitRepositoryHandler handler; + private final GitReceiveHook hook; + + @Inject + public SshReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + this.handler = handler; + this.hook = new GitReceiveHook(hookEventFacade, handler); + } + + @Override + public ReceivePack create(RepositoryContext repositoryContext, Repository repository) { + ReceivePack receivePack = new ReceivePack(repository); + receivePack.setAllowNonFastForwards(isNonFastForwardAllowed()); + + receivePack.setPreReceiveHook(hook); + receivePack.setPostReceiveHook(hook); + + // apply collecting listener, to be able to check which commits are new + CollectingPackParserListener.set(receivePack); + + return receivePack; + } + + private boolean isNonFastForwardAllowed() { + return ! handler.getConfig().isNonFastForwardDisallowed(); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshUploadPackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshUploadPackFactory.java new file mode 100644 index 0000000000..9e8c46cff4 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshUploadPackFactory.java @@ -0,0 +1,13 @@ +package sonia.scm.protocolcommand.git; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.transport.resolver.UploadPackFactory; +import sonia.scm.protocolcommand.RepositoryContext; + +public class SshUploadPackFactory implements UploadPackFactory { + @Override + public UploadPack create(RepositoryContext repositoryContext, Repository repository) { + return new UploadPack(repository); + } +} From 0c46d639da6166835195f1e1061cecbe360116e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 5 Mar 2019 13:29:57 +0100 Subject: [PATCH 05/29] Extract base class for ReceivePackFactory implementations --- .../git/BaseReceivePackFactory.java | 42 +++++++++++++++++++ .../protocolcommand/git/GitSshProtocol.java | 4 ++ .../git/SshReceivePackFactory.java | 31 ++------------ .../sonia/scm/web/GitReceivePackFactory.java | 30 ++++--------- 4 files changed, 57 insertions(+), 50 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java new file mode 100644 index 0000000000..4fc3a5415c --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java @@ -0,0 +1,42 @@ +package sonia.scm.protocolcommand.git; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.ReceivePack; +import org.eclipse.jgit.transport.resolver.ReceivePackFactory; +import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import sonia.scm.repository.GitRepositoryHandler; +import sonia.scm.repository.spi.HookEventFacade; +import sonia.scm.web.CollectingPackParserListener; +import sonia.scm.web.GitReceiveHook; + +public abstract class BaseReceivePackFactory implements ReceivePackFactory { + + private final GitRepositoryHandler handler; + private final GitReceiveHook hook; + + protected BaseReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + this.handler = handler; + this.hook = new GitReceiveHook(hookEventFacade, handler); + } + + @Override + public final ReceivePack create(T connection, Repository repository) throws ServiceNotAuthorizedException, ServiceNotEnabledException { + ReceivePack receivePack = createBasicReceivePack(connection, repository); + receivePack.setAllowNonFastForwards(isNonFastForwardAllowed()); + + receivePack.setPreReceiveHook(hook); + receivePack.setPostReceiveHook(hook); + // apply collecting listener, to be able to check which commits are new + CollectingPackParserListener.set(receivePack); + + return receivePack; + } + + protected abstract ReceivePack createBasicReceivePack(T request, Repository repository) + throws ServiceNotEnabledException, ServiceNotAuthorizedException; + + private boolean isNonFastForwardAllowed() { + return ! handler.getConfig().isNonFastForwardDisallowed(); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java index c80127f1a7..df4ae99553 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java @@ -5,6 +5,8 @@ import org.eclipse.jgit.lib.RepositoryCache; import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.RemoteConfig; import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.util.FS; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,6 +51,8 @@ public class GitSshProtocol implements ScmSshProtocol { try (Repository repository = open(repositoryContext)) { ReceivePack receivePack = receivePackFactory.create(repositoryContext, repository); receivePack.receive(commandContext.getInputStream(), commandContext.getOutputStream(), commandContext.getErrorStream()); + } catch (ServiceNotEnabledException | ServiceNotAuthorizedException e) { + throw new IOException("error creating receive pack for ssh", e); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java index 819b92ca27..18bf1968d5 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/SshReceivePackFactory.java @@ -3,42 +3,19 @@ package sonia.scm.protocolcommand.git; import com.google.inject.Inject; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ReceivePack; -import org.eclipse.jgit.transport.resolver.ReceivePackFactory; import sonia.scm.protocolcommand.RepositoryContext; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.spi.HookEventFacade; -import sonia.scm.web.CollectingPackParserListener; -import sonia.scm.web.GitReceiveHook; -/** - * TODO we should have a single/abstract ReceivePackFactory for http and ssh. - */ -public class SshReceivePackFactory implements ReceivePackFactory { - - private final GitRepositoryHandler handler; - private final GitReceiveHook hook; +public class SshReceivePackFactory extends BaseReceivePackFactory { @Inject public SshReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { - this.handler = handler; - this.hook = new GitReceiveHook(hookEventFacade, handler); + super(handler, hookEventFacade); } @Override - public ReceivePack create(RepositoryContext repositoryContext, Repository repository) { - ReceivePack receivePack = new ReceivePack(repository); - receivePack.setAllowNonFastForwards(isNonFastForwardAllowed()); - - receivePack.setPreReceiveHook(hook); - receivePack.setPostReceiveHook(hook); - - // apply collecting listener, to be able to check which commits are new - CollectingPackParserListener.set(receivePack); - - return receivePack; - } - - private boolean isNonFastForwardAllowed() { - return ! handler.getConfig().isNonFastForwardDisallowed(); + protected ReceivePack createBasicReceivePack(RepositoryContext repositoryContext, Repository repository) { + return new ReceivePack(repository); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java index 5cb8007986..d6e7f398d4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java @@ -43,6 +43,7 @@ import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.resolver.ReceivePackFactory; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import sonia.scm.protocolcommand.git.BaseReceivePackFactory; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.spi.HookEventFacade; @@ -56,38 +57,21 @@ import javax.servlet.http.HttpServletRequest; * * @author Sebastian Sdorra */ -public class GitReceivePackFactory implements ReceivePackFactory +public class GitReceivePackFactory extends BaseReceivePackFactory { - private final GitRepositoryHandler handler; - - private ReceivePackFactory wrapped; - - private final GitReceiveHook hook; + private ReceivePackFactory wrapped; @Inject public GitReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { - this.handler = handler; - this.hook = new GitReceiveHook(hookEventFacade, handler); - this.wrapped = new DefaultReceivePackFactory(); + super(handler, hookEventFacade); + this.wrapped = new DefaultReceivePackFactory(); } @Override - public ReceivePack create(HttpServletRequest request, Repository repository) + protected ReceivePack createBasicReceivePack(HttpServletRequest request, Repository repository) throws ServiceNotEnabledException, ServiceNotAuthorizedException { - ReceivePack receivePack = wrapped.create(request, repository); - receivePack.setAllowNonFastForwards(isNonFastForwardAllowed()); - - receivePack.setPreReceiveHook(hook); - receivePack.setPostReceiveHook(hook); - // apply collecting listener, to be able to check which commits are new - CollectingPackParserListener.set(receivePack); - - return receivePack; - } - - private boolean isNonFastForwardAllowed() { - return ! handler.getConfig().isNonFastForwardDisallowed(); + return wrapped.create(request, repository); } @VisibleForTesting From ebb21ee3081a3fac3e2d3f5d7ed6bdb36623675e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 5 Mar 2019 13:47:46 +0100 Subject: [PATCH 06/29] Modify test to test base, not specific class This was necessary after extracting base class. --- .../sonia/scm/web/GitReceivePackFactory.java | 6 ---- .../git/BaseReceivePackFactoryTest.java} | 29 +++++++++++-------- 2 files changed, 17 insertions(+), 18 deletions(-) rename scm-plugins/scm-git-plugin/src/test/java/sonia/scm/{web/GitReceivePackFactoryTest.java => protocolcommand/git/BaseReceivePackFactoryTest.java} (80%) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java index d6e7f398d4..b59fa7526b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java @@ -35,7 +35,6 @@ package sonia.scm.web; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.annotations.VisibleForTesting; import com.google.inject.Inject; import org.eclipse.jgit.http.server.resolver.DefaultReceivePackFactory; import org.eclipse.jgit.lib.Repository; @@ -73,9 +72,4 @@ public class GitReceivePackFactory extends BaseReceivePackFactory wrappedReceivePackFactory; - private GitReceivePackFactory factory; + private BaseReceivePackFactory factory; - @Mock - private HttpServletRequest request; + private Object request = new Object(); private Repository repository; @@ -89,8 +89,12 @@ public class GitReceivePackFactoryTest { ReceivePack receivePack = new ReceivePack(repository); when(wrappedReceivePackFactory.create(request, repository)).thenReturn(receivePack); - factory = new GitReceivePackFactory(handler, null); - factory.setWrapped(wrappedReceivePackFactory); + factory = new BaseReceivePackFactory(handler, null) { + @Override + protected ReceivePack createBasicReceivePack(Object request, Repository repository) throws ServiceNotEnabledException, ServiceNotAuthorizedException { + return wrappedReceivePackFactory.create(request, repository); + } + }; } private Repository createRepositoryForTesting() throws GitAPIException, IOException { @@ -105,6 +109,7 @@ public class GitReceivePackFactoryTest { assertThat(receivePack.getPreReceiveHook(), instanceOf(GitReceiveHook.class)); assertThat(receivePack.getPostReceiveHook(), instanceOf(GitReceiveHook.class)); assertTrue(receivePack.isAllowNonFastForwards()); + verify(wrappedReceivePackFactory).create(request, repository); } @Test From dd71fb4c3b46a35dcf3f8a0ff7a239d87fd4afb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 5 Mar 2019 14:20:31 +0100 Subject: [PATCH 07/29] Expose ScmCommandProtocol as extension point This is necessary so that multiple providers (git, hg, ...) can implement this. The using class has to find the matching implementation. --- ...mSshProtocol.java => ScmCommandProtocol.java} | 7 ++++++- ...tSshProtocol.java => GitCommandProtocol.java} | 16 ++++++++++++---- .../protocolcommand/git/GitProtocolModule.java | 2 -- 3 files changed, 18 insertions(+), 7 deletions(-) rename scm-core/src/main/java/sonia/scm/protocolcommand/{ScmSshProtocol.java => ScmCommandProtocol.java} (51%) rename scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/{GitSshProtocol.java => GitCommandProtocol.java} (82%) diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java similarity index 51% rename from scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java rename to scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java index 0a016c3806..8def99c057 100644 --- a/scm-core/src/main/java/sonia/scm/protocolcommand/ScmSshProtocol.java +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java @@ -1,8 +1,13 @@ package sonia.scm.protocolcommand; +import sonia.scm.plugin.ExtensionPoint; + import java.io.IOException; -public interface ScmSshProtocol { +@ExtensionPoint +public interface ScmCommandProtocol { + + boolean canHandle(RepositoryContext repositoryContext); void handle(CommandContext context, RepositoryContext repositoryContext) throws IOException; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java similarity index 82% rename from scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java rename to scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java index df4ae99553..02e75c2015 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitSshProtocol.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java @@ -10,27 +10,35 @@ import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.util.FS; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.CommandContext; import sonia.scm.protocolcommand.RepositoryContext; -import sonia.scm.protocolcommand.ScmSshProtocol; +import sonia.scm.protocolcommand.ScmCommandProtocol; +import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.RepositoryPermissions; import javax.inject.Inject; import java.io.IOException; -public class GitSshProtocol implements ScmSshProtocol { +@Extension +public class GitCommandProtocol implements ScmCommandProtocol { - private static final Logger LOG = LoggerFactory.getLogger(GitSshProtocol.class); + private static final Logger LOG = LoggerFactory.getLogger(GitCommandProtocol.class); private SshUploadPackFactory uploadPackFactory; private SshReceivePackFactory receivePackFactory; @Inject - public GitSshProtocol(SshUploadPackFactory uploadPackFactory, SshReceivePackFactory receivePackFactory) { + public GitCommandProtocol(SshUploadPackFactory uploadPackFactory, SshReceivePackFactory receivePackFactory) { this.uploadPackFactory = uploadPackFactory; this.receivePackFactory = receivePackFactory; } + @Override + public boolean canHandle(RepositoryContext repositoryContext) { + return GitRepositoryHandler.TYPE_NAME.equals(repositoryContext.getRepository().getType()); + } + @Override public void handle(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { String subCommand = commandContext.getArgs()[0]; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java index 897f71ad4b..8301852408 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitProtocolModule.java @@ -3,13 +3,11 @@ package sonia.scm.protocolcommand.git; import com.google.inject.servlet.ServletModule; import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.RepositoryContextResolver; -import sonia.scm.protocolcommand.ScmSshProtocol; @Extension public class GitProtocolModule extends ServletModule { @Override protected void configureServlets() { bind(RepositoryContextResolver.class).to(GitRepositoryContextResolver.class); - bind(ScmSshProtocol.class).to(GitSshProtocol.class); } } From a16bb0d0af26dfc34258db9288b33546b78fdc3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 5 Mar 2019 15:01:57 +0100 Subject: [PATCH 08/29] Wrap parser and handler into interpreter --- .../scm/protocolcommand/CommandInterpreter.java | 13 +++++++++++++ .../sonia/scm/protocolcommand/CommandParser.java | 1 - .../scm/protocolcommand/ScmCommandProtocol.java | 5 ----- .../scm/protocolcommand/git/GitCommandProtocol.java | 6 ------ 4 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java new file mode 100644 index 0000000000..1e04eb3b66 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java @@ -0,0 +1,13 @@ +package sonia.scm.protocolcommand; + +import sonia.scm.plugin.ExtensionPoint; + +@ExtensionPoint +public interface CommandInterpreter { + + boolean canHandle(String command); + + CommandParser getParser(); + + ScmCommandProtocol getProtocolHandler(); +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java index 86fa9554e9..1d1f0c9b34 100644 --- a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandParser.java @@ -1,6 +1,5 @@ package sonia.scm.protocolcommand; -@FunctionalInterface public interface CommandParser { String[] parse(String command); diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java index 8def99c057..e7dbf65cad 100644 --- a/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java @@ -1,14 +1,9 @@ package sonia.scm.protocolcommand; -import sonia.scm.plugin.ExtensionPoint; - import java.io.IOException; -@ExtensionPoint public interface ScmCommandProtocol { - boolean canHandle(RepositoryContext repositoryContext); - void handle(CommandContext context, RepositoryContext repositoryContext) throws IOException; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java index 02e75c2015..b81e00a13c 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java @@ -14,7 +14,6 @@ import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.CommandContext; import sonia.scm.protocolcommand.RepositoryContext; import sonia.scm.protocolcommand.ScmCommandProtocol; -import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.RepositoryPermissions; import javax.inject.Inject; @@ -34,11 +33,6 @@ public class GitCommandProtocol implements ScmCommandProtocol { this.receivePackFactory = receivePackFactory; } - @Override - public boolean canHandle(RepositoryContext repositoryContext) { - return GitRepositoryHandler.TYPE_NAME.equals(repositoryContext.getRepository().getType()); - } - @Override public void handle(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { String subCommand = commandContext.getArgs()[0]; From 75fe4de4780c95c5d80d12033dc896245376ab4e Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 5 Mar 2019 16:31:41 +0100 Subject: [PATCH 09/29] added option to disable scrollToTop --- .../ui-components/src/buttons/SubmitButton.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/buttons/SubmitButton.js b/scm-ui-components/packages/ui-components/src/buttons/SubmitButton.js index 0f03d850b2..cca5b9eb8a 100644 --- a/scm-ui-components/packages/ui-components/src/buttons/SubmitButton.js +++ b/scm-ui-components/packages/ui-components/src/buttons/SubmitButton.js @@ -2,9 +2,17 @@ import React from "react"; import Button, { type ButtonProps } from "./Button"; -class SubmitButton extends React.Component { +type SubmitButtonProps = ButtonProps & { + scrollToTop: boolean +} + +class SubmitButton extends React.Component { + static defaultProps = { + scrollToTop: true + }; + render() { - const { action } = this.props; + const { action, scrollToTop } = this.props; return (