Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add attribute converters for "standard" values #1381

Merged
merged 6 commits into from
Aug 27, 2024

Conversation

ianbotsf
Copy link
Contributor

@ianbotsf ianbotsf commented Aug 7, 2024

Issue #

Related to #472

Description of changes

This change fills out new ValueConverter implementations for "standard" types (using a somewhat arbitrary definition of the word "standard"). Specifically they include:

  • Various scalar types such as String, Int, Boolean, etc.
  • Collections like List, Map, and Set
  • smithy-kotlin runtime types like Url, Document, and Instant
  • Nullable types

I also introduced a ValueConvertersTest class to streamline unit testing with a DSL. I'll be interested to hear feedback on whether this is effective or if it just obscures things.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ianbotsf ianbotsf requested a review from a team as a code owner August 7, 2024 17:35
Copy link

github-actions bot commented Aug 7, 2024

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

+ `fromAv`/`toAv` → `fromAttributeValue`→`toAttributeValue`
+ Commonizing string/char/chararray converters
+ Update number parsing to use non-nullable lambda types
+ Miscellaneous fixes and naming tweaks
Copy link

github-actions bot commented Aug 8, 2024

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

Copy link
Contributor

@0marperez 0marperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I like the DSL test setup

* Converts between potentially `null` values and
* [DynamoDB `NULL` values](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.DataTypes.Null).
* Note that this class is a [SplittingConverter] and the logic for handling non-null values is undefined in this class.
* Thus, it is typically used in conjunction with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unfinished sentence

* Creates a [ValueConverter] which converts between number type [N] and
* [DynamoDB `N` values](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.DataTypes.Number)
*/
public fun <N> numberConverter(numberToStringConverter: Converter<N, String>): ValueConverter<N> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we capitalize this function name to match others?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was inconsistent here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add metadata like description, module name

Comment on lines 20 to 22
plugins {
id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this being used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, that's an unintentional addition (I was using it on another branch for some debug). Removing!

…in KDocs, remove frivolous dependencies, add module metadata just like I asked of others
Copy link

@ianbotsf ianbotsf merged commit 936aaa9 into feat-ddb-mapper Aug 27, 2024
10 of 11 checks passed
@ianbotsf ianbotsf deleted the ddb-attribute-converters branch August 27, 2024 16:32
ianbotsf added a commit that referenced this pull request Oct 29, 2024
…lin (#1451)

* initial poc commit of DynamoDB Mapper (#1232)

* add support for Mapper initialization (#1237)

* implement mapper pipeline (#1266)

* initial implementation of codegen for low-level operations/types (#1357)

* initial implementation of secondary index support (#1375)

* Create new codegen module and refactor annotation processor to use it (#1382)

* feat: add Schema generator Gradle plugin (#1385)

* Fix plugin test package

* add attribute converters for "standard" values (#1381)

* fix: schema generator plugin test module (#1394)

* feat: annotation processor codegen configuration (#1392)

* feat: add `@DynamoDbIgnore` annotation (#1402)

* DDB Mapper filter expressions (runtime components) (#1401)

* feat: basic annotation processing (#1399)

* add DSL overloads, paginators, and better builder integration for DDB Mapper ops codegen (#1409)

* chore: split dynamodb-mapper-codegen into two modules (#1414)

* emit DDB_MAPPER business metric (#1426)

* feat: setup DynamoDbMapper publication (#1419)

* DDB Mapper filter expressions (codegen components) (#1424)

* correct docs

* mark every HLL/DDBM API experimental (#1428)

* fix accidental inclusion of expression attribute members in high-level DynamoDB Mapper requests (#1432)

* Upgrade to latest build plugin version

* fix: various issues found during testing (#1450)

* chore: update Athena changelog notes for 1.3.57 (2024-10-18) release (#1449)

* feat: update AWS API models

* feat: update AWS service endpoints metadata

* chore: release 1.3.60

* chore: bump snapshot version to 1.3.61-SNAPSHOT

* feat: initial release of Developer Preview of DynamoDB Mapper for Kotlin

* Fix Kotlin gradle-plugin version

* fix: ddb mapper tests (#1453)

* Bump build plugin version

---------

Co-authored-by: Matas <lauzmata@amazon.com>
Co-authored-by: aws-sdk-kotlin-ci <aws-kotlin-sdk-automation@amazon.com>
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.

3 participants