From 9f461eb2716b4f39efae5c182d1be232afc93824 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 6 Sep 2013 17:29:46 +0200 Subject: [PATCH] SONAR-4644 Failure "Duplicate Keys not allowed" in PicoContainer --- .../sonar/api/config/PropertyDefinition.java | 9 ++- .../NotificationDispatcherMetadata.java | 21 +++++ .../api/platform/ComponentContainer.java | 16 ++-- .../org/sonar/api/platform/ComponentKeys.java | 57 +++++++++++++ .../org/sonar/api/resources/ResourceType.java | 6 +- .../sonar/api/resources/ResourceTypeTree.java | 15 ++-- .../api/config/PropertyDefinitionTest.java | 6 ++ .../sonar/api/platform/ComponentKeysTest.java | 80 +++++++++++++++++++ 8 files changed, 190 insertions(+), 20 deletions(-) create mode 100644 sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentKeys.java create mode 100644 sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentKeysTest.java 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 cb1324f7df6..5764da363c7 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 @@ -278,6 +278,14 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension return index; } + @Override + public String toString() { + if (StringUtils.isEmpty(propertySetKey)) { + return key; + } + return new StringBuilder().append(propertySetKey).append('|').append(key).toString(); + } + public static final class Result { private static final Result SUCCESS = new Result(null); private String errorKey = null; @@ -516,5 +524,4 @@ public final class PropertyDefinition implements BatchExtension, ServerExtension } } } - } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationDispatcherMetadata.java b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationDispatcherMetadata.java index b9edf02f382..9111b589222 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationDispatcherMetadata.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/notifications/NotificationDispatcherMetadata.java @@ -78,4 +78,25 @@ public final class NotificationDispatcherMetadata implements ServerExtension { return dispatcherKey; } + @Override + public String toString() { + return dispatcherKey; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + NotificationDispatcherMetadata that = (NotificationDispatcherMetadata) o; + return dispatcherKey.equals(that.dispatcherKey); + } + + @Override + public int hashCode() { + return dispatcherKey.hashCode(); + } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java index 4301fdcca16..41aec974153 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentContainer.java @@ -44,6 +44,7 @@ public class ComponentContainer implements BatchComponent, ServerComponent { ComponentContainer parent, child; MutablePicoContainer pico; PropertyDefinitions propertyDefinitions; + ComponentKeys componentKeys; /** * Create root container @@ -52,6 +53,7 @@ public class ComponentContainer implements BatchComponent, ServerComponent { this.parent = null; this.child = null; this.pico = createPicoContainer(); + this.componentKeys = new ComponentKeys(); propertyDefinitions = new PropertyDefinitions(); addSingleton(propertyDefinitions); addSingleton(this); @@ -65,6 +67,7 @@ public class ComponentContainer implements BatchComponent, ServerComponent { this.pico = parent.pico.makeChildContainer(); this.parent.child = this; this.propertyDefinitions = parent.propertyDefinitions; + this.componentKeys = new ComponentKeys(); addSingleton(this); } @@ -159,13 +162,15 @@ public class ComponentContainer implements BatchComponent, ServerComponent { * is returned each time the component is requested */ public ComponentContainer addComponent(Object component, boolean singleton) { - pico.as(singleton ? Characteristics.CACHE : Characteristics.NO_CACHE).addComponent(getComponentKey(component), component); + Object key = componentKeys.of(component); + pico.as(singleton ? Characteristics.CACHE : Characteristics.NO_CACHE).addComponent(key, component); declareExtension(null, component); return this; } public ComponentContainer addExtension(@Nullable PluginMetadata plugin, Object extension) { - pico.as(Characteristics.CACHE).addComponent(getComponentKey(extension), extension); + Object key = componentKeys.of(extension); + pico.as(Characteristics.CACHE).addComponent(key, extension); declareExtension(plugin, extension); return this; } @@ -208,13 +213,6 @@ public class ComponentContainer implements BatchComponent, ServerComponent { return new DefaultPicoContainer(new OptInCaching(), lifecycleStrategy, null); } - static Object getComponentKey(Object component) { - if (component instanceof Class) { - return component; - } - return new StringBuilder().append(component.getClass().getCanonicalName()).append("-").append(component.toString()).toString(); - } - public ComponentContainer getParent() { return parent; } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentKeys.java b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentKeys.java new file mode 100644 index 00000000000..e13d9cddaad --- /dev/null +++ b/sonar-plugin-api/src/main/java/org/sonar/api/platform/ComponentKeys.java @@ -0,0 +1,57 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.api.platform; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.regex.Pattern; + +/** + * @since 3.7.1 + */ +class ComponentKeys { + + private static final Pattern IDENTITY_HASH_PATTERN = Pattern.compile(".+@[a-f0-9]+"); + private final Set objectsWithoutToString = new HashSet(); + + Object of(Object component) { + return of(component, LoggerFactory.getLogger(ComponentKeys.class)); + } + + @VisibleForTesting + Object of(Object component, Logger log) { + if (component instanceof Class) { + return component; + } + String key = component.toString(); + if (IDENTITY_HASH_PATTERN.matcher(key).matches()) { + if (!objectsWithoutToString.add(component.getClass())) { + log.warn(String.format("Bad component key: %s. Please implement toString() method on class %s", key, component.getClass().getName())); + } + key += UUID.randomUUID().toString(); + } + return new StringBuilder().append(component.getClass().getCanonicalName()).append("-").append(key).toString(); + } +} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceType.java b/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceType.java index 3d7f4f6b8fa..b5511604f0d 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceType.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceType.java @@ -51,7 +51,6 @@ import java.util.Map; * * @since 2.14 */ -@Beta @Immutable public class ResourceType { @@ -231,4 +230,9 @@ public class ResourceType { public int hashCode() { return qualifier.hashCode(); } + + @Override + public String toString() { + return qualifier; + } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceTypeTree.java b/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceTypeTree.java index 069ba012722..2f972c1ad51 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceTypeTree.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/resources/ResourceTypeTree.java @@ -19,20 +19,13 @@ */ package org.sonar.api.resources; -import com.google.common.annotations.Beta; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Collections2; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; -import com.google.common.collect.ListMultimap; -import com.google.common.collect.Lists; +import com.google.common.collect.*; import org.sonar.api.ServerExtension; import org.sonar.api.task.TaskExtension; import javax.annotation.concurrent.Immutable; - import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -40,7 +33,6 @@ import java.util.List; /** * @since 2.14 */ -@Beta @Immutable public class ResourceTypeTree implements TaskExtension, ServerExtension { @@ -74,6 +66,11 @@ public class ResourceTypeTree implements TaskExtension, ServerExtension { })); } + @Override + public String toString() { + return root.getQualifier(); + } + public static Builder builder() { return new Builder(); } 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 34180731b0b..876a50d00ca 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 @@ -32,6 +32,12 @@ import static org.fest.assertions.Fail.fail; public class PropertyDefinitionTest { + @Test + public void should_override_toString() { + PropertyDefinition def = PropertyDefinition.builder("hello").build(); + assertThat(def.toString()).isEqualTo("hello"); + } + @Test public void should_create_property() { PropertyDefinition def = PropertyDefinition.builder("hello") diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentKeysTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentKeysTest.java new file mode 100644 index 00000000000..0b307d71120 --- /dev/null +++ b/sonar-plugin-api/src/test/java/org/sonar/api/platform/ComponentKeysTest.java @@ -0,0 +1,80 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.api.platform; + +import org.junit.Test; +import org.slf4j.Logger; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Matchers.startsWith; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +public class ComponentKeysTest { + + ComponentKeys keys = new ComponentKeys(); + + @Test + public void generate_key_of_class() throws Exception { + assertThat(keys.of(FakeComponent.class)).isEqualTo(FakeComponent.class); + } + + @Test + public void generate_key_of_object() throws Exception { + assertThat(keys.of(new FakeComponent())).isEqualTo("org.sonar.api.platform.ComponentKeysTest.FakeComponent-fake"); + } + + @Test + public void should_log_warning_if_toString_is_not_overridden() throws Exception { + Logger log = mock(Logger.class); + keys.of(new Object(), log); + verifyZeroInteractions(log); + + // only on non-first runs, to avoid false-positives on singletons + keys.of(new Object(), log); + verify(log).warn(startsWith("Bad component key")); + } + + @Test + public void should_generate_unique_key_when_toString_is_not_overridden() throws Exception { + Object key = keys.of(new WrongToStringImpl()); + assertThat(key).isNotEqualTo(WrongToStringImpl.KEY); + + Object key2 = keys.of(new WrongToStringImpl()); + assertThat(key2).isNotEqualTo(key); + } + + static class FakeComponent { + @Override + public String toString() { + return "fake"; + } + } + + static class WrongToStringImpl { + static final String KEY = "my.Component@123a"; + + @Override + public String toString() { + return KEY; + } + } +} -- 2.39.5