diff --git a/changelog/unreleased/SOLR-18078.yml b/changelog/unreleased/SOLR-18078.yml new file mode 100644 index 000000000000..11a8cb1508cc --- /dev/null +++ b/changelog/unreleased/SOLR-18078.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Support deleting implictly created RequestHandlers through ConfigAPI +type: added # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Eric Pugh +links: + - name: SOLR-18078 + url: https://issues.apache.org/jira/browse/SOLR-18078 diff --git a/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java b/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java index 811ea865cd60..f575697682f8 100644 --- a/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java +++ b/solr/core/src/java/org/apache/solr/core/ConfigOverlay.java @@ -23,6 +23,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.solr.common.MapSerializable; import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CoreAdminParams; @@ -38,6 +39,7 @@ public class ConfigOverlay implements MapSerializable { private final Map data; private Map props; private Map userProps; + private final Set deletedPlugins; @SuppressWarnings({"unchecked"}) public ConfigOverlay(Map jsonObj, int version) { @@ -48,6 +50,13 @@ public ConfigOverlay(Map jsonObj, int version) { if (props == null) props = Collections.emptyMap(); userProps = (Map) data.get("userProps"); if (userProps == null) userProps = Collections.emptyMap(); + + List deleted = (List) data.get("deleted"); + if (deleted == null) { + deletedPlugins = Collections.emptySet(); + } else { + deletedPlugins = Set.copyOf(deleted); + } } public Object getXPathProperty(String xpath) { @@ -262,12 +271,53 @@ public ConfigOverlay addNamedPlugin(Map info, String typ) { @SuppressWarnings({"unchecked"}) public ConfigOverlay deleteNamedPlugin(String name, String typ) { Map dataCopy = Utils.getDeepCopy(data, 4); + + // Remove from overlay if present Map reqHandler = (Map) dataCopy.get(typ); - if (reqHandler == null) return this; - reqHandler.remove(name); + if (reqHandler != null) { + reqHandler.remove(name); + } + + // Add to deleted set (tombstone marker) + List deleted = (List) dataCopy.get("deleted"); + if (deleted == null) { + deleted = new ArrayList<>(); + dataCopy.put("deleted", deleted); + } else { + // Make a copy since the list might be immutable + deleted = new ArrayList<>(deleted); + dataCopy.put("deleted", deleted); + } + + String deletionKey = typ + ":" + name; + if (!deleted.contains(deletionKey)) { + deleted.add(deletionKey); + } + return new ConfigOverlay(dataCopy, this.version); } + /** + * Checks if a plugin has been marked as deleted in the overlay. + * + * @param typ the plugin type (e.g., "requestHandler") + * @param name the plugin name (e.g., "/update/json") + * @return true if the plugin is marked as deleted + */ + public boolean isPluginDeleted(String typ, String name) { + String deletionKey = typ + ":" + name; + return deletedPlugins.contains(deletionKey); + } + + /** + * Gets the set of all deleted plugin keys in the format "type:name". + * + * @return an unmodifiable set of deleted plugin keys + */ + public Set getDeletedPlugins() { + return deletedPlugins; + } + public static final String ZNODEVER = "znodeVersion"; public static final String NAME = "overlay"; } diff --git a/solr/core/src/java/org/apache/solr/core/PluginBag.java b/solr/core/src/java/org/apache/solr/core/PluginBag.java index 7624a4c13ddd..9167dd1b3e64 100644 --- a/solr/core/src/java/org/apache/solr/core/PluginBag.java +++ b/solr/core/src/java/org/apache/solr/core/PluginBag.java @@ -349,9 +349,27 @@ void init(Map defaults, SolrCore solrCore, List infos) { infos.stream().map(i -> i.name).collect(Collectors.toList())); } } + + // Get the ConfigOverlay to check for deleted plugins + ConfigOverlay overlay = solrCore.getSolrConfig().getOverlay(); + + // Register default plugins, but skip those marked as deleted in the overlay for (Map.Entry e : defaults.entrySet()) { - if (!contains(e.getKey())) { - put(e.getKey(), new PluginHolder<>(null, e.getValue())); + String pluginName = e.getKey(); + + // Check if this default plugin has been marked as deleted + if (overlay.isPluginDeleted(meta.getCleanTag(), pluginName)) { + if (log.isDebugEnabled()) { + log.debug( + "Skipping default {} '{}' because it is marked as deleted in ConfigOverlay", + meta.getCleanTag(), + pluginName); + } + continue; + } + + if (!contains(pluginName)) { + put(pluginName, new PluginHolder<>(null, e.getValue())); } } } diff --git a/solr/core/src/java/org/apache/solr/core/RequestHandlers.java b/solr/core/src/java/org/apache/solr/core/RequestHandlers.java index dca7c832e3fa..c8ef20ad485a 100644 --- a/solr/core/src/java/org/apache/solr/core/RequestHandlers.java +++ b/solr/core/src/java/org/apache/solr/core/RequestHandlers.java @@ -104,10 +104,17 @@ public PluginBag getRequestHandlers() { */ void initHandlersFromConfig(SolrConfig config) { List implicits = core.getImplicitHandlers(); + ConfigOverlay overlay = config.getOverlay(); + // use link map so we iterate in the same order Map infoMap = new LinkedHashMap<>(); - // deduping implicit and explicit requesthandlers - for (PluginInfo info : implicits) infoMap.put(info.name, info); + // deduping implicit and explicit requesthandlers, and filtering out deleted ones + for (PluginInfo info : implicits) { + // Skip implicit handlers that have been marked as deleted in the overlay + if (!overlay.isPluginDeleted(SolrRequestHandler.TYPE, info.name)) { + infoMap.put(info.name, info); + } + } for (PluginInfo info : config.getPluginInfos(SolrRequestHandler.class.getName())) infoMap.put(info.name, info); ArrayList infos = new ArrayList<>(infoMap.values()); diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java index cba47bad98d2..e06e6497b408 100644 --- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java @@ -615,12 +615,34 @@ private ConfigOverlay deleteNamedComponent( CommandOperation op, ConfigOverlay overlay, String typ) { String name = op.getStr(CommandOperation.ROOT_OBJ); if (op.hasError()) return overlay; + + // Check if it exists in the overlay if (overlay.getNamedPlugins(typ).containsKey(name)) { return overlay.deleteNamedPlugin(name, typ); - } else { - op.addError(formatString("NO such {0} ''{1}'' ", typ, name)); - return overlay; } + + // Check if it's an implicit handler (for requestHandler type) + if (SolrRequestHandler.TYPE.equals(typ)) { + List implicitHandlers = req.getCore().getImplicitHandlers(); + for (PluginInfo pluginInfo : implicitHandlers) { + if (name.equals(pluginInfo.name)) { + // It's an implicit handler, so we can delete it by adding a tombstone marker + return overlay.deleteNamedPlugin(name, typ); + } + } + } + + // Check if it's a default response writer (for queryResponseWriter type) + if ("queryResponseWriter".equals(typ)) { + if (SolrCore.DEFAULT_RESPONSE_WRITERS.containsKey(name)) { + // It's a default response writer, so we can delete it by adding a tombstone marker + return overlay.deleteNamedPlugin(name, typ); + } + } + + // Not found anywhere + op.addError(formatString("NO such {0} ''{1}'' ", typ, name)); + return overlay; } private ConfigOverlay updateNamedPlugin( @@ -663,6 +685,11 @@ private ConfigOverlay updateNamedPlugin( private boolean pluginExists( SolrConfig.SolrPluginInfo info, ConfigOverlay overlay, String name) { + // Don't consider deleted plugins as existing + if (overlay.isPluginDeleted(info.getCleanTag(), name)) { + return false; + } + List l = req.getCore().getSolrConfig().getPluginInfos(info.clazz.getName()); for (PluginInfo pluginInfo : l) if (name.equals(pluginInfo.name)) return true; return overlay.getNamedPlugins(info.getCleanTag()).containsKey(name); diff --git a/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java b/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java index c12b6916e44e..fbd0365cd85a 100644 --- a/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java +++ b/solr/core/src/test/org/apache/solr/core/TestConfigOverlay.java @@ -19,8 +19,12 @@ import static org.apache.solr.core.ConfigOverlay.isEditableProp; import java.util.Collections; +import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.solr.SolrTestCase; +import org.apache.solr.common.params.CoreAdminParams; public class TestConfigOverlay extends SolrTestCase { @@ -64,4 +68,129 @@ public void testSetProperty() { assertEquals(1, map.size()); assertEquals(100, map.get("initialSize")); } + + public void testDeletedPluginTombstone() { + ConfigOverlay overlay = new ConfigOverlay(Collections.emptyMap(), 0); + + // Initially, no plugins should be deleted + assertFalse(overlay.isPluginDeleted("requestHandler", "/update/json")); + assertEquals(0, overlay.getDeletedPlugins().size()); + + // Delete a plugin + overlay = overlay.deleteNamedPlugin("/update/json", "requestHandler"); + + // Verify the plugin is marked as deleted + assertTrue(overlay.isPluginDeleted("requestHandler", "/update/json")); + Set deleted = overlay.getDeletedPlugins(); + assertEquals(1, deleted.size()); + assertTrue(deleted.contains("requestHandler:/update/json")); + + // Delete another plugin + overlay = overlay.deleteNamedPlugin("/sql", "requestHandler"); + assertTrue(overlay.isPluginDeleted("requestHandler", "/sql")); + assertEquals(2, overlay.getDeletedPlugins().size()); + + // Verify the first one is still deleted + assertTrue(overlay.isPluginDeleted("requestHandler", "/update/json")); + } + + public void testDeletedPluginFromOverlay() { + // Create an overlay with a custom request handler + Map data = new HashMap<>(); + Map requestHandlers = new HashMap<>(); + Map handlerConfig = new HashMap<>(); + handlerConfig.put(CoreAdminParams.NAME, "/custom"); + handlerConfig.put("class", "solr.DumpRequestHandler"); + requestHandlers.put("/custom", handlerConfig); + data.put("requestHandler", requestHandlers); + + ConfigOverlay overlay = new ConfigOverlay(data, 0); + + // Verify the handler exists in overlay + assertEquals(1, overlay.getNamedPlugins("requestHandler").size()); + + // Delete it + overlay = overlay.deleteNamedPlugin("/custom", "requestHandler"); + + // Verify it's removed from overlay and added to deleted list + assertEquals(0, overlay.getNamedPlugins("requestHandler").size()); + assertTrue(overlay.isPluginDeleted("requestHandler", "/custom")); + } + + public void testDeletedPluginPersistence() { + // Create an overlay with deleted plugins in the data + Map data = new HashMap<>(); + List deleted = List.of("requestHandler:/update/json", "requestHandler:/sql"); + data.put("deleted", deleted); + + ConfigOverlay overlay = new ConfigOverlay(data, 0); + + // Verify deleted plugins are loaded correctly + assertTrue(overlay.isPluginDeleted("requestHandler", "/update/json")); + assertTrue(overlay.isPluginDeleted("requestHandler", "/sql")); + assertEquals(2, overlay.getDeletedPlugins().size()); + } + + public void testDeleteSamePluginTwice() { + ConfigOverlay overlay = new ConfigOverlay(Collections.emptyMap(), 0); + + // Delete a plugin + overlay = overlay.deleteNamedPlugin("/update/json", "requestHandler"); + assertTrue(overlay.isPluginDeleted("requestHandler", "/update/json")); + assertEquals(1, overlay.getDeletedPlugins().size()); + + // Delete the same plugin again + overlay = overlay.deleteNamedPlugin("/update/json", "requestHandler"); + + // Should still only have one entry + assertTrue(overlay.isPluginDeleted("requestHandler", "/update/json")); + assertEquals(1, overlay.getDeletedPlugins().size()); + } + + public void testDeleteResponseWriter() { + ConfigOverlay overlay = new ConfigOverlay(Collections.emptyMap(), 0); + + // Initially, no writers should be deleted + assertFalse(overlay.isPluginDeleted("queryResponseWriter", "xml")); + assertFalse(overlay.isPluginDeleted("queryResponseWriter", "json")); + + // Delete a response writer + overlay = overlay.deleteNamedPlugin("xml", "queryResponseWriter"); + + // Verify the writer is marked as deleted + assertTrue(overlay.isPluginDeleted("queryResponseWriter", "xml")); + Set deleted = overlay.getDeletedPlugins(); + assertEquals(1, deleted.size()); + assertTrue(deleted.contains("queryResponseWriter:xml")); + + // Delete another writer + overlay = overlay.deleteNamedPlugin("csv", "queryResponseWriter"); + assertTrue(overlay.isPluginDeleted("queryResponseWriter", "csv")); + assertEquals(2, overlay.getDeletedPlugins().size()); + + // Verify both are still deleted + assertTrue(overlay.isPluginDeleted("queryResponseWriter", "xml")); + assertTrue(overlay.isPluginDeleted("queryResponseWriter", "csv")); + } + + public void testDeleteMixedPluginTypes() { + ConfigOverlay overlay = new ConfigOverlay(Collections.emptyMap(), 0); + + // Delete different plugin types + overlay = overlay.deleteNamedPlugin("/update", "requestHandler"); + overlay = overlay.deleteNamedPlugin("json", "queryResponseWriter"); + overlay = overlay.deleteNamedPlugin("mycomponent", "searchComponent"); + + // Verify all are marked as deleted + assertTrue(overlay.isPluginDeleted("requestHandler", "/update")); + assertTrue(overlay.isPluginDeleted("queryResponseWriter", "json")); + assertTrue(overlay.isPluginDeleted("searchComponent", "mycomponent")); + + // Verify the count + assertEquals(3, overlay.getDeletedPlugins().size()); + + // Verify they don't interfere with each other + assertFalse(overlay.isPluginDeleted("requestHandler", "json")); + assertFalse(overlay.isPluginDeleted("queryResponseWriter", "/update")); + } } diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java index c0efb0c32746..5221354f0bb6 100644 --- a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java +++ b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java @@ -1060,4 +1060,139 @@ public void testSetPropertyEnableAndDisableCache() throws Exception { restTestHarness.setServerProvider(oldProvider); } + + public void testDeleteImplicitHandler() throws Exception { + RestTestHarness harness = restTestHarness; + + // First verify the implicit handler /admin/ping exists in the config + testForResponseElement( + harness, + null, + "/config", + null, + asList("config", "requestHandler", "/admin/ping", "class"), + "solr.PingRequestHandler", + TIMEOUT_S); + + // Delete the implicit handler + String payload = "{\n" + "'delete-requesthandler' : '/admin/ping'" + "}"; + runConfigCommand(harness, "/config", payload); + + // Verify it's in the deleted list in the overlay + boolean success = false; + long startTime = System.nanoTime(); + int maxTimeoutSeconds = 10; + while (TimeUnit.SECONDS.convert(System.nanoTime() - startTime, TimeUnit.NANOSECONDS) + < maxTimeoutSeconds) { + String uri = "/config/overlay"; + Map m = getRespMap(uri, harness); + + // The overlay response has structure: {responseHeader: {...}, overlay: {...}} + // We need to look inside the "overlay" key + Map overlayData = (Map) m.get("overlay"); + if (overlayData == null) { + var keys = m.keySet(); + log.info("No overlay key found in response. Keys: {}", keys); + Thread.sleep(100); + continue; + } + + Object deletedObj = overlayData.get("deleted"); + + if (deletedObj instanceof List) { + List deleted = (List) deletedObj; + log.info("Deleted list contains: {}", deleted); + if (deleted.contains("requestHandler:/admin/ping")) { + success = true; + break; + } + } + Thread.sleep(100); + } + assertTrue("Implicit handler should be marked as deleted in overlay", success); + + // Try to recreate the deleted handler - should succeed since it's now deleted + payload = + "{\n" + + "'create-requesthandler' : { 'name' : '/admin/ping', " + + "'class': 'org.apache.solr.handler.DumpRequestHandler' }\n" + + "}"; + runConfigCommand(harness, "/config", payload); + + // Verify the handler was recreated in the overlay + testForResponseElement( + harness, + null, + "/config/overlay", + null, + asList("overlay", "requestHandler", "/admin/ping", "class"), + "org.apache.solr.handler.DumpRequestHandler", + TIMEOUT_S); + + // Clean up - delete the recreated handler + payload = "{\n" + "'delete-requesthandler' : '/admin/ping'" + "}"; + runConfigCommand(harness, "/config", payload); + } + + public void testDeleteDefaultResponseWriter() throws Exception { + RestTestHarness harness = restTestHarness; + + // Verify the default response writer "xml" exists in DEFAULT_RESPONSE_WRITERS + // We can't directly test if it works without making a query, but we can verify + // the deletion marker is added to the overlay + + // Delete the default "xml" response writer + String payload = "{\n" + "'delete-queryresponsewriter' : 'xml'" + "}"; + runConfigCommand(harness, "/config", payload); + + // Verify it's in the deleted list in the overlay + boolean success = false; + long startTime = System.nanoTime(); + int maxTimeoutSeconds = 10; + while (TimeUnit.SECONDS.convert(System.nanoTime() - startTime, TimeUnit.NANOSECONDS) + < maxTimeoutSeconds) { + String uri = "/config/overlay"; + Map m = getRespMap(uri, harness); + + // The overlay response has structure: {responseHeader: {...}, overlay: {...}} + Map overlayData = (Map) m.get("overlay"); + if (overlayData == null) { + Thread.sleep(100); + continue; + } + + Object deletedObj = overlayData.get("deleted"); + + if (deletedObj instanceof List) { + List deleted = (List) deletedObj; + if (deleted.contains("queryResponseWriter:xml")) { + success = true; + break; + } + } + Thread.sleep(100); + } + assertTrue("Default response writer should be marked as deleted in overlay", success); + + // Try to recreate the deleted writer - should succeed since it's now deleted + payload = "{\n" + + "'create-queryresponsewriter' : { 'name' : 'xml', " + + "'class': 'solr.XMLResponseWriter' }\n" + + "}"; + runConfigCommand(harness, "/config", payload); + + // Verify the writer was recreated in the overlay + testForResponseElement( + harness, + null, + "/config/overlay", + null, + asList("overlay", "queryResponseWriter", "xml", "class"), + "solr.XMLResponseWriter", + TIMEOUT_S); + + // Clean up - delete the recreated writer + payload = "{\n" + "'delete-queryresponsewriter' : 'xml'" + "}"; + runConfigCommand(harness, "/config", payload); + } } diff --git a/solr/solr-ref-guide/modules/configuration-guide/pages/config-api.adoc b/solr/solr-ref-guide/modules/configuration-guide/pages/config-api.adoc index 27bc358782bb..60633a164a12 100644 --- a/solr/solr-ref-guide/modules/configuration-guide/pages/config-api.adoc +++ b/solr/solr-ref-guide/modules/configuration-guide/pages/config-api.adoc @@ -144,7 +144,7 @@ The Config API commands for modifications are categorized into 3 types, each of These types are: * `set-property` and `unset-property` for <> -* Component-specific `add-`, `update-`, and `delete-` commands for <> +* Component-specific `add-`, `update-`, and `delete-` commands for <> * `set-user-property` and `unset-user-property` for <> === Commands for Common Properties @@ -324,7 +324,7 @@ In each case, `add-` commands add a new configuration to `configoverlay.json`, w `update-` commands overwrite an existing setting in `configoverlay.json`. -`delete-` commands remove the setting from `configoverlay.json`. +`delete-` commands remove the setting from `configoverlay.json` or for components that ship with Solr marks it as deleted in `configoverlay.json`. Settings removed from `configoverlay.json` are not removed from `solrconfig.xml` if they happen to be duplicated there.