Skip to content

Commit 517392f

Browse files
committed
Ignore HTTP binding locations on non-input shapes per Smithy spec
1 parent cbb38f4 commit 517392f

5 files changed

Lines changed: 251 additions & 6 deletions

File tree

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,13 @@ private ParameterHttpMapping generateParameterHttpMapping(Shape parentShape,
311311

312312
ParameterHttpMapping mapping = new ParameterHttpMapping();
313313

314+
// https://smithy.io/2.0/spec/http-bindings.html#httplabel-is-only-used-on-top-level-input
315+
Location location = isDirectOperationInputOrOutput(parentShape, allC2jShapes)
316+
? Location.forValue(member.getLocation())
317+
: null;
318+
314319
Shape memberShape = allC2jShapes.get(member.getShape());
315-
mapping.withLocation(Location.forValue(member.getLocation()))
320+
mapping.withLocation(location)
316321
.withPayload(member.isPayload()).withStreaming(member.isStreaming())
317322
.withFlattened(isFlattened(member, memberShape))
318323
.withUnmarshallLocationName(deriveUnmarshallerLocationName(memberShape, memberName, member))
@@ -323,6 +328,26 @@ private ParameterHttpMapping generateParameterHttpMapping(Shape parentShape,
323328
return mapping;
324329
}
325330

331+
private boolean isDirectOperationInputOrOutput(Shape parentShape, Map<String, Shape> allC2jShapes) {
332+
for (Operation operation : builder.getService().getOperations().values()) {
333+
if (operation.getInput() != null) {
334+
String inputShapeName = operation.getInput().getShape();
335+
Shape inputShape = allC2jShapes.get(inputShapeName);
336+
if (parentShape.equals(inputShape)) {
337+
return true;
338+
}
339+
}
340+
if (operation.getOutput() != null) {
341+
String outputShapeName = operation.getOutput().getShape();
342+
Shape outputShape = allC2jShapes.get(outputShapeName);
343+
if (parentShape.equals(outputShape)) {
344+
return true;
345+
}
346+
}
347+
}
348+
return false;
349+
}
350+
326351
private boolean isFlattened(Member member, Shape memberShape) {
327352
return member.isFlattened()
328353
|| memberShape.isFlattened();
@@ -354,12 +379,10 @@ private boolean isGreedy(Shape parentShape, Map<String, Shape> allC2jShapes, Par
354379
/**
355380
* Given a shape, finds the Request URI for the operation that references it as input.
356381
* Returns empty if the shape is not a direct operation input.
357-
* Throws if the shape is a direct operation input but the operation is missing a requestUri.
358382
*
359383
* @param parentShape Shape to find operation's request URI for.
360384
* @param allC2jShapes All shapes in the service model.
361385
* @return Request URI for operation, or empty if the shape is not a direct operation input.
362-
* @throws ModelInvalidException If the shape is a direct operation input but requestUri is missing.
363386
*/
364387
private Optional<String> findRequestUri(Shape parentShape, Map<String, Shape> allC2jShapes) {
365388
Optional<Operation> operation = builder.getService().getOperations().values().stream()

codegen/src/test/java/software/amazon/awssdk/codegen/AddShapesTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ void generateShapeModel_memberRequiredByNestedShape_setsMemberModelAsRequired()
8484
MemberModel requiredMemberModel = requestShapeModel.findMemberModelByC2jName(queryParamName);
8585

8686
assertThat(requestShapeModel.getRequired()).contains(queryParamName);
87-
assertThat(requiredMemberModel.getHttp().getLocation()).isEqualTo(Location.QUERY_STRING);
87+
assertThat(requiredMemberModel.getHttp().getLocation()).isNull();
8888
assertThat(requiredMemberModel.isRequired()).isTrue();
8989
}
9090

codegen/src/test/java/software/amazon/awssdk/codegen/CodeGeneratorTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@
4444
import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
4545
import software.amazon.awssdk.codegen.model.config.customization.UnderscoresInNameBehavior;
4646
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
47+
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
48+
import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
4749
import software.amazon.awssdk.codegen.model.rules.endpoints.EndpointTestSuiteModel;
50+
import software.amazon.awssdk.codegen.model.service.Location;
4851
import software.amazon.awssdk.codegen.model.service.ServiceModel;
4952
import software.amazon.awssdk.codegen.poet.ClientTestModels;
5053
import software.amazon.awssdk.codegen.validation.ModelInvalidException;
@@ -188,6 +191,24 @@ void execute_uriLocationOnNonInputShape_isIgnored() throws IOException {
188191
// Per the Smithy spec, httpLabel on non-input shapes has no meaning and is simply ignored.
189192
assertThatNoException().isThrownBy(
190193
() -> generateCodeFromC2jModels(models, outputDir, true, Collections.emptyList()));
194+
195+
IntermediateModel intermediateModel = new IntermediateModelBuilder(models).build();
196+
ShapeModel inputShape = intermediateModel.getShapes().get("SomeOperationRequest");
197+
MemberModel uriMember = inputShape.findMemberModelByC2jName("thingId");
198+
assertThat(uriMember.getHttp().getLocation()).isEqualTo(Location.URI);
199+
200+
ShapeModel nestedShape = intermediateModel.getShapes().get("NestedOptions");
201+
MemberModel nestedUriMember = nestedShape.findMemberModelByC2jName("pageSize");
202+
assertThat(nestedUriMember.getHttp().getLocation()).isNull();
203+
assertThat(nestedUriMember.getHttp().isGreedy()).isFalse();
204+
205+
Path generatedNestedOptions = Files.walk(outputDir)
206+
.filter(p -> p.getFileName().toString().equals("NestedOptions.java"))
207+
.findFirst()
208+
.orElseThrow(() -> new AssertionError("NestedOptions.java not found in generated output"));
209+
String actual = new String(Files.readAllBytes(generatedNestedOptions), StandardCharsets.UTF_8);
210+
String expected = resourceAsString("expected-nested-options.java");
211+
assertThat(actual).isEqualTo(expected);
191212
}
192213

193214
@Test
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
5+
* the License. A copy of the License is located at
6+
*
7+
* http://aws.amazon.com/apache2.0
8+
*
9+
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
10+
* CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
11+
* and limitations under the License.
12+
*/
13+
14+
package software.amazon.awssdk.services.restjson.model;
15+
16+
import java.io.Serializable;
17+
import java.util.Arrays;
18+
import java.util.Collections;
19+
import java.util.HashMap;
20+
import java.util.List;
21+
import java.util.Map;
22+
import java.util.Objects;
23+
import java.util.Optional;
24+
import java.util.function.BiConsumer;
25+
import java.util.function.Function;
26+
import software.amazon.awssdk.annotations.Generated;
27+
import software.amazon.awssdk.annotations.Mutable;
28+
import software.amazon.awssdk.annotations.NotThreadSafe;
29+
import software.amazon.awssdk.core.SdkField;
30+
import software.amazon.awssdk.core.SdkPojo;
31+
import software.amazon.awssdk.core.protocol.MarshallLocation;
32+
import software.amazon.awssdk.core.protocol.MarshallingType;
33+
import software.amazon.awssdk.core.traits.LocationTrait;
34+
import software.amazon.awssdk.utils.ToString;
35+
import software.amazon.awssdk.utils.builder.CopyableBuilder;
36+
import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
37+
38+
/**
39+
*/
40+
@Generated("software.amazon.awssdk:codegen")
41+
public final class NestedOptions implements SdkPojo, Serializable, ToCopyableBuilder<NestedOptions.Builder, NestedOptions> {
42+
private static final SdkField<String> PAGE_SIZE_FIELD = SdkField.<String> builder(MarshallingType.STRING)
43+
.memberName("pageSize").getter(getter(NestedOptions::pageSize)).setter(setter(Builder::pageSize))
44+
.traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("pageSize").build()).build();
45+
46+
private static final List<SdkField<?>> SDK_FIELDS = Collections.unmodifiableList(Arrays.asList(PAGE_SIZE_FIELD));
47+
48+
private static final Map<String, SdkField<?>> SDK_NAME_TO_FIELD = memberNameToFieldInitializer();
49+
50+
private static final long serialVersionUID = 1L;
51+
52+
private final String pageSize;
53+
54+
private NestedOptions(BuilderImpl builder) {
55+
this.pageSize = builder.pageSize;
56+
}
57+
58+
/**
59+
* Returns the value of the PageSize property for this object.
60+
*
61+
* @return The value of the PageSize property for this object.
62+
*/
63+
public final String pageSize() {
64+
return pageSize;
65+
}
66+
67+
@Override
68+
public Builder toBuilder() {
69+
return new BuilderImpl(this);
70+
}
71+
72+
public static Builder builder() {
73+
return new BuilderImpl();
74+
}
75+
76+
public static Class<? extends Builder> serializableBuilderClass() {
77+
return BuilderImpl.class;
78+
}
79+
80+
@Override
81+
public final int hashCode() {
82+
int hashCode = 1;
83+
hashCode = 31 * hashCode + Objects.hashCode(pageSize());
84+
return hashCode;
85+
}
86+
87+
@Override
88+
public final boolean equals(Object obj) {
89+
return equalsBySdkFields(obj);
90+
}
91+
92+
@Override
93+
public final boolean equalsBySdkFields(Object obj) {
94+
if (this == obj) {
95+
return true;
96+
}
97+
if (obj == null) {
98+
return false;
99+
}
100+
if (!(obj instanceof NestedOptions)) {
101+
return false;
102+
}
103+
NestedOptions other = (NestedOptions) obj;
104+
return Objects.equals(pageSize(), other.pageSize());
105+
}
106+
107+
/**
108+
* Returns a string representation of this object. This is useful for testing and debugging. Sensitive data will be
109+
* redacted from this string using a placeholder value.
110+
*/
111+
@Override
112+
public final String toString() {
113+
return ToString.builder("NestedOptions").add("PageSize", pageSize()).build();
114+
}
115+
116+
public final <T> Optional<T> getValueForField(String fieldName, Class<T> clazz) {
117+
switch (fieldName) {
118+
case "pageSize":
119+
return Optional.ofNullable(clazz.cast(pageSize()));
120+
default:
121+
return Optional.empty();
122+
}
123+
}
124+
125+
@Override
126+
public final List<SdkField<?>> sdkFields() {
127+
return SDK_FIELDS;
128+
}
129+
130+
@Override
131+
public final Map<String, SdkField<?>> sdkFieldNameToField() {
132+
return SDK_NAME_TO_FIELD;
133+
}
134+
135+
private static Map<String, SdkField<?>> memberNameToFieldInitializer() {
136+
Map<String, SdkField<?>> map = new HashMap<>();
137+
map.put("pageSize", PAGE_SIZE_FIELD);
138+
return Collections.unmodifiableMap(map);
139+
}
140+
141+
private static <T> Function<Object, T> getter(Function<NestedOptions, T> g) {
142+
return obj -> g.apply((NestedOptions) obj);
143+
}
144+
145+
private static <T> BiConsumer<Object, T> setter(BiConsumer<Builder, T> s) {
146+
return (obj, val) -> s.accept((Builder) obj, val);
147+
}
148+
149+
@Mutable
150+
@NotThreadSafe
151+
public interface Builder extends SdkPojo, CopyableBuilder<Builder, NestedOptions> {
152+
/**
153+
* Sets the value of the PageSize property for this object.
154+
*
155+
* @param pageSize
156+
* The new value for the PageSize property for this object.
157+
* @return Returns a reference to this object so that method calls can be chained together.
158+
*/
159+
Builder pageSize(String pageSize);
160+
}
161+
162+
static final class BuilderImpl implements Builder {
163+
private String pageSize;
164+
165+
private BuilderImpl() {
166+
}
167+
168+
private BuilderImpl(NestedOptions model) {
169+
pageSize(model.pageSize);
170+
}
171+
172+
public final String getPageSize() {
173+
return pageSize;
174+
}
175+
176+
public final void setPageSize(String pageSize) {
177+
this.pageSize = pageSize;
178+
}
179+
180+
@Override
181+
public final Builder pageSize(String pageSize) {
182+
this.pageSize = pageSize;
183+
return this;
184+
}
185+
186+
@Override
187+
public NestedOptions build() {
188+
return new NestedOptions(this);
189+
}
190+
191+
@Override
192+
public List<SdkField<?>> sdkFields() {
193+
return SDK_FIELDS;
194+
}
195+
196+
@Override
197+
public Map<String, SdkField<?>> sdkFieldNameToField() {
198+
return SDK_NAME_TO_FIELD;
199+
}
200+
}
201+
}

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/nestedqueryparameteroperation.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@ public final class NestedQueryParameterOperation implements SdkPojo, Serializabl
3333
.memberName("QueryParamOne")
3434
.getter(getter(NestedQueryParameterOperation::queryParamOne))
3535
.setter(setter(Builder::queryParamOne))
36-
.traits(LocationTrait.builder().location(MarshallLocation.QUERY_PARAM).locationName("QueryParamOne").build(),
36+
.traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("QueryParamOne").build(),
3737
RequiredTrait.create()).build();
3838

3939
private static final SdkField<String> QUERY_PARAM_TWO_FIELD = SdkField.<String> builder(MarshallingType.STRING)
4040
.memberName("QueryParamTwo").getter(getter(NestedQueryParameterOperation::queryParamTwo))
4141
.setter(setter(Builder::queryParamTwo))
42-
.traits(LocationTrait.builder().location(MarshallLocation.QUERY_PARAM).locationName("QueryParamTwo").build()).build();
42+
.traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("QueryParamTwo").build()).build();
4343

4444
private static final List<SdkField<?>> SDK_FIELDS = Collections.unmodifiableList(Arrays.asList(QUERY_PARAM_ONE_FIELD,
4545
QUERY_PARAM_TWO_FIELD));

0 commit comments

Comments
 (0)