fix: preserve alert receiver fields during partial updates (#6711) by juicewcode · Pull Request #7038 · apache/shenyu

juicewcode · 2026-09-07T15:32:55Z

Summary

Replace updateByPrimaryKey with updateByPrimaryKeySelective for alert receiver updates.
Add the selective mapper method and dynamic MyBatis SQL.
Convert enable and matchAll from primitive boolean to nullable Boolean.
Omitted nullable fields are no longer written back to the database.
Preserve the default value true when creating a new alert receiver.
Add null-safe checks in alert dispatch logic.

Test

Updated AlertReceiverServiceTest to verify selective updates and prevent calls to the full-column update method.
Updated AlertTransferTest for nullable Boolean mapping.
Added coverage for null enable and matchAll values.
Existing alert dispatch test paths continue to set explicit Boolean values.
Performed static checks for remaining primitive getter usage, XML validity, and whitespace errors.

Make sure that:

You have read the contribution guidelines.
You submit test cases (unit or integration tests) that back your changes.
Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

) Use selective updates for alert receivers and convert enable and matchAll to nullable Boolean values. Preserve database values when these fields are omitted, keep true as the default for new receivers, and add null-safe alert dispatch handling.

Copilot

🟡 Changes recommended

The new selective MyBatis update binds accessToken as jdbcType=TINYINT despite being a String/VARCHAR, which can cause incorrect updates or runtime SQL type conversion errors.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR addresses #6711 by preventing partial alert receiver updates from overwriting omitted columns with NULL, switching the admin update path to a selective MyBatis update and making enable/matchAll nullable so “not provided” can be preserved.

Changes:

Use updateByPrimaryKeySelective for alert receiver updates and add the corresponding mapper method + dynamic SQL.
Convert enable and matchAll from primitive boolean to nullable Boolean, and update mapping/tests accordingly.
Make alert dispatch filtering null-safe (Boolean.TRUE.equals(...)) and preserve true defaults on create in the service layer.

File summaries

File	Description
shenyu-alert/src/main/java/org/apache/shenyu/alert/model/AlertReceiverDTO.java	Make `enable`/`matchAll` nullable to support selective updates without implicit false/default writes.
shenyu-admin/src/test/java/org/apache/shenyu/admin/transfer/AlertTransferTest.java	Update mapping assertions for nullable Booleans and add null-mapping coverage.
shenyu-admin/src/test/java/org/apache/shenyu/admin/service/AlertReceiverServiceTest.java	Verify selective update usage and ensure full-column update isn’t called.
shenyu-admin/src/main/resources/mappers/alert-receiver-sqlmap.xml	Add `updateByPrimaryKeySelective` dynamic SQL for partial updates.
shenyu-admin/src/main/java/org/apache/shenyu/admin/transfer/AlertTransfer.java	Update DTO/DO mapping for nullable `enable`/`matchAll`.
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/AlertReceiverServiceImpl.java	Default `enable`/`matchAll` to `true` on create and switch update to selective update.
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/AlertDispatchServiceImpl.java	Make dispatch filtering null-safe for nullable Booleans.
shenyu-admin/src/main/java/org/apache/shenyu/admin/model/entity/AlertReceiverDO.java	Make persisted `enable`/`matchAll` nullable to preserve omitted fields.
shenyu-admin/src/main/java/org/apache/shenyu/admin/mapper/AlertReceiverMapper.java	Add `updateByPrimaryKeySelective` to the mapper interface.

Review details

Files reviewed: 9/9 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+            <if test="email != null">email = #{email,jdbcType=VARCHAR},</if>
+            <if test="hookUrl != null">hook_url = #{hookUrl,jdbcType=VARCHAR},</if>
+            <if test="wechatId != null">wechat_id = #{wechatId,jdbcType=VARCHAR},</if>
+            <if test="accessToken != null">access_token = #{accessToken,jdbcType=TINYINT},</if>


Aias00

Approved as PMC (Aias00). Coherent, well-scoped fix with regression tests; green CI, mergeable. Reviewed the diff.

Aias00

The selective update maps accessToken with jdbcType=TINYINT even though access_token and the Java field are strings. Updating a DingTalk token can fail conversion or persist the wrong value. Please use VARCHAR and add a selective-update regression test for accessToken.

Map accessToken as VARCHAR in the insert, full update, and selective update statements to match the Java field and database column. Add a mapper regression test that selectively updates a DingTalk access token, reads it back, and checks that an unrelated field is preserved.

Aias00

Reviewed current head 7f102f6243dd.

The prior CHANGES_REQUESTED item is fixed in current head source. The PR now preserves omitted receiver fields through a selective update, keeps create defaults for nullable booleans, updates dispatch filtering to be null-safe, and includes mapper/service/transfer coverage for the changed behavior.

Validation: exact-head targeted Maven reactor tests passed on Java 17.

