Skip to content

Conversation

@ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented May 6, 2025

Purpose of the pull request

close #17158
Avoid write a lot of method e.g. queryByCode, queryByCodeAndVersion, queryByCodeAndVersionAndStatus

Brief change log

  • Remove some method in WorkflowInstanceMapper

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

@ruanwenjun ruanwenjun added the improvement make more easy to user or prompt friendly label May 6, 2025
@ruanwenjun ruanwenjun added the 3.1.x for 3.1.x version label May 6, 2025
@ruanwenjun ruanwenjun removed the 3.1.x for 3.1.x version label May 6, 2025
@ruanwenjun ruanwenjun added this to the 3.3.1 milestone May 6, 2025
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_17158 branch 2 times, most recently from c134d05 to 326997b Compare May 7, 2025 14:46
@nielifeng nielifeng requested a review from Copilot May 9, 2025 09:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors query methods by removing multiple dedicated methods (e.g. queryByCode variants) and consolidating them into a generic query-by-wrapper approach in the BaseDao. Key changes include removal of redundant mapper methods, conversion of query methods in WorkflowInstanceDaoImpl and BaseDao to use lambda-based wrappers, and corresponding adjustments in tests and service implementations for workflow instance queries.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

File Description
dolphinscheduler-dao/* Removed XML and mapper query methods and simplified DAO implementations
dolphinscheduler-api/* Updated service and test layers to use the new wrapper-based query methods in place of deprecated ones
BaseDao.java, IDao.java Adapted interface signatures to use Java Function-based query conditions
Comments suppressed due to low confidence (1)

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/WorkerGroupServiceTest.java:255

  • Instead of returning null for query methods in tests, return an empty list to align with expected non-null collection responses.
when(workflowInstanceDao.queryByCondition(Mockito.any())).thenReturn(null);

ruanwenjun and others added 2 commits May 13, 2025 10:32
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
15.7% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

@SbloodyS SbloodyS removed this from the 3.3.1 milestone Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend improvement make more easy to user or prompt friendly test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement][Dao] Further encapsulation of BaseDao avoids writing many conditional queries

2 participants