From 23c73d8ff85f9b6eea30e2c03930d9e8ba116da0 Mon Sep 17 00:00:00 2001 From: Joel Bernstein Date: Tue, 24 Nov 2015 16:27:24 +0000 Subject: [PATCH] SOLR-8179: SQL JDBC - DriverImpl loadParams doesn't support keys with no values in the connection string git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1716198 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 + .../solr/client/solrj/io/sql/DriverImpl.java | 105 +++++++--------- .../client/solrj/io/sql/JdbcDriverTest.java | 80 ++++++++++++ .../solr/client/solrj/io/sql/JdbcTest.java | 115 +++++++++++------- 4 files changed, 200 insertions(+), 103 deletions(-) create mode 100644 solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcDriverTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 348fbf31aff..bc7b7043312 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -156,6 +156,9 @@ Other Changes * SOLR-8271: Change implicit default Similarity to use SchemaSimilarityFactory when luceneMatchVersion >= 6 (hossman) +* SOLR-8179: SQL JDBC - DriverImpl loadParams doesn't support keys with no values in the connection string + (Kevin Risden, Joel Bernstein) + ================== 5.4.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/sql/DriverImpl.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/sql/DriverImpl.java index 5ca4e686dbd..e90d4b7d039 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/sql/DriverImpl.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/sql/DriverImpl.java @@ -18,23 +18,27 @@ package org.apache.solr.client.solrj.io.sql; */ -import java.net.URLDecoder; +import java.net.URI; +import java.net.URISyntaxException; import java.sql.Connection; import java.sql.Driver; import java.sql.DriverManager; import java.sql.DriverPropertyInfo; import java.sql.SQLException; +import java.util.List; import java.util.Properties; import java.util.logging.Logger; + +import org.apache.http.NameValuePair; +import org.apache.http.client.utils.URLEncodedUtils; import org.apache.solr.common.util.SuppressForbidden; /** - * Get a Connection with with a url and properties. + * Get a Connection with with a url and properties. * - * jdbc:solr://zkhost:port?collection=collection&aggregationMode=map_reduce + * jdbc:solr://zkhost:port?collection=collection&aggregationMode=map_reduce **/ - public class DriverImpl implements Driver { static { @@ -50,55 +54,26 @@ public class DriverImpl implements Driver { return null; } - StringBuilder buf = new StringBuilder(url); - boolean needsAmp = true; - if(!url.contains("?")) { - buf.append("?"); - needsAmp = false; + URI uri = processUrl(url); + + loadParams(uri, props); + + if (!props.containsKey("collection")) { + throw new SQLException("The connection url has no connection properties. At a mininum the collection must be specified."); + } + String collection = (String) props.remove("collection"); + + if (!props.containsKey("aggregationMode")) { + props.setProperty("aggregationMode", "facet"); } - for(Object key : props.keySet()) { - Object value = props.get(key); - if(needsAmp) { - buf.append("&"); - } - buf.append(key.toString()).append("=").append(value); - needsAmp = true; - } + String zkHost = uri.getAuthority() + uri.getPath(); - return connect(buf.toString()); + return new ConnectionImpl(zkHost, collection, props); } public Connection connect(String url) throws SQLException { - - if(!acceptsURL(url)) { - return null; - } - - String[] parts = url.split("://", 0); - - if(parts.length < 2) { - throw new SQLException("The zkHost must start with ://"); - } - - String zkUrl = parts[1]; - String[] zkUrlParts = zkUrl.split("\\?"); - - if(zkUrlParts.length < 2) { - throw new SQLException("The connection url has no connection properties. At a mininum the collection must be specified."); - } - - String connectionProps = zkUrlParts[1]; - String zkHost = zkUrlParts[0]; - Properties props = new Properties(); - loadParams(connectionProps, props); - String collection = (String)props.remove("collection"); - - if(!props.containsKey("aggregationMode")) { - props.setProperty("aggregationMode","facet"); - } - - return new ConnectionImpl(zkHost, collection, props); + return connect(url, new Properties()); } public int getMajorVersion() { @@ -110,11 +85,7 @@ public class DriverImpl implements Driver { } public boolean acceptsURL(String url) { - if(url.startsWith("jdbc:solr")) { - return true; - } else { - return false; - } + return url != null && url.startsWith("jdbc:solr"); } public boolean jdbcCompliant() { @@ -132,17 +103,29 @@ public class DriverImpl implements Driver { return null; } - private void loadParams(String params, Properties props) throws SQLException { + protected URI processUrl(String url) throws SQLException { + URI uri; try { - String[] pairs = params.split("&"); - for (String pair : pairs) { - String[] keyValue = pair.split("="); - String key = URLDecoder.decode(keyValue[0], "UTF-8"); - String value = URLDecoder.decode(keyValue[1], "UTF-8"); - props.put(key, value); - } - } catch(Exception e) { + uri = new URI(url.replaceFirst("jdbc:", "")); + } catch (URISyntaxException e) { throw new SQLException(e); } + + if (uri.getAuthority() == null) { + throw new SQLException("The zkHost must not be null"); + } + + return uri; + } + + private void loadParams(URI uri, Properties props) throws SQLException { + List parsedParams = URLEncodedUtils.parse(uri, "UTF-8"); + for (NameValuePair pair : parsedParams) { + if (pair.getValue() != null) { + props.put(pair.getName(), pair.getValue()); + } else { + props.put(pair.getName(), ""); + } + } } } \ No newline at end of file diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcDriverTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcDriverTest.java new file mode 100644 index 00000000000..215abff22a9 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcDriverTest.java @@ -0,0 +1,80 @@ +package org.apache.solr.client.solrj.io.sql; + +/* + * 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. + */ + +import java.net.URI; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Properties; + +import org.apache.solr.SolrTestCaseJ4; +import org.junit.Test; + +/** + * Tests the connection string part of the JDBC Driver + **/ + +public class JdbcDriverTest extends SolrTestCaseJ4 { + + @Test(expected = SQLException.class) + public void testNullZKConnectionString() throws Exception { + Connection con = DriverManager.getConnection("jdbc:solr://?collection=collection1"); + } + + @Test(expected = SQLException.class) + public void testInvalidJDBCConnectionString() throws Exception { + Connection con = DriverManager.getConnection("jdbc:mysql://"); + } + + @Test(expected = SQLException.class) + public void testNoCollectionProvidedInURL() throws Exception { + Connection con = DriverManager.getConnection("jdbc:solr://?collection=collection1"); + } + + @Test(expected = SQLException.class) + public void testNoCollectionProvidedInProperties() throws Exception { + Connection con = DriverManager.getConnection("jdbc:solr://", new Properties()); + } + + @Test + public void testProcessUrl() throws Exception { + DriverImpl driver = new DriverImpl(); + + List zkHostStrings = Arrays.asList("zoo1", "zoo1:9983", "zoo1,zoo2,zoo3", "zoo1:9983,zoo2:9983,zoo3:9983"); + List chroots = Arrays.asList("", "/", "/foo", "/foo/bar"); + List paramStrings = Arrays.asList("", "collection=collection1", "collection=collection1&test=test1"); + + for(String zkHostString : zkHostStrings) { + for(String chroot : chroots) { + for(String paramString : paramStrings) { + String url = "jdbc:solr://" + zkHostString + chroot + "?" + paramString; + + URI uri = driver.processUrl(url); + + assertEquals(zkHostString, uri.getAuthority()); + assertEquals(chroot, uri.getPath()); + assertEquals(paramString, uri.getQuery()); + } + } + } + } +} diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java index 09af65e3ed1..2a638ddfa03 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java @@ -19,17 +19,15 @@ package org.apache.solr.client.solrj.io.sql; import java.io.File; import java.sql.Connection; -import java.sql.Statement; -import java.sql.ResultSet; import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.Statement; import java.util.Properties; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase.Slow; -import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; import org.apache.solr.cloud.AbstractFullDistribZkTestBase; import org.apache.solr.cloud.AbstractZkTestCase; -import org.apache.solr.common.SolrInputDocument; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -37,17 +35,16 @@ import org.junit.BeforeClass; import org.junit.Test; /** - * All base tests will be done with CloudSolrStream. Under the covers CloudSolrStream uses SolrStream so - * SolrStream will get fully exercised through these tests. - * + * All base tests will be done with CloudSolrStream. Under the covers CloudSolrStream uses SolrStream so + * SolrStream will get fully exercised through these tests. **/ @Slow -@LuceneTestCase.SuppressCodecs({"Lucene3x", "Lucene40","Lucene41","Lucene42","Lucene45"}) +@LuceneTestCase.SuppressCodecs({"Lucene3x", "Lucene40", "Lucene41", "Lucene42", "Lucene45"}) public class JdbcTest extends AbstractFullDistribZkTestBase { private static final String SOLR_HOME = getFile("solrj" + File.separator + "solr").getAbsolutePath(); - private StreamFactory streamFactory; + static { schemaString = "schema-sql.xml"; @@ -55,7 +52,7 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { @BeforeClass public static void beforeSuperClass() { - AbstractZkTestCase.SOLRHOME = new File(SOLR_HOME()); + AbstractZkTestCase.SOLRHOME = new File(SOLR_HOME); } @AfterClass @@ -67,25 +64,17 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { return "solrconfig-sql.xml"; } - @Override public String getSolrHome() { return SOLR_HOME; } - public static String SOLR_HOME() { - return SOLR_HOME; + + @Override + public void distribSetUp() throws Exception { + super.distribSetUp(); } - @Before - @Override - public void setUp() throws Exception { - super.setUp(); - // we expect this time of exception as shards go up and down... - //ignoreException(".*"); - //System.setProperty("export.test", "true"); - System.setProperty("numShards", Integer.toString(sliceCount)); - } @Override @After @@ -94,14 +83,8 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { resetExceptionIgnores(); } - public JdbcTest() { - super(); - sliceCount = 2; - - - } - @Test + @ShardsFixed(num = 2) public void doTest() throws Exception { waitForRecoveriesToFinish(false); @@ -183,8 +166,6 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { con.close(); //Test facet aggregation - - props = new Properties(); props.put("aggregationMode", "facet"); con = DriverManager.getConnection("jdbc:solr://" + zkHost + "?collection=collection1", props); @@ -208,9 +189,7 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { stmt.close(); con.close(); - //Test map / reduce aggregation - props = new Properties(); props.put("aggregationMode", "map_reduce"); props.put("numWorkers", "2"); @@ -234,10 +213,9 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { con.close(); //Test params on the url - con = DriverManager.getConnection("jdbc:solr://" + zkHost + "?collection=collection1&aggregationMode=map_reduce&numWorkers=2"); - Properties p = ((ConnectionImpl)con).props; + Properties p = ((ConnectionImpl) con).props; assert(p.getProperty("aggregationMode").equals("map_reduce")); assert(p.getProperty("numWorkers").equals("2")); @@ -260,13 +238,66 @@ public class JdbcTest extends AbstractFullDistribZkTestBase { stmt.close(); con.close(); - del("*:*"); - commit(); - } + // Test JDBC paramters in URL + con = DriverManager.getConnection( + "jdbc:solr://" + zkHost + "?collection=collection1&username=&password=&testKey1=testValue&testKey2"); - @Override - protected void indexr(Object... fields) throws Exception { - SolrInputDocument doc = getDoc(fields); - indexDoc(doc); + p = ((ConnectionImpl) con).props; + assert(p.getProperty("username").equals("")); + assert(p.getProperty("password").equals("")); + assert(p.getProperty("testKey1").equals("testValue")); + assert(p.getProperty("testKey2").equals("")); + + stmt = con.createStatement(); + rs = stmt.executeQuery("select a_s, sum(a_f) from collection1 group by a_s order by sum(a_f) desc"); + + assert(rs.next()); + assert(rs.getString("a_s").equals("hello3")); + assert(rs.getDouble("sum(a_f)") == 26); + + assert(rs.next()); + assert(rs.getString("a_s").equals("hello0")); + assert(rs.getDouble("sum(a_f)") == 18); + + assert(rs.next()); + assert(rs.getString("a_s").equals("hello4")); + assert(rs.getDouble("sum(a_f)") == 11); + + stmt.close(); + con.close(); + + // Test JDBC paramters in properties + Properties providedProperties = new Properties(); + providedProperties.put("collection", "collection1"); + providedProperties.put("username", ""); + providedProperties.put("password", ""); + providedProperties.put("testKey1", "testValue"); + providedProperties.put("testKey2", ""); + + con = DriverManager.getConnection("jdbc:solr://" + zkHost, providedProperties); + + p = ((ConnectionImpl) con).props; + assert(p.getProperty("username").equals("")); + assert(p.getProperty("password").equals("")); + assert(p.getProperty("testKey1").equals("testValue")); + assert(p.getProperty("testKey2").equals("")); + + stmt = con.createStatement(); + rs = stmt.executeQuery("select a_s, sum(a_f) from collection1 group by a_s order by sum(a_f) desc"); + + assert(rs.next()); + assert(rs.getString("a_s").equals("hello3")); + assert(rs.getDouble("sum(a_f)") == 26); + + assert(rs.next()); + assert(rs.getString("a_s").equals("hello0")); + assert(rs.getDouble("sum(a_f)") == 18); + + assert(rs.next()); + assert(rs.getString("a_s").equals("hello4")); + assert(rs.getDouble("sum(a_f)") == 11); + + stmt.close(); + con.close(); } }