Mobian RTD Provider: use URL based cache#14481
Mobian RTD Provider: use URL based cache#14481RomuloVitoi wants to merge 3 commits intoprebid:masterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11edab9cd8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| function makeMemoizedFetch() { | ||
| let cachedResponse = null; | ||
| const cache = new Map(); |
There was a problem hiding this comment.
The new per-URL memoization uses a process-lifetime Map with no eviction, so long-lived SPA sessions that continuously change window.location.href will accumulate entries indefinitely and retain all prior context payloads in memory. This is a regression from the previous single-entry cache footprint and can degrade performance over time in production; consider limiting the cache (e.g., last URL only, TTL, or LRU max size).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the Mobian RTD Provider to implement URL-based caching instead of a single global cache. This change addresses the need to handle single-page applications (SPAs) where the URL and page content change without a full page reload, requiring fresh contextual assessments for each URL.
Changes:
- Replaces single cached response with a Map-based cache keyed by page URL
- Each unique URL now gets its own cached assessment response
- Minor whitespace formatting change in error handler
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function makeMemoizedFetch() { | ||
| let cachedResponse = null; | ||
| const cache = new Map(); | ||
| return async function () { | ||
| if (cachedResponse) { | ||
| return Promise.resolve(cachedResponse); | ||
| const pageUrl = window.location.href; | ||
| if (cache.has(pageUrl)) { | ||
| return Promise.resolve(cache.get(pageUrl)); | ||
| } | ||
| try { | ||
| const response = await fetchContextData(); | ||
| cachedResponse = makeDataFromResponse(response); | ||
| const cachedResponse = makeDataFromResponse(response); | ||
| cache.set(pageUrl, cachedResponse); | ||
| return cachedResponse; |
There was a problem hiding this comment.
The URL-based caching behavior should be covered by tests. Consider adding test cases that verify: 1) different URLs result in different cache entries, 2) subsequent calls with the same URL return cached data without making additional API calls, and 3) URL changes trigger new API calls. This will ensure the SPA behavior works as intended.
|
|
||
| function makeMemoizedFetch() { | ||
| let cachedResponse = null; | ||
| const cache = new Map(); |
There was a problem hiding this comment.
The URL-based cache implemented as an unbounded Map can lead to a memory leak in single-page applications. As the user navigates to different URLs, the cache will grow indefinitely without any cleanup mechanism or size limits. Consider implementing a cache size limit (e.g., LRU cache) or a time-based expiration strategy to prevent unbounded memory growth.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@RomuloVitoi please resolve the bot feedback |
2 participants
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
The Mobian RTD Provider currently caches the response of the assessment and reuse it for every bid on the page, this does not work well for single page applications where the URL and the page content is updated. I'm updating the cache to be a Map keyed of the page URL so we can have a new assessment response in case the page URL changes.
Other information