Skip to content

Commit

Permalink
Removing try catch, fixes test bug and filters null elements
Browse files Browse the repository at this point in the history
  • Loading branch information
@cenedhryn
cenedhryn committed May 10, 2024
1 parent 79554af commit ad1d283
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 209 deletions.
16 changes: 0 additions & 16 deletions .../main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverInterceptorSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,11 @@
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.CollectionUtils;
import software.amazon.awssdk.utils.HostnameValidator;
import software.amazon.awssdk.utils.Logger;
import software.amazon.awssdk.utils.StringUtils;
import software.amazon.awssdk.utils.internal.CodegenNamingUtils;

public class EndpointResolverInterceptorSpec implements ClassSpec {

private static final String LOGGER_FIELD_NAME = "LOG";
private final IntermediateModel model;
private final EndpointRulesSpecUtils endpointRulesSpecUtils;
private final EndpointParamsKnowledgeIndex endpointParamsKnowledgeIndex;
Expand Down Expand Up @@ -128,8 +126,6 @@ public TypeSpec poetSpec() {
.addAnnotation(SdkInternalApi.class)
.addSuperinterface(ExecutionInterceptor.class);

b.addField(logger());

if (!useSraAuth) {
b.addField(endpointAuthSchemeStrategyFieldSpec);
b.addMethod(constructorMethodSpec(endpointAuthSchemeStrategyFieldSpec.name));
Expand Down Expand Up @@ -573,13 +569,7 @@ private MethodSpec setOperationContextParamsMethod(OperationModel opModel) {
.add(")")
.build();

b.beginControlFlow("try");
b.addStatement(addParam);
b.endControlFlow();
b.beginControlFlow("catch ($T e)", Exception.class);
b.addStatement("$N.warn(() -> \"Error resolving operation context parameter '$L'; will set param to null."
+ " \\nError message: \" + e.getMessage())", LOGGER_FIELD_NAME, setterName);
b.endControlFlow();
} else {
throw new RuntimeException("Invalid operation context parameter path for " + opModel.getOperationName() +
". Expected VALUE_STRING, but got " + value.getValue().asToken());
Expand Down Expand Up @@ -895,10 +885,4 @@ private MethodSpec constructorMethodSpec(String endpointAuthSchemeFieldName) {
return b.build();
}

private FieldSpec logger() {
return FieldSpec.builder(Logger.class, LOGGER_FIELD_NAME)
.addModifiers(Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL)
.initializer("$T.loggerFor($T.class)", Logger.class, className())
.build();
}
}
12 changes: 8 additions & 4 deletions .../src/main/resources/software/amazon/awssdk/codegen/jmespath/JmesPathRuntime.java.resource
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,15 @@ public final class JmesPathRuntime {
if (type == Type.LIST) {
List<String> result = new ArrayList<>();
for (Object listEntry : listValue) {
Value listValue = new Value(listEntry);
if (listValue.type != Type.STRING) {
Value entryAsValue = new Value(listEntry);
if (entryAsValue.type == Type.NULL) {
continue;
}
if (entryAsValue.type != Type.STRING) {
throw new IllegalStateException("Expected list elements to be of type String, but were "
+ listValue.type);
+ entryAsValue.type);
}
result.add(listValue.stringValue);
result.add(entryAsValue.stringValue);
}
return result;
}
Expand Down Expand Up @@ -478,6 +481,7 @@ public final class JmesPathRuntime {
.map(Value::new)
.map(functionToApply)
.map(Value::value)
.filter(Objects::nonNull)
.collect(toList()),
true);
}
Expand Down
17 changes: 2 additions & 15 deletions ...ources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolve-interceptor-preSra.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,10 @@
import software.amazon.awssdk.services.query.model.OperationWithOperationContextParamRequest;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.CompletableFutureUtils;
import software.amazon.awssdk.utils.Logger;

@Generated("software.amazon.awssdk:codegen")
@SdkInternalApi
public final class QueryResolveEndpointInterceptor implements ExecutionInterceptor {
private static final Logger LOG = Logger.loggerFor(QueryResolveEndpointInterceptor.class);

private final EndpointAuthSchemeStrategy endpointAuthSchemeStrategy;

public QueryResolveEndpointInterceptor() {
Expand Down Expand Up @@ -220,25 +217,15 @@ private static void setOperationContextParams(QueryEndpointParams.Builder params
private static void setOperationContextParams(QueryEndpointParams.Builder params,
OperationWithCustomizedOperationContextParamRequest request) {
JmesPathRuntime.Value input = new JmesPathRuntime.Value(request);
try {
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
.stringValues());
} catch (Exception e) {
LOG.warn(() -> "Error resolving operation context parameter 'customEndpointArray'; will set param to null. \nError message: "
+ e.getMessage());
}
}

private static void setOperationContextParams(QueryEndpointParams.Builder params,
OperationWithOperationContextParamRequest request) {
JmesPathRuntime.Value input = new JmesPathRuntime.Value(request);
try {
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
.stringValues());
} catch (Exception e) {
LOG.warn(() -> "Error resolving operation context parameter 'customEndpointArray'; will set param to null. \nError message: "
+ e.getMessage());
}
}

private static Optional<String> hostPrefix(String operationName, SdkRequest request) {
Expand Down
16 changes: 2 additions & 14 deletions ...est/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolve-interceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@
import software.amazon.awssdk.services.query.model.OperationWithOperationContextParamRequest;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.CompletableFutureUtils;
import software.amazon.awssdk.utils.Logger;

@Generated("software.amazon.awssdk:codegen")
@SdkInternalApi
public final class QueryResolveEndpointInterceptor implements ExecutionInterceptor {
private static final Logger LOG = Logger.loggerFor(QueryResolveEndpointInterceptor.class);

@Override
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
Expand Down Expand Up @@ -207,25 +205,15 @@ private static void setOperationContextParams(QueryEndpointParams.Builder params
private static void setOperationContextParams(QueryEndpointParams.Builder params,
OperationWithCustomizedOperationContextParamRequest request) {
JmesPathRuntime.Value input = new JmesPathRuntime.Value(request);
try {
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
.stringValues());
} catch (Exception e) {
LOG.warn(() -> "Error resolving operation context parameter 'customEndpointArray'; will set param to null. \nError message: "
+ e.getMessage());
}
}

private static void setOperationContextParams(QueryEndpointParams.Builder params,
OperationWithOperationContextParamRequest request) {
JmesPathRuntime.Value input = new JmesPathRuntime.Value(request);
try {
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
params.customEndpointArray(input.field("ListMember").field("StringList").wildcard().field("LeafString")
.stringValues());
} catch (Exception e) {
LOG.warn(() -> "Error resolving operation context parameter 'customEndpointArray'; will set param to null. \nError message: "
+ e.getMessage());
}
}

private static Optional<String> hostPrefix(String operationName, SdkRequest request) {
Expand Down
2 changes: 1 addition & 1 deletion services/s3/src/main/resources/codegen-resources/customization.config
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@
"operationName": "DeleteObjects",
"operationContextParamsMap": {
"DeleteObjectKeys": {
"value": "Delete.Objects[*].key"
"value": "Delete.Objects[*].Key"
}
}
}
Expand Down
22 changes: 3 additions & 19 deletions ...-classes-test/src/main/resources/codegen-resources/endpointproviders/customization.config
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,16 @@
"documentation": "A list of string from a wildcard projection",
"type": "StringArray"
},
"EmptyKeyListOfString": {
"KeysListOfString": {
"required": false,
"documentation": "A list of string from a wildcard projection that evaluates to null elements",
"documentation": "A list of string that are map keys",
"type": "StringArray"
},
"MissingRequestValuesListOfString": {
"required": false,
"documentation": "A list of string from a field that is not populated in the request",
"type": "StringArray"
},
"MissingFieldListOfString": {
"required": false,
"documentation": "A list of string from a field that does not exist. Can only happen with customization config.",
"type": "StringArray"
},
"KeysListOfString": {
"required": false,
"documentation": "A list of string that are map keys",
"type": "StringArray"
}
}
},
"customOperationContextParams": [
{
Expand All @@ -55,14 +45,8 @@
"KeysListOfString": {
"value": "keys(PojoKeys)"
},
"EmptyKeyListOfString": {
"value": "Nested.ListOfNested[*].NonExistingMember"
},
"MissingRequestValuesListOfString": {
"value": "Nested.ListOfString"
},
"MissingFieldListOfString": {
"value": "Nested.NonExistingMember"
}
}
}
Expand Down
78 changes: 23 additions & 55 deletions ...ava/software/amazon/awssdk/services/endpointproviders/OperationContextParametersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,9 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.Comparator;
import java.util.List;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.commons.logging.impl.Log4JLogger;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.core.LogEvent;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -43,6 +36,7 @@
import software.amazon.awssdk.services.restjsonendpointproviders.RestJsonEndpointProvidersClient;
import software.amazon.awssdk.services.restjsonendpointproviders.endpoints.RestJsonEndpointProvidersEndpointParams;
import software.amazon.awssdk.services.restjsonendpointproviders.endpoints.RestJsonEndpointProvidersEndpointProvider;
import software.amazon.awssdk.services.restjsonendpointproviders.model.NestedContainersOperationRequest;
import software.amazon.awssdk.services.restjsonendpointproviders.model.NestedContainersOperationResponse;
import software.amazon.awssdk.services.restjsonendpointproviders.model.NestedContainersStructure;
import software.amazon.awssdk.services.restjsonendpointproviders.model.PayloadStructType;
Expand All @@ -55,7 +49,6 @@ class OperationContextParametersTest extends LogCaptor.LogCaptorTestBase {
private static final Region REGION = Region.of("us-east-9000");

private RestJsonEndpointProvidersEndpointProvider mockEndpointProvider;
Log4JLogger fooLogger;

@BeforeEach
public void setup() {
Expand All @@ -66,28 +59,22 @@ public void setup() {

static Stream<Arguments> paramAssertions() {
return Stream.of(
Arguments.of("Single String parameter works as expected", new TestAssertion(
Arguments.of("Single String parameter", new TestAssertion(
p -> assertThat(p.pojoString()).isEqualTo("StringMemberA"))),

Arguments.of("List of String parameter works as expected", new TestAssertion(
Arguments.of("List of String parameter", new TestAssertion(
p -> assertThat(p.basicListOfString()).isNotEmpty().hasSize(2).containsExactly("StringA", "StringB"))),

Arguments.of("List of String parameter from wildcard works as expected", new TestAssertion(
p -> assertThat(p.wildcardKeyListOfString()).isNotEmpty().hasSize(2)
.containsExactly("StringMemberS1", "StringMemberS2"))),
Arguments.of("List of String parameter from wildcard with null members", new TestAssertion(
p -> assertThat(p.wildcardKeyListOfString()).isNotEmpty().hasSize(1)
.containsExactly("StringMemberS2"))),

Arguments.of("List of String parameter from 'keys' function works as expected", new TestAssertion(
Arguments.of("List of String parameter from 'keys' function", new TestAssertion(
p -> assertThat(p.keysListOfString()).isNotEmpty().hasSize(2)
.containsExactly("PayloadMemberOne", "PayloadMemberTwo"))),

Arguments.of("List of String parameter is null if list is empty", new TestAssertion(
p -> assertThat(p.emptyKeyListOfString()).isNull())),

Arguments.of("List of String parameter is empty if request does not have list elements", new TestAssertion(
p -> assertThat(p.missingRequestValuesListOfString()).isEmpty())),

Arguments.of("List of String parameter is null if request is missing structure", new TestAssertion(
p -> assertThat(p.missingFieldListOfString()).isNull()))
p -> assertThat(p.missingRequestValuesListOfString()).isEmpty()))
);
}

Expand All @@ -105,52 +92,33 @@ void operationContextParams_customization_resolvedCorrectly(String description,
assertion.verify.accept(params);
}

@Test
void testErrorsAreEncapsulatedAndLogged() {
assertThatThrownBy(this::createClientAndCallApi).isInstanceOf(RuntimeException.class);
List<LogEvent> jmesRuntimeEvents = loggedEvents().stream()
.filter(e -> e.getLoggerName().contains(
"RestJsonEndpointProvidersResolveEndpointInterceptor"))
.collect(Collectors.toList());
assertThat(jmesRuntimeEvents).hasSize(2);
assertThat(jmesRuntimeEvents).extracting("level").containsOnly(Level.WARN);
assertThat(jmesRuntimeEvents).extracting("message.formattedMessage")
.usingElementComparator(stringContains())
.contains("emptyKeyListOfString", "missingFieldListOfString")
.contains("No such field: NonExistingMember");
}

private NestedContainersOperationResponse createClientAndCallApi() {
RestJsonEndpointProvidersClient client = RestJsonEndpointProvidersClient.builder()
.region(REGION)
.credentialsProvider(CREDENTIALS)
.endpointProvider(mockEndpointProvider)
.build();
return client.nestedContainersOperation(createRequestObject());
}

NestedContainersStructure nestedShallow1 = NestedContainersStructure.builder()
.stringMember("StringMemberS1")
.build();
NestedContainersStructure nestedShallow2 = NestedContainersStructure.builder()
.stringMember("StringMemberS2")
.build();
private static NestedContainersOperationRequest createRequestObject() {
NestedContainersStructure pojoInListWithoutProjectionMember = NestedContainersStructure.builder().build();
NestedContainersStructure pojoInListWithProjectionMember = NestedContainersStructure.builder()
.stringMember("StringMemberS2")
.build();
NestedContainersStructure nested = NestedContainersStructure.builder()
.stringMember("StringMemberA")
.listOfNested(nestedShallow1, nestedShallow2)
.listOfNested(pojoInListWithoutProjectionMember,
pojoInListWithProjectionMember)
.build();
PayloadStructType pojoKeys = PayloadStructType.builder().payloadMemberOne("p1").payloadMemberTwo("p2").build();
return client.nestedContainersOperation(r -> r.nested(nested)
.listOfString("StringA", "StringB")
.listOfNested(nestedShallow1, nestedShallow2)
.pojoKeys(pojoKeys));
}

Comparator<Object> stringContains() {
return (t1, t2) -> {
if (((String) t1).contains((CharSequence) t2)) {
return 0;
}
return -1;
};
return NestedContainersOperationRequest.builder()
.nested(nested)
.listOfString("StringA", "StringB")
.listOfNested(pojoInListWithoutProjectionMember, pojoInListWithProjectionMember)
.pojoKeys(pojoKeys)
.build();
}

private static class TestAssertion {
Expand Down
Loading

0 comments on commit ad1d283

Please sign in to comment.