[konlue]

Description

Fixes #7589.

Renames the misspelled loged column of the selector and rule tables to logged, and finishes the corresponding field renames across the codebase. The rename was already half-landed (SelectorData in shenyu-common and the e2e models used logged while RuleData and the admin DTOs used loged), so this makes the naming consistent everywhere in one pass.

Changes

• Schemas: loged → logged in the mysql/pg/oracle init schemas, the h2 schema, and the mysql seed INSERT column list.
• Upgrade scripts: new db/upgrade/2.7.2-upgrade-2.7.3-{mysql,pg,oracle,og,ob}.sql issuing ALTER TABLE ... RENAME COLUMN loged TO logged for both tables, following the existing 2.7.x upgrade script conventions. Existing deployments must run the script for their dialect before upgrading.
• Mappers: selector-sqlmap.xml / rule-sqlmap.xml column references updated (12 + 11 occurrences).
• Java: SelectorDTO, RuleDTO, SelectorDO, RuleDO, SelectorVO, RuleVO, events, SelectorUtil, register services, gateway-side RuleData, AbstractShenyuPlugin, LocalPluginController, the k8s parsers and the e2e models now use logged/getLogged/setLogged consistently. Tests updated accordingly (no assertion changes beyond the rename).
• Compatibility (one release):
  • SelectorDTO/RuleDTO carry @JsonAlias("loged"), so REST requests posting the old field name keep working; responses now always serialize logged.
  • RateLimiterHandle uses @SerializedName(value = "logged", alternate = "loged"), so rate-limiter rule handle JSON stored with the old key by existing deployments keeps parsing; new writes use logged (Gson writes the primary name only, so re-saved handles converge to logged).
• Admin console bundle: updated the 3 "loged" form-field occurrences in the checked-in index.f5d66cf1.js bundle. Note for maintainers: the upstream shenyu-dashboard source needs the matching one-word change in a follow-up (I can link a PR there if wanted).
• Release notes: recorded under Unreleased → Behavior Changes with the upgrade-script requirement and the REST response field change.

Verbatim check

git grep -in loged returns 0 hits after this change.

Checklist

• I have read the CONTRIBUTING doc.
• I have updated the documentation accordingly (release notes + upgrade scripts).
• ./mvnw -B install -DskipTests -Dmaven.javadoc.skip=true -pl shenyu-admin,shenyu-kubernetes-controller -am → BUILD SUCCESS
• ./mvnw -B test -pl shenyu-admin -Dtest='*Selector*,*Rule*,*Register*,*RateLimiter*' → 407 tests, 0 failures; ./mvnw -B test -pl shenyu-common,shenyu-plugin/shenyu-plugin-base,shenyu-web → all green; shenyu-plugin-ratelimiter -am → 37/37; shenyu-plugin-modify-response,shenyu-plugin-wasm-base -am → all green

[Aias00]

Request changes — three items, all mechanical. The rename itself is complete and clean (details in "What I verified"); my asks are about the two compatibility edges the PR does not yet cover and one doc file.

What I verified

The rename is complete. I grepped the full tree at cf0d8330 case-insensitively for loged. The only remaining hits are the intentional ones:

• RELEASE-NOTES.md (prose describing the rename),
• @JsonAlias("loged") in SelectorDTO / RuleDTO,
• @SerializedName(value = "logged", alternate = "loged") in RateLimiterHandle,
• the five ALTER TABLE ... RENAME COLUMN statements.

Everything else moved: the four init schemas plus the MySQL seed INSERT column list, the h2 schema, both sqlmaps (result column, plain select lists, INSERT column+values, the dynamic <if> update columns and the UPDATE ... SET clause), SelectorDO/RuleDO/SelectorDTO/RuleDTO/SelectorVO/RuleVO, the two changed events, SelectorUtil, both register services, gateway-side RuleData, AbstractShenyuPlugin, LocalPluginController, all seven k8s parsers, the e2e models, the tests, and the minified dashboard bundle (one line, all three occurrences).

Both compatibility mechanisms are correctly wired.

