Skip to content

Commit 7dac554

Browse files
committed
Review comments
1 parent c05de37 commit 7dac554

File tree

3 files changed

+31
-31
lines changed

3 files changed

+31
-31
lines changed

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,11 @@ private void recordChecksumBusinessMetrics(ExecutionAttributes executionAttribut
374374
executionAttributes.getAttribute(SdkInternalExecutionAttribute.RESPONSE_CHECKSUM_VALIDATION))
375375
.ifPresent(businessMetrics::addMetric);
376376

377-
BusinessMetricsUtils.resolveChecksumSpecsMetric(
378-
executionAttributes.getAttribute(RESOLVED_CHECKSUM_SPECS))
379-
.ifPresent(businessMetrics::addMetric);
377+
ChecksumSpecs checksumSpecs = executionAttributes.getAttribute(RESOLVED_CHECKSUM_SPECS);
378+
if (checksumSpecs != null && checksumSpecs.algorithmV2() != null) {
379+
BusinessMetricsUtils.resolveChecksumAlgorithmMetric(checksumSpecs.algorithmV2())
380+
.ifPresent(businessMetrics::addMetric);
381+
}
380382
}
381383

382384
static final class ChecksumCalculatingStreamProvider implements ContentStreamProvider {

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/useragent/BusinessMetricsUtils.java

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import software.amazon.awssdk.annotations.SdkInternalApi;
2020
import software.amazon.awssdk.checksums.DefaultChecksumAlgorithm;
2121
import software.amazon.awssdk.checksums.spi.ChecksumAlgorithm;
22-
import software.amazon.awssdk.core.checksums.ChecksumSpecs;
2322
import software.amazon.awssdk.core.checksums.RequestChecksumCalculation;
2423
import software.amazon.awssdk.core.checksums.ResponseChecksumValidation;
2524
import software.amazon.awssdk.core.retry.RetryMode;
@@ -63,24 +62,26 @@ public static Optional<String> resolveRetryMode(RetryPolicy retryPolicy, RetrySt
6362

6463
public static Optional<String> resolveRequestChecksumCalculationMetric(
6564
RequestChecksumCalculation requestChecksumCalculation) {
66-
if (requestChecksumCalculation == RequestChecksumCalculation.WHEN_SUPPORTED) {
67-
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED.value());
65+
switch (requestChecksumCalculation) {
66+
case WHEN_SUPPORTED:
67+
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED.value());
68+
case WHEN_REQUIRED:
69+
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED.value());
70+
default:
71+
return Optional.empty();
6872
}
69-
if (requestChecksumCalculation == RequestChecksumCalculation.WHEN_REQUIRED) {
70-
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED.value());
71-
}
72-
return Optional.empty();
7373
}
7474

7575
public static Optional<String> resolveResponseChecksumValidationMetric(
7676
ResponseChecksumValidation responseChecksumValidation) {
77-
if (responseChecksumValidation == ResponseChecksumValidation.WHEN_SUPPORTED) {
78-
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED.value());
77+
switch (responseChecksumValidation) {
78+
case WHEN_SUPPORTED:
79+
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED.value());
80+
case WHEN_REQUIRED:
81+
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED.value());
82+
default:
83+
return Optional.empty();
7984
}
80-
if (responseChecksumValidation == ResponseChecksumValidation.WHEN_REQUIRED) {
81-
return Optional.of(BusinessMetricFeatureId.FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED.value());
82-
}
83-
return Optional.empty();
8485
}
8586

8687
public static Optional<String> resolveChecksumAlgorithmMetric(ChecksumAlgorithm algorithm) {
@@ -107,10 +108,4 @@ public static Optional<String> resolveChecksumAlgorithmMetric(ChecksumAlgorithm
107108
return Optional.empty();
108109
}
109110

110-
public static Optional<String> resolveChecksumSpecsMetric(ChecksumSpecs checksumSpecs) {
111-
if (checksumSpecs != null && checksumSpecs.algorithmV2() != null) {
112-
return resolveChecksumAlgorithmMetric(checksumSpecs.algorithmV2());
113-
}
114-
return Optional.empty();
115-
}
116111
}

test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/FlexibleChecksumBusinessMetricTest.java

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,11 @@ void when_noChecksumConfigurationIsSet_defaultConfigMetricsAreAdded() {
6868
client.allTypes(r -> {});
6969
String userAgent = getUserAgentFromLastRequest();
7070

71-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply("Z"));
72-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply("b"));
73-
assertThat(userAgent).doesNotMatch(METRIC_SEARCH_PATTERN.apply("a"));
74-
assertThat(userAgent).doesNotMatch(METRIC_SEARCH_PATTERN.apply("c"));
71+
assertThat(userAgent)
72+
.matches(METRIC_SEARCH_PATTERN.apply("Z"))
73+
.matches(METRIC_SEARCH_PATTERN.apply("b"))
74+
.doesNotMatch(METRIC_SEARCH_PATTERN.apply("a"))
75+
.doesNotMatch(METRIC_SEARCH_PATTERN.apply("c"));
7576
}
7677

7778
@ParameterizedTest
@@ -117,8 +118,9 @@ void when_checksumConfigurationIsSet_correctMetricIsAdded(RequestChecksumCalcula
117118
client.allTypes(r -> {});
118119

119120
String userAgent = getUserAgentFromLastRequest();
120-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(expectedRequestMetric));
121-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(expectedResponseMetric));
121+
assertThat(userAgent)
122+
.matches(METRIC_SEARCH_PATTERN.apply(expectedRequestMetric))
123+
.matches(METRIC_SEARCH_PATTERN.apply(expectedResponseMetric));
122124
}
123125

124126
static Stream<Arguments> checksumConfigurationTestCases() {
@@ -157,9 +159,10 @@ void when_checksumConfigurationAndAlgorithmAreSet_correctMetricsAreAdded(
157159

158160
String userAgent = getUserAgentFromLastRequest();
159161

160-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(expectedRequestMetric));
161-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(expectedResponseMetric));
162-
assertThat(userAgent).matches(METRIC_SEARCH_PATTERN.apply(expectedAlgorithmMetric));
162+
assertThat(userAgent)
163+
.matches(METRIC_SEARCH_PATTERN.apply(expectedRequestMetric))
164+
.matches(METRIC_SEARCH_PATTERN.apply(expectedResponseMetric))
165+
.matches(METRIC_SEARCH_PATTERN.apply(expectedAlgorithmMetric));
163166
}
164167

165168
static Stream<Arguments> checksumConfigurationWithAlgorithmTestCases() {

0 commit comments

Comments
 (0)