From cad8a44428531aa4a8bf7962381e1a6c9464c06d Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 23 Apr 2013 16:22:02 +0200 Subject: Forbid setting both onQualifiers and onlyOnQualifiers --- .../org/sonar/api/config/PropertyDefinition.java | 20 +++-- .../sonar/api/config/PropertyDefinitionTest.java | 85 +++++++++++++++------- 2 files changed, 70 insertions(+), 35 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java index 0c55c02fb3b..c9d6834344c 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java @@ -80,7 +80,8 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension this.propertySetKey = builder.propertySetKey; this.fields = builder.fields; this.deprecatedKey = builder.deprecatedKey; - this.qualifiers = builder.qualifiers; + this.qualifiers = builder.onQualifiers; + this.qualifiers.addAll(builder.onlyOnQualifiers); this.index = builder.index; } @@ -288,7 +289,8 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension private String defaultValue; private String category; private String subcategory; - private List qualifiers; + private List onQualifiers; + private List onlyOnQualifiers; private boolean global; private PropertyType type; private List options; @@ -310,7 +312,8 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension this.deprecatedKey = ""; this.global = true; this.type = PropertyType.STRING; - this.qualifiers = newArrayList(); + this.onQualifiers = newArrayList(); + this.onlyOnQualifiers = newArrayList(); this.options = newArrayList(); this.fields = newArrayList(); this.hidden = false; @@ -343,25 +346,25 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension } public Builder onQualifiers(String first, String... rest) { - this.qualifiers = Lists.asList(first, rest); + this.onQualifiers.addAll(Lists.asList(first, rest)); this.global = true; return this; } public Builder onQualifiers(List qualifiers) { - this.qualifiers = ImmutableList.copyOf(qualifiers); + this.onQualifiers.addAll(ImmutableList.copyOf(qualifiers)); this.global = true; return this; } public Builder onlyOnQualifiers(String first, String... rest) { - this.qualifiers = Lists.asList(first, rest); + this.onlyOnQualifiers.addAll(Lists.asList(first, rest)); this.global = false; return this; } public Builder onlyOnQualifiers(List qualifiers) { - this.qualifiers = ImmutableList.copyOf(qualifiers); + this.onlyOnQualifiers.addAll(ImmutableList.copyOf(qualifiers)); this.global = false; return this; } @@ -419,7 +422,8 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension public PropertyDefinition build() { Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "Key must be set"); fixType(key, type); - Preconditions.checkArgument(!hidden || qualifiers.isEmpty(), "Cannot be hidden and defining qualifiers on which to display"); + Preconditions.checkArgument(onQualifiers.isEmpty() || onlyOnQualifiers.isEmpty(), "Cannot define both onQualifiers and onlyOnQualifiers"); + Preconditions.checkArgument((!hidden || (onQualifiers.isEmpty()) && (!hidden || (onlyOnQualifiers.isEmpty()))), "Cannot be hidden and defining qualifiers on which to display"); if (hidden) { global = false; } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java index 35ee091cdaa..84978ff1795 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java @@ -28,22 +28,23 @@ import org.sonar.api.resources.Qualifiers; import org.sonar.api.utils.AnnotationUtils; import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; public class PropertyDefinitionTest { @Test public void should_create_property() { PropertyDefinition def = PropertyDefinition.builder("hello") - .name("Hello") - .defaultValue("world") - .category("categ") - .options("de", "en") - .description("desc") - .type(PropertyType.FLOAT) - .onlyOnQualifiers(Qualifiers.FILE, Qualifiers.CLASS) - .multiValues(true) - .propertySetKey("set") - .build(); + .name("Hello") + .defaultValue("world") + .category("categ") + .options("de", "en") + .description("desc") + .type(PropertyType.FLOAT) + .onlyOnQualifiers(Qualifiers.FILE, Qualifiers.CLASS) + .multiValues(true) + .propertySetKey("set") + .build(); assertThat(def.key()).isEqualTo("hello"); assertThat(def.name()).isEqualTo("Hello"); @@ -95,8 +96,8 @@ public class PropertyDefinitionTest { @Test public void should_create_property_with_default_values() { PropertyDefinition def = PropertyDefinition.builder("hello") - .name("Hello") - .build(); + .name("Hello") + .build(); assertThat(def.key()).isEqualTo("hello"); assertThat(def.name()).isEqualTo("Hello"); @@ -136,12 +137,12 @@ public class PropertyDefinitionTest { @Test public void should_support_property_sets() { PropertyDefinition def = PropertyDefinition.builder("hello") - .name("Hello") - .fields( - PropertyFieldDefinition.build("first").name("First").description("Description").options("A", "B").build(), - PropertyFieldDefinition.build("second").name("Second").type(PropertyType.INTEGER).indicativeSize(5).build() - ) - .build(); + .name("Hello") + .fields( + PropertyFieldDefinition.build("first").name("First").description("Description").options("A", "B").build(), + PropertyFieldDefinition.build("second").name("Second").type(PropertyType.INTEGER).indicativeSize(5).build() + ) + .build(); assertThat(def.fields()).hasSize(2); assertThat(def.fields().get(0).key()).isEqualTo("first"); @@ -259,24 +260,54 @@ public class PropertyDefinitionTest { assertThat(def.type()).isEqualTo(PropertyType.LICENSE); } - @Test(expected = IllegalArgumentException.class) - public void should_not_define_qualifier_and_hidden() { - PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.FILE).hidden().build(); + @Test + public void should_not_authorise_empty_key() { + try { + PropertyDefinition.builder(null).build(); + fail(); + } catch (Exception e) { + assertThat(e).hasMessage("Key must be set").isInstanceOf(IllegalArgumentException.class); + } + } + + @Test + public void should_not_authorize_defining_on_qualifiers_and_hidden() { + try { + PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.FILE).hidden().build(); + fail(); + } catch (Exception e) { + assertThat(e).hasMessage("Cannot be hidden and defining qualifiers on which to display").isInstanceOf(IllegalArgumentException.class); + } } - @Test(expected = IllegalArgumentException.class) - public void should_not_authorise_empty_key() { - PropertyDefinition.builder(null).build(); + @Test + public void should_not_authorize_defining_ony_on_qualifiers_and_hidden() { + try { + PropertyDefinition.builder("foo").name("foo").onlyOnQualifiers(Qualifiers.FILE).hidden().build(); + fail(); + } catch (Exception e) { + assertThat(e).hasMessage("Cannot be hidden and defining qualifiers on which to display").isInstanceOf(IllegalArgumentException.class); + } + } + + @Test + public void should_not_authorize_defining_on_qualifiers_and_only_on_qualifiers() { + try { + PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.FILE).onlyOnQualifiers(Qualifiers.PROJECT).build(); + fail(); + } catch (Exception e) { + assertThat(e).hasMessage("Cannot define both onQualifiers and onlyOnQualifiers").isInstanceOf(IllegalArgumentException.class); + } } @Properties(@Property(key = "hello", name = "Hello", defaultValue = "world", description = "desc", - options = {"de", "en"}, category = "categ", type = PropertyType.FLOAT, global = false, project = true, module = true, multiValues = true, propertySetKey = "set")) + options = {"de", "en"}, category = "categ", type = PropertyType.FLOAT, global = false, project = true, module = true, multiValues = true, propertySetKey = "set")) static class Init { } @Properties(@Property(key = "hello", name = "Hello", fields = { - @PropertyField(key = "first", name = "First", description = "Description", options = {"A", "B"}), - @PropertyField(key = "second", name = "Second", type = PropertyType.INTEGER, indicativeSize = 5)})) + @PropertyField(key = "first", name = "First", description = "Description", options = {"A", "B"}), + @PropertyField(key = "second", name = "Second", type = PropertyType.INTEGER, indicativeSize = 5)})) static class WithPropertySet { } -- cgit v1.2.3