From e95a89dd43c1a47dde000147454f029b8d4b7794 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 22 Sep 2015 14:34:39 +0200 Subject: [PATCH] Create a builder for Component --- .../computation/component/Component.java | 5 +- .../computation/component/ComponentImpl.java | 161 +++++++++++------- .../step/BuildComponentTreeStep.java | 13 +- .../component/ComponentImplTest.java | 102 +++++++---- 4 files changed, 185 insertions(+), 96 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/component/Component.java b/server/sonar-server/src/main/java/org/sonar/server/computation/component/Component.java index d9b992c5465..2a575a56ec3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/component/Component.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/component/Component.java @@ -23,7 +23,6 @@ import java.util.EnumSet; import java.util.List; import java.util.Set; import javax.annotation.CheckForNull; -import org.sonar.server.computation.step.FillComponentsStep; public interface Component { enum Type { @@ -62,12 +61,12 @@ public interface Component { Type getType(); /** - * Returns the component uuid only when {@link FillComponentsStep} has been executed, otherwise it will throw an exception. + * Returns the component uuid */ String getUuid(); /** - * Returns the component key only when {@link FillComponentsStep} has been executed, otherwise it will throw an exception. + * Returns the component key */ String getKey(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/component/ComponentImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/component/ComponentImpl.java index 3e4f4a6a160..e2a65f0c142 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/component/ComponentImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/component/ComponentImpl.java @@ -19,22 +19,27 @@ */ package org.sonar.server.computation.component; -import java.util.Collections; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import java.util.ArrayList; import java.util.List; import javax.annotation.CheckForNull; -import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import org.sonar.batch.protocol.Constants; import org.sonar.batch.protocol.output.BatchReport; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.ImmutableList.copyOf; -import static com.google.common.collect.Iterables.filter; +import static java.util.Arrays.asList; +@Immutable public class ComponentImpl implements Component { private final Type type; private final String name; + private final String key; + private final String uuid; + @CheckForNull private final String description; private final List children; @@ -43,50 +48,15 @@ public class ComponentImpl implements Component { @CheckForNull private final FileAttributes fileAttributes; - // Mutable values - private String key; - private String uuid; - - public ComponentImpl(BatchReport.Component component, @Nullable Iterable children) { - this.type = convertType(component.getType()); - this.name = checkNotNull(component.getName()); - this.description = component.hasDescription() ? component.getDescription() : null; - this.reportAttributes = createBatchAttributes(component); - this.fileAttributes = createFileAttributes(component); - this.children = children == null ? Collections.emptyList() : copyOf(filter(children, notNull())); - } - - private static ReportAttributes createBatchAttributes(BatchReport.Component component) { - return ReportAttributes.newBuilder(component.getRef()) - .setVersion(component.hasVersion() ? component.getVersion() : null) - .setPath(component.hasPath() ? component.getPath() : null) - .build(); - } - - @CheckForNull - private static FileAttributes createFileAttributes(BatchReport.Component component) { - if (component.getType() != Constants.ComponentType.FILE) { - return null; - } - - return new FileAttributes( - component.hasIsTest() && component.getIsTest(), - component.hasLanguage() ? component.getLanguage() : null); - } - - public static Type convertType(Constants.ComponentType type) { - switch (type) { - case PROJECT: - return Type.PROJECT; - case MODULE: - return Type.MODULE; - case DIRECTORY: - return Type.DIRECTORY; - case FILE: - return Type.FILE; - default: - throw new IllegalArgumentException("Unsupported Constants.ComponentType value " + type); - } + private ComponentImpl(Builder builder) { + this.type = builder.type; + this.key = builder.key; + this.name = builder.name == null ? String.valueOf(builder.key) : builder.name; + this.description = builder.description; + this.uuid = builder.uuid; + this.reportAttributes = builder.reportAttributes; + this.fileAttributes = builder.fileAttributes; + this.children = ImmutableList.copyOf(builder.children); } @Override @@ -102,11 +72,6 @@ public class ComponentImpl implements Component { return uuid; } - public ComponentImpl setUuid(String uuid) { - this.uuid = uuid; - return this; - } - @Override public String getKey() { if (key == null) { @@ -115,11 +80,6 @@ public class ComponentImpl implements Component { return key; } - public ComponentImpl setKey(String key) { - this.key = key; - return this; - } - @Override public String getName() { return this.name; @@ -152,6 +112,89 @@ public class ComponentImpl implements Component { throw new IllegalStateException("Only component of type PROJECT_VIEW have a FileAttributes object"); } + public static Builder builder(BatchReport.Component component) { + return new Builder(component); + } + + public static final class Builder { + + private final Type type; + private final ReportAttributes reportAttributes; + private String uuid; + private String key; + private String name; + private String description; + private FileAttributes fileAttributes; + private final List children = new ArrayList<>(); + + private Builder(BatchReport.Component component) { + checkNotNull(component); + this.type = convertType(component.getType()); + this.name = checkNotNull(component.getName()); + this.description = component.hasDescription() ? component.getDescription() : null; + this.reportAttributes = createBatchAttributes(component); + this.fileAttributes = createFileAttributes(component); + } + + public Builder setUuid(String s) { + this.uuid = checkNotNull(s); + return this; + } + + public Builder setKey(String s) { + this.key = checkNotNull(s); + return this; + } + + public Builder addChildren(Component... c) { + for (Component component : c) { + checkArgument(component.getType().isReportType()); + } + this.children.addAll(asList(c)); + return this; + } + + public ComponentImpl build() { + checkNotNull(key); + checkNotNull(uuid); + return new ComponentImpl(this); + } + + private static ReportAttributes createBatchAttributes(BatchReport.Component component) { + return ReportAttributes.newBuilder(component.getRef()) + .setVersion(component.hasVersion() ? component.getVersion() : null) + .setPath(component.hasPath() ? component.getPath() : null) + .build(); + } + + @CheckForNull + private static FileAttributes createFileAttributes(BatchReport.Component component) { + if (component.getType() != Constants.ComponentType.FILE) { + return null; + } + + return new FileAttributes( + component.hasIsTest() && component.getIsTest(), + component.hasLanguage() ? component.getLanguage() : null); + } + + @VisibleForTesting + static Type convertType(Constants.ComponentType type) { + switch (type) { + case PROJECT: + return Type.PROJECT; + case MODULE: + return Type.MODULE; + case DIRECTORY: + return Type.DIRECTORY; + case FILE: + return Type.FILE; + default: + throw new IllegalArgumentException("Unsupported Constants.ComponentType value " + type); + } + } + } + @Override public String toString() { return "ComponentImpl{" + diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/BuildComponentTreeStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/BuildComponentTreeStep.java index 74d8f179fc5..6e1e2a2fdc5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/BuildComponentTreeStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/BuildComponentTreeStep.java @@ -35,6 +35,9 @@ import org.sonar.server.computation.component.ComponentImpl; import org.sonar.server.computation.component.MutableTreeRootHolder; import org.sonar.server.computation.component.UuidFactory; +import static com.google.common.collect.Iterables.toArray; +import static org.sonar.server.computation.component.ComponentImpl.builder; + /** * Populates the {@link MutableTreeRootHolder} and {@link MutableAnalysisMetadataHolder} from the {@link BatchReportReader} */ @@ -96,11 +99,11 @@ public class BuildComponentTreeStep implements ComputationStep { } private ComponentImpl buildComponent(BatchReport.Component reportComponent, String componentKey, String latestModuleKey){ - // TODO create builder for component - ComponentImpl component = new ComponentImpl(reportComponent, buildChildren(reportComponent, latestModuleKey)); - component.setKey(componentKey); - component.setUuid(uuidFactory.getOrCreateForKey(componentKey)); - return component; + return builder(reportComponent) + .addChildren(toArray(buildChildren(reportComponent, latestModuleKey), Component.class)) + .setKey(componentKey) + .setUuid(uuidFactory.getOrCreateForKey(componentKey)) + .build(); } private Iterable buildChildren(BatchReport.Component component, final String latestModuleKey) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComponentImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComponentImplTest.java index a9074d27180..fde6c340d66 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComponentImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/component/ComponentImplTest.java @@ -19,9 +19,9 @@ */ package org.sonar.server.computation.component; -import java.util.Collections; -import java.util.List; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.sonar.batch.protocol.output.BatchReport; import static com.google.common.base.Predicates.equalTo; @@ -31,60 +31,78 @@ import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.sonar.batch.protocol.Constants.ComponentType; +import static org.sonar.batch.protocol.Constants.ComponentType.FILE; +import static org.sonar.server.computation.component.ComponentImpl.builder; public class ComponentImplTest { - private static final List EMPTY_CHILD_LIST = Collections.emptyList(); + static final String KEY = "KEY"; + static final String UUID = "UUID"; - @Test(expected = NullPointerException.class) - public void constructor_throws_NPE_if_component_arg_is_Null() { - new ComponentImpl(null, null); + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Test + public void verify_key_and_uuid() throws Exception { + ComponentImpl component = builder(BatchReport.Component.newBuilder().build()).setKey(KEY).setUuid(UUID).build(); + + assertThat(component.getKey()).isEqualTo(KEY); + assertThat(component.getUuid()).isEqualTo(UUID); } - @Test(expected = UnsupportedOperationException.class) - public void getUuid_throws_UOE_if_uuid_has_not_been_set_yet() { - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().build(), EMPTY_CHILD_LIST); - component.getUuid(); + @Test + public void builder_throws_NPE_if_component_arg_is_Null() { + thrown.expect(NullPointerException.class); + + builder(null); } - @Test(expected = UnsupportedOperationException.class) - public void getKey_throws_UOE_if_uuid_has_not_been_set_yet() { - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().build(), EMPTY_CHILD_LIST); - component.getKey(); + @Test + public void set_key_throws_NPE_if_component_arg_is_Null() { + thrown.expect(NullPointerException.class); + + builder(BatchReport.Component.newBuilder().build()).setUuid(null); } @Test - public void verify_setUuid() { - String uuid = "toto"; - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().build(), EMPTY_CHILD_LIST).setUuid(uuid); - assertThat(component.getUuid()).isEqualTo(uuid); + public void set_uuid_throws_NPE_if_component_arg_is_Null() { + thrown.expect(NullPointerException.class); + + builder(BatchReport.Component.newBuilder().build()).setKey(null); } @Test - public void verify_setKey() { - String key = "toto"; - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().build(), EMPTY_CHILD_LIST).setKey(key); - assertThat(component.getKey()).isEqualTo(key); + public void build_without_key_throws_NPE_if_component_arg_is_Null() { + thrown.expect(NullPointerException.class); + + builder(BatchReport.Component.newBuilder().build()).setUuid("ABCD").build(); + } + + @Test + public void build_without_uuid_throws_NPE_if_component_arg_is_Null() { + thrown.expect(NullPointerException.class); + + builder(BatchReport.Component.newBuilder().build()).setKey(KEY).build(); } @Test public void get_name_from_batch_component() { String name = "project"; - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().setName(name).build(), EMPTY_CHILD_LIST); + ComponentImpl component = buildSimpleComponent(BatchReport.Component.newBuilder().setName(name).build()); assertThat(component.getName()).isEqualTo(name); } @Test public void get_version_from_batch_component() { String version = "1.0"; - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().setVersion(version).build(), EMPTY_CHILD_LIST); + ComponentImpl component = buildSimpleComponent(BatchReport.Component.newBuilder().setVersion(version).build()); assertThat(component.getReportAttributes().getVersion()).isEqualTo(version); } @Test public void getFileAttributes_throws_ISE_if_BatchComponent_does_not_have_type_FILE() { - for (ComponentType componentType : from(asList(ComponentType.values())).filter(not(equalTo(ComponentType.FILE)))) { - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().setType(componentType).build(), EMPTY_CHILD_LIST); + for (ComponentType componentType : from(asList(ComponentType.values())).filter(not(equalTo(FILE)))) { + ComponentImpl component = buildSimpleComponent(BatchReport.Component.newBuilder().setType(componentType).build()); try { component.getFileAttributes(); fail("A IllegalStateException should have been raised"); @@ -96,7 +114,7 @@ public class ComponentImplTest { @Test public void isUnitTest_returns_true_if_IsTest_is_set_in_BatchComponent() { - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().setType(ComponentType.FILE).setIsTest(true).build(), EMPTY_CHILD_LIST); + ComponentImpl component = buildSimpleComponent(BatchReport.Component.newBuilder().setType(FILE).setIsTest(true).build()); assertThat(component.getFileAttributes().isUnitTest()).isTrue(); } @@ -104,15 +122,41 @@ public class ComponentImplTest { @Test public void isUnitTest_returns_value_of_language_of_BatchComponent() { String languageKey = "some language key"; - ComponentImpl component = new ComponentImpl(BatchReport.Component.newBuilder().setType(ComponentType.FILE).setLanguage(languageKey).build(), EMPTY_CHILD_LIST); + ComponentImpl component = buildSimpleComponent(BatchReport.Component.newBuilder().setType(FILE).setLanguage(languageKey).build()); assertThat(component.getFileAttributes().getLanguageKey()).isEqualTo(languageKey); } + @Test + public void build_with_child() throws Exception { + buildSimpleComponent(BatchReport.Component.newBuilder().build()); + + ComponentImpl child = builder(BatchReport.Component.newBuilder().setType(FILE).build()) + .setKey("CHILD_KEY") + .setUuid("CHILD_UUID") + .build(); + ComponentImpl componentImpl = builder(BatchReport.Component.newBuilder().build()) + .setKey(KEY) + .setUuid(UUID) + .addChildren(child) + .build(); + + assertThat(componentImpl.getChildren()).hasSize(1); + Component childReloaded = componentImpl.getChildren().iterator().next(); + assertThat(childReloaded.getKey()).isEqualTo("CHILD_KEY"); + assertThat(childReloaded.getUuid()).isEqualTo("CHILD_UUID"); + assertThat(childReloaded.getType()).isEqualTo(Component.Type.FILE); + } + @Test public void convertType() { for (ComponentType componentType : ComponentType.values()) { - assertThat(ComponentImpl.convertType(componentType)).isEqualTo(Component.Type.valueOf(componentType.name())); + assertThat(ComponentImpl.Builder.convertType(componentType)).isEqualTo(Component.Type.valueOf(componentType.name())); } } + + private static ComponentImpl buildSimpleComponent(BatchReport.Component reportComponent) { + return builder(reportComponent).setKey(KEY).setUuid(UUID).build(); + } + } -- 2.39.5