• @JsonAlias is deserialization-only, so old REST requests posting loged keep working while responses always emit logged — exactly as documented. Worth noting the alias is load-bearing, not cosmetic: logged carries @NotNull, so without it old clients would get a 400 rather than a silently-null field.
• For RateLimiterHandle I confirmed the only parse site is RateLimiterPluginDataHandler.java:88 (GsonUtils.getInstance().fromJson(s, RateLimiterHandle.class)), so the Gson alternate is effective, and Gson serializes the primary name only, matching your convergence claim.

Blocking

1. RuleData.logged is a wire-format rename with no compatibility, and the failure mode is an NPE on the hot path

RuleData is the admin→bootstrap sync contract. On master it is private Boolean loged; (RuleData.java:39); this PR renames it to logged. Both sync paths deserialize with Gson, which silently drops unknown keys:

• HTTP long-poll: RuleDataRefresh.java:55-56 → GsonUtils.getGson().fromJson(data, new TypeToken<ConfigData<RuleData>>() {}.getType())
• WebSocket: ShenyuWebsocketClient.java:442 → GsonUtils.getInstance().fromJson(result, WebsocketData.class)

So during a rolling upgrade (admin and bootstrap are separate processes, usually not restarted together):

• new bootstrap + old admin: the old admin emits "loged": true; the new bootstrap drops it, leaving RuleData.logged == null. AbstractShenyuPlugin.printLog(RuleData, String) then does if (ruleData.getLogged()) (AbstractShenyuPlugin.java:352) — unboxing a null Boolean. That call sits on the request path at AbstractShenyuPlugin.java:116 and :132, i.e. every matched rule throws NPE, which would take the gateway down for all traffic.
• new admin + old bootstrap: symmetric — the old bootstrap's loged stays null and its own printLog NPEs. This direction cannot be fixed in this repo, it has to be handled by upgrade ordering.

This is inconsistent with the rest of the PR: you protected the nested rate-limiter handle and the REST DTOs, but not the outer field that carries them. Two cheap options, either is fine:

• add @SerializedName(value = "logged", alternate = "loged") to RuleData.logged (one line, mirrors what you already did for RateLimiterHandle, and Gson is the only deserializer on this path), and
• make printLog null-safe (Boolean.TRUE.equals(...)) as defence in depth — LocalPluginController.buildDefaultRuleData already guards this field, so the guard is clearly considered necessary elsewhere.

For context on why I still think this is worth doing despite the precedent: SelectorData.logged was renamed the same way back in the 2.5.0 cycle (it is already logged at tag v2.5.0, and no historical db/upgrade script ever mentioned loged), so the project has shipped this pattern before. But that rename predates the compat discipline this PR applies elsewhere. At minimum, please state the required upgrade order explicitly in the release note ("upgrade all bootstrap instances before, or together with, admin"), since the new-admin/old-bootstrap direction is unfixable in code.

2. The five new upgrade scripts are not executed by any test

.github/scripts/native-sql-matrix.py:469 hardcodes the migration file as db/upgrade/2.7.1-upgrade-2.7.2-{dialect}.sql and pins BASELINE to v2.7.1 (:27). The native-sql (mysql / ob / pg / og / oracle) checks that passed therefore ran the old upgrade script plus the fresh-install schema — the new ALTER TABLE ... RENAME COLUMN statements were never executed on a real engine. The fresh-install flow does cover the renamed schema columns, so the schema half is verified; the migration half is not.

These are one-shot migrations that operators must run before upgrading, and the harness aborts on any SQL error, so extending it is a genuine gate rather than paperwork. Please either extend the matrix to also execute the newest upgrade script (baseline bumped to the PR head, both scripts in sequence), or verify each of the five manually on a real engine and paste the results into the PR.

3. db/upgrade/upgrade-guide.md still points at the previous migration

