-
Notifications
You must be signed in to change notification settings - Fork 138
fix(flashcard): clone Date object in getFlashcardStats to prevent side-effect mutation (#1196) #1526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(flashcard): clone Date object in getFlashcardStats to prevent side-effect mutation (#1196) #1526
Changes from all commits
786faff
1a8f2fa
df78a99
2ba135e
dd9c627
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,11 +4,15 @@ const JobCache = require("../models/JobCache"); | |
|
|
||
| const ADZUNA_APP_ID = process.env.ADZUNA_APP_ID; | ||
| const ADZUNA_API_KEY = process.env.ADZUNA_API_KEY; | ||
| const ADZUNA_COUNTRY = process.env.ADZUNA_COUNTRY || "in"; | ||
| const ADZUNA_COUNTRY = (process.env.ADZUNA_COUNTRY || "in").toLowerCase(); | ||
| const CACHE_TTL_MS = 24 * 60 * 60 * 1000; | ||
|
|
||
| // The Jobs feature is optional: without Adzuna credentials it stays dormant | ||
| // instead of crashing the server or spamming failed API calls. | ||
| // Supported Adzuna country codes | ||
| const SUPPORTED_COUNTRIES = new Set([ | ||
| "gb", "us", "in", "ca", "au", "de", "fr", "br", "za", | ||
| "at", "be", "ch", "es", "it", "nl", "nz", "pl", "ru", "sg" | ||
| ]); | ||
|
|
||
| const isAdzunaConfigured = () => Boolean(ADZUNA_APP_ID && ADZUNA_API_KEY); | ||
|
|
||
| // Bound the set of cache keys: role/country are client-controlled, so we trim, | ||
|
|
@@ -32,25 +36,49 @@ async function fetchFromAdzuna(role, country = ADZUNA_COUNTRY) { | |
| const url = `https://api.adzuna.com/v1/api/jobs/${country}/search/1`; | ||
| const { data } = await axios.get(url, { | ||
| params: { | ||
| app_id: ADZUNA_APP_ID, | ||
| app_key: ADZUNA_API_KEY, | ||
| what: role, | ||
| app_id: ADZUNA_APP_ID, | ||
| app_key: ADZUNA_API_KEY, | ||
| what: role, | ||
| results_per_page: 10, | ||
| }, | ||
| }); | ||
| return (data.results || []).map((j) => ({ | ||
| id: j.id, | ||
| title: j.title, | ||
| company: j.company?.display_name || "Unknown", | ||
| location: j.location?.display_name || "Remote", | ||
| salary_min: j.salary_min || null, | ||
| salary_max: j.salary_max ?? null, | ||
| description: j.description ?? "", | ||
| id: j.id, | ||
| title: j.title, | ||
| company: j.company?.display_name || "Unknown", | ||
| location: j.location?.display_name || "Remote", | ||
| salary_min: j.salary_min || null, | ||
| salary_max: j.salary_max ?? null, | ||
| description: j.description ?? "", | ||
| redirect_url: j.redirect_url, | ||
| created: j.created, | ||
| created: j.created, | ||
| })); | ||
| } | ||
|
|
||
| /** | ||
| * Handles concurrent upserts safely by catching E11000 duplicate key errors | ||
| * and falling back to a standard update. | ||
| */ | ||
| async function upsertJobCache(cacheKey, jobs) { | ||
| const updateData = { jobs, fetchedAt: new Date() }; | ||
| try { | ||
| return await JobCache.findOneAndUpdate( | ||
| { cacheKey }, | ||
| updateData, | ||
| { upsert: true, new: true } | ||
| ); | ||
| } catch (err) { | ||
| if (err.code === 11000) { | ||
| return await JobCache.findOneAndUpdate( | ||
| { cacheKey }, | ||
| updateData, | ||
| { new: true } | ||
| ); | ||
| } | ||
| throw err; | ||
| } | ||
| } | ||
|
|
||
| exports.getJobs = async (req, res) => { | ||
| try { | ||
| if (!isAdzunaConfigured()) { | ||
|
|
@@ -63,15 +91,22 @@ exports.getJobs = async (req, res) => { | |
| }); | ||
| } | ||
|
|
||
| // Validate country parameter if explicitly provided | ||
| let country = req.query.country ? String(req.query.country).toLowerCase() : ADZUNA_COUNTRY; | ||
| if (!SUPPORTED_COUNTRIES.has(country)) { | ||
| return res.status(400).json({ | ||
| message: `Invalid or unsupported country code: "${req.query.country}". Supported codes are: ${Array.from(SUPPORTED_COUNTRIES).join(", ")}`, | ||
| }); | ||
| } | ||
|
Comment on lines
+94
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π‘ Minor | β‘ Quick win Update the invalid-country unit test.
Change the test to assert the HTTP 400 response and that no cache write occurs. π€ Prompt for AI Agents |
||
|
|
||
| const userId = req.user._id; | ||
|
|
||
| const latestSession = await Session.findOne({ user: userId }) | ||
| .sort({ createdAt: -1 }) | ||
| .select("role"); | ||
|
|
||
| const role = normalizeRole(req.query.role) || normalizeRole(latestSession?.role) || "software engineer"; | ||
| const country = normalizeCountry(req.query.country); | ||
| const cacheKey = `${role}|${country}`; | ||
| const role = req.query.role || latestSession?.role || "software engineer"; | ||
| const cacheKey = `${role.toLowerCase()}|${country}`; | ||
|
Comment on lines
+108
to
+109
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ποΈ Data Integrity & Integration | π Major | β‘ Quick win Restore role normalization in both cache-write paths. Both paths bypass
π Affects 1 file
π€ Prompt for AI Agents |
||
|
|
||
| const cached = await JobCache.findOne({ cacheKey }); | ||
| if (cached && Date.now() - cached.fetchedAt.getTime() < CACHE_TTL_MS) { | ||
|
|
@@ -80,11 +115,7 @@ exports.getJobs = async (req, res) => { | |
|
|
||
| const jobs = await fetchFromAdzuna(role, country); | ||
|
|
||
| await JobCache.findOneAndUpdate( | ||
| { cacheKey }, | ||
| { jobs, fetchedAt: new Date() }, | ||
| { upsert: true, new: true } | ||
| ); | ||
| await upsertJobCache(cacheKey, jobs); | ||
|
|
||
| return res.json({ jobs, role, source: "api" }); | ||
| } catch (err) { | ||
|
|
@@ -95,24 +126,25 @@ exports.getJobs = async (req, res) => { | |
|
|
||
| exports.refreshJobCache = async () => { | ||
| if (!isAdzunaConfigured()) return; | ||
|
|
||
| const targetCountry = SUPPORTED_COUNTRIES.has(ADZUNA_COUNTRY) ? ADZUNA_COUNTRY : "in"; | ||
|
|
||
| try { | ||
| const roles = await Session.distinct("role", { | ||
| createdAt: { $gte: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000) }, | ||
| }); | ||
|
|
||
| for (const role of roles) { | ||
| const normalizedRole = normalizeRole(role); | ||
| if (!normalizedRole) continue; | ||
| const cacheKey = `${normalizedRole}|${ADZUNA_COUNTRY}`; | ||
| const jobs = await fetchFromAdzuna(normalizedRole); | ||
| await JobCache.findOneAndUpdate( | ||
| { cacheKey }, | ||
| { jobs, fetchedAt: new Date() }, | ||
| { upsert: true, new: true } | ||
| ); | ||
|
|
||
| try { | ||
| const cacheKey = `${role.toLowerCase()}|${targetCountry}`; | ||
| const jobs = await fetchFromAdzuna(role, targetCountry); | ||
| await upsertJobCache(cacheKey, jobs); | ||
| console.log(`[JobCron] Refreshed cache for: ${role}`); | ||
| } catch (roleErr) { | ||
| console.error(`[JobCron] Failed to refresh role "${role}":`, roleErr.message); | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error("[JobCron] Refresh failed:", err.message); | ||
| } | ||
| }; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π©Ί Stability & Availability | π Major | β‘ Quick win
Use one validated effective default country.
If
ADZUNA_COUNTRYis invalid or contains whitespace,getJobsreturns HTTP 400 even when the client does not sendcountry.refreshJobCacheinstead falls back to"in", so the API and refresh paths use different configuration behavior.Trim and validate the configured value at initialization. Use the same validated fallback in both paths.
π€ Prompt for AI Agents