Skip to content

Commit fbf4b41

Browse files
pjdartonpjdarton
committed
Improve code docs
1 parent 5718d2f commit fbf4b41

File tree

4 files changed

+63
-48
lines changed

4 files changed

+63
-48
lines changed

‎src/main/java/io/jenkins/plugins/credentials/secretsmanager/factory/BaseAwsJsonCredentials.java‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,19 @@
2323
@Restricted(NoExternalUse.class)
2424
public abstract class BaseAwsJsonCredentials extends BaseStandardCredentials {
2525
/** How to access the JSON */
26-
private final Supplier<Secret> supplierOfSecretData;
26+
private final Supplier<Secret> json;
2727

2828
/**
2929
* Constructs a new instance with new data.
3030
*
31-
* @param id The value for {@link #getId()}.
32-
* @param description The value for {@link #getDescription()}.
33-
* @param secretJsonSupplier Supplies the data that {@link #getSecretJson()}
34-
* will decode.
31+
* @param id The value for {@link #getId()}.
32+
* @param description The value for {@link #getDescription()}.
33+
* @param json Supplies the data that {@link #getSecretJson()} will
34+
* decode.
3535
*/
36-
protected BaseAwsJsonCredentials(String id, String description, Supplier<Secret> secretJsonSupplier) {
36+
protected BaseAwsJsonCredentials(String id, String description, Supplier<Secret> json) {
3737
super(id, description);
38-
this.supplierOfSecretData = secretJsonSupplier;
38+
this.json = json;
3939
}
4040

4141
/**
@@ -45,14 +45,15 @@ protected BaseAwsJsonCredentials(String id, String description, Supplier<Secret>
4545
*/
4646
protected BaseAwsJsonCredentials(BaseAwsJsonCredentials toSnapshot) {
4747
super(toSnapshot.getId(), toSnapshot.getDescription());
48-
final Secret secretDataToSnapshot = toSnapshot.supplierOfSecretData.get();
49-
this.supplierOfSecretData = new Snapshot<Secret>(secretDataToSnapshot);
48+
final Secret secretDataToSnapshot = toSnapshot.json.get();
49+
this.json = new Snapshot<Secret>(secretDataToSnapshot);
5050
}
5151

5252
// Note:
5353
// We MUST NOT tell anyone what the JSON is, or give any hints as to its
54-
// contents, as that could then leak sensitive data so we have to suppress the
55-
// informative exception(s) and just tell the user that it didn't work.
54+
// contents, as that could then leak sensitive data so, if anything goes wrong,
55+
// we have to suppress the informative exception(s) and just tell the user that
56+
// it didn't work.
5657

5758
/**
5859
* Reads the secret JSON and returns the field requested.
@@ -98,7 +99,7 @@ protected String getOptionalField(@NonNull JSONObject secretJson, @NonNull Strin
9899
*/
99100
@NonNull
100101
protected JSONObject getSecretJson() {
101-
final Secret secret = supplierOfSecretData.get();
102+
final Secret secret = json.get();
102103
final String rawSecretJson = secret == null ? "" : secret.getPlainText();
103104
if (rawSecretJson.isEmpty()) {
104105
throw new CredentialsUnavailableException("secret", Messages.noValidJsonError(getId()));

‎src/main/java/io/jenkins/plugins/credentials/secretsmanager/factory/ssh_user_private_key/AwsJsonSshUserPrivateKey.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@
1919
import io.jenkins.plugins.credentials.secretsmanager.Messages;
2020
import io.jenkins.plugins.credentials.secretsmanager.factory.BaseAwsJsonCredentials;
2121

22+
/**
23+
* Similar to {@link AwsSshUserPrivateKey} but expects the AWS secret data to be
24+
* JSON containing two or three fields, {@value #JSON_FIELDNAME_FOR_USERNAME},
25+
* {@value #JSON_FIELDNAME_FOR_PRIVATE_KEY} and optionally a
26+
* {@value #JSON_FIELDNAME_FOR_PASSPHRASE} that provide the username, key and
27+
* (optional) passphrase. The secret JSON may contain other fields too, but
28+
* we'll ignore them.
29+
*/
2230
public class AwsJsonSshUserPrivateKey extends BaseAwsJsonCredentials implements SSHUserPrivateKey {
2331
/**
2432
* Name of the JSON field that we expect to be present and to contain the
@@ -42,17 +50,16 @@ public class AwsJsonSshUserPrivateKey extends BaseAwsJsonCredentials implements
4250
/**
4351
* Constructs a new instance.
4452
*
45-
* @param id The value for {@link #getId()}.
46-
* @param description The value for {@link #getDescription()}.
47-
* @param jsonContainingPrivateKey Supplies JSON containing a
48-
* {@value #JSON_FIELDNAME_FOR_USERNAME} field,
49-
* a {@value #JSON_FIELDNAME_FOR_PRIVATE_KEY}
50-
* field and optionally a
51-
* {@value #JSON_FIELDNAME_FOR_PASSPHRASE}
52-
* field.
53+
* @param id The value for {@link #getId()}.
54+
* @param description The value for {@link #getDescription()}.
55+
* @param json Supplies JSON containing a
56+
* {@value #JSON_FIELDNAME_FOR_USERNAME} field, a
57+
* {@value #JSON_FIELDNAME_FOR_PRIVATE_KEY} field and
58+
* optionally a {@value #JSON_FIELDNAME_FOR_PASSPHRASE}
59+
* field.
5360
*/
54-
public AwsJsonSshUserPrivateKey(String id, String description, Supplier<Secret> jsonContainingPrivateKey) {
55-
super(id, description, jsonContainingPrivateKey);
61+
public AwsJsonSshUserPrivateKey(String id, String description, Supplier<Secret> json) {
62+
super(id, description, json);
5663
}
5764

5865
/**

‎src/main/java/io/jenkins/plugins/credentials/secretsmanager/factory/username_password/AwsJsonUsernamePasswordCredentials.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,14 @@ public class AwsJsonUsernamePasswordCredentials extends BaseAwsJsonCredentials
4242
/**
4343
* Constructs a new instance.
4444
*
45-
* @param id The value for {@link #getId()}.
46-
* @param description The value for {@link #getDescription()}.
47-
* @param usernameAndPasswordJson Supplies JSON containing a
48-
* {@value #JSON_FIELDNAME_FOR_USERNAME} field
49-
* and a {@value #JSON_FIELDNAME_FOR_PASSWORD}
50-
* field.
45+
* @param id The value for {@link #getId()}.
46+
* @param description The value for {@link #getDescription()}.
47+
* @param json Supplies JSON containing a
48+
* {@value #JSON_FIELDNAME_FOR_USERNAME} field and a
49+
* {@value #JSON_FIELDNAME_FOR_PASSWORD} field.
5150
*/
52-
public AwsJsonUsernamePasswordCredentials(String id, String description, Supplier<Secret> usernameAndPasswordJson) {
53-
super(id, description, usernameAndPasswordJson);
51+
public AwsJsonUsernamePasswordCredentials(String id, String description, Supplier<Secret> json) {
52+
super(id, description, json);
5453
}
5554

5655
/**

‎src/test/java/io/jenkins/plugins/credentials/secretsmanager/factory/BaseAwsJsonCredentialsSnapshotTakerTest.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,11 @@ public void snapshotGivenInstanceThenCreatesSnapshot() throws Exception {
108108
// Given
109109
final ST instance = makeInstanceOrThrow();
110110
final C credential = makeCredential();
111-
final Method[] allMethods = credentialBeingSnapshotted.getDeclaredMethods();
112-
final Map<Method, Object> expectedResults = new IdentityHashMap<>();
113-
for (final Method m : allMethods) {
111+
final String expectedId = credential.getId();
112+
final String expectedDescription = credential.getDescription();
113+
// Use reflection to call all the getter methods
114+
final Map<Method, Object> expectedSnapshotContents = new IdentityHashMap<>();
115+
for (final Method m : credentialBeingSnapshotted.getDeclaredMethods()) {
114116
if (m.getParameterCount() != 0 || !m.isAccessible()) {
115117
continue;
116118
}
@@ -120,35 +122,37 @@ public void snapshotGivenInstanceThenCreatesSnapshot() throws Exception {
120122
} catch (IllegalAccessException e) {
121123
continue;
122124
}
123-
expectedResults.put(m, expectedValue);
125+
expectedSnapshotContents.put(m, expectedValue);
124126
}
125-
final String expectedId = credential.getId();
126-
final String expectedDescription = credential.getDescription();
127127

128128
// When
129129
final C actual = instance.snapshot(credential);
130-
limitedUseSupplier.noFurtherUsePermitted();
131-
final String actualId = actual.getId();
132-
final String actualDescription = actual.getDescription();
133-
final Map<Method, Object> actualResults = new IdentityHashMap<>();
134-
for (final Method m : expectedResults.keySet()) {
135-
final Object actualValue = m.invoke(actual);
136-
actualResults.put(m, actualValue);
137-
}
130+
limitedUseSupplier.stopAnyFurtherCallsToTheSupplier();
138131

139132
// Then
140133
assertThat(actual).isNotNull();
141134
assertThat(actual).isNotSameAs(credential);
142135
assertThat(actual).isInstanceOf(credentialBeingSnapshotted);
136+
// ...now check it contains the right data AND that getting that data doesn't
137+
// trigger the supplier.
138+
final String actualId = actual.getId();
143139
assertThat(actualId).as("getId()").isEqualTo(expectedId);
140+
final String actualDescription = actual.getDescription();
144141
assertThat(actualDescription).as("getDescription").isEqualTo(expectedDescription);
145-
for (final Method m : expectedResults.keySet()) {
146-
final Object expectedValue = expectedResults.get(m);
147-
final Object actualValue = actualResults.get(m);
142+
for (final Method m : expectedSnapshotContents.keySet()) {
143+
final Object expectedValue = expectedSnapshotContents.get(m);
144+
final Object actualValue = m.invoke(actual);
148145
assertThat(actualValue).as(m.toGenericString()).isEqualTo(expectedValue);
149146
}
150147
}
151148

149+
/**
150+
* {@link Supplier} that can be told to stop supplying so that we can verify
151+
* that accessing a "snapshot" is using snapshotted values rather than still
152+
* using live values from here.
153+
*
154+
* @param <T> Type being supplied.
155+
*/
152156
private static class LimitedUseSupplier<T> implements Supplier<T> {
153157
private final T supplied;
154158
private boolean noFurtherUsePermitted = false;
@@ -157,7 +161,11 @@ public LimitedUseSupplier(T supplied) {
157161
this.supplied = supplied;
158162
}
159163

160-
public void noFurtherUsePermitted() {
164+
/**
165+
* Causes all subsequent calls to {@link #get()} to throw an
166+
* {@link IllegalStateException}.
167+
*/
168+
public void stopAnyFurtherCallsToTheSupplier() {
161169
noFurtherUsePermitted = true;
162170
}
163171

0 commit comments

Comments
 (0)