This is an automated email from the ASF dual-hosted git repository.
zhangliang 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 c4fb46f5a8c Improve coverage for transaction and unicast handlers
(#37564)
c4fb46f5a8c is described below
commit c4fb46f5a8ccafb5492b6835a8044624cdc70ebd
Author: Liang Zhang <[email protected]>
AuthorDate: Sun Dec 28 23:26:55 2025 +0800
Improve coverage for transaction and unicast handlers (#37564)
* Improve coverage for transaction and unicast handlers
* Improve coverage for transaction and unicast handlers
* Improve coverage for transaction and unicast handlers
---
AGENTS.md | 8 +++
.../jdbc/datasource/JDBCBackendDataSourceTest.java | 46 +++++++++++-
.../UnicastDatabaseProxyBackendHandlerTest.java | 83 ++++++++++++++++++++++
.../backend/session/ConnectionSessionTest.java | 10 +++
4 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index 4d570f81438..2188cfe9b70 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -80,6 +80,14 @@ This guide is written **for AI coding agents only**. Follow
it literally; improv
- Validate: run the narrowest meaningful checks (see Verification & Commands)
and prefer scoped runs; note any sandbox or limit blocks and alternatives.
- Report & self-check: share intent, edits, verification results, and next
steps; ensure all required instructions, coverage, and mocking rules are
satisfied, with remaining risks called out.
+## Compliance Guardrails & Checklists
+- **Pre-task checklist (do before planning/coding):** re-read AGENTS.md and
`CODE_OF_CONDUCT.md`; restate user goal, constraints, forbidden tools/APIs,
coverage expectations, sandbox/network/approval limits; prefer
`rg`/`./mvnw`/`apply_patch`; avoid destructive commands (`git reset --hard`,
`git checkout --`, bulk deletes) and generated paths like `target/`.
+- **Risk gate:** if any action fits the Dangerous Operation Checklist, pause
and use the confirmation template before proceeding.
+- **Planning rules:** use Sequential Thinking with 3-10 actionable steps (no
single-step plans) via the plan tool for non-trivial tasks; convert all hard
requirements (SPI usage, mocking rules, coverage/test naming, forbidden APIs)
into a checklist inside the plan and do not code until each item is addressed
or explicitly waived.
+- **Execution discipline:** inspect existing code before edits; keep changes
minimal; default to mocks and SPI loaders; keep variable declarations near
first use and mark retained values `final`; delete dead code and avoid
placeholders/TODOs.
+- **Post-task self-check (before replying):** confirm all instructions were
honored; verify no placeholders/unused code; ensure Checkstyle/Spotless gates
for touched modules are satisfied or explain why not run and what to run; list
commands with exit codes; call out risks and follow-ups.
+- **Final response template:** include intent/why, changed files with paths,
rationale per file/section, commands run (with exit codes), verification
status, and remaining risks/next actions (if tests skipped, state reason and
the exact command to run).
+
## Coverage & Branch Checklist
- When coverage targets are declared (including 100%), list every branch/path
with its planned test before coding.
- Map each branch to exactly one test; add cases until all declared branches
are covered or explicitly waived.
diff --git
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/datasource/JDBCBackendDataSourceTest.java
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/datasource/JDBCBackendDataSourceTest.java
index 13826d8f425..0112f1105dc 100644
---
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/datasource/JDBCBackendDataSourceTest.java
+++
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/datasource/JDBCBackendDataSourceTest.java
@@ -18,6 +18,7 @@
package org.apache.shardingsphere.proxy.backend.connector.jdbc.datasource;
import lombok.RequiredArgsConstructor;
+import
org.apache.shardingsphere.database.connector.core.GlobalDataSourceRegistry;
import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
import org.apache.shardingsphere.infra.config.props.ConfigurationProperties;
import
org.apache.shardingsphere.infra.exception.kernel.connection.OverallConnectionNotEnoughException;
@@ -25,6 +26,7 @@ import
org.apache.shardingsphere.infra.executor.sql.execute.engine.ConnectionMod
import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
import
org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import
org.apache.shardingsphere.infra.metadata.database.resource.ResourceMetaData;
+import
org.apache.shardingsphere.infra.metadata.database.resource.unit.StorageUnit;
import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
import
org.apache.shardingsphere.infra.metadata.statistics.ShardingSphereStatistics;
import
org.apache.shardingsphere.infra.metadata.statistics.builder.ShardingSphereStatisticsFactory;
@@ -35,13 +37,14 @@ import
org.apache.shardingsphere.proxy.backend.connector.jdbc.datasource.fixture
import org.apache.shardingsphere.proxy.backend.context.ProxyContext;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.AutoMockExtension;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.StaticMockSettings;
+import
org.apache.shardingsphere.transaction.ShardingSphereTransactionManagerEngine;
+import org.apache.shardingsphere.transaction.api.TransactionType;
import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import
org.apache.shardingsphere.transaction.spi.ShardingSphereDistributedTransactionManager;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.MockedStatic;
-import org.mockito.junit.jupiter.MockitoSettings;
-import org.mockito.quality.Strictness;
import javax.sql.DataSource;
import java.sql.Connection;
@@ -67,11 +70,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(AutoMockExtension.class)
@StaticMockSettings(ProxyContext.class)
-@MockitoSettings(strictness = Strictness.LENIENT)
class JDBCBackendDataSourceTest {
private static final String DATA_SOURCE_PATTERN = "ds_%s";
@@ -118,6 +122,42 @@ class JDBCBackendDataSourceTest {
assertThrows(OverallConnectionNotEnoughException.class, () -> new
JDBCBackendDataSource().getConnections("schema",
String.format(DATA_SOURCE_PATTERN, 1), 6, ConnectionMode.MEMORY_STRICTLY));
}
+ @Test
+ void assertGetConnectionsWithConnectionStrictlyMode() throws SQLException {
+ List<Connection> actual = new
JDBCBackendDataSource().getConnections("schema",
String.format(DATA_SOURCE_PATTERN, 0), 2, ConnectionMode.CONNECTION_STRICTLY);
+ assertThat(actual.size(), is(2));
+ }
+
+ @Test
+ void assertGetConnectionsFromDistributedTransactionManagerAndSetCatalog()
throws SQLException {
+ ContextManager contextManager = mock(ContextManager.class,
RETURNS_DEEP_STUBS);
+ StorageUnit storageUnit = mock(StorageUnit.class, RETURNS_DEEP_STUBS);
+ DataSource dataSource = mock(DataSource.class);
+ when(storageUnit.getDataSource()).thenReturn(dataSource);
+
when(contextManager.getMetaDataContexts().getMetaData().getDatabase("schema").getResourceMetaData().getStorageUnits().get("cached.ds1")).thenReturn(storageUnit);
+ ShardingSphereDistributedTransactionManager
distributedTransactionManager =
mock(ShardingSphereDistributedTransactionManager.class);
+ when(distributedTransactionManager.isInTransaction()).thenReturn(true);
+ Connection connection = mock(Connection.class);
+ when(distributedTransactionManager.getConnection("schema",
"cached.ds1")).thenReturn(connection);
+ ShardingSphereTransactionManagerEngine engine =
mock(ShardingSphereTransactionManagerEngine.class);
+
when(engine.getTransactionManager(TransactionType.XA)).thenReturn(distributedTransactionManager);
+ TransactionRule transactionRule = mock(TransactionRule.class);
+ when(transactionRule.getDefaultType()).thenReturn(TransactionType.XA);
+ when(transactionRule.getResource()).thenReturn(engine);
+ RuleMetaData ruleMetaData = new
RuleMetaData(Collections.singleton(transactionRule));
+
when(contextManager.getMetaDataContexts().getMetaData().getGlobalRuleMetaData()).thenReturn(ruleMetaData);
+
GlobalDataSourceRegistry.getInstance().getCachedDataSources().put("cached",
dataSource);
+ try {
+
when(ProxyContext.getInstance().getContextManager()).thenReturn(contextManager);
+ List<Connection> actual = new
JDBCBackendDataSource().getConnections("schema", "cached.ds1", 1,
ConnectionMode.CONNECTION_STRICTLY);
+ assertThat(actual.size(), is(1));
+ verify(connection).setCatalog("ds1");
+ verify(dataSource, never()).getConnection();
+ } finally {
+
GlobalDataSourceRegistry.getInstance().getCachedDataSources().clear();
+ }
+ }
+
@Test
void assertGetConnectionsByMultiThreads() throws InterruptedException {
JDBCBackendDataSource jdbcBackendDataSource = new
JDBCBackendDataSource();
diff --git
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/type/UnicastDatabaseProxyBackendHandlerTest.java
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/type/UnicastDatabaseProxyBackendHandlerTest.java
index 29401d05bbf..d5ac667cd95 100644
---
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/type/UnicastDatabaseProxyBackendHandlerTest.java
+++
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/handler/data/type/UnicastDatabaseProxyBackendHandlerTest.java
@@ -17,12 +17,15 @@
package org.apache.shardingsphere.proxy.backend.handler.data.type;
+import org.apache.shardingsphere.authority.model.ShardingSpherePrivileges;
+import org.apache.shardingsphere.authority.rule.AuthorityRule;
import org.apache.shardingsphere.database.connector.core.type.DatabaseType;
import
org.apache.shardingsphere.infra.binder.context.statement.SQLStatementContext;
import org.apache.shardingsphere.infra.hint.HintValueContext;
import org.apache.shardingsphere.infra.metadata.ShardingSphereMetaData;
import
org.apache.shardingsphere.infra.metadata.database.ShardingSphereDatabase;
import
org.apache.shardingsphere.infra.metadata.database.resource.ResourceMetaData;
+import org.apache.shardingsphere.infra.metadata.database.rule.RuleMetaData;
import
org.apache.shardingsphere.infra.metadata.statistics.ShardingSphereStatistics;
import
org.apache.shardingsphere.infra.metadata.statistics.builder.ShardingSphereStatisticsFactory;
import org.apache.shardingsphere.infra.session.connection.ConnectionContext;
@@ -35,6 +38,7 @@ import
org.apache.shardingsphere.proxy.backend.connector.DatabaseProxyConnectorF
import
org.apache.shardingsphere.proxy.backend.connector.ProxyDatabaseConnectionManager;
import org.apache.shardingsphere.proxy.backend.response.header.ResponseHeader;
import
org.apache.shardingsphere.proxy.backend.response.header.update.UpdateResponseHeader;
+import org.apache.shardingsphere.proxy.backend.response.data.QueryResponseRow;
import org.apache.shardingsphere.proxy.backend.session.ConnectionSession;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.AutoMockExtension;
import
org.apache.shardingsphere.test.infra.framework.extension.mock.StaticMockSettings;
@@ -43,9 +47,13 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.sql.SQLException;
+import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.Optional;
import static org.hamcrest.CoreMatchers.is;
@@ -55,10 +63,13 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(AutoMockExtension.class)
@StaticMockSettings(DatabaseProxyConnectorFactory.class)
+@MockitoSettings(strictness = Strictness.LENIENT)
class UnicastDatabaseProxyBackendHandlerTest {
private static final String EXECUTE_SQL = "SELECT 1 FROM user WHERE id =
1";
@@ -114,4 +125,76 @@ class UnicastDatabaseProxyBackendHandlerTest {
assertThat(unicastDatabaseProxyBackendHandler.getRowData().getData().size(),
is(1));
}
}
+
+ @Test
+ void assertExecuteWithNullCurrentDatabaseChoosesFirstAvailable() throws
SQLException {
+ ConnectionSession connectionSession = mock(ConnectionSession.class,
RETURNS_DEEP_STUBS);
+
when(connectionSession.getConnectionContext().getCurrentDatabaseName()).thenReturn(Optional.empty());
+ AuthorityRule authorityRule = mock(AuthorityRule.class,
RETURNS_DEEP_STUBS);
+ ShardingSpherePrivileges privileges =
mock(ShardingSpherePrivileges.class);
+ when(privileges.hasPrivileges("bar_db")).thenReturn(true);
+
when(authorityRule.findPrivileges(any())).thenReturn(Optional.of(privileges));
+ ContextManager contextManager =
mockContextManagerWithAuthority(authorityRule, Arrays.asList("foo_db",
"bar_db"), Collections.singletonList("bar_db"));
+
when(DatabaseProxyConnectorFactory.newInstance(any(QueryContext.class),
any(ProxyDatabaseConnectionManager.class), eq(false)))
+ .thenReturn(mock(DatabaseProxyConnector.class,
RETURNS_DEEP_STUBS));
+ QueryContext queryContext = new
QueryContext(mock(SQLStatementContext.class, RETURNS_DEEP_STUBS), EXECUTE_SQL,
Collections.emptyList(), new HintValueContext(),
+ connectionSession.getConnectionContext(),
contextManager.getMetaDataContexts().getMetaData());
+ new UnicastDatabaseProxyBackendHandler(queryContext, contextManager,
connectionSession).execute();
+ verify(connectionSession).setCurrentDatabaseName("bar_db");
+ }
+
+ @Test
+ void assertGetRowDataAndCloseAfterExecute() throws SQLException {
+ AuthorityRule authorityRule = mock(AuthorityRule.class,
RETURNS_DEEP_STUBS);
+ ShardingSpherePrivileges privileges =
mock(ShardingSpherePrivileges.class);
+ when(privileges.hasPrivileges("foo_db")).thenReturn(true);
+
when(authorityRule.findPrivileges(any())).thenReturn(Optional.of(privileges));
+ ContextManager contextManager =
mockContextManagerWithAuthority(authorityRule,
Collections.singletonList("foo_db"), Collections.singletonList("foo_db"));
+ ConnectionSession connectionSession = mock(ConnectionSession.class,
RETURNS_DEEP_STUBS);
+ when(connectionSession.getCurrentDatabaseName()).thenReturn("foo_db");
+
when(connectionSession.getConnectionContext().getCurrentDatabaseName()).thenReturn(Optional.of("foo_db"));
+ DatabaseProxyConnector connector = mock(DatabaseProxyConnector.class);
+ QueryResponseRow row = mock(QueryResponseRow.class);
+ when(connector.execute()).thenReturn(new UpdateResponseHeader(mock()));
+ when(connector.next()).thenReturn(true, false);
+ when(connector.getRowData()).thenReturn(row);
+
when(DatabaseProxyConnectorFactory.newInstance(any(QueryContext.class),
any(ProxyDatabaseConnectionManager.class), eq(false))).thenReturn(connector);
+ QueryContext queryContext = new
QueryContext(mock(SQLStatementContext.class, RETURNS_DEEP_STUBS), EXECUTE_SQL,
Collections.emptyList(), new HintValueContext(),
+ connectionSession.getConnectionContext(),
contextManager.getMetaDataContexts().getMetaData());
+ UnicastDatabaseProxyBackendHandler handler = new
UnicastDatabaseProxyBackendHandler(queryContext, contextManager,
connectionSession);
+ handler.execute();
+ handler.next();
+ assertThat(handler.getRowData(), is(row));
+ handler.close();
+ verify(connector).close();
+ }
+
+ @Test
+ void assertCloseWithoutExecute() throws SQLException {
+ AuthorityRule authorityRule = mock(AuthorityRule.class,
RETURNS_DEEP_STUBS);
+ ContextManager contextManager =
mockContextManagerWithAuthority(authorityRule,
Collections.singletonList("foo_db"), Collections.singletonList("foo_db"));
+ ConnectionSession connectionSession = mock(ConnectionSession.class,
RETURNS_DEEP_STUBS);
+ QueryContext queryContext = new
QueryContext(mock(SQLStatementContext.class, RETURNS_DEEP_STUBS), EXECUTE_SQL,
Collections.emptyList(), new HintValueContext(),
+ connectionSession.getConnectionContext(),
contextManager.getMetaDataContexts().getMetaData());
+ DatabaseProxyConnector connector = mock(DatabaseProxyConnector.class);
+
when(DatabaseProxyConnectorFactory.newInstance(any(QueryContext.class),
any(ProxyDatabaseConnectionManager.class), eq(false))).thenReturn(connector);
+ new UnicastDatabaseProxyBackendHandler(queryContext, contextManager,
connectionSession).close();
+ verify(connector, never()).close();
+ }
+
+ private ContextManager mockContextManagerWithAuthority(final AuthorityRule
authorityRule, final List<String> databaseNames, final List<String>
databasesWithStorageUnit) {
+ ContextManager result = mock(ContextManager.class, RETURNS_DEEP_STUBS);
+ when(result.getAllDatabaseNames()).thenReturn(databaseNames);
+ databaseNames.forEach(each -> {
+ ShardingSphereDatabase database =
mock(ShardingSphereDatabase.class);
+
when(database.containsDataSource()).thenReturn(databasesWithStorageUnit.contains(each));
+ when(result.getDatabase(each)).thenReturn(database);
+ });
+ ShardingSphereMetaData metaData = mock(ShardingSphereMetaData.class,
RETURNS_DEEP_STUBS);
+ RuleMetaData ruleMetaData = new
RuleMetaData(Collections.singleton(authorityRule));
+ when(metaData.getGlobalRuleMetaData()).thenReturn(ruleMetaData);
+ when(result.getMetaDataContexts().getMetaData()).thenReturn(metaData);
+
when(result.getMetaDataContexts().getMetaData().getGlobalRuleMetaData()).thenReturn(ruleMetaData);
+ return result;
+ }
}
diff --git
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/session/ConnectionSessionTest.java
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/session/ConnectionSessionTest.java
index cf97fd94c62..81178e5d818 100644
---
a/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/session/ConnectionSessionTest.java
+++
b/proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/session/ConnectionSessionTest.java
@@ -50,6 +50,7 @@ import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.when;
@ExtendWith(AutoMockExtension.class)
@@ -82,6 +83,15 @@ class ConnectionSessionTest {
verify(contextMock, times(1)).setCurrentDatabaseName("db");
}
+ @Test
+ void assertSetCurrentSchemaWithSameNameDoNothing() throws
ReflectiveOperationException {
+ connectionSession.setCurrentDatabaseName("db");
+ ConnectionContext contextMock = mock(ConnectionContext.class);
+ getConnectionContextReference().set(contextMock);
+ connectionSession.setCurrentDatabaseName("db");
+ verify(contextMock, never()).setCurrentDatabaseName("db");
+ }
+
@Test
void assertSetCurrentSchemaWithNull() throws ReflectiveOperationException {
connectionSession.setCurrentDatabaseName("db");
