Skip to content

Commit d0daa36

Browse files
committed
Fix comments
1 parent a4cbd8e commit d0daa36

5 files changed

Lines changed: 148 additions & 34 deletions

File tree

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

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
import static software.amazon.awssdk.codegen.internal.Utils.isScalar;
2323

2424
import java.util.Collections;
25+
import java.util.HashSet;
2526
import java.util.List;
2627
import java.util.Map;
2728
import java.util.Optional;
29+
import java.util.Set;
2830
import software.amazon.awssdk.codegen.internal.TypeUtils;
2931
import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
3032
import software.amazon.awssdk.codegen.model.intermediate.EnumModel;
@@ -55,6 +57,7 @@ abstract class AddShapes {
5557

5658
private final IntermediateModelBuilder builder;
5759
private final NamingStrategy namingStrategy;
60+
private Set<Shape> directOperationShapes;
5861

5962
AddShapes(IntermediateModelBuilder builder) {
6063
this.builder = builder;
@@ -313,7 +316,7 @@ private ParameterHttpMapping generateParameterHttpMapping(Shape parentShape,
313316
ParameterHttpMapping mapping = new ParameterHttpMapping();
314317

315318
// https://smithy.io/2.0/spec/http-bindings.html#httplabel-is-only-used-on-top-level-input
316-
Location location = isDirectOperationInputOrOutput(parentShape, allC2jShapes)
319+
Location location = isDirectOperationShape(parentShape, allC2jShapes)
317320
? Location.forValue(member.getLocation())
318321
: null;
319322

@@ -329,34 +332,24 @@ private ParameterHttpMapping generateParameterHttpMapping(Shape parentShape,
329332
return mapping;
330333
}
331334

332-
private boolean isDirectOperationInputOrOutput(Shape parentShape, Map<String, Shape> allC2jShapes) {
333-
for (Operation operation : builder.getService().getOperations().values()) {
334-
if (operation.getInput() != null) {
335-
String inputShapeName = operation.getInput().getShape();
336-
Shape inputShape = allC2jShapes.get(inputShapeName);
337-
if (parentShape.equals(inputShape)) {
338-
return true;
335+
private boolean isDirectOperationShape(Shape parentShape, Map<String, Shape> allC2jShapes) {
336+
if (directOperationShapes == null) {
337+
directOperationShapes = new HashSet<>();
338+
for (Operation operation : builder.getService().getOperations().values()) {
339+
if (operation.getInput() != null) {
340+
directOperationShapes.add(allC2jShapes.get(operation.getInput().getShape()));
339341
}
340-
}
341-
if (operation.getOutput() != null) {
342-
String outputShapeName = operation.getOutput().getShape();
343-
Shape outputShape = allC2jShapes.get(outputShapeName);
344-
if (parentShape.equals(outputShape)) {
345-
return true;
342+
if (operation.getOutput() != null) {
343+
directOperationShapes.add(allC2jShapes.get(operation.getOutput().getShape()));
346344
}
347-
}
348-
349-
if (operation.getErrors() != null) {
350-
for (ErrorMap error : operation.getErrors()) {
351-
String errorShapeName = error.getShape();
352-
Shape outputShape = allC2jShapes.get(errorShapeName);
353-
if (parentShape.equals(outputShape)) {
354-
return true;
345+
if (operation.getErrors() != null) {
346+
for (ErrorMap error : operation.getErrors()) {
347+
directOperationShapes.add(allC2jShapes.get(error.getShape()));
355348
}
356349
}
357350
}
358351
}
359-
return false;
352+
return directOperationShapes.contains(parentShape);
360353
}
361354

362355
private boolean isFlattened(Member member, Shape memberShape) {
@@ -413,7 +406,7 @@ private Optional<String> findRequestUri(Shape parentShape, Map<String, Shape> al
413406
.filter(e -> e.getValue().equals(parentShape))
414407
.map(Map.Entry::getKey)
415408
.findFirst()
416-
.get();
409+
.orElseThrow(() -> new IllegalStateException("Shape not found in model: " + parentShape));
417410
String detailMsg = "Could not find request URI for input shape '" + shapeName
418411
+ "'. No operation was found that references this shape as its input.";
419412
ValidationEntry entry =

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

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

8686
assertThat(requestShapeModel.getRequired()).contains(queryParamName);
87-
assertThat(requiredMemberModel.getHttp().getLocation()).isNull();
8887
assertThat(requiredMemberModel.isRequired()).isTrue();
8988
}
9089

90+
@Test
91+
void generateShapeModel_locationOnNestedShape_isIgnored() {
92+
ShapeModel nestedShape = intermediateModel.getShapes().get("NestedQueryParameterOperation");
93+
MemberModel queryParam = nestedShape.findMemberModelByC2jName("QueryParamOne");
94+
assertThat(queryParam.getHttp().getLocation()).isNull();
95+
}
96+
97+
@Test
98+
void generateShapeModel_locationOnDirectInputShape_isPreserved() {
99+
ShapeModel inputShape = intermediateModel.getShapes().get("QueryParameterOperationRequest");
100+
assertThat(inputShape.findMemberModelByC2jName("PathParam").getHttp().getLocation()).isEqualTo(Location.URI);
101+
assertThat(inputShape.findMemberModelByC2jName("QueryParamOne").getHttp().getLocation()).isEqualTo(Location.QUERY_STRING);
102+
assertThat(inputShape.findMemberModelByC2jName("StringHeaderMember").getHttp().getLocation()).isEqualTo(Location.HEADER);
103+
}
104+
91105
}

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,15 @@ void execute_uriLocationOnNonInputShape_isIgnored() throws IOException {
193193
() -> generateCodeFromC2jModels(models, outputDir, true, Collections.emptyList()));
194194

195195
IntermediateModel intermediateModel = new IntermediateModelBuilder(models).build();
196+
196197
ShapeModel inputShape = intermediateModel.getShapes().get("SomeOperationRequest");
197-
MemberModel uriMember = inputShape.findMemberModelByC2jName("thingId");
198-
assertThat(uriMember.getHttp().getLocation()).isEqualTo(Location.URI);
198+
assertThat(inputShape.findMemberModelByC2jName("thingId").getHttp().getLocation()).isEqualTo(Location.URI);
199199

200200
ShapeModel nestedShape = intermediateModel.getShapes().get("NestedOptions");
201-
MemberModel nestedUriMember = nestedShape.findMemberModelByC2jName("pageSize");
202-
assertThat(nestedUriMember.getHttp().getLocation()).isNull();
203-
assertThat(nestedUriMember.getHttp().isGreedy()).isFalse();
201+
assertThat(nestedShape.findMemberModelByC2jName("pageSize").getHttp().getLocation()).isNull();
202+
assertThat(nestedShape.findMemberModelByC2jName("pageSize").getHttp().isGreedy()).isFalse();
203+
assertThat(nestedShape.findMemberModelByC2jName("headerParam").getHttp().getLocation()).isNull();
204+
assertThat(nestedShape.findMemberModelByC2jName("queryParam").getHttp().getLocation()).isNull();
204205

205206
Path generatedNestedOptions = Files.walk(outputDir)
206207
.filter(p -> p.getFileName().toString().equals("NestedOptions.java"))

codegen/src/test/resources/software/amazon/awssdk/codegen/expected-nested-options.java

Lines changed: 99 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,32 @@ public final class NestedOptions implements SdkPojo, Serializable, ToCopyableBui
4343
.memberName("pageSize").getter(getter(NestedOptions::pageSize)).setter(setter(Builder::pageSize))
4444
.traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("pageSize").build()).build();
4545

46-
private static final List<SdkField<?>> SDK_FIELDS = Collections.unmodifiableList(Arrays.asList(PAGE_SIZE_FIELD));
46+
private static final SdkField<String> HEADER_PARAM_FIELD = SdkField.<String> builder(MarshallingType.STRING)
47+
.memberName("headerParam").getter(getter(NestedOptions::headerParam)).setter(setter(Builder::headerParam))
48+
.traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("x-amz-nested-header").build())
49+
.build();
50+
51+
private static final SdkField<String> QUERY_PARAM_FIELD = SdkField.<String> builder(MarshallingType.STRING)
52+
.memberName("queryParam").getter(getter(NestedOptions::queryParam)).setter(setter(Builder::queryParam))
53+
.traits(LocationTrait.builder().location(MarshallLocation.PAYLOAD).locationName("nestedQuery").build()).build();
54+
55+
private static final List<SdkField<?>> SDK_FIELDS = Collections.unmodifiableList(Arrays.asList(PAGE_SIZE_FIELD,
56+
HEADER_PARAM_FIELD, QUERY_PARAM_FIELD));
4757

4858
private static final Map<String, SdkField<?>> SDK_NAME_TO_FIELD = memberNameToFieldInitializer();
4959

5060
private static final long serialVersionUID = 1L;
5161

5262
private final String pageSize;
5363

64+
private final String headerParam;
65+
66+
private final String queryParam;
67+
5468
private NestedOptions(BuilderImpl builder) {
5569
this.pageSize = builder.pageSize;
70+
this.headerParam = builder.headerParam;
71+
this.queryParam = builder.queryParam;
5672
}
5773

5874
/**
@@ -64,6 +80,24 @@ public final String pageSize() {
6480
return pageSize;
6581
}
6682

83+
/**
84+
* Returns the value of the HeaderParam property for this object.
85+
*
86+
* @return The value of the HeaderParam property for this object.
87+
*/
88+
public final String headerParam() {
89+
return headerParam;
90+
}
91+
92+
/**
93+
* Returns the value of the QueryParam property for this object.
94+
*
95+
* @return The value of the QueryParam property for this object.
96+
*/
97+
public final String queryParam() {
98+
return queryParam;
99+
}
100+
67101
@Override
68102
public Builder toBuilder() {
69103
return new BuilderImpl(this);
@@ -81,6 +115,8 @@ public static Class<? extends Builder> serializableBuilderClass() {
81115
public final int hashCode() {
82116
int hashCode = 1;
83117
hashCode = 31 * hashCode + Objects.hashCode(pageSize());
118+
hashCode = 31 * hashCode + Objects.hashCode(headerParam());
119+
hashCode = 31 * hashCode + Objects.hashCode(queryParam());
84120
return hashCode;
85121
}
86122

@@ -101,7 +137,8 @@ public final boolean equalsBySdkFields(Object obj) {
101137
return false;
102138
}
103139
NestedOptions other = (NestedOptions) obj;
104-
return Objects.equals(pageSize(), other.pageSize());
140+
return Objects.equals(pageSize(), other.pageSize()) && Objects.equals(headerParam(), other.headerParam())
141+
&& Objects.equals(queryParam(), other.queryParam());
105142
}
106143

107144
/**
@@ -110,13 +147,18 @@ public final boolean equalsBySdkFields(Object obj) {
110147
*/
111148
@Override
112149
public final String toString() {
113-
return ToString.builder("NestedOptions").add("PageSize", pageSize()).build();
150+
return ToString.builder("NestedOptions").add("PageSize", pageSize()).add("HeaderParam", headerParam())
151+
.add("QueryParam", queryParam()).build();
114152
}
115153

116154
public final <T> Optional<T> getValueForField(String fieldName, Class<T> clazz) {
117155
switch (fieldName) {
118156
case "pageSize":
119157
return Optional.ofNullable(clazz.cast(pageSize()));
158+
case "headerParam":
159+
return Optional.ofNullable(clazz.cast(headerParam()));
160+
case "queryParam":
161+
return Optional.ofNullable(clazz.cast(queryParam()));
120162
default:
121163
return Optional.empty();
122164
}
@@ -135,6 +177,8 @@ public final Map<String, SdkField<?>> sdkFieldNameToField() {
135177
private static Map<String, SdkField<?>> memberNameToFieldInitializer() {
136178
Map<String, SdkField<?>> map = new HashMap<>();
137179
map.put("pageSize", PAGE_SIZE_FIELD);
180+
map.put("x-amz-nested-header", HEADER_PARAM_FIELD);
181+
map.put("nestedQuery", QUERY_PARAM_FIELD);
138182
return Collections.unmodifiableMap(map);
139183
}
140184

@@ -157,16 +201,40 @@ public interface Builder extends SdkPojo, CopyableBuilder<Builder, NestedOptions
157201
* @return Returns a reference to this object so that method calls can be chained together.
158202
*/
159203
Builder pageSize(String pageSize);
204+
205+
/**
206+
* Sets the value of the HeaderParam property for this object.
207+
*
208+
* @param headerParam
209+
* The new value for the HeaderParam property for this object.
210+
* @return Returns a reference to this object so that method calls can be chained together.
211+
*/
212+
Builder headerParam(String headerParam);
213+
214+
/**
215+
* Sets the value of the QueryParam property for this object.
216+
*
217+
* @param queryParam
218+
* The new value for the QueryParam property for this object.
219+
* @return Returns a reference to this object so that method calls can be chained together.
220+
*/
221+
Builder queryParam(String queryParam);
160222
}
161223

162224
static final class BuilderImpl implements Builder {
163225
private String pageSize;
164226

227+
private String headerParam;
228+
229+
private String queryParam;
230+
165231
private BuilderImpl() {
166232
}
167233

168234
private BuilderImpl(NestedOptions model) {
169235
pageSize(model.pageSize);
236+
headerParam(model.headerParam);
237+
queryParam(model.queryParam);
170238
}
171239

172240
public final String getPageSize() {
@@ -183,6 +251,34 @@ public final Builder pageSize(String pageSize) {
183251
return this;
184252
}
185253

254+
public final String getHeaderParam() {
255+
return headerParam;
256+
}
257+
258+
public final void setHeaderParam(String headerParam) {
259+
this.headerParam = headerParam;
260+
}
261+
262+
@Override
263+
public final Builder headerParam(String headerParam) {
264+
this.headerParam = headerParam;
265+
return this;
266+
}
267+
268+
public final String getQueryParam() {
269+
return queryParam;
270+
}
271+
272+
public final void setQueryParam(String queryParam) {
273+
this.queryParam = queryParam;
274+
}
275+
276+
@Override
277+
public final Builder queryParam(String queryParam) {
278+
this.queryParam = queryParam;
279+
return this;
280+
}
281+
186282
@Override
187283
public NestedOptions build() {
188284
return new NestedOptions(this);

codegen/src/test/resources/software/amazon/awssdk/codegen/uri-on-non-input-shape-service.json

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,22 @@
4646
"shape": "String",
4747
"location": "uri",
4848
"locationName": "pageSize"
49+
},
50+
"headerParam": {
51+
"shape": "String",
52+
"location": "header",
53+
"locationName": "x-amz-nested-header"
54+
},
55+
"queryParam": {
56+
"shape": "String",
57+
"location": "querystring",
58+
"locationName": "nestedQuery"
4959
}
5060
}
5161
},
5262
"String": {
5363
"type": "string"
5464
}
5565
},
56-
"documentation": "A service with a uri-bound member on a non-input shape"
66+
"documentation": "A service with HTTP binding locations on non-input shapes"
5767
}

0 commit comments

Comments
 (0)