DDB Enhanced:Added support to read Nested objects in attributesToProj… #2035

joviegas · 2020-09-14T04:58:46Z

Description DDB Enhanced: Added support for Nested objects projection expressions RequestOnPR

Motivation and Context

DDB Enhanced: Added support for projection expression #1756 missed out to support projecting Nested Attributes
with this fix we can project Nested attributes

Fix

Added a new Class NestedAttributeName , this can be used to represent Nested Variables


/**
* Public API methods for creating NestedAttributeName
*/

//Simple Attribute creation "foo"
NestedAttributeName s1 = AttributeName.builder().elements("foo").build();
s1 = NestedAttributeName.builder().addElement("foo").build()
s1 = NestedAttributeName.create("foo")

//Nested Attribute creation "foo.bar"
s2 = NestedAttributeName.builder().elements("foo", "bar").build();
s2 = NestedAttributeName.create("foo", "bar");


//Getter Output for s1
s1.elements() = ["foo"]
//Getter Output for s2
s2.elements() = ["foo", "bar"]

EnhancedRequest

/**
*SAMPLE Builder usage And GETTERS for EnhancedRequest
*/   

/***Case1*: One Attribute and attributesToProject(String) used**/
 final QueryEnhancedRequest q1 = 
 QueryEnhancedRequest.builder().attributesToProject("foo").build();
 
 q1.attributesToProject() = ["foo"].
 q1.nestedAttributeNameToProject() = ["foo"]
 
 /***Case2* :One Nested Attribute **/
 final QueryEnhancedRequest q1 = 
 QueryEnhancedRequest.builder()
    .addNestedAttributesToProject(NestedAttributeName.create("foo", "bar")).build();
 
 q1.attributesToProject() = ["foo.bar"].
 q1.nestedAttributeNameToProject() = [["foo","bar"]]
 
  /***Case3* :One Nested Attribute and One Non Nested Attribute **/
 final QueryEnhancedRequest q1 = 
 QueryEnhancedRequest.builder()
    .attributesToProject("bazz")
    .addNestedAttributesToProject(NestedAttributeName.create("foo", "bar")).build();
 
 q1.attributesToProject() = ["bazz","foo.bar"].
 q1.nestedAttributeNameToProject() = [["bazz"], ["foo","bar"]]

Testing

Added JUnit for following casses
1.Projection of a nested Attribute .
2. Projection of a nested Attribute and not nested attribute (using older builder method).
3. Projection of Multiple Attributes
4. Projection of Null attributes passing Null NestedAttributeName --> Returns all elements as per the doc.

Tested getter for attributeForProject.

Screenshots (if appropriate)

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Checklist

License

I confirm that this pull request can be released under the Apache 2 license

.../dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/AttributeName.java

bmaizels · 2020-09-14T18:11:37Z

.../dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/AttributeName.java