The guide's "To Shenyu Users" section (upgrade-guide.md:9-27) still tells operators to run 2.7.1-upgrade-2.7.2-<dialect>.sql and describes the 2.7.2 data-permission change. Because this rename makes running an upgrade script mandatory for existing deployments, the new scripts need to be listed there in the same bullet format — otherwise an operator following the guide will skip the migration and the new admin will fail against the old schema.

CI status

The red is a pre-existing flake, not a regression:

• The e2e job that failed in 3s (job 116840545041) is the fail-fast aggregator — its log is just [[ "failure" == "success" ]] || exit -1.
• The real failure is e2e-storage (mysql) (job 116836501899): DividePluginTest.testDivide:105->lambda$testDivide$1:105 endpoint '/anything/xx/yy' not exists, case single-divide uri starts_with. That test file and its case provider are untouched by this PR (git diff origin/master cf0d8330 -- .../testcase/divide/DividePluginTest.java .../DividePluginCases.java is empty).
• Master hit the identical assertion three hours earlier on a different engine: run 38836735684 (head 297ac6f8), job 116769167809 (postgres) — same message, same endpoint. The three most recent master runs are green.

So no code change is needed for CI, but the run does need to be re-triggered (the fail-fast cancellation also took out the h2/pg/og storage siblings) before merge.

Minor

• The dashboard bundle edit is fine, but as you note the matching one-word change in the shenyu-dashboard source should land (and be linked) before this bundle gets rebuilt, otherwise the next dashboard release will silently revert the fix.

Once the three blocking items are addressed and a clean e2e run lands, this is good to merge — no other code concerns from me.

[konlue]

Hi @Aias00, thank you for the thorough review — the RuleData wire-format catch in particular was exactly the edge I missed. All three items are addressed:

1. RuleData sync-contract compatibility

   • RuleData.logged now carries @SerializedName(value = "logged", alternate = "loged") — the same mechanism as RateLimiterHandle, and Gson is the deserializer on both sync paths.
   • AbstractShenyuPlugin.printLog now uses Boolean.TRUE.equals(ruleData.getLogged()) as defence in depth.
   • The release note states the required upgrade order explicitly: “upgrade all bootstrap instances before, or together with, admin”, since the new-admin/old-bootstrap direction cannot be tolerated in code.
   • Red-green verified both ways:
     • testDeserializationAcceptsLegacyLogedKey fails without the alternate when parsing legacy {"loged": true} sync JSON.
     • printLogToleratesNullLoggedFlag throws NPE without the guard when invoking printLog with a null flag.

2. Upgrade scripts gated on real engines

   • native-sql-matrix.py now executes both scripts in sequence on top of the pinned v2.7.1 baseline:
     MIGRATIONS_FROM_BASELINE = ("2.7.1-upgrade-2.7.2", "2.7.2-upgrade-2.7.3"), keeping abort-on-SQL-error.
   • The native-sql jobs on this PR will therefore run the ALTER TABLE ... RENAME COLUMN statements on all five engines.
   • The migration files are read from the PR tree, so future additions only need one list entry.
   • The matrix’s post-migration assertions only read plugin columns and table ids, so the renamed selector/rule columns do not disturb them.

3. Upgrade guide

   • db/upgrade/upgrade-guide.md gained a “loged column renamed to logged in 2.7.3” section listing all five scripts as mandatory.
   • It includes the upgrade-order note and the stop-admin/run/restart sequence mirroring the 2.7.2 section.

Minor: the matching one-word change in the dashboard source is ready as apache/shenyu-dashboard#723 (3 files, 18 occurrences: Selector.js, Rule.js, ToolsModal.js).

Verification: shenyu-common + shenyu-plugin-base reactor tests 250/250 green with checkstyle. I agree the previous e2e red was the pre-existing DividePluginTest flake; I’ll watch the new run and re-trigger if it recurs.

[Aias00]

Summary

Re-review at current head 6ffa0d8107. The loged → logged rename is complete and all three items from my previous review are now addressed.

Resolved

