You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by "juicewcode (via GitHub)" <gi...@apache.org> on 2026/09/07 15:32:58 UTC

[PR] fix: preserve alert receiver fields during partial updates (#6711) [shenyu]

juicewcode opened a new pull request, #7038:
URL: https://github.com/apache/shenyu/pull/7038

   ## 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.
   
   <!-- Describe your PR here; e.g. Fixes #issueNo -->
   
   <!--
   Thank you for proposing a pull request. This template will guide you through the essential steps necessary for a pull request.
   -->
   Make sure that:
   
   - [x] You have read the [contribution guidelines](https://shenyu.apache.org/community/contributor-guide).
   - [x] You submit test cases (unit or integration tests) that back your changes.
   - [x] Your local test passed `./mvnw clean install -Dmaven.javadoc.skip=true`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: preserve alert receiver fields during partial updates (#6711) [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #7038:
URL: https://github.com/apache/shenyu/pull/7038#issuecomment-6391728402

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: preserve alert receiver fields during partial updates (#6711) [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #7038:
URL: https://github.com/apache/shenyu/pull/7038#issuecomment-6358890319

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: preserve alert receiver fields during partial updates (#6711) [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #7038:
URL: https://github.com/apache/shenyu/pull/7038#issuecomment-6381647103

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: preserve alert receiver fields during partial updates (#6711) [shenyu]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7038:
URL: https://github.com/apache/shenyu/pull/7038#discussion_r4032584383


##########
shenyu-admin/src/main/resources/mappers/alert-receiver-sqlmap.xml:
##########
@@ -125,6 +125,39 @@
         FROM alert_receiver
         WHERE id = #{id, jdbcType=VARCHAR}
     </select>
+    <update id="updateByPrimaryKeySelective" parameterType="org.apache.shenyu.admin.model.entity.AlertReceiverDO">
+        update alert_receiver
+        <set>
+            <if test="name != null">name = #{name,jdbcType=VARCHAR},</if>
+            <if test="type != null">type = #{type,jdbcType=TINYINT},</if>
+            <if test="phone != null">phone = #{phone,jdbcType=VARCHAR},</if>
+            <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>

Review Comment:
   `accessToken` is a `String` (and the resultMap already maps `access_token` as `jdbcType=VARCHAR`), but the new selective update binds it as `jdbcType=TINYINT`. This type mismatch can cause JDBC conversion errors or incorrect values when updating DingTalk robot access tokens. Align the parameter JDBC type with the column mapping.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix: preserve alert receiver fields during partial updates (#6711) [shenyu]

Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 merged PR #7038:
URL: https://github.com/apache/shenyu/pull/7038


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shenyu.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org