From 0bbc61f258e6cbf4bbb9178dfbc6d63bdfb0bd28 Mon Sep 17 00:00:00 2001 From: Liang Zhang Date: Mon, 16 Nov 2020 15:42:32 +0800 Subject: [PATCH] Do not permit return null for ProxyContext.getMetaData() (#8168) --- .../DatabaseCommunicationEngineFactory.java | 11 ++------- .../proxy/backend/context/ProxyContext.java | 13 ++++++---- .../text/admin/ShowTablesBackendHandler.java | 10 ++------ .../text/query/QueryBackendHandler.java | 8 +------ .../ShardingCTLExplainBackendHandler.java | 4 ---- .../executor/HintShowTableStatusExecutor.java | 4 ---- .../backend/context/ProxyContextTest.java | 24 ++++++++++++++++--- .../bind/PostgreSQLComBindExecutor.java | 6 ++--- 8 files changed, 36 insertions(+), 44 deletions(-) diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngineFactory.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngineFactory.java index 89897b8887..5fd12127ab 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngineFactory.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/DatabaseCommunicationEngineFactory.java @@ -17,7 +17,6 @@ package org.apache.shardingsphere.proxy.backend.communication; -import com.google.common.base.Preconditions; import lombok.AccessLevel; import lombok.NoArgsConstructor; import org.apache.shardingsphere.infra.binder.LogicSQL; @@ -62,7 +61,7 @@ public final class DatabaseCommunicationEngineFactory { * @return text protocol backend handler */ public DatabaseCommunicationEngine newTextProtocolInstance(final SQLStatement sqlStatement, final String sql, final BackendConnection backendConnection) { - ShardingSphereMetaData metaData = getMetaData(backendConnection); + ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); LogicSQL logicSQL = createLogicSQL(sqlStatement, sql, Collections.emptyList(), metaData); JDBCExecuteEngine jdbcExecuteEngine = new JDBCExecuteEngine(backendConnection, new StatementAccessor()); return new JDBCDatabaseCommunicationEngine(logicSQL, metaData, jdbcExecuteEngine); @@ -78,18 +77,12 @@ public final class DatabaseCommunicationEngineFactory { * @return binary protocol backend handler */ public DatabaseCommunicationEngine newBinaryProtocolInstance(final SQLStatement sqlStatement, final String sql, final List parameters, final BackendConnection backendConnection) { - ShardingSphereMetaData metaData = getMetaData(backendConnection); + ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); LogicSQL logicSQL = createLogicSQL(sqlStatement, sql, new ArrayList<>(parameters), metaData); JDBCExecuteEngine jdbcExecuteEngine = new JDBCExecuteEngine(backendConnection, new PreparedStatementAccessor()); return new JDBCDatabaseCommunicationEngine(logicSQL, metaData, jdbcExecuteEngine); } - private ShardingSphereMetaData getMetaData(final BackendConnection backendConnection) { - ShardingSphereMetaData result = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); - Preconditions.checkNotNull(result); - return result; - } - private LogicSQL createLogicSQL(final SQLStatement sqlStatement, final String sql, final List parameters, final ShardingSphereMetaData metaData) { SQLStatementContext sqlStatementContext = SQLStatementContextFactory.newInstance(metaData.getSchema(), parameters, sqlStatement); return new LogicSQL(sqlStatementContext, sql, parameters); diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/context/ProxyContext.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/context/ProxyContext.java index 9b70febb03..f8a1d263c0 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/context/ProxyContext.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/context/ProxyContext.java @@ -23,6 +23,7 @@ import org.apache.shardingsphere.infra.context.metadata.MetaDataContexts; import org.apache.shardingsphere.infra.context.metadata.impl.StandardMetaDataContexts; import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; import org.apache.shardingsphere.proxy.backend.communication.jdbc.datasource.JDBCBackendDataSource; +import org.apache.shardingsphere.proxy.backend.exception.NoDatabaseSelectedException; import org.apache.shardingsphere.transaction.context.TransactionContexts; import org.apache.shardingsphere.transaction.context.impl.StandardTransactionContexts; @@ -30,7 +31,6 @@ import javax.sql.DataSource; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; /** @@ -84,13 +84,16 @@ public final class ProxyContext { } /** - * Get meta data. + * Get ShardingSphere meta data. * * @param schemaName schema name - * @return meta data + * @return ShardingSphere meta data */ public ShardingSphereMetaData getMetaData(final String schemaName) { - return Strings.isNullOrEmpty(schemaName) ? null : metaDataContexts.getMetaDataMap().get(schemaName); + if (Strings.isNullOrEmpty(schemaName) || !metaDataContexts.getMetaDataMap().containsKey(schemaName)) { + throw new NoDatabaseSelectedException(); + } + return metaDataContexts.getMetaDataMap().get(schemaName); } /** @@ -112,7 +115,7 @@ public final class ProxyContext { if (schemaNames.isEmpty()) { return Optional.empty(); } - Map dataSources = Objects.requireNonNull(getMetaData(schemaNames.get(0))).getResource().getDataSources(); + Map dataSources = getMetaData(schemaNames.get(0)).getResource().getDataSources(); return dataSources.values().stream().findFirst(); } } diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/ShowTablesBackendHandler.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/ShowTablesBackendHandler.java index c429ba5224..6043376a47 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/ShowTablesBackendHandler.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/admin/ShowTablesBackendHandler.java @@ -19,12 +19,10 @@ package org.apache.shardingsphere.proxy.backend.text.admin; import lombok.RequiredArgsConstructor; import org.apache.shardingsphere.infra.executor.sql.raw.execute.result.query.QueryHeader; -import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; import org.apache.shardingsphere.proxy.backend.communication.DatabaseCommunicationEngine; import org.apache.shardingsphere.proxy.backend.communication.DatabaseCommunicationEngineFactory; import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection; import org.apache.shardingsphere.proxy.backend.context.ProxyContext; -import org.apache.shardingsphere.proxy.backend.exception.NoDatabaseSelectedException; import org.apache.shardingsphere.proxy.backend.response.BackendResponse; import org.apache.shardingsphere.proxy.backend.response.query.QueryData; import org.apache.shardingsphere.proxy.backend.response.query.QueryResponse; @@ -53,14 +51,10 @@ public final class ShowTablesBackendHandler implements TextProtocolBackendHandle @Override public BackendResponse execute() throws SQLException { - ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); - if (null == metaData) { - throw new NoDatabaseSelectedException(); - } - if (!metaData.isComplete()) { + if (!ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()).isComplete()) { return getDefaultQueryResponse(backendConnection.getSchemaName()); } - // TODO Get all tables from meta data. + // TODO Get all tables from meta data databaseCommunicationEngine = databaseCommunicationEngineFactory.newTextProtocolInstance(sqlStatement, sql, backendConnection); return databaseCommunicationEngine.execute(); } diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/query/QueryBackendHandler.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/query/QueryBackendHandler.java index 1aa191ecef..a89454285a 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/query/QueryBackendHandler.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/query/QueryBackendHandler.java @@ -18,12 +18,10 @@ package org.apache.shardingsphere.proxy.backend.text.query; import lombok.RequiredArgsConstructor; -import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; import org.apache.shardingsphere.proxy.backend.communication.DatabaseCommunicationEngine; import org.apache.shardingsphere.proxy.backend.communication.DatabaseCommunicationEngineFactory; import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection; import org.apache.shardingsphere.proxy.backend.context.ProxyContext; -import org.apache.shardingsphere.proxy.backend.exception.NoDatabaseSelectedException; import org.apache.shardingsphere.proxy.backend.exception.RuleNotExistsException; import org.apache.shardingsphere.proxy.backend.response.BackendResponse; import org.apache.shardingsphere.proxy.backend.response.query.QueryData; @@ -50,11 +48,7 @@ public final class QueryBackendHandler implements TextProtocolBackendHandler { @Override public BackendResponse execute() throws SQLException { - ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); - if (null == metaData) { - throw new NoDatabaseSelectedException(); - } - if (!metaData.isComplete()) { + if (!ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()).isComplete()) { throw new RuleNotExistsException(); } databaseCommunicationEngine = databaseCommunicationEngineFactory.newTextProtocolInstance(sqlStatement, sql, backendConnection); diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/explain/ShardingCTLExplainBackendHandler.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/explain/ShardingCTLExplainBackendHandler.java index e44b2c0d34..e61ffdcfc8 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/explain/ShardingCTLExplainBackendHandler.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/explain/ShardingCTLExplainBackendHandler.java @@ -29,7 +29,6 @@ import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; import org.apache.shardingsphere.infra.parser.ShardingSphereSQLParserEngine; import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection; import org.apache.shardingsphere.proxy.backend.context.ProxyContext; -import org.apache.shardingsphere.proxy.backend.exception.NoDatabaseSelectedException; import org.apache.shardingsphere.proxy.backend.exception.RuleNotExistsException; import org.apache.shardingsphere.proxy.backend.response.BackendResponse; import org.apache.shardingsphere.proxy.backend.response.query.QueryData; @@ -68,9 +67,6 @@ public final class ShardingCTLExplainBackendHandler implements TextProtocolBacke throw new InvalidShardingCTLFormatException(sql); } ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); - if (null == metaData) { - throw new NoDatabaseSelectedException(); - } if (!metaData.isComplete()) { throw new RuleNotExistsException(); } diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/hint/internal/executor/HintShowTableStatusExecutor.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/hint/internal/executor/HintShowTableStatusExecutor.java index 2d4ac488eb..1856c581c4 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/hint/internal/executor/HintShowTableStatusExecutor.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/sctl/hint/internal/executor/HintShowTableStatusExecutor.java @@ -25,7 +25,6 @@ import org.apache.shardingsphere.infra.merge.result.MergedResult; import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection; import org.apache.shardingsphere.proxy.backend.context.ProxyContext; -import org.apache.shardingsphere.proxy.backend.exception.NoDatabaseSelectedException; import org.apache.shardingsphere.proxy.backend.exception.RuleNotExistsException; import org.apache.shardingsphere.proxy.backend.text.sctl.hint.internal.command.HintShowTableStatusCommand; import org.apache.shardingsphere.proxy.backend.text.sctl.hint.internal.result.HintShowTableStatusResult; @@ -60,9 +59,6 @@ public final class HintShowTableStatusExecutor extends AbstractHintQueryExecutor protected MergedResult createMergedResult() { Map results = new HashMap<>(); ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); - if (null == metaData) { - throw new NoDatabaseSelectedException(); - } if (!metaData.isComplete()) { throw new RuleNotExistsException(); } diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/context/ProxyContextTest.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/context/ProxyContextTest.java index 3ab8d36f02..2feeed155e 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/context/ProxyContextTest.java +++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/test/java/org/apache/shardingsphere/proxy/backend/context/ProxyContextTest.java @@ -25,6 +25,7 @@ import org.apache.shardingsphere.infra.database.type.dialect.MySQLDatabaseType; import org.apache.shardingsphere.infra.executor.kernel.ExecutorKernel; import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; import org.apache.shardingsphere.jdbc.test.MockedDataSource; +import org.apache.shardingsphere.proxy.backend.exception.NoDatabaseSelectedException; import org.apache.shardingsphere.transaction.context.TransactionContexts; import org.junit.Test; @@ -86,6 +87,26 @@ public final class ProxyContextTest { assertFalse(ProxyContext.getInstance().schemaExists("schema_2")); } + @Test(expected = NoDatabaseSelectedException.class) + public void assertGetSchemaWithNull() { + assertNull(ProxyContext.getInstance().getMetaData(null)); + } + + @Test(expected = NoDatabaseSelectedException.class) + public void assertGetSchemaWithEmptyString() { + assertNull(ProxyContext.getInstance().getMetaData("")); + } + + @Test(expected = NoDatabaseSelectedException.class) + public void assertGetSchemaWhenNotExisted() throws NoSuchFieldException, IllegalAccessException { + Map metaDataMap = mockMetaDataMap(Collections.emptyMap()); + Field metaDataContexts = ProxyContext.getInstance().getClass().getDeclaredField("metaDataContexts"); + metaDataContexts.setAccessible(true); + metaDataContexts.set(ProxyContext.getInstance(), + new StandardMetaDataContexts(metaDataMap, mock(ExecutorKernel.class), new Authentication(), new ConfigurationProperties(new Properties()), new MySQLDatabaseType())); + ProxyContext.getInstance().getMetaData("schema1"); + } + @Test public void assertGetSchema() throws NoSuchFieldException, IllegalAccessException { Map metaDataMap = mockMetaDataMap(Collections.emptyMap()); @@ -93,9 +114,6 @@ public final class ProxyContextTest { metaDataContexts.setAccessible(true); metaDataContexts.set(ProxyContext.getInstance(), new StandardMetaDataContexts(metaDataMap, mock(ExecutorKernel.class), new Authentication(), new ConfigurationProperties(new Properties()), new MySQLDatabaseType())); - assertNull(ProxyContext.getInstance().getMetaData(null)); - assertNull(ProxyContext.getInstance().getMetaData("")); - assertNull(ProxyContext.getInstance().getMetaData("schema1")); assertThat(metaDataMap.get("schema"), is(ProxyContext.getInstance().getMetaData("schema"))); } diff --git a/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/binary/bind/PostgreSQLComBindExecutor.java b/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/binary/bind/PostgreSQLComBindExecutor.java index a8f6f275d2..0801e0d715 100644 --- a/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/binary/bind/PostgreSQLComBindExecutor.java +++ b/shardingsphere-proxy/shardingsphere-proxy-frontend/shardingsphere-proxy-frontend-postgresql/src/main/java/org/apache/shardingsphere/proxy/frontend/postgresql/command/query/binary/bind/PostgreSQLComBindExecutor.java @@ -32,7 +32,7 @@ import org.apache.shardingsphere.db.protocol.postgresql.packet.generic.PostgreSQ import org.apache.shardingsphere.infra.database.type.DatabaseTypeRegistry; import org.apache.shardingsphere.infra.executor.sql.QueryResult; import org.apache.shardingsphere.infra.executor.sql.raw.execute.result.query.QueryHeader; -import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData; +import org.apache.shardingsphere.infra.parser.ShardingSphereSQLParserEngine; import org.apache.shardingsphere.proxy.backend.communication.DatabaseCommunicationEngine; import org.apache.shardingsphere.proxy.backend.communication.DatabaseCommunicationEngineFactory; import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection; @@ -43,7 +43,6 @@ import org.apache.shardingsphere.proxy.backend.response.query.QueryResponse; import org.apache.shardingsphere.proxy.backend.response.update.UpdateResponse; import org.apache.shardingsphere.proxy.frontend.command.executor.QueryCommandExecutor; import org.apache.shardingsphere.proxy.frontend.command.executor.ResponseType; -import org.apache.shardingsphere.infra.parser.ShardingSphereSQLParserEngine; import org.apache.shardingsphere.sql.parser.sql.common.statement.SQLStatement; import java.sql.ResultSetMetaData; @@ -69,8 +68,7 @@ public final class PostgreSQLComBindExecutor implements QueryCommandExecutor { public PostgreSQLComBindExecutor(final PostgreSQLComBindPacket packet, final BackendConnection backendConnection) { this.packet = packet; - ShardingSphereMetaData metaData = ProxyContext.getInstance().getMetaData(backendConnection.getSchemaName()); - if (null != packet.getSql() && null != metaData) { + if (null != packet.getSql()) { ShardingSphereSQLParserEngine sqlStatementParserEngine = new ShardingSphereSQLParserEngine( DatabaseTypeRegistry.getTrunkDatabaseTypeName(ProxyContext.getInstance().getMetaDataContexts().getDatabaseType())); SQLStatement sqlStatement = sqlStatementParserEngine.parse(packet.getSql(), true); -- GitLab