Skip to content

Add checkstyle rules fit for v2 - #7325

Open
RanVaknin wants to merge 10 commits into
feature/master/DDB-mapperv2from
rvaknin/fix-checkstyle-to-fit-v2
Open

Add checkstyle rules fit for v2#7325
RanVaknin wants to merge 10 commits into
feature/master/DDB-mapperv2from
rvaknin/fix-checkstyle-to-fit-v2

Conversation

@RanVaknin

@RanVaknin RanVaknin commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

mapper v2 "house cleaning" PR

dynamodb-mapper (mapper v2) is a faithful 1:1 port of the EOL v1 DynamoDBMapper. Enabling the full v2 checkstyle ruleset would rename public types, change signatures, reorder members, and add code absent from v1. That turns the port into a refactor which is harder to diff against v1, and poses a behavior change risk.

This PR is "house cleaning", it makes sure the mapper module adheres to the v2 style and release requirement every module adheres to (where possible).

I did my best to break down the PR into independent commits by subject. Every subject is listed as a mechanical or low discretion change:

Changes Breakdown:

1. Static analysis suppressions + SDK annotations (mechanical, some discretion)

A faithful port trips three static analysis gates. Rather than adopting the full v2 rulesets, we suppress narrowly:

  • Checkstyle: instead of the full ruleset we allowlist 3 rules:
    • HideUtilityClassConstructor (5): added private constructors to utility classes. Mechanical.
    • UnnecessaryFinalOnLocalVariable (216): removed unnecessary final on locals across 17 files. Mechanical.
    • MissingSdkAnnotation (110): added SDK API annotations. Discretion: classification follows the Enhanced Client convention.
      • 40 public visibility types → @SdkPublicApi
      • 68 marshaller/unmarshaller converters + 2 package private helpers → @SdkInternalApi
      • No @SdkProtectedApi
  • Spotbugs (build-tools/.../spotbugs-suppressions.xml): 15 bug patterns carried over verbatim from v1, scoped to mapper.dynamodb.*, suppressed to keep the port byte-faithful rather than refactoring. Mechanical.
  • ArchUnit (test/architecture-tests): 3 mapper classes added to the shouldNotAbuseWarnLog allowlist; DynamoDBMapperModelFactory, UpdateExpressionGenerator, and VersionAttributeConditionExpressionGenerator moved to internal/ to satisfy PackageContainmentTest. Mechanical.

Cross module note: this PR edits two shared files outside dynamodb-mapper: build-tools spotbugs-suppressions.xml, and archtests/ArchUtils.java (new classWithInnerClassesToPattern(String) helper, needed to allowlist an inner class warn by name without the arch tests module depending on the mapper).

2. Repo layout (mechanical)

Moved marshallers/unmarshallers/utils under internal/; moved true integ tests to src/it/. This is just package relative moves, no code changes.

3. Swap remaining v1 imports + logging (mechanical)

Remaining v1 SDK imports → v2 equivalents; commons-logging → SLF4J.

4. Remove dead code (mechanical)

Deleted unused DynamoDBReflector/ReflectionUtils; removed deprecated RequestMetricCollector from config (no v2 equivalent).

5. Restore data plane functional tests (mechanical)

Un excluded the DynamoDB Local functional tests that only use table admin for fixture setup (they actually test data plane operations) Pure table admin and S3Link tests stay excluded pending their ports.

6. Replace log4j 1.x with log4j2 (mechanical)

Swapped the log4j 1.x test dep for the SDK standard log4j-api/log4j-core/log4j-slf4j-impl (bom managed); replaced log4j.properties with log4j2.xml.

7. Build metadata (mechanical)

Declared the module's previously transitive v2 dependencies explicitly (sdk-core, aws-core, http-client-spi, auth, regions, annotations, utils). added the mapper to the .brazil.json mapping exclusion list.

Not in this PR (deferred to dedicated port PRs)

Table-admin/GSI port, S3Link port, and the v1 coupled tests they block.


**DDB mapper v2 roadmap: **

