Repository navigation
feat(properties): Prometheus metrics, query optimisation, Redis caching and per-route rate limits for the Property API - #1471
Open
ahmadogo wants to merge 2 commits into
Conversation
…ng, 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`). - MettaChain#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 - MettaChain#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 - MettaChain#1369: `GET /properties/:id` is read through `PropertiesCacheService` (key `properties:detail:<id>`, 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 - MettaChain#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 MettaChain#1367 Closes MettaChain#1368 Closes MettaChain#1369 Closes MettaChain#1370
|
@ahmadogo Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the Property API (
src/properties) observable, faster and abuse-resistant. This mirrors the conventions already established for the Users API in #1457–#1463 (users-metrics.ts/users-cache.service.ts/UsersRouteRateLimitGuard), so the two modules stay symmetrical.#1367 — Prometheus metrics
src/properties/properties-metrics.ts+interceptors/properties-metrics.interceptor.ts, applied at the controller:properties_requests_totaloperation,outcome(success/client_error/server_error)properties_request_duration_secondsoperationBoth are surfaced on the existing
/metricsendpoint.operationis a controller handler name andoutcomehas three values, so cardinality is bounded and does not grow with property input — the failure mode that makes property-scoped metrics unusable. Metrics are resolved throughregister.getSingleMetricbefore construction so hot reloads and the test suite do not register them twice.→
docs/api/properties-metrics.md#1368 — Database query optimisation
Stable pagination.
GET /propertiesandGET /properties/searchpage withskip/takebut guaranteed no total order. Two rows sharing a sort value could come back in a different order per call, so a property could appear on page 1 and page 2, or be skipped entirely. Both paths now order by the sort column plusidas a tie-breaker, andGET /propertiesdefaults tocreatedAt desc, id descinstead of deferring to the database.findByOwnerIdgets the same treatment.Composite indexes. Every listing query filters
is_deleted = false AND status = 'ACTIVE'. The schema only hadstatusandis_deletedindexed separately (plus(status, created_at)and(status, price)from the earlier composite-index migration), so the planner intersected indexes and then sorted — on the single most common query in the product.prisma/migrations/20261001000000_add_property_listing_indexesadds(status, is_deleted, created_at DESC),(status, is_deleted, price)and(city, state, status, is_deleted), with matching@@indexentries on thePropertymodel.→
docs/api/properties-db-optimization.md#1369 — Redis caching
GET /properties/:idis read throughPropertiesCacheServiceon the sharedCacheService(Redis when configured): keyproperties:detail:<id>, 60s TTL, also registered underCACHE_TAGS.PROPERTIESso existing tag invalidation covers it.Deliberate decisions worth reviewing:
nullinto a 404; caching that would hide a property created moments later for a full TTL.transitionStatus(so approve/reject), agent assignment create/update/remove, and the amenity endpoints.transitionStatusinvalidates before its follow-upfindOne, otherwise the new status would be read back out of the cache.PropertiesCacheServiceis injected with@Optional(), soPropertiesServicestill constructs without it and the existing unit-test module is unaffected.→
docs/api/properties-caching.md#1370 — Rate limiting
PropertiesRouteRateLimitGuard+@PropertiesRouteLimit({ max, windowMs }), run after the JWT guard so the caller is known (authenticated user id, falling back to client address), with an independent counter per route:POST /propertiesPOST /properties/bulk/status,POST /properties/bulk/deletePOST /properties/bulk/exportGET /properties/:id/reportOver the limit:
429withRetry-After. Counters are namespacedproperties-route:<METHOD> <path>, so they do not interact with the users route limits or the existing in-service geocoding throttle.→
docs/api/properties-rate-limiting.mdScope
Module-local additions plus one doc under
docs/api/per issue, mirroring the Users API PRs. Nothing was installed, built, linted or tested — please verify in CI. The service test module needed no changes because of the@Optional()injection.Closes #1367
Closes #1368
Closes #1369
Closes #1370