diff --git a/migrations/006: Proper rent index.sql b/migrations/006: Proper rent index.sql index 9ba2732..f1123a1 100644 --- a/migrations/006: Proper rent index.sql +++ b/migrations/006: Proper rent index.sql @@ -7,6 +7,12 @@ -- rent_index: materialised p25/p50/p75 per (city, locality, room_type). -- Refreshed nightly by cron/rentIndexRefresh.js. -- Listings JOIN this table to expose rentDeviation in API responses. +-- +-- Fix (originally migration 011): The trigger now also fires when expires_at +-- changes so that renewals (status stays 'active' but expires_at advances) +-- correctly record a 'listing_renewed' observation. The UPDATE condition was +-- broadened to: NEW.status = 'active' AND (OLD.status <> 'active' OR +-- NEW.expires_at IS DISTINCT FROM OLD.expires_at). CREATE TABLE IF NOT EXISTS rent_observations ( observation_id UUID PRIMARY KEY DEFAULT gen_random_uuid (), @@ -56,8 +62,13 @@ CREATE TABLE IF NOT EXISTS rent_index ( CREATE INDEX IF NOT EXISTS idx_rent_index_lookup ON rent_index (city, locality, room_type); -- Trigger function: fires after INSERT on listings (new listing goes active) --- and after UPDATE when status flips to 'active' (listing renewed / reactivated). --- Runs inside the same transaction as the listing write — observation is never lost. +-- and after UPDATE when status flips to 'active' OR expires_at changes while +-- already active (i.e. a renewal). Runs inside the same transaction as the +-- listing write — observation is never lost. +-- +-- Fix vs original: the UPDATE branch previously only fired when OLD.status <> +-- 'active', which meant renewals (status unchanged, only expires_at advancing) +-- were silently dropped. Now we also fire when expires_at changes. CREATE OR REPLACE FUNCTION capture_rent_observation() RETURNS TRIGGER AS $$ BEGIN @@ -75,8 +86,11 @@ BEGIN ELSIF TG_OP = 'UPDATE' AND NEW.status = 'active' - AND OLD.status <> 'active' AND NEW.deleted_at IS NULL + AND ( + OLD.status <> 'active' + OR NEW.expires_at IS DISTINCT FROM OLD.expires_at + ) THEN INSERT INTO rent_observations (listing_id, city, locality, room_type, rent_per_month, source) @@ -94,8 +108,10 @@ BEGIN END; $$ LANGUAGE plpgsql; +-- Watch both status and expires_at so renewals (expires_at advances while +-- status stays 'active') also fire the trigger. DROP TRIGGER IF EXISTS trg_capture_rent_observation ON listings; CREATE TRIGGER trg_capture_rent_observation - AFTER INSERT OR UPDATE OF status ON listings + AFTER INSERT OR UPDATE OF status, expires_at ON listings FOR EACH ROW EXECUTE FUNCTION capture_rent_observation(); \ No newline at end of file diff --git a/migrations/007_fix_roommate_constraints.sql b/migrations/007_fix_roommate_constraints.sql index a7485e7..b7d63eb 100644 --- a/migrations/007_fix_roommate_constraints.sql +++ b/migrations/007_fix_roommate_constraints.sql @@ -1,41 +1,72 @@ -- Active: 1777192065763@@127.0.0.1@5432@roomies_db --- Migration 007: Fix roommate_blocks FK cascade + redundant index + add looking timestamp check +-- Migration 007: Fix roommate_blocks FK cascade + redundant index + CHECK constraint -- -- Fixes applied from migration 005: -- 1. Add CHECK constraint: looking_for_roommate = TRUE requires looking_updated_at IS NOT NULL -- 2. Change roommate_blocks FKs from ON DELETE RESTRICT to ON DELETE CASCADE -- 3. Drop redundant idx_roommate_blocks_blocker (covered by PK on blocker_id, blocked_id) +-- +-- Revised approach (safe for environments that already have data in roommate_blocks): +-- Instead of DROP TABLE + recreate (which destroys all block rows), we use +-- ALTER TABLE to swap FK actions and add the CHECK constraint in-place. +-- A fresh environment that has no roommate_blocks table yet gets a CREATE TABLE. --- Fix 1: Add CHECK constraint on student_profiles --- Only safe to add if it won't violate existing data. --- First backfill: any row already set to TRUE with NULL timestamp gets NOW(). +-- Fix 1: Backfill timestamp — required before adding the CHECK constraint. UPDATE student_profiles -SET - looking_updated_at = NOW() -WHERE - looking_for_roommate = TRUE - AND looking_updated_at IS NULL; +SET looking_updated_at = NOW() +WHERE looking_for_roommate = TRUE + AND looking_updated_at IS NULL; +-- Add CHECK constraint (safe now that all TRUE rows have a timestamp). ALTER TABLE student_profiles ADD CONSTRAINT chk_looking_has_timestamp CHECK ( looking_for_roommate = FALSE OR looking_updated_at IS NOT NULL ); --- Fix 2 + 3: Recreate roommate_blocks with CASCADE and without redundant index --- We must drop and recreate because ALTER CONSTRAINT is not supported for FK ON DELETE in PG. +-- Fix 2 + 3: Create roommate_blocks with CASCADE if it doesn't exist yet +-- (fresh environment), otherwise alter FKs in-place to preserve existing data. +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM information_schema.tables + WHERE table_name = 'roommate_blocks' + ) THEN + -- Fresh environment: create with correct constraints from the start. + CREATE TABLE roommate_blocks ( + blocker_id UUID NOT NULL REFERENCES users (user_id) ON DELETE CASCADE, + blocked_id UUID NOT NULL REFERENCES users (user_id) ON DELETE CASCADE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + PRIMARY KEY (blocker_id, blocked_id), + CONSTRAINT chk_no_self_block CHECK (blocker_id <> blocked_id) + ); + ELSE + -- Existing environment: swap RESTRICT FKs to CASCADE without touching data. + ALTER TABLE roommate_blocks + DROP CONSTRAINT IF EXISTS roommate_blocks_blocker_id_fkey, + DROP CONSTRAINT IF EXISTS roommate_blocks_blocked_id_fkey; --- Drop old table (no data worth preserving in prod at this stage — blocks are --- user-generated UX state, not business-critical records). -DROP TABLE IF EXISTS roommate_blocks; + ALTER TABLE roommate_blocks + ADD CONSTRAINT roommate_blocks_blocker_id_fkey + FOREIGN KEY (blocker_id) REFERENCES users (user_id) ON DELETE CASCADE, + ADD CONSTRAINT roommate_blocks_blocked_id_fkey + FOREIGN KEY (blocked_id) REFERENCES users (user_id) ON DELETE CASCADE; -CREATE TABLE roommate_blocks ( - blocker_id UUID NOT NULL REFERENCES users (user_id) ON DELETE CASCADE, - blocked_id UUID NOT NULL REFERENCES users (user_id) ON DELETE CASCADE, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - PRIMARY KEY (blocker_id, blocked_id), - CONSTRAINT chk_no_self_block CHECK (blocker_id <> blocked_id) -); + -- Add self-block check only if it doesn't already exist. + IF NOT EXISTS ( + SELECT 1 FROM information_schema.check_constraints + WHERE constraint_name = 'chk_no_self_block' + ) THEN + ALTER TABLE roommate_blocks + ADD CONSTRAINT chk_no_self_block CHECK (blocker_id <> blocked_id); + END IF; + END IF; +END; +$$; + +-- Fix 3: Drop redundant index (blocker_id is the leftmost column of the PK, +-- so the PK index already satisfies all blocker_id lookups). +DROP INDEX IF EXISTS idx_roommate_blocks_blocker; --- Only keep the blocked_id index (blocker_id is covered by the PK leftmost prefix) +-- Keep only the blocked_id index (needed for reverse-lookup: "who is blocking me"). CREATE INDEX IF NOT EXISTS idx_roommate_blocks_blocked ON roommate_blocks (blocked_id); \ No newline at end of file diff --git a/migrations/009_idx_listings_posted_by_status_city.sql b/migrations/009_idx_listings_posted_by_status_city.sql index eae8418..73195d6 100644 --- a/migrations/009_idx_listings_posted_by_status_city.sql +++ b/migrations/009_idx_listings_posted_by_status_city.sql @@ -7,7 +7,14 @@ -- -- The existing idx_listings_posted_by covers posted_by alone but not the -- full predicate. This index covers the common access pattern. +-- +-- Revised (originally migration 013): The original index keyed raw city but the +-- query uses LOWER(l.city) LIKE LOWER($n), so the planner could not use it. +-- The index is recreated as an expression index on LOWER(city) so the EXISTS +-- subquery is covered and the planner can use an index scan. + +DROP INDEX IF EXISTS idx_listings_posted_by_status_city; -CREATE INDEX IF NOT EXISTS idx_listings_posted_by_status_city ON listings (posted_by, status, city) -WHERE - deleted_at IS NULL; \ No newline at end of file +CREATE INDEX IF NOT EXISTS idx_listings_posted_by_status_city + ON listings (posted_by, status, LOWER(city)) + WHERE deleted_at IS NULL; \ No newline at end of file diff --git a/src/app.js b/src/app.js index 9842046..363de72 100644 --- a/src/app.js +++ b/src/app.js @@ -40,7 +40,8 @@ app.use( return callback(null, true); } - callback(new Error(`CORS: origin '${origin}' is not allowed`)); + logger.warn({ origin }, "CORS: origin not allowed"); + callback(null, false); }, credentials: true, diff --git a/src/controllers/auth.controller.js b/src/controllers/auth.controller.js index f290eb7..e192ea9 100644 --- a/src/controllers/auth.controller.js +++ b/src/controllers/auth.controller.js @@ -1,3 +1,15 @@ +// src/controllers/auth.controller.js +// +// Token transport strategy (mobile + web): +// - Cookies are always set (HttpOnly, Secure) — browser clients use these. +// - The JSON body includes tokens ONLY when the request signals it is a +// native mobile client, detected via the X-Client-Type: mobile header. +// Android / iOS clients must send this header to receive tokens in the body. +// - Browser clients (no header) get { authenticated: true } — no token leak. +// +// This keeps the API secure for browsers while remaining compatible with native +// apps that cannot access HttpOnly cookies. + import * as authService from "../services/auth.service.js"; import { parseTtlSeconds } from "../services/auth.service.js"; import { AppError } from "../middleware/errorHandler.js"; @@ -22,12 +34,30 @@ const clearAuthCookies = (res) => { }); }; +/** + * Returns true when the caller is a native mobile app. + * Mobile clients MUST send `X-Client-Type: mobile` to receive tokens in the body. + * Browsers never send this header, so they only get HttpOnly cookies. + */ +const isMobileClient = (req) => req.headers["x-client-type"]?.toLowerCase() === "mobile"; + +/** + * Build the JSON data payload for auth responses. + * - Mobile: full token pair so the client can store them in secure storage. + * - Browser: no tokens in body; cookies carry the session. + */ +const authResponseData = (req, tokens) => { + if (isMobileClient(req)) { + return tokens; // { accessToken, refreshToken, user, sid } + } + return { user: tokens.user }; // cookies-only transport for browsers +}; + export const register = async (req, res, next) => { try { const tokens = await authService.register(req.body); setAuthCookies(res, tokens.accessToken, tokens.refreshToken); - - res.status(201).json({ status: "success", data: tokens }); + res.status(201).json({ status: "success", data: authResponseData(req, tokens) }); } catch (err) { next(err); } @@ -37,8 +67,7 @@ export const login = async (req, res, next) => { try { const tokens = await authService.login(req.body); setAuthCookies(res, tokens.accessToken, tokens.refreshToken); - - res.json({ status: "success", data: tokens }); + res.json({ status: "success", data: authResponseData(req, tokens) }); } catch (err) { next(err); } @@ -81,10 +110,8 @@ export const refresh = async (req, res, next) => { } const tokens = await authService.refresh(incomingRefreshToken); - setAuthCookies(res, tokens.accessToken, tokens.refreshToken); - - res.json({ status: "success", data: tokens }); + res.json({ status: "success", data: authResponseData(req, tokens) }); } catch (err) { next(err); } @@ -149,8 +176,7 @@ export const googleCallback = async (req, res, next) => { try { const tokens = await authService.googleOAuth(req.body); setAuthCookies(res, tokens.accessToken, tokens.refreshToken); - - res.json({ status: "success", data: tokens }); + res.json({ status: "success", data: authResponseData(req, tokens) }); } catch (err) { next(err); } diff --git a/src/controllers/verification.controller.js b/src/controllers/verification.controller.js index 2eaa552..3a992b9 100644 --- a/src/controllers/verification.controller.js +++ b/src/controllers/verification.controller.js @@ -1,8 +1,14 @@ +// src/controllers/verification.controller.js + import * as verificationService from "../services/verification.service.js"; export const submitDocument = async (req, res, next) => { try { - const result = await verificationService.submitDocument(req.user.userId, req.params.userId, req.body); + const result = await verificationService.submitDocument( + req.user.userId, + req.user.userId, // targetUserId = self + req.body, + ); res.status(201).json({ status: "success", data: result }); } catch (err) { next(err); diff --git a/src/cron/savedSearchAlert.js b/src/cron/savedSearchAlert.js index 8db8bbe..be744d3 100644 --- a/src/cron/savedSearchAlert.js +++ b/src/cron/savedSearchAlert.js @@ -6,8 +6,14 @@ import { logger } from "../logger/index.js"; import { enqueueNotificationsBulk } from "../workers/notificationQueue.js"; const SCHEDULE = process.env.CRON_SAVED_SEARCH_ALERT ?? "0 8 * * *"; -const SEARCH_BATCH_SIZE = Number(process.env.SAVED_SEARCH_ALERT_BATCH_SIZE) || 500; -const NOTIFICATION_CHUNK_SIZE = Number(process.env.SAVED_SEARCH_ALERT_NOTIFICATION_CHUNK_SIZE) || 100; + +const toPositiveInt = (value, fallback) => { + const n = Number(value); + return Number.isInteger(n) && n > 0 ? n : fallback; +}; + +const SEARCH_BATCH_SIZE = toPositiveInt(process.env.SAVED_SEARCH_ALERT_BATCH_SIZE, 500); +const NOTIFICATION_CHUNK_SIZE = toPositiveInt(process.env.SAVED_SEARCH_ALERT_NOTIFICATION_CHUNK_SIZE, 100); const chunk = (items, size) => { const chunks = []; diff --git a/src/db/client.js b/src/db/client.js index 57a66da..7232659 100644 --- a/src/db/client.js +++ b/src/db/client.js @@ -4,7 +4,8 @@ import { logger } from "../logger/index.js"; const { Pool } = pg; -const DB_POOL_MAX = Number(process.env.DB_POOL_MAX) || 10; +const parsedPoolMax = Number.parseInt(process.env.DB_POOL_MAX ?? "", 10); +const DB_POOL_MAX = Number.isInteger(parsedPoolMax) && parsedPoolMax > 0 ? parsedPoolMax : 10; export const pool = new Pool({ connectionString: config.DATABASE_URL, diff --git a/src/db/utils/roommateCompatibility.js b/src/db/utils/roommateCompatibility.js index 913e8ac..3d52817 100644 --- a/src/db/utils/roommateCompatibility.js +++ b/src/db/utils/roommateCompatibility.js @@ -1,31 +1,7 @@ // src/db/utils/roommateCompatibility.js -// -// Jaccard similarity between two students' preference sets. -// -// A preference is the pair (preference_key, preference_value). -// Both fields must match for it to count as shared — having -// smoking=smoker vs smoking=non_smoker is a mismatch, not a partial hit. -// -// Score formula: -// jaccard = |A ∩ B| / |A ∪ B| -// where |A ∪ B| = |A| + |B| - |A ∩ B| -// -// Returned as an integer 0–100. -// Returns 0 when either user has no preferences (union = 0) — the caller -// should set compatibilityAvailable = false in that case. - import { pool } from "../client.js"; import { logger } from "../../logger/index.js"; -// scoreUsersForUser -// -// requestingUserId: UUID of the student whose feed is being built. -// candidateIds: Array of UUIDs of the candidate students to score. -// Already filtered for opt-in, blocks, and city before this call. -// -// Returns: { [userId]: score 0–100 } -// On DB failure: logs the error and returns {} so the feed still renders -// without compatibility scores rather than crashing the request. export const scoreUsersForUser = async (requestingUserId, candidateIds, client = pool) => { if (!candidateIds.length) return {}; @@ -47,13 +23,16 @@ export const scoreUsersForUser = async (requestingUserId, candidateIds, client = ); // Step 2 — individual preference counts for union computation. - // We fetch the requesting user alongside the candidates in one query. + // FIX: pass a single array $1 for ANY($1::uuid[]) — previously the code + // spread [requestingUserId, ...candidateIds] which bound multiple positional + // params while the SQL only had one placeholder ($1), causing a DB error. + const allIds = [requestingUserId, ...candidateIds]; const { rows: countRows } = await client.query( `SELECT user_id, COUNT(*)::int AS pref_count FROM user_preferences WHERE user_id = ANY($1::uuid[]) GROUP BY user_id`, - [requestingUserId, ...candidateIds], + [allIds], ); const myCount = countRows.find((r) => r.user_id === requestingUserId)?.pref_count ?? 0; @@ -64,8 +43,6 @@ export const scoreUsersForUser = async (requestingUserId, candidateIds, client = const shared = sharedMap[id] ?? 0; const theirCount = countMap[id] ?? 0; const union = myCount + theirCount - shared; - // When union is 0 both users have no preferences — score 0, caller sets - // compatibilityAvailable = false so the UI can hide the score badge. acc[id] = union === 0 ? 0 : Math.round((shared / union) * 100); return acc; }, {}); @@ -74,17 +51,10 @@ export const scoreUsersForUser = async (requestingUserId, candidateIds, client = { err, requestingUserId, candidateCount: candidateIds.length }, "scoreUsersForUser: DB error computing compatibility scores — returning empty scores", ); - // Return safe default so the feed still renders without scores return {}; } }; -// hasPreferences -// -// Quick check: does the given user have at least one preference row? -// Used to set compatibilityAvailable on the requesting user's own side. -// On DB failure: logs the error and returns false so the feed degrades -// gracefully (no compatibility scores shown) rather than crashing. export const hasPreferences = async (userId, client = pool) => { try { const { rows } = await client.query( diff --git a/src/middleware/requireAdmin.js b/src/middleware/requireAdmin.js index 2d631fe..9deb212 100644 --- a/src/middleware/requireAdmin.js +++ b/src/middleware/requireAdmin.js @@ -7,7 +7,8 @@ import { AppError } from "./errorHandler.js"; const assertEmailVerified = (req, res, next) => { if (!req.user) { - return next(new AppError("authenticate middleware must run before requireAdmin", 500)); + // 401 — the caller is unauthenticated, not a server misconfiguration. + return next(new AppError("Authentication required", 401)); } if (!req.user.isEmailVerified) { return next(new AppError("Email verification required", 403)); diff --git a/src/routes/verification.js b/src/routes/verification.js index cf7419e..2066243 100644 --- a/src/routes/verification.js +++ b/src/routes/verification.js @@ -1,9 +1,4 @@ // src/routes/verification.js -// Verification request routes: -// POST /verification/:userId/submit — PG owner submits a document -// GET /verification/queue — Admin: paginated pending queue -// PATCH /verification/:requestId/approve — Admin: approve a request -// PATCH /verification/:requestId/reject — Admin: reject a request import { Router } from "express"; import { authenticate } from "../middleware/authenticate.js"; @@ -13,16 +8,9 @@ import * as vc from "../controllers/verification.controller.js"; export const verificationRouter = Router(); -// ── PG owner routes ──────────────────────────────────────────────────────────── -// A PG owner submits verification documents for their own profile -verificationRouter.post("/:userId/submit", authenticate, authorize("pg_owner"), vc.submitDocument); +verificationRouter.post("/submit", authenticate, authorize("pg_owner"), vc.submitDocument); // ── Admin routes ─────────────────────────────────────────────────────────────── -// GET /verification/queue — paginated list of pending verification requests verificationRouter.get("/queue", authenticate, ...requireAdmin, vc.getVerificationQueue); - -// PATCH /verification/:requestId/approve verificationRouter.patch("/:requestId/approve", authenticate, ...requireAdmin, vc.approveRequest); - -// PATCH /verification/:requestId/reject verificationRouter.patch("/:requestId/reject", authenticate, ...requireAdmin, vc.rejectRequest); diff --git a/src/services/listingRenewal.service.js b/src/services/listingRenewal.service.js index ece5802..db5602e 100644 --- a/src/services/listingRenewal.service.js +++ b/src/services/listingRenewal.service.js @@ -8,13 +8,14 @@ const RENEWABLE_STATUSES = new Set(["active", "expired", "deactivated"]); const RENEWAL_INTERVAL_DAYS = 60; export const renewListing = async (posterId, listingId) => { - // Single atomic UPDATE — no need for a transaction since this is one - // statement. We check ownership, status eligibility, and apply the change - // in one round-trip. + // Use GREATEST(expires_at, NOW()) so an active listing with time remaining + // gets the full renewal interval *added on top of* its current expiry, not + // replaced from NOW(). COALESCE handles the NULL case for listings that have + // never had an expiry set. const { rows } = await pool.query( `UPDATE listings SET status = 'active'::listing_status_enum, - expires_at = NOW() + ($1::int * INTERVAL '1 day'), + expires_at = GREATEST(COALESCE(expires_at, NOW()), NOW()) + ($1::int * INTERVAL '1 day'), updated_at = NOW() WHERE listing_id = $2 AND posted_by = $3 @@ -27,7 +28,6 @@ export const renewListing = async (posterId, listingId) => { [RENEWAL_INTERVAL_DAYS, listingId, posterId, [...RENEWABLE_STATUSES]], ); - // Nothing updated — work out why so we can return the right error. if (!rows.length) { const { rows: check } = await pool.query( `SELECT status, posted_by @@ -45,7 +45,6 @@ export const renewListing = async (posterId, listingId) => { throw new AppError("Forbidden", 403); } - // Must be a non-renewable status (filled). throw new AppError( `Listing cannot be renewed from its current status '${check[0].status}'. ` + `Only active, expired, or deactivated listings can be renewed.`, diff --git a/src/services/profilePhoto.service.js b/src/services/profilePhoto.service.js index dd9e7dd..bfddf73 100644 --- a/src/services/profilePhoto.service.js +++ b/src/services/profilePhoto.service.js @@ -1,4 +1,7 @@ // src/services/profilePhoto.service.js +// Fix: use CTE-based atomic UPDATE...RETURNING to get old_url in the same +// round-trip as the write. This eliminates the pre-read race where concurrent +// uploads could orphan a blob that was overwritten between SELECT and UPDATE. import sharp from "sharp"; import { pool } from "../db/client.js"; @@ -9,8 +12,6 @@ import { AppError } from "../middleware/errorHandler.js"; const MAX_DIMENSION_PX = 400; const WEBP_QUALITY = 82; -const profileBlobPath = (userId) => `profiles/${userId}/${userId}.webp`; - const compressAndUpload = async (stagingPath, userId) => { const buffer = await sharp(stagingPath) .resize(MAX_DIMENSION_PX, MAX_DIMENSION_PX, { fit: "cover", withoutEnlargement: true }) @@ -36,38 +37,35 @@ export const uploadStudentPhoto = async (requestingUserId, targetUserId, staging throw new AppError("Forbidden", 403); } - const { rows } = await pool.query( - `SELECT profile_photo_url - FROM student_profiles - WHERE user_id = $1 - AND deleted_at IS NULL`, - [targetUserId], - ); - - if (!rows.length) { - throw new AppError("Student profile not found", 404); - } - - const oldUrl = rows[0].profile_photo_url; - + // Upload first, then atomically swap + capture old URL in one UPDATE. const newUrl = await compressAndUpload(stagingPath, targetUserId); - const { rowCount } = await pool.query( - `UPDATE student_profiles + const { rows } = await pool.query( + `WITH current AS ( + SELECT profile_photo_url + FROM student_profiles + WHERE user_id = $2 AND deleted_at IS NULL + ) + UPDATE student_profiles sp SET profile_photo_url = $1, updated_at = NOW() - WHERE user_id = $2 - AND deleted_at IS NULL`, + FROM current + WHERE sp.user_id = $2 + AND sp.deleted_at IS NULL + RETURNING current.profile_photo_url AS old_url`, [newUrl, targetUserId], ); - if (rowCount === 0) { + if (!rows.length) { + // Profile disappeared between compress and update — clean up the uploaded blob. await tryDeleteBlob(newUrl, { userId: targetUserId, stage: "post-upload-race" }); throw new AppError("Student profile not found", 404); } + const oldUrl = rows[0].old_url; logger.info({ userId: targetUserId, newUrl, hadPrevious: Boolean(oldUrl) }, "Student profile photo updated"); + // Delete old blob after DB commit so storage errors don't affect the response. await tryDeleteBlob(oldUrl, { userId: targetUserId, stage: "replace-old" }); return { profilePhotoUrl: newUrl }; @@ -99,9 +97,7 @@ export const deleteStudentPhoto = async (requestingUserId, targetUserId) => { } const oldUrl = result[0].old_url; - logger.info({ userId: targetUserId, hadPhoto: Boolean(oldUrl) }, "Student profile photo deleted"); - await tryDeleteBlob(oldUrl, { userId: targetUserId, stage: "delete-photo" }); return { profilePhotoUrl: null }; @@ -112,46 +108,36 @@ export const uploadPgOwnerPhoto = async (requestingUserId, targetUserId, staging throw new AppError("Forbidden", 403); } - const { rows } = await pool.query( - `SELECT profile_photo_url - FROM pg_owner_profiles - WHERE user_id = $1 - AND deleted_at IS NULL`, - [targetUserId], - ); - - if (!rows.length) { - throw new AppError("PG owner profile not found", 404); - } - - const oldUrl = rows[0].profile_photo_url; - const newUrl = await compressAndUpload(stagingPath, targetUserId); - const { rowCount } = await pool.query( - `UPDATE pg_owner_profiles + const { rows } = await pool.query( + `WITH current AS ( + SELECT profile_photo_url + FROM pg_owner_profiles + WHERE user_id = $2 AND deleted_at IS NULL + ) + UPDATE pg_owner_profiles pop SET profile_photo_url = $1, updated_at = NOW() - WHERE user_id = $2 - AND deleted_at IS NULL`, + FROM current + WHERE pop.user_id = $2 + AND pop.deleted_at IS NULL + RETURNING current.profile_photo_url AS old_url`, [newUrl, targetUserId], ); - if (rowCount === 0) { + if (!rows.length) { await tryDeleteBlob(newUrl, { userId: targetUserId, stage: "post-upload-race" }); throw new AppError("PG owner profile not found", 404); } + const oldUrl = rows[0].old_url; logger.info({ userId: targetUserId, newUrl, hadPrevious: Boolean(oldUrl) }, "PG owner profile photo updated"); - await tryDeleteBlob(oldUrl, { userId: targetUserId, stage: "replace-old" }); return { profilePhotoUrl: newUrl }; }; -/** - * Remove the profile photo for a PG owner. - */ export const deletePgOwnerPhoto = async (requestingUserId, targetUserId) => { if (requestingUserId !== targetUserId) { throw new AppError("Forbidden", 403); @@ -178,9 +164,7 @@ export const deletePgOwnerPhoto = async (requestingUserId, targetUserId) => { } const oldUrl = result[0].old_url; - logger.info({ userId: targetUserId, hadPhoto: Boolean(oldUrl) }, "PG owner profile photo deleted"); - await tryDeleteBlob(oldUrl, { userId: targetUserId, stage: "delete-photo" }); return { profilePhotoUrl: null }; diff --git a/src/services/rentIndex.service.js b/src/services/rentIndex.service.js index 0f0663c..d1e56a0 100644 --- a/src/services/rentIndex.service.js +++ b/src/services/rentIndex.service.js @@ -1,5 +1,4 @@ // src/services/rentIndex.service.js -// import { pool } from "../db/client.js"; import { AppError } from "../middleware/errorHandler.js"; @@ -7,8 +6,10 @@ import { AppError } from "../middleware/errorHandler.js"; const paiseToRupees = (paise) => (paise != null ? Math.round(paise / 100) : null); export const getRentIndex = async ({ city, locality, roomType }) => { - const normCity = city ? city.toLowerCase().trim() : city; - const normLocality = locality ? locality.toLowerCase().trim() : null; + // Normalize: trim + lowercase, then treat empty string as null. + // This ensures " " → null, consistent with DB trigger behaviour. + const normCity = city?.trim().toLowerCase() || null; + const normLocality = locality?.trim().toLowerCase() || null; const { rows } = await pool.query( `SELECT @@ -42,9 +43,9 @@ export const getRentIndex = async ({ city, locality, roomType }) => { return { city: normCity, - locality: normLocality, // normalized value (null when blank/absent) + locality: normLocality, roomType, - resolution: row.resolution, // 'locality' | 'city' + resolution: row.resolution, p25: paiseToRupees(row.p25), p50: paiseToRupees(row.p50), p75: paiseToRupees(row.p75), diff --git a/src/validators/rentIndex.validators.js b/src/validators/rentIndex.validators.js index f3ea6a8..62fee88 100644 --- a/src/validators/rentIndex.validators.js +++ b/src/validators/rentIndex.validators.js @@ -4,8 +4,8 @@ import { z } from "zod"; export const getRentIndexSchema = z.object({ query: z.object({ - city: z.string({ error: "city is required" }).min(1, { error: "city is required" }).max(100), - locality: z.string().max(100).optional(), + city: z.string({ error: "city is required" }).trim().min(1, { error: "city is required" }).max(100), + locality: z.string().trim().min(1, { error: "locality cannot be empty if provided" }).max(100).optional(), roomType: z.enum(["single", "double", "triple", "entire_flat"], { error: "roomType must be one of: single, double, triple, entire_flat", }), diff --git a/src/validators/roommate.validators.js b/src/validators/roommate.validators.js index f43c64c..b5c5b9c 100644 --- a/src/validators/roommate.validators.js +++ b/src/validators/roommate.validators.js @@ -8,7 +8,6 @@ export const getRoommateFeedSchema = z.object({ city: z.string().min(1).max(100).optional(), }).transform((data) => ({ ...data, - // Clamp limit to 50 for the roommate feed (tighter than listing search) limit: Math.min(data.limit, 50), })), }); @@ -26,8 +25,13 @@ export const updateRoommateProfileSchema = z.object({ }); export const blockTargetParamsSchema = z.object({ - params: z.object({ - userId: z.uuid({ error: "Invalid user ID" }), - targetUserId: z.uuid({ error: "Invalid target user ID" }), - }), + params: z + .object({ + userId: z.uuid({ error: "Invalid user ID" }), + targetUserId: z.uuid({ error: "Invalid target user ID" }), + }) + .refine((p) => p.userId !== p.targetUserId, { + message: "You cannot block yourself", + path: ["targetUserId"], + }), }); diff --git a/src/workers/mediaProcessor.js b/src/workers/mediaProcessor.js index 3f1b427..e712b07 100644 --- a/src/workers/mediaProcessor.js +++ b/src/workers/mediaProcessor.js @@ -99,7 +99,7 @@ export const startMediaWorker = () => { logger.error({ jobId: job?.id, photoId: job?.data?.photoId, err }, "Media worker: job failed"); if (job && job.attemptsMade >= (job.opts.attempts ?? 1)) { - const { photoId, listingId } = job.data ?? {}; + const { photoId, listingId, stagingPath } = job.data ?? {}; if (!photoId || !listingId) { logger.error({ jobId: job.id }, "Media worker: missing photoId or listingId in job data"); return; @@ -124,6 +124,23 @@ export const startMediaWorker = () => { "Media worker: failed to clean provisional row after permanent failure", ); } + + if (stagingPath) { + try { + await fs.unlink(stagingPath); + logger.warn( + { stagingPath, photoId, listingId }, + "Media worker: staging file cleaned after permanent failure", + ); + } catch (fsErr) { + if (fsErr.code !== "ENOENT") { + logger.error( + { stagingPath, photoId, listingId, err: fsErr }, + "Media worker: failed to delete staging file after permanent failure", + ); + } + } + } } }); diff --git a/src/workers/notificationQueue.js b/src/workers/notificationQueue.js index 845d4c6..99ea9a3 100644 --- a/src/workers/notificationQueue.js +++ b/src/workers/notificationQueue.js @@ -28,7 +28,10 @@ export const enqueueNotification = (payload) => { }; export const enqueueNotificationsBulk = async (payloads) => { - if (!payloads.length) return 0; + if (!Array.isArray(payloads)) { + throw new TypeError("enqueueNotificationsBulk: payloads must be an array"); + } + if (payloads.length === 0) return 0; try { await getQueue(NOTIFICATION_QUEUE_NAME).addBulk(