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-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); 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"); }