DiscoveryResultBuilder's withProperties method replaces existing prop…

…erties instead of appending

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

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<String, Object> properties = new HashMap<>();
+ private Map<String, Object> 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<String, Object> properties) {
- if (properties != null) {
- this.properties.putAll(properties);
- }
+ public DiscoveryResultBuilder withProperties(Map<String, Object> properties) {
+ this.properties = properties;
  return this;
  }
 

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<String, Object> properties = discoveryResultDTO.properties;
+ if (properties != null) {
+ builder.withProperties(properties);
+ }
+ return builder.build();
  }
 }

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<String, Object> getProperties() {
- return properties;
+ synchronized (this) {
+ return Collections.unmodifiableMap(new HashMap<>(properties));
+ }
  }
 
  @Override

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())));
  }
 }