fix(query-graphql): add @Parent() decorator to resolveReference for Federation #410 #414
Conversation
This example successfully reproduces the bug where @ResolveReference() receives undefined for the representation parameter when @context() and @InjectDataLoaderConfig() decorators are present. Test environment includes: - user-service: User entity with referenceBy config - todo-service: TodoItem with @reference to User - gateway: Apollo Gateway composing federated schema - PostgreSQL database The bug is triggered when creating a TodoItem with assigneeId and requesting the assignee field, causing: 'Cannot read properties of undefined (reading id)' Issue: TriPSs#410
- Add tag-service with UUID primary key to test string ID type - Update todo-service to reference both User (numeric ID) and Tag (UUID ID) - Update gateway to include tag-service subgraph - Update README with fix documentation and expected results - Add seeder data for tags with fixed UUIDs This validates the fix for issue TriPSs#410 works with different ID types.
…ederation TriPSs#410 - Add @parent() decorator to representation parameter in reference.resolver.ts Without this decorator, NestJS does not inject the representation object - Use String(id) for Map key comparison in reference.loader.ts Federation representation.id may be number while entity.id is string (or vice versa)
- Add e2e-test service with Jest test suite - Validate Federation reference resolution - Test numeric ID (User) and UUID string ID (Tag) - Verify issue TriPSs#410 fix works correctly - Use docker compose profile 'test' for test runner
- Run Federation V2 E2E tests in CI - Use docker/setup-compose-action to ensure Docker Compose available - Validates issue TriPSs#410 fix with Docker Compose - Shows logs on failure for debugging
- Add true Federation integration test that simulates Gateway requests - Use ApolloFederationDriver to create real Federation subgraph - Add @directive('@key(fields: "id")') to DTOs for Federation entity support - Test cases cover: - Single entity reference resolution (Issue TriPSs#410 validation) - Multiple entities batching (DataLoader) - Mixed entity types in single query - ID type handling (number vs string) - Error cases (non-existent, missing key) - SDL validation with @key directive Related to: Issue TriPSs#410
WalkthroughAdds a Federation V2 end-to-end test environment (multi-service Docker Compose, gateway and subgraphs, E2E runner) and fixes federation reference resolution by annotating the resolveReference representation with Changes
sequenceDiagram
participant Client
participant Gateway
participant TodoSvc as Todo Service
participant UserSvc as User Service
participant TagSvc as Tag Service
participant DB as PostgreSQL
Client->>Gateway: Query todoItems (includes assignee/tag)
Gateway->>TodoSvc: Fetch todoItems
TodoSvc->>DB: SELECT todo_items
DB-->>TodoSvc: rows
TodoSvc-->>Gateway: TodoItems with assigneeId/tagId
Gateway->>UserSvc: _entities (representations for User)
note right of UserSvc `#lightblue`: resolveReference receives representation via `@Parent`()\nLoader lookups use String(id)
UserSvc->>DB: SELECT users WHERE id IN (...)
DB-->>UserSvc: rows
UserSvc-->>Gateway: Resolved User objects
Gateway->>TagSvc: _entities (representations for Tag)
note right of TagSvc `#lightblue`: resolveReference receives representation via `@Parent`()\nUUIDs coerced to strings for lookup
TagSvc->>DB: SELECT tags WHERE id IN (...)
DB-->>TagSvc: rows
TagSvc-->>Gateway: Resolved Tag objects
Gateway-->>Client: Final response with nested assignee & tag
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit edf9fc4
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
examples/federation-v2-e2e/user-service/Dockerfile (3)
30-30: Consider building only required packages instead of--all.Line 30 uses
yarn nx run-many --target=build --all, which builds every package in the monorepo. For this user-service image, onlycore,query-graphql, andquery-typeormare needed. Building only these packages could reduce build time and image size.Apply this diff to reduce build scope:
-RUN yarn nx run-many --target=build --all +RUN yarn nx run-many --target=build --projects=@ptc-org/nestjs-query-core,@ptc-org/nestjs-query-graphql,@ptc-org/nestjs-query-typeormNote: Verify the project names match your nx.json configuration.
56-59: Consolidate npm install commands for clarity.The three separate
npm installcommands in stage 2 can be consolidated into a single invocation for better readability and atomicity.Apply this diff to consolidate the install commands:
-RUN npm install ./packages/ptc-org-nestjs-query-core-*.tgz \ - ./packages/ptc-org-nestjs-query-graphql-*.tgz \ - ./packages/ptc-org-nestjs-query-typeorm-*.tgz \ - && npm install +RUN npm install ./packages/ptc-org-nestjs-query-core-*.tgz \ + ./packages/ptc-org-nestjs-query-graphql-*.tgz \ + ./packages/ptc-org-nestjs-query-typeorm-*.tgzThis removes the separate
npm installcall (the dependencies specified in package.json are installed along with the tarballs).
45-73: Consider running the application as a non-root user.For improved security posture, create a non-root user in stage 2 and run the application with that user.
Apply this diff to add a non-root user:
FROM node:22 WORKDIR /app + +# Create a non-root user to run the application +RUN useradd -m -u 1001 appuser # Copy built packages from builder stage COPY --from=builder /build/dist/*.tgz ./packages/ @@ -68,6 +73,8 @@ RUN npm run build # Expose port EXPOSE 3000 +# Run as non-root user +USER appuser + # Start the service CMD ["node", "dist/main.js"]Note: Ensure file ownership is set correctly (e.g.,
chown -R appuser:appuser /app) after copying files if needed.examples/federation-v2-e2e/todo-service/Dockerfile (1)
56-59: Minor optimization: combine npm install commands.Lines 56–59 perform two separate npm install operations (first for local tarballs, then a global npm install). Consider combining these into a single command to reduce image layer count and build time.
-RUN npm install ./packages/ptc-org-nestjs-query-core-*.tgz \ - ./packages/ptc-org-nestjs-query-graphql-*.tgz \ - ./packages/ptc-org-nestjs-query-typeorm-*.tgz \ - && npm install +RUN npm install ./packages/ptc-org-nestjs-query-core-*.tgz \ + ./packages/ptc-org-nestjs-query-graphql-*.tgz \ + ./packages/ptc-org-nestjs-query-typeorm-*.tgzNote: The second
npm install(without arguments) is redundant if the service'spackage.jsonlists no additional production dependencies beyond the local packages.examples/federation-v2-e2e/init-scripts/01-create-databases.sql (1)
1-12: Consider dropping SUPERUSER for test rolesFor an isolated Dockerized e2e environment this is acceptable, but
WITH SUPERUSERis more powerful than needed. You can generally create non-superusers and make them owners of their respective databases (or grant needed privileges) to follow least-privilege, even in tests.examples/federation-v2-e2e/tag-service/src/main.ts (1)
6-13: Consider adding error handling to the bootstrap function.The bootstrap function lacks error handling, which could result in uncaught promise rejections if the application fails to start. For test infrastructure, this might be acceptable, but adding basic error handling would improve debuggability.
Consider applying this diff:
async function bootstrap() { const app = await NestFactory.create(AppModule) const port = process.env.PORT || 3000 await app.listen(port) Logger.log(`Tag service running on port ${port}`) } -bootstrap() +bootstrap().catch((err) => { + Logger.error('Failed to start tag service', err) + process.exit(1) +})examples/federation-v2-e2e/gateway/src/main.ts (2)
8-8: Consider using an environment variable for the port.The gateway uses a hardcoded port (3000) while the tag-service uses
process.env.PORT || 3000. For consistency across the e2e environment and deployment flexibility, consider using an environment variable here as well.Apply this diff:
- await app.listen(3000) - console.log('Apollo Gateway running on port 3000') + const port = process.env.PORT || 3000 + await app.listen(port) + console.log(`Apollo Gateway running on port ${port}`)
5-12: Consider adding error handling to the bootstrap function.Similar to the tag-service, the bootstrap function lacks error handling. Adding basic error handling would improve debuggability if the gateway fails to start.
Consider applying this diff:
-void bootstrap() +void bootstrap().catch((err) => { + console.error('Failed to start gateway', err) + process.exit(1) +})examples/federation-v2-e2e/tag-service/package.json (1)
1-39: Consider marking the tag-service package as privateSince this looks like an internal E2E/example package, you may want to add
"private": trueto avoid accidental publishing to npm; everything else in the manifest looks fine for its purpose.{ "name": "tag-service", "version": "1.0.0", + "private": true, "main": "dist/main.js", "scripts": {examples/federation-v2-e2e/gateway/tsconfig.json (1)
1-23: TS config is fine; optionally consider sharing settings via a base configThese compiler options are reasonable for the gateway and should work as expected. If the repo already has a root/base tsconfig, you might consider using
extendshere to keep compiler settings consistent across services and reduce duplication, but it’s not required.examples/federation-v2-e2e/docker-compose.yml (1)
1-127: Compose topology is well-structured; just ensure healthcheck tooling matches your imagesThe service graph (Postgres → subgraphs → gateway → e2e-test) and healthcheck-based
depends_onwiring look good for reliably running the federation E2E tests. One thing to double-check is that each built service image includescurl; otherwise the GraphQL-based healthchecks will fail even if the app is healthy. If any image lackscurl, consider installing it there or switching the healthchecks to a simpler built-in mechanism (e.g., Node-based HTTP check or another available CLI).packages/query-graphql/__tests__/integration/federation-n1/federation-reference.spec.ts (1)
302-305: Consider using a more flexible error assertion.The assertion
toContain('Unable to find TodoList')assumes a specific error message format. If the error message changes, this test will break. Consider checking for a more stable indicator (e.g., error code, or simply that an error exists and the data is null/incomplete).That said, this is acceptable for an integration test validating expected behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
examples/federation-v2-e2e/e2e-test/package-lock.jsonis excluded by!**/package-lock.jsonexamples/federation-v2-e2e/gateway/package-lock.jsonis excluded by!**/package-lock.jsonexamples/federation-v2-e2e/tag-service/package-lock.jsonis excluded by!**/package-lock.jsonexamples/federation-v2-e2e/todo-service/package-lock.jsonis excluded by!**/package-lock.jsonexamples/federation-v2-e2e/user-service/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (58)
.github/workflows/test.yml(1 hunks)examples/federation-v2-e2e/.dockerignore(1 hunks)examples/federation-v2-e2e/.gitignore(1 hunks)examples/federation-v2-e2e/README.md(1 hunks)examples/federation-v2-e2e/docker-compose.yml(1 hunks)examples/federation-v2-e2e/e2e-test/Dockerfile(1 hunks)examples/federation-v2-e2e/e2e-test/e2e/federation.spec.ts(1 hunks)examples/federation-v2-e2e/e2e-test/jest.config.js(1 hunks)examples/federation-v2-e2e/e2e-test/package.json(1 hunks)examples/federation-v2-e2e/e2e-test/tsconfig.json(1 hunks)examples/federation-v2-e2e/gateway/Dockerfile(1 hunks)examples/federation-v2-e2e/gateway/nest-cli.json(1 hunks)examples/federation-v2-e2e/gateway/package.json(1 hunks)examples/federation-v2-e2e/gateway/src/app.module.ts(1 hunks)examples/federation-v2-e2e/gateway/src/main.ts(1 hunks)examples/federation-v2-e2e/gateway/tsconfig.json(1 hunks)examples/federation-v2-e2e/init-scripts/01-create-databases.sql(1 hunks)examples/federation-v2-e2e/tag-service/Dockerfile(1 hunks)examples/federation-v2-e2e/tag-service/nest-cli.json(1 hunks)examples/federation-v2-e2e/tag-service/package.json(1 hunks)examples/federation-v2-e2e/tag-service/src/app.module.ts(1 hunks)examples/federation-v2-e2e/tag-service/src/main.ts(1 hunks)examples/federation-v2-e2e/tag-service/src/tag/tag.dto.ts(1 hunks)examples/federation-v2-e2e/tag-service/src/tag/tag.entity.ts(1 hunks)examples/federation-v2-e2e/tag-service/src/tag/tag.input.ts(1 hunks)examples/federation-v2-e2e/tag-service/src/tag/tag.module.ts(1 hunks)examples/federation-v2-e2e/tag-service/src/tag/tag.seeder.ts(1 hunks)examples/federation-v2-e2e/tag-service/tsconfig.json(1 hunks)examples/federation-v2-e2e/todo-service/Dockerfile(1 hunks)examples/federation-v2-e2e/todo-service/nest-cli.json(1 hunks)examples/federation-v2-e2e/todo-service/package.json(1 hunks)examples/federation-v2-e2e/todo-service/src/app.module.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/main.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/tag-reference.dto.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.entity.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.input.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.module.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.seeder.ts(1 hunks)examples/federation-v2-e2e/todo-service/src/todo-item/user-reference.dto.ts(1 hunks)examples/federation-v2-e2e/todo-service/tsconfig.json(1 hunks)examples/federation-v2-e2e/user-service/Dockerfile(1 hunks)examples/federation-v2-e2e/user-service/nest-cli.json(1 hunks)examples/federation-v2-e2e/user-service/package.json(1 hunks)examples/federation-v2-e2e/user-service/src/app.module.ts(1 hunks)examples/federation-v2-e2e/user-service/src/main.ts(1 hunks)examples/federation-v2-e2e/user-service/src/user/user.dto.ts(1 hunks)examples/federation-v2-e2e/user-service/src/user/user.entity.ts(1 hunks)examples/federation-v2-e2e/user-service/src/user/user.input.ts(1 hunks)examples/federation-v2-e2e/user-service/src/user/user.module.ts(1 hunks)examples/federation-v2-e2e/user-service/src/user/user.seeder.ts(1 hunks)examples/federation-v2-e2e/user-service/tsconfig.json(1 hunks)packages/query-graphql/__tests__/integration/federation-n1/dtos/todo-item.dto.ts(1 hunks)packages/query-graphql/__tests__/integration/federation-n1/dtos/todo-list.dto.ts(1 hunks)packages/query-graphql/__tests__/integration/federation-n1/federation-reference.spec.ts(1 hunks)packages/query-graphql/__tests__/integration/federation-n1/fixtures/test-data.ts(1 hunks)packages/query-graphql/src/loader/reference.loader.ts(1 hunks)packages/query-graphql/src/resolvers/reference.resolver.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (18)
examples/federation-v2-e2e/todo-service/src/todo-item/tag-reference.dto.ts (2)
examples/federation-v2-e2e/tag-service/src/tag/tag.dto.ts (1)
ObjectType(4-22)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts (1)
ObjectType(7-38)
examples/federation-v2-e2e/user-service/src/app.module.ts (4)
examples/federation-v2-e2e/gateway/src/app.module.ts (1)
Module(6-22)examples/federation-v2-e2e/user-service/src/user/user.module.ts (1)
Module(11-32)examples/federation-v2-e2e/todo-service/src/app.module.ts (1)
Module(8-29)tools/actions/set-master-vars/src/index.js (1)
process(4-4)
examples/federation-v2-e2e/user-service/src/user/user.seeder.ts (1)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.seeder.ts (1)
Injectable(36-63)
examples/federation-v2-e2e/tag-service/src/tag/tag.module.ts (2)
examples/federation-v2-e2e/gateway/src/app.module.ts (1)
Module(6-22)examples/federation-v2-e2e/tag-service/src/app.module.ts (1)
Module(8-29)
examples/federation-v2-e2e/todo-service/src/todo-item/user-reference.dto.ts (2)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts (1)
ObjectType(7-38)examples/federation-v2-e2e/user-service/src/user/user.dto.ts (1)
ObjectType(4-21)
examples/federation-v2-e2e/tag-service/src/tag/tag.entity.ts (1)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.entity.ts (1)
Entity(3-29)
examples/federation-v2-e2e/tag-service/src/tag/tag.dto.ts (2)
examples/federation-v2-e2e/todo-service/src/todo-item/tag-reference.dto.ts (1)
ObjectType(8-13)examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts (1)
ObjectType(7-38)
examples/federation-v2-e2e/user-service/src/user/user.input.ts (1)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.input.ts (2)
InputType(3-16)InputType(18-31)
examples/federation-v2-e2e/tag-service/src/app.module.ts (3)
examples/federation-v2-e2e/gateway/src/app.module.ts (1)
Module(6-22)examples/federation-v2-e2e/tag-service/src/tag/tag.module.ts (1)
Module(11-31)tools/actions/set-master-vars/src/index.js (1)
process(4-4)
packages/query-graphql/__tests__/integration/federation-n1/federation-reference.spec.ts (1)
packages/query-graphql/__tests__/integration/federation-n1/fixtures/test-data.ts (1)
createTestData(7-35)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.entity.ts (1)
examples/federation-v2-e2e/tag-service/src/tag/tag.entity.ts (1)
Entity(3-20)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.module.ts (1)
examples/federation-v2-e2e/user-service/src/user/user.module.ts (1)
Module(11-32)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.seeder.ts (2)
examples/federation-v2-e2e/tag-service/src/tag/tag.seeder.ts (1)
Injectable(15-42)examples/federation-v2-e2e/user-service/src/user/user.seeder.ts (1)
Injectable(14-41)
examples/federation-v2-e2e/user-service/src/user/user.entity.ts (1)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.entity.ts (1)
Entity(3-29)
examples/federation-v2-e2e/tag-service/src/tag/tag.seeder.ts (1)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.seeder.ts (1)
Injectable(36-63)
examples/federation-v2-e2e/user-service/src/user/user.dto.ts (2)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts (1)
ObjectType(7-38)examples/federation-v2-e2e/todo-service/src/todo-item/user-reference.dto.ts (1)
ObjectType(4-9)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.input.ts (1)
examples/federation-v2-e2e/user-service/src/user/user.input.ts (2)
InputType(3-10)InputType(12-19)
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts (4)
examples/federation-v2-e2e/tag-service/src/tag/tag.dto.ts (1)
ObjectType(4-22)examples/federation-v2-e2e/todo-service/src/todo-item/tag-reference.dto.ts (1)
ObjectType(8-13)examples/federation-v2-e2e/todo-service/src/todo-item/user-reference.dto.ts (1)
ObjectType(4-9)examples/federation-v2-e2e/user-service/src/user/user.dto.ts (1)
ObjectType(4-21)
🪛 markdownlint-cli2 (0.18.1)
examples/federation-v2-e2e/README.md
16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: e2e-test (21.x, postgres)
- GitHub Check: check (23.x, test)
- GitHub Check: check (22.x, test)
- GitHub Check: e2e-test (20.x, mysql)
- GitHub Check: e2e-test (21.x, mysql)
- GitHub Check: e2e-test (20.x, postgres)
- GitHub Check: Analyze
- GitHub Check: federation-e2e-test
🔇 Additional comments (59)
examples/federation-v2-e2e/todo-service/nest-cli.json (1)
1-8: LGTM!The Nest CLI configuration is correctly formatted and follows standard conventions. The schema reference is valid, schematics collection is appropriate, and compiler options (automatic output directory cleanup) are sensible for development environments.
examples/federation-v2-e2e/user-service/nest-cli.json (1)
1-8: Configuration looks good.The Nest CLI configuration is standard and properly formatted. The
deleteOutDir: trueoption ensures clean builds, which is appropriate for an example project.examples/federation-v2-e2e/user-service/tsconfig.json (1)
1-23: Configuration looks good.This tsconfig.json appropriately enables decorator support (required for NestJS/GraphQL), enforces strict type checking, and targets ES2021. The setup is consistent with NestJS backend service conventions and will support the federation v2 e2e test environment.
examples/federation-v2-e2e/user-service/Dockerfile (1)
1-73: Well-structured multi-stage Dockerfile with good caching strategy.The build follows Docker best practices: immutable config/deps are copied early to leverage layer caching, source code is added later to avoid cache invalidation on code changes, and the final image is lean.
examples/federation-v2-e2e/todo-service/Dockerfile (2)
4-73: Well-structured multi-stage build with good layer caching strategy.The Dockerfile correctly leverages multi-stage builds to compile nestjs-query packages in an isolated builder stage and then uses them in the runtime stage. Dependency files are copied before source code, which maximizes Docker layer caching. The use of
npm packto create local tarballs for installation is a solid approach for testing local package versions in an E2E environment.
56-59: Verify package names and versions are correct.Ensure that the glob patterns for the local package tarballs (
ptc-org-nestjs-query-core-*.tgz, etc.) match the actual package names and are available after the builder stage. If package versions are pinned or vary, confirm the patterns will reliably resolve.examples/federation-v2-e2e/user-service/package.json (1)
1-38: The dependencies are correctly configured for Express 5 and Apollo Server v5 federation; remove the incorrect @parent() decorator claim.The package.json looks well-configured. However, the original review comment contains a technical misunderstanding:
@parent() is not used in @ResolveReference(): The review incorrectly suggests that @nestjs/graphql 13.2.0 includes a @parent() decorator fix for @ResolveReference(). In reality, @ResolveReference receives the reference object directly as a parameter. The @parent() decorator is used in field resolvers (@ResolveField), not in federation reference resolution. This concern should be removed.
Express 5 + Apollo v5 compatibility is correct: @as-integrations/[email protected] explicitly supports @apollo/server v5 (added in v1.1.2), so the combination of @as-integrations/express5@^1.1.2 with @apollo/server@^5.2.0 is compatible and properly configured. No issue here.
Caret versioning is acceptable: While using
^for all dependencies could theoretically allow minor version drift, the chosen versions are stable and well-tested. If stricter reproducibility is needed for CI, pinning could be considered as a future optimization, but it's not necessary.Likely an incorrect or invalid review comment.
examples/federation-v2-e2e/.dockerignore (1)
1-2: Ignoring node_modules and dist in Docker context is appropriateThis will keep build contexts small and avoid copying unnecessary artifacts into images.
examples/federation-v2-e2e/.gitignore (1)
1-1: Un-ignoring package-lock.json here makes sense for reproducible e2e installsThis allows committing a dedicated lockfile for this example even if the root ignores it.
packages/query-graphql/__tests__/integration/federation-n1/fixtures/test-data.ts (1)
8-10: FK-aware deletion order in test data setup looks goodDeleting
TodoItemrows beforeTodoListrows matches the FK relationship and avoids constraint violations during test setup/teardown.examples/federation-v2-e2e/e2e-test/tsconfig.json (1)
1-15: TS config is reasonable for the Jest/Node e2e setupStrict mode, ES2022 target, and
rootDir: "./e2e"are all sensible for this small test project; nothing concerning here.examples/federation-v2-e2e/gateway/nest-cli.json (1)
1-8: Gateway Nest CLI configuration is standardUsing
sourceRoot: "src"withdeleteOutDir: trueis the usual minimal setup; nothing problematic here.examples/federation-v2-e2e/e2e-test/package.json (1)
1-23: > Likely an incorrect or invalid review comment.examples/federation-v2-e2e/todo-service/package.json (1)
1-38: Dependency versions are compatible and well-aligned.The pinned versions are mutually compatible:
@nestjs/graphqlv13.2.0 explicitly supports Apollo Server v5,graphql16.12.0 satisfies Apollo Server v5's peer requirement (>= 16.11.0),@apollo/subgraph2.12.1 is compatible with Apollo Server 5 and graphql v16, and@nestjs/typeormv11.0.0 aligns with NestJS v11. Ensure Node.js v20+ is used in CI/Docker, as required by NestJS v11 and Apollo Server v5..github/workflows/test.yml (1)
99-118: LGTM! Well-structured federation e2e test job.The new CI job appropriately uses Docker Compose to run federation end-to-end tests in isolation. The use of
--exit-code-from e2e-testensures test failures propagate correctly, and the failure log step aids debugging.examples/federation-v2-e2e/gateway/package.json (1)
1-35: LGTM! Gateway dependencies are well-configured.The package configuration correctly sets up Apollo Federation V2 gateway with compatible NestJS and GraphQL versions.
examples/federation-v2-e2e/e2e-test/jest.config.js (1)
1-7: LGTM! Jest configuration is appropriate for e2e tests.The 60-second timeout is suitable for Docker-based federation tests, and the test pattern correctly targets e2e specs.
packages/query-graphql/src/loader/reference.loader.ts (2)
25-30: LGTM! String-based Map keys solve ID type mismatch.The change to use
Map<string, DTO>withString(id)coercion ensures consistent lookups regardless of whether federation sends numeric or string IDs. The explanatory comment clearly documents the rationale for this approach.
33-39: LGTM! Consistent string coercion in lookup path.The
String(arg.id)conversion matches the storage logic, ensuring references are correctly resolved even with ID type mismatches between federation representations and entity IDs.examples/federation-v2-e2e/e2e-test/e2e/federation.spec.ts (3)
47-120: LGTM! Comprehensive reference resolution tests.The tests thoroughly validate cross-service reference resolution with different ID types (numeric for users, UUID for tags), ensuring the
@Parent()decorator fix works correctly across scenarios.
163-187: Excellent validation of issue #410 fix.This test explicitly verifies that the representation parameter is not undefined after the
@Parent()decorator fix. The comment at line 166 clearly documents the regression and how it manifested.
189-217: LGTM! Validates the String(id) Map lookup fix.This test confirms that ID type mismatches between federation representations and entities are handled correctly through string coercion in
reference.loader.ts.examples/federation-v2-e2e/user-service/src/user/user.dto.ts (1)
4-8: LGTM! Correct federation key configuration.The
@key(fields: "id")directive correctly marks this entity for federation reference resolution. The numeric ID type aligns with the e2e tests validating numeric ID handling.packages/query-graphql/src/resolvers/reference.resolver.ts (2)
2-2: LGTM! Parent decorator import added for the fix.The
Parentdecorator import is necessary for the critical fix on line 37.
36-41: Critical fix: @parent() decorator resolves issue #410.This fix addresses the root cause where the
representationparameter wasundefinedwhen other parameter decorators were present. The@Parent()decorator correctly extracts the federation representation from the GraphQL execution context.The fix is minimal, targeted, and maintains compatibility with existing DataLoader support.
examples/federation-v2-e2e/user-service/src/main.ts (1)
5-12: LGTM! Standard NestJS service bootstrap.The entrypoint correctly initializes the service on port 3000, consistent with the docker-compose configuration and other federation services.
examples/federation-v2-e2e/e2e-test/Dockerfile (1)
1-12: LGTM!The Dockerfile follows standard practices for containerized test execution with appropriate Node.js base image, dependency installation, and test command.
examples/federation-v2-e2e/tag-service/nest-cli.json (1)
1-8: LGTM!Standard NestJS CLI configuration with appropriate schema reference and compiler options.
examples/federation-v2-e2e/gateway/Dockerfile (1)
1-27: LGTM!The Dockerfile appropriately implements a single-stage build for the gateway service with correct dependency management and port exposure.
examples/federation-v2-e2e/tag-service/tsconfig.json (1)
1-23: LGTM!The TypeScript configuration appropriately enables strict mode and uses sensible compiler options for a NestJS service.
packages/query-graphql/__tests__/integration/federation-n1/dtos/todo-list.dto.ts (1)
1-1: LGTM!The addition of the
@Directive('@key(fields: "id")')decorator correctly enables Federation entity resolution forTodoListDto, aligning with the PR's objective to test and fix Federation reference resolution.Also applies to: 8-8
examples/federation-v2-e2e/gateway/src/app.module.ts (1)
1-22: File path and code content do not match the review.The review references
examples/federation-v2-e2e/gateway/src/app.module.tswith service namesuser-service,todo-service, andtag-service. However, the actual file is located atexamples/federation-v2/graphql-gateway/src/app.module.tsand useslocalhosthardcoded URLs (ports 3001-3004) with service namestodo-items,sub-tasks,tags, anduser. The code snippet and gateway configuration in the review do not match the actual implementation.Likely an incorrect or invalid review comment.
packages/query-graphql/__tests__/integration/federation-n1/dtos/todo-item.dto.ts (1)
1-18: Federation key directive onTodoItemDtolooks correctAdding
@Directive('@key(fields: "id")')with the correspondingDirectiveimport is the right way to exposeTodoItemas a federated entity keyed byid; this aligns with the federation tests and should work smoothly with_entitiesresolution.examples/federation-v2-e2e/user-service/src/app.module.ts (1)
1-29: User service AppModule wiring is consistent and appropriate for E2EThe TypeORM and GraphQL federation configuration mirrors the todo/tag services and is suitable for this dedicated E2E environment (env-based DB config,
autoLoadEntities,synchronize, and federation v2 schema output), so this module setup looks good.examples/federation-v2-e2e/tag-service/src/app.module.ts (1)
1-29: Tag service AppModule configuration is consistent and suitable for E2EThe Postgres and federation GraphQL setup (env-driven DB config,
autoLoadEntities,synchronize, federation v2 schema) mirrors the other subgraphs and fits the E2E federation scenario, so this module wiring looks good.examples/federation-v2-e2e/tag-service/src/tag/tag.module.ts (1)
1-31: TagModule resolver config correctly exercisesreferenceBywith string IDsThe nestjs-query resolver setup for
TagDTOwithreferenceBy: { key: 'id' }, plus the seeder, is a good way to validate federation reference resolution for UUID/string IDs and directly supports the regression coverage described in the PR.examples/federation-v2-e2e/user-service/src/user/user.module.ts (1)
1-32: UserModule resolver config correctly models the regression scenarioThe nestjs-query resolver configuration for
UserDTOwithreferenceBy: { key: 'id' }(and the accompanying comments) is exactly what’s needed to reproduce and guard against the federationreferenceByregression described in issue #410, so this setup looks appropriate.examples/federation-v2-e2e/user-service/src/user/user.input.ts (1)
1-19: LGTM! Clean input type definitions for e2e testing.The input types are well-structured and follow NestJS GraphQL conventions. The separation of required fields in
CreateUserInputand optional fields inUpdateUserInputis appropriate.examples/federation-v2-e2e/tag-service/Dockerfile (1)
1-73: LGTM! Well-structured multi-stage build for e2e testing.The multi-stage approach correctly builds nestjs-query packages from source and installs them as local dependencies. This allows testing against local changes rather than published packages.
examples/federation-v2-e2e/tag-service/src/tag/tag.entity.ts (1)
1-20: LGTM! UUID primary key for testing string ID types in federation.The use of
@PrimaryGeneratedColumn('uuid')is appropriate for validating that federation works correctly with string-based IDs, complementing the numeric IDs used byUserEntity.examples/federation-v2-e2e/user-service/src/user/user.entity.ts (1)
1-19: LGTM! Standard entity definition for numeric ID testing.The entity correctly uses
@PrimaryGeneratedColumn()for auto-incrementing numeric IDs, which complements the UUID-basedTagEntityfor comprehensive ID type coverage in federation testing.examples/federation-v2-e2e/tag-service/src/tag/tag.dto.ts (1)
1-22: LGTM! Well-structured federation DTO for Tag entity.The DTO correctly implements Apollo Federation requirements with the
@keydirective and uses appropriate field decorators. The string-based ID field aligns with the UUID primary key inTagEntity.examples/federation-v2-e2e/tag-service/src/tag/tag.seeder.ts (1)
7-13: File not found in repository.The review references
examples/federation-v2-e2e/tag-service/src/tag/tag.seeder.ts, but this file does not exist in the repository. The actual directory structure usesexamples/federation-v2/tag-graphql/instead ofexamples/federation-v2-e2e/tag-service/, and notag.seeder.tsfile exists in the codebase. The concerns raised cannot be verified against a non-existent file.Likely an incorrect or invalid review comment.
examples/federation-v2-e2e/user-service/src/user/user.seeder.ts (1)
8-12: The file path and code referenced in this review comment do not exist in the repository.The review references
examples/federation-v2-e2e/user-service/src/user/user.seeder.tswith hardcoded IDs (1, 2, 3), but this file does not exist. The actual implementation is inexamples/federation-v2/user-graphql/e2e/fixtures.tsand does not use hardcoded IDs:await userRepo.save([ { name: 'User 1', email: '[email protected]' }, { name: 'User 2', email: '[email protected]' }, { name: 'User 3', email: '[email protected]' } ])The e2e tests confirm that auto-generated IDs (1, 2, 3) are correctly persisted and queryable, with no sequence conflicts. The current approach—allowing
@PrimaryGeneratedColumn()to generate IDs naturally—works correctly and avoids the concerns raised in this review.Likely an incorrect or invalid review comment.
examples/federation-v2-e2e/tag-service/src/tag/tag.input.ts (1)
1-19: LGTM!Input types are well-structured with proper nullable annotations. The
CreateTagInputcorrectly enforces requirednamewhile allowing optionalcolor, andUpdateTagInputappropriately makes both fields optional for partial updates.packages/query-graphql/__tests__/integration/federation-n1/federation-reference.spec.ts (4)
30-84: Well-structured test setup and teardown.The test configuration properly sets up an isolated Federation subgraph with in-memory SQLite. The lifecycle management correctly handles cleanup even if setup fails. The
referenceBy: { key: 'id' }configuration directly exercises the fix for Issue #410.
101-129: Excellent core regression test for Issue #410.This test directly validates the
@Parent()decorator fix. The comments clearly explain the before/after behavior, and the assertions verify that representations are correctly resolved. This prevents future regressions.
258-280: Good test for string-to-number ID coercion.This test validates the secondary fix (using
String(id)for consistent Map key handling). It ensures Federation works regardless of whether the gateway sends IDs as numbers or strings.
326-329: No action required. The assertion is correct: the error message thrown by the reference resolver (Unable to resolve reference, missing required key ${key} for ${baseName}) contains the substring'missing required key', and the.toContain()assertion will match it properly.examples/federation-v2-e2e/todo-service/src/todo-item/tag-reference.dto.ts (1)
1-13: Tag reference DTO is correctly shaped for federation usage.Object type name, key directive, and UUID/string
idfield all line up with the Tag subgraph; this is a clean minimal reference DTO for the todo-service side.examples/federation-v2-e2e/todo-service/src/main.ts (1)
1-12: Bootstrap entrypoint is straightforward and appropriate for the e2e service.Nest app creation, port binding, and startup logging are minimal and sufficient for this example.
examples/federation-v2-e2e/todo-service/src/app.module.ts (1)
1-29: AppModule configuration looks correct for a federation v2 test subgraph.TypeORM and GraphQL federation wiring matches the intended todo-service role in the e2e setup, and TodoItemModule is properly imported.
examples/federation-v2-e2e/todo-service/tsconfig.json (1)
1-23: TypeScript configuration is consistent and suitable for the NestJS service.Decorator support, output paths, and strictness options are all appropriate for this e2e module.
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.module.ts (1)
1-30: TodoItemModule wiring correctly mirrors the user/tag modules for federation tests.TypeORM, NestJS Query, resolver config (including
referenceBy: { key: 'id' }), and seeder wiring all look consistent and appropriate for exercising the federation reference path.examples/federation-v2-e2e/todo-service/src/todo-item/user-reference.dto.ts (1)
1-9: User reference DTO cleanly models the federated User key.The type name, key directive, and numeric ID field align with the user-service UserDTO, so this will work well as a reference from TodoItem.
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.entity.ts (1)
1-29: TodoItemEntity maps cleanly to the intended schema and test data.Column definitions and types (including
tagIdas UUID string) match the DTO and seeder expectations for federation tests.examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.seeder.ts (1)
1-63: Seeder service follows the same robust pattern as user/tag seeders.On module init seeding, id/assigneeId/tagId values, and logging behavior provide predictable data for federation e2e tests without impacting non-empty databases.
examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.input.ts (1)
1-31: LGTM!The input DTOs follow the established patterns from the user-service. The use of
defaultValue: falseforcompletedis appropriate, and the nullable annotations are consistent between the TypeScript types and GraphQL decorators.examples/federation-v2-e2e/todo-service/src/todo-item/todo-item.dto.ts (1)
7-38: Well-structured Federation DTO with good test coverage for ID types.The DTO correctly demonstrates both numeric (
assigneeId→User) and string/UUID (tagId→Tag) reference types, which is valuable for validating the federation fix across different ID representations. The@Referencemappings and field types are consistent with their corresponding reference DTOs.
|
@TriPSs This restoration process
|
- Fix prettier and eslint errors in federation-v2-e2e files - Add testPathIgnorePatterns to exclude federation-v2-e2e from main e2e tests (it uses Docker and has its own Jest config)
|
Can you fix the lint/test issues? |
I've checked, it's not a problem I introduced, it's a formatting issue with the previous code. examples:lint logs |
| // Clear existing data (must delete items first due to foreign key) | ||
| await dataSource.getRepository(TodoItem).delete({}) | ||
| await dataSource.getRepository(TodoList).delete({}) |
There was a problem hiding this comment.
Previously, deleteAll() reported that the method didn't exist when I ran it locally. Now, it works perfectly locally. So what are the lint rules? Why isn't it working?
There was a problem hiding this comment.
Probably outdated deps? TypeORM was updated recently
Those are warnings, the errors: Added a comment why the tests are failng |
I've run lint locally and it shows no problems in my IDE. Could it be that this is a separate project with its own dependencies, and shouldn't be checked by lint?
|
|
You are running |
lint examples |
|
I understand. I've already run |
Not sure that I follow? The CI does install deps. Can you push the fixes for the testing, I will checkout the linting. |
2 participants
Summary
Fixes #410 - Federation
referenceBybroken since v9.2.0Problem
The auto-generated
@ResolveReference()method fails in Apollo Federation because therepresentationparameter isundefined. This regression was introduced in v9.2.0 when DataLoader support was added with@Context()and@InjectDataLoaderConfig()parameter decorators.Root Cause:
@nestjs/graphql's@ResolveReference()decorator has known incompatibilities with parameter decorators (see NestJS issues #945, #2127). When parameter decorators are present without a decorator on the first parameter, it becomesundefined.Solution
Add
@Parent()decorator to therepresentationparameter inresolveReference()method:Also fixed a type consistency issue in
ReferenceLoaderwhere Map keys neededString()conversion for consistent lookups.Changes
packages/query-graphql/src/resolvers/reference.resolver.ts
@Parent()decorator torepresentationparameterpackages/query-graphql/src/loader/reference.loader.ts
Map<string, DTO>for consistent key handlingString(id)conversion for Map operationspackages/query-graphql/tests/integration/federation-n1/federation-reference.spec.ts (NEW)
ApolloFederationDriver_entitiesquery which simulates Gateway requests to subgraphTesting
The new integration test (
federation-reference.spec.ts) validates:_entitiesquery_entitiesquery_entitiesquery@keydirectiveVerification: Removing
@Parent()causes all_entitiestests to fail withCannot read properties of undefined (reading 'id')- the exact error reported in #410.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.