[dengliming]

Run the plugin-created resource initialization listener in one transaction. If any resource or downstream permission write fails, all writes from the listener now roll back together, matching the existing plugin-deletion listener behavior.

Adds a regression check for the listener transaction configuration and rollback policy.

Tested with:
./mvnw -q -pl shenyu-admin -am -DskipTests=false -Dcheckstyle.skip=false -Dtest=ResourceServiceTest -DfailIfNoTests=false test

Fixes #6622

[Aias00]

Correct change. onPluginCreated performs several writes (resource row, permission/resource bindings) and previously had no transaction, so a failure partway through left an orphaned resource behind — and because the Assert failure path is precisely where you'd want a rollback, the missing @Transactional was easy to hit.

rollbackFor = Exception.class is the right choice here rather than the Spring default: Assert.isNull(...) throws IllegalArgumentException (a RuntimeException, so it would roll back anyway), but any checked exception surfacing from the mapper would otherwise commit a half-built resource.

Approving with one observation on the test: testPluginCreationListenerIsTransactional asserts the annotation exists via reflection. That pins the declarative metadata but not the behaviour — it will not catch, for example, a self-invocation problem or a proxy-less call path where the annotation is present but never applied. An integration-style test (force a mapper failure and assert the resource insert is rolled back) would be a much stronger guarantee. Not a blocker, just noting that the current test is a guard against accidental deletion rather than a functional check.

[Aias00]

Reviewed at head a637bda. Resource, permission events, and plugin button inserts now share one synchronous rollback boundary; targeted transaction tests pass. I am approving code but will not merge until the failed E2E reruns green.

[Aias00]

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]

@dengliming This PR is approved, but 7 checks are red: e2e, e2e-case (e2e-websocket-sync-compose, shenyu-e2e-case-websocket, :shenyu-example-spring-native-we..., e2e-case (e2e-springcloud-sync-compose, shenyu-e2e-case-spring-cloud, :shenyu-examples-eureka,:sh..., e2e-case (e2e-logging-rocketmq-compose, shenyu-e2e-case-logging-rocketmq, :shenyu-examples-http) (+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]

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

[Aias00]

Hi @dengliming, this PR is already approved, but it currently cannot be merged. Here is the blocker and what is needed:

Blocker
This PR is approved but blocked. The e2e / http case fails from a stale base.

What we need
Please rebase onto current master and re-run CI.

Once the branch is up to date and CI is green, we will squash-merge it. Thanks!

[Aias00]

Reviewed the transaction boundary around plugin resource initialization and rollback of partial resource creation. No actionable finding.

Validation: 14 PluginResourceTransactionTest / ResourceServiceTest tests passed. GitHub integration CI currently includes a WebSocket UploadControllerTest assertion failure; this review does not certify those integration jobs or authorize bypassing required checks. Apache RAT passed locally. Scope: source review and targeted verification; full integration/e2e suites were not run locally for this review.

[Aias00]

Re-reviewed the transaction around plugin resource initialization and rollback behavior on current master. No actionable finding. Focused transaction/resource service and Admin long-polling tests: 57 tests passed. E2E CI still has a failing aggregate and remains a merge gate.

Checkstyle and a separate Apache RAT check passed locally. This review covers source and the stated targeted verification; full Docker integration/E2E suites were not run locally.

[Aias00]

Stream Agent analysis

FINAL synthesis of PR #7192 at head 84d01e2. Child partition part-0 PASS with no blocking findings; complete_diff true, all_parts_passed true. However CI status is complete but passed=false (40 checks). Per review policy, FINAL PASS requires CI passed; thus verdict cannot be PASS. No defects anchored, so not NEEDS_CHANGES. Marking INCONCLUSIVE pending green CI.

Evidence

• Child review part-0: PASS, no blocking findings
• CI: complete=true, passed=false, check_count=40
• complete_diff=true, all_parts_passed=true, head=84d01e2

Recommended next steps

• Investigate CI failure (40 checks reported passed=false) and fix or re-run
• After CI green, finalize approval as changes appear sound

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: c6578656-b9db-4629-9687-bcee846ba6e4 · Commit: 84d01e2ff3f75a02a9de965cd00e455a9972ae85

Review verdict: INCONCLUSIVE

Scope FINAL: assessed cross-file from child review summary. Child part-0 reviewed three file segments and returned PASS with no HIGH/CRITICAL findings. complete_diff=true, global_complete_diff=true, all_parts_passed=true. CI indicates complete=true but passed=false (40 checks). Rule: all_parts_passed true, complete_diff true and CI passed required before FINAL PASS. Since CI not passed, cannot output PASS. No invented defects, so NEEDS_CHANGES inappropriate. Verdict INCONCLUSIVE due to unverified CI state preventing final approval.

Publication: COMMENT (INCONCLUSIVE_REVIEW)

Review details

The code changes for transactional plugin resource initialization look correct in the supplied diffs and tests are added. However, the CI pipeline (40 checks) has not passed (passed=false). According to review policy, we cannot mark this PR as passed until CI is green. Please investigate the CI failures and re-run checks. Once CI passes, this can be approved. No code defects were identified in the reviewed partitions.

[Aias00]

Stream Agent analysis

FINAL review of #7192 at head 84d01e2. Cross-file synthesis from child part-0 (PASS) shows @transactional added to ResourceServiceImpl.onPluginCreated with regression tests PluginResourceTransactionTest and ResourceServiceTest. complete_diff true, all_parts_passed true, CI 68 checks passed. No cross-file defects or contract violations identified. Verdict PASS.

Evidence

• Child review part-0 PASS: transactional annotation and tests consistent
• CI passed: 68 checks complete, no failures
• complete_diff true, all_parts_passed true, scope FINAL

Recommended next steps

• Maintainer to merge following project review process
• Optionally extend tests to multi-plugin batch scenarios

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: 0164630b-1e1c-4d3f-aa04-3ba422fe5ae0 · Commit: 84d01e2ff3f75a02a9de965cd00e455a9972ae85

Review verdict: PASS

Scope FINAL synthesis. Child part-0 reviewed PARTITION with complete_diff true, covering ResourceServiceImpl.java @transactional addition, PluginResourceTransactionTest, ResourceServiceTest. No HIGH/CRITICAL findings. all_parts_passed true, global_complete_diff true, CI passed (68 checks). Cross-file assessment shows consistent transactional pattern with existing plugin-deletion listener, no contract or compatibility issues. Therefore FINAL PASS is justified per policy.

Publication: APPROVE (EXPLICIT_PASS)

Review details

Based on the supplied review evidence and FINAL scope, this PR correctly introduces transactional boundaries for plugin resource initialization and includes tests validating rollback. All partitioned reviews passed, diff is complete, and CI is green. Recommendation: approve and merge per maintainer workflow. No blocking findings.