feat(v2): batch validation with partial failure

aws-powertools / powertools-lambda-java

Powertools is a developer toolkit to implement Serverless best practices and increase developer velocity.

https://docs.powertools.aws.dev/lambda/java/

MIT No Attribution

289 stars 88 forks source link

feat(v2): batch validation with partial failure #1621

Open jeromevdl opened 7 months ago

jeromevdl commented 7 months ago

Issue #, if available: #1496

Description of changes:

Adding partial failure for validation with SQS and Kinesis. Modification of the ValidationAspect.java to validate each messages of SQS/Kinesis batches and put invalid messages in partial batch failures list of the response. After the handler, we merge with user batch failures.

Checklist

* [x] [Meet tenets criteria](https://docs.powertools.aws.dev/lambda-java/#tenets) * [x] Update tests * [x] Update docs * [x] PR title follows [conventional commit semantics](https://www.conventionalcommits.org/en/v1.0.0/) ## Breaking change checklist

RFC issue #:

[ ] Migration process documented
[ ] Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions[bot] commented 7 months ago

:floppy_disk: Artifacts Size Report

Module	Version	Size (KB)
powertools-common	2.0.0-SNAPSHOT	9.63
powertools-serialization	2.0.0-SNAPSHOT	17.23
powertools-logging	2.0.0-SNAPSHOT	33.10
powertools-logging-log4j	2.0.0-SNAPSHOT	20.70
powertools-logging-logback	2.0.0-SNAPSHOT	16.92
powertools-tracing	2.0.0-SNAPSHOT	14.00
powertools-metrics	2.0.0-SNAPSHOT	13.78
powertools-parameters	2.0.0-SNAPSHOT	17.46
powertools-validation	2.0.0-SNAPSHOT	21.32
powertools-cloudformation	2.0.0-SNAPSHOT	16.54
powertools-idempotency-core	2.0.0-SNAPSHOT	34.70
powertools-idempotency-dynamodb	2.0.0-SNAPSHOT	12.38
powertools-large-messages	2.0.0-SNAPSHOT	17.52
powertools-batch	2.0.0-SNAPSHOT	21.51
powertools-parameters-ssm	2.0.0-SNAPSHOT	10.75
powertools-parameters-secrets	2.0.0-SNAPSHOT	9.97
powertools-parameters-dynamodb	2.0.0-SNAPSHOT	12.01
powertools-parameters-appconfig	2.0.0-SNAPSHOT	11.51

sonarcloud[bot] commented 7 months ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

scottgerring commented 7 months ago

Hey @jeromevdl this is a monster. Do you mind when you have a chance doing a quick writeup of the changes at a high level to start from?

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 89.47368% with 12 lines in your changes missing coverage. Please review.

Project coverage is 77.84%. Comparing base (82d4b30) to head (0923826). Report is 76 commits behind head on v2.

:exclamation: Current head 0923826 differs from pull request most recent head 503170a

Please upload reports for the commit 503170a to get more accurate results.

Files	Patch %	Lines
...wertools/validation/internal/ValidationAspect.java	89.47%	7 Missing and 5 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## v2 #1621 +/- ## ============================================= - Coverage 89.79% 77.84% -11.96% - Complexity 406 479 +73 ============================================= Files 44 49 +5 Lines 1274 1733 +459 Branches 165 261 +96 ============================================= + Hits 1144 1349 +205 - Misses 88 295 +207 - Partials 42 89 +47 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jeromevdl commented 4 months ago

@scottgerring this one is ready for review

jeromevdl commented 4 months ago

I should create one (or 2 ? kinesis/sqs) end-to-end tests for this maybe, wdyt @scottgerring ?

scottgerring commented 4 months ago

I should create one (or 2 ? kinesis/sqs) end-to-end tests for this maybe, wdyt @scottgerring ?

To be honest I feel like it might be better to invest time stabilizing the existing tests. As it stands more tests are just going to lead to more "was that an actual build failure or not", which has already put us in a situation where we don't trust the suite.

Could we get coverage out of integration style tests instead?

jeromevdl commented 4 months ago