refactor: replace custom rate limiter with symfony/rate-limiter#4082
Merged
refactor: replace custom rate limiter with symfony/rate-limiter#4082
Conversation
Replaced the custom SQL-based fixed-window rate limiter with symfony/rate-limiter's FixedWindowLimiter. This eliminates the faqrate_limits database table entirely — rate limit state is now held in memory (swappable to a cache adapter for persistence). The public API (check/headers/getHeaders) and HTTP header format (X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, Retry-After) remain unchanged.
📝 WalkthroughWalkthroughReplaces DB-backed API rate limiter with Symfony RateLimiter and cache-backed storage; removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Listener as ApiRateLimiterListener
participant RL as RateLimiter
participant Factory as RateLimiterFactory
participant Storage as Cache/Storage
Client->>Listener: HTTP API request (client key)
Listener->>RL: check(key, limit, interval)
RL->>Factory: createLimiter("api", key)
Factory->>Storage: consume(1)
Storage-->>Factory: result (accepted, remaining, reset)
Factory-->>RL: consumption outcome
alt accepted
RL-->>Listener: allowed + headers(limit, remaining, reset)
Listener-->>Client: proceed (no interruption)
else denied
RL-->>Listener: denied + headers(limit, remaining=0, reset)
Listener-->>Client: 429 JSON + Retry-After + rate headers
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
Owner
Author
|
@coderabbitai configuration |
|
Configuration used: defaults CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
inheritance: false
reviews:
profile: chill
request_changes_workflow: false
high_level_summary: true
high_level_summary_instructions: ''
high_level_summary_placeholder: '@coderabbitai summary'
high_level_summary_in_walkthrough: false
auto_title_placeholder: '@coderabbitai'
auto_title_instructions: ''
review_status: true
review_details: false
commit_status: true
fail_commit_status: false
collapse_walkthrough: true
changed_files_summary: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: true
related_prs: true
suggested_labels: true
labeling_instructions: []
auto_apply_labels: false
suggested_reviewers: true
auto_assign_reviewers: false
in_progress_fortune: true
poem: true
enable_prompt_for_ai_agents: true
path_filters: []
path_instructions: []
abort_on_close: true
disable_cache: false
auto_review:
enabled: true
description_keyword: ''
auto_incremental_review: true
auto_pause_after_reviewed_commits: 5
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
ignore_usernames: []
finishing_touches:
docstrings:
enabled: true
unit_tests:
enabled: true
simplify:
enabled: false
custom: []
pre_merge_checks:
override_requested_reviewers_only: false
docstrings:
mode: warning
threshold: 80
title:
mode: warning
requirements: ''
description:
mode: warning
issue_assessment:
mode: warning
custom_checks: []
tools:
ast-grep:
rule_dirs: []
util_dirs: []
essential_rules: true
packages: []
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_rules: []
disabled_rules: []
enabled_categories: []
disabled_categories: []
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
phpmd:
enabled: true
phpcs:
enabled: true
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
trufflehog:
enabled: true
checkov:
enabled: true
tflint:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
flake8:
enabled: true
fortitudeLint:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
clang:
enabled: true
cppcheck:
enabled: true
opengrep:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
clippy:
enabled: true
sqlfluff:
enabled: true
trivy:
enabled: true
prismaLint:
enabled: true
pylint:
enabled: true
oxc:
enabled: true
shopifyThemeCheck:
enabled: true
luacheck:
enabled: true
brakeman:
enabled: true
dotenvLint:
enabled: true
htmlhint:
enabled: true
stylelint:
enabled: true
checkmake:
enabled: true
osvScanner:
enabled: true
blinter:
enabled: true
smartyLint:
enabled: true
emberTemplateLint:
enabled: true
psscriptanalyzer:
enabled: true
chat:
art: true
allow_non_org_members: true
auto_reply: true
integrations:
jira:
usage: auto
linear:
usage: auto
knowledge_base:
opt_out: false
web_search:
enabled: true
code_guidelines:
enabled: true
filePatterns: []
learnings:
scope: auto
issues:
scope: auto
jira:
usage: auto
project_keys: []
linear:
usage: auto
team_keys: []
pull_requests:
scope: auto
mcp:
usage: auto
disabled_servers: []
linked_repositories: []
code_generation:
docstrings:
language: en-US
path_instructions: []
unit_tests:
path_instructions: []
issue_enrichment:
auto_enrich:
enabled: false
planning:
enabled: true
auto_planning:
enabled: true
labels: []
labeling:
labeling_instructions: []
auto_apply_labels: false
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php (1)
54-59: Consider caching theRateLimiterFactoryinstance.A new
RateLimiterFactoryis instantiated on everycheck()call. While the overhead is minimal, caching factory instances per configuration (limit + interval) would be more efficient for high-throughput scenarios.♻️ Proposed optimization
+ /** `@var` array<string, RateLimiterFactory> */ + private array $factories = []; + public function check(string $key, int $limit, int $intervalSeconds): bool { $limit = max(1, $limit); $intervalSeconds = max(1, $intervalSeconds); - $factory = new RateLimiterFactory(config: [ - 'id' => 'api', - 'policy' => 'fixed_window', - 'limit' => $limit, - 'interval' => $intervalSeconds . ' seconds', - ], storage: $this->storage); + $factoryKey = "{$limit}_{$intervalSeconds}"; + if (!isset($this->factories[$factoryKey])) { + $this->factories[$factoryKey] = new RateLimiterFactory(config: [ + 'id' => 'api', + 'policy' => 'fixed_window', + 'limit' => $limit, + 'interval' => $intervalSeconds . ' seconds', + ], storage: $this->storage); + } + $factory = $this->factories[$factoryKey];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php` around lines 54 - 59, The code currently creates a new RateLimiterFactory inside check() for every call; change check() to reuse cached factory instances keyed by the configuration (e.g., "$limit:$intervalSeconds") instead of recreating each time: create a private property (e.g., $factoryCache) on the class and, inside check(), compute a cache key from $limit and $intervalSeconds, look up or instantiate a RateLimiterFactory (using the same config and $this->storage) and store it in the cache for subsequent calls; reference RateLimiterFactory, the check() method, $limit, $intervalSeconds and $this->storage when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php`:
- Around line 37-43: The RateLimiter currently falls back to InMemoryStorage in
the constructor (RateLimiter::__construct) which resets per PHP request and
disables persistent limits in production; change the constructor to require or
obtain a persistent StorageInterface (e.g., resolve a CacheStorage backed by
Redis/Memcached via configuration) instead of defaulting to InMemoryStorage, or
throw a clear exception/documentation note when no persistent storage is
provided so services.php usage is fixed; also stop instantiating a new
RateLimiterFactory on every check() call—add a private readonly
?RateLimiterFactory property on the RateLimiter class, initialize it once
(lazily in the constructor or first call) and reuse that instance inside
RateLimiter::check() to avoid repeated allocations.
---
Nitpick comments:
In `@phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php`:
- Around line 54-59: The code currently creates a new RateLimiterFactory inside
check() for every call; change check() to reuse cached factory instances keyed
by the configuration (e.g., "$limit:$intervalSeconds") instead of recreating
each time: create a private property (e.g., $factoryCache) on the class and,
inside check(), compute a cache key from $limit and $intervalSeconds, look up or
instantiate a RateLimiterFactory (using the same config and $this->storage) and
store it in the cache for subsequent calls; reference RateLimiterFactory, the
check() method, $limit, $intervalSeconds and $this->storage when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83da6eb7-7d5b-4032-ac66-d57779e7cc21
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
composer.jsonphpmyfaq/src/phpMyFAQ/Http/RateLimiter.phpphpmyfaq/src/phpMyFAQ/Setup/Installation/DatabaseSchema.phpphpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.phptests/phpMyFAQ/Http/RateLimiterTest.phptests/phpMyFAQ/Setup/Installation/DatabaseSchemaTest.phptests/phpMyFAQ/Setup/Installation/SchemaInstallerTest.phptests/phpMyFAQ/Setup/Migration/Versions/Migration420AlphaTest.php
💤 Files with no reviewable changes (2)
- tests/phpMyFAQ/Setup/Migration/Versions/Migration420AlphaTest.php
- phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php
Added CacheFactory that creates either a FilesystemAdapter (default) or RedisAdapter based on storage.cache* configuration keys. The rate limiter now uses cache-backed storage for cross-request persistence. New config keys in the Storage section: cacheAdapter, cacheRedisDsn, cacheRedisPrefix, cacheRedisConnectTimeout, cacheDefaultTtl.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php (1)
53-58: Consider caching theRateLimiterFactoryinstance for better performance.A new
RateLimiterFactoryis created on everycheck()call. While functionally correct, this is inefficient ifcheck()is called multiple times per request (though unlikely given typical API usage).This is a minor optimization opportunity—cache the factory if the config parameters match.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php` around lines 53 - 58, Cache the RateLimiterFactory instead of instantiating it on every check() call: add a private property (e.g., $rateLimiterFactoryCache) keyed by the config tuple (limit and intervalSeconds) and return the cached RateLimiterFactory when the same config is requested; only create a new RateLimiterFactory (the code creating $factory) when the cache miss occurs, storing it in the cache using the config key and reusing it for subsequent check() calls; ensure you still pass $this->storage to the factory constructor so behavior is unchanged.tests/phpMyFAQ/Http/RateLimiterTest.php (1)
98-105: Minor flakiness risk with time-based assertion.Using
time()in assertions can cause intermittent failures if the test runs across a second boundary. The risk is low here sincegetTimestamp()andtime()are called close together, but consider usingassertGreaterThan(time() - 1, $resetTime)for slightly more tolerance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpMyFAQ/Http/RateLimiterTest.php` around lines 98 - 105, The time-based assertion in testResetHeaderIsInTheFuture (using time() directly) is susceptible to one-second boundary flakiness; update the assertion that checks $limiter->headers['X-RateLimit-Reset'] to allow a small tolerance (for example assertGreaterThan(time() - 1, $resetTime) or assertGreaterThanOrEqual(time() - 1, $resetTime)) so the RateLimiter test is robust across second boundaries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpmyfaq/src/phpMyFAQ/Cache/CacheSettingsResolver.php`:
- Around line 49-52: Trim the fetched storage.cacheRedisPrefix value before
applying the fallback so whitespace-only entries don't bypass the default; in
CacheSettingsResolver (where $redisPrefix is set from
$this->databaseConfigurationStore->fetchValue('storage.cacheRedisPrefix')), call
trim() on the fetched value and then check if it's an empty string, assigning
self::DEFAULT_REDIS_PREFIX when trim($redisPrefix) === ''.
In `@phpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.php`:
- Around line 291-295: The migration Migration420Alpha is missing the cache
configuration keys that DefaultDataSeeder seeds for fresh installs
('storage.cacheAdapter', 'storage.cacheRedisDsn', 'storage.cacheRedisPrefix',
'storage.cacheRedisConnectTimeout', 'storage.cacheDefaultTtl'), causing upgrades
to rely on resolver fallbacks; update the up() (or equivalent) logic in
Migration420Alpha to insert or backfill these exact keys with the same default
values used in DefaultDataSeeder so upgraded instances persist the same cache
settings (ensure you use the same key names and default values and handle
existing keys safely to avoid overwriting user-customized values).
In `@tests/phpMyFAQ/Cache/CacheFactoryTest.php`:
- Around line 46-59: The test
testCreateReturnsFilesystemAdapterForInvalidAdapterValue duplicates
testCreateReturnsFilesystemAdapterByDefault and should instead simulate an
unsupported adapter value so CacheFactory/CacheSettingsResolver falls back to
filesystem; update this test to mock Configuration->getDb() to return a
DatabaseDriver mock where query/fetchObject return a config_value of an invalid
adapter (e.g., 'memcached') then assert the created cache from CacheFactory is
an instance of FilesystemAdapter and still clean up the temp cacheDir with
removeDirectory.
---
Nitpick comments:
In `@phpmyfaq/src/phpMyFAQ/Http/RateLimiter.php`:
- Around line 53-58: Cache the RateLimiterFactory instead of instantiating it on
every check() call: add a private property (e.g., $rateLimiterFactoryCache)
keyed by the config tuple (limit and intervalSeconds) and return the cached
RateLimiterFactory when the same config is requested; only create a new
RateLimiterFactory (the code creating $factory) when the cache miss occurs,
storing it in the cache using the config key and reusing it for subsequent
check() calls; ensure you still pass $this->storage to the factory constructor
so behavior is unchanged.
In `@tests/phpMyFAQ/Http/RateLimiterTest.php`:
- Around line 98-105: The time-based assertion in testResetHeaderIsInTheFuture
(using time() directly) is susceptible to one-second boundary flakiness; update
the assertion that checks $limiter->headers['X-RateLimit-Reset'] to allow a
small tolerance (for example assertGreaterThan(time() - 1, $resetTime) or
assertGreaterThanOrEqual(time() - 1, $resetTime)) so the RateLimiter test is
robust across second boundaries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b39b4fa2-7329-4a7a-8270-31b8553bf58c
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
composer.jsonphpmyfaq/src/phpMyFAQ/Cache/CacheFactory.phpphpmyfaq/src/phpMyFAQ/Cache/CacheSettings.phpphpmyfaq/src/phpMyFAQ/Cache/CacheSettingsResolver.phpphpmyfaq/src/phpMyFAQ/Http/RateLimiter.phpphpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.phpphpmyfaq/src/services.phpphpmyfaq/translations/language_en.phptests/phpMyFAQ/Cache/CacheFactoryTest.phptests/phpMyFAQ/Cache/CacheSettingsResolverTest.phptests/phpMyFAQ/Cache/CacheSettingsTest.phptests/phpMyFAQ/Http/RateLimiterTest.php
Select inputs for storage.cacheAdapter were empty because options are populated via a JS->API->PHP pipeline, not from the language file. Added the full chain: Helper::renderCacheAdapterOptions, ConfigurationTabController::cacheAdapter route, fetchCacheAdapter API function, and handleCacheAdapter tab handler.
- Move cache directory to phpmyfaq/content/core/cache/ and add to .gitignore - Add cache config keys to Migration420Alpha for upgrades - Trim redisPrefix in CacheSettingsResolver for consistency - Fix duplicate CacheFactoryTest to actually test invalid adapter fallback
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpmyfaq/src/phpMyFAQ/Cache/CacheSettingsResolver.php`:
- Around line 37-66: The resolve() method is making five separate
databaseConfigurationStore->fetchValue(...) calls which trigger multiple SQL
queries; collapse these into a single lookup and then derive adapter, redisDsn,
redisPrefix, connectTimeout and defaultTtl from that result before constructing
CacheSettings. Modify resolve() to call a single multi-key fetch (e.g.
databaseConfigurationStore->fetchValues([...]) or a single
fetchAll()/getAllConfig() method or accept an injected already-loaded config
array), then apply the same trimming/defaulting logic to the keys and return new
CacheSettings(...); ensure the change is reflected where CacheFactory::create()
uses CacheSettings so no extra DB round-trips occur during cache creation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ffd8288-db4f-4740-9e63-7ecf4822f658
📒 Files selected for processing (9)
.gitignorephpmyfaq/admin/assets/src/api/configuration.tsphpmyfaq/admin/assets/src/configuration/configuration.tsphpmyfaq/src/phpMyFAQ/Administration/Helper.phpphpmyfaq/src/phpMyFAQ/Cache/CacheSettingsResolver.phpphpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ConfigurationTabController.phpphpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.phpphpmyfaq/src/services.phptests/phpMyFAQ/Cache/CacheFactoryTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- phpmyfaq/src/services.php
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/phpMyFAQ/Controller/Api/PdfControllerTest.php (1)
178-179: Test assertions loosened from exact values to type checks.Replacing
-1with$this->isInt()and[-1]with$this->isArray()weakens these expectations. The original values likely have semantic meaning (e.g.,-1representing a guest/public user). The type-only constraints won't catch bugs where the wrong integer or array is passed.If the actual values can legitimately vary, this change is acceptable. Otherwise, consider preserving the exact value assertions to maintain test specificity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpMyFAQ/Controller/Api/PdfControllerTest.php` around lines 178 - 179, The test weakened expectations by replacing exact value assertions with type checks for setUser and setGroups; revert to asserting the specific expected values instead of using $this->isInt() and $this->isArray() so the test catches incorrect values—update the PdfControllerTest mocks that call setUser to expect the exact sentinel (e.g., -1) and setGroups to expect the exact array (e.g., [-1]) using PHPUnit equality/identical matchers on the setUser and setGroups expectations.tests/phpMyFAQ/EventListener/ApiRateLimiterListenerTest.php (1)
81-107: Consider adding assertion forX-RateLimit-Resetheader.The PR description states the HTTP header format includes
X-RateLimit-Reset, but this header is not verified in the test. Adding an assertion would ensure complete coverage of all rate limit response headers.🧪 Suggested addition
$this->assertSame('0', $response->headers->get('X-RateLimit-Remaining')); + $this->assertNotNull($response->headers->get('X-RateLimit-Reset')); $this->assertNotNull($response->headers->get('Retry-After'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/phpMyFAQ/EventListener/ApiRateLimiterListenerTest.php` around lines 81 - 107, The test testOnKernelRequestReturnsTooManyRequestsResponseWhenLimitIsExceeded is missing an assertion for the X-RateLimit-Reset header described in the PR; update the ApiRateLimiterListener test to assert that $response->headers->has('X-RateLimit-Reset') and that its value is present and sensible (e.g. a numeric timestamp or seconds until reset — assertIsNumeric or assertGreaterThanOrEqual(0) against (int) $response->headers->get('X-RateLimit-Reset')) so the rate-limit response headers are fully verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpmyfaq/translations/language_de.php`:
- Line 1689: Replace the mixed English UI strings in language_de.php with proper
German translations: locate the entries containing "If disabled, phpMyFAQ uses
the database only" and the other English phrases at the nearby entries (the
exact English strings present at the commented lines) and replace them with
consistent German text (for example: "Wenn deaktiviert, verwendet phpMyFAQ nur
die Datenbank"). Ensure each translated string preserves the original meaning
and punctuation and update all occurrences listed (the entries matching the
shown English phrases near lines 1689, 1716, 1720-1721, 1725-1726, 1730-1731).
---
Nitpick comments:
In `@tests/phpMyFAQ/Controller/Api/PdfControllerTest.php`:
- Around line 178-179: The test weakened expectations by replacing exact value
assertions with type checks for setUser and setGroups; revert to asserting the
specific expected values instead of using $this->isInt() and $this->isArray() so
the test catches incorrect values—update the PdfControllerTest mocks that call
setUser to expect the exact sentinel (e.g., -1) and setGroups to expect the
exact array (e.g., [-1]) using PHPUnit equality/identical matchers on the
setUser and setGroups expectations.
In `@tests/phpMyFAQ/EventListener/ApiRateLimiterListenerTest.php`:
- Around line 81-107: The test
testOnKernelRequestReturnsTooManyRequestsResponseWhenLimitIsExceeded is missing
an assertion for the X-RateLimit-Reset header described in the PR; update the
ApiRateLimiterListener test to assert that
$response->headers->has('X-RateLimit-Reset') and that its value is present and
sensible (e.g. a numeric timestamp or seconds until reset — assertIsNumeric or
assertGreaterThanOrEqual(0) against (int)
$response->headers->get('X-RateLimit-Reset')) so the rate-limit response headers
are fully verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4df7d35c-284a-4022-bacc-9d80166a6071
📒 Files selected for processing (24)
phpmyfaq/src/phpMyFAQ/Cache/CacheSettingsResolver.phpphpmyfaq/src/phpMyFAQ/Configuration/Storage/DatabaseConfigurationStore.phpphpmyfaq/src/phpMyFAQ/EventListener/ApiRateLimiterListener.phpphpmyfaq/src/phpMyFAQ/Kernel.phpphpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.phpphpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.phpphpmyfaq/translations/language_de.phptests/phpMyFAQ/Cache/CacheFactoryTest.phptests/phpMyFAQ/Cache/CacheSettingsResolverTest.phptests/phpMyFAQ/Configuration/DatabaseConfigurationStoreTest.phptests/phpMyFAQ/Controller/AbstractControllerTest.phptests/phpMyFAQ/Controller/Api/PaginatedResponseOptionsTest.phptests/phpMyFAQ/Controller/Api/PdfControllerTest.phptests/phpMyFAQ/Controller/Frontend/AttachmentControllerTest.phptests/phpMyFAQ/Controller/Frontend/AuthenticationControllerTest.phptests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.phptests/phpMyFAQ/Controller/Frontend/ContactControllerTest.phptests/phpMyFAQ/Controller/Frontend/CustomPageControllerTest.phptests/phpMyFAQ/Controller/Frontend/FaqControllerTest.phptests/phpMyFAQ/Controller/Frontend/PdfControllerTest.phptests/phpMyFAQ/EventListener/ApiRateLimiterListenerTest.phptests/phpMyFAQ/Functional/WebTestCase.phptests/phpMyFAQ/KernelTest.phptests/phpMyFAQ/Setup/Installation/DefaultDataSeederTest.php
✅ Files skipped from review due to trivial changes (3)
- tests/phpMyFAQ/Controller/AbstractControllerTest.php
- tests/phpMyFAQ/Controller/Frontend/AttachmentControllerTest.php
- tests/phpMyFAQ/Controller/Frontend/PdfControllerTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/phpMyFAQ/Cache/CacheFactoryTest.php
- phpmyfaq/src/phpMyFAQ/Cache/CacheSettingsResolver.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaced the custom SQL-based fixed-window rate limiter with symfony/rate-limiter's FixedWindowLimiter. This eliminates the faqrate_limits database table entirely — rate limit state is now held in memory (swappable to a cache adapter for persistence).
The public API (check/headers/getHeaders) and HTTP header format (X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, Retry-After) remain unchanged.
Summary by CodeRabbit