From 6eee84661fd1874d930a5c00b9a10cbbffea694c Mon Sep 17 00:00:00 2001
From: lolodomo <lg.hc@free.fr>
Date: Tue, 4 May 2021 19:11:25 +0200
Subject: [PATCH] [inbox] Approval with newThingId: only one segment allowed
 (#2338)

* [inbox] Approval with newThingId: only one segment allowed

Build of the new thing UID corrected in case newThingId is provided

Fix #2331

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
---
 .../config/discovery/internal/PersistentInbox.java    | 11 ++++++++---
 .../discovery/internal/PersistentInboxTest.java       | 10 +++++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/PersistentInbox.java b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/PersistentInbox.java
index 06666e1f202..3cdc625288e 100644
--- a/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/PersistentInbox.java
+++ b/bundles/org.openhab.core.config.discovery/src/main/java/org/openhab/core/config/discovery/internal/PersistentInbox.java
@@ -36,6 +36,7 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.core.common.AbstractUID;
 import org.openhab.core.common.ThreadPoolManager;
 import org.openhab.core.config.core.ConfigDescription;
 import org.openhab.core.config.core.ConfigDescriptionParameter;
@@ -183,6 +184,9 @@ protected void deactivate() {
         if (results.isEmpty()) {
             throw new IllegalArgumentException("No Thing with UID " + thingUID.getAsString() + " in inbox");
         }
+        if (newThingId != null && newThingId.contains(AbstractUID.SEPARATOR)) {
+            throw new IllegalArgumentException("New Thing ID " + newThingId + " must not contain multiple segments");
+        }
         DiscoveryResult result = results.get(0);
         final Map<String, String> properties = new HashMap<>();
         final Map<String, Object> configParams = new HashMap<>();
@@ -191,11 +195,12 @@ protected void deactivate() {
         ThingTypeUID thingTypeUID = result.getThingTypeUID();
         ThingUID newThingUID = thingUID;
         if (newThingId != null) {
+            String newUID = thingUID.getAsString().substring(0,
+                    thingUID.getAsString().lastIndexOf(AbstractUID.SEPARATOR) + 1) + newThingId;
             try {
-                newThingUID = new ThingUID(thingTypeUID, newThingId);
+                newThingUID = new ThingUID(newUID);
             } catch (IllegalArgumentException e) {
-                logger.warn("Cannot create thing: {}", e.getMessage());
-                return null;
+                throw new IllegalArgumentException("Invalid thing UID " + newUID, e);
             }
         }
         Thing newThing = ThingFactory.createThing(newThingUID, config, properties, result.getBridgeUID(), thingTypeUID,
diff --git a/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/internal/PersistentInboxTest.java b/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/internal/PersistentInboxTest.java
index 2b364a887b7..8d1529064cf 100644
--- a/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/internal/PersistentInboxTest.java
+++ b/bundles/org.openhab.core.config.discovery/src/test/java/org/openhab/core/config/discovery/internal/PersistentInboxTest.java
@@ -172,7 +172,15 @@ public void testApproveWithInvalidThingId() throws URISyntaxException {
         when(storage.getValues()).thenReturn(List.of(result));
 
         inbox.activate();
-        assertNull(inbox.approve(THING_UID, "Test", "invalid:id"));
+        Exception exception = assertThrows(IllegalArgumentException.class, () -> {
+            inbox.approve(THING_UID, "Test", "invalid:id");
+        });
+        assertEquals("New Thing ID invalid:id must not contain multiple segments", exception.getMessage());
+
+        exception = assertThrows(IllegalArgumentException.class, () -> {
+            inbox.approve(THING_UID, "Test", "invalid$id");
+        });
+        assertEquals("Invalid thing UID test:test:invalid$id", exception.getMessage());
     }
 
     @Test