Skip to content

Commit

Permalink
SONAR-6260 Apply PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
julienlancelot committed Jun 10, 2015
1 parent f0966d3 commit 091ec85
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 54 deletions.
Expand Up @@ -25,8 +25,8 @@
import javax.annotation.Nullable; import javax.annotation.Nullable;
import org.sonar.api.resources.Scopes; import org.sonar.api.resources.Scopes;
import org.sonar.core.component.SnapshotDto; import org.sonar.core.component.SnapshotDto;
import org.sonar.core.component.SnapshotQuery;
import org.sonar.core.component.db.SnapshotMapper; import org.sonar.core.component.db.SnapshotMapper;
import org.sonar.core.component.db.SnapshotQuery;
import org.sonar.core.persistence.DaoComponent; import org.sonar.core.persistence.DaoComponent;
import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbSession;
import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.NotFoundException;
Expand Down
Expand Up @@ -32,6 +32,11 @@
*/ */
public interface PeriodsHolder { public interface PeriodsHolder {


/**
* Return the list of differential periods.
*
* @throws IllegalStateException if the periods haven't been initialized
*/
List<Period> getPeriods(); List<Period> getPeriods();


} }
Expand Up @@ -21,22 +21,24 @@
package org.sonar.server.computation.period; package org.sonar.server.computation.period;


import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import java.util.ArrayList; import com.google.common.collect.ImmutableList;
import java.util.List; import java.util.List;
import javax.annotation.CheckForNull;


