Skip to content

Commit 2f9d303

Browse files
S-Saranya1S-Saranya1
committed
Address PR feedback
1 parent 05a972a commit 2f9d303

File tree

3 files changed

+32
-115
lines changed

3 files changed

+32
-115
lines changed

‎.changes/next-release/feature-AWSSDKforJavav2-aa60cd3.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"type": "feature",
3-
"category": "AWS SDK for Java v2",
3+
"category": "AWS S3",
44
"contributor": "",
55
"description": "Implemented business metrics tracking for S3_Express_Bucket (featureID \"J\") through User-Agent header."
66
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/s3express/S3ExpressUtils.java‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,13 @@ public static boolean useS3ExpressAuthScheme(ExecutionAttributes executionAttrib
6363
/**
6464
* Adds S3 Express business metric if applicable for the current operation.
6565
*/
66-
public static void addS3ExpressBusinessMetricIfApplicable(ExecutionAttributes executionAttributes) {
67-
if (useS3Express(executionAttributes) && useS3ExpressAuthScheme(executionAttributes)) {
68-
BusinessMetricCollection businessMetrics =
69-
executionAttributes.getAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS);
70-
71-
if (businessMetrics != null) {
72-
businessMetrics.addMetric(BusinessMetricFeatureId.S3_EXPRESS_BUCKET.value());
66+
public static void addS3ExpressBusinessMetricIfApplicable(ExecutionAttributes executionAttributes){
67+
if(executionAttributes != null && useS3Express(executionAttributes) && useS3ExpressAuthScheme(executionAttributes)){
68+
executionAttributes.getOptionalAttribute(SdkInternalExecutionAttribute.BUSINESS_METRICS)
69+
.ifPresent(businessMetrics ->
70+
businessMetrics.addMetric(BusinessMetricFeatureId.S3_EXPRESS_BUCKET.value())
71+
);
7372
}
74-
}
73+
7574
}
7675
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/s3express/S3ExpressUserAgentTest.java‎

Lines changed: 24 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,13 @@
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
1919

20-
import java.util.ArrayList;
2120
import java.util.List;
22-
import java.util.concurrent.atomic.AtomicReference;
23-
import java.util.regex.Matcher;
24-
import java.util.regex.Pattern;
21+
import java.util.function.UnaryOperator;
2522
import org.junit.jupiter.api.BeforeEach;
2623
import org.junit.jupiter.api.Test;
2724
import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider;
28-
import software.amazon.awssdk.core.interceptor.Context;
29-
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
30-
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
3125
import software.amazon.awssdk.core.sync.RequestBody;
3226
import software.amazon.awssdk.core.sync.ResponseTransformer;
33-
import software.amazon.awssdk.core.useragent.BusinessMetricFeatureId;
3427
import software.amazon.awssdk.http.AbortableInputStream;
3528
import software.amazon.awssdk.http.HttpExecuteResponse;
3629
import software.amazon.awssdk.http.SdkHttpRequest;
@@ -51,8 +44,10 @@ public class S3ExpressUserAgentTest {
5144
private static final String CONTENTS = "test content for feature id validation";
5245
private static final String S3_EXPRESS_BUCKET = "my-test-bucket--use1-az4--x-s3";
5346
private static final String REGULAR_BUCKET = "my-test-bucket-regular";
47+
48+
public static final UnaryOperator<String> METRIC_SEARCH_PATTERN =
49+
metric -> ".*m/[a-zA-Z0-9+-,]*" + metric + ".*";
5450

55-
private final UserAgentCapturingInterceptor userAgentInterceptor = new UserAgentCapturingInterceptor();
5651
private MockSyncHttpClient mockHttpClient;
5752
private S3Client s3Client;
5853

@@ -101,10 +96,7 @@ void setup() {
10196
.region(Region.US_EAST_1)
10297
.credentialsProvider(AnonymousCredentialsProvider.create())
10398
.httpClient(mockHttpClient)
104-
.overrideConfiguration(o -> o.addExecutionInterceptor(userAgentInterceptor))
10599
.build();
106-
107-
userAgentInterceptor.reset();
108100
}
109101

110102
@Test
@@ -116,18 +108,13 @@ void putObject_whenS3ExpressBucket_shouldIncludeS3ExpressFeatureIdInUserAgent()
116108

117109
s3Client.putObject(putRequest, RequestBody.fromString(CONTENTS));
118110

119-
List<String> capturedUserAgents = userAgentInterceptor.getCapturedUserAgents();
120-
assertThat(capturedUserAgents).hasSize(2); // CreateSession + PutObject calls
121-
122-
// The second User-Agent is from the actual PutObject call
123-
String userAgent = capturedUserAgents.get(1);
124-
assertThat(userAgent).isNotNull();
111+
SdkHttpRequest lastRequest = mockHttpClient.getLastRequest();
112+
assertThat(lastRequest).isNotNull();
125113

126-
String expectedFeatureId = BusinessMetricFeatureId.S3_EXPRESS_BUCKET.value();
127-
String businessMetrics = extractBusinessMetrics(userAgent);
114+
List<String> userAgentHeaders = lastRequest.headers().get("User-Agent");
115+
assertThat(userAgentHeaders).isNotNull().hasSize(1);
128116

129-
assertThat(businessMetrics).contains(expectedFeatureId);
130-
assertThat(userAgent).contains(" m/" + businessMetrics);
117+
assertThat(userAgentHeaders.get(0)).matches(METRIC_SEARCH_PATTERN.apply("J"));
131118
}
132119

133120
@Test
@@ -139,18 +126,13 @@ void getObject_whenS3ExpressBucket_shouldIncludeS3ExpressFeatureIdInUserAgent()
139126

140127
s3Client.getObject(getRequest, ResponseTransformer.toBytes());
141128

142-
List<String> capturedUserAgents = userAgentInterceptor.getCapturedUserAgents();
143-
assertThat(capturedUserAgents).hasSize(2);
129+
SdkHttpRequest lastRequest = mockHttpClient.getLastRequest();
130+
assertThat(lastRequest).isNotNull();
144131

145-
String userAgent = capturedUserAgents.get(1);
146-
assertThat(userAgent).isNotNull();
132+
List<String> userAgentHeaders = lastRequest.headers().get("User-Agent");
133+
assertThat(userAgentHeaders).isNotNull().hasSize(1);
147134

148-
String expectedFeatureId = BusinessMetricFeatureId.S3_EXPRESS_BUCKET.value();
149-
String businessMetrics = extractBusinessMetrics(userAgent);
150-
151-
assertThat(businessMetrics).isNotNull();
152-
assertThat(businessMetrics).contains(expectedFeatureId);
153-
assertThat(userAgent).contains(" m/" + businessMetrics);
135+
assertThat(userAgentHeaders.get(0)).matches(METRIC_SEARCH_PATTERN.apply("J"));
154136
}
155137

156138
@Test
@@ -162,19 +144,13 @@ void putObject_whenRegularS3Bucket_shouldNotIncludeS3ExpressFeatureIdInUserAgent
162144

163145
s3Client.putObject(putRequest, RequestBody.fromString(CONTENTS));
164146

165-
List<String> capturedUserAgents = userAgentInterceptor.getCapturedUserAgents();
166-
assertThat(capturedUserAgents).hasSize(1);
167-
168-
String userAgent = capturedUserAgents.get(0);
169-
assertThat(userAgent).isNotNull();
147+
SdkHttpRequest lastRequest = mockHttpClient.getLastRequest();
148+
assertThat(lastRequest).isNotNull();
170149

171-
String s3ExpressFeatureId = BusinessMetricFeatureId.S3_EXPRESS_BUCKET.value();
172-
173-
String businessMetrics = extractBusinessMetrics(userAgent);
174-
if (businessMetrics != null) {
175-
assertThat(businessMetrics).doesNotContain(s3ExpressFeatureId);
176-
}
150+
List<String> userAgentHeaders = lastRequest.headers().get("User-Agent");
151+
assertThat(userAgentHeaders).isNotNull().hasSize(1);
177152

153+
assertThat(userAgentHeaders.get(0)).doesNotMatch(METRIC_SEARCH_PATTERN.apply("J"));
178154
}
179155

