BentsiLeviav commented on code in PR #37611: URL: https://github.com/apache/beam/pull/37611#discussion_r2818368753
########## sdks/java/io/clickhouse/src/main/java/org/apache/beam/sdk/io/clickhouse/ClickHouseJdbcUrlParser.java: ########## @@ -0,0 +1,261 @@ +/* + * 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.beam.sdk.io.clickhouse; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.List; +import java.util.Properties; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Splitter; +import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings; + +/** + * Utility class for parsing ClickHouse JDBC URLs and extracting connection parameters. + * + * <p>Used for supporting backward compatibility with the deprecated {@link + * ClickHouseIO#write(String, String)} method that accepts JDBC URLs. New code should use {@link + * ClickHouseIO#write(String, String, String)} with explicit parameters instead. + * + * @deprecated Use {@link ClickHouseIO#write(String, String, String)} with separate clickHouseUrl, + * database, and table parameters instead of JDBC URL format. + */ +@Deprecated +class ClickHouseJdbcUrlParser { + + /** + * Represents parsed components of a ClickHouse JDBC URL. + * + * <p>Contains the extracted HTTP/HTTPS URL, database name, and connection properties from a JDBC + * URL string. + * + * @deprecated This class supports the deprecated JDBC URL-based API. Use separate parameters for + * clickHouseUrl, database, and properties instead. + */ + @Deprecated + static class ParsedJdbcUrl { + private final String clickHouseUrl; + private final String database; + private final Properties properties; + + ParsedJdbcUrl(String clickHouseUrl, String database, Properties properties) { + this.clickHouseUrl = clickHouseUrl; + this.database = database; + this.properties = properties; + } + + public String getClickHouseUrl() { + return clickHouseUrl; + } + + public String getDatabase() { + return database; + } + + public Properties getProperties() { + return properties; + } + } + + /** + * Parses a ClickHouse JDBC URL into its components. + * + * <p>Supported formats: + * + * <ul> + * <li>jdbc:clickhouse://host:port/database?param=value + * <li>jdbc:clickhouse:http://host:port/database?param=value + * <li>jdbc:clickhouse:https://host:port/database?param=value + * <li>jdbc:ch://host:port/database?param=value (ClickHouse JDBC driver shorthand) + * </ul> + * + * @param jdbcUrl the JDBC URL to parse + * @return ParsedJdbcUrl containing the HTTP/HTTPS URL, database, and properties + * @throws IllegalArgumentException if the URL format is invalid + */ + static ParsedJdbcUrl parse(String jdbcUrl) { + if (Strings.isNullOrEmpty(jdbcUrl)) { + throw new IllegalArgumentException("JDBC URL cannot be null or empty"); + } + + String actualUrl = extractHttpUrl(jdbcUrl); + + try { + URI uri = new URI(actualUrl); + + validateScheme(uri.getScheme()); + String host = validateAndGetHost(uri.getHost(), jdbcUrl); + int port = getPortOrDefault(uri.getPort(), uri.getScheme()); + + String clickHouseUrl = String.format("%s://%s:%d", uri.getScheme(), host, port); + String database = extractDatabase(uri.getPath()); + Properties properties = extractProperties(uri.getQuery()); Review Comment: Good question! The parser is designed to handle the most common JDBC URL formats and fails early with `IllegalArgumentException` for invalid inputs. Here's what's validated: Early validation checks: **Non-null/non-empty URL - throws if `jdbcUrl` is null or empty * Valid JDBC prefix - throws if not `jdbc:clickhouse:` or `jdbc:ch:` * Valid scheme - throws if scheme is not `http` or `https` (rejects `ftp://`, etc.) * Non-empty host - throws if host is missing * URI syntax - throws `IllegalArgumentException` if URL is malformed Default handling: if we are missing a port, the code defaults to 8123 (http) or 8443 (https). For database, we default to "default" and for missing properties, we return an empty Properties object Example failures caught early (copied from the tests): ```java parse(null) // ✗ IllegalArgumentException: "cannot be null" parse("jdbc:mysql://localhost/db") // ✗ IllegalArgumentException: "Expected 'jdbc:clickhouse:'" parse("jdbc:clickhouse:ftp://host/db";) // ✗ IllegalArgumentException: "Invalid scheme... Got: ftp" parse("jdbc:clickhouse://:8123/db") // ✗ IllegalArgumentException: "Host cannot be empty" ``` The parser is permissive for optional components like port, database, properties but strict on required components like prefix, scheme, host. This matches the behavior of the old JDBC driver, which also provided defaults for missing optional parts. I've added comprehensive tests in `ClickHouseJdbcUrlParserTest` covering valid formats, edge cases, and invalid inputs. Would you like me to add any additional validation? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
