You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2018/10/14 02:16:01 UTC
lucene-solr:master: SOLR-8808: Add null/empty check to
SolrClient.deleteByIds
Repository: lucene-solr
Updated Branches:
refs/heads/master 9c8ffabfe -> 6d1b2e2f3
SOLR-8808: Add null/empty check to SolrClient.deleteByIds
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/6d1b2e2f
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/6d1b2e2f
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/6d1b2e2f
Branch: refs/heads/master
Commit: 6d1b2e2f38a8979c65c1a208a0dd4cfc2de951ed
Parents: 9c8ffab
Author: Jason Gerlowski <ge...@apache.org>
Authored: Sat Oct 13 21:22:01 2018 -0400
Committer: Jason Gerlowski <ge...@apache.org>
Committed: Sat Oct 13 22:15:36 2018 -0400
----------------------------------------------------------------------
.../apache/solr/client/solrj/SolrClient.java | 9 +-
.../solrj/impl/CloudSolrClientBadInputTest.java | 73 +++++++++++++++
.../ConcurrentUpdateSolrClientBadInputTest.java | 91 +++++++++++++++++++
.../solrj/impl/HttpSolrClientBadInputTest.java | 93 ++++++++++++++++++++
.../impl/LBHttpSolrClientBadInputTest.java | 89 +++++++++++++++++++
5 files changed, 352 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6d1b2e2f/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
index 4b555e1..1c79efe 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrClient.java
@@ -775,7 +775,7 @@ public abstract class SolrClient implements Serializable, Closeable {
* Deletes a list of documents by unique ID
*
* @param collection the Solr collection to delete the documents from
- * @param ids the list of document IDs to delete
+ * @param ids the list of document IDs to delete; must be non-null and contain elements
*
* @return an {@link org.apache.solr.client.solrj.response.UpdateResponse} containing the response
* from the server
@@ -790,7 +790,7 @@ public abstract class SolrClient implements Serializable, Closeable {
/**
* Deletes a list of documents by unique ID
*
- * @param ids the list of document IDs to delete
+ * @param ids the list of document IDs to delete; must be non-null and contain elements
*
* @return an {@link org.apache.solr.client.solrj.response.UpdateResponse} containing the response
* from the server
@@ -806,7 +806,7 @@ public abstract class SolrClient implements Serializable, Closeable {
* Deletes a list of documents by unique ID, specifying max time before commit
*
* @param collection the Solr collection to delete the documents from
- * @param ids the list of document IDs to delete
+ * @param ids the list of document IDs to delete; must be non-null and contain elements
* @param commitWithinMs max time (in ms) before a commit will happen
*
* @return an {@link org.apache.solr.client.solrj.response.UpdateResponse} containing the response
@@ -818,6 +818,9 @@ public abstract class SolrClient implements Serializable, Closeable {
* @since 5.1
*/
public UpdateResponse deleteById(String collection, List<String> ids, int commitWithinMs) throws SolrServerException, IOException {
+ if (ids == null) throw new IllegalArgumentException("'ids' parameter must be non-null");
+ if (ids.isEmpty()) throw new IllegalArgumentException("'ids' parameter must not be empty; should contain IDs to delete");
+
UpdateRequest req = new UpdateRequest();
req.deleteById(ids);
req.setCommitWithin(commitWithinMs);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6d1b2e2f/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBadInputTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBadInputTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBadInputTest.java
new file mode 100644
index 0000000..5ec5bc6
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientBadInputTest.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj.impl;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.google.common.collect.Lists;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.internal.matchers.StringContains.containsString;
+
+public class CloudSolrClientBadInputTest extends SolrCloudTestCase {
+ private static final List<String> NULL_STR_LIST = null;
+ private static final List<String> EMPTY_STR_LIST = new ArrayList<>();
+ private static final String ANY_COLLECTION = "ANY_COLLECTION";
+ private static final int ANY_COMMIT_WITHIN_TIME = -1;
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1)
+ .configure();
+
+ final List<String> solrUrls = new ArrayList<>();
+ }
+
+ @Test
+ public void testDeleteByIdReportsInvalidIdLists() throws Exception {
+ try (SolrClient client = getCloudSolrClient(cluster)) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+ }
+
+ private void assertExceptionThrownWithMessageContaining(Class expectedType, List<String> expectedStrings, LuceneTestCase.ThrowingRunnable runnable) {
+ Throwable thrown = expectThrows(expectedType, runnable);
+
+ if (expectedStrings != null) {
+ for (String expectedString : expectedStrings) {
+ assertThat(thrown.getMessage(), containsString(expectedString));
+ }
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6d1b2e2f/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientBadInputTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientBadInputTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientBadInputTest.java
new file mode 100644
index 0000000..f28d9c0
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ConcurrentUpdateSolrClientBadInputTest.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj.impl;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.google.common.collect.Lists;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.embedded.JettyConfig;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.internal.matchers.StringContains.containsString;
+
+public class ConcurrentUpdateSolrClientBadInputTest extends SolrJettyTestBase {
+ private static final List<String> NULL_STR_LIST = null;
+ private static final List<String> EMPTY_STR_LIST = new ArrayList<>();
+ private static final String ANY_COLLECTION = "ANY_COLLECTION";
+ private static final int ANY_COMMIT_WITHIN_TIME = -1;
+ private static final int ANY_QUEUE_SIZE = 1;
+ private static final int ANY_MAX_NUM_THREADS = 1;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ JettyConfig jettyConfig = JettyConfig.builder()
+ .withSSLConfig(sslConfig)
+ .build();
+ createJetty(legacyExampleCollection1SolrHome(), jettyConfig);
+ }
+
+ @Test
+ public void testDeleteByIdReportsInvalidIdLists() throws Exception {
+ try (SolrClient client = getConcurrentUpdateSolrClient(jetty.getBaseUrl().toString() + "/" + ANY_COLLECTION, ANY_QUEUE_SIZE, ANY_MAX_NUM_THREADS)) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+
+ try (SolrClient client = getConcurrentUpdateSolrClient(jetty.getBaseUrl().toString(), ANY_QUEUE_SIZE, ANY_MAX_NUM_THREADS)) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+ }
+
+ private void assertExceptionThrownWithMessageContaining(Class expectedType, List<String> expectedStrings, LuceneTestCase.ThrowingRunnable runnable) {
+ Throwable thrown = expectThrows(expectedType, runnable);
+
+ if (expectedStrings != null) {
+ for (String expectedString : expectedStrings) {
+ assertThat(thrown.getMessage(), containsString(expectedString));
+ }
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6d1b2e2f/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientBadInputTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientBadInputTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientBadInputTest.java
new file mode 100644
index 0000000..cf97829
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientBadInputTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj.impl;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.google.common.collect.Lists;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.embedded.JettyConfig;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import static org.junit.internal.matchers.StringContains.containsString;
+
+/**
+ * Tests {@link HttpSolrClient}'s response to a variety of bad inputs.
+ */
+public class HttpSolrClientBadInputTest extends SolrJettyTestBase {
+ private static final List<String> NULL_STR_LIST = null;
+ private static final List<String> EMPTY_STR_LIST = new ArrayList<>();
+ private static final String ANY_COLLECTION = "ANY_COLLECTION";
+ private static final int ANY_COMMIT_WITHIN_TIME = -1;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ JettyConfig jettyConfig = JettyConfig.builder()
+ .withSSLConfig(sslConfig)
+ .build();
+ createJetty(legacyExampleCollection1SolrHome(), jettyConfig);
+ }
+
+ private void assertExceptionThrownWithMessageContaining(Class expectedType, List<String> expectedStrings, ThrowingRunnable runnable) {
+ Throwable thrown = expectThrows(expectedType, runnable);
+
+ if (expectedStrings != null) {
+ for (String expectedString : expectedStrings) {
+ assertThat(thrown.getMessage(), containsString(expectedString));
+ }
+ }
+ }
+
+ @Test
+ public void testDeleteByIdReportsInvalidIdLists() throws Exception {
+ try (SolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString() + "/" + ANY_COLLECTION)) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+
+ try (SolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString())) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+ }
+
+
+
+}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6d1b2e2f/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientBadInputTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientBadInputTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientBadInputTest.java
new file mode 100644
index 0000000..6c0ad81
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttpSolrClientBadInputTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj.impl;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import com.google.common.collect.Lists;
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.solr.SolrJettyTestBase;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.embedded.JettyConfig;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.internal.matchers.StringContains.containsString;
+
+public class LBHttpSolrClientBadInputTest extends SolrJettyTestBase {
+ private static final List<String> NULL_STR_LIST = null;
+ private static final List<String> EMPTY_STR_LIST = new ArrayList<>();
+ private static final String ANY_COLLECTION = "ANY_COLLECTION";
+ private static final int ANY_COMMIT_WITHIN_TIME = -1;
+
+ @BeforeClass
+ public static void beforeTest() throws Exception {
+ JettyConfig jettyConfig = JettyConfig.builder()
+ .withSSLConfig(sslConfig)
+ .build();
+ createJetty(legacyExampleCollection1SolrHome(), jettyConfig);
+ }
+
+ @Test
+ public void testDeleteByIdReportsInvalidIdLists() throws Exception {
+ try (SolrClient client = getLBHttpSolrClient(jetty.getBaseUrl().toString() + "/" + ANY_COLLECTION)) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+
+ try (SolrClient client = getLBHttpSolrClient(jetty.getBaseUrl().toString())) {
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "null"), () -> {
+ client.deleteById(ANY_COLLECTION, NULL_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ assertExceptionThrownWithMessageContaining(IllegalArgumentException.class, Lists.newArrayList("ids", "empty"), () -> {
+ client.deleteById(ANY_COLLECTION, EMPTY_STR_LIST, ANY_COMMIT_WITHIN_TIME);
+ });
+ }
+ }
+
+ private void assertExceptionThrownWithMessageContaining(Class expectedType, List<String> expectedStrings, LuceneTestCase.ThrowingRunnable runnable) {
+ Throwable thrown = expectThrows(expectedType, runnable);
+
+ if (expectedStrings != null) {
+ for (String expectedString : expectedStrings) {
+ assertThat(thrown.getMessage(), containsString(expectedString));
+ }
+ }
+ }
+}