180156
@Test
@@ -186,71 +162,13 @@ void getObject_whenRegularS3Bucket_shouldNotIncludeS3ExpressFeatureIdInUserAgent
186162

187163
s3Client.getObject(getRequest, ResponseTransformer.toBytes());
188164

189-
List<String> capturedUserAgents = userAgentInterceptor.getCapturedUserAgents();
190-
assertThat(capturedUserAgents).hasSize(1);
191-
192-
String userAgent = capturedUserAgents.get(0);
193-
assertThat(userAgent).isNotNull();
194-
195-
String s3ExpressFeatureId = BusinessMetricFeatureId.S3_EXPRESS_BUCKET.value();
196-
197-
String businessMetrics = extractBusinessMetrics(userAgent);
198-
if (businessMetrics != null) {
199-
assertThat(businessMetrics).doesNotContain(s3ExpressFeatureId);
200-
}
165+
SdkHttpRequest lastRequest = mockHttpClient.getLastRequest();
166+
assertThat(lastRequest).isNotNull();
201167

202-
}
168+
List<String> userAgentHeaders = lastRequest.headers().get("User-Agent");
169+
assertThat(userAgentHeaders).isNotNull().hasSize(1);
203170

204-
/**
205-
* Extracts the business metrics section from a User-Agent string.
206-
* Business metrics appear as "m/D,J" where D and J are feature IDs.
207-
*/
208-
private String extractBusinessMetrics(String userAgent) {
209-
if (userAgent == null) {
210-
return null;
211-
}
212-
213-
// Pattern to match business metrics: " m/feature1,feature2"
214-
Pattern pattern = Pattern.compile(" m/([A-Za-z0-9+\\-,]+)");
215-
Matcher matcher = pattern.matcher(userAgent);
216-
217-
if (matcher.find()) {
218-
return matcher.group(1);
219-
}
220-
221-
return null;
171+
assertThat(userAgentHeaders.get(0)).doesNotMatch(METRIC_SEARCH_PATTERN.apply("J"));
222172
}
223173

224-
/**
225-
* Interceptor to capture User-Agent headers from HTTP requests
226-
*/
227-
private static class UserAgentCapturingInterceptor implements ExecutionInterceptor {
228-
private final List<String> capturedUserAgents = new ArrayList<>();
229-
private final AtomicReference<String> lastUserAgent = new AtomicReference<>();
230-
231-
@Override
232-
public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) {
233-
SdkHttpRequest httpRequest = context.httpRequest();
234-
List<String> userAgentHeaders = httpRequest.headers().get("User-Agent");
235-
236-
if (userAgentHeaders != null && !userAgentHeaders.isEmpty()) {
237-
String userAgent = userAgentHeaders.get(0);
238-
capturedUserAgents.add(userAgent);
239-
lastUserAgent.set(userAgent);
240-
}
241-
}
242-
243-
public List<String> getCapturedUserAgents() {
244-
return new ArrayList<>(capturedUserAgents);
245-
}
246-
247-
public String getLastUserAgent() {
248-
return lastUserAgent.get();
249-
}
250-
251-
public void reset() {
252-
capturedUserAgents.clear();
253-
lastUserAgent.set(null);
254-
}
255-
}
256174
}

0 commit comments

Comments
 (0)