You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by me...@apache.org on 2021/09/28 13:18:47 UTC

[shardingsphere] branch master updated: Valid DataSourceConfigurationValidator (#12795)

This is an automated email from the ASF dual-hosted git repository.

menghaoran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git


The following commit(s) were added to refs/heads/master by this push:
     new ddbaec8  Valid DataSourceConfigurationValidator (#12795)
ddbaec8 is described below

commit ddbaec890aa4a4c2b21125b0c99a943aeaf21a1a
Author: Liang Zhang <te...@163.com>
AuthorDate: Tue Sep 28 21:18:13 2021 +0800

    Valid DataSourceConfigurationValidator (#12795)
---
 .../DataSourceConfigurationValidator.java          | 31 +++++++++++++++++-----
 .../DataSourceConfigurationValidatorTest.java      | 27 ++++++++++++++-----
 .../rdl/resource/AddResourceBackendHandler.java    | 25 +++--------------
 .../rdl/resource/AlterResourceBackendHandler.java  | 24 +++--------------
 4 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/datasource/DataSourceConfigurationValidator.java b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/datasource/DataSourceConfigurationValidator.java
index fd9dc49..dfc663e 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/datasource/DataSourceConfigurationValidator.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/config/datasource/DataSourceConfigurationValidator.java
@@ -17,7 +17,13 @@
 
 package org.apache.shardingsphere.infra.config.datasource;
 
+import org.apache.shardingsphere.infra.distsql.exception.resource.InvalidResourcesException;
+
 import javax.sql.DataSource;
+import java.util.Collection;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.Map.Entry;
 
 /**
  * Data source configuration validator.
@@ -25,13 +31,26 @@ import javax.sql.DataSource;
 public final class DataSourceConfigurationValidator {
     
     /**
-     * Validate data source configuration.
-     *
-     * @param dataSourceConfigName data source configuration name to be valid
-     * @param dataSourceConfig data source configuration to be valid
-     * @throws InvalidDataSourceConfigurationException invalid data source configuration exception
+     * Validate data source configurations.
+     * 
+     * @param dataSourceConfigs data source configurations
+     * @throws InvalidResourcesException invalid resources exception
      */
-    public void validate(final String dataSourceConfigName, final DataSourceConfiguration dataSourceConfig) throws InvalidDataSourceConfigurationException {
+    public void validate(final Map<String, DataSourceConfiguration> dataSourceConfigs) throws InvalidResourcesException {
+        Collection<String> errorMessages = new LinkedList<>();
+        for (Entry<String, DataSourceConfiguration> entry : dataSourceConfigs.entrySet()) {
+            try {
+                validate(entry.getKey(), entry.getValue());
+            } catch (final InvalidDataSourceConfigurationException ex) {
+                errorMessages.add(ex.getMessage());
+            }
+        }
+        if (!errorMessages.isEmpty()) {
+            throw new InvalidResourcesException(errorMessages);
+        }
+    }
+    
+    private void validate(final String dataSourceConfigName, final DataSourceConfiguration dataSourceConfig) throws InvalidDataSourceConfigurationException {
         DataSource dataSource = null;
         try {
             dataSource = DataSourceConverter.getDataSource(dataSourceConfig);
diff --git a/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/datasource/DataSourceConfigurationValidatorTest.java b/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/datasource/DataSourceConfigurationValidatorTest.java
index dcfc468..a5a60a9 100644
--- a/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/datasource/DataSourceConfigurationValidatorTest.java
+++ b/shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/datasource/DataSourceConfigurationValidatorTest.java
@@ -20,22 +20,23 @@ package org.apache.shardingsphere.infra.datasource;
 import com.zaxxer.hikari.HikariDataSource;
 import org.apache.shardingsphere.infra.config.datasource.DataSourceConfiguration;
 import org.apache.shardingsphere.infra.config.datasource.DataSourceConfigurationValidator;
-import org.apache.shardingsphere.infra.config.datasource.InvalidDataSourceConfigurationException;
+import org.apache.shardingsphere.infra.distsql.exception.resource.InvalidResourcesException;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 public final class DataSourceConfigurationValidatorTest {
     
     @Test
-    public void assertValidate() throws InvalidDataSourceConfigurationException {
-        DataSourceConfigurationValidator dataSourceConfigurationValidator = new DataSourceConfigurationValidator();
-        dataSourceConfigurationValidator.validate("name", createDataSourceConfiguration());
+    public void assertValidateSuccess() throws InvalidResourcesException {
+        DataSourceConfigurationValidator validator = new DataSourceConfigurationValidator();
+        validator.validate(Collections.singletonMap("name", createValidDataSourceConfiguration()));
     }
     
-    private DataSourceConfiguration createDataSourceConfiguration() {
-        Map<String, Object> props = new HashMap<>(16, 1);
+    private DataSourceConfiguration createValidDataSourceConfiguration() {
+        Map<String, Object> props = new HashMap<>();
         props.put("driverClassName", "org.h2.Driver");
         props.put("jdbcUrl", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1;DATABASE_TO_UPPER=false;MODE=MySQL");
         props.put("username", "root");
@@ -44,4 +45,18 @@ public final class DataSourceConfigurationValidatorTest {
         result.getProps().putAll(props);
         return result;
     }
+    
+    @Test(expected = InvalidResourcesException.class)
+    public void assertValidateFailed() throws InvalidResourcesException {
+        DataSourceConfigurationValidator validator = new DataSourceConfigurationValidator();
+        validator.validate(Collections.singletonMap("name", createInvalidDataSourceConfiguration()));
+    }
+    
+    private DataSourceConfiguration createInvalidDataSourceConfiguration() {
+        Map<String, Object> props = new HashMap<>();
+        props.put("driverClassName", "InvalidDriver");
+        DataSourceConfiguration result = new DataSourceConfiguration(HikariDataSource.class.getName());
+        result.getProps().putAll(props);
+        return result;
+    }
 }
diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AddResourceBackendHandler.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AddResourceBackendHandler.java
index db9e1fb..bc2edf8 100644
--- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AddResourceBackendHandler.java
+++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AddResourceBackendHandler.java
@@ -21,11 +21,9 @@ import org.apache.shardingsphere.distsql.parser.segment.DataSourceSegment;
 import org.apache.shardingsphere.distsql.parser.statement.rdl.create.AddResourceStatement;
 import org.apache.shardingsphere.infra.config.datasource.DataSourceConfiguration;
 import org.apache.shardingsphere.infra.config.datasource.DataSourceConfigurationValidator;
-import org.apache.shardingsphere.infra.config.datasource.InvalidDataSourceConfigurationException;
 import org.apache.shardingsphere.infra.database.type.DatabaseType;
 import org.apache.shardingsphere.infra.distsql.exception.DistSQLException;
 import org.apache.shardingsphere.infra.distsql.exception.resource.DuplicateResourceException;
-import org.apache.shardingsphere.infra.distsql.exception.resource.InvalidResourcesException;
 import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection;
 import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
 import org.apache.shardingsphere.proxy.backend.response.header.ResponseHeader;
@@ -35,12 +33,9 @@ import org.apache.shardingsphere.proxy.config.util.DataSourceParameterConverter;
 import org.apache.shardingsphere.proxy.converter.ResourceSegmentsConverter;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 
 /**
@@ -60,30 +55,16 @@ public final class AddResourceBackendHandler extends SchemaRequiredBackendHandle
     
     @Override
     public ResponseHeader execute(final String schemaName, final AddResourceStatement sqlStatement) throws DistSQLException {
-        check(schemaName, sqlStatement);
+        checkSQLStatement(schemaName, sqlStatement);
         Map<String, DataSourceConfiguration> dataSourceConfigs = DataSourceParameterConverter.getDataSourceConfigurationMap(
                 DataSourceParameterConverter.getDataSourceParameterMapFromYamlConfiguration(ResourceSegmentsConverter.convert(databaseType, sqlStatement.getDataSources())));
-        validateDataSourceConfigurations(dataSourceConfigs);
+        dataSourceConfigValidator.validate(dataSourceConfigs);
         // TODO update meta data context in memory
         ProxyContext.getInstance().getContextManager().getMetaDataContexts().getMetaDataPersistService().ifPresent(optional -> optional.getDataSourceService().append(schemaName, dataSourceConfigs));
         return new UpdateResponseHeader(sqlStatement);
     }
-        
-    private void validateDataSourceConfigurations(final Map<String, DataSourceConfiguration> dataSourceConfigs) throws InvalidResourcesException {
-        Collection<String> errorMessages = new LinkedList<>();
-        for (Entry<String, DataSourceConfiguration> entry : dataSourceConfigs.entrySet()) {
-            try {
-                dataSourceConfigValidator.validate(entry.getKey(), entry.getValue());
-            } catch (final InvalidDataSourceConfigurationException ex) {
-                errorMessages.add(ex.getMessage());
-            }
-        }
-        if (!errorMessages.isEmpty()) {
-            throw new InvalidResourcesException(errorMessages);
-        }
-    }
     
-    private void check(final String schemaName, final AddResourceStatement sqlStatement) throws DuplicateResourceException {
+    private void checkSQLStatement(final String schemaName, final AddResourceStatement sqlStatement) throws DuplicateResourceException {
         List<String> dataSourceNames = new ArrayList<>(sqlStatement.getDataSources().size());
         Set<String> duplicateDataSourceNames = new HashSet<>(sqlStatement.getDataSources().size(), 1);
         for (DataSourceSegment each : sqlStatement.getDataSources()) {
diff --git a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AlterResourceBackendHandler.java b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AlterResourceBackendHandler.java
index 0d69a88..1749e4f 100644
--- a/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AlterResourceBackendHandler.java
+++ b/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rdl/resource/AlterResourceBackendHandler.java
@@ -21,11 +21,9 @@ import org.apache.shardingsphere.distsql.parser.segment.DataSourceSegment;
 import org.apache.shardingsphere.distsql.parser.statement.rdl.alter.AlterResourceStatement;
 import org.apache.shardingsphere.infra.config.datasource.DataSourceConfiguration;
 import org.apache.shardingsphere.infra.config.datasource.DataSourceConfigurationValidator;
-import org.apache.shardingsphere.infra.config.datasource.InvalidDataSourceConfigurationException;
 import org.apache.shardingsphere.infra.database.type.DatabaseType;
 import org.apache.shardingsphere.infra.distsql.exception.DistSQLException;
 import org.apache.shardingsphere.infra.distsql.exception.resource.DuplicateResourceException;
-import org.apache.shardingsphere.infra.distsql.exception.resource.InvalidResourcesException;
 import org.apache.shardingsphere.infra.distsql.exception.resource.RequiredResourceMissedException;
 import org.apache.shardingsphere.proxy.backend.communication.jdbc.connection.BackendConnection;
 import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
@@ -37,9 +35,7 @@ import org.apache.shardingsphere.proxy.converter.ResourceSegmentsConverter;
 
 import javax.sql.DataSource;
 import java.util.Collection;
-import java.util.LinkedList;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.stream.Collectors;
 
 /**
@@ -59,36 +55,22 @@ public final class AlterResourceBackendHandler extends SchemaRequiredBackendHand
     
     @Override
     public ResponseHeader execute(final String schemaName, final AlterResourceStatement sqlStatement) throws DistSQLException {
-        check(schemaName, sqlStatement);
+        checkSQLStatement(schemaName, sqlStatement);
         Map<String, DataSourceConfiguration> dataSourceConfigs = DataSourceParameterConverter.getDataSourceConfigurationMap(
                 DataSourceParameterConverter.getDataSourceParameterMapFromYamlConfiguration(ResourceSegmentsConverter.convert(databaseType, sqlStatement.getDataSources())));
-        validateDataSourceConfigurations(dataSourceConfigs);
+        dataSourceConfigValidator.validate(dataSourceConfigs);
         // TODO update meta data context in memory
         ProxyContext.getInstance().getContextManager()
                 .getMetaDataContexts().getMetaDataPersistService().ifPresent(optional -> optional.getDataSourceService().append(schemaName, dataSourceConfigs));
         return new UpdateResponseHeader(sqlStatement);
     }
     
-    private void check(final String schemaName, final AlterResourceStatement sqlStatement) throws DuplicateResourceException, RequiredResourceMissedException {
+    private void checkSQLStatement(final String schemaName, final AlterResourceStatement sqlStatement) throws DuplicateResourceException, RequiredResourceMissedException {
         Collection<String> toBeAlteredResourceNames = getToBeAlteredResourceNames(sqlStatement);
         checkToBeAlteredDuplicateResourceNames(toBeAlteredResourceNames);
         checkResourceNameExisted(schemaName, toBeAlteredResourceNames);
     }
     
-    private void validateDataSourceConfigurations(final Map<String, DataSourceConfiguration> dataSourceConfigs) throws DistSQLException {
-        Collection<String> errorMessages = new LinkedList<>();
-        for (Entry<String, DataSourceConfiguration> entry : dataSourceConfigs.entrySet()) {
-            try {
-                dataSourceConfigValidator.validate(entry.getKey(), entry.getValue());
-            } catch (final InvalidDataSourceConfigurationException ex) {
-                errorMessages.add(ex.getMessage());
-            }
-        }
-        if (!errorMessages.isEmpty()) {
-            throw new InvalidResourcesException(errorMessages);
-        }
-    }
-    
     private Collection<String> getToBeAlteredResourceNames(final AlterResourceStatement sqlStatement) {
         return sqlStatement.getDataSources().stream().map(DataSourceSegment::getName).collect(Collectors.toList());
     }