diff --git a/lib/FredyPipelineExecutioner.js b/lib/FredyPipelineExecutioner.js index 134fd5d..244759c 100755 --- a/lib/FredyPipelineExecutioner.js +++ b/lib/FredyPipelineExecutioner.js @@ -99,8 +99,8 @@ class FredyPipelineExecutioner { /** * Optionally, enrich new listings with data from their detail pages. * Only called when the provider config defines a `fetchDetails` function. - * Runs all fetches in parallel. Each fetch must handle its own errors - * and always resolve (never reject) to avoid aborting other listings. + * Fetches are performed sequentially to avoid overloading the provider or + * the shared browser instance. * * @param {Listing[]} newListings New listings to enrich. * @returns {Promise} Resolves with enriched listings. @@ -223,24 +223,15 @@ class FredyPipelineExecutioner { * @param {string} url The provider URL to fetch from. * @returns {Promise} Resolves with an array of listings (empty when none found). */ - _getListings(url) { + async _getListings(url) { const extractor = new Extractor({ ...this._providerConfig.puppeteerOptions, browser: this._browser }); - return new Promise((resolve, reject) => { - extractor - .execute(url, this._providerConfig.waitForSelector, this._providerId) - .then(() => { - const listings = extractor.parseResponseText( - this._providerConfig.crawlContainer, - this._providerConfig.crawlFields, - url, - ); - resolve(listings == null ? [] : listings); - }) - .catch((err) => { - reject(err); - logger.error(err); - }); - }); + await extractor.execute(url, this._providerConfig.waitForSelector, this._providerId); + const listings = extractor.parseResponseText( + this._providerConfig.crawlContainer, + this._providerConfig.crawlFields, + url, + ); + return listings == null ? [] : listings; } /** diff --git a/lib/api/routes/jobRouter.js b/lib/api/routes/jobRouter.js index 8ea68f8..c6b6f4d 100644 --- a/lib/api/routes/jobRouter.js +++ b/lib/api/routes/jobRouter.js @@ -195,6 +195,9 @@ export default async function jobPlugin(fastify) { const settings = await getSettings(); try { const job = jobStorage.getJob(jobId); + if (!job) { + return reply.code(404).send({ error: 'Job not found' }); + } if (settings.demoMode && !isAdmin(request) && job.name === DEMO_JOB_NAME) { return reply.code(403).send({ error: 'Sorry, but you cannot remove the Demo Job ;)' }); } @@ -216,6 +219,9 @@ export default async function jobPlugin(fastify) { const settings = await getSettings(); try { const job = jobStorage.getJob(jobId); + if (!job) { + return reply.code(404).send({ error: 'Job not found' }); + } if (settings.demoMode && !isAdmin(request) && job.name === DEMO_JOB_NAME) { return reply.code(403).send({ error: 'Sorry, but you cannot change the Status of our Demo Job ;)' }); diff --git a/lib/api/routes/listingsRouter.js b/lib/api/routes/listingsRouter.js index cd956a8..7328e53 100644 --- a/lib/api/routes/listingsRouter.js +++ b/lib/api/routes/listingsRouter.js @@ -8,7 +8,7 @@ import * as watchListStorage from '../../services/storage/watchListStorage.js'; import { isAdmin as isAdminFn } from '../security.js'; import logger from '../../services/logger.js'; import { nullOrEmpty } from '../../utils.js'; -import { getJobs } from '../../services/storage/jobStorage.js'; +import { getJob } from '../../services/storage/jobStorage.js'; import { getSettings } from '../../services/storage/settingsStorage.js'; import { trackPoi } from '../../services/tracking/Tracker.js'; import { TRACKING_POIS } from '../../TRACKING_POIS.js'; @@ -46,9 +46,8 @@ export default async function listingsPlugin(fastify) { let jobFilter = null; let jobIdFilter = null; - const jobs = getJobs(); if (!nullOrEmpty(jobNameFilter)) { - const job = jobs.find((j) => j.id === jobNameFilter); + const job = getJob(jobNameFilter); jobFilter = job != null ? job.name : null; jobIdFilter = job != null ? job.id : null; } @@ -159,6 +158,16 @@ export default async function listingsPlugin(fastify) { if (settings.demoMode && !isAdminFn(request)) { return reply.code(403).send({ error: 'Sorry, but you cannot remove listings in demo mode ;)' }); } + const job = getJob(jobId); + if (!job) { + return reply.code(404).send({ error: 'Job not found' }); + } + const userId = request.session.currentUser; + if (!isAdminFn(request) && job.userId !== userId && !job.shared_with_user.includes(userId)) { + return reply + .code(403) + .send({ error: 'You are trying to remove listings for a job that is not associated to your user' }); + } listingStorage.deleteListingsByJobId(jobId, hardDelete); } catch (error) { logger.error(error); @@ -169,7 +178,11 @@ export default async function listingsPlugin(fastify) { fastify.delete('/', async (request, reply) => { const { ids, hardDelete = false } = request.body; + const settings = await getSettings(); try { + if (settings.demoMode && !isAdminFn(request)) { + return reply.code(403).send({ error: 'Sorry, but you cannot remove listings in demo mode ;)' }); + } if (Array.isArray(ids) && ids.length > 0) { listingStorage.deleteListingsById(ids, hardDelete); } diff --git a/lib/api/routes/userSettingsRoute.js b/lib/api/routes/userSettingsRoute.js index 3e52073..c97febd 100644 --- a/lib/api/routes/userSettingsRoute.js +++ b/lib/api/routes/userSettingsRoute.js @@ -3,13 +3,11 @@ * Licensed under Apache-2.0 with Commons Clause and Attribution/Naming Clause */ -import SqliteConnection from '../../services/storage/SqliteConnection.js'; -import { getSettings, upsertSettings } from '../../services/storage/settingsStorage.js'; +import { getSettings, getUserSettings, upsertSettings } from '../../services/storage/settingsStorage.js'; import { isAdmin } from '../security.js'; import { resetGeocoordinatesAndDistanceForUser } from '../../services/storage/listingsStorage.js'; import { geocodeAddress } from '../../services/geocoding/geoCodingService.js'; import { autocompleteAddress } from '../../services/geocoding/autocompleteService.js'; -import { fromJson } from '../../utils.js'; import { trackPoi } from '../../services/tracking/Tracker.js'; import { TRACKING_POIS } from '../../TRACKING_POIS.js'; import logger from '../../services/logger.js'; @@ -21,12 +19,7 @@ import { runGeoCordTask } from '../../services/crons/geocoding-cron.js'; export default async function userSettingsPlugin(fastify) { fastify.get('/', async (request) => { const userId = request.session.currentUser; - const rows = SqliteConnection.query('SELECT name, value FROM settings WHERE user_id = @userId', { userId }); - const settings = {}; - for (const r of rows) { - settings[r.name] = fromJson(r.value, null); - } - return settings; + return getUserSettings(userId); }); fastify.get('/autocomplete', async (request, reply) => { diff --git a/lib/notification/notify.js b/lib/notification/notify.js index f9368f7..32d9f7d 100755 --- a/lib/notification/notify.js +++ b/lib/notification/notify.js @@ -23,7 +23,7 @@ const findAdapter = (notificationAdapter) => { export const send = (serviceName, newListings, notificationConfig, jobKey, baseUrl) => { //this is not being used in tests, therefore adapter are always set return notificationConfig - .filter((notificationAdapter) => findAdapter(notificationAdapter) != null) .map((notificationAdapter) => findAdapter(notificationAdapter)) + .filter(Boolean) .map((a) => a.send({ serviceName, newListings, notificationConfig, jobKey, baseUrl })); }; diff --git a/lib/services/storage/listingsStorage.js b/lib/services/storage/listingsStorage.js index 86a2801..3cbe2fd 100755 --- a/lib/services/storage/listingsStorage.js +++ b/lib/services/storage/listingsStorage.js @@ -308,13 +308,15 @@ export const queryListings = ({ } if (freeTextFilter && String(freeTextFilter).trim().length > 0) { params.filter = `%${String(freeTextFilter).trim()}%`; - whereParts.push(`(title LIKE @filter OR address LIKE @filter OR provider LIKE @filter OR link LIKE @filter)`); + whereParts.push( + `(l.title LIKE @filter OR l.address LIKE @filter OR l.provider LIKE @filter OR l.link LIKE @filter)`, + ); } // activityFilter: when true -> only active listings (is_active = 1), false -> only inactive if (activityFilter === true) { - whereParts.push('(is_active = 1)'); + whereParts.push('(l.is_active = 1)'); } else if (activityFilter === false) { - whereParts.push('(is_active = 0)'); + whereParts.push('(l.is_active = 0)'); } // Prefer filtering by job id when provided (unambiguous and robust) if (jobIdFilter && String(jobIdFilter).trim().length > 0) { @@ -328,7 +330,7 @@ export const queryListings = ({ // providerFilter: when provided as string (assumed provider name), filter listings where provider equals that name (exact match) if (providerFilter && String(providerFilter).trim().length > 0) { params.providerName = String(providerFilter).trim(); - whereParts.push('(provider = @providerName)'); + whereParts.push('(l.provider = @providerName)'); } // watchListFilter: when true -> only watched listings, false -> only unwatched if (watchListFilter === true) { @@ -351,11 +353,11 @@ export const queryListings = ({ // Time range filters (unix timestamps in milliseconds) if (Number.isFinite(createdAfter) && createdAfter > 0) { params.createdAfter = createdAfter; - whereParts.push('(created_at >= @createdAfter)'); + whereParts.push('(l.created_at >= @createdAfter)'); } if (Number.isFinite(createdBefore) && createdBefore > 0) { params.createdBefore = createdBefore; - whereParts.push('(created_at <= @createdBefore)'); + whereParts.push('(l.created_at <= @createdBefore)'); } // Price range filters if (Number.isFinite(minPrice) && minPrice >= 0) { @@ -370,32 +372,22 @@ export const queryListings = ({ // Build whereSql (filtering by manually_deleted = 0) whereParts.push('(l.manually_deleted = 0)'); - const whereSql = whereParts.length ? `WHERE ${whereParts.join(' AND ')}` : ''; - const whereSqlWithAlias = whereSql - .replace(/\btitle\b/g, 'l.title') - .replace(/\bdescription\b/g, 'l.description') - .replace(/\baddress\b/g, 'l.address') - .replace(/\bprovider\b/g, 'l.provider') - .replace(/\blink\b/g, 'l.link') - .replace(/\bis_active\b/g, 'l.is_active') - .replace(/\bj\.user_id\b/g, 'j.user_id') - .replace(/\bj\.name\b/g, 'j.name') - .replace(/\bwl\.id\b/g, 'wl.id'); + const whereSqlWithAlias = whereParts.length ? `WHERE ${whereParts.join(' AND ')}` : ''; - // whitelist sortable fields to avoid SQL injection - const sortable = new Set(['created_at', 'price', 'size', 'provider', 'title', 'job_name', 'is_active', 'isWatched']); - const safeSortField = sortField && sortable.has(sortField) ? sortField : null; + // whitelist sortable fields to avoid SQL injection; map to fully-qualified expressions + const sortableMap = { + created_at: 'l.created_at', + price: 'l.price', + size: 'l.size', + provider: 'l.provider', + title: 'l.title', + job_name: 'j.name', + is_active: 'l.is_active', + isWatched: 'CASE WHEN wl.id IS NOT NULL THEN 1 ELSE 0 END', + }; + const safeSortExpr = sortField && sortableMap[sortField] ? sortableMap[sortField] : null; const safeSortDir = String(sortDir).toLowerCase() === 'desc' ? 'DESC' : 'ASC'; - const orderSql = safeSortField ? `ORDER BY ${safeSortField} ${safeSortDir}` : 'ORDER BY created_at DESC'; - const orderSqlWithAlias = orderSql - .replace(/\bcreated_at\b/g, 'l.created_at') - .replace(/\bprice\b/g, 'l.price') - .replace(/\bsize\b/g, 'l.size') - .replace(/\bprovider\b/g, 'l.provider') - .replace(/\btitle\b/g, 'l.title') - .replace(/\bjob_name\b/g, 'j.name') - // Sort by computed watch flag when requested - .replace(/\bisWatched\b/g, 'CASE WHEN wl.id IS NOT NULL THEN 1 ELSE 0 END'); + const orderSqlWithAlias = safeSortExpr ? `ORDER BY ${safeSortExpr} ${safeSortDir}` : 'ORDER BY l.created_at DESC'; // count total with same WHERE const countRow = SqliteConnection.query( diff --git a/lib/services/storage/settingsStorage.js b/lib/services/storage/settingsStorage.js index a75e8a3..1846dcd 100644 --- a/lib/services/storage/settingsStorage.js +++ b/lib/services/storage/settingsStorage.js @@ -123,8 +123,11 @@ export function upsertSettings(settingsMapOrEntry, userId = null) { ); } } - // keep cache in sync (only for global settings) + // Invalidate cache synchronously so the next getSettings() call rebuilds it. + // refreshSettingsCache() is async (reads config.json), so we cannot await it + // here without making upsertSettings async everywhere. Nulling is safe because + // getSettings() will call refreshSettingsCache() on the next invocation. if (userId == null) { - refreshSettingsCache(); + cachedSettingsConfig = null; } }