From: Duarte Meneses Date: Wed, 19 Aug 2020 19:03:05 +0000 (-0500) Subject: SONAR-13791 Matching a scanner report to an existing project is not resilient X-Git-Tag: 8.5.0.37579~129 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0e9ee3d4acf0bcd3f22ebe2199119b95d7a2a6f6;p=sonarqube.git SONAR-13791 Matching a scanner report to an existing project is not resilient --- diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/BranchSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/BranchSupport.java index 02f375026e7..86dcd80d580 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/BranchSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/BranchSupport.java @@ -24,16 +24,13 @@ import java.util.Objects; import java.util.Optional; import javax.annotation.CheckForNull; import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; import org.sonar.api.server.ServerSide; import org.sonar.db.DbSession; import org.sonar.db.component.BranchDto; -import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; import org.sonar.db.organization.OrganizationDto; import static com.google.common.base.Preconditions.checkState; -import static java.util.Objects.requireNonNull; /** * Branch code for {@link ReportSubmitter}. @@ -78,12 +75,12 @@ public class BranchSupport { public abstract String getDbKey(); - public abstract Optional getBranch(); + public abstract Optional getBranchName(); public abstract Optional getPullRequestKey(); public final boolean isMainBranch() { - return !getBranch().isPresent() && !getPullRequestKey().isPresent(); + return !getBranchName().isPresent() && !getPullRequestKey().isPresent(); } /** @@ -92,49 +89,6 @@ public class BranchSupport { public abstract ComponentKey getMainBranchComponentKey(); } - @Immutable - public static final class Branch { - private final String name; - private final BranchType type; - - public Branch(String name, BranchType type) { - this.name = requireNonNull(name, "name can't be null"); - this.type = requireNonNull(type, "type can't be null"); - } - - public String getName() { - return name; - } - - public BranchType getType() { - return type; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - Branch branch = (Branch) o; - return name.equals(branch.name) && type == branch.type; - } - - @Override - public int hashCode() { - return Objects.hash(name, type); - } - - @Override - public String toString() { - return "Branch{" + - "name='" + name + '\'' + - ", type=" + type + - '}'; - } - } private static final class ComponentKeyImpl extends ComponentKey { private final String key; @@ -156,7 +110,7 @@ public class BranchSupport { } @Override - public Optional getBranch() { + public Optional getBranchName() { return Optional.empty(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/ReportSubmitter.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/ReportSubmitter.java index b853c99d508..f4d6b9268c3 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/ReportSubmitter.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/ReportSubmitter.java @@ -76,24 +76,51 @@ public class ReportSubmitter { * @throws NotFoundException if the organization with the specified key does not exist * @throws IllegalArgumentException if the organization with the specified key is not the organization of the specified project (when it already exists in DB) */ - public CeTask submit(String organizationKey, String projectKey, @Nullable String projectName, - Map characteristics, InputStream reportInput) { + public CeTask submit(String organizationKey, String projectKey, @Nullable String projectName, Map characteristics, InputStream reportInput) { try (DbSession dbSession = dbClient.openSession(false)) { + boolean projectCreated = false; OrganizationDto organizationDto = getOrganizationDtoOrFail(dbSession, organizationKey); + // Note: when the main branch is analyzed, the characteristics may or may not have the branch name, so componentKey#isMainBranch is not reliable! BranchSupport.ComponentKey componentKey = branchSupport.createComponentKey(projectKey, characteristics); - Optional existingComponent = dbClient.componentDao().selectByKey(dbSession, componentKey.getDbKey()); - ComponentDto component; - if (existingComponent.isPresent()) { - component = existingComponent.get(); - validateProject(dbSession, component, projectKey); - ensureOrganizationIsConsistent(component, organizationDto); + Optional mainBranchComponentOpt = dbClient.componentDao().selectByKey(dbSession, componentKey.getKey()); + ComponentDto mainBranchComponent; + + if (mainBranchComponentOpt.isPresent()) { + mainBranchComponent = mainBranchComponentOpt.get(); + validateProject(dbSession, mainBranchComponent, projectKey); + ensureOrganizationIsConsistent(mainBranchComponent, organizationDto); } else { - component = createComponent(dbSession, organizationDto, componentKey, projectName); + mainBranchComponent = createProject(dbSession, organizationDto, componentKey.getMainBranchComponentKey(), projectName); + projectCreated = true; } - checkScanPermission(component); - return submitReport(dbSession, reportInput, component, characteristics); + BranchDto mainBranch = dbClient.branchDao().selectByUuid(dbSession, mainBranchComponent.projectUuid()) + .orElseThrow(() -> new IllegalStateException("Couldn't find the main branch of the project")); + ComponentDto branchComponent; + if (isMainBranch(componentKey, mainBranch)) { + branchComponent = mainBranchComponent; + } else { + branchComponent = dbClient.componentDao().selectByKey(dbSession, componentKey.getDbKey()) + .orElseGet(() -> branchSupport.createBranchComponent(dbSession, componentKey, organizationDto, mainBranchComponent, mainBranch)); + } + + if (projectCreated) { + componentUpdater.commitAndIndex(dbSession, mainBranchComponent); + } else { + dbSession.commit(); + } + + checkScanPermission(branchComponent); + return submitReport(dbSession, reportInput, branchComponent, characteristics); + } + } + + private static boolean isMainBranch(BranchSupport.ComponentKey componentKey, BranchDto mainBranch) { + if (componentKey.isMainBranch()) { + return true; } + + return componentKey.getBranchName().isPresent() && componentKey.getBranchName().get().equals(mainBranch.getKey()); } private void checkScanPermission(ComponentDto project) { @@ -102,8 +129,7 @@ public class ReportSubmitter { // they don't have the direct permission on the project. // That means that dropping the permission on the project does not have any effects // if user has still the permission on the organization - if (!userSession.hasComponentPermission(UserRole.SCAN, project) && - !userSession.hasPermission(OrganizationPermission.SCAN, project.getOrganizationUuid())) { + if (!userSession.hasComponentPermission(UserRole.SCAN, project) && !userSession.hasPermission(OrganizationPermission.SCAN, project.getOrganizationUuid())) { throw insufficientPrivilegesException(); } } @@ -137,29 +163,7 @@ public class ReportSubmitter { project.getDbKey(), organizationDto.getKey()); } - private ComponentDto createComponent(DbSession dbSession, OrganizationDto organization, BranchSupport.ComponentKey componentKey, @Nullable String projectName) { - if (componentKey.isMainBranch()) { - ComponentDto project = createProject(dbSession, organization, componentKey, projectName); - componentUpdater.commitAndIndex(dbSession, project); - return project; - } - - Optional existingMainComponent = dbClient.componentDao().selectByKey(dbSession, componentKey.getKey()); - ComponentDto mainComponentDto = existingMainComponent - .orElseGet(() -> createProject(dbSession, organization, componentKey.getMainBranchComponentKey(), projectName)); - BranchDto mainComponentBranchDto = dbClient.branchDao().selectByUuid(dbSession, mainComponentDto.uuid()) - .orElseThrow(() -> new IllegalStateException("Branch of main component does not exist")); - ComponentDto branchComponent = branchSupport.createBranchComponent(dbSession, componentKey, organization, mainComponentDto, mainComponentBranchDto); - if (existingMainComponent.isPresent()) { - dbSession.commit(); - } else { - componentUpdater.commitAndIndex(dbSession, mainComponentDto); - } - return branchComponent; - } - - private ComponentDto createProject(DbSession dbSession, OrganizationDto organization, BranchSupport.ComponentKey componentKey, - @Nullable String projectName) { + private ComponentDto createProject(DbSession dbSession, OrganizationDto organization, BranchSupport.ComponentKey componentKey, @Nullable String projectName) { userSession.checkPermission(OrganizationPermission.PROVISION_PROJECTS, organization); String userUuid = userSession.getUuid(); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchReportSubmitterTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchReportSubmitterTest.java index 293b2b6d7b3..4c7fb6fcd8c 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchReportSubmitterTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchReportSubmitterTest.java @@ -66,8 +66,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.core.permission.GlobalPermissions.PROVISIONING; import static org.sonar.core.permission.GlobalPermissions.SCAN_EXECUTION; @@ -110,7 +110,7 @@ public class BranchReportSubmitterTest { underTest.submit(organization.getKey(), project.getDbKey(), project.name(), emptyMap(), reportInput); - verifyZeroInteractions(branchSupportDelegate); + verifyNoInteractions(branchSupportDelegate); } @Test @@ -129,8 +129,8 @@ public class BranchReportSubmitterTest { underTest.submit(organization.getKey(), project.getDbKey(), project.name(), randomCharacteristics, reportInput); - verifyZeroInteractions(permissionTemplateService); - verifyZeroInteractions(favoriteUpdater); + verifyNoInteractions(permissionTemplateService); + verifyNoInteractions(favoriteUpdater); verify(branchSupport, times(0)).createBranchComponent(any(), any(), any(), any(), any()); verify(branchSupportDelegate).createComponentKey(project.getDbKey(), randomCharacteristics); verify(branchSupportDelegate, times(0)).createBranchComponent(any(), any(), any(), any(), any()); @@ -157,8 +157,8 @@ public class BranchReportSubmitterTest { underTest.submit(organization.getKey(), existingProject.getDbKey(), existingProject.name(), randomCharacteristics, reportInput); - verifyZeroInteractions(permissionTemplateService); - verifyZeroInteractions(favoriteUpdater); + verifyNoInteractions(permissionTemplateService); + verifyNoInteractions(favoriteUpdater); verify(branchSupport).createBranchComponent(any(DbSession.class), same(componentKey), eq(organization), eq(existingProject), eq(exitingProjectMainBranch)); verify(branchSupportDelegate).createComponentKey(existingProject.getDbKey(), randomCharacteristics); verify(branchSupportDelegate).createBranchComponent(any(DbSession.class), same(componentKey), eq(organization), eq(existingProject), eq(exitingProjectMainBranch)); @@ -281,7 +281,7 @@ public class BranchReportSubmitterTest { when(mainComponentKey.getMainBranchComponentKey()).thenReturn(mainComponentKey); BranchSupport.ComponentKey componentKey = mockComponentKey(branch.getKey(), branch.getDbKey()); - when(componentKey.getBranch()).thenReturn(Optional.ofNullable(branch).map(b -> new BranchSupport.Branch(b.name(), BranchType.BRANCH))); + when(componentKey.getBranchName()).thenReturn(Optional.of(branch).map(ComponentDto::name)); when(componentKey.getMainBranchComponentKey()).thenReturn(mainComponentKey); return componentKey; diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchSupportTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchSupportTest.java index 550ad10ee32..e7738b76159 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchSupportTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchSupportTest.java @@ -64,7 +64,7 @@ public class BranchSupportTest { assertThat(componentKey.getKey()).isEqualTo(projectKey); assertThat(componentKey.getDbKey()).isEqualTo(projectKey); assertThat(componentKey.getMainBranchComponentKey()).isSameAs(componentKey); - assertThat(componentKey.getBranch()).isEmpty(); + assertThat(componentKey.getBranchName()).isEmpty(); assertThat(componentKey.getPullRequestKey()).isEmpty(); }