From 7f2c7ffda72acccb9c03f02042764c13c5caf73f Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 4 Sep 2019 10:17:57 +0200 Subject: [PATCH 1/4] redirect missing branch to default branch if available on sourcesView --- .../src/repos/sources/containers/Sources.js | 59 +++++++++++++++---- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/scm-ui/src/repos/sources/containers/Sources.js b/scm-ui/src/repos/sources/containers/Sources.js index 130326e8f4..be68e3c5c9 100644 --- a/scm-ui/src/repos/sources/containers/Sources.js +++ b/scm-ui/src/repos/sources/containers/Sources.js @@ -1,20 +1,25 @@ // @flow import React from "react"; -import {connect} from "react-redux"; -import {withRouter} from "react-router-dom"; -import type {Branch, Repository} from "@scm-manager/ui-types"; +import { connect } from "react-redux"; +import { withRouter } from "react-router-dom"; +import type { Branch, Repository } from "@scm-manager/ui-types"; import FileTree from "../components/FileTree"; -import {BranchSelector, Breadcrumb, ErrorNotification, Loading} from "@scm-manager/ui-components"; -import {translate} from "react-i18next"; +import { + BranchSelector, + Breadcrumb, + ErrorNotification, + Loading +} from "@scm-manager/ui-components"; +import { translate } from "react-i18next"; import { fetchBranches, getBranches, getFetchBranchesFailure, isFetchBranchesPending } from "../../branches/modules/branches"; -import {compose} from "redux"; +import { compose } from "redux"; import Content from "./Content"; -import {fetchSources, isDirectory} from "../modules/sources"; +import { fetchSources, isDirectory } from "../modules/sources"; type Props = { repository: Repository, @@ -33,6 +38,7 @@ type Props = { // Context props history: any, match: any, + location: any, t: string => string }; @@ -55,31 +61,62 @@ class Sources extends React.Component { repository, revision, path, + branches, + baseUrl, fetchSources } = this.props; fetchBranches(repository); fetchSources(repository, revision, path); + + if (branches) { + const defaultBranches = branches.filter(b => b.defaultBranch); + + if (defaultBranches.length > 0) + this.setState({ selectedBranch: defaultBranches[0] }); + this.props.history.push(`${baseUrl}/${defaultBranches[0].name}/`); + } } + componentDidUpdate(prevProps) { - const { fetchSources, repository, revision, path } = this.props; + const { + fetchSources, + repository, + revision, + path, + branches, + baseUrl + } = this.props; if (prevProps.revision !== revision || prevProps.path !== path) { fetchSources(repository, revision, path); } + + const currentUrl = this.props.location.pathname; + + if ( + branches && + (currentUrl.endsWith("sources") || currentUrl.endsWith("sources/")) + ) { + const defaultBranches = branches.filter(b => b.defaultBranch); + + if (defaultBranches.length > 0) + this.setState({ selectedBranch: defaultBranches[0] }); + this.props.history.push(`${baseUrl}/${defaultBranches[0].name}/`); + } } branchSelected = (branch?: Branch) => { const { baseUrl, history, path } = this.props; let url; if (branch) { - this.setState({selectedBranch: branch}); + this.setState({ selectedBranch: branch }); if (path) { url = `${baseUrl}/${encodeURIComponent(branch.name)}/${path}`; } else { url = `${baseUrl}/${encodeURIComponent(branch.name)}/`; } } else { - this.setState({selectedBranch: null}); + this.setState({ selectedBranch: null }); url = `${baseUrl}/`; } history.push(url); @@ -97,7 +134,7 @@ class Sources extends React.Component { currentFileIsDirectory } = this.props; - const {selectedBranch} = this.state; + const { selectedBranch } = this.state; if (error) { return ; From 790910166f503e93a4028892788dc03f1a78b060 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 4 Sep 2019 11:52:38 +0200 Subject: [PATCH 2/4] check if currentUrl is branchUrl for extensionPoint --- scm-ui-components/packages/ui-components/src/Breadcrumb.js | 5 +++-- scm-ui/src/repos/sources/containers/Sources.js | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/Breadcrumb.js b/scm-ui-components/packages/ui-components/src/Breadcrumb.js index eae11c248c..d81a4cbf63 100644 --- a/scm-ui-components/packages/ui-components/src/Breadcrumb.js +++ b/scm-ui-components/packages/ui-components/src/Breadcrumb.js @@ -10,6 +10,7 @@ import classNames from "classnames"; type Props = { branch: Branch, defaultBranch: Branch, + branches: Branch[], revision: string, path: string, baseUrl: string, @@ -62,7 +63,7 @@ class Breadcrumb extends React.Component { } render() { - const { classes, baseUrl, branch, defaultBranch, path } = this.props; + const { classes, baseUrl, branch, defaultBranch, branches, revision, path } = this.props; return ( <> @@ -76,7 +77,7 @@ class Breadcrumb extends React.Component { b.name === revision).length > 0 }} renderAll={true} /> diff --git a/scm-ui/src/repos/sources/containers/Sources.js b/scm-ui/src/repos/sources/containers/Sources.js index be68e3c5c9..9a485459a6 100644 --- a/scm-ui/src/repos/sources/containers/Sources.js +++ b/scm-ui/src/repos/sources/containers/Sources.js @@ -156,6 +156,7 @@ class Sources extends React.Component { defaultBranch={ branches && branches.filter(b => b.defaultBranch === true)[0] } + branches={branches && branches} /> Date: Wed, 4 Sep 2019 13:05:22 +0200 Subject: [PATCH 3/4] Add requested revision to browse result --- .../java/sonia/scm/repository/BrowserResult.java | 10 ++++++++++ .../sonia/scm/repository/spi/GitBrowseCommand.java | 4 ++-- .../BrowserResultToFileObjectDtoMapper.java | 2 +- .../resources/FileObjectToFileObjectDtoMapper.java | 13 +++++++------ .../FileObjectToFileObjectDtoMapperTest.java | 9 +++++---- 5 files changed, 25 insertions(+), 13 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/BrowserResult.java b/scm-core/src/main/java/sonia/scm/repository/BrowserResult.java index 17c447eb87..44c6b963ad 100644 --- a/scm-core/src/main/java/sonia/scm/repository/BrowserResult.java +++ b/scm-core/src/main/java/sonia/scm/repository/BrowserResult.java @@ -53,13 +53,19 @@ import java.io.Serializable; public class BrowserResult implements Serializable { private String revision; + private String requestedRevision; private FileObject file; public BrowserResult() { } public BrowserResult(String revision, FileObject file) { + this(revision, revision, file); + } + + public BrowserResult(String revision, String requestedRevision, FileObject file) { this.revision = revision; + this.requestedRevision = requestedRevision; this.file = file; } @@ -67,6 +73,10 @@ public class BrowserResult implements Serializable { return revision; } + public String getRequestedRevision() { + return requestedRevision; + } + public FileObject getFile() { return file; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java index 93fb01176b..2a254d96ce 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBrowseCommand.java @@ -124,7 +124,7 @@ public class GitBrowseCommand extends AbstractGitCommand if (revId != null) { - result = new BrowserResult(revId.getName(), getEntry(repo, request, revId)); + result = new BrowserResult(revId.getName(), request.getRevision(), getEntry(repo, request, revId)); } else { @@ -138,7 +138,7 @@ public class GitBrowseCommand extends AbstractGitCommand logger.warn("could not find head of repository, empty?"); } - result = new BrowserResult(Constants.HEAD, createEmtpyRoot()); + result = new BrowserResult(Constants.HEAD, request.getRevision(), createEmtpyRoot()); } return result; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultToFileObjectDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultToFileObjectDtoMapper.java index 9720fb5b1a..588a8b3b2f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultToFileObjectDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BrowserResultToFileObjectDtoMapper.java @@ -15,7 +15,7 @@ public class BrowserResultToFileObjectDtoMapper { } public FileObjectDto map(BrowserResult browserResult, NamespaceAndName namespaceAndName) { - FileObjectDto fileObjectDto = fileObjectToFileObjectDtoMapper.map(browserResult.getFile(), namespaceAndName, browserResult.getRevision()); + FileObjectDto fileObjectDto = fileObjectToFileObjectDtoMapper.map(browserResult.getFile(), namespaceAndName, browserResult); fileObjectDto.setRevision( browserResult.getRevision() ); return fileObjectDto; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapper.java index 608dea9f26..42da3e90c8 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapper.java @@ -6,6 +6,7 @@ import org.mapstruct.Context; import org.mapstruct.Mapper; import org.mapstruct.Mapping; import org.mapstruct.ObjectFactory; +import sonia.scm.repository.BrowserResult; import sonia.scm.repository.FileObject; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.SubRepository; @@ -22,23 +23,23 @@ public abstract class FileObjectToFileObjectDtoMapper extends HalAppenderMapper private ResourceLinks resourceLinks; @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes - protected abstract FileObjectDto map(FileObject fileObject, @Context NamespaceAndName namespaceAndName, @Context String revision); + protected abstract FileObjectDto map(FileObject fileObject, @Context NamespaceAndName namespaceAndName, @Context BrowserResult browserResult); abstract SubRepositoryDto mapSubrepository(SubRepository subRepository); @ObjectFactory - FileObjectDto createDto(@Context NamespaceAndName namespaceAndName, @Context String revision, FileObject fileObject) { + FileObjectDto createDto(@Context NamespaceAndName namespaceAndName, @Context BrowserResult browserResult, FileObject fileObject) { String path = removeFirstSlash(fileObject.getPath()); Links.Builder links = Links.linkingTo(); if (fileObject.isDirectory()) { - links.self(resourceLinks.source().sourceWithPath(namespaceAndName.getNamespace(), namespaceAndName.getName(), revision, path)); + links.self(resourceLinks.source().sourceWithPath(namespaceAndName.getNamespace(), namespaceAndName.getName(), browserResult.getRevision(), path)); } else { - links.self(resourceLinks.source().content(namespaceAndName.getNamespace(), namespaceAndName.getName(), revision, path)); - links.single(link("history", resourceLinks.fileHistory().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), revision, path))); + links.self(resourceLinks.source().content(namespaceAndName.getNamespace(), namespaceAndName.getName(), browserResult.getRevision(), path)); + links.single(link("history", resourceLinks.fileHistory().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), browserResult.getRevision(), path))); } Embedded.Builder embeddedBuilder = embeddedBuilder(); - applyEnrichers(new EdisonHalAppender(links, embeddedBuilder), fileObject, namespaceAndName, revision); + applyEnrichers(new EdisonHalAppender(links, embeddedBuilder), fileObject, namespaceAndName, browserResult, browserResult.getRevision()); return new FileObjectDto(links.build(), embeddedBuilder.build()); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapperTest.java index 55058a1684..de357848c5 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/FileObjectToFileObjectDtoMapperTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.repository.BrowserResult; import sonia.scm.repository.FileObject; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.SubRepository; @@ -49,7 +50,7 @@ public class FileObjectToFileObjectDtoMapperTest { @Test public void shouldMapAttributesCorrectly() { FileObject fileObject = createFileObject(); - FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("namespace", "name"), "revision"); + FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("namespace", "name"), new BrowserResult("revision", fileObject)); assertEqualAttributes(fileObject, dto); } @@ -57,7 +58,7 @@ public class FileObjectToFileObjectDtoMapperTest { @Test public void shouldHaveCorrectSelfLinkForDirectory() { FileObject fileObject = createDirectoryObject(); - FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("namespace", "name"), "revision"); + FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("namespace", "name"), new BrowserResult("revision", fileObject)); assertThat(dto.getLinks().getLinkBy("self").get().getHref()).isEqualTo(expectedBaseUri.resolve("namespace/name/sources/revision/foo/bar").toString()); } @@ -66,7 +67,7 @@ public class FileObjectToFileObjectDtoMapperTest { public void shouldHaveCorrectContentLink() { FileObject fileObject = createFileObject(); fileObject.setDirectory(false); - FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("namespace", "name"), "revision"); + FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("namespace", "name"), new BrowserResult("revision", fileObject)); assertThat(dto.getLinks().getLinkBy("self").get().getHref()).isEqualTo(expectedBaseUri.resolve("namespace/name/content/revision/foo/bar").toString()); } @@ -84,7 +85,7 @@ public class FileObjectToFileObjectDtoMapperTest { mapper.setRegistry(registry); FileObject fileObject = createFileObject(); - FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("hitchhiker", "hog"), "42"); + FileObjectDto dto = mapper.map(fileObject, new NamespaceAndName("hitchhiker", "hog"), new BrowserResult("42", fileObject)); assertThat(dto.getLinks().getLinkBy("hog").get().getHref()).isEqualTo("http://hitchhiker/hog/foo/42"); } From 5f037762c7d1bff1f4e604ef11a20404b420f2e6 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Wed, 4 Sep 2019 13:24:59 +0200 Subject: [PATCH 4/4] Verify that branch is a branch --- .../scm/repository/spi/GitModifyCommand.java | 7 +++++++ .../scm/repository/spi/GitModifyCommandTest.java | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 4ff60445f7..9aa3eebe27 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -3,10 +3,13 @@ package sonia.scm.repository.spi; import org.apache.commons.lang.StringUtils; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; import sonia.scm.BadRequestException; import sonia.scm.ConcurrentModificationException; import sonia.scm.ContextEntry; +import sonia.scm.ScmConstraintViolationException; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; @@ -21,6 +24,7 @@ import java.util.Optional; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static sonia.scm.AlreadyExistsException.alreadyExists; import static sonia.scm.ContextEntry.ContextBuilder.entity; +import static sonia.scm.ScmConstraintViolationException.Builder.doThrow; public class GitModifyCommand extends AbstractGitCommand implements ModifyCommand { @@ -52,6 +56,9 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman if (!StringUtils.isEmpty(request.getBranch())) { checkOutBranch(request.getBranch()); } + Ref head = getClone().getRepository().exactRef(Constants.HEAD); + doThrow().violation("branch has to be a valid branch, no revision", "branch", request.getBranch()).when(head == null || !head.isSymbolic()); + getClone().getRepository().getFullBranch(); if (!StringUtils.isEmpty(request.getExpectedRevision())) { if (!request.getExpectedRevision().equals(getCurrentRevision().getName())) { throw new ConcurrentModificationException("branch", request.getBranch() == null? "default": request.getBranch()); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java index 96cc256cf4..8d9cc5bc5a 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -16,6 +16,7 @@ import org.junit.rules.TemporaryFolder; import sonia.scm.AlreadyExistsException; import sonia.scm.BadRequestException; import sonia.scm.ConcurrentModificationException; +import sonia.scm.ScmConstraintViolationException; import sonia.scm.repository.Person; import sonia.scm.repository.util.WorkdirProvider; @@ -156,6 +157,21 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { command.execute(request); } + @Test(expected = ScmConstraintViolationException.class) + public void shouldFailWithConstraintViolationIfBranchIsNoBranch() throws IOException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "irrelevant\n".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setCommitMessage("test commit"); + request.setBranch("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("irrelevant", newFile, true)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + command.execute(request); + } + private void assertInTree(TreeAssertions assertions) throws IOException, GitAPIException { try (Git git = new Git(createContext().open())) { RevCommit lastCommit = getLastCommit(git);