public class PeriodsHolderImpl implements PeriodsHolder { public class PeriodsHolderImpl implements PeriodsHolder {


private boolean isPeriodsInitialized = false; @CheckForNull
private List<Period> periods = new ArrayList<>(); private ImmutableList<Period> periods;


public void setPeriods(List<Period> periods) { public void setPeriods(List<Period> periods) {
this.periods = periods; Preconditions.checkNotNull(periods, "Periods cannot be null");
isPeriodsInitialized = true; Preconditions.checkState(this.periods == null, "Periods have already been initialized");
this.periods = ImmutableList.copyOf(periods);
} }


@Override @Override
public List<Period> getPeriods() { public List<Period> getPeriods() {
Preconditions.checkArgument(isPeriodsInitialized, "Periods have not been initialized yet"); Preconditions.checkState(periods != null, "Periods have not been initialized yet");
return periods; return periods;
} }


Expand Down
Expand Up @@ -37,7 +37,7 @@
import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Loggers;
import org.sonar.core.component.ComponentDto; import org.sonar.core.component.ComponentDto;
import org.sonar.core.component.SnapshotDto; import org.sonar.core.component.SnapshotDto;
import org.sonar.core.component.SnapshotQuery; import org.sonar.core.component.db.SnapshotQuery;
import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbSession;
import org.sonar.server.computation.batch.BatchReportReader; import org.sonar.server.computation.batch.BatchReportReader;
import org.sonar.server.computation.component.Component; import org.sonar.server.computation.component.Component;
Expand All @@ -46,9 +46,9 @@
import org.sonar.server.computation.period.PeriodsHolderImpl; import org.sonar.server.computation.period.PeriodsHolderImpl;
import org.sonar.server.db.DbClient; import org.sonar.server.db.DbClient;


import static org.sonar.core.component.SnapshotQuery.SORT_FIELD.BY_DATE; import static org.sonar.core.component.db.SnapshotQuery.SORT_FIELD.BY_DATE;
import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.ASC; import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.ASC;
import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.DESC; import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.DESC;


/** /**
* Populates the {@link org.sonar.server.computation.period.PeriodsHolder} * Populates the {@link org.sonar.server.computation.period.PeriodsHolder}
Expand All @@ -71,7 +71,7 @@ public class FeedPeriodsStep implements ComputationStep {
private final PeriodsHolderImpl periodsHolder; private final PeriodsHolderImpl periodsHolder;


public FeedPeriodsStep(DbClient dbClient, Settings settings, TreeRootHolder treeRootHolder, BatchReportReader batchReportReader, public FeedPeriodsStep(DbClient dbClient, Settings settings, TreeRootHolder treeRootHolder, BatchReportReader batchReportReader,
PeriodsHolderImpl periodsHolder) { PeriodsHolderImpl periodsHolder) {
this.dbClient = dbClient; this.dbClient = dbClient;
this.settings = settings; this.settings = settings;
this.treeRootHolder = treeRootHolder; this.treeRootHolder = treeRootHolder;
Expand Down
Expand Up @@ -68,30 +68,29 @@ public void execute() {
org.sonar.server.computation.component.Component root = treeRootHolder.getRoot(); org.sonar.server.computation.component.Component root = treeRootHolder.getRoot();
List<ComponentDto> existingComponents = dbClient.componentDao().selectComponentsFromProjectKey(session, root.getKey()); List<ComponentDto> existingComponents = dbClient.componentDao().selectComponentsFromProjectKey(session, root.getKey());
Map<String, ComponentDto> existingComponentDtosByKey = componentDtosByKey(existingComponents); Map<String, ComponentDto> existingComponentDtosByKey = componentDtosByKey(existingComponents);
PersisComponent persisComponent = new PersisComponent(session, existingComponentDtosByKey, reportReader); PersistComponentExecutor persistComponentExecutor = new PersistComponentExecutor(session, existingComponentDtosByKey, reportReader);

persistComponentExecutor.recursivelyProcessComponent(root, null);
persisComponent.recursivelyProcessComponent(root, null);
session.commit(); session.commit();
} finally { } finally {
session.close(); session.close();
} }
} }


private class PersisComponent { private class PersistComponentExecutor {


private final BatchReportReader reportReader; private final BatchReportReader reportReader;
private final Map<String, ComponentDto> existingComponentDtosByKey; private final Map<String, ComponentDto> existingComponentDtosByKey;
private final DbSession dbSession; private final DbSession dbSession;


private ComponentDto project; private ComponentDto project;


public PersisComponent(DbSession dbSession, Map<String, ComponentDto> existingComponentDtosByKey, BatchReportReader reportReader) { public PersistComponentExecutor(DbSession dbSession, Map<String, ComponentDto> existingComponentDtosByKey, BatchReportReader reportReader) {
this.reportReader = reportReader; this.reportReader = reportReader;
this.existingComponentDtosByKey = existingComponentDtosByKey; this.existingComponentDtosByKey = existingComponentDtosByKey;
this.dbSession = dbSession; this.dbSession = dbSession;
} }


private void recursivelyProcessComponent(Component component, @Nullable ComponentDto lastModule) { public void recursivelyProcessComponent(Component component, @Nullable ComponentDto lastModule) {
BatchReport.Component reportComponent = reportReader.readComponent(component.getRef()); BatchReport.Component reportComponent = reportReader.readComponent(component.getRef());


switch (component.getType()) { switch (component.getType()) {
Expand Down
Expand Up @@ -27,14 +27,14 @@
import org.junit.Test; import org.junit.Test;
import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.DateUtils;
import org.sonar.core.component.SnapshotDto; import org.sonar.core.component.SnapshotDto;
import org.sonar.core.component.SnapshotQuery; import org.sonar.core.component.db.SnapshotQuery;
import org.sonar.core.persistence.AbstractDaoTestCase; import org.sonar.core.persistence.AbstractDaoTestCase;
import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbSession;


import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.sonar.core.component.SnapshotQuery.SORT_FIELD.BY_DATE; import static org.sonar.core.component.db.SnapshotQuery.SORT_FIELD.BY_DATE;
import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.ASC; import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.ASC;
import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.DESC; import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.DESC;


public class SnapshotDaoTest extends AbstractDaoTestCase { public class SnapshotDaoTest extends AbstractDaoTestCase {


Expand Down
Expand Up @@ -45,10 +45,44 @@ public void get_periods() throws Exception {
} }


@Test @Test
public void fail_to_get_periods_if_not_initialized() throws Exception { public void get_periods_throws_illegal_state_exception_if_not_initialized() throws Exception {
thrown.expect(IllegalArgumentException.class); thrown.expect(IllegalStateException.class);
thrown.expectMessage("Periods have not been initialized yet"); thrown.expectMessage("Periods have not been initialized yet");


new PeriodsHolderImpl().getPeriods(); new PeriodsHolderImpl().getPeriods();
} }

@Test
public void set_null_periods_trows_null_pointer_exception() throws Exception {
thrown.expect(NullPointerException.class);
thrown.expectMessage("Periods cannot be null");

new PeriodsHolderImpl().setPeriods(null);
}

@Test
public void set_periods_throws_illegal_state_exception_if_already_initialized() throws Exception {
thrown.expect(IllegalStateException.class);
thrown.expectMessage("Periods have already been initialized");

List<Period> periods = new ArrayList<>();
periods.add(new Period(1, "mode", null, 1000L));

PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl();
periodsHolder.setPeriods(periods);
periodsHolder.setPeriods(periods);
}

@Test
public void update_periods_throws_unsupported_operation_exception() throws Exception {
thrown.expect(UnsupportedOperationException.class);

List<Period> periods = new ArrayList<>();
periods.add(new Period(1, "mode", null, 1000L));

PeriodsHolderImpl periodsHolder = new PeriodsHolderImpl();
periodsHolder.setPeriods(periods);

periodsHolder.getPeriods().add(new Period(2, "mode", null, 1001L));
}
} }
Expand Up @@ -44,7 +44,7 @@ public void evaluate() throws Throwable {
} }


private void clear() { private void clear() {
this.periods = new ArrayList<>(); this.periods.clear();
} }


@Override @Override
Expand Down
Expand Up @@ -781,7 +781,7 @@ public void update_module_uuid_when_moving_a_module() throws Exception {
} }


@Test @Test
public void not_update_create_at() throws Exception { public void do_not_update_created_at_on_existing_component() throws Exception {
Date oldDate = DateUtils.parseDate("2015-01-01"); Date oldDate = DateUtils.parseDate("2015-01-01");
ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY).setName("Project").setCreatedAt(oldDate); ComponentDto project = ComponentTesting.newProjectDto("ABCD").setKey(PROJECT_KEY).setName("Project").setCreatedAt(oldDate);
dbClient.componentDao().insert(session, project); dbClient.componentDao().insert(session, project);
Expand Down
Expand Up @@ -33,7 +33,7 @@
import org.sonar.batch.protocol.output.BatchReport; import org.sonar.batch.protocol.output.BatchReport;
import org.sonar.core.component.ComponentDto; import org.sonar.core.component.ComponentDto;
import org.sonar.core.component.SnapshotDto; import org.sonar.core.component.SnapshotDto;
import org.sonar.core.component.SnapshotQuery; import org.sonar.core.component.db.SnapshotQuery;
import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbSession;
import org.sonar.core.persistence.DbTester; import org.sonar.core.persistence.DbTester;
import org.sonar.server.component.ComponentTesting; import org.sonar.server.component.ComponentTesting;
Expand Down
Expand Up @@ -42,9 +42,7 @@
import org.sonar.api.resources.ResourceUtils; import org.sonar.api.resources.ResourceUtils;
import org.sonar.api.utils.Duration; import org.sonar.api.utils.Duration;
import org.sonar.api.utils.Durations; import org.sonar.api.utils.Durations;
import org.sonar.batch.index.BatchComponentCache;
import org.sonar.core.qualitygate.db.QualityGateConditionDto; import org.sonar.core.qualitygate.db.QualityGateConditionDto;
import org.sonar.core.timemachine.Periods;


public class QualityGateVerifier implements Decorator { public class QualityGateVerifier implements Decorator {


Expand All @@ -58,15 +56,11 @@ public class QualityGateVerifier implements Decorator {


private QualityGate qualityGate; private QualityGate qualityGate;


private Periods periods;
private I18n i18n; private I18n i18n;
private Durations durations; private Durations durations;
private BatchComponentCache resourceCache;


public QualityGateVerifier(QualityGate qualityGate, BatchComponentCache resourceCache, Periods periods, I18n i18n, Durations durations) { public QualityGateVerifier(QualityGate qualityGate, I18n i18n, Durations durations) {
this.qualityGate = qualityGate; this.qualityGate = qualityGate;
this.resourceCache = resourceCache;
this.periods = periods;
this.i18n = i18n; this.i18n = i18n;
this.durations = durations; this.durations = durations;
} }
Expand Down Expand Up @@ -166,11 +160,11 @@ private String getAlertLabel(Resource project, ResolvedCondition condition, Metr
.append(" ").append(operatorLabel(condition.operator())).append(" ") .append(" ").append(operatorLabel(condition.operator())).append(" ")
.append(alertValue(condition, level)); .append(alertValue(condition, level));


// Disabled because snapshot is no more created by the batch // TODO Disabled because snapshot is no more created by the batch, but should be reactivated when the decorator will be moved to the batch
// if (alertPeriod != null) { // if (alertPeriod != null) {
// Snapshot snapshot = resourceCache.get(project).snapshot(); // Snapshot snapshot = resourceCache.get(project).snapshot();
// stringBuilder.append(" ").append(periods.label(snapshot, alertPeriod)); // stringBuilder.append(" ").append(periods.label(snapshot, alertPeriod));
// } // }


return stringBuilder.toString(); return stringBuilder.toString();
} }
Expand Down
Expand Up @@ -42,9 +42,7 @@
import org.sonar.api.test.IsMeasure; import org.sonar.api.test.IsMeasure;
import org.sonar.api.utils.Duration; import org.sonar.api.utils.Duration;
import org.sonar.api.utils.Durations; import org.sonar.api.utils.Durations;
import org.sonar.batch.index.BatchComponentCache;
import org.sonar.core.qualitygate.db.QualityGateConditionDto; import org.sonar.core.qualitygate.db.QualityGateConditionDto;
import org.sonar.core.timemachine.Periods;


import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
Expand All @@ -67,14 +65,12 @@ public class QualityGateVerifierTest {
Measure measureComplexity; Measure measureComplexity;
Resource project; Resource project;
Snapshot snapshot; Snapshot snapshot;
Periods periods;
I18n i18n; I18n i18n;
Durations durations; Durations durations;


@Before @Before
public void before() { public void before() {
context = mock(DecoratorContext.class); context = mock(DecoratorContext.class);
periods = mock(Periods.class);
i18n = mock(I18n.class); i18n = mock(I18n.class);
when(i18n.message(any(Locale.class), eq("variation"), eq("variation"))).thenReturn("variation"); when(i18n.message(any(Locale.class), eq("variation"), eq("variation"))).thenReturn("variation");
durations = mock(Durations.class); durations = mock(Durations.class);
Expand All @@ -93,10 +89,7 @@ public void before() {


project = new Project("foo"); project = new Project("foo");


BatchComponentCache resourceCache = new BatchComponentCache(); verifier = new QualityGateVerifier(qualityGate, i18n, durations);
resourceCache.add(project, null).setSnapshot(snapshot);

verifier = new QualityGateVerifier(qualityGate, resourceCache, periods, i18n, durations);
} }


@Test @Test
Expand Down Expand Up @@ -361,7 +354,7 @@ public void shouldLabelAlertContainsPeriod() {
measureClasses.setVariation1(40d); measureClasses.setVariation1(40d);


when(i18n.message(any(Locale.class), eq("metric.classes.name"), anyString())).thenReturn("Classes"); when(i18n.message(any(Locale.class), eq("metric.classes.name"), anyString())).thenReturn("Classes");
when(periods.label(snapshot, 1)).thenReturn("since someday"); // when(periods.label(snapshot, 1)).thenReturn("since someday");


ArrayList<ResolvedCondition> conditions = Lists.newArrayList( ArrayList<ResolvedCondition> conditions = Lists.newArrayList(
mockCondition(CoreMetrics.CLASSES, QualityGateConditionDto.OPERATOR_GREATER_THAN, null, "30", 1) // generates warning because classes mockCondition(CoreMetrics.CLASSES, QualityGateConditionDto.OPERATOR_GREATER_THAN, null, "30", 1) // generates warning because classes
Expand All @@ -385,7 +378,7 @@ public void shouldLabelAlertForNewMetricDoNotContainsVariationWord() {
measureClasses.setVariation1(40d); measureClasses.setVariation1(40d);


when(i18n.message(any(Locale.class), eq("metric.new_metric_key.name"), anyString())).thenReturn("New Measure"); when(i18n.message(any(Locale.class), eq("metric.new_metric_key.name"), anyString())).thenReturn("New Measure");
when(periods.label(snapshot, 1)).thenReturn("since someday"); // when(periods.label(snapshot, 1)).thenReturn("since someday");


ArrayList<ResolvedCondition> conditions = Lists.newArrayList( ArrayList<ResolvedCondition> conditions = Lists.newArrayList(
mockCondition(newMetric, QualityGateConditionDto.OPERATOR_GREATER_THAN, null, "30", 1) // generates warning because classes increases mockCondition(newMetric, QualityGateConditionDto.OPERATOR_GREATER_THAN, null, "30", 1) // generates warning because classes increases
Expand Down
Expand Up @@ -24,7 +24,6 @@
import javax.annotation.CheckForNull; import javax.annotation.CheckForNull;
import org.apache.ibatis.annotations.Param; import org.apache.ibatis.annotations.Param;
import org.sonar.core.component.SnapshotDto; import org.sonar.core.component.SnapshotDto;
import org.sonar.core.component.SnapshotQuery;


public interface SnapshotMapper { public interface SnapshotMapper {


Expand Down
Expand Up @@ -18,7 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/ */


package org.sonar.core.component; package org.sonar.core.component.db;


import javax.annotation.CheckForNull; import javax.annotation.CheckForNull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
Expand Down
Expand Up @@ -18,13 +18,13 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/ */


package org.sonar.core.component; package org.sonar.core.component.db;


import org.junit.Test; import org.junit.Test;


import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.sonar.core.component.SnapshotQuery.SORT_FIELD.BY_DATE; import static org.sonar.core.component.db.SnapshotQuery.SORT_FIELD.BY_DATE;
import static org.sonar.core.component.SnapshotQuery.SORT_ORDER.ASC; import static org.sonar.core.component.db.SnapshotQuery.SORT_ORDER.ASC;


public class SnapshotQueryTest { public class SnapshotQueryTest {


Expand Down

0 comments on commit 091ec85

Please sign in to comment.