Repository navigation
refactor(backend): consolidate delivery controllers, remove unsafe casts, unify query contract & extract SDK interfaces (#219, #220, #221, #222) - #285
Merged
Tybravo merged 2 commits intoSep 30, 2026
Conversation
…sts, unify query contract & extract provider interfaces (SwiftChainn#219, SwiftChainn#220, SwiftChainn#221, SwiftChainn#222) SwiftChainn#219 — collapse the triple delivery controllers into one canonical src/controllers/delivery.controller.ts (plus a re-export shim and DI cleanup). SwiftChainn#220 — remove `as unknown as` / `as any` casts across auth, delivery, notification, repository and user layers by introducing typed projections (UserDTO), zod parsing and proper AppError typing. SwiftChainn#221 — roll the shared query middleware out to every list endpoint (deliveries, users/deleted, disputes, fleets, notifications, webhooks, event log) with a uniform buildPaginationMeta contract, documented in docs/query-contract.md, and add tests/integration/queryContract.test.ts covering all seven endpoints end to end. SwiftChainn#222 — extract external SDK clients behind interfaces (IRoutingProvider, ISorobanRpcClient, IImagesStorage) with swappable adapters, injected through awilix DI, so services no longer import Google Maps or the Stellar SDK directly. Also fixes two pre-existing crashes that blocked app-boot test suites (required for SwiftChainn#221's integration tests): rateLimiter TEST_UNLIMITED_MAX undefined and delivery.routes validateRequest not imported. Verification: tsc --noEmit errors 79 -> 76 (no new errors; 3 removed by the bug fixes), eslint 0 errors in touched files, jest 16 failing suites vs 30 at baseline (14 suites fixed, 0 new failures; remaining failures are pre-existing upstream drift), tsc build unchanged. Closes SwiftChainn#219 Closes SwiftChainn#220 Closes SwiftChainn#221 Closes SwiftChainn#222 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@johnkoye19 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! 🚀 |
Author
Verification Evidence (local, full toolchain)All commands run from the repo root against the project's pnpm 9 toolchain (binaries invoked directly, e.g. TypeScript — no new errorsESLint — 0 errors in touched filesTests — zero new failures, 14 suites fixed
BuildNote on CI: the workflow run for this PR is held in |
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.
Refactor: Consolidate Delivery Controllers, Remove Unsafe Casts, Unify Query Contract & Extract Provider Interfaces
Overview
This PR implements four related backend refactoring issues in one branch, moving the SwiftChain backend toward the target layered architecture: single canonical controller per resource, typed service boundaries (no unsafe casts), one shared query contract for all list endpoints, and external SDK clients hidden behind interfaces injected via DI.
Related Issues
Closes #219
Closes #220
Closes #221
Closes #222
Changes
#219 — Collapse triple delivery controllers
getDeliveryETAhandler in the canonicalsrc/controllers/delivery.controller.ts.src/controllers/deliveryController.tsreduced to a re-export shim for backward compatibility.src/controllers/deliveryCrudController.ts(third duplicate implementation).src/di/tokens.ts+src/di/container.ts: removed tokens/registrations pointing at deleted controller.docs/architecture.mdupdated to reflect the single-controller layout.#220 — Remove
as unknown as/as anycastsUserDTO+toUserDTOtyped projection insrc/services/authService.ts(replacesas unknown asuser shaping).src/services/deliveryService.ts: typedAppErrorinstead of cast-laden error handling.src/controllers/notificationController.ts: zod parse instead ofas anybody access.src/repositories/BaseRepository.ts,src/repositories/DeliveryRepository.ts,src/services/userService.ts: casts removed with properly typed queries/projections (also dropped unusedIDeliveryimport).#221 — Query middleware rollout to all list endpoints
src/routes/delivery.routes.ts:buildQueryOptionsonGET /andGET /archived(sortablecreatedAt; filterablestatus,driver; searchabletrackingNumber,customer.name,customer.phone).src/routes/userRoutes.ts+src/controllers/userController.ts+src/services/userService.ts:GET /users/deletednow paginates/sorts/filters via the middleware (keeps legacy top-levelpaginationkey for API compatibility).src/routes/disputeRoutes.ts+src/controllers/disputeController.ts+src/services/disputeService.ts: admin list endpoint migrated; ObjectId-format validation preserved.src/routes/fleetRoutes.ts+src/controllers/fleetController.ts: fleet list migrated; legacypagination.pageskey replaced by the shared meta contract.src/routes/notificationRoutes.ts+src/controllers/notificationController.ts+src/services/notificationService.ts+src/repositories/NotificationRepository.ts+src/repositories/types.ts: history list migrated (zod query schema replaced by the middleware).src/routes/webhookRoutes.ts+src/controllers/webhookController.ts+src/services/webhookService.ts: merchant webhook list migrated from an unpaginated array to a paged, filterable result.src/routes/eventLogRoutes.ts+src/controllers/eventLogController.ts+src/services/eventLogService.ts:/unprocessednow paged (limit clamped to 100).docs/query-contract.md: parameters table, meta JSON shape, per-endpoint config matrix.tests/integration/queryContract.test.ts: 9 supertest-based integration tests against MongoMemoryServer covering all 7 endpoints — page/limit, whitelisted filters, search, sort, 400 on bad sort/page/operator, identical meta keys across endpoints, fleetspageskey removed, webhook/notification scoping.tests/delivery.test.ts,tests/dispute.test.ts,tests/disputeRoutes.test.ts,tests/notificationService.test.ts.src/middlewares/queryMiddleware.ts: addedflattenQueryFilterhelper for services that need to inspect whitelisted filter keys.#222 — Extract external SDK clients behind interfaces
src/services/providers/routingProvider.ts:IRoutingProvider(+Coordinates,RouteInfo,ETARequest,ETAResponse).src/services/providers/sorobanRpcClient.ts:ISorobanRpcClientmirroring therpc.Serversurface (getAccount, getHealth, getTransaction, getEvents, getLatestLedger, getNetwork, prepareTransaction, sendTransaction) with derived types.src/services/providers/imagesStorage.ts:IImagesStoragereusing the existing storage abstraction.src/services/providers/adapters.ts:GoogleMapsRoutingProvider(axios + Haversine fallback + FALLBACK_SPEEDS_KMH),SdkSorobanRpcClient(delegates 1:1 torpc.Server),defaultSorobanRpcClient.src/services/routingService.tsrewritten as a facade over the injected provider; consumers (deliveryService,poolingService) takeIRoutingProvidervia constructor.stellarService,soroban.service,transactionService,indexerService,escrowHandlersnow depend onISorobanRpcClient(setEscrowRpcClient()test seam in escrowHandlers);stellarService.send()returns a typedSendTransactionResultunion — the synthetic bad-seq response is typedRawSendTransactionResponsewithoutas unknown as.src/di/tokens.ts+src/di/container.ts:routingProvider,sorobanRpcClient,imagesStoragetokens registered.tests/providers.test.ts: 8 tests (Google Maps path/fallback/anti-meridian/error-wrap, SDK adapter delegation, routing facade, PoolingService with fake provider).Enabling bug fixes (required for #221's integration tests)
Two pre-existing upstream crashes made every app-booting test suite fail before any assertion could run:
src/middlewares/rateLimiter.ts:TEST_UNLIMITED_MAXwas referenced but never defined (upstream file uses it at lines 14/31) — addedconst TEST_UNLIMITED_MAX = 1_000_000;.src/routes/delivery.routes.ts:validateRequest(...)was called but never imported — changed tovalidate(...), matching the existing import.Verification Results
All commands run with the project's pnpm 9 toolchain (
node_modules/.bin/...directly):node_modules/.bin/tsc --noEmitenv/AWS keys insrc/config/env.ts)node_modules/.bin/eslint src --ext .tssrc/config/stellar.ts; 49 warnings pre-existing)node_modules/.bin/eslint tests --ext .tsCI=true MONGO_URI=... JWT_SECRET=... node_modules/.bin/jestnode_modules/.bin/tsc(build)--noEmit; no new build breakageNew/updated suites:
tests/providers.test.ts8/8 PASS,tests/integration/queryContract.test.ts9/9 PASS,tests/di.container.test.ts36 PASS,tests/delivery.test.tsPASS (was FAIL),tests/notificationService.test.tsPASS (was FAIL),tests/health.test.ts,tests/monitorRoutes.test.ts,tests/user.schema.hooks.test.ts,tests/transaction.escrowLock.test.ts,tests/auth.test.ts,tests/admin.test.ts,tests/security.test.tsetc. all PASS now (were baseline FAIL due to the two bug fixes).The 16 still-failing suites all fail at baseline too (verified with
commagainst the baseline FAIL list). They are pre-existing upstream drift, now merely visible because the suites can boot: e.g.dispute.test.ts/disputeRoutes.test.tsuse non-admin tokens against the admin-onlyGET /disputes(upstream gated it admin already),fleet.test.tsPOSTs{name}only while the Fleet model requirestreasuryAddress/businessMetadata,swagger.test.tsexpects docs for routes mounted upstream without annotations (/deliveries/fee-estimate,/pooling/*).Acceptance Criteria
#219 — Single canonical delivery controller
delivery.controller.tsused by routes and DI#220 — No unsafe casts
as unknown asremoved from auth, delivery, notification, repository, and user layersUserDTO/toUserDTO) replace cast-based shapingas anyin notification controller#221 — Shared query contract on every list endpoint
page,limit,sort, whitelisted filters,searchbuildPaginationMetameta structure returned by every endpointdocs/query-contract.mddocuments parameters, meta shape, per-endpoint config/users/deletedpagination)#222 — SDK clients behind interfaces
IRoutingProvider,ISorobanRpcClient,IImagesStoragedefined insrc/services/providers/GoogleMapsRoutingProvider,SdkSorobanRpcClient) isolated inadapters.tsroutingProvider,sorobanRpcClient,imagesStoragetokens)setEscrowRpcClient, constructor injection) — no SDK mocking gymnastics in suitesas unknown aseliminated from stellarService (typedRawSendTransactionResponse+ result union)General
/api/v1versioning unchangedas unknown ascasts with typed projections and DTO mapping #220 / Closes [Refactor] Roll out the query middleware consistently across all list endpoints #221 / Closes [Refactor] Extract external SDK clients behind interfaces for testability #222