]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-13791 Matching a scanner report to an existing project is not resilient
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Wed, 19 Aug 2020 19:03:05 +0000 (14:03 -0500)
committersonartech <sonartech@sonarsource.com>
Mon, 24 Aug 2020 20:06:06 +0000 (20:06 +0000)
server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/BranchSupport.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/ce/queue/ReportSubmitter.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchReportSubmitterTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/ce/queue/BranchSupportTest.java

index 02f375026e74c47254f3bff3532545910876c7fc..86dcd80d5801b024589b6631c3725ffa559042f5 100644 (file)
@@ -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<Branch> getBranch();
+    public abstract Optional<String> getBranchName();
 
     public abstract Optional<String> 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<Branch> getBranch() {
+    public Optional<String> getBranchName() {
       return Optional.empty();
     }
 
index b853c99d508eb6c6e8ba661d32c3f9328daf3fe1..f4d6b9268c3284b3b86fca627d7ec1bdf2fc17ea 100644 (file)
@@ -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<String, String> characteristics, InputStream reportInput) {
+  public CeTask submit(String organizationKey, String projectKey, @Nullable String projectName, Map<String, String> 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<ComponentDto> existingComponent = dbClient.componentDao().selectByKey(dbSession, componentKey.getDbKey());
-      ComponentDto component;
-      if (existingComponent.isPresent()) {
-        component = existingComponent.get();
-        validateProject(dbSession, component, projectKey);
-        ensureOrganizationIsConsistent(component, organizationDto);
+      Optional<ComponentDto> 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<ComponentDto> 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();
 
index 293b2b6d7b3a2722620d64d35c2381ec9d896666..4c7fb6fcd8cc01b96fb3d9cc777fdc0d85ffcbdd 100644 (file)
@@ -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;
index 550ad10ee32ec345a3a15d444a9ff262348c676e..e7738b76159932f812fb91b5aded17ff1aa4ec00 100644 (file)
@@ -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();
   }