From f3b4693081ed2bd7194526e902b80278aec92edc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A1s=20Salamon?= Date: Tue, 1 Jun 2021 19:50:10 +0200 Subject: [PATCH] SOLR-15421: ConfigSet existence now checks for solrconfig.xml (#135) ConfigSet API now checks for solrconfig.xml when checking for the existence of a configset --- solr/CHANGES.txt | 2 ++ .../java/org/apache/solr/cloud/ConfigSetCmds.java | 4 ---- .../org/apache/solr/cloud/ZkConfigSetService.java | 6 +++--- .../solr/core/FileSystemConfigSetService.java | 4 ++-- .../OverseerCollectionConfigSetProcessorTest.java | 3 ++- .../org/apache/solr/cloud/TestConfigSetsAPI.java | 13 ++++++------- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f0fc29bf4a84..eed141042b85 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -129,6 +129,8 @@ when told to. The admin UI now tells it to. (Nazerke Seidan, David Smiley) * SOLR-15414: Use ConfigSet API instead of Zookeeper data node to list out configsets available in Solr Admin UI. (Nazerke Seidan via Eric Pugh) +* SOLR-15421: ConfigSet API also checks for solrconfig.xml when checking the existence of a configset (Andras Salamon via David Smiley) + Other Changes ---------------------- * SOLR-14656: Autoscaling framework removed (Ishan Chattopadhyaya, noble, Ilan Ginzburg) diff --git a/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java b/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java index 5d5983a5f0c6..8568f1493acb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java +++ b/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java @@ -184,10 +184,6 @@ public static void deleteConfigSet(ZkNodeProps message, CoreContainer coreContai } private static void deleteConfigSet(String configSetName, boolean force, CoreContainer coreContainer) throws IOException { - if (!coreContainer.getConfigSetService().checkConfigExists(configSetName)) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "ConfigSet does not exist to delete: " + configSetName); - } - ZkStateReader zkStateReader = coreContainer.getZkController().getZkStateReader(); for (Map.Entry entry : zkStateReader.getClusterState().getCollectionsMap().entrySet()) { diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java index 5c0c22b9a36e..3db4efe5932f 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -142,9 +142,9 @@ public String configSetName(CoreDescriptor cd) { @Override public boolean checkConfigExists(String configName) throws IOException { try { - Boolean existsConfig = zkClient.exists(CONFIGS_ZKNODE + "/" + configName, true); - if (existsConfig == null) return false; - return existsConfig; + Boolean existsSolrConfigXml = zkClient.exists(CONFIGS_ZKNODE + "/" + configName + "/solrconfig.xml", true); + if (existsSolrConfigXml == null) return false; + return existsSolrConfigXml; } catch (KeeperException | InterruptedException e) { throw new IOException("Error checking whether config exists", SolrZkClient.checkInterrupted(e)); diff --git a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java index 9db6245a36d1..a4029a207431 100644 --- a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java @@ -61,8 +61,8 @@ public String configSetName(CoreDescriptor cd) { @Override public boolean checkConfigExists(String configName) throws IOException { - Path configSetDirectory = configSetBase.resolve(configName); - return Files.isDirectory(configSetDirectory); + Path solrConfigXmlFile= configSetBase.resolve(configName).resolve("solrconfig.xml"); + return Files.exists(solrConfigXmlFile); } @Override diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java index 82e600ecb943..ab6ac68473b7 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java @@ -547,7 +547,8 @@ public Void answer(InvocationOnMock invocation) { return null; }}).when(distribStateManagerMock).makePath(anyString()); - zkClientData.put("/configs/myconfig", new byte[1]); + zkClientData.put("/configs/"+CONFIG_NAME, new byte[1]); + zkClientData.put("/configs/"+CONFIG_NAME+"/solrconfig.xml", new byte[1]); when(solrMetricsContextMock.getChildContext(any(Object.class))).thenReturn(solrMetricsContextMock); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java index 1af32d27284d..45c5806aaaee 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java @@ -1121,10 +1121,6 @@ public void testDeleteErrors() throws Exception { DeleteNoErrorChecking delete = new DeleteNoErrorChecking(); verifyException(solrClient, delete, NAME); - // ConfigSet doesn't exist - delete.setConfigSetName("configSetBogus"); - verifyException(solrClient, delete, "ConfigSet does not exist"); - // ConfigSet is immutable delete.setConfigSetName("configSet"); verifyException(solrClient, delete, "Requested delete of immutable ConfigSet"); @@ -1148,11 +1144,16 @@ public void testDelete() throws Exception { final SolrClient solrClient = getHttpSolrClient(baseUrl); final String configSet = "testDelete"; getConfigSetService().uploadConfig(configSet, configset("configset-2")); + assertDelete(solrClient, configSet, true); + assertDelete(solrClient, "configSetBogus", false); + solrClient.close(); + } + private void assertDelete(SolrClient solrClient, String configSet, boolean assertExists) throws IOException, SolrServerException { SolrZkClient zkClient = new SolrZkClient(cluster.getZkServer().getZkAddress(), AbstractZkTestCase.TIMEOUT, AbstractZkTestCase.TIMEOUT, null); try { - assertTrue(getConfigSetService().checkConfigExists(configSet)); + assertEquals(assertExists, getConfigSetService().checkConfigExists(configSet)); Delete delete = new Delete(); delete.setConfigSetName(configSet); @@ -1162,8 +1163,6 @@ public void testDelete() throws Exception { } finally { zkClient.close(); } - - solrClient.close(); } @Test