You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/08/19 06:49:52 UTC

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #11895: Refactor single table meta data load

strongduanmu commented on a change in pull request #11895:
URL: https://github.com/apache/shardingsphere/pull/11895#discussion_r691815108



##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/metadata/EncryptTableMetaDataBuilderTest.java
##########
@@ -175,19 +168,6 @@ private ResultSet createDataTypeResultSet() throws SQLException {
         return dataTypeResultSet;
     }
     
-    @Test
-    public void assertLoadByExistedTable() throws SQLException {
-        EncryptRule encryptRule = createEncryptRule();
-        Collection<ShardingSphereRule> rules = Arrays.asList(createSingleTableRule(), encryptRule);
-        EncryptTableMetaDataBuilder loader = (EncryptTableMetaDataBuilder) OrderedSPIRegistry.getRegisteredServices(RuleBasedTableMetaDataBuilder.class, rules).get(encryptRule);
-        when(databaseType.formatTableNamePattern(TABLE_NAME)).thenReturn(TABLE_NAME);
-        Optional<TableMetaData> actual = loader.load(TABLE_NAME, databaseType, Collections.singletonMap("logic_db", dataSource), new DataNodes(rules), encryptRule, props);
-        assertTrue(actual.isPresent());
-        assertThat(actual.get().getColumnMetaData(0).getName(), is("id"));
-        assertThat(actual.get().getColumnMetaData(1).getName(), is("pwd_cipher"));
-        assertThat(actual.get().getColumnMetaData(2).getName(), is("pwd_plain"));
-    }
-    
     @Test
     public void assertLoadByExistedTables() throws SQLException {

Review comment:
       @tuichenchuxin Please modify this method name, keep it consistent with the name of the method to be tested.

##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/metadata/schema/builder/TableMetaDataBuilder.java
##########
@@ -55,73 +55,52 @@
      * @throws SQLException SQL exception
      */
     public static Optional<TableMetaData> build(final String tableName, final SchemaBuilderMaterials materials) throws SQLException {
-        Optional<TableMetaData> tableMetaData = load(tableName, materials);
-        return tableMetaData.map(optional -> decorate(tableName, optional, materials.getRules()));
+        return Optional.ofNullable(load(Collections.singleton(tableName), materials).getOrDefault(tableName, null)).map(tableMetaData -> decorate(tableName, tableMetaData, materials.getRules()));
     }
     
     /**
-     * Load physical table metadata.
-     * 
-     * @param tableName table name
-     * @param materials schema builder materials
-     * @return table meta data
-     * @throws SQLException SQL exception
-     */
-    @SuppressWarnings({"unchecked", "rawtypes"})
-    public static Optional<TableMetaData> load(final String tableName, final SchemaBuilderMaterials materials) throws SQLException {
-        DataNodes dataNodes = new DataNodes(materials.getRules());
-        for (Entry<ShardingSphereRule, RuleBasedTableMetaDataBuilder> entry : OrderedSPIRegistry.getRegisteredServices(RuleBasedTableMetaDataBuilder.class, materials.getRules()).entrySet()) {
-            if (entry.getKey() instanceof TableContainedRule) {
-                TableContainedRule rule = (TableContainedRule) entry.getKey();
-                RuleBasedTableMetaDataBuilder loader = entry.getValue();
-                Optional<TableMetaData> result = loader.load(tableName, materials.getDatabaseType(), materials.getDataSourceMap(), dataNodes, rule, materials.getProps());
-                if (result.isPresent()) {
-                    TableMetaData tableMetaData = new TableMetaData(tableName, result.get().getColumns().values(), result.get().getIndexes().values());
-                    return Optional.of(tableMetaData);
-                }
-            }
-        }
-        return Optional.empty();
-    }
-    
-    /**
-     * Load logic table metadata.
+     * Load all table metadata.
      *
+     * @param tableNames table name collection
      * @param materials schema builder materials
-     * @param executorService executorService
-     * @return table meta data collection
+     * @return table meta data map
      * @throws SQLException SQL exception
      */
     @SuppressWarnings("rawtypes")
-    public static Collection<TableMetaData> loadLogicTables(final SchemaBuilderMaterials materials, final ExecutorService executorService) throws SQLException {
-        Collection<TableMetaData> result = new LinkedList<>();
+    public static Map<String, TableMetaData> load(final Collection<String> tableNames, final SchemaBuilderMaterials materials) throws SQLException {
+        Map<String, TableMetaData> result = new LinkedHashMap<>();
         for (Entry<ShardingSphereRule, RuleBasedTableMetaDataBuilder> entry : OrderedSPIRegistry.getRegisteredServices(RuleBasedTableMetaDataBuilder.class, materials.getRules()).entrySet()) {
             if (entry.getKey() instanceof TableContainedRule) {
-                loadTableContainedRuleTables(materials, executorService, result, entry);
+                loadTableContainedRuleTables(tableNames, materials, result, entry);
             }
         }
         return result;
     }
     
     @SuppressWarnings({"unchecked", "rawtypes"})
-    private static void loadTableContainedRuleTables(final SchemaBuilderMaterials materials, final ExecutorService executorService, final Collection<TableMetaData> result,
+    private static void loadTableContainedRuleTables(final Collection<String> tableNames, final SchemaBuilderMaterials materials, final Map<String, TableMetaData> result,
                                                      final Entry<ShardingSphereRule, RuleBasedTableMetaDataBuilder> ruleBuilderEntry) throws SQLException {
         TableContainedRule rule = (TableContainedRule) ruleBuilderEntry.getKey();
         RuleBasedTableMetaDataBuilder loader = ruleBuilderEntry.getValue();
-        Collection<String> loadedTables = result.stream().map(TableMetaData::getName).collect(Collectors.toSet());
-        Collection<String> needLoadTables = rule.getTables().stream().filter(each -> !loadedTables.contains(each)).collect(Collectors.toList());
+        Collection<String> needLoadTables = tableNames.stream().filter(each -> rule.getTables().contains(each)).filter(each -> !result.containsKey(each)).collect(Collectors.toList());
         if (!needLoadTables.isEmpty()) {
-            Map<String, TableMetaData> tableMetaDataMap = loader.load(needLoadTables, rule, materials, executorService);
-            result.addAll(tableMetaDataMap.entrySet().stream()
-                    .map(entry -> new TableMetaData(entry.getKey(), entry.getValue().getColumns().values(), entry.getValue().getIndexes().values())).collect(Collectors.toList()));
+            Map<String, TableMetaData> tableMetaDataMap = loader.load(needLoadTables, rule, materials);
+            Map<String, TableMetaData> metaDataMap = changeTableNameIfLogic(tableMetaDataMap);
+            result.putAll(metaDataMap);
         }
     }
     
+    private static Map<String, TableMetaData> changeTableNameIfLogic(final Map<String, TableMetaData> tableMetaDataMap) {
+        return tableMetaDataMap.entrySet().stream().map(entry -> new TableMetaData(entry.getKey(), entry.getValue().getColumns().values(), entry.getValue().getIndexes().values()))
+                .collect(Collectors.toMap(TableMetaData::getName, Function.identity(), (oldValue, currentValue) -> oldValue, LinkedHashMap::new));
+    }
+    
     /**
      * Load logic table metadata.
-     * @param tableName table name
+     *

Review comment:
       @tuichenchuxin Please modify java doc.

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/metadata/EncryptTableMetaDataBuilderTest.java
##########
@@ -212,16 +190,14 @@ public void assertLoadByExistedTablesH2() throws SQLException {
         EncryptTableMetaDataBuilder loader = (EncryptTableMetaDataBuilder) OrderedSPIRegistry.getRegisteredServices(RuleBasedTableMetaDataBuilder.class, rules).get(encryptRule);

Review comment:
       @tuichenchuxin This code appears in multiple test cases, can it be extracted into one method?




-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org