./mvnw -pl shenyu-admin -am test -Dtest=AlertReceiverMapperTest,AlertReceiverServiceTest,AlertTransferTest -Dsurefire.failIfNoSpecifiedTests=false -Drat.skip=true -Djacoco.skip=true -DskipRemoteResources=true -Dmaven.javadoc.skip=true

GitHub still reports failing checks: e2e, e2e-case (e2e-logging-rabbitmq-compose, shenyu-e2e-case-logging-rabbitmq, :shenyu-examples-http). This review does not claim those checks have passed.

Aias00 · 2026-10-23T06:25:46Z

Ping on state: this PR is approved on the code, but the required checks are red and it cannot be merged yet.

The failing jobs are e2e / e2e-case / list-contract / native-sql, which are currently failing across the repository and are tracked in #7970. The build job in this run does not fail on compilation: it stops at the gate step that consumes the e2e result ([[ "failure" == "success" ]] || exit -1), so there is nothing in this PR to fix for that.

No code change is expected from you. Once the shared e2e problem is resolved, please rerun the failed jobs (or push a trivial rebase) so the required checks go green, and this can be merged.

Aias00 · 2026-10-24T07:49:58Z

@juicewcode This PR is approved, but 7 checks are red: e2e, e2e-case (e2e-logging-rocketmq-compose, shenyu-e2e-case-logging-rocketmq, :shenyu-examples-http), e2e-case (e2e-apache-dubbo-sync-compose, shenyu-e2e-case-apache-dubbo, :shenyu-examples-apache-du..., e2e-case (e2e-springcloud-sync-compose, shenyu-e2e-case-spring-cloud, :shenyu-examples-eureka,:sh... (+3 more).

I inspected these lanes today and they fail for infrastructure reasons rather than on the diff — the integrated-test jobs die while starting their containers (Elasticsearch /_cat/health returning 000 until the job is cancelled), and the http lane also hits the known CachePluginTest.testMemoryModle / testRedisModle flake. The same jobs are red on master and on unrelated PRs right now.

Please rebase onto master and push (or ask for a re-run) so the checks execute again. If a case is still failing on a fresh run, paste the test name and I'll look properly.

Aias00 · 2026-10-25T01:04:00Z

@juicewcode — a correction to the note I left here yesterday. The failures on this branch all predate the fix, so nothing in your change is implicated.

master went green at a8080ae: the SubscribeTest fixture isolation (#8092), the MQTT session scoping (#8093) and the pr_build dependency fix (#8075) all landed, and the latest ci.yml, integrated-test.yml and CodeQL runs on master are all success.

That means the red checks recorded here are stale. A push — an empty commit or a rebase, whichever is easier — re-runs them against the current baseline. I would not spend time reading the old logs.

For the e2e jobs specifically: /retest on a run whose shenyu-images artifact has expired now falls back to a full rerun and rebuilds it (#8094 landed), so that path works again too.

…iver-partial-update-

juicewcode added 2 commits September 7, 2026 23:31

fix ci

0c7a8cb

dengliming requested a lite review from Copilot September 17, 2026 02:34

Copilot started reviewing on behalf of dengliming September 17, 2026 02:35

View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Aias00 approved these changes Sep 19, 2026

View reviewed changes

juicewcode added 2 commits September 20, 2026 20:14

Merge branch 'master' into fix/6711-alert-receiver-partial-update-

3b8a28c

Merge branch 'master' into fix/6711-alert-receiver-partial-update-

273fcaa

Aias00 requested changes Oct 4, 2026

View reviewed changes

Aias00 approved these changes Oct 8, 2026

View reviewed changes

Aias00 mentioned this pull request Oct 17, 2026

[CI] e2e matrix (e2e / e2e-case *-sync-compose / e2e-storage) is flaky and blocks unrelated approved PRs #7768

Closed

Merge remote-tracking branch 'origin/master' into fix/6711-alert-rece…

e457a69

…iver-partial-update-

Aias00 merged commit baf548f into apache:master Oct 26, 2026
55 checks passed

Navigation Menu

Repository navigation

fix: preserve alert receiver fields during partial updates (#6711) - #7038

fix: preserve alert receiver fields during partial updates (#6711)#7038
Aias00 merged 6 commits into
apache:masterfrom
juicewcode:fix/6711-alert-receiver-partial-update-

juicewcode commented Sep 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Aias00 left a comment

Uh oh!

Aias00 left a comment

Uh oh!

Aias00 left a comment

Uh oh!

Aias00 commented Oct 23, 2026

Uh oh!

Aias00 commented Oct 24, 2026

Uh oh!

Aias00 commented Oct 25, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

juicewcode commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Uh oh!

Aias00 commented Oct 23, 2026

Uh oh!

Aias00 commented Oct 24, 2026

Uh oh!

Aias00 commented Oct 25, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

juicewcode commented Sep 7, 2026 •

edited

Loading