You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by zh...@apache.org on 2026/07/14 09:36:39 UTC
(shardingsphere) branch master updated: Clean up redundant MCP workflow helpers (#39108)
This is an automated email from the ASF dual-hosted git repository.
terrymanu 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 9d2a76d810d Clean up redundant MCP workflow helpers (#39108)
9d2a76d810d is described below
commit 9d2a76d810db5a16b6cb379231ebcc876e30bb9d
Author: Liang Zhang <zh...@apache.org>
AuthorDate: Tue Jul 14 17:36:32 2026 +0800
Clean up redundant MCP workflow helpers (#39108)
---
.../core/workflow/WorkflowExecutionService.java | 8 +----
.../workflow/WorkflowExecutionServiceTest.java | 34 ++++++++++++----------
.../handler/MaskWorkflowToolResponseBuilder.java | 5 +---
.../ShardingAlgorithmRecommendationService.java | 6 +---
.../MCPWorkflowApplySynchronizationHandler.java | 3 --
5 files changed, 22 insertions(+), 34 deletions(-)
diff --git a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionService.java b/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionService.java
index 45dd484de34..2237fc0ba4b 100644
--- a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionService.java
+++ b/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionService.java
@@ -254,7 +254,7 @@ public final class WorkflowExecutionService {
applyOutcome.addExecutedArtifact(each);
}
if (!applyOutcome.hasSkippedArtifacts()) {
- synchronizeAppliedWorkflow(snapshot, metadataQueryFacade, queryFacade, executionFacade, workflowApplySynchronizationHandler, sessionId);
+ workflowApplySynchronizationHandler.synchronize(snapshot, metadataQueryFacade, queryFacade, executionFacade, sessionId);
}
return completeApply(workflowSessionContext, snapshot, executionMode, applyOutcome);
} catch (final WorkflowSynchronizationException ex) {
@@ -266,12 +266,6 @@ public final class WorkflowExecutionService {
}
}
- private void synchronizeAppliedWorkflow(final WorkflowContextSnapshot snapshot, final MCPMetadataQueryFacade metadataQueryFacade, final MCPFeatureQueryFacade queryFacade,
- final MCPFeatureExecutionFacade executionFacade, final MCPWorkflowApplySynchronizationHandler workflowApplySynchronizationHandler,
- final String sessionId) {
- workflowApplySynchronizationHandler.synchronize(snapshot, metadataQueryFacade, queryFacade, executionFacade, sessionId);
- }
-
private Map<String, Object> completeApply(final WorkflowSessionContext workflowSessionContext, final WorkflowContextSnapshot snapshot, final String executionMode,
final WorkflowApplyOutcome applyOutcome) {
persistSnapshot(workflowSessionContext, snapshot, WorkflowLifecycle.STEP_EXECUTED, WorkflowLifecycle.STATUS_EXECUTED);
diff --git a/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionServiceTest.java b/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionServiceTest.java
index d9139a693d4..87747dba237 100644
--- a/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionServiceTest.java
+++ b/mcp/core/src/test/java/org/apache/shardingsphere/mcp/core/workflow/WorkflowExecutionServiceTest.java
@@ -59,6 +59,10 @@ import static org.mockito.Mockito.when;
class WorkflowExecutionServiceTest {
+ private static final MCPWorkflowApplySynchronizationHandler NO_OP_APPLY_SYNCHRONIZATION_HANDLER =
+ (snapshot, metadataQueryFacade, queryFacade, executionFacade, sessionId) -> {
+ };
+
@Test
void assertApplyMasksManualArtifactPackage() {
WorkflowContextSnapshot snapshot = createSnapshot();
@@ -115,7 +119,7 @@ class WorkflowExecutionServiceTest {
workflowSessionContext.save(createSnapshot());
WorkflowExecutionService executionService = new WorkflowExecutionService();
Map<String, Object> actualResponse = executionService.apply(workflowSessionContext, mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class),
- mock(MCPFeatureExecutionFacade.class), MCPWorkflowApplySynchronizationHandler.NO_OP, MCPWorkflowApplyArtifactValidator.NO_OP,
+ mock(MCPFeatureExecutionFacade.class), NO_OP_APPLY_SYNCHRONIZATION_HANDLER, MCPWorkflowApplyArtifactValidator.NO_OP,
"session-2", workflowSessionContext.getRequired("plan-1"), List.of(), "review-then-execute");
assertThat(actualResponse.get("status"), is("failed"));
assertThat(actualResponse.get("response_mode"), is("terminal"));
@@ -140,7 +144,7 @@ class WorkflowExecutionServiceTest {
void assertApplyRejectsMissingExecutionMode() {
WorkflowExecutionService executionService = new WorkflowExecutionService();
MCPInvalidRequestException actual = assertThrows(MCPInvalidRequestException.class, () -> executionService.apply(new InMemoryWorkflowSessionContext(),
- mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class), mock(MCPFeatureExecutionFacade.class), MCPWorkflowApplySynchronizationHandler.NO_OP,
+ mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class), mock(MCPFeatureExecutionFacade.class), NO_OP_APPLY_SYNCHRONIZATION_HANDLER,
MCPWorkflowApplyArtifactValidator.NO_OP, "session-1", createSnapshot(), List.of(), ""));
assertThat(actual.getMessage(), is("database_gateway_apply_workflow execution_mode is required."));
}
@@ -149,7 +153,7 @@ class WorkflowExecutionServiceTest {
void assertApplyRejectsUnsupportedExecutionMode() {
WorkflowExecutionService executionService = new WorkflowExecutionService();
MCPInvalidRequestException actual = assertThrows(MCPInvalidRequestException.class, () -> executionService.apply(new InMemoryWorkflowSessionContext(),
- mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class), mock(MCPFeatureExecutionFacade.class), MCPWorkflowApplySynchronizationHandler.NO_OP,
+ mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class), mock(MCPFeatureExecutionFacade.class), NO_OP_APPLY_SYNCHRONIZATION_HANDLER,
MCPWorkflowApplyArtifactValidator.NO_OP, "session-1", createSnapshot(), List.of(), "auto-execute"));
assertThat(actual.getMessage(), is("database_gateway_apply_workflow execution_mode must be one of [preview, review-then-execute, manual-only]."));
}
@@ -158,7 +162,7 @@ class WorkflowExecutionServiceTest {
void assertApplyRejectsUnsupportedApprovedStep() {
WorkflowExecutionService executionService = new WorkflowExecutionService();
MCPInvalidRequestException actual = assertThrows(MCPInvalidRequestException.class, () -> executionService.apply(new InMemoryWorkflowSessionContext(),
- mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class), mock(MCPFeatureExecutionFacade.class), MCPWorkflowApplySynchronizationHandler.NO_OP,
+ mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class), mock(MCPFeatureExecutionFacade.class), NO_OP_APPLY_SYNCHRONIZATION_HANDLER,
MCPWorkflowApplyArtifactValidator.NO_OP, "session-1", createSnapshot(), List.of("review"), "review-then-execute"));
assertThat(actual.getMessage(), is("approved_steps must contain only [ddl, index_ddl, rule_distsql]."));
}
@@ -170,7 +174,7 @@ class WorkflowExecutionServiceTest {
snapshot.getDdlArtifacts().add(new DDLArtifact("add-column", "ALTER TABLE orders ADD COLUMN order_id_cipher VARCHAR(32)", 10));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
when(executionFacade.execute(any())).thenReturn(mock(SQLExecutionResult.class));
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of(), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of(), "review-then-execute");
assertThat(actualResponse.get("status"), is("failed"));
assertThat(((Map<?, ?>) ((List<?>) actualResponse.get("issues")).getFirst()).get("code"), is(WorkflowIssueCode.WORKFLOW_STATUS_INVALID));
verify(executionFacade, never()).execute(any());
@@ -181,7 +185,7 @@ class WorkflowExecutionServiceTest {
WorkflowContextSnapshot snapshot = createSnapshot();
snapshot.getDdlArtifacts().add(new DDLArtifact("add-column", "ALTER TABLE orders ADD COLUMN order_id_cipher VARCHAR(32)", 10));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("ddl"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("ddl"), "review-then-execute");
assertThat(actualResponse.get("status"), is("failed"));
assertThat(((Map<?, ?>) ((List<?>) actualResponse.get("issues")).getFirst()).get("code"), is(WorkflowIssueCode.WORKFLOW_STATUS_INVALID));
verify(executionFacade, never()).execute(any());
@@ -193,7 +197,7 @@ class WorkflowExecutionServiceTest {
snapshot.setStatus("previewed");
snapshot.getRuleArtifacts().add(new RuleArtifact("create", "CREATE MASK RULE orders"));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("ddl"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("ddl"), "review-then-execute");
assertThat(actualResponse.get("status"), is("failed"));
assertThat(((Map<?, ?>) ((List<?>) actualResponse.get("issues")).getFirst()).get("code"), is(WorkflowIssueCode.WORKFLOW_STATUS_INVALID));
verify(executionFacade, never()).execute(any());
@@ -284,7 +288,7 @@ class WorkflowExecutionServiceTest {
workflowSessionContext.save(snapshot);
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
Map<String, Object> actualResponse = new WorkflowExecutionService().apply(workflowSessionContext, mock(MCPMetadataQueryFacade.class), mock(MCPFeatureQueryFacade.class),
- executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP,
+ executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER,
(workflowSnapshot, artifacts) -> List.of(new WorkflowIssue(WorkflowIssueCode.SQL_EXECUTABILITY_FAILED, "error", "review",
"Generated workflow artifact is invalid.", "Regenerate the workflow artifact before approval.", true,
Map.of("sql", artifacts.iterator().next().displaySql())).toMap()),
@@ -325,7 +329,7 @@ class WorkflowExecutionServiceTest {
WorkflowSessionContext workflowSessionContext = createWorkflowSessionContext(snapshot);
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
when(executionFacade.execute(any())).thenReturn(mock(SQLExecutionResult.class));
- Map<String, Object> actualResponse = apply(workflowSessionContext, snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP,
+ Map<String, Object> actualResponse = apply(workflowSessionContext, snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER,
List.of("ddl", "index_ddl", "rule_distsql"), "review-then-execute");
assertThat(actualResponse.get("status"), is("completed"));
assertThat(actualResponse.get("response_mode"), is("executed"));
@@ -345,7 +349,7 @@ class WorkflowExecutionServiceTest {
snapshot.getRuleArtifacts().add(new RuleArtifact("create", "CREATE ENCRYPT RULE orders (PROPERTIES('aes-key-value'='123456'))"));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
when(executionFacade.execute(any())).thenReturn(mock(SQLExecutionResult.class));
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("rule_distsql"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("rule_distsql"), "review-then-execute");
assertThat(actualResponse.get("status"), is("completed"));
assertThat(((List<?>) actualResponse.get("executed_distsql")).getFirst(), is("CREATE ENCRYPT RULE orders (PROPERTIES('aes-key-value'='******'))"));
assertThat(((Map<?, ?>) ((List<?>) actualResponse.get("step_results")).getFirst()).get("sql"),
@@ -358,7 +362,7 @@ class WorkflowExecutionServiceTest {
void assertApplyRequiresManualExecutionForSecretReference() {
WorkflowContextSnapshot snapshot = createSecretReferenceSnapshot();
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("rule_distsql"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("rule_distsql"), "review-then-execute");
assertThat(actualResponse.get("status"), is("failed"));
assertThat(actualResponse.get("response_mode"), is("recovery"));
assertThat(actualResponse.get("category"), is(MCPDiagnosticCategory.SECRET_REFERENCE_MANUAL_EXECUTION_REQUIRED));
@@ -418,7 +422,7 @@ class WorkflowExecutionServiceTest {
snapshot.getDdlArtifacts().add(new DDLArtifact("add-column", "ALTER TABLE orders ADD COLUMN order_id_cipher VARCHAR(32)", 10));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
when(executionFacade.execute(any())).thenThrow(new IllegalStateException("ddl failed"));
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("ddl"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("ddl"), "review-then-execute");
Map<?, ?> actualIssue = (Map<?, ?>) ((List<?>) actualResponse.get("issues")).getFirst();
Map<?, ?> actualStep = (Map<?, ?>) ((List<?>) actualResponse.get("step_results")).getFirst();
assertThat(actualResponse.get("status"), is("failed"));
@@ -433,7 +437,7 @@ class WorkflowExecutionServiceTest {
snapshot.getIndexPlans().add(new IndexPlan("idx_orders_order_id_cipher", "order_id_cipher", "assist lookup", "CREATE INDEX idx_orders_order_id_cipher ON orders(order_id_cipher)"));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
when(executionFacade.execute(any())).thenThrow(new IllegalStateException("index failed"));
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("index_ddl"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("index_ddl"), "review-then-execute");
Map<?, ?> actualIssue = (Map<?, ?>) ((List<?>) actualResponse.get("issues")).getFirst();
Map<?, ?> actualStep = (Map<?, ?>) ((List<?>) actualResponse.get("step_results")).getFirst();
assertThat(actualResponse.get("status"), is("failed"));
@@ -450,7 +454,7 @@ class WorkflowExecutionServiceTest {
snapshot.getRuleArtifacts().add(new RuleArtifact("create", "CREATE ENCRYPT RULE orders (PROPERTIES('aes-key-value'='123456'))"));
MCPFeatureExecutionFacade executionFacade = mock(MCPFeatureExecutionFacade.class);
when(executionFacade.execute(any())).thenThrow(new IllegalStateException("Failed to execute CREATE ENCRYPT RULE orders (PROPERTIES('aes-key-value'='123456'))"));
- Map<String, Object> actualResponse = apply(snapshot, executionFacade, MCPWorkflowApplySynchronizationHandler.NO_OP, List.of("rule_distsql"), "review-then-execute");
+ Map<String, Object> actualResponse = apply(snapshot, executionFacade, NO_OP_APPLY_SYNCHRONIZATION_HANDLER, List.of("rule_distsql"), "review-then-execute");
Map<?, ?> actualIssue = (Map<?, ?>) ((List<?>) actualResponse.get("issues")).getFirst();
assertThat(actualResponse.get("status"), is("failed"));
assertThat(actualIssue.get("code"), is(WorkflowIssueCode.RULE_EXECUTION_FAILED));
@@ -459,7 +463,7 @@ class WorkflowExecutionServiceTest {
}
private Map<String, Object> apply(final WorkflowContextSnapshot snapshot, final List<String> approvedSteps, final String executionMode) {
- return apply(createWorkflowSessionContext(snapshot), snapshot, mock(MCPFeatureExecutionFacade.class), MCPWorkflowApplySynchronizationHandler.NO_OP, approvedSteps, executionMode);
+ return apply(createWorkflowSessionContext(snapshot), snapshot, mock(MCPFeatureExecutionFacade.class), NO_OP_APPLY_SYNCHRONIZATION_HANDLER, approvedSteps, executionMode);
}
private Map<String, Object> apply(final WorkflowContextSnapshot snapshot, final MCPFeatureExecutionFacade executionFacade,
diff --git a/mcp/features/mask/src/main/java/org/apache/shardingsphere/mcp/feature/mask/tool/handler/MaskWorkflowToolResponseBuilder.java b/mcp/features/mask/src/main/java/org/apache/shardingsphere/mcp/feature/mask/tool/handler/MaskWorkflowToolResponseBuilder.java
index f304a378afd..87043cd588a 100644
--- a/mcp/features/mask/src/main/java/org/apache/shardingsphere/mcp/feature/mask/tool/handler/MaskWorkflowToolResponseBuilder.java
+++ b/mcp/features/mask/src/main/java/org/apache/shardingsphere/mcp/feature/mask/tool/handler/MaskWorkflowToolResponseBuilder.java
@@ -34,7 +34,7 @@ final class MaskWorkflowToolResponseBuilder {
private final MaskAlgorithmPropertyTemplateService propertyTemplateService;
Map<String, Object> buildPlanResponse(final WorkflowContextSnapshot snapshot) {
- WorkflowPropertySource propertySource = getPropertySource(snapshot);
+ WorkflowPropertySource propertySource = snapshot.getRequest();
Map<String, Object> result = WorkflowPlanPayloadBuilder.buildRuleDistSQLOnly(snapshot, propertySource);
result.put("masked_property_preview", createMaskedPropertyPreview(snapshot, propertySource));
return result;
@@ -50,7 +50,4 @@ final class MaskWorkflowToolResponseBuilder {
: snapshot.getPropertyRequirements().stream().filter(each -> "primary".equals(each.getAlgorithmRole())).toList();
}
- private WorkflowPropertySource getPropertySource(final WorkflowContextSnapshot snapshot) {
- return snapshot.getRequest();
- }
}
diff --git a/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/service/ShardingAlgorithmRecommendationService.java b/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/service/ShardingAlgorithmRecommendationService.java
index 6f59f36c6df..0c5828c1a1e 100644
--- a/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/service/ShardingAlgorithmRecommendationService.java
+++ b/mcp/features/sharding/src/main/java/org/apache/shardingsphere/mcp/feature/sharding/tool/service/ShardingAlgorithmRecommendationService.java
@@ -54,7 +54,7 @@ public final class ShardingAlgorithmRecommendationService {
recommendAlgorithm(request.getAlgorithmType(), actualAlgorithmRows, issues).forEach(result::add);
}
if (includeKeyGenerator) {
- recommendKeyGenerator(resolveKeyGeneratorType(request), actualKeyGeneratorRows, issues).forEach(result::add);
+ recommendKeyGenerator(request.getKeyGeneratorType(), actualKeyGeneratorRows, issues).forEach(result::add);
}
return result;
}
@@ -93,10 +93,6 @@ public final class ShardingAlgorithmRecommendationService {
return List.of(createCandidate("key_generator", recommendedType, 90, "Recommended from current key-generator availability."));
}
- private String resolveKeyGeneratorType(final ShardingWorkflowRequest request) {
- return request.getKeyGeneratorType();
- }
-
private AlgorithmCandidate createCandidate(final String role, final String algorithmType, final int score, final String reason) {
return AlgorithmCandidate.builder().algorithmRole(role).algorithmType(algorithmType).recommendationScore(score).recommendationReason(reason).riskNotes("").build();
}
diff --git a/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/spi/MCPWorkflowApplySynchronizationHandler.java b/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/spi/MCPWorkflowApplySynchronizationHandler.java
index b153198c0e8..98521f8f0b0 100644
--- a/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/spi/MCPWorkflowApplySynchronizationHandler.java
+++ b/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/spi/MCPWorkflowApplySynchronizationHandler.java
@@ -28,9 +28,6 @@ import org.apache.shardingsphere.mcp.support.workflow.model.WorkflowContextSnaps
@FunctionalInterface
public interface MCPWorkflowApplySynchronizationHandler {
- MCPWorkflowApplySynchronizationHandler NO_OP = (snapshot, metadataQueryFacade, queryFacade, executionFacade, sessionId) -> {
- };
-
/**
* Synchronize workflow state after apply execution.
*