From 28548c13e77baadafdb3bd98a43c18ef1c937fcd Mon Sep 17 00:00:00 2001 From: gongycn Date: Thu, 5 Dec 2024 17:30:59 +0800 Subject: [PATCH] [ISSUE #12342]: Improve the retrieval of ConfigInfoState to facilitate the extension and implementation of databases like Oracle. (#12343) * Improve the retrieval of ConfigInfoState to facilitate the extension and implementation of databases like Oracle. * Add unit tests for the SQL construction part of the `ExternalConfigInfoPersistServiceImpl#findConfigInfoState` method. * Enhance the construction of the delete statement in AbstractMapper by using a unified appendWhereClause method to construct the WHERE clause. Modify appendWhereClause to be protected, allowing for customization based on different database types, such as adjustments according to column names. --- .../ExternalConfigInfoPersistServiceImpl.java | 7 +++++-- .../ExternalConfigInfoPersistServiceImplTest.java | 14 +++++++++++++- .../plugin/datasource/mapper/AbstractMapper.java | 11 +++-------- .../datasource/mapper/AbstractMapperTest.java | 2 +- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/config/src/main/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImpl.java b/config/src/main/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImpl.java index 1f04f57a4c3..4c6b939edfe 100644 --- a/config/src/main/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImpl.java +++ b/config/src/main/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImpl.java @@ -1030,8 +1030,11 @@ public ConfigAllInfo findConfigAllInfo(final String dataId, final String group, public ConfigInfoStateWrapper findConfigInfoState(final String dataId, final String group, final String tenant) { String tenantTmp = StringUtils.isBlank(tenant) ? StringUtils.EMPTY : tenant; try { - return this.jt.queryForObject( - "SELECT id,data_id,group_id,tenant_id,gmt_modified FROM config_info WHERE data_id=? AND group_id=? AND tenant_id=?", + ConfigInfoMapper configInfoMapper = mapperManager.findMapper(dataSourceService.getDataSourceType(), + TableConstant.CONFIG_INFO); + return this.jt.queryForObject(configInfoMapper.select( + Arrays.asList("id", "data_id", "group_id", "tenant_id", "gmt_modified"), + Arrays.asList("data_id", "group_id", "tenant_id")), new Object[] {dataId, group, tenantTmp}, CONFIG_INFO_STATE_WRAPPER_ROW_MAPPER); } catch (EmptyResultDataAccessException e) { // Indicates that the data does not exist, returns null. return null; diff --git a/config/src/test/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImplTest.java b/config/src/test/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImplTest.java index 37d7614afe0..adce23f16e0 100644 --- a/config/src/test/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImplTest.java +++ b/config/src/test/java/com/alibaba/nacos/config/server/service/repository/extrnal/ExternalConfigInfoPersistServiceImplTest.java @@ -34,6 +34,7 @@ import com.alibaba.nacos.persistence.datasource.DataSourceService; import com.alibaba.nacos.persistence.datasource.DynamicDataSource; import com.alibaba.nacos.persistence.model.Page; +import com.alibaba.nacos.plugin.datasource.MapperManager; import com.alibaba.nacos.plugin.datasource.constants.TableConstant; import com.alibaba.nacos.plugin.datasource.mapper.ConfigInfoMapper; import com.alibaba.nacos.sys.env.EnvUtil; @@ -1114,7 +1115,7 @@ void testFindConfigInfoState() { assertTrue(e.getMessage().endsWith("mock exp")); } } - + @Test void testFindAllConfigInfo4Export() { @@ -1250,5 +1251,16 @@ void testFindAllConfigInfoFragment() { } } + + @Test + void testBuildFindConfigInfoStateSql() { + MapperManager mapperManager = MapperManager.instance(false); + ConfigInfoMapper configInfoMapper = mapperManager.findMapper(dataSourceService.getDataSourceType(), + TableConstant.CONFIG_INFO); + String select = configInfoMapper.select( + Arrays.asList("id", "data_id", "group_id", "tenant_id", "gmt_modified"), + Arrays.asList("data_id", "group_id", "tenant_id")); + assertEquals("SELECT id,data_id,group_id,tenant_id,gmt_modified FROM config_info WHERE data_id = ? AND group_id = ? AND tenant_id = ?", select); + } } diff --git a/plugin/datasource/src/main/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapper.java b/plugin/datasource/src/main/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapper.java index ba85b7ffaaa..b86e481efc9 100644 --- a/plugin/datasource/src/main/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapper.java +++ b/plugin/datasource/src/main/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapper.java @@ -121,13 +121,8 @@ public String update(List columns, List where) { public String delete(List params) { StringBuilder sql = new StringBuilder(); String method = "DELETE "; - sql.append(method).append("FROM ").append(getTableName()).append(" ").append("WHERE "); - for (int i = 0; i < params.size(); i++) { - sql.append(params.get(i)).append(" ").append("=").append(" ? "); - if (i != params.size() - 1) { - sql.append("AND "); - } - } + sql.append(method).append("FROM ").append(getTableName()).append(" "); + appendWhereClause(params, sql); return sql.toString(); } @@ -155,7 +150,7 @@ public String[] getPrimaryKeyGeneratedKeys() { return new String[]{"id"}; } - private void appendWhereClause(List where, StringBuilder sql) { + protected void appendWhereClause(List where, StringBuilder sql) { sql.append("WHERE "); for (int i = 0; i < where.size(); i++) { sql.append(where.get(i)).append(" = ").append("?"); diff --git a/plugin/datasource/src/test/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapperTest.java b/plugin/datasource/src/test/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapperTest.java index 899cb01adf1..6531881e782 100644 --- a/plugin/datasource/src/test/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapperTest.java +++ b/plugin/datasource/src/test/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapperTest.java @@ -71,7 +71,7 @@ void testUpdate() { @Test void testDelete() { String sql = abstractMapper.delete(Arrays.asList("id")); - assertEquals("DELETE FROM tenant_info WHERE id = ? ", sql); + assertEquals("DELETE FROM tenant_info WHERE id = ?", sql); } @Test