From 459cdda63de638b3778ae3c920a06acb9586ab31 Mon Sep 17 00:00:00 2001 From: Liang Zhang Date: Sat, 16 May 2020 07:26:30 +0800 Subject: [PATCH] Decouple ShardingSphereServiceLoader and SQL parser module (#5633) * rename ParsingHookRegistry to SPIParsingHook * rename ParsingHookRegistry to SPIParsingHook * decouple ShardingSphereServiceLoader and SQL parser module * remove dependency of shardingsphere-spi --- .../hook/OpenTracingParsingHookTest.java | 12 ++-- .../shardingsphere-sql-parser-engine/pom.xml | 5 -- .../sql/parser/SQLParserEngine.java | 13 ++-- .../core/SQLParserConfigurationRegistry.java | 63 +++++++++++++++++++ .../parser/core/parser/SQLParserFactory.java | 13 +--- .../core/visitor/ParseTreeVisitorFactory.java | 9 +-- ...singHook.java => ParsingHookRegistry.java} | 57 +++++++++++------ ...Test.java => ParsingHookRegistryTest.java} | 11 ++-- 8 files changed, 123 insertions(+), 60 deletions(-) create mode 100644 shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/SQLParserConfigurationRegistry.java rename shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/{SPIParsingHook.java => ParsingHookRegistry.java} (52%) rename shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/{SPIParsingHookTest.java => ParsingHookRegistryTest.java} (85%) diff --git a/sharding-opentracing/src/test/java/org/apache/shardingsphere/opentracing/hook/OpenTracingParsingHookTest.java b/sharding-opentracing/src/test/java/org/apache/shardingsphere/opentracing/hook/OpenTracingParsingHookTest.java index 6d542f97b4..e5d153e39b 100644 --- a/sharding-opentracing/src/test/java/org/apache/shardingsphere/opentracing/hook/OpenTracingParsingHookTest.java +++ b/sharding-opentracing/src/test/java/org/apache/shardingsphere/opentracing/hook/OpenTracingParsingHookTest.java @@ -22,7 +22,7 @@ import io.opentracing.tag.Tags; import org.apache.shardingsphere.opentracing.constant.ShardingTags; import org.apache.shardingsphere.sharding.spi.ShardingSphereServiceLoader; import org.apache.shardingsphere.sql.parser.hook.ParsingHook; -import org.apache.shardingsphere.sql.parser.hook.SPIParsingHook; +import org.apache.shardingsphere.sql.parser.hook.ParsingHookRegistry; import org.apache.shardingsphere.sql.parser.sql.statement.SQLStatement; import org.apache.shardingsphere.underlying.common.exception.ShardingSphereException; import org.junit.BeforeClass; @@ -36,7 +36,7 @@ import static org.mockito.Mockito.mock; public final class OpenTracingParsingHookTest extends BaseOpenTracingHookTest { - private final ParsingHook parsingHook = new SPIParsingHook(); + private final ParsingHookRegistry registry = ParsingHookRegistry.getInstance(); @BeforeClass public static void registerSPI() { @@ -45,8 +45,8 @@ public final class OpenTracingParsingHookTest extends BaseOpenTracingHookTest { @Test public void assertExecuteSuccess() { - parsingHook.start("SELECT * FROM XXX;"); - parsingHook.finishSuccess(mock(SQLStatement.class)); + registry.start("SELECT * FROM XXX;"); + registry.finishSuccess(mock(SQLStatement.class)); MockSpan actual = getActualSpan(); assertThat(actual.operationName(), is("/ShardingSphere/parseSQL/")); Map actualTags = actual.tags(); @@ -57,8 +57,8 @@ public final class OpenTracingParsingHookTest extends BaseOpenTracingHookTest { @Test public void assertExecuteFailure() { - parsingHook.start("SELECT * FROM XXX;"); - parsingHook.finishFailure(new ShardingSphereException("parse SQL error")); + registry.start("SELECT * FROM XXX;"); + registry.finishFailure(new ShardingSphereException("parse SQL error")); MockSpan actual = getActualSpan(); assertThat(actual.operationName(), is("/ShardingSphere/parseSQL/")); Map actualTags = actual.tags(); diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/pom.xml b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/pom.xml index ecb667b45b..3b6c4d3681 100644 --- a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/pom.xml +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/pom.xml @@ -27,11 +27,6 @@ ${project.artifactId} - - org.apache.shardingsphere - shardingsphere-spi - ${project.version} - org.apache.shardingsphere shardingsphere-sql-parser-spi diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/SQLParserEngine.java b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/SQLParserEngine.java index 6d90f8026a..80f0d8527a 100644 --- a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/SQLParserEngine.java +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/SQLParserEngine.java @@ -22,9 +22,8 @@ import org.antlr.v4.runtime.tree.ParseTree; import org.apache.shardingsphere.sql.parser.cache.SQLParseResultCache; import org.apache.shardingsphere.sql.parser.core.parser.SQLParserExecutor; import org.apache.shardingsphere.sql.parser.core.visitor.ParseTreeVisitorFactory; -import org.apache.shardingsphere.sql.parser.hook.ParsingHook; -import org.apache.shardingsphere.sql.parser.hook.SPIParsingHook; import org.apache.shardingsphere.sql.parser.core.visitor.VisitorRule; +import org.apache.shardingsphere.sql.parser.hook.ParsingHookRegistry; import org.apache.shardingsphere.sql.parser.sql.statement.SQLStatement; import java.util.Optional; @@ -39,13 +38,14 @@ public final class SQLParserEngine { private final SQLParseResultCache cache = new SQLParseResultCache(); + private ParsingHookRegistry parsingHookRegistry = ParsingHookRegistry.getInstance(); + // TODO check skywalking plugin /* * To make sure SkyWalking will be available at the next release of ShardingSphere, * a new plugin should be provided to SkyWalking project if this API changed. * * @see Plugin Development Guide - * */ /** * Parse SQL. @@ -55,16 +55,15 @@ public final class SQLParserEngine { * @return SQL statement */ public SQLStatement parse(final String sql, final boolean useCache) { - ParsingHook parsingHook = new SPIParsingHook(); - parsingHook.start(sql); + parsingHookRegistry.start(sql); try { SQLStatement result = parse0(sql, useCache); - parsingHook.finishSuccess(result); + parsingHookRegistry.finishSuccess(result); return result; // CHECKSTYLE:OFF } catch (final Exception ex) { // CHECKSTYLE:ON - parsingHook.finishFailure(ex); + parsingHookRegistry.finishFailure(ex); throw ex; } } diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/SQLParserConfigurationRegistry.java b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/SQLParserConfigurationRegistry.java new file mode 100644 index 0000000000..3076143eba --- /dev/null +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/SQLParserConfigurationRegistry.java @@ -0,0 +1,63 @@ +/* + * 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.shardingsphere.sql.parser.core; + +import org.apache.shardingsphere.sql.parser.spi.SQLParserConfiguration; + +import java.util.HashMap; +import java.util.Map; +import java.util.ServiceLoader; + +/** + * SQL parser configuration registry. + */ +public final class SQLParserConfigurationRegistry { + + private static final SQLParserConfigurationRegistry INSTANCE = new SQLParserConfigurationRegistry(); + + private final Map configurations; + + private SQLParserConfigurationRegistry() { + configurations = new HashMap<>(); + for (SQLParserConfiguration each : ServiceLoader.load(SQLParserConfiguration.class)) { + configurations.put(each.getDatabaseTypeName(), each); + } + } + + /** + * Get instance. + * + * @return instance + */ + public static SQLParserConfigurationRegistry getInstance() { + return INSTANCE; + } + + /** + * Get SQL parser configuration. + * + * @param databaseTypeName database type name + * @return SQL parser configuration + */ + public SQLParserConfiguration getSQLParserConfiguration(final String databaseTypeName) { + if (configurations.containsKey(databaseTypeName)) { + return configurations.get(databaseTypeName); + } + throw new UnsupportedOperationException(String.format("Cannot support database type '%s'", databaseTypeName)); + } +} diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/parser/SQLParserFactory.java b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/parser/SQLParserFactory.java index ed385292a1..dbcfaff42c 100644 --- a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/parser/SQLParserFactory.java +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/parser/SQLParserFactory.java @@ -25,8 +25,8 @@ import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; import org.antlr.v4.runtime.Lexer; import org.antlr.v4.runtime.TokenStream; -import org.apache.shardingsphere.sharding.spi.ShardingSphereServiceLoader; import org.apache.shardingsphere.sql.parser.api.parser.SQLParser; +import org.apache.shardingsphere.sql.parser.core.SQLParserConfigurationRegistry; import org.apache.shardingsphere.sql.parser.spi.SQLParserConfiguration; /** @@ -35,10 +35,6 @@ import org.apache.shardingsphere.sql.parser.spi.SQLParserConfiguration; @NoArgsConstructor(access = AccessLevel.PRIVATE) public final class SQLParserFactory { - static { - ShardingSphereServiceLoader.register(SQLParserConfiguration.class); - } - /** * New instance of SQL parser. * @@ -47,12 +43,7 @@ public final class SQLParserFactory { * @return SQL parser */ public static SQLParser newInstance(final String databaseTypeName, final String sql) { - for (SQLParserConfiguration each : ShardingSphereServiceLoader.newServiceInstances(SQLParserConfiguration.class)) { - if (each.getDatabaseTypeName().equals(databaseTypeName)) { - return createSQLParser(sql, each); - } - } - throw new UnsupportedOperationException(String.format("Cannot support database type '%s'", databaseTypeName)); + return createSQLParser(sql, SQLParserConfigurationRegistry.getInstance().getSQLParserConfiguration(databaseTypeName)); } @SneakyThrows(ReflectiveOperationException.class) diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/visitor/ParseTreeVisitorFactory.java b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/visitor/ParseTreeVisitorFactory.java index 9e48b44081..c0edab9edf 100644 --- a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/visitor/ParseTreeVisitorFactory.java +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/core/visitor/ParseTreeVisitorFactory.java @@ -21,8 +21,8 @@ import lombok.AccessLevel; import lombok.NoArgsConstructor; import lombok.SneakyThrows; import org.antlr.v4.runtime.tree.ParseTreeVisitor; -import org.apache.shardingsphere.sharding.spi.ShardingSphereServiceLoader; import org.apache.shardingsphere.sql.parser.api.visitor.SQLVisitorFacade; +import org.apache.shardingsphere.sql.parser.core.SQLParserConfigurationRegistry; import org.apache.shardingsphere.sql.parser.exception.SQLParsingException; import org.apache.shardingsphere.sql.parser.spi.SQLParserConfiguration; import org.apache.shardingsphere.sql.parser.sql.statement.SQLStatementType; @@ -41,12 +41,7 @@ public final class ParseTreeVisitorFactory { * @return parse tree visitor */ public static ParseTreeVisitor newInstance(final String databaseTypeName, final VisitorRule visitorRule) { - for (SQLParserConfiguration each : ShardingSphereServiceLoader.newServiceInstances(SQLParserConfiguration.class)) { - if (each.getDatabaseTypeName().equals(databaseTypeName)) { - return createParseTreeVisitor(each, visitorRule.getType()); - } - } - throw new UnsupportedOperationException(String.format("Cannot support database type '%s'", databaseTypeName)); + return createParseTreeVisitor(SQLParserConfigurationRegistry.getInstance().getSQLParserConfiguration(databaseTypeName), visitorRule.getType()); } @SneakyThrows diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/SPIParsingHook.java b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/ParsingHookRegistry.java similarity index 52% rename from shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/SPIParsingHook.java rename to shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/ParsingHookRegistry.java index aebd079b1f..56d082e1c5 100644 --- a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/SPIParsingHook.java +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/main/java/org/apache/shardingsphere/sql/parser/hook/ParsingHookRegistry.java @@ -18,39 +18,60 @@ package org.apache.shardingsphere.sql.parser.hook; import org.apache.shardingsphere.sql.parser.sql.statement.SQLStatement; -import org.apache.shardingsphere.sharding.spi.ShardingSphereServiceLoader; import java.util.Collection; +import java.util.LinkedList; +import java.util.ServiceLoader; /** - * Parsing hook for SPI. + * Parsing hook registry. */ -public final class SPIParsingHook implements ParsingHook { +public final class ParsingHookRegistry { - private final Collection parsingHooks = ShardingSphereServiceLoader.newServiceInstances(ParsingHook.class); + private static final ParsingHookRegistry INSTANCE = new ParsingHookRegistry(); - static { - ShardingSphereServiceLoader.register(ParsingHook.class); + private final Collection hooks; + + private ParsingHookRegistry() { + hooks = new LinkedList<>(); + for (ParsingHook each : ServiceLoader.load(ParsingHook.class)) { + hooks.add(each); + } + } + + /** + * Get instance. + * + * @return instance + */ + public static ParsingHookRegistry getInstance() { + return INSTANCE; } - @Override + /** + * Handle when parse started. + * + * @param sql SQL to be parsed + */ public void start(final String sql) { - for (ParsingHook each : parsingHooks) { - each.start(sql); - } + hooks.forEach(each -> each.start(sql)); } - @Override + /** + * Handle when parse finished success. + * + * @param sqlStatement sql statement + */ public void finishSuccess(final SQLStatement sqlStatement) { - for (ParsingHook each : parsingHooks) { - each.finishSuccess(sqlStatement); - } + hooks.forEach(each -> each.finishSuccess(sqlStatement)); } - @Override + /** + * Handle when parse finished failure. + * + * @param cause failure cause + */ public void finishFailure(final Exception cause) { - for (ParsingHook each : parsingHooks) { - each.finishFailure(cause); - } + hooks.forEach(each -> each.finishFailure(cause)); } } diff --git a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/SPIParsingHookTest.java b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/ParsingHookRegistryTest.java similarity index 85% rename from shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/SPIParsingHookTest.java rename to shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/ParsingHookRegistryTest.java index 936e93503b..0ece57cfef 100644 --- a/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/SPIParsingHookTest.java +++ b/shardingsphere-sql-parser/shardingsphere-sql-parser-engine/src/test/java/org/apache/shardingsphere/sql/parser/hook/ParsingHookRegistryTest.java @@ -23,31 +23,30 @@ import org.junit.Test; import static org.junit.Assert.assertTrue; -public final class SPIParsingHookTest { +public final class ParsingHookRegistryTest { - private SPIParsingHook spiParsingHook; + private ParsingHookRegistry registry = ParsingHookRegistry.getInstance(); @Before public void setUp() { ParsingHookFixture.clearActions(); - spiParsingHook = new SPIParsingHook(); } @Test public void assertStart() { - spiParsingHook.start(""); + registry.start(""); assertTrue(ParsingHookFixture.containsAction("start")); } @Test public void assertFinishSuccess() { - spiParsingHook.finishSuccess(null); + registry.finishSuccess(null); assertTrue(ParsingHookFixture.containsAction("finishSuccess")); } @Test public void assertFinishFailure() { - spiParsingHook.finishFailure(null); + registry.finishFailure(null); assertTrue(ParsingHookFixture.containsAction("finishFailure")); } } -- GitLab