Skip to content

refactor(api): replace inline SQL with modular loader registry#1217

Merged
pyramation merged 2 commits into
mainfrom
feat/api-use-module-loaders
May 23, 2026
Merged

refactor(api): replace inline SQL with modular loader registry#1217
pyramation merged 2 commits into
mainfrom
feat/api-use-module-loaders

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

@pyramation pyramation commented May 23, 2026

Summary

Wires graphql/server/src/middleware/api.ts to use the loader registry from PR #1215 (@constructive-io/express-context) for all per-database module resolution.

What changed

api.ts (1035 → 603 lines, -430 lines):

  • resolveApiNameHeader and resolveDomainLookup now call resolveModuleSettings() which delegates to the loader registry's resolve() method for each module
  • Removed 17 SQL constants (RLS_SETTINGS_SQL, CORS_SETTINGS_SQL, DATABASE_SETTINGS_SQL, AUTH_SETTINGS_DISCOVERY_SQL, AUTH_SETTINGS_SQL, etc.)
  • Removed 9 row/module interfaces (RlsModuleData, AuthSettingsRow, CorsSettingsRow, PubkeySettingsRow, WebauthnSettingsRow, DatabaseSettingsRow, etc.)
  • Removed 12 query/transform functions (queryRlsModule, queryCorsOrigins, queryDatabaseSettings, queryAuthSettings, toRlsModule, toRlsModuleFromSettings, toAuthSettings, toPubkeyChallengeSettings, etc.)
  • All of this logic now lives in the individual loader modules in express-context/src/loaders/

graphql/server/src/types.ts (131 → 20 lines):

  • Re-exports all shared types from @constructive-io/express-context instead of duplicating definitions
  • Only ApiOptions (local to graphql-server) remains defined here

express-context/src/loaders/create-loader.ts (cache key fix):

  • LRU cache key is now databaseId:apiId (composite) instead of just databaseId
  • This is critical for loaders like databaseSettings and corsOrigins where per-API overrides (api_settings) can differ for the same database
  • Invalidation by databaseId clears all entries for that database regardless of apiId

express-context/README.md:

  • Added centered logo, CI badge, license badge, and npm version badge matching other packages in the monorepo

Zero breaking changes

Before After Status
req.api.rlsModule req.api.rlsModule Unchanged
req.api.authSettings req.api.authSettings Unchanged
req.api.corsOrigins req.api.corsOrigins Unchanged
req.api.databaseSettings req.api.databaseSettings Unchanged
req.api.pubkeyChallengeSettings req.api.pubkeyChallengeSettings Unchanged
req.api.webauthnSettings req.api.webauthnSettings Unchanged
svcCache per-domain caching svcCache per-domain caching Unchanged
import { RlsModule } from '../types' import { RlsModule } from '../types' Re-exported

ApiStructure fields are still populated identically. Every downstream consumer (auth.ts, cookie.ts, cors.ts, graphile.ts, upload.ts) continues to import from ../types and gets the same types.

Architecture

Request → api.ts (domain/API row lookup) 
        → resolveModuleSettings(registry, loaderCtx)
            → registry.resolve('rlsModule', ctx)        // express-context/loaders/rls.ts
            → registry.resolve('corsOrigins', ctx)       // express-context/loaders/cors.ts
            → registry.resolve('databaseSettings', ctx)  // express-context/loaders/database-settings.ts
            → registry.resolve('authSettings', ctx)      // express-context/loaders/auth-settings.ts
            → registry.resolve('pubkeyChallengeSettings', ctx)  // express-context/loaders/pubkey.ts
            → registry.resolve('webauthnSettings', ctx)  // express-context/loaders/webauthn.ts
        → toApiStructure(row, opts, settings) → svcCache

Each loader has its own per-databaseId:apiId LRU cache. When svcCache misses but loaders have cached data, no SQL is re-executed.

Review & Testing Checklist for Human

  • Query equivalence: Diff the SQL in each loader (express-context/src/loaders/*.ts) against the removed SQL from api.ts to confirm they're functionally identical. The loaders were written to mirror api.ts exactly, but a review is warranted.
  • Type re-export correctness: Verify that import { RlsModule } from '../types' in auth.ts, upload.ts, cookie.ts, etc. still resolves correctly through the re-export chain (graphql/server/types.tsexpress-context/types.ts)
  • Auth flow integrity: The critical chain is api.ts → auth.ts (reads req.api.rlsModule) → pgSettings. Confirm auth still works end-to-end with a real database.

Notes

  • The initial CI run caught a real bug: the loader cache was keyed only by databaseId, which caused stale hits when two APIs on the same database had different api_settings overrides. Fixed by using composite databaseId:apiId keys. The upload integration test ("Bob restricted API: uploadAppFile NOT exposed") now passes.
  • graphql/server has 6 pre-existing build errors in graphile.ts, llm-api.ts, and upload.ts related to unresolved workspace deps (graphile-settings, graphile-llm). These exist identically on main.
  • The isDev helper was removed from api.ts as it was already unused — getNodeEnv() is called directly in buildDevFallbackError.

Link to Devin session: https://app.devin.ai/sessions/151753dc357b48528652af31a63decc0
Requested by: @pyramation

Wire api.ts to use the express-context loader registry for all
per-database module resolution. This removes ~430 lines of inline
SQL queries, row types, query functions, and transform helpers that
are now encapsulated in individual loader modules.

Changes:
- api.ts: resolveApiNameHeader + resolveDomainLookup now call
  resolveModuleSettings() which delegates to the loader registry
- api.ts: removed 17 SQL constants, 9 row/module interfaces,
  12 query/transform functions — all now live in express-context loaders
- graphql/server/src/types.ts: re-exports from express-context instead
  of duplicating type definitions
- express-context README: added logo + CI badge + npm version badge to
  match other packages in the monorepo

Zero breaking changes: ApiStructure fields (rlsModule, authSettings,
corsOrigins, etc.) are still populated identically. req.api.rlsModule
continues to work. svcCache still caches the full ApiStructure by
domain key.

api.ts: 1035 → 603 lines
types.ts: 131 → 20 lines
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@blacksmith-sh

This comment has been minimized.

The LRU cache was keyed only by databaseId, causing incorrect cache
hits when multiple APIs share the same database but have different
api_settings overrides (e.g. per-API feature flag gating).

Now the cache key includes apiId when present:
  databaseId:apiId  (when apiId is available)
  databaseId        (when apiId is not available)

Invalidation by databaseId clears all entries for that database
regardless of apiId.
@pyramation pyramation merged commit 84b3752 into main May 23, 2026
37 checks passed
@pyramation pyramation deleted the feat/api-use-module-loaders branch May 23, 2026 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant