GitNexus/gitnexus/test/unit/fastapi-router-bindings.test.ts
henry201605 b565c7c990
feat(ingestion): resolve FastAPI include_router(prefix=...) cross-file routes (#1877)
* feat(ingestion): resolve FastAPI include_router(prefix=...) cross-file routes

FastAPI sub-route files declare paths via @router.<verb> while the entry
file mounts the router with app.include_router(<router>, prefix='/x').
Previously both the ingestion-layer Route graph nodes and the group-layer
ExtractedContract URLs lost the cross-file prefix, breaking provider <->
consumer matching.

Ingestion layer:
  - parse-worker emits routerIncludes / routerImports + decoratorReceiver
  - parsing-processor / parse-impl thread the new fields and aggregate
    prefixesByModule across chunks; decorator routes whose receiver is
    'router' are duplicated once per matching prefix
  - routes.ts joins prefix via normalizeExtractedRoutePath

Group layer:
  - HttpLanguagePlugin gains an optional prepareRepo() pre-pass and a
    repoContext arg to scan(); python.ts builds prefixesByModule and
    falls back to the bare path when no entry matches
  - http-route-extractor caches one repoContext per plugin

Tests:
  - 3 new http-route-extractor cases (attr / named-import / no-prefix)
  - ParseWorkerResult literals in 3 test files updated to the new shape

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(ingestion,group): address PR #1877 review — relative imports, cross-package collisions, host names, ingestion tests

Follow-ups to the FastAPI `include_router(prefix=...)` cross-file fix
based on PR #1877's automated production-readiness review. Three
correctness gaps and one test coverage gap addressed:

1. Relative-import support in the worker regex (FINDING 2)
   `FROM_IMPORT_ROUTER_RE` now accepts module paths starting with a
   `.` (e.g. `from .calls import router as calls_router`). The
   previous `[A-Za-z_][\w.]*` rejected leading dots and silently
   dropped every relative-import Shape-B include — a real pattern
   from the PR description's own motivating example. The matching
   helpers now strip leading dots before keying so absolute and
   relative imports collapse to the same module key.

2. Cross-package same-name module collisions (FINDING 3)
   Two-tier module keying replaces the previous basename-only key:
     • short key — `users`            (file basename without `.py`)
     • long  key — `api/users`        (parent dir + stem)
   `prefixesByLongKey` is consulted first and only falls back to
   `prefixesByShortKey` when no long-key match is available. Both
   the ingestion pipeline (parse-impl.ts) and the group extractor
   (http-patterns/python.ts) carry the same scheme so the graph
   nodes and HTTP contracts agree on which prefix applies.

   New protocol field `ExtractedRouterModuleAlias` (parse-worker →
   parsing-processor → parse-impl) lets Shape-A
   `<host>.include_router(<mod>.router, prefix='/x')` calls promote
   to a long key when the same file imports `<mod>` via
   `from <pkg> import <mod>`. Without this, `api/users.py` and
   `admin/users.py` collided on the basename `users` and the admin
   file's routes inherited the `/users` prefix that was only meant
   for `api/users.py`.

3. Non-`app` host variable names (FINDING 4)
   The group-layer `INCLUDE_ROUTER_*_PATTERNS` queries pinned the
   host identifier to the literal `"app"` and dropped every
   `application = FastAPI()` / `api = FastAPI()` pattern — the
   constraint was redundant given that the call shape
   (`include_router` invoked with a router argument and a
   `prefix=` keyword) is already specific enough. The pin is
   removed; the ingestion regex was already unrestricted.

4. Ingestion-layer regression tests (FINDING 1)
   The previous PR added group-layer tests
   (`http-route-extractor.test.ts`) but zero in-tree tests for the
   ingestion path. Two new suites pin the
   worker → parse-impl → routes flow:

   - `test/unit/fastapi-router-bindings.test.ts` (23 cases):
     `extractFastAPIRouterBindings()` is split into a stand-alone
     module so it can be unit-tested without booting a worker
     thread, then pinned for regex shape, two-tier key emission,
     relative-import support, and negative cases.
   - `test/integration/fastapi-prefix-pipeline.test.ts` (5 cases)
     plus `test/fixtures/fastapi-prefix-app/` — runs the full
     `runPipelineFromRepo()` against a realistic multi-package
     fixture (containing both `api/users.py` and `admin/users.py`)
     and inspects the resulting `Route` graph nodes for cross-file
     prefix joining and absence of cross-package bleed.

Verification

  - `npx tsc --noEmit`: pass
  - PR-touched test suites (6 files / 117 cases): all green
  - `npx prettier --check`: pass on touched files
  - `npx eslint`: 0 errors on touched files

Cache / compatibility

  The new `routerModuleAliases?` field on `ParseWorkerResult` and
  `routerModuleAliases` on `WorkerExtractedData` are optional /
  guarded with `?? []`, so historical parse-cache entries continue
  to load without forced re-scan.

Refs PR #1877.

* refactor(ingestion): move fastapi-router-bindings out of workers/ — pure module, not a worker

Addresses @magyargergo's `CHANGES_REQUESTED` review on PR #1877:

> Sorry I just found that we are introducing a new worker in the PR.

`gitnexus/src/core/ingestion/workers/fastapi-router-bindings.ts` was a
**pure-function module** — it never imported `worker_threads` or
`parentPort`, never spawned a worker, and was never registered as a
worker entry. It was placed in `workers/` purely because it was split
out of `workers/parse-worker.ts` to make its functions unit-testable
without booting a worker thread (parse-worker is itself the worker
entry and cannot be loaded from the main thread).

To remove the misleading directory placement:

  • The implementation moves to
    `gitnexus/src/core/ingestion/route-extractors/fastapi-router-bindings.ts`,
    alongside the other framework-specific route extractors (`expo`,
    `nextjs`, `php`, `laravel`, `middleware`, `response-shapes`).
  • `workers/parse-worker.ts` keeps a thin re-export so the worker
    entry can keep using `extractFastAPIRouterBindings` directly. The
    re-export now carries an explicit comment stating that the imported
    file is **not** a worker and that the `workers/` directory
    deliberately hosts only true worker entries (`parse-worker.ts`,
    `worker-pool.ts`, `quarantine.ts`).
  • The new file's leading docstring opens with "NOT A WORKER" and
    explains why it exists where it does.
  • The unit test (`test/unit/fastapi-router-bindings.test.ts`) is
    updated to import from the new path.

No behaviour change. The function body, signatures, and exported types
are identical.

Verification

  • `npx tsc --noEmit`: pass
  • `npx tsc` (dist rebuild): pass
  • `test/unit/fastapi-router-bindings.test.ts` (23 cases): all green
  • `test/integration/fastapi-prefix-pipeline.test.ts` (5 cases): all green
  • `test/unit/group/http-route-extractor.test.ts` (63 cases): all green
  • `npx prettier --check` on touched files: pass
  • `npx eslint` on touched files: 0 errors

Refs PR #1877.

* refactor(ingestion): drop parse-worker re-exports; consumers import router types directly from route-extractors

Addresses @magyargergo's two remaining review comments on PR #1877:

1. **`gitnexus/src/core/ingestion/workers/parse-worker.ts:247`** —
   "Can you please remove them and update the call sites?"

   The `export type { ExtractedRouterInclude, ExtractedRouterImport,
   ExtractedRouterModuleAlias } from '../route-extractors/...'` block
   in parse-worker.ts is gone. The remaining `import type {…}` is
   purely local — used only to type the corresponding fields on
   `ParseWorkerResult` below — and the leading comment now says so
   explicitly ("this file does NOT re-export them"). The
   `extractFastAPIRouterBindings` symbol is also no longer re-exported
   from parse-worker.ts; it's still imported here so the worker entry
   can call it per file, but downstream consumers must reach it via
   `route-extractors/fastapi-router-bindings` directly.

   Call sites updated:
     - `gitnexus/src/core/ingestion/parsing-processor.ts`
     - `gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts`

   Both files now `import type { ExtractedRouterInclude,
   ExtractedRouterImport, ExtractedRouterModuleAlias }` directly from
   `route-extractors/fastapi-router-bindings.js`. The worker types
   they still need (`ParseWorkerResult`, `ExtractedToolDef`, etc.)
   keep coming from `workers/parse-worker.js`.

   The unit + integration tests already imported from the new path,
   so no test changes were required.

2. **`gitnexus/src/core/ingestion/parsing-processor.ts:168`** —
   suggested simplification:

       for (const item of result.routerIncludes ?? []) allRouterIncludes.push(item);
       for (const item of result.routerImports ?? []) allRouterImports.push(item);
       for (const item of result.routerModuleAliases ?? []) allRouterModuleAliases.push(item);

   Applied verbatim. Replaces the previous `if (result.…) for …`
   guards. The cache-compat semantics are unchanged — historical
   parse-cache entries that lack these fields still load cleanly,
   the new form just spells the fallback inline.

No behavior change, no tests touched, no public API change.

Verification

  • `npx tsc --noEmit`: pass
  • `npx tsc` (dist rebuild): pass
  • PR-touched test suites (6 files / 117 cases): all green
  • `npx prettier --check` on touched files: pass
  • `npx eslint` on touched files: 0 errors

Refs PR #1877.

* refactor(ingestion): hoist fastapi-router-bindings type imports to top of parse-worker.ts

Move the `import type { ExtractedRouterInclude, ExtractedRouterImport,
ExtractedRouterModuleAlias }` block to the top of the file with the
other type imports, and drop the comment that previously sat next to
ExtractedDecoratorRoute.

---------

Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-05-28 19:04:19 +01:00

287 lines
10 KiB
TypeScript

/**
* Unit tests for {@link extractFastAPIRouterBindings} — the per-file
* regex extractor that the parse worker calls on every Python file.
* The cross-file aggregation that turns these raw records into prefix
* maps lives in parse-impl and is covered by
* `fastapi-prefix-pipeline.test.ts` (integration) plus
* `http-route-extractor.test.ts` (group layer). This file pins the
* shape the worker emits, so a regression in either regex or in the
* import-list parsing fails here first.
*
* What this file is responsible for:
* • Shape A `app.include_router(<mod>.router, prefix=…)` and
* Shape B `app.include_router(<local>, prefix=…)` are both
* captured.
* • `<host>.include_router` matches any host name, not just `app`.
* • Module path keying is two-tiered: short basename (always) and
* long `<parent>/<stem>` key (whenever the import path was
* multi-segment).
* • Relative imports (`from .calls import …`,
* `from ..siblings.calls import …`) are captured.
* • `as`-aliased imports route the prefix to the alias, not to
* `router`.
* • Nothing is emitted when `include_router` is absent or has no
* `prefix=` keyword.
*/
import { describe, it, expect } from 'vitest';
import {
extractFastAPIRouterBindings,
lastDottedSegment,
lastTwoSegmentsAsPath,
type ExtractedRouterInclude,
type ExtractedRouterImport,
} from '../../src/core/ingestion/route-extractors/fastapi-router-bindings.js';
function run(filePath: string, content: string) {
const includes: ExtractedRouterInclude[] = [];
const imports: ExtractedRouterImport[] = [];
extractFastAPIRouterBindings(filePath, content, includes, imports);
return { includes, imports };
}
describe('lastDottedSegment', () => {
it('returns the last segment of an absolute dotted path', () => {
expect(lastDottedSegment('api.users')).toBe('users');
expect(lastDottedSegment('api.v2.users')).toBe('users');
});
it('strips leading dots from a relative path', () => {
expect(lastDottedSegment('.users')).toBe('users');
expect(lastDottedSegment('..api.users')).toBe('users');
expect(lastDottedSegment('...users')).toBe('users');
});
it('returns the input when there is no dot after stripping', () => {
expect(lastDottedSegment('users')).toBe('users');
});
it('returns the empty string for pure-dot inputs', () => {
expect(lastDottedSegment('.')).toBe('');
expect(lastDottedSegment('..')).toBe('');
expect(lastDottedSegment('...')).toBe('');
});
});
describe('lastTwoSegmentsAsPath', () => {
it('joins the last two segments with `/`', () => {
expect(lastTwoSegmentsAsPath('api.users')).toBe('api/users');
expect(lastTwoSegmentsAsPath('app.api.users')).toBe('api/users');
});
it('strips leading dots before joining', () => {
expect(lastTwoSegmentsAsPath('..api.users')).toBe('api/users');
});
it('returns the empty string when the path has only one segment', () => {
// Single-segment imports cannot be promoted to a long key.
expect(lastTwoSegmentsAsPath('users')).toBe('');
expect(lastTwoSegmentsAsPath('.users')).toBe('');
});
it('returns the empty string for pure-dot inputs', () => {
expect(lastTwoSegmentsAsPath('.')).toBe('');
expect(lastTwoSegmentsAsPath('..')).toBe('');
});
});
describe('extractFastAPIRouterBindings — Shape A (`<mod>.router`)', () => {
it('captures app.include_router(<mod>.router, prefix=…)', () => {
const { includes } = run(
'main.py',
[
'from fastapi import FastAPI',
'from api import users',
'app = FastAPI()',
"app.include_router(users.router, prefix='/users', tags=['users'])",
'',
].join('\n'),
);
expect(includes).toHaveLength(1);
expect(includes[0]).toMatchObject({
filePath: 'main.py',
routerExpr: 'users.router',
prefix: '/users',
});
// Line number is 1-indexed and points to the include_router call.
expect(includes[0].lineNumber).toBe(4);
});
it('captures non-`app` host variables', () => {
// FINDING 4: production code commonly uses `api`, `application`,
// `asgi_app` etc. Pinning the regex to `app.` would silently drop
// these, which used to leave the ingestion and group layers
// disagreeing on whether a prefix was applied.
const { includes } = run(
'main.py',
[
'from fastapi import FastAPI',
'from api import users',
'api = FastAPI()',
"api.include_router(users.router, prefix='/users')",
'',
].join('\n'),
);
expect(includes).toHaveLength(1);
expect(includes[0].routerExpr).toBe('users.router');
expect(includes[0].prefix).toBe('/users');
});
it('captures multiple Shape-A includes in the same file', () => {
const { includes } = run(
'main.py',
[
'from api import users, calls',
'app = FastAPI()',
"app.include_router(users.router, prefix='/users')",
"app.include_router(calls.router, prefix='/calls')",
'',
].join('\n'),
);
expect(includes).toHaveLength(2);
expect(includes.map((i) => i.routerExpr).sort()).toEqual(['calls.router', 'users.router']);
});
});
describe('extractFastAPIRouterBindings — Shape B (bare local name)', () => {
it('captures app.include_router(<local>, prefix=…) and the import', () => {
const { includes, imports } = run(
'main.py',
[
'from fastapi import FastAPI',
'from api.users import router as users_router',
'app = FastAPI()',
"app.include_router(users_router, prefix='/users')",
'',
].join('\n'),
);
expect(imports).toHaveLength(1);
expect(imports[0]).toMatchObject({
filePath: 'main.py',
localName: 'users_router',
moduleKey: 'users',
moduleKeyLong: 'api/users',
});
expect(includes).toHaveLength(1);
expect(includes[0]).toMatchObject({
filePath: 'main.py',
routerExpr: 'users_router',
prefix: '/users',
});
});
it('captures the unaliased shape `from <mod> import router`', () => {
const { imports } = run('main.py', ['from api.users import router', ''].join('\n'));
expect(imports).toHaveLength(1);
expect(imports[0]).toMatchObject({
localName: 'router',
moduleKey: 'users',
moduleKeyLong: 'api/users',
});
});
it('does NOT re-capture Shape A as Shape B (`<mod>.router` is not bare)', () => {
// Anti-regression: INCLUDE_ROUTER_NAME_RE is intentionally
// permissive (`(identifier)`). Without the lookahead in
// extractFastAPIRouterBindings it would re-capture the bare
// module name `users` from `users.router` and add a phantom
// include with `routerExpr: "users"`.
const { includes } = run(
'main.py',
["app.include_router(users.router, prefix='/users')", ''].join('\n'),
);
const shapes = includes.map((i) => i.routerExpr).sort();
expect(shapes).toEqual(['users.router']);
});
});
describe('extractFastAPIRouterBindings — relative imports', () => {
it('captures single-dot relative imports (`from .calls import router as …`)', () => {
// FINDING 2: the previous regex `[A-Za-z_][\w.]*` rejected
// module paths starting with `.`, silently dropping every
// relative-import Shape-B include. The PR description's own
// motivating example used this shape — now pinned.
const { imports } = run(
'main.py',
['from .calls import router as calls_router', ''].join('\n'),
);
expect(imports).toHaveLength(1);
expect(imports[0]).toMatchObject({
localName: 'calls_router',
moduleKey: 'calls',
});
// Single-segment relative paths cannot be promoted to a long key.
expect(imports[0].moduleKeyLong).toBeUndefined();
});
it('captures multi-segment relative imports and emits a long key', () => {
const { imports } = run(
'main.py',
['from ..api.users import router as users_router', ''].join('\n'),
);
expect(imports).toHaveLength(1);
expect(imports[0]).toMatchObject({
localName: 'users_router',
moduleKey: 'users',
moduleKeyLong: 'api/users',
});
});
});
describe('extractFastAPIRouterBindings — long-key precision', () => {
it('emits long key `api/users` for a multi-segment absolute import', () => {
// FINDING 3: short-key-only collides for `api/users.py` vs
// `admin/users.py`. The long key gives parse-impl the precision
// it needs to bind a Shape-B include to the right file.
const { imports } = run('main.py', ['from api.users import router', ''].join('\n'));
expect(imports[0].moduleKeyLong).toBe('api/users');
});
it('omits the long key for a single-segment top-level import', () => {
const { imports } = run('main.py', ['from users import router', ''].join('\n'));
expect(imports[0].moduleKey).toBe('users');
expect(imports[0].moduleKeyLong).toBeUndefined();
});
});
describe('extractFastAPIRouterBindings — negative cases', () => {
it('emits nothing for files without any include_router or import', () => {
const { includes, imports } = run('helpers.py', 'def add(a, b):\n return a + b\n');
expect(includes).toEqual([]);
expect(imports).toEqual([]);
});
it('does not capture include_router calls without a prefix= keyword', () => {
const { includes } = run(
'main.py',
['app.include_router(users.router, tags=["users"])', ''].join('\n'),
);
expect(includes).toEqual([]);
});
it('does not capture include_router calls with a non-string prefix', () => {
// The current regex requires a string literal for the prefix
// value. Variables / f-strings / concatenations are not
// resolvable at parse time.
const { includes } = run(
'main.py',
['app.include_router(users.router, prefix=PREFIX_USERS)', ''].join('\n'),
);
expect(includes).toEqual([]);
});
it('ignores non-router names in `from … import` lists', () => {
const { imports } = run('main.py', ['from api.users import schemas, helpers', ''].join('\n'));
expect(imports).toEqual([]);
});
it('correctly handles a mixed import list (router + others)', () => {
const { imports } = run(
'main.py',
['from api.users import router, schemas, helpers', ''].join('\n'),
);
expect(imports).toHaveLength(1);
expect(imports[0].localName).toBe('router');
expect(imports[0].moduleKey).toBe('users');
});
});