You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "Zhengcy05 (via GitHub)" <gi...@apache.org> on 2026/06/09 08:38:31 UTC
[PR] feat:automatic update after BusinessActionContext modify [incubator-seata]
Zhengcy05 opened a new pull request, #8140:
URL: https://github.com/apache/incubator-seata/pull/8140
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<!-- Please make sure you have read and understood the contributing guidelines -->
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines.
- [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes).
### Ⅰ. Describe what this PR did
This PR adds a tracked wrapper for `BusinessActionContext.actionContext` so that Map-level mutations in the Try phase, such as `put`, `putAll`, `remove`, `clear`, `replace`, `compute*`, and `merge`, automatically mark `updated` as `true`. This reduces the need for business code to call `setUpdated(true)` manually.
It also adjusts the initialization timing in `ActionInterceptorHandler` to ensure framework-side setup does not incorrectly trigger `updated`, while business-side changes are still captured and reported correctly in phase two.
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
fixes #7954
### Ⅲ. Why don't you add test cases (unit test/integration test)?
Test cases have been added to cover the new behavior.
### Ⅳ. Describe how to verify it
Run:
```bash
mvn -P '!java17+' -Dremoteresources.skip=true -Djacoco.skip=true -Dmaven.simplify.skip=true -DfailIfNoTests=false -Dmaven.repo.local=/private/tmp/codex-m2 -f pom.xml -pl integration-tx-api -am -Dtest=ActionInterceptorHandlerTest,BusinessActionContextTest test
```
Verification:
- BusinessActionContextTest covers actionContext mutations and automatic updated marking.
- ActionInterceptorHandlerTest covers business-side changes being reported correctly in the Try phase.
### Ⅴ. Special notes for reviews
This change only covers Map-level tracking for actionContext. It does not add deep proxying or dirty tracking for nested object fields.
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat:automatic update after BusinessActionContext modify [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8140:
URL: https://github.com/apache/incubator-seata/pull/8140#discussion_r3379243960
##########
integration-tx-api/src/main/java/org/apache/seata/rm/tcc/api/BusinessActionContext.java:
##########
@@ -253,4 +288,177 @@ public String toString() {
.append("]");
return sb.toString();
}
+
+ /**
+ * The tracked action context map.
+ */
+ private static final class TrackedActionContextMap extends AbstractMap<String, Object> implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ private final BusinessActionContext owner;
+
+ private final Map<String, Object> delegate;
+
+ private TrackedActionContextMap(BusinessActionContext owner) {
+ this.owner = owner;
+ this.delegate = new HashMap<>(8);
+ }
+
+ private TrackedActionContextMap(BusinessActionContext owner, Map<String, Object> source) {
+ this.owner = owner;
+ this.delegate = new HashMap<>(source);
+ }
+
+ @Override
+ public Object put(String key, Object value) {
+ boolean hadKey = delegate.containsKey(key);
+ Object previousValue = delegate.put(key, value);
+ if (!hadKey || !Objects.equals(previousValue, value)) {
+ owner.markUpdatedOnActionContextMutation();
+ }
+ return previousValue;
+ }
+
+ @Override
+ public void putAll(Map<? extends String, ? extends Object> m) {
+ if (m == null || m.isEmpty()) {
+ return;
+ }
+ for (Map.Entry<? extends String, ? extends Object> entry : m.entrySet()) {
+ put(entry.getKey(), entry.getValue());
+ }
+ }
Review Comment:
`putAll(null)` currently returns silently. This diverges from the `Map` contract / standard implementations (e.g., `HashMap`), which throw `NullPointerException` for a null argument, and may hide programming errors.
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: automatic update after BusinessActionContext modify [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #8140:
URL: https://github.com/apache/incubator-seata/pull/8140#issuecomment-4776432835
If you're using the DingTalk app, please send your DingTalk account to my email at [jianbin@apache.org](mailto:jianbin@apache.org), and I will invite you to join the Seata developer group
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: automatic update after BusinessActionContext modify [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #8140:
URL: https://github.com/apache/incubator-seata/pull/8140
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat:automatic update after BusinessActionContext modify [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8140:
URL: https://github.com/apache/incubator-seata/pull/8140#issuecomment-4692034364
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `67.46988%` with `27 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 72.84%. Comparing base ([`2f3de1c`](https://app.codecov.io/gh/apache/incubator-seata/commit/2f3de1cad03a81bd1145811d69ba7d097c3320dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bea4abe`](https://app.codecov.io/gh/apache/incubator-seata/commit/bea4abe64bc95a67fae1bcc16b0bb49acd2fb3ab?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...apache/seata/rm/tcc/api/BusinessActionContext.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Ftcc%2Fapi%2FBusinessActionContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9ybS90Y2MvYXBpL0J1c2luZXNzQWN0aW9uQ29udGV4dC5qYXZh) | 65.38% | [21 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #8140 +/- ##
============================================
- Coverage 72.92% 72.84% -0.09%
+ Complexity 883 880 -3
============================================
Files 1327 1327
Lines 50769 50851 +82
Branches 6058 6071 +13
============================================
+ Hits 37025 37042 +17
- Misses 10735 10800 +65
Partials 3009 3009
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...n/tx/api/interceptor/ActionInterceptorHandler.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fintegration%2Ftx%2Fapi%2Finterceptor%2FActionInterceptorHandler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9pbnRlZ3JhdGlvbi90eC9hcGkvaW50ZXJjZXB0b3IvQWN0aW9uSW50ZXJjZXB0b3JIYW5kbGVyLmphdmE=) | `75.23% <100.00%> (+0.98%)` | :arrow_up: |
| [...apache/seata/rm/tcc/api/BusinessActionContext.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Ftcc%2Fapi%2FBusinessActionContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9ybS90Y2MvYXBpL0J1c2luZXNzQWN0aW9uQ29udGV4dC5qYXZh) | `79.85% <65.38%> (-20.15%)` | :arrow_down: |
... and [34 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/8140/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/8140?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat:automatic update after BusinessActionContext modify [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8140:
URL: https://github.com/apache/incubator-seata/pull/8140#discussion_r3403878022
##########
integration-tx-api/src/main/java/org/apache/seata/rm/tcc/api/BusinessActionContext.java:
##########
@@ -253,4 +287,179 @@ public String toString() {
.append("]");
return sb.toString();
}
+
+ /**
+ * The tracked action context map.
+ */
+ private static final class TrackedActionContextMap extends AbstractMap<String, Object> implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ private final BusinessActionContext owner;
+
+ private final Map<String, Object> delegate;
+
+ private TrackedActionContextMap(BusinessActionContext owner) {
+ this.owner = owner;
+ this.delegate = new HashMap<>(8);
+ }
+
+ private TrackedActionContextMap(BusinessActionContext owner, Map<String, Object> source) {
+ this.owner = owner;
+ this.delegate = new HashMap<>(source);
+ }
+
+ @Override
+ public Object put(String key, Object value) {
+ boolean hadKey = delegate.containsKey(key);
+ Object previousValue = delegate.put(key, value);
+ if (!hadKey || !Objects.equals(previousValue, value)) {
+ owner.markUpdatedOnActionContextMutation();
+ }
+ return previousValue;
+ }
+
+ @Override
+ public void putAll(Map<? extends String, ? extends Object> m) {
+ Objects.requireNonNull(m, "m");
+ if (m.isEmpty()) {
+ return;
+ }
+ for (Map.Entry<? extends String, ? extends Object> entry : m.entrySet()) {
+ put(entry.getKey(), entry.getValue());
+ }
+ }
+
+ @Override
+ public Object remove(Object key) {
+ boolean hadKey = delegate.containsKey(key);
+ Object previousValue = delegate.remove(key);
+ if (hadKey) {
+ owner.markUpdatedOnActionContextMutation();
+ }
+ return previousValue;
+ }
+
+ @Override
+ public void clear() {
+ if (!delegate.isEmpty()) {
+ delegate.clear();
+ owner.markUpdatedOnActionContextMutation();
+ }
+ }
+
+ @Override
+ public Set<Entry<String, Object>> entrySet() {
+ return new AbstractSet<Entry<String, Object>>() {
+ @Override
+ public Iterator<Entry<String, Object>> iterator() {
+ Iterator<Entry<String, Object>> iterator =
+ delegate.entrySet().iterator();
+ return new Iterator<Entry<String, Object>>() {
+ private boolean canRemove;
+
+ @Override
+ public boolean hasNext() {
+ return iterator.hasNext();
+ }
+
+ @Override
+ public Entry<String, Object> next() {
+ Entry<String, Object> current = iterator.next();
+ canRemove = true;
+ return new TrackingEntry(current);
+ }
+
+ @Override
+ public void remove() {
+ iterator.remove();
+ if (canRemove) {
+ owner.markUpdatedOnActionContextMutation();
+ }
+ canRemove = false;
+ }
+ };
+ }
+
+ // The following are read-only methods that delegate directly to the native Map.
Review Comment:
The comment says the following methods are "read-only", but the block includes mutating operations (e.g., `remove`). This can confuse future maintainers about what is expected to trigger tracking.
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: automatic update after BusinessActionContext modify [incubator-seata]
Posted by "Zhengcy05 (via GitHub)" <gi...@apache.org>.
Zhengcy05 commented on PR #8140:
URL: https://github.com/apache/incubator-seata/pull/8140#issuecomment-4714585345
> LGTM Please register your PR in those two files : https://github.com/apache/incubator-seata/blob/2.x/changes/zh-cn/2.x.md https://github.com/apache/incubator-seata/blob/2.x/changes/en-us/2.x.md
Done. 🫰
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org