fix: allow false for resolver request/response mapping templates (#598)#714
Conversation
The BaseResolverConfig type allows request/response to be string | false and the resolver compiler already handles false (it keeps the VTL path and omits the mapping template), but the validation schema only accepted strings, rejecting valid configs. Adds a mappingTemplate schema definition (string or false) and points resolverConfig.request/response at it. Pipeline functions stay string-only (type and schema already agree). Adds valid (false) and invalid (true) test cases. Fixes #598
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extends GraphQL AppSync schema validation to allow resolver mapping templates to be explicitly disabled via ChangesMapping Template Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Fix resolver
request/responsevalidation to allowfalseFixes #598.
Problem
The resolver config type allows the mapping templates to be
false:…but the validation schema only accepts strings, so a config like
request: falseis rejected before it ever reaches the resolver compiler:This is a type-vs-validation mismatch: the type (and the compiler) support
false, but validation does not.Why
falseis a real featureThe resolver compiler already handles
falsecorrectly:isVTLResolver = 'request' in this.config || 'response' in this.config— settingrequest: falsestill puts the resolver on the VTL path (rather than being treated as a JS resolver), andresolveMappingTemplate()doesconst templateName = this.config[type]; if (templateName) { … }— withfalse, that's falsy, so the correspondingRequestMappingTemplate/ResponseMappingTemplatesimply isn't emitted.So
falseis a deliberate way to declare a VTL resolver that omits a request and/or response mapping template. Only the schema was out of sync.Change
mappingTemplateschema definition:oneOf: [{ type: 'string' }, { const: false }], with a clear error message.resolverConfig.request/resolverConfig.responseat it.string | false; this brings validation in line with them.The validation error message changes from
must be stringtomust be a string (path to the template) or false to omit the mapping template, which the existing negative-case snapshot reflects.Out of scope (intentional)
Pipeline functions (
PipelineFunctionConfig) keeprequest/responseasstring-only. There the type and the schema already agree (both string-only), so there's no mismatch to fix; addingfalsethere would be a separate feature change (type + schema) rather than a bug fix.Tests
request: false/response: false, and one withrequest: false+ a response template path.request: true/response: trueis rejected (guards against widening to any boolean).request: 123) still fails, with the updated message.npm test(333),npm run test:e2e(84),npm run lint(0 errors),npm run build.Summary by CodeRabbit
New Features
falsein addition to specifying template paths.Tests