From ec79a4da1293e240ed745141834cb0cadc204b64 Mon Sep 17 00:00:00 2001 From: ahmadogo Date: Sun, 18 Oct 2026 12:12:27 +0100 Subject: [PATCH] feat(properties): Prometheus metrics, query optimisation, Redis caching, per-caller rate limits Observability, performance and abuse protection for the Property API (`src/properties`), following the conventions already established by the Users API (metrics -> `src/users/users-metrics.ts`, caching -> `users-cache.service.ts`, rate limits -> `guards/users-route-rate-limit.guard.ts`). - #1367: `properties_requests_total` (counter, `operation` + `outcome`) and `properties_request_duration_seconds` (histogram, `operation`) recorded by `PropertiesMetricsInterceptor` on every controller handler. Labels are bounded, so cardinality cannot grow with property input; docs/api/properties-metrics.md - #1368: listing queries now sort on the requested column plus `id`, and `GET /properties` defaults to `createdAt desc, id desc`, so skip/take paging can no longer repeat or drop a row. Added `prisma/migrations/20261001000000_add_property_listing_indexes`: every listing query filters `is_deleted = false AND status = 'ACTIVE'`, which previously required the planner to intersect the separate `status`/`is_deleted` indexes. docs/api/properties-db-optimization.md - #1369: `GET /properties/:id` is read through `PropertiesCacheService` (key `properties:detail:`, 60s TTL, also registered under `CACHE_TAGS.PROPERTIES`). Lookups that miss are not cached, so a 404 cannot be cached in front of a property created moments later. Every write path invalidates the key, and `transitionStatus` invalidates before its follow-up `findOne` so a stale status is never served. Cache errors fall back to the database and never fail a request. docs/api/properties-caching.md - #1370: `PropertiesRouteRateLimitGuard` + `@PropertiesRouteLimit` cap create (30/h), bulk status and delete (20/h), bulk export (10/h) and PDF report generation (15/h) per caller, answering 429 with `Retry-After`. docs/api/properties-rate-limiting.md `PropertiesCacheService` is `@Optional()`, so `PropertiesService` still constructs without it and the existing unit test module is unaffected. No tests were run and nothing was installed, built or linted; please verify in CI. Closes #1367 Closes #1368 Closes #1369 Closes #1370 --- docs/api/properties-caching.md | 26 +++++ docs/api/properties-db-optimization.md | 36 +++++++ docs/api/properties-metrics.md | 12 +++ docs/api/properties-rate-limiting.md | 23 ++++ .../migration.sql | 20 ++++ prisma/schema.prisma | 5 + .../properties-route-rate-limit.guard.ts | 69 ++++++++++++ .../properties-metrics.interceptor.ts | 30 ++++++ src/properties/properties-cache.service.ts | 66 ++++++++++++ src/properties/properties-metrics.ts | 32 ++++++ src/properties/properties.controller.ts | 19 +++- src/properties/properties.module.ts | 7 ++ src/properties/properties.service.ts | 101 +++++++++++++----- 13 files changed, 418 insertions(+), 28 deletions(-) create mode 100644 docs/api/properties-caching.md create mode 100644 docs/api/properties-db-optimization.md create mode 100644 docs/api/properties-metrics.md create mode 100644 docs/api/properties-rate-limiting.md create mode 100644 prisma/migrations/20261001000000_add_property_listing_indexes/migration.sql create mode 100644 src/properties/guards/properties-route-rate-limit.guard.ts create mode 100644 src/properties/interceptors/properties-metrics.interceptor.ts create mode 100644 src/properties/properties-cache.service.ts create mode 100644 src/properties/properties-metrics.ts diff --git a/docs/api/properties-caching.md b/docs/api/properties-caching.md new file mode 100644 index 00000000..153de791 --- /dev/null +++ b/docs/api/properties-caching.md @@ -0,0 +1,26 @@ +# Property API Redis caching + +`PropertiesService.findOne` (`GET /properties/:id`) is cached per property for 60 +seconds through `PropertiesCacheService`, which uses the shared `CacheService` +(Redis when configured). + +- Key: `properties:detail:`; TTL 60s, so a detail view can be up to a + minute stale. The key is also registered under `CACHE_TAGS.PROPERTIES`, so the + existing tag invalidation covers it. +- Read-through only: cache errors, or no cache being available, fall back to the + database, so caching can never fail a request. +- A lookup that finds nothing is **not** cached. The controller turns `null` into + a 404, and caching that would hide a property created moments later for a whole + TTL. +- Search and listing results are deliberately **not** cached: their result sets + change on almost every write and their query keys are unbounded. + +Every write path drops the affected key, so a stale entry cannot outlive a write +by more than the TTL: `create`, `update`, `remove`, `restore`, `transitionStatus` +(including approve/reject), agent assignment create/update/remove, and the amenity +endpoints. `transitionStatus` invalidates *before* its follow-up `findOne`, so the +new status is never read from the cache. Bulk operations invalidate by tag, as +before. + +`PropertiesCacheService` is injected with `@Optional()`, so the service still +constructs without it and existing unit tests are unaffected. diff --git a/docs/api/properties-db-optimization.md b/docs/api/properties-db-optimization.md new file mode 100644 index 00000000..aa92b35b --- /dev/null +++ b/docs/api/properties-db-optimization.md @@ -0,0 +1,36 @@ +# Property API database query optimisation + +## Stable pagination ordering + +`GET /properties` and `GET /properties/search` page with `skip`/`take`, but neither +guaranteed a total order. Rows with the same value in the sort column could be +returned in a different order on each call, so a row could appear on page 1 and +again on page 2, or be skipped entirely. + +Both paths now order by the requested sort column **and** `id` as a tie-breaker. +`GET /properties` additionally defaults to `createdAt desc, id desc` instead of +leaving the order to the database. + +## Composite indexes for the listing read path + +Every listing query filters `is_deleted = false AND status = 'ACTIVE'`. Before this +change the schema only had `status` and `is_deleted` indexed separately, plus +`(status, created_at)` and `(status, price)` from +`prisma/migrations/20260325120000_composite_indexes`, so the planner had to +intersect indexes and then sort. + +`prisma/migrations/20261001000000_add_property_listing_indexes` adds: + +| Index | Serves | +| --- | --- | +| `(status, is_deleted, created_at DESC)` | default listing order | +| `(status, is_deleted, price)` | price-sorted listing and price-range search | +| `(city, state, status, is_deleted)` | location search restricted to public listings | + +The matching `@@index` entries are declared on the `Property` model in +`prisma/schema.prisma`. + +## Owner listings + +`findByOwnerId` gained the same `id` tie-breaker so owner dashboard paging is +stable too. diff --git a/docs/api/properties-metrics.md b/docs/api/properties-metrics.md new file mode 100644 index 00000000..1998ca00 --- /dev/null +++ b/docs/api/properties-metrics.md @@ -0,0 +1,12 @@ +# Property API Prometheus metrics + +Exposed on the existing `/metrics` endpoint: + +| Metric | Type | Labels | +| --- | --- | --- | +| `properties_requests_total` | counter | `operation` (controller handler name), `outcome` (`success`, `client_error`, `server_error`) | +| `properties_request_duration_seconds` | histogram | `operation` | + +`PropertiesMetricsInterceptor` records both for every `PropertiesController` handler. Labels are bounded (a fixed set of handler names and three outcomes), so cardinality does not grow with property input. + +`properties_requests_total{operation="create",outcome="client_error"}` is the cheapest way to see a spike in bad property payloads, and the duration histogram shows which handler dominates API latency. The pre-existing `properties_total` counter (properties created) is unchanged. diff --git a/docs/api/properties-rate-limiting.md b/docs/api/properties-rate-limiting.md new file mode 100644 index 00000000..cfc0fe88 --- /dev/null +++ b/docs/api/properties-rate-limiting.md @@ -0,0 +1,23 @@ +# Property API rate limiting + +`PropertiesRouteRateLimitGuard` + `@PropertiesRouteLimit({ max, windowMs })` limit +costly or abusable routes per caller (the authenticated user id, falling back to +the client address). Each route has its own counter. + +| Route | Limit | +| --- | --- | +| `POST /properties` | 30 per hour | +| `POST /properties/bulk/status` | 20 per hour | +| `POST /properties/bulk/delete` | 20 per hour | +| `POST /properties/bulk/export` | 10 per hour | +| `GET /properties/:id/report` | 15 per hour | + +Over the limit the response is `429` with a `Retry-After` header. The guard is +registered after the JWT guard on authenticated routes so the caller is known. + +Counters are namespaced `properties-route: ` in the shared +`RateLimitService`, so they are independent from the users route limits and from +the existing in-service geocoding throttle. + +The public read routes (`GET /properties`, `GET /properties/search`) are not +limited here; they are unauthenticated and are covered separately. diff --git a/prisma/migrations/20261001000000_add_property_listing_indexes/migration.sql b/prisma/migrations/20261001000000_add_property_listing_indexes/migration.sql new file mode 100644 index 00000000..89188626 --- /dev/null +++ b/prisma/migrations/20261001000000_add_property_listing_indexes/migration.sql @@ -0,0 +1,20 @@ +-- Composite indexes for the public Property/Listing read paths. +-- +-- Every listing query filters `is_deleted = false AND status = 'ACTIVE'`. The +-- existing indexes cover `status` and `is_deleted` separately and +-- `(status, created_at)` / `(status, price)`, so the database has to intersect +-- two or three indexes for the commonest query in the product and then sort. +-- These replace that with a single index seek. + +-- Default listing order: active, not deleted, newest first (GET /properties and +-- GET /properties/search with no explicit sort). +CREATE INDEX "properties_status_deleted_created_at_idx" +ON "properties" ("status", "is_deleted", "created_at" DESC); + +-- Price-sorted listing order and price-range search. +CREATE INDEX "properties_status_deleted_price_idx" +ON "properties" ("status", "is_deleted", "price"); + +-- Location search (city/state) restricted to the public listing set. +CREATE INDEX "properties_city_state_status_deleted_idx" +ON "properties" ("city", "state", "status", "is_deleted"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 1f1a3a90..0524e01c 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -522,6 +522,11 @@ model Property { @@index([propertyType]) @@index([viewCount]) @@index([neighborhoodId]) + // Public listing read paths: status + soft-delete + the sort/predicate column, + // so the commonest query is one index seek instead of several intersected. + @@index([status, deleted, createdAt(sort: Desc)]) + @@index([status, deleted, price]) + @@index([city, state, status, deleted]) @@map("properties") } diff --git a/src/properties/guards/properties-route-rate-limit.guard.ts b/src/properties/guards/properties-route-rate-limit.guard.ts new file mode 100644 index 00000000..2e58aa7f --- /dev/null +++ b/src/properties/guards/properties-route-rate-limit.guard.ts @@ -0,0 +1,69 @@ +import { + CanActivate, + ExecutionContext, + HttpException, + HttpStatus, + Injectable, + SetMetadata, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { RateLimitService } from '../../auth/rate-limit.service'; + +export const PROPERTIES_ROUTE_LIMIT_KEY = 'properties-route-limit'; + +export interface PropertiesRouteLimitOptions { + /** Requests allowed per caller per window. */ + max: number; + windowMs: number; +} + +/** Limit one Property API route per caller. Apply after the JWT guard. */ +export const PropertiesRouteLimit = (options: PropertiesRouteLimitOptions) => + SetMetadata(PROPERTIES_ROUTE_LIMIT_KEY, options); + +/** + * Per-caller rate limit for costly or abusable Property API routes. The caller is + * the authenticated user id (falling back to the client address), and each route + * has its own counter, so heavy use of one route does not consume another's budget. + * Over the limit the response is `429` with `Retry-After`. + */ +@Injectable() +export class PropertiesRouteRateLimitGuard implements CanActivate { + constructor( + private readonly reflector: Reflector, + private readonly rateLimitService: RateLimitService, + ) {} + + async canActivate(context: ExecutionContext): Promise { + const options = this.reflector.get( + PROPERTIES_ROUTE_LIMIT_KEY, + context.getHandler(), + ); + if (!options) return true; + + const request = context.switchToHttp().getRequest(); + const response = context.switchToHttp().getResponse(); + const route = `${request.method} ${request.route?.path ?? request.url}`; + const caller = String(request.authUser?.sub ?? request.user?.sub ?? request.ip ?? 'unknown'); + + const status = await this.rateLimitService.checkScopedRateLimit( + `properties-route:${route}`, + caller, + options.max, + options.windowMs, + ); + + if (status.isExceeded) { + if (status.retryAfter !== undefined) response.setHeader('Retry-After', String(status.retryAfter)); + throw new HttpException( + { + statusCode: HttpStatus.TOO_MANY_REQUESTS, + message: 'Too many requests. Please try again later.', + retryAfter: status.retryAfter, + }, + HttpStatus.TOO_MANY_REQUESTS, + ); + } + return true; + } +} diff --git a/src/properties/interceptors/properties-metrics.interceptor.ts b/src/properties/interceptors/properties-metrics.interceptor.ts new file mode 100644 index 00000000..435cfbc6 --- /dev/null +++ b/src/properties/interceptors/properties-metrics.interceptor.ts @@ -0,0 +1,30 @@ +import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common'; +import { Observable } from 'rxjs'; +import { tap } from 'rxjs/operators'; +import { + propertiesOutcomeForStatus, + propertiesRequestDurationSeconds, + propertiesRequestsTotal, +} from '../properties-metrics'; + +/** Records `properties_requests_total` and `properties_request_duration_seconds` for each handler call. */ +@Injectable() +export class PropertiesMetricsInterceptor implements NestInterceptor { + intercept(context: ExecutionContext, next: CallHandler): Observable { + const operation = context.getHandler().name; + const end = propertiesRequestDurationSeconds.startTimer({ operation }); + + const record = (status: number): void => { + end(); + propertiesRequestsTotal.inc({ operation, outcome: propertiesOutcomeForStatus(status) }); + }; + + return next.handle().pipe( + tap({ + next: () => record(context.switchToHttp().getResponse().statusCode ?? 200), + error: (err: { status?: number; getStatus?: () => number }) => + record(typeof err?.getStatus === 'function' ? err.getStatus() : (err?.status ?? 500)), + }), + ); + } +} diff --git a/src/properties/properties-cache.service.ts b/src/properties/properties-cache.service.ts new file mode 100644 index 00000000..b2807ea7 --- /dev/null +++ b/src/properties/properties-cache.service.ts @@ -0,0 +1,66 @@ +import { Injectable, Optional } from '@nestjs/common'; +import { CacheService } from '../cache/cache.service'; +import { CacheOperationStatus } from '../cache/cache.types'; +import { CACHE_TAGS } from '../cache/cache.config'; + +export const PROPERTY_DETAIL_TTL_SECONDS = 60; + +/** + * Read-through Redis cache for property detail responses (`GET /properties/:id`). + * + * Only the read-only detail view is cached. Status transitions, edits, deletes and + * agent assignment all call `invalidateProperty`, so a stale entry can never + * outlive a write by more than the TTL. + * + * Cache failures never fail a request: on any cache error, or when no cache is + * available, the loader runs as if caching were off. + */ +@Injectable() +export class PropertiesCacheService { + constructor(@Optional() private readonly cache?: CacheService) {} + + propertyKey(propertyId: string): string { + return `properties:detail:${propertyId}`; + } + + async remember(key: string, ttlSeconds: number, loader: () => Promise): Promise { + if (!this.cache) return loader(); + try { + const hit = await this.cache.get(key); + if (hit.status === CacheOperationStatus.SUCCESS && hit.data !== undefined) return hit.data as T; + } catch { + return loader(); + } + const value = await loader(); + try { + await this.cache.set(key, value, ttlSeconds, CACHE_TAGS.PROPERTIES); + } catch { + /* a failed write only means the next call recomputes */ + } + return value; + } + + /** Cache the property detail view under its own key. */ + rememberPropertyDetail(propertyId: string, loader: () => Promise): Promise { + return this.remember(this.propertyKey(propertyId), PROPERTY_DETAIL_TTL_SECONDS, loader); + } + + /** + * Cache a property detail view only when the row actually exists. A `null` + * lookup is deliberately not cached: the controller turns it into a 404, and + * caching that would hide a property created moments later for a whole TTL. + */ + async rememberExistingProperty(propertyId: string, loader: () => Promise): Promise { + const value = await this.rememberPropertyDetail(propertyId, loader); + if (value === null || value === undefined) await this.invalidateProperty(propertyId); + return value; + } + + async invalidateProperty(propertyId: string): Promise { + try { + await this.cache?.del(this.propertyKey(propertyId)); + } catch { + /* the entry expires on its own within the TTL */ + } + } +} diff --git a/src/properties/properties-metrics.ts b/src/properties/properties-metrics.ts new file mode 100644 index 00000000..6debb686 --- /dev/null +++ b/src/properties/properties-metrics.ts @@ -0,0 +1,32 @@ +import { Counter, Histogram, register } from 'prom-client'; + +/** + * Prometheus metrics for the Property API. Labels are bounded: `operation` is a + * controller handler name and `outcome` one of three values, so cardinality + * cannot grow with property input. Metrics are looked up before being created so + * hot reloads and test suites do not register them twice. + */ +export type PropertiesOutcome = 'success' | 'client_error' | 'server_error'; + +export const propertiesRequestsTotal: Counter<'operation' | 'outcome'> = + (register.getSingleMetric('properties_requests_total') as Counter<'operation' | 'outcome'>) ?? + new Counter({ + name: 'properties_requests_total', + help: 'Property API requests by handler and outcome', + labelNames: ['operation', 'outcome'], + }); + +export const propertiesRequestDurationSeconds: Histogram<'operation'> = + (register.getSingleMetric('properties_request_duration_seconds') as Histogram<'operation'>) ?? + new Histogram({ + name: 'properties_request_duration_seconds', + help: 'Property API handler latency in seconds', + labelNames: ['operation'], + buckets: [0.01, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5], + }); + +export function propertiesOutcomeForStatus(status: number): PropertiesOutcome { + if (status >= 500) return 'server_error'; + if (status >= 400) return 'client_error'; + return 'success'; +} diff --git a/src/properties/properties.controller.ts b/src/properties/properties.controller.ts index 90da43da..aa5294eb 100644 --- a/src/properties/properties.controller.ts +++ b/src/properties/properties.controller.ts @@ -9,6 +9,7 @@ import { Patch, Query, UseGuards, + UseInterceptors, Res, // eslint-disable-next-line @typescript-eslint/no-unused-vars HttpStatus, @@ -35,8 +36,14 @@ import { CreateAmenityDto, UpdateAmenityDto } from './dto/amenity.dto'; import { PropertyReportService } from './report/property-report.service'; import { Response } from 'express'; import { ApiTags, ApiBearerAuth } from '@nestjs/swagger'; +import { PropertiesMetricsInterceptor } from './interceptors/properties-metrics.interceptor'; +import { + PropertiesRouteLimit, + PropertiesRouteRateLimitGuard, +} from './guards/properties-route-rate-limit.guard'; @ApiTags('Properties') +@UseInterceptors(PropertiesMetricsInterceptor) @Controller('properties') export class PropertiesController { constructor( @@ -45,7 +52,8 @@ export class PropertiesController { ) {} @ApiBearerAuth('access-token') - @UseGuards(JwtAuthGuard) + @UseGuards(JwtAuthGuard, PropertiesRouteRateLimitGuard) + @PropertiesRouteLimit({ max: 30, windowMs: 60 * 60 * 1000 }) @Post() create(@Body() createPropertyDto: CreatePropertyDto, @CurrentUser() user: AuthUserPayload) { return this.propertiesService.create(createPropertyDto, user.sub); @@ -107,7 +115,8 @@ export class PropertiesController { } @ApiBearerAuth('access-token') - @UseGuards(JwtAuthGuard) + @UseGuards(JwtAuthGuard, PropertiesRouteRateLimitGuard) + @PropertiesRouteLimit({ max: 15, windowMs: 60 * 60 * 1000 }) @Get(':id/report') async generateReport(@Param('id') id: string, @Res() res: Response): Promise { try { @@ -182,6 +191,8 @@ export class PropertiesController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.ADMIN) @Post('bulk/status') + @UseGuards(PropertiesRouteRateLimitGuard) + @PropertiesRouteLimit({ max: 20, windowMs: 60 * 60 * 1000 }) async bulkUpdatePropertyStatus( @Body() body: BulkPropertyStatusUpdateDto, // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -194,6 +205,8 @@ export class PropertiesController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.ADMIN) @Post('bulk/delete') + @UseGuards(PropertiesRouteRateLimitGuard) + @PropertiesRouteLimit({ max: 20, windowMs: 60 * 60 * 1000 }) async bulkDeleteProperties( @Body() body: BulkPropertyDeleteDto, // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -206,6 +219,8 @@ export class PropertiesController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.ADMIN) @Post('bulk/export') + @UseGuards(PropertiesRouteRateLimitGuard) + @PropertiesRouteLimit({ max: 10, windowMs: 60 * 60 * 1000 }) async bulkExportProperties( @Body() body: BulkPropertyExportDto, // eslint-disable-next-line @typescript-eslint/no-unused-vars diff --git a/src/properties/properties.module.ts b/src/properties/properties.module.ts index e9dfe9b9..c6c98bf9 100644 --- a/src/properties/properties.module.ts +++ b/src/properties/properties.module.ts @@ -14,6 +14,9 @@ import { FraudModule } from '../fraud/fraud.module'; import { DocumentsModule } from '../documents/documents.module'; import { PropertyReportService } from './report/property-report.service'; import { CacheModuleConfig } from '../cache/cache.module'; +import { PropertiesCacheService } from './properties-cache.service'; +import { PropertiesRouteRateLimitGuard } from './guards/properties-route-rate-limit.guard'; +import { RateLimitService } from '../auth/rate-limit.service'; @Module({ imports: [ @@ -32,6 +35,9 @@ import { CacheModuleConfig } from '../cache/cache.module'; GeocodingService, PropertyExpiryService, PropertyReportService, + PropertiesCacheService, + RateLimitService, + PropertiesRouteRateLimitGuard, ], exports: [ PropertiesService, @@ -39,6 +45,7 @@ import { CacheModuleConfig } from '../cache/cache.module'; PropertyImagesService, GeocodingService, PropertyExpiryService, + PropertiesCacheService, ], }) export class PropertiesModule {} diff --git a/src/properties/properties.service.ts b/src/properties/properties.service.ts index 5a46cd7d..e06e3fef 100644 --- a/src/properties/properties.service.ts +++ b/src/properties/properties.service.ts @@ -3,6 +3,7 @@ import { ForbiddenException, Injectable, NotFoundException, + Optional, } from '@nestjs/common'; import { AppLogger } from '../common/logger'; import { Decimal } from '@prisma/client/runtime/library'; @@ -18,6 +19,7 @@ import { GeocodingService } from './geocoding.service'; import { PropertyStatus, UserRole } from '../types/prisma.types'; import { CacheService } from '../cache/cache.service'; import { CACHE_TAGS } from '../cache/cache.config'; +import { PropertiesCacheService } from './properties-cache.service'; import { canTransitionPropertyStatus, getAllowedNextPropertyStatuses, @@ -51,8 +53,26 @@ export class PropertiesService { private readonly fraudService: FraudService, private readonly geocodingService: GeocodingService, private readonly cacheService: CacheService, + @Optional() private readonly propertiesCache?: PropertiesCacheService, ) {} + /** + * Drop the cached detail view of a property. Best-effort: a cache failure is + * logged and ignored, because the entry also expires on its own within the TTL. + */ + private async invalidateCachedProperty(propertyId: string): Promise { + if (!this.propertiesCache) return; + try { + await this.propertiesCache.invalidateProperty(propertyId); + } catch (err) { + this.logger.warn('Property detail cache invalidation failed', { + event: 'properties.detail_cache_invalidation_failed', + propertyId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + private async checkGeocodeRateLimit(): Promise { const now = Date.now(); this.geocodeTimestamps = this.geocodeTimestamps.filter( @@ -233,6 +253,7 @@ export class PropertiesService { error: err instanceof Error ? err.message : String(err), }); } + await this.invalidateCachedProperty(property.id); return property; } @@ -243,11 +264,16 @@ export class PropertiesService { ? { ...where, status: where.status ?? PropertyStatus.ACTIVE, deleted: false } : { status: PropertyStatus.ACTIVE, deleted: false }; + // Without an explicit sort the database returned rows in arbitrary order, so + // `skip`/`take` paging could repeat or skip a property between calls. Default + // to newest first with `id` as a tie-breaker to make paging stable. + const finalOrderBy = orderBy ?? { createdAt: 'desc' as const, id: 'desc' as const }; + return (this.prisma.property as any).findMany({ skip, take, where: finalWhere, - orderBy, + orderBy: finalOrderBy, include: { owner: { select: { @@ -275,33 +301,38 @@ export class PropertiesService { } async findOne(id: string) { - return (this.prisma.property as any).findUnique({ - where: { id }, - include: { - owner: { - select: { - id: true, - firstName: true, - lastName: true, - email: true, + const load = () => + (this.prisma.property as any).findUnique({ + where: { id }, + include: { + owner: { + select: { + id: true, + firstName: true, + lastName: true, + email: true, + }, }, - }, - documents: true, - agents: { - include: { - agent: { - select: { - id: true, - firstName: true, - lastName: true, - email: true, - phone: true, + documents: true, + agents: { + include: { + agent: { + select: { + id: true, + firstName: true, + lastName: true, + email: true, + phone: true, + }, }, }, }, }, - }, - }); + }); + + // Served from Redis when the module is wired with PropertiesCacheService. + if (!this.propertiesCache) return load(); + return this.propertiesCache.rememberExistingProperty(id, load); } async update(id: string, updatePropertyDto: UpdatePropertyDto, userId?: string) { @@ -421,6 +452,7 @@ export class PropertiesService { }); await this.cacheService.invalidateByTag(CACHE_TAGS.PROPERTIES); + await this.invalidateCachedProperty(id); return updatedProperty; } @@ -449,6 +481,7 @@ export class PropertiesService { }); await this.cacheService.invalidateByTag(CACHE_TAGS.PROPERTIES); + await this.invalidateCachedProperty(id); this.logger.warn('Property deleted', { event: 'properties.deleted', propertyId: id, userId: user.sub }); return updated; } @@ -478,6 +511,7 @@ export class PropertiesService { }); await this.cacheService.invalidateByTag(CACHE_TAGS.PROPERTIES); + await this.invalidateCachedProperty(id); this.logger.log('Property restored', { event: 'properties.restored', propertyId: id, userId: user.sub }); return updated; } @@ -485,7 +519,7 @@ export class PropertiesService { async findByOwnerId(ownerId: string) { return this.prisma.property.findMany({ where: { ownerId }, - orderBy: { createdAt: 'desc' }, + orderBy: { createdAt: 'desc', id: 'desc' }, }); } @@ -572,6 +606,10 @@ export class PropertiesService { data: { status: nextStatus }, }); + // Drop the cached detail view first: the read below would otherwise serve the + // pre-transition status. + await this.invalidateCachedProperty(propertyId); + return this.findOne(propertyId); } @@ -594,7 +632,7 @@ export class PropertiesService { // Fetch all matching properties with neighborhood to filter by trust score in-memory const allItems = await this.prisma.property.findMany({ where, - orderBy: { [sortBy]: sortOrder }, + orderBy: [{ [sortBy]: sortOrder }, { id: sortOrder }], include: { owner: { select: { id: true, firstName: true, lastName: true, email: true } }, neighborhood: true, @@ -636,7 +674,9 @@ export class PropertiesService { where, skip, take: limit, - orderBy: { [sortBy]: sortOrder }, + // `id` breaks ties so two rows with the same sort value cannot move + // between pages and be shown twice or not at all. + orderBy: [{ [sortBy]: sortOrder }, { id: sortOrder }], include: { owner: { select: { id: true, firstName: true, lastName: true, email: true } }, neighborhood: true, @@ -1008,6 +1048,8 @@ export class PropertiesService { throw new BadRequestException('Agent is already assigned to this property'); } + await this.invalidateCachedProperty(propertyId); + return this.prisma.propertyAgent.create({ data: { propertyId, @@ -1064,6 +1106,8 @@ export class PropertiesService { throw new NotFoundException('Agent assignment not found for this property'); } + await this.invalidateCachedProperty(propertyId); + return this.prisma.propertyAgent.update({ where: { propertyId_agentId: { @@ -1117,6 +1161,8 @@ export class PropertiesService { throw new NotFoundException('Agent assignment not found for this property'); } + await this.invalidateCachedProperty(propertyId); + return this.prisma.propertyAgent.delete({ where: { propertyId_agentId: { @@ -1170,6 +1216,7 @@ export class PropertiesService { if (!property) { throw new NotFoundException('Property not found'); } + await this.invalidateCachedProperty(propertyId); return this.prisma.propertyAmenity.create({ data: { propertyId, @@ -1199,6 +1246,7 @@ export class PropertiesService { if (!amenity || amenity.propertyId !== propertyId) { throw new NotFoundException('Amenity not found for this property'); } + await this.invalidateCachedProperty(propertyId); return this.prisma.propertyAmenity.update({ where: { id: amenityId }, data: { @@ -1217,6 +1265,7 @@ export class PropertiesService { if (!amenity || amenity.propertyId !== propertyId) { throw new NotFoundException('Amenity not found for this property'); } + await this.invalidateCachedProperty(propertyId); return this.prisma.propertyAmenity.delete({ where: { id: amenityId } }); } }