Add handling when duplicated branch part cannot be created (#1692)

Add handling when duplicated branch cannot be created because a part of the name already exists as a branch
This commit is contained in:
Florian Scholdei
2021-06-09 14:58:59 +02:00
committed by GitHub
parent 35fe536170
commit f274b7f4b2
8 changed files with 112 additions and 9 deletions

View File

@@ -0,0 +1,2 @@
- type: fixed
description: Add handling when duplicated branch part cannot be created ([#1692](https://github.com/scm-manager/scm-manager/pull/1692))

View File

@@ -0,0 +1,53 @@
/*
* 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.
*/
package sonia.scm;
import java.util.List;
import static java.util.stream.Collectors.joining;
public class BranchAlreadyExistsException extends ExceptionWithContext {
private static final String CODE = "FgSZpu9FR1";
public static BranchAlreadyExistsException branchAlreadyExists(ContextEntry.ContextBuilder builder) {
return new BranchAlreadyExistsException(builder.build());
}
private BranchAlreadyExistsException(List<ContextEntry> context) {
super(context, createMessage(context));
}
@Override
public String getCode() {
return CODE;
}
private static String createMessage(List<ContextEntry> context) {
return context.stream()
.map(c -> c.getType().toLowerCase() + " with id " + c.getId())
.collect(joining(" in ", "", " already exists"));
}
}

View File

@@ -37,6 +37,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import sonia.scm.AlreadyExistsException; import sonia.scm.AlreadyExistsException;
import sonia.scm.BadRequestException; import sonia.scm.BadRequestException;
import sonia.scm.BranchAlreadyExistsException;
import sonia.scm.ConcurrentModificationException; import sonia.scm.ConcurrentModificationException;
import sonia.scm.NotFoundException; import sonia.scm.NotFoundException;
import sonia.scm.ScmConstraintViolationException; import sonia.scm.ScmConstraintViolationException;
@@ -91,6 +92,7 @@ public class RestDispatcher {
public EnhanceableExceptionMapper() { public EnhanceableExceptionMapper() {
registerException(NotFoundException.class, Status.NOT_FOUND); registerException(NotFoundException.class, Status.NOT_FOUND);
registerException(AlreadyExistsException.class, Status.CONFLICT); registerException(AlreadyExistsException.class, Status.CONFLICT);
registerException(BranchAlreadyExistsException.class, Status.CONFLICT);
registerException(ConcurrentModificationException.class, Status.CONFLICT); registerException(ConcurrentModificationException.class, Status.CONFLICT);
registerException(UnauthorizedException.class, Status.FORBIDDEN); registerException(UnauthorizedException.class, Status.FORBIDDEN);
registerException(AuthorizationException.class, Status.FORBIDDEN); registerException(AuthorizationException.class, Status.FORBIDDEN);

View File

@@ -57,10 +57,6 @@ const CreateBranch: FC<Props> = ({ repository }) => {
return <ErrorNotification error={errorList} />; return <ErrorNotification error={errorList} />;
} }
if (errorCreate) {
return <ErrorNotification error={errorCreate} />;
}
if (isLoadingList || !branches) { if (isLoadingList || !branches) {
return <Loading />; return <Loading />;
} }
@@ -68,6 +64,7 @@ const CreateBranch: FC<Props> = ({ repository }) => {
return ( return (
<> <>
<Subtitle subtitle={t("branches.create.title")} /> <Subtitle subtitle={t("branches.create.title")} />
{errorCreate ? <ErrorNotification error={errorCreate} /> : null}
<BranchForm <BranchForm
submitForm={create} submitForm={create}
loading={isLoadingCreate} loading={isLoadingCreate}

View File

@@ -62,7 +62,7 @@ import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.util.Optional; import java.util.Optional;
import static sonia.scm.AlreadyExistsException.alreadyExists; import static sonia.scm.BranchAlreadyExistsException.branchAlreadyExists;
import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.ContextEntry.ContextBuilder.entity;
import static sonia.scm.NotFoundException.notFound; import static sonia.scm.NotFoundException.notFound;
@@ -254,7 +254,7 @@ public class BranchRootResource {
String parentName = branchRequest.getParent(); String parentName = branchRequest.getParent();
try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) { try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
if (branchExists(branchName, repositoryService)) { if (branchExists(branchName, repositoryService)) {
throw alreadyExists(entity(Branch.class, branchName).in(Repository.class, namespace + "/" + name)); throw branchAlreadyExists(entity(Branch.class, branchName).in(Repository.class, namespace + "/" + name));
} }
Repository repository = repositoryService.getRepository(); Repository repository = repositoryService.getRepository();
RepositoryPermissions.push(repository).check(); RepositoryPermissions.push(repository).check();
@@ -275,7 +275,16 @@ public class BranchRootResource {
.getBranches() .getBranches()
.getBranches() .getBranches()
.stream() .stream()
.anyMatch(branch -> branchName.equals(branch.getName())); .anyMatch(branch -> branchName.equals(branch.getName())
|| branchNamespaceAlreadyExistsInGitRepo(branchName, branch.getName(), repositoryService));
}
private boolean branchNamespaceAlreadyExistsInGitRepo(String branchName, String branchName2, RepositoryService repositoryService) {
if (repositoryService.getRepository().getType().equals("hg")) {
return false;
}
return (branchName.contains("/") && branchName2.equals(branchName.substring(0, branchName.indexOf("/"))))
|| (branchName2.contains("/") && branchName.equals(branchName2.substring(0, branchName2.indexOf("/"))));
} }
/** /**

View File

@@ -193,11 +193,15 @@
}, },
"FtR7UznKU1": { "FtR7UznKU1": {
"displayName": "Existiert bereits", "displayName": "Existiert bereits",
"description": "Ein Datensatz mit den gegebenen Schlüsselwerten existiert bereits" "description": "Ein Datensatz mit den gegebenen Schlüsselwerten existiert bereits."
},
"FgSZpu9FR1": {
"displayName": "Branch existiert bereits",
"description": "Ein Branch oder Branchverzeichnis mit den gegebenen Schlüsselwerten existiert bereits."
}, },
"9BR7qpDAe1": { "9BR7qpDAe1": {
"displayName": "Passwortänderung nicht erlaubt", "displayName": "Passwortänderung nicht erlaubt",
"description": "Sie haben nicht die Berechtigung, das Passwort zu ändern" "description": "Sie haben nicht die Berechtigung, das Passwort zu ändern."
}, },
"2wR7UzpPG1": { "2wR7UzpPG1": {
"displayName": "Konkurrierende Änderungen", "displayName": "Konkurrierende Änderungen",

View File

@@ -195,6 +195,10 @@
"displayName": "Already exists", "displayName": "Already exists",
"description": "There is already an entity with the same key values." "description": "There is already an entity with the same key values."
}, },
"FgSZpu9FR1": {
"displayName": "Branch already exists",
"description": "There is already an branch or branch directory with the same key values."
},
"9BR7qpDAe1": { "9BR7qpDAe1": {
"displayName": "Password change not allowed", "displayName": "Password change not allowed",
"description": "You do not have the permission to change the password." "description": "You do not have the permission to change the password."

View File

@@ -257,6 +257,38 @@ public class BranchRootResourceTest extends RepositoryTestBase {
verify(branchCommandBuilder, never()).branch(anyString()); verify(branchCommandBuilder, never()).branch(anyString());
} }
@Test
public void shouldNotCreateBranchIfDirectoryAsBranchAlreadyExists() throws Exception {
when(branchesCommandBuilder.getBranches()).thenReturn(new Branches(createBranch("existing_branch")));
MockHttpRequest request = MockHttpRequest
.post("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo/branches/")
.content("{\"name\": \"existing_branch/abc\"}".getBytes())
.contentType(VndMediaType.BRANCH_REQUEST);
MockHttpResponse response = new MockHttpResponse();
dispatcher.invoke(request, response);
assertEquals(409, response.getStatus());
verify(branchCommandBuilder, never()).branch(anyString());
}
@Test
public void shouldNotCreateBranchIfBranchWithDirectoryAlreadyExists() throws Exception {
when(branchesCommandBuilder.getBranches()).thenReturn(new Branches(createBranch("existing_branch/abc")));
MockHttpRequest request = MockHttpRequest
.post("/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + "space/repo/branches/")
.content("{\"name\": \"existing_branch\"}".getBytes())
.contentType(VndMediaType.BRANCH_REQUEST);
MockHttpResponse response = new MockHttpResponse();
dispatcher.invoke(request, response);
assertEquals(409, response.getStatus());
verify(branchCommandBuilder, never()).branch(anyString());
}
@Test @Test
public void shouldFailForMissingParentBranch() throws Exception { public void shouldFailForMissingParentBranch() throws Exception {
when(branchesCommandBuilder.getBranches()).thenReturn(new Branches(createBranch("existing_branch"))); when(branchesCommandBuilder.getBranches()).thenReturn(new Branches(createBranch("existing_branch")));