You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
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:
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.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 verifyusing 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 freshJacksonDefaults.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.