feat: add ResolveAttribute #412

ajewellamz · 2023-09-13T13:51:30Z

Issue #, if available:

Description of changes:

Add ResolveAttribute
Also change build to preferentially use MavenLocal over MavenCentral

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

lucasmcdonald3

Can we add a similar demonstration for the compound beacon example?

We can validate that resolveOutput.CompoundBeacons contains the expected value.

lucasmcdonald3 · 2023-09-13T16:18:02Z

...zon/cryptography/examples/searchableencryption/VirtualBeaconSearchableEncryptionExample.java

@@ -417,6 +424,21 @@ public static void PutItemQueryItemWithVirtualBeacon(String ddbTableName, String
    // Validate the item has the expected attributes
    assert returnedItem.get("state").s().equals("CA");
    assert returnedItem.get("hasTestResult").bool().equals(true);
+
+    // 14. Check virtual field values directly


Should this go immediately after we construct the item? Maybe line 357?
When I see this code located after my queries, I'm expecting that I need the result of some query to validate my beacon transforms. But I actually only need to construct the item. I think we can move this to immediately after we construct the item as a validation on our config and item up until that point.

I like it at the end, because it's something a customer wouldn't normally do.

I updated the comment to make that clearer.

lucasmcdonald3 · 2023-09-13T16:24:14Z

...zon/cryptography/examples/searchableencryption/VirtualBeaconSearchableEncryptionExample.java

+    Map<String, String> vf = new HashMap<>();
+    assert resolveOutput.CompoundBeacons().equals(vf);


Suggested change

Map<String, String> vf = new HashMap<>();

assert resolveOutput.CompoundBeacons().equals(vf);

// We did not construct any compound beacons

assert resolveOutput.CompoundBeacons().isEmpty();

// Validate our virtual field is constructed as expected

Map<String, String> vf = new HashMap<>();

lucasmcdonald3 · 2023-09-13T16:57:20Z

...on/cryptography/examples/searchableencryption/CompoundBeaconSearchableEncryptionExample.java

+    final HashMap<String, AttributeValue> item = new HashMap<>();
+    item.put("work_id", AttributeValue.builder().s("9ce39272-8068-4efd-a211-cd162ad65d4c").build());
+    item.put("inspection_date", AttributeValue.builder().s("2023-06-13").build());
+    item.put("inspector_id_last4", AttributeValue.builder().s("5678").build());
+    item.put("unit", AttributeValue.builder().s("011899988199").build());


I somehow want to point this at the config on line ~296 so readers know where this is coming from. Maybe either

Refactor into one (e.g.) createHashmapForExampleItem function, call from both locations

Add a comment here "this is the same item config as in the PutAndQueryItemWithCompoundBeacon function"

lucasmcdonald3 · 2023-09-13T16:57:43Z

...on/cryptography/examples/searchableencryption/CompoundBeaconSearchableEncryptionExample.java

+        .build();
+
+    final ResolveAttributesOutput resolveOutput = trans.ResolveAttributes(resolveInput);
+    Map<String, String> vf = new HashMap<>();


vf -> compoundBeacons or cbs?

lucasmcdonald3 · 2023-09-13T16:57:58Z

...on/cryptography/examples/searchableencryption/CompoundBeaconSearchableEncryptionExample.java

+    Map<String, String> vf = new HashMap<>();
+
+    // VirtualFields is empty because we have no Virtual Fields configured
+    assert resolveOutput.VirtualFields().equals(vf);


Suggested change

assert resolveOutput.VirtualFields().equals(vf);

assert resolveOutput.VirtualFields().isEmpty();

lucasmcdonald3 · 2023-09-13T16:58:16Z

...zon/cryptography/examples/searchableencryption/VirtualBeaconSearchableEncryptionExample.java

+    Map<String, String> vf = new HashMap<>();
+
+    // CompoundBeacons is empty because we have no Compound Beacons configured
+    assert resolveOutput.CompoundBeacons().equals(vf);


Suggested change

assert resolveOutput.CompoundBeacons().equals(vf);

assert resolveOutput.CompoundBeacons().isEmpty();

lavaleri · 2023-09-13T17:30:49Z

.github/workflows/ci_test_java.yml

-          # Clear MPL from cache
-          # We have to do this because MakeFile does not do this yet. The MakeFile automatically builds and deploys dependencies
-          # instead it should be picking it up from Maven.
-          rm -rf ~/.m2/repository/software/amazon/cryptography/aws-cryptographic-material-providers


What is the result of this change, and why is it necessary for these changes? Does this mean we are now testing against the locally built version (what the submodule points to)?

Should we instead update the Makefile to include a build target that does not recursively build/locally deploy the MPL when building the DB-ESDK?

This change will make our local development simpler, as we can reason about always verifying/testing against the MPL submodule. This also allows us to develop feature branches in this repo against pending releases in the MPL, when no official release yet exists in Maven Central.

This is safe as long as we ensure at release time that we are testing against the MPL in Maven Central, and have some additional check to ensure that the commit of the submodule matches the commit that was released for the MPL version configured in the build.gradle.

Talking offline with @ajewellamz, we will remove this change from this PR for now, and reintroduce it once we have those additional mechanisms in place.

It looks like our release script does test the staging artifact without a locally built MPL (https://github.com/aws/aws-database-encryption-sdk-dynamodb-java/blob/main/codebuild/staging/validate-staging.yml#L54-L57), so we just need to ensure that we have an additional check that the submodule matches the version in our build.gradle (This can start out as a manual check).

DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/Model/DynamoDbEncryptionTransforms.smithy

…namoDbEncryptionTransforms.smithy Co-authored-by: Lucas McDonald <[email protected]>

lucasmcdonald3 · 2023-09-13T17:50:36Z

LGTM pending comments from @lavaleri .

* feat: add ResolveAttribute

feat: add ResolveAttribute

ad5d5ad

ajewellamz requested a review from a team as a code owner September 13, 2023 13:51

m

f8d3afd

ajewellamz requested review from lucasmcdonald3 and lavaleri September 13, 2023 15:26

lucasmcdonald3 reviewed Sep 13, 2023

View reviewed changes

ajewellamz added 2 commits September 13, 2023 12:26

m

3b39e17

m

b7cd26d

lucasmcdonald3 reviewed Sep 13, 2023

View reviewed changes

m

dcdd761

lavaleri reviewed Sep 13, 2023

View reviewed changes

lucasmcdonald3 reviewed Sep 13, 2023

View reviewed changes

DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/Model/DynamoDbEncryptionTransforms.smithy Outdated Show resolved Hide resolved

Update DynamoDbEncryption/dafny/DynamoDbEncryptionTransforms/Model/Dy…

38c6856

…namoDbEncryptionTransforms.smithy Co-authored-by: Lucas McDonald <[email protected]>

m

0b44ca8

lucasmcdonald3 approved these changes Sep 13, 2023

View reviewed changes

ajewellamz merged commit 26f3eba into main Sep 13, 2023

ajewellamz deleted the resolve-atribute branch September 13, 2023 19:38

ajewellamz added a commit that referenced this pull request Nov 14, 2023

feat: add ResolveAttribute (#412)

09e23c5

* feat: add ResolveAttribute

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add ResolveAttribute #412

feat: add ResolveAttribute #412

Uh oh!

ajewellamz commented Sep 13, 2023

Uh oh!

lucasmcdonald3 left a comment

Uh oh!

lucasmcdonald3 Sep 13, 2023

Uh oh!

ajewellamz Sep 13, 2023

Uh oh!

lucasmcdonald3 Sep 13, 2023

Uh oh!

lucasmcdonald3 Sep 13, 2023

Uh oh!

lucasmcdonald3 Sep 13, 2023

Uh oh!

lucasmcdonald3 Sep 13, 2023

Uh oh!

lucasmcdonald3 Sep 13, 2023

Uh oh!

lavaleri Sep 13, 2023

Uh oh!

lavaleri Sep 13, 2023

Uh oh!

ajewellamz Sep 13, 2023

Uh oh!

lavaleri Sep 13, 2023

Uh oh!

Uh oh!

lucasmcdonald3 commented Sep 13, 2023

Uh oh!

Uh oh!

		Map<String, String> vf = new HashMap<>();
		assert resolveOutput.CompoundBeacons().equals(vf);

	assert resolveOutput.VirtualFields().equals(vf);
	assert resolveOutput.VirtualFields().isEmpty();

	assert resolveOutput.CompoundBeacons().equals(vf);
	assert resolveOutput.CompoundBeacons().isEmpty();

feat: add ResolveAttribute #412

feat: add ResolveAttribute #412

Uh oh!

Conversation

ajewellamz commented Sep 13, 2023

Uh oh!

lucasmcdonald3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lucasmcdonald3 commented Sep 13, 2023

Uh oh!

Uh oh!