Skip to content

Commit 685e012

Browse files
alextwoodsalextwoods
authored
Fix Endpoint StringArray Parameter + operationContextParams issues (#6712)
* WIP - start adding generator * Add support for stringArray staticContextParams * Improve support for parsing multi-select lists in JMESPath * Added support for multiselect expressions in jmespath acceptor generator + fix endpoitprovider task checks for operationContextParams * Cleanup and fix checkstyles * Implement tests for all string array bindings * Improve comments/naming + fix generated class test * Add changelog
1 parent f219b1d commit 685e012

16 files changed

Lines changed: 990 additions & 16 deletions

File tree

‎.changes/next-release/bugfix-AWSSDKforJavav2-eade5c9.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Improve support for operationContextParams with chained index and multi-select expressions and improve support for StringArray endpoint parametes."
6+
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/emitters/tasks/EndpointProviderTasks.java‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.util.Arrays;
2020
import java.util.Collection;
2121
import java.util.List;
22-
import java.util.Locale;
2322
import java.util.Map;
2423
import software.amazon.awssdk.codegen.emitters.GeneratorTask;
2524
import software.amazon.awssdk.codegen.emitters.GeneratorTaskParams;
@@ -160,21 +159,16 @@ private boolean shouldGenerateJmesPathRuntime() {
160159
return true;
161160
}
162161

163-
Map<String, ParameterModel> endpointParameters = model.getCustomizationConfig().getEndpointParameters();
162+
Map<String, ParameterModel> endpointParameters = model.getEndpointRuleSetModel().getParameters();
164163
if (endpointParameters == null) {
165164
return false;
166165
}
167166

168-
return endpointParameters.values().stream().anyMatch(this::paramRequiresPathParserRuntime);
169-
}
170-
171-
private boolean paramRequiresPathParserRuntime(ParameterModel parameterModel) {
172-
return paramIsOperationalContextParam(parameterModel) &&
173-
"stringarray".equals(parameterModel.getType().toLowerCase(Locale.US));
174-
}
175-
176-
//TODO (string-array-params): resolve this logical test before finalizing coding
177-
private boolean paramIsOperationalContextParam(ParameterModel parameterModel) {
178-
return true;
167+
// if any operation has operationContextParams then we must include jmesPathRuntime
168+
return model.getOperations().values().stream()
169+
.anyMatch(op -> {
170+
Map<String, ?> opContextParams = op.getOperationContextParams();
171+
return opContextParams != null && !opContextParams.isEmpty();
172+
});
179173
}
180174
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/jmespath/component/BracketSpecifier.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ public static BracketSpecifier withWildcardExpressionContents(WildcardExpression
5050
return withContents(BracketSpecifierWithContents.wildcardExpression(wildcardExpression));
5151
}
5252

