Skip to content

[Improvement-18249][DAO] Route TaskDefinitionMapper access through TaskDefinitionDao#18254

Merged
SbloodyS merged 2 commits into
apache:devfrom
ruanwenjun:dao/task-definition
May 20, 2026
Merged

[Improvement-18249][DAO] Route TaskDefinitionMapper access through TaskDefinitionDao#18254
SbloodyS merged 2 commits into
apache:devfrom
ruanwenjun:dao/task-definition

Conversation

@ruanwenjun
Copy link
Copy Markdown
Member

Was this PR generated or assisted by AI?

YES, ops 4.7

Purpose of the pull request

Encapsulate TaskDefinitionMapper behind TaskDefinitionDao so the api layer depends only on the repository abstraction.

Add methods to TaskDefinitionDao: queryByName, queryByWorkerGroup, countByEnvironmentCode, queryByEnvironmentCodeAndWorkerGroup. The latter three replace inlined LambdaQueryWrapper builders that previously leaked mapper internals into the api layer.

Switch three call sites in TaskDefinitionServiceImpl from int updateById to boolean: the (update & insert) != 1 check becomes !updateSuccess || insert != 1, the <=0 check becomes !switchSuccess, and the XOR check becomes updateSuccess != (updateLog == 1).

TaskDefinitionServiceImplTest keeps its TaskDefinitionMapper mock for the @Injectmocks ProcessServiceImpl since the service module migration is out of scope.

Tracking issue: #18249

Brief change log

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 improvement make more easy to user or prompt friendly refactor labels May 14, 2026
@ruanwenjun ruanwenjun added this to the 3.4.2 milestone May 14, 2026
@ruanwenjun ruanwenjun force-pushed the dao/task-definition branch 7 times, most recently from e41d523 to d873622 Compare May 18, 2026 14:41
SbloodyS
SbloodyS previously approved these changes May 19, 2026
Copy link
Copy Markdown
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM

…skDefinitionDao

Encapsulate TaskDefinitionMapper behind TaskDefinitionDao so the api layer
depends only on the repository abstraction.

Add methods to TaskDefinitionDao: queryByName, queryByWorkerGroup,
countByEnvironmentCode, queryByEnvironmentCodeAndWorkerGroup. The latter
three replace inlined LambdaQueryWrapper builders that previously leaked
mapper internals into the api layer.

Switch three call sites in TaskDefinitionServiceImpl from int updateById
to boolean: the (update & insert) != 1 check becomes
!updateSuccess || insert != 1, the <=0 check becomes !switchSuccess,
and the XOR check becomes updateSuccess != (updateLog == 1).

TaskDefinitionServiceImplTest keeps its TaskDefinitionMapper mock for
the @Injectmocks ProcessServiceImpl since the service module migration
is out of scope.

Tracking issue: apache#18249
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

LGTM

@SbloodyS SbloodyS merged commit cd8abba into apache:dev May 20, 2026
121 of 123 checks passed
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 refactor test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants