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

Introduce string set handling #34

Merged
merged 10 commits into from Jun 17, 2020
Merged

Introduce string set handling #34

merged 10 commits into from Jun 17, 2020

Conversation

hannes-angst
Copy link

(1) Implements handling of CONTAINS on SS (string set) and NOT_CONTAINS on SS (string set).
(2) Fixes object mis-match for collection parameters on SS (string set) properties.

boostchicken and others added 9 commits February 25, 2020 00:52
* Read Consistency Setting

* #8

Custom Mapper Spec

* #8

Custom Mapper CDI fix

* Fix symlink to CONTRIBUTING.md (#13)

* #8 Don't use autowired or constructor injection with FactoryBeans (#12)

* [maven-release-plugin] prepare release v5.2.2

* [maven-release-plugin] prepare for next development iteration

* 5.2.2 docs fix

* Hibernate Validator CVE fix
GHSA-m8p2-495h-ccmh

* Documentation updates

* Create FUNDING.yml

* Fixed a Type (#17)

Co-authored-by: thedevluffy <52121827+TheDevLuffy@users.noreply.github.com>

* Documentation updates

* 5.2.3 prep

* [maven-release-plugin] prepare release v5.2.3

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Christian Frommeyer <frommeyerc@googlemail.com>
Co-authored-by: thedevluffy <52121827+TheDevLuffy@users.noreply.github.com>
# Conflicts:
#	README.md
#	pom.xml
#	src/changes/changes.xml
#	src/main/java/org/socialsignin/spring/data/dynamodb/repository/Query.java
@boostchicken
Copy link
Owner

Wow @hannes-angst , I was working on this myself. You beat me to the punch. Will test and merge here in a bit

@boostchicken
Copy link
Owner

Did you need this released to central anytime soon?

@boostchicken boostchicken changed the base branch from master to develop June 17, 2020 05:17
@boostchicken boostchicken self-assigned this Jun 17, 2020
@boostchicken boostchicken added this to the 5.2.5 milestone Jun 17, 2020
@boostchicken boostchicken added the enhancement New feature or request label Jun 17, 2020
@boostchicken boostchicken linked an issue Jun 17, 2020 that may be closed by this pull request
@boostchicken boostchicken moved this from To do to In Testing in Spring Data DynamoDB Jun 17, 2020
@boostchicken boostchicken removed this from In Testing in Spring Data DynamoDB Jun 17, 2020
@boostchicken boostchicken merged commit c6a729e into boostchicken:develop Jun 17, 2020
@hannes-angst
Copy link
Author

Did you need this released to central anytime soon?

Would be nice. ❤️

@boostchicken
Copy link
Owner

Are you planning on adding any other contributions? If not I can cut 5.2.5 with your additions now.

@hannes-angst
Copy link
Author

I was looking into the OR challenge but I identified at least three major reworks

  • updated to dynamodb library 2
  • move from query criteria to filter expressions
  • part tree processing rewrite 😢

So I would suggest to just wrap it up and call it a 5.2.5 ;)

@boostchicken
Copy link
Owner

boostchicken commented Jun 17, 2020

So if we move to DynamoDB 2, might as well jsut re-write it from the ground up. I have used their new EnhancedMapper and it's great, however, nothing here would really carry over all that much. There are years of tech debt and old spring conventions built up here. It is probably time to start fresh.

Is this something you are interested in working on? I want to do it, but to do it by myself would take god knows how long

As far as filter expressions, you will noticed I wrote an ugly hack to get in there quickly using the Query annotation. Again, it's such a fundamental switch that it would require a large re-write :)

boostchicken added a commit that referenced this pull request Jun 17, 2020
* Update FUNDING.yml

* Introduce string set handling (#34)

* v5.2.3 (#18)

* Read Consistency Setting

* #8

Custom Mapper Spec

* #8

Custom Mapper CDI fix

* Fix symlink to CONTRIBUTING.md (#13)

* #8 Don't use autowired or constructor injection with FactoryBeans (#12)

* [maven-release-plugin] prepare release v5.2.2

* [maven-release-plugin] prepare for next development iteration

* 5.2.2 docs fix

* Hibernate Validator CVE fix
GHSA-m8p2-495h-ccmh

* Documentation updates

* Create FUNDING.yml

* Fixed a Type (#17)

Co-authored-by: thedevluffy <52121827+TheDevLuffy@users.noreply.github.com>

* Documentation updates

* 5.2.3 prep

* [maven-release-plugin] prepare release v5.2.3

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Christian Frommeyer <frommeyerc@googlemail.com>
Co-authored-by: thedevluffy <52121827+TheDevLuffy@users.noreply.github.com>

* ✨ Allow CONTAINS and NOT_CONTAINS is repositories

* 🎨 cleanup

* 🔧 check for collection

* 🔧 check for collection

* ✏️ cleanup

* 🎨 cleanup imports

* Remove funding changes

Co-authored-by: John D <dorman@overlooked.us>
Co-authored-by: Christian Frommeyer <frommeyerc@googlemail.com>
Co-authored-by: thedevluffy <52121827+TheDevLuffy@users.noreply.github.com>
Co-authored-by: John Dorman <dorman@ataxia.cloud>
Co-authored-by: Hannes Angst <hannes.angst1@porsche.de>

* Updating changelog for #33

* Updating for release

* [maven-release-plugin] prepare release v5.2.5

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Hannes Angst <hannes-angst@users.noreply.github.com>
Co-authored-by: Christian Frommeyer <frommeyerc@googlemail.com>
Co-authored-by: thedevluffy <52121827+TheDevLuffy@users.noreply.github.com>
Co-authored-by: Hannes Angst <hannes.angst1@porsche.de>
@boostchicken
Copy link
Owner

boostchicken commented Jun 17, 2020

@hannes-angst It has been released and is synced on central.
https://repo1.maven.org/maven2/io/github/boostchicken/spring-data-dynamodb/5.2.5/

I don't know if you are using it over at Porsche, but if you are I might to upgrade my 991 C2S to utilize whatever service it is powering :)

@hannes-angst
Copy link
Author

hannes-angst commented Jun 17, 2020

991 C2S is a beauty. I would love to own a Porsche Taycan Turbo 4S 😂

I was baffled that there is no spring repository support directly from AWS and I hate to wait for testcontainers to come up with a local dynamodb instance just to do things I already know are working 😉.

For the rewrite, there would be some things to tackle:

  • Walk the parts tree to come up with the expression filter we need (see below).
  • Workaround the limitations of dynamo DB (SS contains with multiple elements)
  • Decide when to query (hash or index) and when to scan (would be awesome if we could determine this from the entity definition and/or the table)
  • Ordering (don't know if this is even possible in dynamoDB between pages)
  • Not sure how ORM works with v2. (Have to research this further 🤔)

I'm pretty sure there is much more to consider and I can totally feel you not to go solo on it.
I can have a look at the parts tree stuff. This seems easy to grasp 🤥 😉

Scans:

 ScanRequest scan = ScanRequest.builder()
                .tableName("User")
                .filterExpression("contains(#key1, :value1) AND contains(#key2, :value2) AND #key3 = :value3")
                .expressionAttributeNames(Map.of(
                        "#key1", "tags",
                        "#key2", "tags",
                        "#key3", "postCode"))
                .expressionAttributeValues(Map.of(
                        ":value1", AttributeValue.builder().s("tag-a").build(),
                        ":value2", AttributeValue.builder().s("tag-b").build(),
                        ":value3", AttributeValue.builder().s("1234").build()
                )).build();

Queries:

        QueryRequest query = QueryRequest.builder()
                .tableName(tableName)
                .keyConditionExpression("#key1 = :value1")
                .expressionAttributeNames(Map.of(
                        "key1", "id"))
                .expressionAttributeValues(Map.of(
                        "value1", AttributeValue.builder().s("4711").build())).build();

@boostchicken
Copy link
Owner

boostchicken commented Jun 17, 2020

https://github.com/boostchicken/spring-data-dynamodb/wiki/Change-Log

Check that page for how the library handles filters right now. I know its kind of jankey but with out the mechanism to decide when to query or scan, this is the best I could do on my timeline. It would be great to re-write the Part Tree and GSI code to figure out when a GSI is available and when it would require filters as opposed to queries

https://aws.amazon.com/blogs/developer/introducing-enhanced-dynamodb-client-in-the-aws-sdk-for-java-v2/

That is the library that replaces DynamoDBMapper. I moved one of my apps from this project to that. If you are interested in seeing an actual in-depth production implementation let me know and we can set up a screen share (can't make the code public). While there are some similarities, there is not enough to make me want to carry all this code forward.

@hannes-angst
Copy link
Author

@boostchicken Any way to chat with you?

@hannes-angst
Copy link
Author

I created a branch that uses the current code and add expression filters to the queries. This will allow for advanced -- scan -- queries.
This is definitively WIP since I need to add much more test cases. 😁

@boostchicken
Copy link
Owner

@hannes-angst Are you on discord? boostchicken#3135. We can also setup a Slack

@boostchicken
Copy link
Owner

boostchicken commented Jun 21, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow single object as parameter to query a set/list
2 participants