This is an automated email from the ASF dual-hosted git repository. kou pushed a commit to branch maint-10.0.x in repository https://gitbox.apache.org/repos/asf/arrow.git
commit 26b0ec4db3c6f6b94e76ed4515e407e8955f1889 Author: David Li <li.david...@gmail.com> AuthorDate: Wed Nov 9 19:59:17 2022 -0500 ARROW-18296: [Java] Honor Driver#connect API contract in JDBC driver (#14617) Authored-by: David Li <li.david...@gmail.com> Signed-off-by: David Li <li.david...@gmail.com> --- .../arrow/driver/jdbc/ArrowFlightJdbcDriver.java | 16 +++++---- .../driver/jdbc/ArrowFlightJdbcDriverTest.java | 41 +++++++++++----------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java b/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java index a72fbd3a4d..aa1b460fc1 100644 --- a/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java +++ b/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java @@ -29,6 +29,7 @@ import java.nio.charset.StandardCharsets; import java.sql.SQLException; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Properties; import org.apache.arrow.driver.jdbc.utils.ArrowFlightConnectionConfigImpl.ArrowFlightConnectionProperty; @@ -72,7 +73,11 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver { properties.putAll(info); if (url != null) { - final Map<Object, Object> propertiesFromUrl = getUrlsArgs(url); + final Optional<Map<Object, Object>> maybeProperties = getUrlsArgs(url); + if (!maybeProperties.isPresent()) { + return null; + } + final Map<Object, Object> propertiesFromUrl = maybeProperties.get(); properties.putAll(propertiesFromUrl); } @@ -199,11 +204,11 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver { * </table> * * @param url The url to parse. - * @return the parsed arguments. + * @return the parsed arguments, or an empty optional if the driver does not handle this URL. * @throws SQLException If an error occurs while trying to parse the URL. */ @VisibleForTesting // ArrowFlightJdbcDriverTest - Map<Object, Object> getUrlsArgs(String url) + Optional<Map<Object, Object>> getUrlsArgs(String url) throws SQLException { /* @@ -240,8 +245,7 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver { if (!Objects.equals(uri.getScheme(), "arrow-flight") && !Objects.equals(uri.getScheme(), "arrow-flight-sql")) { - throw new SQLException("URL Scheme must be 'arrow-flight'. Expected format: " + - CONNECTION_STRING_EXPECTED); + return Optional.empty(); } if (uri.getHost() == null) { @@ -258,7 +262,7 @@ public class ArrowFlightJdbcDriver extends UnregisteredDriver { resultMap.putAll(keyValuePairs); } - return resultMap; + return Optional.of(resultMap); } static Properties lowerCasePropertyKeys(final Properties properties) { diff --git a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java index f1958b4fc8..9b8fa96d23 100644 --- a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java +++ b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriverTest.java @@ -17,9 +17,11 @@ package org.apache.arrow.driver.jdbc; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.sql.Connection; import java.sql.Driver; @@ -91,17 +93,15 @@ public class ArrowFlightJdbcDriverTest { } /** - * Tests whether the {@link ArrowFlightJdbcDriver} fails when provided with an + * Tests whether the {@link ArrowFlightJdbcDriver} returns null when provided with an * unsupported URL prefix. - * - * @throws SQLException If the test passes. */ - @Test(expected = SQLException.class) + @Test public void testShouldDeclineUrlWithUnsupportedPrefix() throws Exception { final Driver driver = new ArrowFlightJdbcDriver(); - driver.connect("jdbc:mysql://localhost:32010", dataSource.getProperties("flight", "flight123")) - .close(); + assertNull(driver.connect("jdbc:mysql://localhost:32010", + dataSource.getProperties("flight", "flight123"))); } /** @@ -263,7 +263,8 @@ public class ArrowFlightJdbcDriverTest { final ArrowFlightJdbcDriver driver = new ArrowFlightJdbcDriver(); final Map<Object, Object> parsedArgs = driver.getUrlsArgs( - "jdbc:arrow-flight-sql://localhost:2222/?key1=value1&key2=value2&a=b"); + "jdbc:arrow-flight-sql://localhost:2222/?key1=value1&key2=value2&a=b") + .orElseThrow(() -> new RuntimeException("URL was rejected")); // Check size == the amount of args provided (scheme not included) assertEquals(5, parsedArgs.size()); @@ -284,7 +285,8 @@ public class ArrowFlightJdbcDriverTest { public void testDriverUrlParsingMechanismShouldReturnTheDesiredArgsFromUrlWithSemicolon() throws Exception { final ArrowFlightJdbcDriver driver = new ArrowFlightJdbcDriver(); final Map<Object, Object> parsedArgs = driver.getUrlsArgs( - "jdbc:arrow-flight-sql://localhost:2222/;key1=value1;key2=value2;a=b"); + "jdbc:arrow-flight-sql://localhost:2222/;key1=value1;key2=value2;a=b") + .orElseThrow(() -> new RuntimeException("URL was rejected")); // Check size == the amount of args provided (scheme not included) assertEquals(5, parsedArgs.size()); @@ -305,7 +307,8 @@ public class ArrowFlightJdbcDriverTest { public void testDriverUrlParsingMechanismShouldReturnTheDesiredArgsFromUrlWithOneSemicolon() throws Exception { final ArrowFlightJdbcDriver driver = new ArrowFlightJdbcDriver(); final Map<Object, Object> parsedArgs = driver.getUrlsArgs( - "jdbc:arrow-flight-sql://localhost:2222/;key1=value1"); + "jdbc:arrow-flight-sql://localhost:2222/;key1=value1") + .orElseThrow(() -> new RuntimeException("URL was rejected")); // Check size == the amount of args provided (scheme not included) assertEquals(3, parsedArgs.size()); @@ -320,16 +323,10 @@ public class ArrowFlightJdbcDriverTest { assertEquals(parsedArgs.get("key1"), "value1"); } - /** - * Tests whether an exception is thrown upon attempting to connect to a - * malformed URI. - * - */ @Test - public void testDriverUrlParsingMechanismShouldThrowExceptionUponProvidedWithMalformedUrl() { + public void testDriverUrlParsingMechanismShouldReturnEmptyOptionalForUnknownScheme() throws SQLException { final ArrowFlightJdbcDriver driver = new ArrowFlightJdbcDriver(); - assertThrows(SQLException.class, () -> driver.getUrlsArgs( - "jdbc:malformed-url-flight://localhost:2222")); + assertFalse(driver.getUrlsArgs("jdbc:malformed-url-flight://localhost:2222").isPresent()); } /** @@ -341,7 +338,8 @@ public class ArrowFlightJdbcDriverTest { @Test public void testDriverUrlParsingMechanismShouldWorkWithIPAddress() throws Exception { final ArrowFlightJdbcDriver driver = new ArrowFlightJdbcDriver(); - final Map<Object, Object> parsedArgs = driver.getUrlsArgs("jdbc:arrow-flight-sql://0.0.0.0:2222"); + final Map<Object, Object> parsedArgs = driver.getUrlsArgs("jdbc:arrow-flight-sql://0.0.0.0:2222") + .orElseThrow(() -> new RuntimeException("URL was rejected")); // Check size == the amount of args provided (scheme not included) assertEquals(2, parsedArgs.size()); @@ -364,7 +362,8 @@ public class ArrowFlightJdbcDriverTest { throws Exception { final ArrowFlightJdbcDriver driver = new ArrowFlightJdbcDriver(); final Map<Object, Object> parsedArgs = driver.getUrlsArgs( - "jdbc:arrow-flight-sql://0.0.0.0:2222?test1=test1value&test2%26continue=test2value&test3=test3value"); + "jdbc:arrow-flight-sql://0.0.0.0:2222?test1=test1value&test2%26continue=test2value&test3=test3value") + .orElseThrow(() -> new RuntimeException("URL was rejected")); // Check size == the amount of args provided (scheme not included) assertEquals(5, parsedArgs.size());