diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryResultBuilder.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryResultBuilder.java index 331fb3bc858..6771e9b331c 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryResultBuilder.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/DiscoveryResultBuilder.java @@ -36,7 +36,7 @@ public class DiscoveryResultBuilder { private final ThingUID thingUID; private @Nullable ThingUID bridgeUID; - private final Map properties = new HashMap<>(); + private Map properties = new HashMap<>(0); private @Nullable String representationProperty; private @Nullable String label; private long ttl = DiscoveryResult.TTL_UNLIMITED; @@ -74,10 +74,8 @@ public DiscoveryResultBuilder withThingType(@Nullable ThingTypeUID thingTypeUID) * @param properties of the desired result * @return the updated builder */ - public DiscoveryResultBuilder withProperties(@Nullable Map properties) { - if (properties != null) { - this.properties.putAll(properties); - } + public DiscoveryResultBuilder withProperties(Map properties) { + this.properties = properties; return this; } diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/dto/DiscoveryResultDTOMapper.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/dto/DiscoveryResultDTOMapper.java index b64aa847f33..726b9cad1dd 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/dto/DiscoveryResultDTOMapper.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/dto/DiscoveryResultDTOMapper.java @@ -12,6 +12,8 @@ */ package org.openhab.core.config.discovery.dto; +import java.util.Map; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.config.discovery.DiscoveryResult; import org.openhab.core.config.discovery.DiscoveryResultBuilder; @@ -57,9 +59,13 @@ public static DiscoveryResult map(DiscoveryResultDTO discoveryResultDTO) { final String dtoBridgeUID = discoveryResultDTO.bridgeUID; final ThingUID bridgeUID = dtoBridgeUID != null ? new ThingUID(dtoBridgeUID) : null; - return DiscoveryResultBuilder.create(thingUID).withThingType(thingTypeUID).withBridge(bridgeUID) - .withLabel(discoveryResultDTO.label) - .withRepresentationProperty(discoveryResultDTO.representationProperty) - .withProperties(discoveryResultDTO.properties).build(); + final DiscoveryResultBuilder builder = DiscoveryResultBuilder.create(thingUID).withThingType(thingTypeUID) + .withBridge(bridgeUID).withLabel(discoveryResultDTO.label) + .withRepresentationProperty(discoveryResultDTO.representationProperty); + final Map properties = discoveryResultDTO.properties; + if (properties != null) { + builder.withProperties(properties); + } + return builder.build(); } } diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryResultImpl.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryResultImpl.java index c1c0836b261..ca5a683715c 100644 --- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryResultImpl.java +++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/DiscoveryResultImpl.java @@ -14,6 +14,7 @@ import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.Map; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -79,7 +80,7 @@ public DiscoveryResultImpl(@Nullable ThingTypeUID thingTypeUID, ThingUID thingUI this.thingUID = thingUID; this.thingTypeUID = thingTypeUID; this.bridgeUID = bridgeUID; - this.properties = properties == null ? Collections.emptyMap() : Collections.unmodifiableMap(properties); + this.properties = properties == null ? Collections.emptyMap() : properties; this.representationProperty = representationProperty; this.label = label == null ? "" : label; @@ -112,7 +113,9 @@ public String getBindingId() { @Override public Map getProperties() { - return properties; + synchronized (this) { + return Collections.unmodifiableMap(new HashMap<>(properties)); + } } @Override diff --git a/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/DiscoveryResultBuilderTest.java b/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/DiscoveryResultBuilderTest.java index 0bfd6755ae4..91e0ae0d03c 100644 --- a/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/DiscoveryResultBuilderTest.java +++ b/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/DiscoveryResultBuilderTest.java @@ -16,12 +16,10 @@ import static org.hamcrest.collection.IsMapContaining.hasEntry; import static org.junit.Assert.assertThat; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingUID; @@ -49,17 +47,32 @@ public class DiscoveryResultBuilderTest { } }; private DiscoveryResultBuilder builder; - private DiscoveryResult discoveryResult; @Before public void setup() { - builder = DiscoveryResultBuilder.create(THING_UID).withThingType(THING_TYPE_UID).withProperties(properties) + builder = DiscoveryResultBuilder.create(THING_UID).withThingType(THING_TYPE_UID) .withRepresentationProperty(KEY1).withLabel("Test"); - discoveryResult = builder.build(); } @Test - public void testDiscoveryResultBuilder() { + public void testDiscoveryResultBuilderWithProperty() { + DiscoveryResult discoveryResult = builder.withProperty(KEY1, VALUE1).withProperty(KEY2, VALUE2).build(); + + assertThat(discoveryResult.getThingUID(), is(THING_UID)); + assertThat(discoveryResult.getThingTypeUID(), is(THING_TYPE_UID)); + assertThat(discoveryResult.getBindingId(), is(BINDING_ID)); + assertThat(discoveryResult.getLabel(), is("Test")); + assertThat(discoveryResult.getProperties().size(), is(2)); + assertThat(discoveryResult.getProperties(), hasEntry(KEY1, VALUE1)); + assertThat(discoveryResult.getProperties(), hasEntry(KEY2, VALUE2)); + assertThat(discoveryResult.getRepresentationProperty(), is(KEY1)); + assertThat(discoveryResult.getTimeToLive(), is(DiscoveryResult.TTL_UNLIMITED)); + } + + @Test + public void testDiscoveryResultBuilderWithProperties() { + DiscoveryResult discoveryResult = builder.withProperties(properties).build(); + assertThat(discoveryResult.getThingUID(), is(THING_UID)); assertThat(discoveryResult.getThingTypeUID(), is(THING_TYPE_UID)); assertThat(discoveryResult.getBindingId(), is(BINDING_ID)); @@ -93,12 +106,14 @@ public void testDiscoveryResultBuilderWithBridge() { } @Test - @Ignore public void subsequentBuildsCreateIndependentDiscoveryResults() { - DiscoveryResult otherDiscoveryResult = builder.withLabel("Second Test").withProperties(Collections.emptyMap()) - .build(); + DiscoveryResult discoveryResult = builder.withProperty(KEY1, VALUE1).build(); + DiscoveryResult otherDiscoveryResult = builder.withProperties(properties).withRepresentationProperty(KEY2) + .withLabel("Second Test").build(); assertThat(otherDiscoveryResult.getLabel(), is(not(discoveryResult.getLabel()))); assertThat(otherDiscoveryResult.getProperties().size(), is(not(discoveryResult.getProperties().size()))); + assertThat(otherDiscoveryResult.getRepresentationProperty(), + is(not(discoveryResult.getRepresentationProperty()))); } }