1. RuleData sync-contract compatibility — RuleData.java:41-42 now carries @SerializedName(value = "logged", alternate = "loged"), so Gson keeps deserializing the old admin payload during a rolling upgrade; AbstractShenyuPlugin.printLog(RuleData) (line 352) now uses Boolean.TRUE.equals(ruleData.getLogged()), removing the NPE-on-the-hot-path risk. The release note also states the required upgrade ordering.
2. Migration execution is now covered — native-sql-matrix.py applies the upgrade chain from the v2.7.1 baseline (MIGRATIONS_FROM_BASELINE = ("2.7.1-upgrade-2.7.2", "2.7.2-upgrade-2.7.3")), and the new 2.7.2-upgrade-2.7.3-*.sql scripts (mysql/ob/og/pg/oracle) carry the RENAME COLUMN statements.
3. db/upgrade/upgrade-guide.md now lists the new migration in the "To Shenyu Users" section.

Verification

• Head source confirmed for the annotation and the null-safe printLog guard.
• CI: pr_build, Analyze (java), check-license-header, rat_check, scope, sql-matrix pass. The e2e / *-sync-compose / cluster-jdbc reds are the known httpbin.org / RocketMQ infra flakes, untouched by a rename.

Conclusion

APPROVE.

[konlue]

@Aias00 PTAL. Resolved the RELEASE-NOTES.md conflict with master — both release-note entries were kept, no content change to the PR itself. CI has been re-triggered; ready for re-review.

[Aias00]

Approved. This is a complete rename rather than a partial one, and the compatibility shims show the right instincts.

I checked the completeness mechanically: 243 loged occurrences removed and only 20 remain, and every one of those is intentional — the RENAME COLUMN loged TO logged statements in the five dialect scripts, @JsonAlias("loged") on the request DTOs, @SerializedName(value = "logged", alternate = "loged") on the response VOs, and a legacy-JSON test. Nothing was left half-renamed, which is the failure mode that makes this class of PR dangerous: a mapper still selecting loged against a renamed column fails at runtime, not at compile time.

The scope is right too — init schemas for mysql/pg/oracle/h2, the seed INSERT column list, both SQL maps, e2e models, the kubernetes controller and the plugin/web layers. Updating .github/scripts/native-sql-matrix.py so the column contract test follows the rename is the detail that keeps the guard meaningful instead of silently checking a column that no longer exists.

Two things I would want settled before merge, both about coordination rather than code:

1. ALTER TABLE ... RENAME COLUMN requires MySQL 8.0+. The repository’s own compose files all use mysql:8, so CI and e2e are unaffected — but a production deployment still on MySQL 5.7 (EOL, yet common) gets a syntax error halfway through the upgrade, on the first statement, with the second table untouched. Two options: state Requires MySQL 8.0+ in the script header and the release notes, or use ALTER TABLE ... CHANGE COLUMN loged logged TINYINT(1) ..., which works on both. Either is fine, but silence is not, because the script is meant to be run by an operator on a database they already have.

2. The response field rename is a breaking REST API change, and the dashboard lives in another repository. @SerializedName(value = "logged", alternate = "loged") serialises as logged — the alternate only affects deserialisation. So an admin UI reading loged from SelectorVO/RuleVO will render the flag as absent. The request side is handled (@JsonAlias), and the release notes call out the response change, which is good; the open question is whether apache/shenyu-dashboard has a counterpart change queued. If not, the rename lands before the UI can follow.

Two smaller notes:

• Rolling upgrades. Renaming the column breaks any admin instance still running the old code (SELECT ... loged against a renamed column). The upgrade note should say the script and the deploy have to move together, or that a brief outage is expected during the upgrade — otherwise a rolling restart during the window produces errors that look unrelated to the SQL migration.
• og/ob init schemas. The upgrade scripts cover five dialects, but the init-side changes only touch mysql/pg/oracle/h2. If openGauss and OceanBase initialise from the pg or mysql scripts then nothing is missing; worth confirming rather than assuming, since they got their own upgrade files and therefore clearly diverge somewhere.

Adding the 2.7.2-upgrade-2.7.3-* scripts following the existing conventions, rather than only editing the init schema, is what makes this shippable at all — an init-only change would have left every existing deployment broken with no documented path forward.

