Skip to content

Item metadata REST endpoints: don't allow adding or removing semantics #5390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
holgerfriedrich merged 19 commits into from
Mar 21, 2026
Merged

Item metadata REST endpoints: don't allow adding or removing semantics #5390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
da78b82
reserve semantics namespace
mherwege Feb 24, 2026
abab090
spotless
mherwege Feb 24, 2026
2648c75
fix test
mherwege Feb 24, 2026
8b392f7
fix javadoc
mherwege Feb 24, 2026
fcb7522
fix javadoc
mherwege Feb 24, 2026
0178239
copilot review adjustments
mherwege Mar 3, 2026
355e1d3
fix test
mherwege Mar 3, 2026
6f1ea7c
fix test
mherwege Mar 3, 2026
97da6e1
copilot feedback
mherwege Mar 3, 2026
1301f56
fix deleting all item metadata
mherwege Mar 3, 2026
5e3a4a0
add tests remove all metadata
mherwege Mar 3, 2026
5b25110
fix update for unmanaged and unreserved
mherwege Mar 3, 2026
6c4d37d
eliminate compiler warnings
mherwege Mar 3, 2026
9ba5b7f
handle multiple reservations for same namespace
mherwege Mar 4, 2026
f962ed2
Change log level to debug
mherwege Mar 4, 2026
10267ae
copilot feedback
mherwege Mar 4, 2026
de19943
handle exceptions in metadata console commands
mherwege Mar 4, 2026
39e195a
update javadoc
mherwege Mar 4, 2026
81b14fd
fix console remove return text
mherwege Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 39 additions & 12 deletions .../java/org/openhab/core/io/console/internal/extension/MetadataConsoleCommandExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,21 @@ private void addMetadata(Console console, String itemName, String namespace, Str
MetadataKey key = new MetadataKey(namespace, itemName);
Map<String, Object> configMap = getConfigMap(config);
Metadata metadata = new Metadata(key, value, configMap);
if (metadataRegistry.get(key) != null) {
metadataRegistry.update(metadata);
console.println("Updated: " + metadata);
} else {
metadataRegistry.add(metadata);
console.println("Added: " + metadata);
try {
if (metadataRegistry.get(key) == null) {
metadataRegistry.add(metadata);
console.println("Added: " + metadata);
} else {
if (metadataRegistry.update(metadata) == null) {
console.println("Cannot update metadata in unmanaged provider: " + metadata);
} else {
console.println("Updated: " + metadata);
}
}
} catch (UnsupportedOperationException e) {
console.println("Namespace reserved in unmanaged provider: " + metadata);
} catch (IllegalStateException e) {
console.println("No managed provider available for: " + metadata);
}
}
}
Expand Down Expand Up @@ -190,11 +199,25 @@ private void removeMetadata(Console console, String itemName, @Nullable String n
}

private void removeMetadata(Console console, MetadataKey key) {
Metadata metadata = metadataRegistry.remove(key);
if (metadata != null) {
console.println("Removed: " + metadata);
} else {
console.println("Metadata element for " + key + " could not be found.");
try {
if (metadataRegistry.get(key) != null) {
Metadata removedMetadata = metadataRegistry.remove(key);
if (removedMetadata != null) {
console.println("Removed: " + removedMetadata);
} else {
if (metadataRegistry.get(key) != null) {
console.println("Unmanaged metadata element for " + key + ", could not be removed.");
} else {
console.println("Metadata element for " + key + " could not be found.");
}
}
Comment thread
mherwege marked this conversation as resolved.
} else {
console.println("Metadata element for " + key + " could not be found.");
}
} catch (UnsupportedOperationException e) {
console.println("Unmanaged metadata element for " + key + " in reserved namespace, could not be removed.");
} catch (IllegalStateException e) {
console.println("No managed provider available for metadata with key: " + key);
}
}

Expand All @@ -205,7 +228,11 @@ private void orphan(Console console, String action, Collection<Metadata> metadat
if (!itemNames.contains(md.getUID().getItemName())) {
console.println("Item missing: " + md.getUID());
if ("purge".equals(action)) {
metadataRegistry.remove(md.getUID());
try {
metadataRegistry.remove(md.getUID());
} catch (UnsupportedOperationException | IllegalStateException e) {
// ignore metadata that cannot be removed
}
}
}
});
Expand Down
99 changes: 69 additions & 30 deletions ....io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/item/ItemResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.gson.Gson;
import com.google.gson.JsonObject;

