Skip to content

Adds all fields in RequestOverrideConfiguration in toBuilder #2862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-c20ef41.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "",
"type": "bugfix",
"description": "Adds all fields in RequestOverrideConfiguration when a builder is created from an instance"
}
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,11 @@ protected BuilderImpl(RequestOverrideConfiguration sdkRequestOverrideConfig) {
headers(sdkRequestOverrideConfig.headers);
rawQueryParameters(sdkRequestOverrideConfig.rawQueryParameters);
sdkRequestOverrideConfig.apiNames.forEach(this::addApiName);
apiCallTimeout(sdkRequestOverrideConfig.apiCallTimeout);
apiCallAttemptTimeout(sdkRequestOverrideConfig.apiCallAttemptTimeout);
signer(sdkRequestOverrideConfig.signer().orElse(null));
metricPublishers(sdkRequestOverrideConfig.metricPublishers());
executionAttributes(sdkRequestOverrideConfig.executionAttributes());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

package software.amazon.awssdk.core;


import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
Expand All @@ -27,16 +26,53 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import nl.jqno.equalsverifier.EqualsVerifier;
import org.junit.Test;
import software.amazon.awssdk.core.interceptor.ExecutionAttribute;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
import software.amazon.awssdk.core.signer.Signer;
import software.amazon.awssdk.http.SdkHttpFullRequest;
import software.amazon.awssdk.metrics.MetricPublisher;
import software.amazon.awssdk.utils.ImmutableMap;

public class RequestOverrideConfigurationTest {
private static final String HEADER = "header";
private static final String QUERY_PARAM = "queryparam";

@Test
public void equalsHashcode() {
EqualsVerifier.forClass(RequestOverrideConfiguration.class)
.usingGetClass()
.verify();
}

@Test
public void toBuilder_minimal() {
RequestOverrideConfiguration configuration = SdkRequestOverrideConfiguration.builder()
.build();

assertThat(configuration.toBuilder().build()).usingRecursiveComparison().isEqualTo(configuration);
}

@Test
public void toBuilder_maximal() {
ExecutionAttribute testAttribute = new ExecutionAttribute("TestAttribute");
String expectedValue = "Value1";

RequestOverrideConfiguration configuration = SdkRequestOverrideConfiguration.builder()
.putHeader(HEADER, "foo")
.putRawQueryParameter(QUERY_PARAM, "foo")
.addApiName(a -> a.name("test1").version("1"))
.apiCallTimeout(Duration.ofSeconds(1))
.apiCallAttemptTimeout(Duration.ofSeconds(1))
.signer(new NoOpSigner())
.executionAttributes(ExecutionAttributes.builder().put(testAttribute, expectedValue).build())
.addMetricPublisher(mock(MetricPublisher.class))
.build();

assertThat(configuration.toBuilder().build()).usingRecursiveComparison().isEqualTo(configuration);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could find a way to enforce this validation for all of our classes that implement toBuilder().

}

@Test
public void addingSameItemTwice_shouldOverride() {
RequestOverrideConfiguration configuration = SdkRequestOverrideConfiguration.builder()
Expand Down Expand Up @@ -284,4 +320,12 @@ public void testConfigurationEquals() {
assertThat(request1Override).isEqualTo(request2Override);
assertThat(request1Override).isNotEqualTo(null);
}

private static class NoOpSigner implements Signer {

@Override
public SdkHttpFullRequest sign(SdkHttpFullRequest request, ExecutionAttributes executionAttributes) {
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"pagination": {
"PaginatedOperationWithResultKey": {
"input_token": "NextToken",
"output_token": "NextToken",
"limit_key": "MaxResults",
"result_key": "Items"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,20 @@
"requestUri":"/2016-03-11/operationWithNoInputOrOutput"
}
},
"PaginatedOperationWithResultKey": {
"name": "PaginatedOperationWithResultKey",
"http": {
"method": "POST",
"requestUri": "/"
},
"input": {
"shape": "PaginatedOperationWithResultKeyRequest"
},
"output": {
"shape": "PaginatedOperationWithResultKeyResponse"
},
"documentation": "Some paginated operation with result_key in paginators.json file"
},
"QueryParamWithoutValue":{
"name":"QueryParamWithoutValue",
"http":{
Expand Down Expand Up @@ -572,6 +586,33 @@
}
}
},
"PaginatedOperationWithResultKeyRequest": {
"type": "structure",
"members": {
"NextToken": {
"shape": "String",
"documentation": "<p>Token for the next set of results</p>"
},
"MaxResults": {
"shape": "String",
"documentation": "<p>Maximum number of results in a single page</p>"
}
}
},
"PaginatedOperationWithResultKeyResponse": {
"type": "structure",
"members": {
"NextToken": {
"shape": "String",
"documentation": "<p>Token for the next set of results</p>"
},
"Items": {
"shape": "ListOfSimpleStructs",
"documentation": "<p>Maximum number of results in a single page</p>"
}
},
"documentation": "<p>Response type of a single page</p>"
},
"PayloadStructType":{
"type":"structure",
"members":{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.time.Duration;
import java.util.List;
import java.util.stream.Collectors;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -49,6 +51,8 @@
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient;
import software.amazon.awssdk.services.protocolrestjson.model.EmptyModeledException;
import software.amazon.awssdk.services.protocolrestjson.model.SimpleStruct;
import software.amazon.awssdk.services.protocolrestjson.paginators.PaginatedOperationWithResultKeyIterable;

@RunWith(MockitoJUnitRunner.class)
public class CoreMetricsTest {
Expand Down Expand Up @@ -141,6 +145,20 @@ public void testApiCall_publisherOverriddenOnRequest_requestPublisherTakesPreced
verifyZeroInteractions(mockPublisher);
}

@Test
public void testPaginatingApiCall_publisherOverriddenOnRequest_requestPublisherTakesPrecedence() {
MetricPublisher requestMetricPublisher = mock(MetricPublisher.class);

PaginatedOperationWithResultKeyIterable iterable =
client.paginatedOperationWithResultKeyPaginator(
r -> r.overrideConfiguration(o -> o.addMetricPublisher(requestMetricPublisher)));

List<SimpleStruct> resultingItems = iterable.items().stream().collect(Collectors.toList());

verify(requestMetricPublisher).publish(any(MetricCollection.class));
verifyZeroInteractions(mockPublisher);
}

@Test
public void testApiCall_operationSuccessful_addsMetrics() {
client.allTypes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -39,6 +43,10 @@
import software.amazon.awssdk.metrics.MetricPublisher;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClient;
import software.amazon.awssdk.services.protocolrestjson.model.PaginatedOperationWithResultKeyResponse;
import software.amazon.awssdk.services.protocolrestjson.model.SimpleStruct;
import software.amazon.awssdk.services.protocolrestjson.paginators.PaginatedOperationWithResultKeyIterable;
import software.amazon.awssdk.services.protocolrestjson.paginators.PaginatedOperationWithResultKeyPublisher;

/**
* Core metrics test for async non-streaming API
Expand Down Expand Up @@ -123,4 +131,20 @@ public void apiCall_publisherOverriddenOnRequest_requestPublisherTakesPrecedence
verify(requestMetricPublisher).publish(any(MetricCollection.class));
verifyZeroInteractions(mockPublisher);
}

@Test
public void testPaginatingApiCall_publisherOverriddenOnRequest_requestPublisherTakesPrecedence() throws Exception {
stubSuccessfulResponse();
MetricPublisher requestMetricPublisher = mock(MetricPublisher.class);

PaginatedOperationWithResultKeyPublisher paginatedPublisher =
client.paginatedOperationWithResultKeyPaginator(
r -> r.overrideConfiguration(o -> o.addMetricPublisher(requestMetricPublisher)));

CompletableFuture<Void> future = paginatedPublisher.subscribe(PaginatedOperationWithResultKeyResponse::items);
future.get();

verify(requestMetricPublisher).publish(any(MetricCollection.class));
verifyZeroInteractions(mockPublisher);
}
}