@@ -55,8 +55,7 @@ public class BranchDao implements Dao { | |||
} | |||
public Optional<BranchDto> selectByKey(DbSession dbSession, String projectUuid, BranchKeyType keyType, @Nullable String key) { | |||
String keyInDb = BranchDto.convertKeyToDb(key); | |||
return Optional.ofNullable(mapper(dbSession).selectByKey(projectUuid, keyType, keyInDb)); | |||
return Optional.ofNullable(mapper(dbSession).selectByKey(projectUuid, keyType, key)); | |||
} | |||
public Collection<BranchDto> selectByComponent(DbSession dbSession, ComponentDto component) { |
@@ -19,11 +19,9 @@ | |||
*/ | |||
package org.sonar.db.component; | |||
import javax.annotation.CheckForNull; | |||
import javax.annotation.Nullable; | |||
import static com.google.common.base.Preconditions.checkArgument; | |||
import static org.apache.commons.lang.StringUtils.repeat; | |||
public class BranchDto { | |||
public static final String DEFAULT_MAIN_BRANCH_NAME = "master"; | |||
@@ -33,14 +31,6 @@ public class BranchDto { | |||
*/ | |||
public static final int KEE_MAX_LENGTH = 255; | |||
/** | |||
* Value of {@link #kee} when the name of main branch is not known. | |||
* Used only if {@link #keeType} is {@link BranchKeyType#BRANCH}. | |||
* It does not conflict with names of real branches because the term ':BRANCH:' | |||
* is not accepted. | |||
*/ | |||
public static final String NULL_KEY = repeat("_", KEE_MAX_LENGTH); | |||
/** | |||
* Branch UUID is the projects.uuid that reference projects, branches or pull requests | |||
* (projects.qualifier="TRK"). | |||
@@ -64,7 +54,7 @@ public class BranchDto { | |||
/** | |||
* If {@link #keeType} is {@link BranchKeyType#BRANCH}, then name of branch, for example | |||
* "feature/foo". Can be {@link #NULL_KEY} is the name is not known. | |||
* "feature/foo". | |||
* | |||
* If {@link #keeType} is {@link BranchKeyType#PR}, then id of the pull request, for | |||
* example "1204". | |||
@@ -126,30 +116,26 @@ public class BranchDto { | |||
} | |||
/** | |||
* This is the getter used by MyBatis mapper. It does | |||
* not handle the special value used to map null field. | |||
* This is the getter used by MyBatis mapper. | |||
*/ | |||
private String getKee() { | |||
return kee; | |||
} | |||
@CheckForNull | |||
public String getKey() { | |||
return convertKeyFromDb(getKee()); | |||
return kee; | |||
} | |||
/** | |||
* This is the setter used by MyBatis mapper. It does | |||
* not handle the special value used to map null field. | |||
* This is the setter used by MyBatis mapper. | |||
*/ | |||
private void setKee(String s) { | |||
this.kee = s; | |||
} | |||
public BranchDto setKey(@Nullable String s) { | |||
checkArgument(s == null || s.length() <= KEE_MAX_LENGTH, "Maximum length of branch name or pull request id is %s: %s", KEE_MAX_LENGTH, s); | |||
checkArgument(!NULL_KEY.equals(s), "Branch name is not allowed: %s", s); | |||
setKee(convertKeyToDb(s)); | |||
public BranchDto setKey(String s) { | |||
checkArgument(s.length() <= KEE_MAX_LENGTH, "Maximum length of branch name or pull request id is %s: %s", KEE_MAX_LENGTH, s); | |||
setKee(s); | |||
return this; | |||
} | |||
@@ -197,14 +183,4 @@ public class BranchDto { | |||
sb.append('}'); | |||
return sb.toString(); | |||
} | |||
static String convertKeyToDb(@Nullable String s) { | |||
return s == null ? NULL_KEY : s; | |||
} | |||
@CheckForNull | |||
static String convertKeyFromDb(String s) { | |||
return NULL_KEY.equals(s) ? null : s; | |||
} | |||
} |
@@ -79,7 +79,7 @@ public class BranchDaoTest { | |||
dto.setUuid("U1"); | |||
dto.setBranchType(BranchType.LONG); | |||
dto.setKeeType(BranchKeyType.BRANCH); | |||
dto.setKey(null); | |||
dto.setKey("feature"); | |||
underTest.insert(dbSession, dto); | |||
BranchDto dto2 = new BranchDto(); | |||
@@ -120,22 +120,6 @@ public class BranchDaoTest { | |||
assertThat((String) map.get("pullRequestTitle")).contains("e").isEqualTo(dto.getPullRequestTitle()); | |||
} | |||
@Test | |||
public void null_value_of_kee_column_is_converted_in_db() { | |||
BranchDto dto = new BranchDto(); | |||
dto.setProjectUuid("u1"); | |||
dto.setUuid("u2"); | |||
dto.setKeeType(BranchKeyType.BRANCH); | |||
dto.setKey(null); | |||
underTest.insert(dbSession, dto); | |||
Map<String, Object> map = db.selectFirst(dbSession, SELECT_FROM + " where uuid='" + dto.getUuid() + "'"); | |||
assertThat(map).contains(entry("kee", BranchDto.NULL_KEY)); | |||
BranchDto loaded = underTest.selectByKey(dbSession, dto.getProjectUuid(), dto.getKeeType(), null).get(); | |||
assertThat(loaded.getKey()).isNull(); | |||
} | |||
@Test | |||
public void upsert() { | |||
BranchDto dto = new BranchDto(); |
@@ -60,7 +60,7 @@ public class ProjectConfigurationFactory { | |||
} | |||
Branch branch = branchOpt.get(); | |||
if (!branch.isLegacyFeature() && !branch.isMain()) { | |||
return branch.getName(); | |||
return Optional.of(branch.getName()); | |||
} | |||
return Optional.empty(); | |||
} |
@@ -38,10 +38,9 @@ public interface Branch extends ComponentKeyGenerator { | |||
boolean isLegacyFeature(); | |||
/** | |||
* Name can be empty when it's not known on the main branch | |||
* (regular analysis without the branch parameters) | |||
* Name of the branch | |||
*/ | |||
Optional<String> getName(); | |||
String getName(); | |||
/** | |||
* For short living branches, indicates the branch from which it was forked. |
@@ -158,7 +158,7 @@ public class PostProjectAnalysisTasksExecutor implements ComputationStepExecutor | |||
java.util.Optional<org.sonar.server.computation.task.projectanalysis.analysis.Branch> analysisBranchOpt = analysisMetadataHolder.getBranch(); | |||
if (analysisBranchOpt.isPresent() && !analysisBranchOpt.get().isLegacyFeature()) { | |||
org.sonar.server.computation.task.projectanalysis.analysis.Branch branch = analysisBranchOpt.get(); | |||
return new BranchImpl(branch.isMain(), branch.getName().orElse(null), Branch.Type.valueOf(branch.getType().name())); | |||
return new BranchImpl(branch.isMain(), branch.getName(), Branch.Type.valueOf(branch.getType().name())); | |||
} | |||
return null; | |||
} |
@@ -88,7 +88,7 @@ public class BranchPersister { | |||
// MainBranchProjectUuid will be null if it's a main branch | |||
dto.setProjectUuid(firstNonNull(componentDto.getMainBranchProjectUuid(), componentDto.projectUuid())); | |||
dto.setKeeType(BranchKeyType.BRANCH); | |||
dto.setKey(branch.getName().orElse(null)); | |||
dto.setKey(branch.getName()); | |||
dto.setBranchType(branch.getType()); | |||
// merge branch is only present if it's a short living branch | |||
dto.setMergeBranchUuid(branch.getMergeBranchUuid().orElse(null)); |
@@ -76,8 +76,8 @@ public class DefaultBranchImpl implements Branch { | |||
} | |||
@Override | |||
public Optional<String> getName() { | |||
return Optional.ofNullable(branchName); | |||
public String getName() { | |||
return branchName; | |||
} | |||
@Override |
@@ -107,7 +107,7 @@ public class BuildComponentTreeStep implements ComputationStep { | |||
return new DefaultBranchImpl(); | |||
} | |||
return branch.filter(Branch::isLegacyFeature) | |||
.map(b -> new DefaultBranchImpl(b.getName().orElse(null))) | |||
.map(b -> new DefaultBranchImpl(b.getName())) | |||
.orElseGet(DefaultBranchImpl::new); | |||
} | |||
@@ -132,7 +132,7 @@ public class QualityGateEventsStep implements ComputationStep { | |||
.setFieldValue("alertText", rawStatus.getText()) | |||
.setFieldValue("alertLevel", rawStatus.getStatus().toString()) | |||
.setFieldValue("isNewAlert", Boolean.toString(isNewAlert)); | |||
analysisMetadataHolder.getBranch().ifPresent(branch -> notification.setFieldValue("branch", branch.getName().orElse(null))); | |||
analysisMetadataHolder.getBranch().ifPresent(branch -> notification.setFieldValue("branch", branch.getName())); | |||
notificationService.deliver(notification); | |||
} | |||
@@ -145,7 +145,7 @@ public class SendIssueNotificationsStep implements ComputationStep { | |||
} | |||
private String getBranchName() { | |||
return analysisMetadataHolder.getBranch().filter(b -> !b.isMain()).flatMap(Branch::getName).orElse(null); | |||
return analysisMetadataHolder.getBranch().filter(b -> !b.isMain()).map(Branch::getName).orElse(null); | |||
} | |||
private String getMainBranchProjectKey() { |
@@ -84,7 +84,7 @@ public class ProjectConfigurationFactoryTest { | |||
ComponentDto branch = db.components().insertProjectBranch(project); | |||
db.properties().insertProperties(newComponentPropertyDto(branch).setKey("sonar.leak.period").setValue("1")); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch()))); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch(), false))); | |||
assertThat(config.get("sonar.leak.period")).hasValue("1"); | |||
} | |||
@@ -96,7 +96,7 @@ public class ProjectConfigurationFactoryTest { | |||
ComponentDto branch = db.components().insertProjectBranch(project); | |||
db.properties().insertProperties(newComponentPropertyDto(branch).setKey("sonar.leak.period").setValue("1")); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch()))); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch(), false))); | |||
assertThat(config.get("global")).hasValue("global_value"); | |||
assertThat(config.get("sonar.leak.period")).hasValue("1"); | |||
@@ -109,7 +109,7 @@ public class ProjectConfigurationFactoryTest { | |||
ComponentDto branch = db.components().insertProjectBranch(project); | |||
db.properties().insertProperties(newComponentPropertyDto(branch).setKey("sonar.leak.period").setValue("1")); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch()))); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch(), false))); | |||
assertThat(config.get("key")).hasValue("value"); | |||
assertThat(config.get("sonar.leak.period")).hasValue("1"); | |||
@@ -122,7 +122,7 @@ public class ProjectConfigurationFactoryTest { | |||
ComponentDto branch = db.components().insertProjectBranch(project); | |||
db.properties().insertProperties(newComponentPropertyDto(branch).setKey("sonar.leak.period").setValue("2")); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch()))); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getBranch(), false))); | |||
assertThat(config.get("sonar.leak.period")).hasValue("2"); | |||
} | |||
@@ -131,10 +131,10 @@ public class ProjectConfigurationFactoryTest { | |||
public void main_branch() { | |||
ComponentDto project = db.components().insertMainBranch(); | |||
db.properties().insertProperties(newComponentPropertyDto(project).setKey("sonar.leak.period").setValue("1")); | |||
Branch branch = createBranch(project.getBranch()); | |||
Branch branch = createBranch("master", true); | |||
when(branch.isMain()).thenReturn(true); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(project.getBranch()))); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getName(), true))); | |||
assertThat(config.get("sonar.leak.period")).hasValue("1"); | |||
} | |||
@@ -143,30 +143,18 @@ public class ProjectConfigurationFactoryTest { | |||
public void legacy_branch() { | |||
ComponentDto project = db.components().insertMainBranch(); | |||
db.properties().insertProperties(newComponentPropertyDto(project).setKey("sonar.leak.period").setValue("1")); | |||
Branch branch = createBranch(project.getBranch()); | |||
Branch branch = createBranch("legacy", true); | |||
when(branch.isLegacyFeature()).thenReturn(true); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(project.getBranch()))); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(branch.getName(), true))); | |||
assertThat(config.get("sonar.leak.period")).hasValue("1"); | |||
} | |||
@Test | |||
public void empty_branch_name() { | |||
ComponentDto project = db.components().insertMainBranch(); | |||
db.properties().insertProperties(newComponentPropertyDto(project).setKey("sonar.leak.period").setValue("1")); | |||
Branch branch = mock(Branch.class); | |||
when(branch.getName()).thenReturn(Optional.empty()); | |||
Configuration config = underTest.newProjectConfiguration(project.getKey(), Optional.of(createBranch(project.getBranch()))); | |||
assertThat(config.get("sonar.leak.period")).hasValue("1"); | |||
} | |||
private static Branch createBranch(String name) { | |||
private static Branch createBranch(String name, boolean isMain) { | |||
Branch branch = mock(Branch.class); | |||
when(branch.getName()).thenReturn(Optional.ofNullable(name)); | |||
when(branch.isMain()).thenReturn(false); | |||
when(branch.getName()).thenReturn(name); | |||
when(branch.isMain()).thenReturn(isMain); | |||
return branch; | |||
} | |||
} |
@@ -266,7 +266,7 @@ public class AnalysisMetadataHolderImplTest { | |||
underTest.setBranch(new DefaultBranchImpl("master")); | |||
assertThat(underTest.getBranch().get().getName()).hasValue("master"); | |||
assertThat(underTest.getBranch().get().getName()).isEqualTo("master"); | |||
} | |||
@Test |
@@ -254,8 +254,8 @@ public class PostProjectAnalysisTasksExecutorTest { | |||
} | |||
@Override | |||
public Optional<String> getName() { | |||
return Optional.of("feature/foo"); | |||
public String getName() { | |||
return "feature/foo"; | |||
} | |||
@Override |
@@ -62,7 +62,7 @@ public class BranchLoaderTest { | |||
Branch branch = metadataHolder.getBranch().get(); | |||
assertThat(branch.isMain()).isTrue(); | |||
assertThat(branch.getName().get()).isEqualTo(BranchDto.DEFAULT_MAIN_BRANCH_NAME); | |||
assertThat(branch.getName()).isEqualTo(BranchDto.DEFAULT_MAIN_BRANCH_NAME); | |||
} | |||
@Test | |||
@@ -77,7 +77,7 @@ public class BranchLoaderTest { | |||
Branch branch = metadataHolder.getBranch().get(); | |||
assertThat(branch.isMain()).isTrue(); | |||
assertThat(branch.getName()).hasValue("foo"); | |||
assertThat(branch.getName()).isEqualTo("foo"); | |||
} | |||
@Test | |||
@@ -93,7 +93,7 @@ public class BranchLoaderTest { | |||
Branch branch = metadataHolder.getBranch().get(); | |||
assertThat(branch.isMain()).isTrue(); | |||
assertThat(branch.getName()).hasValue("foo"); | |||
assertThat(branch.getName()).isEqualTo("foo"); | |||
} | |||
private class FakeDelegate implements BranchLoaderDelegate { |
@@ -19,14 +19,7 @@ | |||
*/ | |||
package org.sonar.server.computation.task.projectanalysis.component; | |||
import static org.assertj.core.api.Assertions.assertThat; | |||
import static org.mockito.Mockito.mock; | |||
import static org.mockito.Mockito.when; | |||
import static org.sonar.server.computation.task.projectanalysis.component.Component.Type.PROJECT; | |||
import static org.sonar.server.computation.task.projectanalysis.component.ReportComponent.builder; | |||
import java.util.Optional; | |||
import javax.annotation.Nullable; | |||
import org.junit.Rule; | |||
import org.junit.Test; | |||
import org.junit.rules.ExpectedException; | |||
@@ -38,8 +31,12 @@ import org.sonar.db.component.ComponentTesting; | |||
import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolderRule; | |||
import org.sonar.server.computation.task.projectanalysis.analysis.Branch; | |||
import org.sonar.server.computation.task.projectanalysis.analysis.Project; | |||
import org.sonar.server.computation.task.projectanalysis.component.Component; | |||
import org.sonar.server.computation.task.projectanalysis.component.TreeRootHolderRule; | |||
import static org.assertj.core.api.Assertions.assertThat; | |||
import static org.mockito.Mockito.mock; | |||
import static org.mockito.Mockito.when; | |||
import static org.sonar.server.computation.task.projectanalysis.component.Component.Type.PROJECT; | |||
import static org.sonar.server.computation.task.projectanalysis.component.ReportComponent.builder; | |||
public class BranchPersisterTest { | |||
private final static Component MAIN = builder(PROJECT, 1).setUuid("PROJECT_UUID").setKey("PROJECT_KEY").build(); | |||
@@ -58,7 +55,7 @@ public class BranchPersisterTest { | |||
@Test | |||
public void fail_if_no_component_for_main_branches() { | |||
analysisMetadataHolder.setBranch(createBranch(BranchType.LONG, true, null)); | |||
analysisMetadataHolder.setBranch(createBranch(BranchType.LONG, true, "master")); | |||
treeRootHolder.setRoot(MAIN); | |||
exception.expect(IllegalStateException.class); | |||
@@ -94,10 +91,10 @@ public class BranchPersisterTest { | |||
} | |||
private static Branch createBranch(BranchType type, boolean isMain, @Nullable String name) { | |||
private static Branch createBranch(BranchType type, boolean isMain, String name) { | |||
Branch branch = mock(Branch.class); | |||
when(branch.getType()).thenReturn(type); | |||
when(branch.getName()).thenReturn(Optional.ofNullable(name)); | |||
when(branch.getName()).thenReturn(name); | |||
when(branch.isMain()).thenReturn(isMain); | |||
when(branch.getMergeBranchUuid()).thenReturn(Optional.empty()); | |||
return branch; |
@@ -19,7 +19,6 @@ | |||
*/ | |||
package org.sonar.server.computation.task.projectanalysis.component; | |||
import java.util.Optional; | |||
import org.junit.Rule; | |||
import org.junit.Test; | |||
import org.sonar.api.config.Configuration; | |||
@@ -114,7 +113,7 @@ public class ConfigurationRepositoryTest { | |||
ComponentDto project = db.components().insertMainBranch(); | |||
ComponentDto branchDto = db.components().insertProjectBranch(project); | |||
Branch branch = mock(Branch.class); | |||
when(branch.getName()).thenReturn(Optional.of(branchDto.getBranch())); | |||
when(branch.getName()).thenReturn(branchDto.getBranch()); | |||
analysisMetadataHolder.setProject(Project.copyOf(project)).setBranch(branch); | |||
globalSettings.setProperty("global", "global value"); | |||
insertProjectProperty(project, "project", "project value"); |
@@ -56,7 +56,7 @@ public class DefaultBranchImplTest { | |||
assertThat(branch.isMain()).isTrue(); | |||
assertThat(branch.getType()).isEqualTo(BranchType.LONG); | |||
assertThat(branch.getName().get()).isEqualTo(BranchDto.DEFAULT_MAIN_BRANCH_NAME); | |||
assertThat(branch.getName()).isEqualTo(BranchDto.DEFAULT_MAIN_BRANCH_NAME); | |||
assertThat(branch.supportsCrossProjectCpd()).isTrue(); | |||
assertThat(branch.generateKey(PROJECT, null)).isEqualTo("P"); | |||
@@ -71,7 +71,7 @@ public class DefaultBranchImplTest { | |||
// not a real branch. Parameter sonar.branch forks project. | |||
assertThat(branch.isMain()).isTrue(); | |||
assertThat(branch.getType()).isEqualTo(BranchType.LONG); | |||
assertThat(branch.getName()).hasValue("bar"); | |||
assertThat(branch.getName()).isEqualTo("bar"); | |||
assertThat(branch.supportsCrossProjectCpd()).isFalse(); | |||
assertThat(branch.generateKey(PROJECT, null)).isEqualTo("P:bar"); | |||
@@ -81,7 +81,7 @@ public class DefaultBranchImplTest { | |||
private void assertThatNameIsCorrect(@Nullable String name) { | |||
DefaultBranchImpl branch = new DefaultBranchImpl(name); | |||
assertThat(branch.getName()).hasValue(name); | |||
assertThat(branch.getName()).isEqualTo(name); | |||
} | |||
private void assertThatNameIsNotCorrect(String name) { |
@@ -205,7 +205,7 @@ public class BuildComponentTreeStepTest { | |||
@Test | |||
public void generate_keys_when_using_branch() { | |||
Branch branch = mock(Branch.class); | |||
when(branch.getName()).thenReturn(Optional.of("origin/feature")); | |||
when(branch.getName()).thenReturn("origin/feature"); | |||
when(branch.isMain()).thenReturn(false); | |||
when(branch.isLegacyFeature()).thenReturn(false); | |||
when(branch.generateKey(any(ScannerReport.Component.class), any(ScannerReport.Component.class))).thenReturn("generated"); | |||
@@ -230,7 +230,7 @@ public class BuildComponentTreeStepTest { | |||
@Test | |||
public void generate_keys_when_using_main_branch() { | |||
Branch branch = mock(Branch.class); | |||
when(branch.getName()).thenReturn(Optional.of("origin/master")); | |||
when(branch.getName()).thenReturn("origin/master"); | |||
when(branch.isMain()).thenReturn(true); | |||
when(branch.isLegacyFeature()).thenReturn(false); | |||
when(branch.generateKey(any(ScannerReport.Component.class), any(ScannerReport.Component.class))).thenReturn("generated"); |
@@ -42,8 +42,8 @@ import org.sonar.server.computation.task.projectanalysis.analysis.Branch; | |||
import org.sonar.server.computation.task.projectanalysis.analysis.Project; | |||
import org.sonar.server.computation.task.projectanalysis.component.BranchPersister; | |||
import org.sonar.server.computation.task.projectanalysis.component.Component; | |||
import org.sonar.server.computation.task.projectanalysis.component.FileAttributes; | |||
import org.sonar.server.computation.task.projectanalysis.component.DefaultBranchImpl; | |||
import org.sonar.server.computation.task.projectanalysis.component.FileAttributes; | |||
import org.sonar.server.computation.task.projectanalysis.component.MutableDbIdsRepositoryRule; | |||
import org.sonar.server.computation.task.projectanalysis.component.MutableDisabledComponentsHolder; | |||
import org.sonar.server.computation.task.projectanalysis.component.ReportComponent; | |||
@@ -933,8 +933,8 @@ public class ReportPersistComponentsStepTest extends BaseStepTest { | |||
} | |||
@Override | |||
public java.util.Optional<String> getName() { | |||
return java.util.Optional.ofNullable(name); | |||
public String getName() { | |||
return name; | |||
} | |||
@Override |