Skip to content

test(core): lock inline rate_limit rps/rph acceptance (models & api keys) - #653

Merged
jarvis9443 merged 1 commit into
mainfrom
fix/rate-limit-rps-rph-validation
Jun 26, 2026
Merged

test(core): lock inline rate_limit rps/rph acceptance (models & api keys)#653
jarvis9443 merged 1 commit into
mainfrom
fix/rate-limit-rps-rph-validation

Conversation

@jarvis9443

@jarvis9443 jarvis9443 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Context

#644 reported that the Admin API validation schema rejects inline rate_limit.rps / rate_limit.rph for models and API keys, even though the generated OpenAPI and the RateLimit struct advertise them.

That was accurate for the old hand-written validation schema, but it was already resolved by #638 (merged the day before the issue was filed). #638 made the resource structs the single source of truth: model_root_schema() / apikey_root_schema() now derive from the Model / ApiKey structs via schemars, and RateLimit carries all seven windows (rps / rpm / rph / rpd + tpm / tpd + concurrency). So rps / rph are already accepted today.

Change

No production change is needed. This adds the regression tests the issue asked for, so the per-second and per-hour request windows can't silently drop out of the derived schema again:

  • model_rate_limit_accepts_all_request_windows_incl_rps_rph
  • apikey_rate_limit_accepts_rps_and_rph

Both pass on current main, confirming the fix is in place.

This is a schema-validation regression guard, tested at its own boundary (aisix-core validators), so no separate data-plane E2E is added — there is no new end-to-end runtime behavior, the schema shipped in #638.

Verified locally: cargo test -p aisix-core, cargo fmt --all -- --check, cargo clippy -p aisix-core --all-targets.

Fixes #644

Summary by CodeRabbit

  • Tests
    • Added regression coverage for JSON schema validation of inline rate limits.
    • Expanded validation checks to confirm supported request windows are accepted for models and API keys, including per-second, per-minute, per-hour, and per-day cases.

…i keys

#644 reported the Admin API rejected inline rate_limit.rps/rph. That held
for the old hand-written validation schema but was already fixed by #638,
which derives the model/api-key schemas from the RateLimit struct (rps,
rpm, rph, rpd, tpm, tpd, concurrency). Add regression tests so the
per-second and per-hour request windows can't silently drop out of the
derived schema again.
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 65890235-389c-4bd4-b692-cc9adac781eb

📥 Commits

Reviewing files that changed from the base of the PR and between 40cd364 and f9df3cf.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

Adds regression tests for inline rate_limit schema validation on models and API keys, covering additional request-count windows and successful JSON validation.

Changes

Inline rate_limit schema validation

Layer / File(s) Summary
Regression tests for inline rate_limit
crates/aisix-core/src/models/schema.rs
Adds regression tests for inline rate_limit on models and API keys, covering request-count windows and successful JSON validation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning These are schema-level unit tests only; they stop at validate_model/validate_apikey and don’t exercise the Admin API→service→DB flow required for E2E coverage. Add at least one integration/E2E test that writes a model and an API key through the Admin API with rps/rph and asserts the request is accepted end-to-end; keep the unit tests as regression coverage.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the added regression tests for inline rate_limit rps/rph acceptance in models and API keys.
Linked Issues check ✅ Passed The PR adds the requested regression tests covering rps and rph acceptance for models and API keys, matching the issue's validation scope.
Out of Scope Changes check ✅ Passed The change stays within the issue scope and only adds schema-validation regression tests in aisix-core.
Security Check ✅ Passed Only regression tests were added in schema.rs; no production auth, storage, logging, or secret-handling code changed.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rate-limit-rps-rph-validation

Comment @coderabbitai help to get the list of available commands.

@jarvis9443
jarvis9443 merged commit 6b283c6 into main Jun 26, 2026
10 checks passed
@jarvis9443
jarvis9443 deleted the fix/rate-limit-rps-rph-validation branch June 26, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Admin API validation schema missing rps/rph

1 participant