CI on this branch is red on the inherited InstanceCheckServiceTest pair, not on this change — #7846 fixes it.

[Aias00]

Status update after the master fix landed: the inherited InstanceCheckServiceTest failures are gone, but the re-run now surfaces a compile error that is specific to this PR, so it needs one more commit before it can merge.

The failure

[shenyu-plugin-base] [ERROR] .../AbstractShenyuPluginTest.java:[311,22] cannot find symbol
  symbol:   method setLoged(boolean)
  location: variable ruleData of type org.apache.shenyu.common.dto.RuleData
[shenyu-plugin-base] [ERROR] .../AbstractShenyuPluginTest.java:[331,22] cannot find symbol
  symbol:   method setLoged(boolean)

Why

AbstractShenyuPluginTest is one of the files this PR renames, but master gained two new test methods in that same file after this branch was cut:

a186c1c  fix(log): reduce routine matching and configuration-sync log volume (#7643) (#7663)   2026-10-17 05:47

Those two methods (defaultStrategyRuleMatchShouldNotLogWhenLoggingDisabled, defaultStrategyRuleMatchShouldLogExactlyOnceWhenLoggingEnabled) call ruleData.setLoged(...). Because they occupy regions this PR did not touch, the merge succeeds without a textual conflict and leaves the renamed setter next to the old call sites — the classic "merges cleanly, does not compile" case.

I verified the complete set on the merge ref of this pull request: there are exactly two leftovers and both are in that file.

git grep -nE "setLoged|getLoged|isLoged" <merge-ref> -- "*.java"
# AbstractShenyuPluginTest.java:311:        this.ruleData.setLoged(false);
# AbstractShenyuPluginTest.java:331:        this.ruleData.setLoged(true);

Fix

Rebase onto master and rename those two call sites to setLogged:

• AbstractShenyuPluginTest.java:311 -> this.ruleData.setLogged(false);
• AbstractShenyuPluginTest.java:331 -> this.ruleData.setLogged(true);

Nothing else is needed — every other loged occurrence on master is already covered by this PR.

Two non-blocking reminders from my earlier review still stand, and are worth folding into the same push since you are touching the branch anyway:

1. ALTER TABLE ... RENAME COLUMN requires MySQL 8.0+. Either state the requirement in the script header and release notes, or switch to CHANGE COLUMN, which works on 5.7 as well.
2. @SerializedName(value = "logged", alternate = "loged") serialises as logged, so the dashboard in apache/shenyu-dashboard needs a matching change before this ships.

[konlue]

Merged latest master into this branch. (The rename fix itself is the "Merge master into rename branch" commit ea73fc7c5; the tip merge on top just reconciles history with the earlier conflict resolution.)

• Resolved new merge conflicts: the two new setLoged call sites that came in from fix(log): reduce routine matching and configuration-sync log volume (#7643) #7663 (AbstractShenyuPluginTest 311/331) are renamed to setLogged, and two assertion-message strings picked up from master now say logged too — grep -in loged over this file is back to zero.
• MySQL compatibility note: addressed the note in db/upgrade/upgrade-guide.md. The script's ALTER TABLE ... RENAME COLUMN requires MySQL 8.0+, with the CHANGE COLUMN equivalent spelled out for 5.7.
• Dashboard counterpart: remains queued as fix: rename misspelled loged field to logged in selector/rule forms shenyu-dashboard#723.

[Aias00]

Re-approving after the master merge dismissed the previous approval.

I checked the update rather than re-reading the whole PR: the only new commit is a merge of master, and every file this PR introduces is byte-identical to the version I approved (compared the blob shas at 71fe4acf/d80bda94/c831b431/58f16918 against the current head). The files that did change are the ones master touched in parallel — RELEASE-NOTES.md, the other agent-gateway sources, and AbstractShenyuPluginTest's setLogged rename — i.e. the merge brought them in, it did not rewrite this PR's content.

CI is green on the new head (52 pass / 3 skipping, no failures). No further changes requested from me.