import io.swagger.v3.oas.annotations.Operation;
Expand Down Expand Up @@ -182,8 +181,6 @@ private static void respectForwarded(final UriBuilder uriBuilder, final @Context
}

private final Logger logger = LoggerFactory.getLogger(ItemResource.class);
private final Gson gson = new Gson();

private final DTOMapper dtoMapper;
private final EventPublisher eventPublisher;
private final ItemBuilderFactory itemBuilderFactory;
Expand Down Expand Up @@ -500,7 +497,7 @@ private Response sendItemStateInternal(@Nullable String language, String itemNam
final Locale locale = localeService.getLocale(language);
final ZoneId zoneId = timeZoneProvider.getTimeZone();

source = buildSource(source, securityContext);
String eventSource = buildSource(source, securityContext);

// get Item
Item item = getItem(itemName);
Expand All @@ -512,7 +509,7 @@ private Response sendItemStateInternal(@Nullable String language, String itemNam

if (state != null) {
// set State and report OK
eventPublisher.post(ItemEventFactory.createStateEvent(itemName, state, source));
eventPublisher.post(ItemEventFactory.createStateEvent(itemName, state, eventSource));
return getItemResponse(null, Status.ACCEPTED, null, locale, zoneId, null);
} else {
// State could not be parsed
Expand Down Expand Up @@ -566,7 +563,7 @@ private Response sendItemCommandInternal(String itemName, String value, @Nullabl
SecurityContext securityContext) {
Item item = getItem(itemName);
Command command = null;
source = buildSource(source, securityContext);
String eventSource = buildSource(source, securityContext);
if (item != null) {
if ("toggle".equalsIgnoreCase(value) && (item instanceof SwitchItem || item instanceof RollershutterItem)) {
if (OnOffType.ON.equals(item.getStateAs(OnOffType.class))) {
Expand All @@ -585,7 +582,7 @@ private Response sendItemCommandInternal(String itemName, String value, @Nullabl
command = TypeParser.parseCommand(item.getAcceptedCommandTypes(), value);
}
if (command != null) {
eventPublisher.post(ItemEventFactory.createCommandEvent(itemName, command, source));
eventPublisher.post(ItemEventFactory.createCommandEvent(itemName, command, eventSource));
ResponseBuilder resbuilder = Response.ok();
resbuilder.type(MediaType.TEXT_PLAIN);
return resbuilder.build();
Expand Down Expand Up @@ -750,7 +747,8 @@ public Response removeTag(@PathParam("itemName") @Parameter(description = "item
@ApiResponse(responseCode = "200", description = "OK"), //
@ApiResponse(responseCode = "201", description = "Created"), //
@ApiResponse(responseCode = "404", description = "Item not found."), //
@ApiResponse(responseCode = "405", description = "Metadata not editable.") })
@ApiResponse(responseCode = "405", description = "Metadata not editable."),
@ApiResponse(responseCode = "503", description = "Managed provider not available.") })
public Response addMetadata(@PathParam("itemName") @Parameter(description = "item name") String itemName,
@PathParam("namespace") @Parameter(description = "namespace") String namespace,
@Parameter(description = "metadata", required = true) MetadataDTO metadata) {
Expand All @@ -767,45 +765,81 @@ public Response addMetadata(@PathParam("itemName") @Parameter(description = "ite

MetadataKey key = new MetadataKey(namespace, itemName);
Metadata md = new Metadata(key, value, metadata.config);
if (metadataRegistry.get(key) == null) {
metadataRegistry.add(md);
return Response.status(Status.CREATED).type(MediaType.TEXT_PLAIN).build();
} else {
metadataRegistry.update(md);
return Response.ok(null, MediaType.TEXT_PLAIN).build();
try {
if (metadataRegistry.get(key) == null) {
metadataRegistry.add(md);
return Response.status(Status.CREATED).type(MediaType.TEXT_PLAIN).build();
} else {
if (metadataRegistry.update(md) == null) {

Copilot AI Mar 3, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metadataRegistry.get(key) and metadataRegistry.update(md) are not atomic. If the metadata existed at the time of get but is removed concurrently before update, update can return null and this code will return 405 (“not editable”), which is misleading (it’s actually gone). A safer approach is to re-check metadataRegistry.get(key) when update returns null and return 404 if it’s now absent, reserving 405 for the “exists but not managed/editable” case.

Suggested change
if (metadataRegistry.update(md) == null) {
Metadata previous = metadataRegistry.update(md);
if (previous == null) {
// Metadata may have been removed concurrently; re-check existence
if (metadataRegistry.get(key) == null) {
return Response.status(Status.NOT_FOUND).build();
}

Copilot uses AI. Check for mistakes.

@mherwege mherwege Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good idea. You try to add. Responding you are not adding because the metadata does not exist is not what you would expect. You could try to add again, but you get in a potential loop. I prefer the current not allowed.

// Exists, but not managed
return Response.status(Status.METHOD_NOT_ALLOWED).build();
}
return Response.ok(null, MediaType.TEXT_PLAIN).build();
}
} catch (UnsupportedOperationException e) {
// Trying to add to a reserved namespace that is in an unmanaged provider
return JSONResponse.createErrorResponse(Status.METHOD_NOT_ALLOWED, e.getMessage());
} catch (IllegalStateException e) {
// There is no managed provider available
return Response.status(Status.SERVICE_UNAVAILABLE).build();
}
Comment on lines +768 to 785

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching all IllegalStateException here will also convert unrelated registry failures (e.g., ManagedProvider is not available thrown by AbstractRegistry.add/update/remove) into a 405, which can mask real server/configuration problems. It would be safer to throw/catch a dedicated exception type for “reserved namespace not editable” (e.g., UnsupportedOperationException or a custom runtime exception) and let other IllegalStateExceptions propagate/return a 500-style error.

Copilot uses AI. Check for mistakes.

@mherwege mherwege Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using UnsupportedOperationException now. But looking at the previous code, while the API documentation said it was returning 405 if the metadata was not editable, it did not do that. The IllegalStateException was also never catched and not returned by the REST API. Not having a managed provider should be an acceptable state. It just means nothing can be edited through the REST API or UI. But it should be handled in my opinion.

Comment thread
mherwege marked this conversation as resolved.
}

@DELETE
@RolesAllowed({ Role.ADMIN })
@Path("/{itemName: [a-zA-Z_0-9]+}/metadata")
@Operation(operationId = "removeAllMetadataFromItem", summary = "Removes all managed metadata from an item.", security = {
@SecurityRequirement(name = "oauth2", scopes = { "admin" }) }, responses = {
@ApiResponse(responseCode = "200", description = "OK"),
@ApiResponse(responseCode = "404", description = "Item not found.") })
public Response removeAllMetadata(@PathParam("itemName") @Parameter(description = "item name") String itemName) {
Item item = getItem(itemName);

if (item == null) {
return Response.status(Status.NOT_FOUND).build();
}

metadataRegistry.removeItemMetadata(itemName);
return Response.ok(null, MediaType.TEXT_PLAIN).build();
}

@DELETE
@RolesAllowed({ Role.ADMIN })
@Path("/{itemName: [a-zA-Z_0-9]+}/metadata/{namespace}")
@Operation(operationId = "removeMetadataFromItem", summary = "Removes metadata from an item.", security = {
@Operation(operationId = "removeMetadataFromItem", summary = "Removes metadata in a specific namespace from an item.", security = {
@SecurityRequirement(name = "oauth2", scopes = { "admin" }) }, responses = {
@ApiResponse(responseCode = "200", description = "OK"),
@ApiResponse(responseCode = "404", description = "Item not found."),
@ApiResponse(responseCode = "405", description = "Meta data not editable.") })
@ApiResponse(responseCode = "404", description = "Item or namespace not found."),
@ApiResponse(responseCode = "405", description = "Metadata not editable."),
@ApiResponse(responseCode = "503", description = "Managed provider not available.") })
public Response removeMetadata(@PathParam("itemName") @Parameter(description = "item name") String itemName,
@Nullable @PathParam("namespace") @Parameter(description = "namespace") String namespace) {
@PathParam("namespace") @Parameter(description = "namespace") String namespace) {
Item item = getItem(itemName);

if (item == null) {
return Response.status(Status.NOT_FOUND).build();
}

if (namespace == null) {
metadataRegistry.removeItemMetadata(itemName);
} else {
MetadataKey key = new MetadataKey(namespace, itemName);
MetadataKey key = new MetadataKey(namespace, itemName);
try {
if (metadataRegistry.get(key) != null) {
if (metadataRegistry.remove(key) == null) {
return Response.status(Status.CONFLICT).build();
Metadata removedMetadata = metadataRegistry.remove(key);
if (removedMetadata != null) {
return Response.ok(null, MediaType.TEXT_PLAIN).build();
}
if (metadataRegistry.get(key) != null) {
// Exists, but not managed, and not removed in the mean time
return Response.status(Status.METHOD_NOT_ALLOWED).build();
}
} else {
return Response.status(Status.NOT_FOUND).build();
}
return Response.status(Status.NOT_FOUND).build();
} catch (UnsupportedOperationException e) {
// Trying to remove from a reserved namespace that is in an unmanaged provider
return JSONResponse.createErrorResponse(Status.METHOD_NOT_ALLOWED, e.getMessage());
} catch (IllegalStateException e) {
// There is no managed provider available
return Response.status(Status.SERVICE_UNAVAILABLE).build();
}

return Response.ok(null, MediaType.TEXT_PLAIN).build();
}

@POST
Expand All @@ -818,8 +852,13 @@ public Response purge() {
Collection<String> itemNames = itemRegistry.stream().map(Item::getName)
.collect(Collectors.toCollection(HashSet::new));

metadataRegistry.getAll().stream().filter(md -> !itemNames.contains(md.getUID().getItemName()))
.forEach(md -> metadataRegistry.remove(md.getUID()));
metadataRegistry.getAll().stream().filter(md -> !itemNames.contains(md.getUID().getItemName())).forEach(md -> {
try {
metadataRegistry.remove(md.getUID());
} catch (UnsupportedOperationException | IllegalStateException e) {
// ignore metadata that cannot be removed
}
});
return Response.ok().build();
}

Expand Down
6 changes: 6 additions & 0 deletions ...emantics/src/main/java/org/openhab/core/semantics/internal/SemanticsMetadataProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -108,6 +109,11 @@ public Collection<Metadata> getAll() {
return semantics.values();
}

@Override
public Collection<String> getReservedNamespaces() {
return Set.of(NAMESPACE);
}

/**
* Updates the semantic metadata for an item and notifies all listeners about changes
*
Expand Down
79 changes: 79 additions & 0 deletions .../org.openhab.core/src/main/java/org/openhab/core/internal/items/MetadataRegistryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@
package org.openhab.core.internal.items;

import java.util.Collection;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.common.registry.AbstractRegistry;
import org.openhab.core.common.registry.Provider;
import org.openhab.core.events.EventPublisher;
import org.openhab.core.items.ManagedMetadataProvider;
import org.openhab.core.items.Metadata;
Expand All @@ -31,18 +37,24 @@
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferenceCardinality;
import org.osgi.service.component.annotations.ReferencePolicy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This is the main implementing class of the {@link MetadataRegistry} interface. It
* keeps track of all declared metadata of all metadata providers.
*
* @author Kai Kreuzer - Initial contribution
* @author Mark Herwege - semantics namespace not in managed provider
*/
@Component(immediate = true, service = MetadataRegistry.class)
@NonNullByDefault
public class MetadataRegistryImpl extends AbstractRegistry<Metadata, MetadataKey, MetadataProvider>
implements MetadataRegistry {

private final Logger logger = LoggerFactory.getLogger(MetadataRegistryImpl.class);
private final Map<String, Set<MetadataProvider>> reservedNamespaces = new ConcurrentHashMap<>();

@Activate
public MetadataRegistryImpl(final @Reference ReadyService readyService) {
super(MetadataProvider.class);
Expand Down Expand Up @@ -114,4 +126,71 @@ public void removeItemMetadata(String itemName) {
getManagedProvider()
.ifPresent(managedProvider -> ((ManagedMetadataProvider) managedProvider).removeItemMetadata(itemName));
}

@Override
public Metadata add(Metadata element) {
String namespace = element.getUID().getNamespace();
Set<MetadataProvider> providers = reservedNamespaces.get(namespace);
MetadataProvider managedProvider = (MetadataProvider) getManagedProvider().orElse(null);
if (providers == null || providers.isEmpty() || providers.stream().anyMatch(p -> p.equals(managedProvider))) {
return super.add(element);
Comment thread
mherwege marked this conversation as resolved.
}
throw new UnsupportedOperationException("Cannot add metadata to '" + namespace + "' namespace");
}
Comment on lines +131 to +139

Copilot AI Mar 4, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MetadataRegistryImpl.add/update/remove now throw UnsupportedOperationException for reserved namespaces. There are existing callers that invoke metadataRegistry.remove(...)/update(...) without handling this (e.g., the REST /items/metadata/purge implementation and the metadata console commands), which will now fail with an uncaught runtime exception whenever they touch reserved namespaces like semantics. Consider avoiding exceptions here (e.g., return null for update/remove and block add via a separate check) or update all internal callers to catch UnsupportedOperationException and handle it gracefully.

Copilot uses AI. Check for mistakes.

@mherwege mherwege Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was throwing IllegalStateException before, which was also not handled.
I handle it now, and also updated the console commands to handle this properly.


@Override
public @Nullable Metadata update(Metadata element) {
String namespace = element.getUID().getNamespace();
Set<MetadataProvider> providers = reservedNamespaces.get(namespace);
MetadataProvider managedProvider = (MetadataProvider) getManagedProvider().orElse(null);
if (providers == null || providers.isEmpty() || providers.stream().anyMatch(p -> p.equals(managedProvider))) {
return super.update(element);
}
throw new UnsupportedOperationException("Cannot update metadata in '" + namespace + "' namespace");
}

@Override
public @Nullable Metadata remove(MetadataKey key) {
String namespace = key.getNamespace();
Set<MetadataProvider> providers = reservedNamespaces.get(namespace);
MetadataProvider managedProvider = (MetadataProvider) getManagedProvider().orElse(null);
if (providers == null || providers.isEmpty() || providers.stream().anyMatch(p -> p.equals(managedProvider))) {
return super.remove(key);
}
throw new UnsupportedOperationException("Cannot remove metadata from '" + namespace + "' namespace");
}

@Override
protected void addProvider(Provider<Metadata> provider) {
if (provider instanceof MetadataProvider metadataProvider) {
metadataProvider.getReservedNamespaces().stream().forEach(namespace -> {
Set<MetadataProvider> currentProviders = reservedNamespaces.getOrDefault(namespace, Set.of());
if (!currentProviders.isEmpty()) {
logger.debug("Multiple metadata providers are reserving namespace '{}', there should only be one.",
namespace);
}
Set<MetadataProvider> providers = Stream
.concat(currentProviders.stream(), Set.of(metadataProvider).stream())
.collect(Collectors.toSet());
reservedNamespaces.put(namespace, providers);
});
}
Comment thread
mherwege marked this conversation as resolved.
super.addProvider(provider);
}

@Override
protected void removeProvider(Provider<Metadata> provider) {
if (provider instanceof MetadataProvider metadataProvider) {
metadataProvider.getReservedNamespaces().stream().forEach(namespace -> {
Set<MetadataProvider> providers = reservedNamespaces.getOrDefault(namespace, Set.of()).stream()
.filter(p -> !provider.equals(p)).collect(Collectors.toSet());
if (providers.isEmpty()) {
reservedNamespaces.remove(namespace);
} else {
reservedNamespaces.put(namespace, providers);
}
});
}
Comment thread
mherwege marked this conversation as resolved.
Comment thread
mherwege marked this conversation as resolved.
super.removeProvider(provider);
}
}
Loading
Loading