+    /**
+     * @return List of nested attributes , each entry in the list represent one level of nesting.
+     */
+    public List getNestedAttributeNames() {


Why are you returning a raw-type list here?

.../dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/AttributeName.java

bmaizels · 2020-09-14T18:17:24Z

.../dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/AttributeName.java

+        private String attributeToProject;
+
+        private Builder() {
+


Nit: remove this empty line.

bmaizels · 2020-09-14T18:19:33Z

.../dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/AttributeName.java

+    private final String name;
+    private final List<String> nestedAttributeNames;
+
+    private AttributeName(String name, List<String> nestedAttributeNames) {


Minor: Our standard pattern is to have a constructor that takes an instance of a builder. Eg:

private AttributeName(Builder b) { ... }

.../dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/AttributeName.java

bmaizels · 2020-09-14T18:29:27Z

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

    private final List<String> attributesToProject;
+    private final List<AttributeName> attributeNamesToProject;


My opinion is that we should deprecate the List and just have a List. That will simplify our behavioral logic around this.

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

.../java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/utils/OperationUtils.java

dagnir · 2020-09-14T19:50:41Z

Looks like the previous PR was closed; for future PR's please use the existing pull request when making updates so we can maintain the history

...odb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/NestedAttributeName.java

bmaizels · 2020-09-16T18:05:25Z

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

@@ -347,7 +360,101 @@ public Builder addAttributeToProject(String attributeToProject) {
            return this;
        }

+        /**
+         * <p>
+         * Sets a collection of the NestedAttributeNames to be retrieved from the database. These attributes can include


Change 'sets' to 'adds'

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

...hanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/ScanEnhancedRequest.java

joviegas · 2020-09-18T09:06:42Z

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

@@ -192,7 +206,8 @@ public int hashCode() {
        private Integer limit;
        private Boolean consistentRead;
        private Expression filterExpression;
-        private List<String> attributesToProject;
+        private List<String> attributesToProjectStringList;


attributesToProjectStringList attribute is required to maintain the order of the older attributeToProject in cases of resets.

Is the order important? Conceptually I think of this as a 'set'.

...odb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/NestedAttributeName.java

bmaizels · 2020-09-18T18:10:04Z

...odb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/NestedAttributeName.java

+        this.elements = nestedAttributeNames != null
+                ? Collections.unmodifiableList(nestedAttributeNames) : null;


Having null elements should be invalid. Can we add a validator here?

Added for Null as well as Empty list

bmaizels · 2020-09-18T18:24:04Z

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

+        return attributesToProject != null ? attributesToProject.stream().filter(Objects::nonNull)
+                .map(item -> String.join(".", item.elements())).collect(Collectors.toList()) : null;


I feel like we should add validation to the constructor of the class to avoid the possibility of having null elements in attributesToProject then there would be no need to filter them here.

bmaizels · 2020-09-18T18:25:53Z

...anced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequest.java

@@ -192,7 +206,8 @@ public int hashCode() {
        private Integer limit;
        private Boolean consistentRead;
        private Expression filterExpression;
-        private List<String> attributesToProject;
+        private List<String> attributesToProjectStringList;


Is the order important? Conceptually I think of this as a 'set'.

bmaizels · 2020-09-18T18:28:47Z

...hanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/ScanEnhancedRequest.java

+            //If there is a reset then clear the attributesToProject List.
+            if (attributesToProjectStringList != null) {
+                attributesToProjectStringList
+                        .forEach(attribute -> this.attributesToProject.remove(NestedAttributeName.create(attribute)));
+            }


I think this is really confusing. We don't want to treat these as separate lists. We should make it clear in the javadoc that addNestedAttributeToProject is adding a 'nested attribute' to the 'attributesToProject' list. Let's make it clear to the customer there is just one list, and let's treat it like one list.

...rc/test/java/software/amazon/awssdk/enhanced/dynamodb/ProjectionExpressionConvertorTest.java

...d/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/BasicQueryTest.java

...d/src/test/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequestTest.java

bmaizels · 2020-09-18T18:39:45Z

...d/src/test/java/software/amazon/awssdk/enhanced/dynamodb/model/QueryEnhancedRequestTest.java

+                .addNestedAttributesToProject(NestedAttributeName.create("foo", "bar"))
+                .attributesToProject(additionalElement)
+                .build();
+        List<String> attributesToProjectStringAttributeOverwrittenNestedLast = Arrays.asList( "foo.bar", "three");


See my other comment in the code, I think the 'correct' state here should be "three", and the nested attribute should be overwritten with the others.

codecov-commenter · 2020-09-19T00:15:31Z

Codecov Report

Merging #2035 into master will increase coverage by 0.60%.
The diff coverage is 82.88%.

@@             Coverage Diff              @@
##             master    #2035      +/-   ##
============================================
+ Coverage     76.61%   77.22%   +0.60%     
  Complexity      225      225              
============================================
  Files          1113     1136      +23     
  Lines         33617    34841    +1224     
  Branches       2600     2726     +126     
============================================
+ Hits          25756    26906    +1150     
+ Misses         6581     6577       -4     
- Partials       1280     1358      +78

Flag	Coverage Δ	Complexity Δ
#unittests	`77.22% <82.88%> (+0.60%)`	`225.00 <0.00> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
...on/awssdk/codegen/maven/plugin/GenerationMojo.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../software/amazon/awssdk/codegen/CodeGenerator.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...are/amazon/awssdk/codegen/emitters/CodeWriter.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../amazon/awssdk/codegen/emitters/GeneratorTask.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...zon/awssdk/codegen/emitters/PoetGeneratorTask.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...n/awssdk/codegen/emitters/SimpleGeneratorTask.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...egen/emitters/tasks/AsyncClientGeneratorTasks.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...ssdk/codegen/emitters/tasks/AwsGeneratorTasks.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...itters/tasks/BaseExceptionClassGeneratorTasks.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...sdk/codegen/emitters/tasks/BaseGeneratorTasks.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
... and 185 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc695de...d429e78. Read the comment docs.

…ect Enhanced Scan and Enhanced query requests

sonarqubecloud · 2020-09-21T19:33:06Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

86.6% Coverage
8.2% Duplication

The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

Add support for bucket ARN parsing for object versioning APIs

joviegas requested review from bmaizels and dagnir September 14, 2020 04:59

bmaizels reviewed Sep 14, 2020

View reviewed changes

joviegas requested a review from bmaizels September 16, 2020 07:34

bmaizels reviewed Sep 16, 2020

View reviewed changes

joviegas requested a review from bmaizels September 18, 2020 09:04

joviegas commented Sep 18, 2020

View reviewed changes

bmaizels reviewed Sep 18, 2020

View reviewed changes

joviegas requested a review from bmaizels September 18, 2020 23:32

DDB Enhanced:Added support to read Nested objects in attributesToProj…

b984c82

…ect Enhanced Scan and Enhanced query requests

bmaizels approved these changes Sep 23, 2020

View reviewed changes

joviegas merged commit bc13f17 into aws:master Sep 23, 2020

aws-sdk-java-automation pushed a commit that referenced this pull request Sep 21, 2022

Merge pull request #2035 from aws/salande/object-versioning

f3e99c6

Add support for bucket ARN parsing for object versioning APIs

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DDB Enhanced:Added support to read Nested objects in attributesToProj… #2035

DDB Enhanced:Added support to read Nested objects in attributesToProj… #2035

joviegas commented Sep 14, 2020 •

edited

Loading

bmaizels Sep 14, 2020

bmaizels Sep 14, 2020

bmaizels Sep 14, 2020

bmaizels Sep 14, 2020

dagnir commented Sep 14, 2020

bmaizels Sep 16, 2020

joviegas Sep 18, 2020

bmaizels Sep 18, 2020

bmaizels Sep 18, 2020

joviegas Sep 18, 2020

bmaizels Sep 18, 2020

bmaizels Sep 18, 2020

bmaizels Sep 18, 2020

joviegas Sep 18, 2020 •

edited

Loading

bmaizels Sep 18, 2020

codecov-commenter commented Sep 19, 2020 •

edited

Loading

sonarqubecloud bot commented Sep 21, 2020

		private final List<String> attributesToProject;
		private final List<AttributeName> attributeNamesToProject;

		this.elements = nestedAttributeNames != null
		? Collections.unmodifiableList(nestedAttributeNames) : null;

		return attributesToProject != null ? attributesToProject.stream().filter(Objects::nonNull)
		.map(item -> String.join(".", item.elements())).collect(Collectors.toList()) : null;

DDB Enhanced:Added support to read Nested objects in attributesToProj… #2035

DDB Enhanced:Added support to read Nested objects in attributesToProj… #2035

Conversation

joviegas commented Sep 14, 2020 • edited Loading

Description DDB Enhanced: Added support for Nested objects projection expressions RequestOnPR

Motivation and Context

Fix

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagnir commented Sep 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joviegas Sep 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 19, 2020 • edited Loading

Codecov Report

sonarqubecloud bot commented Sep 21, 2020

joviegas commented Sep 14, 2020 •

edited

Loading

joviegas Sep 18, 2020 •

edited

Loading

codecov-commenter commented Sep 19, 2020 •

edited

Loading