** BASE PACKAGE SETUP **
+ 1. Source verbatim port ✅
+ 2. Test verbatim port ✅
+ 3. Namespace swap (main + datamodeling tests) ✅ 
+ 4. Namespace swap (remaining test packages) ✅

** PORTING OPERATIONS **
+ 0. converters (AttributeValue seam) ✅ 
+ 1. load() ✅
+ 2. save() ✅
+ 3. query() + scan() ✅
+ 4. deleteItem() ✅
+ 5. updateItem() ✅
+ 6. Batch operations ✅
+ 7. Transactions ✅
8. S3Link
9. Table Admin (control plane convenience methods) 
- 10. Cleanup: <--- current PR

** PERFORMANCE IMPROVEMENTS **
1. getTableModel caching
2. Wire "fast" createX AV factory methods to convertors
3. ByteBuffer → SdkBytes copy
4. Others

** DEPENDENCY MODERNIZATION **
+ 1. EasyMock -> Mockito ✅
- 2. Log4j 1.x -> 2.x  <--- current PR
- 3. commons-logging -> SLF4J  <--- current PR

@RanVaknin RanVaknin closed this Aug 26, 2026
@RanVaknin RanVaknin reopened this Aug 26, 2026
@RanVaknin
RanVaknin marked this pull request as ready for review August 26, 2026 22:36
@RanVaknin
RanVaknin requested a review from a team as a code owner August 26, 2026 22:36
@RanVaknin RanVaknin closed this Aug 26, 2026
@RanVaknin RanVaknin reopened this Aug 26, 2026
@github-actions

Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 26, 2026
@RanVaknin RanVaknin closed this Aug 26, 2026
@RanVaknin RanVaknin reopened this Aug 26, 2026
@RanVaknin RanVaknin closed this Aug 26, 2026
@RanVaknin RanVaknin reopened this Aug 26, 2026
…g tests to it/

Relocates the marshaller, unmarshaller, and Mapper*Utils classes under an
internal/ package, and moves the four tests that make real AWS calls to
src/it/java per the v2 layout convention. Adds the v1-coupled S3Link and
table-admin tests to the compiler testExcludes until those ports land.
@RanVaknin

RanVaknin commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Appendix: full annotation classification

@SdkPublicApi: 40 public-visibility types

Core mapper (7)
DynamoDBMapper, AbstractDynamoDBMapper, IDynamoDBMapper, DynamoDBMapperConfig,
DynamoDBTableMapper, DynamoDBMapperTableModel, DynamoDBMapperFieldModel

Operation expressions / requests (9)
DynamoDBQueryExpression, DynamoDBScanExpression, DynamoDBSaveExpression, DynamoDBDeleteExpression,
DynamoDBTransactionLoadExpression, DynamoDBTransactionWriteExpression,
TransactionLoadRequest, TransactionWriteRequest, BatchLoadContext

Result / pagination types (6)
PaginatedList, PaginatedQueryList, PaginatedScanList, PaginatedParallelScanList,
QueryResultPage, ScanResultPage

Conversion & extension SPIs (10)
DynamoDBTypeConverter, DynamoDBTypeConverterFactory, ConversionSchema, ConversionSchemas,
ItemConverter, DynamoDBAutoGenerator, AttributeTransformer, AttributeTransformerChain,
ArgumentMarshaller, ArgumentUnmarshaller

S3 / misc (4)
S3Link, S3ClientCache, KeyPair, DynamoDBMappingException

Deprecated but public (4)
DynamoDBMarshaller, AbstractEnumMarshaller, JsonMarshaller, ParallelScanTask

@SdkInternalApi: 70 types

  • marshallers/*: 23 converter classes (Java type → AttributeValue)
  • unmarshallers/*: 45 converter classes (AttributeValue → Java type)
  • MapperExceptions: internal exception helper

@RanVaknin RanVaknin closed this Aug 28, 2026
@RanVaknin RanVaknin reopened this Aug 28, 2026
@aws aws unlocked this conversation Sep 2, 2026
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