Skip to content

Commit 239d286

Browse files
author
Anirav Kareddy
committed
Fixed changes from first PR review
1 parent 575627e commit 239d286

File tree

5 files changed

+102
-55
lines changed

5 files changed

+102
-55
lines changed

src/main/java/software/amazon/encryption/s3/internal/ContentMetadata.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
66
import software.amazon.encryption.s3.algorithms.AlgorithmSuite;
77
import software.amazon.encryption.s3.materials.EncryptedDataKey;
8+
import software.amazon.encryption.s3.materials.MaterialsDescription;
89

910
import java.util.Collections;
1011
import java.util.Map;
@@ -21,6 +22,7 @@ public class ContentMetadata {
2122
private final String _contentCipher;
2223
private final String _contentCipherTagLength;
2324
private final String _contentRange;
25+
private final MaterialsDescription _materialsDescription;
2426

2527
private ContentMetadata(Builder builder) {
2628
_algorithmSuite = builder._algorithmSuite;
@@ -33,6 +35,7 @@ private ContentMetadata(Builder builder) {
3335
_contentCipher = builder._contentCipher;
3436
_contentCipherTagLength = builder._contentCipherTagLength;
3537
_contentRange = builder._contentRange;
38+
_materialsDescription = builder._materialsDescription;
3639
}
3740

3841
public static Builder builder() {
@@ -51,6 +54,10 @@ public String encryptedDataKeyAlgorithm() {
5154
return _encryptedDataKeyAlgorithm;
5255
}
5356

57+
public MaterialsDescription materialsDescription() {
58+
return _materialsDescription;
59+
}
60+
5461
/**
5562
* Note that the underlying implementation uses a Collections.unmodifiableMap which is
5663
* immutable.
@@ -91,6 +98,7 @@ public static class Builder {
9198
private String _contentCipher;
9299
private String _contentCipherTagLength;
93100
public String _contentRange;
101+
private MaterialsDescription _materialsDescription;
94102

95103
private Builder() {
96104

@@ -125,6 +133,10 @@ public Builder contentRange(String contentRange) {
125133
_contentRange = contentRange;
126134
return this;
127135
}
136+
public Builder materialsDescription(MaterialsDescription materialsDescription) {
137+
_materialsDescription = materialsDescription;
138+
return this;
139+
}
128140

129141
public ContentMetadata build() {
130142
return new ContentMetadata(this);

src/main/java/software/amazon/encryption/s3/materials/AesKeyring.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,6 @@ public Builder reEncryptInstructionFile(boolean reEncryptInstructionFile) {
211211
}
212212

213213
public AesKeyring build() {
214-
if (_reEncryptInstructionFile && _materialsDescription == null) {
215-
throw new S3EncryptionClientException("Materials description must be provided for re-encrypt instruction file!");
216-
}
217214
return new AesKeyring(this);
218215
}
219216
}

src/main/java/software/amazon/encryption/s3/materials/MaterialsDescription.java

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,19 @@
33

44
package software.amazon.encryption.s3.materials;
55

6+
import java.util.Collection;
67
import java.util.Collections;
78
import java.util.HashMap;
9+
import java.util.List;
810
import java.util.Map;
11+
import java.util.Set;
12+
913
/**
10-
* This class is used to store and manage key-value pairs that describe keyring,specifically for AES and RSA Keyring.
11-
* This will be useful during re-encryption of instruction file.
12-
* The stored Materials Description are immutable once created.
14+
* This class is used to provide key-value pairs that describe the key material used with the Keyring, specifically for AES and RSA Keyrings.
15+
* This will be useful during the re-encryption of instruction file.
16+
* The stored Materials Description is immutable once created.
1317
*/
14-
15-
public class MaterialsDescription {
18+
public class MaterialsDescription implements Map<String, String> {
1619
private final Map<String, String> materialsDescription;
1720

1821
private MaterialsDescription(Builder builder) {
@@ -21,10 +24,72 @@ private MaterialsDescription(Builder builder) {
2124
public static Builder builder() {
2225
return new Builder();
2326
}
24-
public Map<String, String> getDescription() {
27+
public Map<String, String> getMaterialsDescription() {
2528
return this.materialsDescription;
2629
}
27-
public static class Builder {
30+
31+
@Override
32+
public int size() {
33+
return materialsDescription.size();
34+
}
35+
36+
@Override
37+
public boolean isEmpty() {
38+
return materialsDescription.isEmpty();
39+
}
40+
41+
@Override
42+
public boolean containsKey(Object key) {
43+
return materialsDescription.containsKey(key);
44+
}
45+
46+
@Override
47+
public boolean containsValue(Object value) {
48+
return materialsDescription.containsValue(value);
49+
}
50+
51+
@Override
52+
public String get(Object key) {
53+
return materialsDescription.get(key);
54+
}
55+
56+
@Override
57+
public String put(String key, String value) {
58+
return materialsDescription.put(key, value);
59+
}
60+
61+
@Override
62+
public String remove(Object key) {
63+
return materialsDescription.remove(key);
64+
}
65+
66+
@Override
67+
public void putAll(Map<? extends String, ? extends String> m) {
68+
materialsDescription.putAll(m);
69+
}
70+
71+
@Override
72+
public void clear() {
73+
materialsDescription.clear();
74+
}
75+
76+
@Override
77+
public Set<String> keySet() {
78+
return materialsDescription.keySet();
79+
}
80+
81+
@Override
82+
public Collection<String> values() {
83+
return materialsDescription.values();
84+
}
85+
86+
@Override
87+
public Set<Entry<String, String>> entrySet() {
88+
return materialsDescription.entrySet();
89+
}
90+
91+
92+
public static class Builder {
2893
private final Map<String, String> materialsDescription = new HashMap<>();
2994
public Builder put(String key, String value) {
3095
if (key == null || value == null) {

src/main/java/software/amazon/encryption/s3/materials/RsaKeyring.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,6 @@ public Builder reEncryptInstructionFile(final boolean reEncryptInstructionFile)
224224
}
225225

226226
public RsaKeyring build() {
227-
if (_reEncryptInstructionFile && _materialsDescription == null) {
228-
throw new S3EncryptionClientException("Materials description must be provided for re-encrypt instruction file!");
229-
}
230227
return new RsaKeyring(this);
231228
}
232229

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

Lines changed: 18 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static org.junit.jupiter.api.Assertions.assertNotNull;
1717
import static org.junit.jupiter.api.Assertions.assertNull;
1818
import static org.junit.jupiter.api.Assertions.assertTrue;
19+
import static org.junit.jupiter.api.Assertions.fail;
1920

2021
public class MaterialsDescriptionTest {
2122
private static SecretKey AES_KEY;
@@ -36,17 +37,17 @@ public void testSimpleMaterialsDescription() {
3637
MaterialsDescription materialsDescription = MaterialsDescription.builder()
3738
.put("version", "1.0")
3839
.build();
39-
assertEquals("1.0", materialsDescription.getDescription().get("version"));
40-
assertEquals(1, materialsDescription.getDescription().size());
40+
assertEquals("1.0", materialsDescription.getMaterialsDescription().get("version"));
41+
assertEquals(1, materialsDescription.getMaterialsDescription().size());
4142
try {
42-
materialsDescription.getDescription().put("version", "2.0");
43-
throw new RuntimeException("Expected UnsupportedOperationException");
43+
materialsDescription.getMaterialsDescription().put("version", "2.0");
44+
fail("Expected UnsupportedOperationException!");
4445
} catch (UnsupportedOperationException e) {
4546
assertNull(e.getMessage());
4647
}
4748
try {
48-
materialsDescription.getDescription().clear();
49-
throw new RuntimeException("Expected UnsupportedOperationException");
49+
materialsDescription.getMaterialsDescription().clear();
50+
fail("Expected UnsupportedOperationException!");
5051
} catch (UnsupportedOperationException e) {
5152
assertNull(e.getMessage());
5253
}
@@ -59,11 +60,11 @@ public void testMaterialsDescriptionPutAll() {
5960
MaterialsDescription materialsDescription = MaterialsDescription.builder()
6061
.putAll(description)
6162
.build();
62-
assertEquals(2, materialsDescription.getDescription().size());
63-
assertTrue(materialsDescription.getDescription().containsKey("version"));
64-
assertTrue(materialsDescription.getDescription().containsKey("next-version"));
65-
assertEquals("1.0", materialsDescription.getDescription().get("version"));
66-
assertEquals("2.0", materialsDescription.getDescription().get("next-version"));
63+
assertEquals(2, materialsDescription.getMaterialsDescription().size());
64+
assertTrue(materialsDescription.getMaterialsDescription().containsKey("version"));
65+
assertTrue(materialsDescription.getMaterialsDescription().containsKey("next-version"));
66+
assertEquals("1.0", materialsDescription.getMaterialsDescription().get("version"));
67+
assertEquals("2.0", materialsDescription.getMaterialsDescription().get("next-version"));
6768
}
6869
@Test
6970
public void testMaterialsDescriptionAesKeyring() {
@@ -76,9 +77,9 @@ public void testMaterialsDescriptionAesKeyring() {
7677
.build())
7778
.build();
7879
assertNotNull(aesKeyring.getMaterialsDescription());
79-
assertEquals("1.0", aesKeyring.getMaterialsDescription().getDescription().get("version"));
80-
assertEquals("yes", aesKeyring.getMaterialsDescription().getDescription().get("admin"));
81-
assertEquals(2, aesKeyring.getMaterialsDescription().getDescription().size());
80+
assertEquals("1.0", aesKeyring.getMaterialsDescription().getMaterialsDescription().get("version"));
81+
assertEquals("yes", aesKeyring.getMaterialsDescription().getMaterialsDescription().get("admin"));
82+
assertEquals(2, aesKeyring.getMaterialsDescription().getMaterialsDescription().size());
8283

8384
}
8485
@Test
@@ -93,35 +94,10 @@ public void testMaterialsDescriptionRsaKeyring() {
9394
.build())
9495
.build();
9596
assertNotNull(rsaKeyring);
96-
assertEquals("1.0", rsaKeyring.getMaterialsDescription().getDescription().get("version"));
97-
assertEquals("yes", rsaKeyring.getMaterialsDescription().getDescription().get("admin"));
98-
assertEquals(2, rsaKeyring.getMaterialsDescription().getDescription().size());
97+
assertEquals("1.0", rsaKeyring.getMaterialsDescription().getMaterialsDescription().get("version"));
98+
assertEquals("yes", rsaKeyring.getMaterialsDescription().getMaterialsDescription().get("admin"));
99+
assertEquals(2, rsaKeyring.getMaterialsDescription().getMaterialsDescription().size());
99100

100101
}
101-
@Test
102-
public void testMaterialsDescriptionRsaKeyringWithNoReEncrypt() {
103-
PartialRsaKeyPair keyPair = new PartialRsaKeyPair(RSA_KEY_PAIR.getPrivate(), RSA_KEY_PAIR.getPublic());
104-
try {
105-
RsaKeyring.builder()
106-
.wrappingKeyPair(keyPair)
107-
.reEncryptInstructionFile(true)
108-
.build();
109-
throw new RuntimeException("Expected failure!");
110-
} catch (S3EncryptionClientException e) {
111-
assertTrue(e.getMessage().contains("Materials description must be provided for re-encrypt instruction file!"));
112-
}
113-
}
114-
@Test
115-
public void testMaterialsDescriptionAesKeyringWithNoReEncrypt() {
116-
try {
117-
AesKeyring.builder()
118-
.wrappingKey(AES_KEY)
119-
.reEncryptInstructionFile(true)
120-
.build();
121-
throw new RuntimeException("Expected fa");
122-
} catch (S3EncryptionClientException e) {
123-
assertTrue(e.getMessage().contains("Materials description must be provided for re-encrypt instruction file!"));
124-
}
125-
}
126102

127103
}

0 commit comments

Comments
 (0)