Skip to content

Share the permissive Lambda runtime mapper - #169

Merged
stevehu merged 3 commits into
masterfrom
fix-shared-lambda-runtime-mapper
Oct 27, 2026
Merged

stevehu merged 3 commits into
masterfrom
fix-shared-lambda-runtime-mapper

Conversation

@stevehu

@stevehu stevehu commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

The custom runtime and native entry points duplicated the same permissive event mapper. Add LambdaRuntimeMappers.mapper() as the shared API and make the custom runtime use it. Compatibility tests call that API directly instead of reading a private field by reflection; they also check that unknown event fields are accepted while the framework mapper stays strict.

Validation: full offline reactor clean verify using a task-owned Maven cache passed: 67 tests, 2 skipped, zero failures or errors. No live AWS execution.

Remote CI: https://github.com/networknt/light-aws-lambda/actions/runs/40449753399 passed. Downloaded reports confirm 67 tests, 2 skipped, zero failures/errors; the strict artifact audit has no failures and no Jackson 2 dependencies.

Review follow-up 33e2aa6: imports now follow the file's style; the redundant identity assertion is removed; the feature comparison uses a fresh JacksonDefaults.builder().build() mapper. Javadoc explicitly restricts this permissive API to Lambda event payloads and excludes configuration binding and validation. All 5 focused compatibility tests pass locally, with no skips or failures. Full CI reruns on this follow-up head; the run above covers the earlier head.

Merge order: this PR first, then schema-isolation PR #170 (stacked on this branch), then native companion networknt/light-lambda-native#190 after its AWS pin is updated to the merged master SHA.

@stevehu

stevehu commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Review of head 96dd98c. I read the diff and the native companion #190 and ran nothing; test is green and the PR is mergeable. I found no blockers.

LambdaRuntimeMappers builds the same mapper Runtime had before (JacksonDefaults.builder().disable(FAIL_ON_UNKNOWN_PROPERTIES).build()), Runtime now uses it, and no other copy of the permissive mapper is left in this repo. The tests call the accessor directly instead of reading a private field by reflection. light-lambda-native already depends on custom-runtime, so #190 can consume this API.

Three small points:

  1. Test style. The new test uses fully qualified names inline (org.junit.jupiter.api.Assertions.assertSame, tools.jackson.databind.DeserializationFeature, com.networknt.jackson.JacksonDefaults) and leaves a double blank line, while the rest of the file uses static imports.
  2. Weak assertion. assertSame(mapper(), mapper()) on a static constant cannot fail. The useful part is the pair of FAIL_ON_UNKNOWN_PROPERTIES assertions, and the second depends on JacksonDefaults.mapper() keeping Jackson 2's strict default, so a change there would fail this test for a reason unrelated to the Lambda runtime. Comparing against a freshly built JacksonDefaults.builder() mapper would isolate what this class changes.
  3. Use restriction. This is now a public API, and the Javadoc does not say the mapper is deliberately more permissive than the framework one. A sentence such as "for Lambda event payloads only" would stop a future caller from using it in a validation or config path, where it would silently accept unknown fields.

@stevehu

stevehu commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Review of head 33e2aa6. I read the delta and ran nothing. The test check had no result yet on this head when I looked.

My three earlier points look addressed: the fully qualified names are now imports and the extra blank line is gone, the redundant assertSame is removed and the test compares against a freshly built JacksonDefaults.builder() mapper, and the Javadoc now says the mapper is for Lambda event payloads only and must not be used for configuration binding or validation.

One tiny nit, not a request: the renamed test acceptsUnknownEventPropertiesWithoutChangingFreshFrameworkMapper only compares two feature flags; it never binds an event with an unknown property (retainsPermissiveRuntimeBinding does). A name such as runtimeMapperDisablesOnlyUnknownPropertyFailure would match what it checks.

@stevehu
stevehu merged commit 8406f40 into master Oct 27, 2026
2 checks passed
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