53+
public static BracketSpecifier withMultiSelectListContents(MultiSelectList multiSelectList) {
54+
return withContents(BracketSpecifierWithContents.multiSelectList(multiSelectList));
55+
}
56+
5357
public static BracketSpecifier withoutContents() {
5458
BracketSpecifier result = new BracketSpecifier();
5559
result.bracketSpecifierWithoutContents = new BracketSpecifierWithoutContents();

‎codegen/src/main/java/software/amazon/awssdk/codegen/jmespath/component/BracketSpecifierWithContents.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,14 @@
2424
* <li>A number, as in [1]</li>
2525
* <li>A star expression, as in [*]</li>
2626
* <li>A slice expression, as in [1:2:3]</li>
27+
* <li>A multi-select-list, as in [string, object.key]</li>
2728
* </ul>
2829
*/
2930
public class BracketSpecifierWithContents {
3031
private Integer number;
3132
private WildcardExpression wildcardExpression;
3233
private SliceExpression sliceExpression;
34+
private MultiSelectList multiSelectList;
3335

3436
private BracketSpecifierWithContents() {
3537
}
@@ -55,6 +57,13 @@ public static BracketSpecifierWithContents sliceExpression(SliceExpression slice
5557
return result;
5658
}
5759

60+
public static BracketSpecifierWithContents multiSelectList(MultiSelectList multiSelectList) {
61+
Validate.notNull(multiSelectList, "multiSelectList");
62+
BracketSpecifierWithContents result = new BracketSpecifierWithContents();
63+
result.multiSelectList = multiSelectList;
64+
return result;
65+
}
66+
5867
public boolean isNumber() {
5968
return number != null;
6069
}
@@ -67,6 +76,10 @@ public boolean isSliceExpression() {
6776
return sliceExpression != null;
6877
}
6978

79+
public boolean isMultiSelectList() {
80+
return multiSelectList != null;
81+
}
82+
7083
public int asNumber() {
7184
Validate.validState(isNumber(), "Not a Number");
7285
return number;
@@ -82,13 +95,20 @@ public SliceExpression asSliceExpression() {
8295
return sliceExpression;
8396
}
8497

98+
public MultiSelectList asMultiSelectList() {
99+
Validate.validState(isMultiSelectList(), "Not a MultiSelectList");
100+
return multiSelectList;
101+
}
102+
85103
public void visit(JmesPathVisitor visitor) {
86104
if (isNumber()) {
87105
visitor.visitNumber(asNumber());
88106
} else if (isWildcardExpression()) {
89107
visitor.visitWildcardExpression(asWildcardExpression());
90108
} else if (isSliceExpression()) {
91109
visitor.visitSliceExpression(asSliceExpression());
110+
} else if (isMultiSelectList()) {
111+
visitor.visitMultiSelectList(asMultiSelectList());
92112
} else {
93113
throw new IllegalStateException();
94114
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/jmespath/parser/JmesPathParser.java‎

Lines changed: 111 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static Expression parse(String jmesPathString) {
7373
private Expression parse() {
7474
ParseResult<Expression> expression = parseExpression(0, input.length());
7575
if (!expression.hasResult()) {
76-
throw new IllegalArgumentException("Failed to parse expression.");
76+
throw new IllegalArgumentException("Failed to parse expression: " + input);
7777
}
7878

7979
return expression.result();
@@ -246,13 +246,21 @@ private ParseResult<IndexExpression> parseIndexExpressionWithLhsExpression(int s
246246
endPosition = trimRightWhitespace(startPosition, endPosition);
247247

248248
List<Integer> bracketPositions = findCharacters(startPosition + 1, endPosition - 1, "[");
249-
for (Integer bracketPosition : bracketPositions) {
249+
250+
// Try bracket positions from right to left to support chaining multiple bracket specifiers
251+
// e.g., listOfUnions[*][string, object.key][] should parse as:
252+
// - left: listOfUnions[*][string, object.key]
253+
// - right: []
254+
// This allows both the left and right side to be recursively parsed
255+
for (int i = bracketPositions.size() - 1; i >= 0; i--) {
256+
Integer bracketPosition = bracketPositions.get(i);
250257
ParseResult<Expression> leftSide = parseExpression(startPosition, bracketPosition);
251258
if (!leftSide.hasResult()) {
252259
continue;
253260
}
254261

255-
ParseResult<BracketSpecifier> rightSide = parseBracketSpecifier(bracketPosition, endPosition);
262+
// we know there is a left hand expression and that the Rhs is a bracketSpecifier
263+
ParseResult<BracketSpecifier> rightSide = parseBracketSpecifierWithLhsExpression(bracketPosition, endPosition);
256264
if (!rightSide.hasResult()) {
257265
continue;
258266
}
@@ -272,6 +280,74 @@ private ParseResult<MultiSelectList> parseMultiSelectList(int startPosition, int
272280
.mapResult(MultiSelectList::new);
273281
}
274282

283+
/**
284+
* Parse multi-select-list content without the surrounding brackets.
285+
* Used when parsing bracket specifier contents like [string, object.key] or [string]
286+
* This parses: expression *( "," expression )
287+
*/
288+
private ParseResult<MultiSelectList> parseMultiSelectListContent(int startPosition, int endPosition) {
289+
startPosition = trimLeftWhitespace(startPosition, endPosition);
290+
endPosition = trimRightWhitespace(startPosition, endPosition);
291+
292+
List<Integer> commaPositions = findCharacters(startPosition, endPosition, ",");
293+
294+
// Single expression case (no commas)
295+
if (commaPositions.isEmpty()) {
296+
ParseResult<Expression> singleExpr = parseExpression(startPosition, endPosition);
297+
if (!singleExpr.hasResult()) {
298+
return ParseResult.error();
299+
}
300+
return ParseResult.success(new MultiSelectList(Collections.singletonList(singleExpr.result())));
301+
}
302+
303+
// Multiple expressions separated by commas
304+
List<Expression> expressions = new ArrayList<>();
305+
306+
// Find first valid entry before a comma
307+
int startOfSecondEntry = -1;
308+
for (Integer comma : commaPositions) {
309+
ParseResult<Expression> result = parseExpression(startPosition, comma);
310+
if (!result.hasResult()) {
311+
continue;
312+
}
313+
314+
expressions.add(result.result());
315+
startOfSecondEntry = comma + 1;
316+
break;
317+
}
318+
319+
if (expressions.size() == 0) {
320+
logError("multi-select-list-content", "Invalid value", startPosition);
321+
return ParseResult.error();
322+
}
323+
324+
// Find any subsequent entries
325+
int startPositionAfterComma = startOfSecondEntry;
326+
for (Integer commaPosition : commaPositions) {
327+
if (startPositionAfterComma > commaPosition) {
328+
continue;
329+
}
330+
331+
ParseResult<Expression> entry = parseExpression(startPositionAfterComma, commaPosition);
332+
if (!entry.hasResult()) {
333+
continue;
334+
}
335+
336+
expressions.add(entry.result());
337+
startPositionAfterComma = commaPosition + 1;
338+
}
339+
340+
// Parse the last entry after the final comma
341+
ParseResult<Expression> entry = parseExpression(startPositionAfterComma, endPosition);
342+
if (!entry.hasResult()) {
343+
logError("multi-select-list-content", "Invalid final entry", startPositionAfterComma);
344+
return ParseResult.error();
345+
}
346+
expressions.add(entry.result());
347+
348+
return ParseResult.success(new MultiSelectList(expressions));
349+
}
350+
275351
/**
276352
* multi-select-hash = "{" ( keyval-expr *( "," keyval-expr ) ) "}"
277353
*/
@@ -410,6 +486,38 @@ private ParseResult<BracketSpecifier> parseBracketSpecifier(int startPosition, i
410486
.parse(startPosition + 1, endPosition - 1);
411487
}
412488

489+
/**
490+
* This is a special case for bracket specifiers with a left-hand expression and which can contain multi-select-lists.
491+
* bracket-specifier-with-multiselect = "[" multi-select-list-content "]"
492+
*/
493+
private ParseResult<BracketSpecifier> parseBracketSpecifierWithLhsExpression(int startPosition, int endPosition) {
494+
startPosition = trimLeftWhitespace(startPosition, endPosition);
495+
endPosition = trimRightWhitespace(startPosition, endPosition);
496+
497+
if (!startsAndEndsWith(startPosition, endPosition, '[', ']')) {
498+
logError("bracket-specifier-with-multiselect", "Expecting '[' and ']'", startPosition);
499+
return ParseResult.error();
500+
}
501+
502+
// "[]"
503+
if (charsInRange(startPosition, endPosition) == 2) {
504+
return ParseResult.success(BracketSpecifier.withoutContents());
505+
}
506+
507+
// "[?" expression "]"
508+
if (input.charAt(startPosition + 1) == '?') {
509+
return parseExpression(startPosition + 2, endPosition - 1)
510+
.mapResult(e -> BracketSpecifier.withQuestionMark(new BracketSpecifierWithQuestionMark(e)));
511+
}
512+
513+
// "[" (number / "*" / slice-expression / multi-select-list-content) "]"
514+
return CompositeParser.firstTry(this::parseNumber, BracketSpecifier::withNumberContents)
515+
.thenTry(this::parseWildcardExpression, BracketSpecifier::withWildcardExpressionContents)
516+
.thenTry(this::parseSliceExpression, BracketSpecifier::withSliceExpressionContents)
517+
.thenTry(this::parseMultiSelectListContent, BracketSpecifier::withMultiSelectListContents)
518+
.parse(startPosition + 1, endPosition - 1);
519+
}
520+
413521
/**
414522
* comparator-expression = expression comparator expression
415523
*/

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverInterceptorSpec.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import com.fasterxml.jackson.core.JsonToken;
1919
import com.fasterxml.jackson.core.TreeNode;
20+
import com.fasterxml.jackson.jr.stree.JrsArray;
2021
import com.fasterxml.jackson.jr.stree.JrsBoolean;
2122
import com.fasterxml.jackson.jr.stree.JrsString;
2223
import com.squareup.javapoet.ClassName;
@@ -364,6 +365,11 @@ private MethodSpec addStaticContextParamsMethod(OperationModel opModel) {
364365
case VALUE_FALSE:
365366
b.addStatement("params.$N($L)", setterName, ((JrsBoolean) value).booleanValue());
366367
break;
368+
case START_ARRAY:
369+
JrsArray arrayValue = (JrsArray) value;
370+
CodeBlock arrayCode = endpointRulesSpecUtils.treeNodeToLiteral(arrayValue);
371+
b.addStatement("params.$N($L)", setterName, arrayCode);
372+
break;
367373
default:
368374
throw new RuntimeException("Don't know how to set parameter of type " + value.asToken());
369375
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/waiters/JmesPathAcceptorGenerator.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ public void visitBracketSpecifierWithContents(BracketSpecifierWithContents input
123123
codeBlock.add(".index(" + input.asNumber() + ")");
124124
} else if (input.isWildcardExpression()) {
125125
codeBlock.add(".wildcard()");
126+
} else if (input.isMultiSelectList()) {
127+
visitMultiSelectList(input.asMultiSelectList());
126128
} else {
127129
throw new UnsupportedOperationException();
128130
}

‎codegen/src/test/java/software/amazon/awssdk/codegen/jmespath/JmesPathParserTest.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.junit.jupiter.api.Test;
2121
import software.amazon.awssdk.codegen.jmespath.component.Comparator;
2222
import software.amazon.awssdk.codegen.jmespath.component.Expression;
23+
import software.amazon.awssdk.codegen.jmespath.component.MultiSelectList;
2324
import software.amazon.awssdk.codegen.jmespath.parser.JmesPathParser;
2425

2526
public class JmesPathParserTest {
@@ -38,6 +39,38 @@ public void testSubExpressionWithMultiSelectList() {
3839
assertThat(expression.asSubExpression().rightSubExpression().asMultiSelectList().expressions().get(0).asIdentifier()).isEqualTo("bar");
3940
}
4041

42+
@Test
43+
public void testSubExpressionWithMultiSelectListAndFlatten() {
44+
Expression expression = JmesPathParser.parse("listOfUnions[*][string, object.key][]");
45+
46+
// The expression should be parsed as:
47+
// IndexExpression(IndexExpression(IndexExpression(listOfUnions, [*]), [string, object.key]), [])
48+
assertThat(expression.isIndexExpression()).isTrue();
49+
50+
// the right most flatten
51+
assertThat(expression.asIndexExpression().bracketSpecifier().isBracketSpecifierWithoutContents()).isTrue();
52+
53+
// Middle: [string, object.key]
54+
Expression middleBracketsExpr = expression.asIndexExpression().expression().get();
55+
assertThat(middleBracketsExpr.isIndexExpression()).isTrue();
56+
assertThat(middleBracketsExpr.asIndexExpression().bracketSpecifier().isBracketSpecifierWithContents()).isTrue();
57+
assertThat(middleBracketsExpr.asIndexExpression().bracketSpecifier().asBracketSpecifierWithContents().isMultiSelectList()).isTrue();
58+
59+
MultiSelectList multiSelectList = middleBracketsExpr.asIndexExpression().bracketSpecifier()
60+
.asBracketSpecifierWithContents().asMultiSelectList();
61+
assertThat(multiSelectList.expressions()).hasSize(2);
62+
assertThat(multiSelectList.expressions().get(0).asIdentifier()).isEqualTo("string");
63+
assertThat(multiSelectList.expressions().get(1).asSubExpression().leftExpression().asIdentifier()).isEqualTo("object");
64+
assertThat(multiSelectList.expressions().get(1).asSubExpression().rightSubExpression().asIdentifier()).isEqualTo("key");
65+
66+
// left most wildcard: listOfUnions[*]
67+
Expression wildCardExpr = middleBracketsExpr.asIndexExpression().expression().get();
68+
assertThat(wildCardExpr.isIndexExpression()).isTrue();
69+
assertThat(wildCardExpr.asIndexExpression().expression().get().asIdentifier()).isEqualTo("listOfUnions");
70+
assertThat(wildCardExpr.asIndexExpression().bracketSpecifier().isBracketSpecifierWithContents()).isTrue();
71+
assertThat(wildCardExpr.asIndexExpression().bracketSpecifier().asBracketSpecifierWithContents().isWildcardExpression()).isTrue();
72+
}
73+
4174
@Test
4275
public void testSubExpressionWithMultiSelectHash() {
4376
Expression expression = JmesPathParser.parse("foo.{bar : baz}");

‎codegen/src/test/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverInterceptorSpecTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,10 @@ void endpointResolverInterceptorClassWithEndpointBasedAuth() {
4646
ClassSpec endpointProviderInterceptor = new EndpointResolverInterceptorSpec(ClientTestModels.queryServiceModelsEndpointAuthParamsWithoutAllowList());
4747
assertThat(endpointProviderInterceptor, generatesTo("endpoint-resolve-interceptor-with-endpointsbasedauth.java"));
4848
}
49+
50+
@Test
51+
public void endpointProviderTestClassWithStringArray() {
52+
ClassSpec endpointProviderInterceptor = new EndpointResolverInterceptorSpec(ClientTestModels.stringArrayServiceModels());
53+
assertThat(endpointProviderInterceptor, generatesTo("endpoint-resolve-interceptor-with-stringarray.java"));
54+
}
4955
}

‎codegen/src/test/java/software/amazon/awssdk/codegen/poet/waiters/JmesPathAcceptorGeneratorTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,13 @@ void testNestedMultiSelectListsLeft() {
185185
+ "x3 -> x3.field(\"baz\"))");
186186
}
187187

188+
@Test
189+
void testMultiSelectListAndFlatten() {
190+
testConversion("listOfUnions[*][string, object.key][]",
191+
"input.field(\"listOfUnions\").wildcard().multiSelectList(x0 -> x0.field(\"string\"), "
192+
+ "x1 -> x1.field(\"object\").field(\"key\")).flatten()");
193+
}
194+
188195
@Test
189196
void testMultiSelectHash2() {
190197
assertThatThrownBy(() -> testConversion("{fooK : fooV, barK : barV}", ""))

0 commit comments

Comments
 (0)