Skip to content

Commit af679ed

Browse files
mtdowlingadwsingh
mtdowling
authored and
adwsingh
committed
Fix various rules engine issues
1 parent e1d56a7 commit af679ed

12 files changed

Lines changed: 163 additions & 65 deletions

File tree

‎aws/client/aws-client-rulesengine/build.gradle.kts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,8 @@ dependencies {
1111
api(project(":aws:client:aws-client-core"))
1212
api(project(":client:client-rulesengine"))
1313
api(libs.smithy.aws.endpoints)
14+
15+
testImplementation(libs.smithy.aws.traits)
16+
testImplementation(project(":aws:client:aws-client-restxml"))
17+
testImplementation(project(":client:dynamic-client"))
1418
}

‎aws/client/aws-client-rulesengine/src/main/java/software/amazon/smithy/java/aws/client/rulesengine/AwsRulesFunction.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,13 @@ public Object apply1(Object arg1) {
9292
return null;
9393
}
9494
return Map.of(
95-
ParseArn.PARTITION,
95+
ParseArn.PARTITION.toString(),
9696
awsArn.getPartition(),
97-
ParseArn.SERVICE,
97+
ParseArn.SERVICE.toString(),
9898
awsArn.getService(),
99-
ParseArn.REGION,
99+
ParseArn.REGION.toString(),
100100
awsArn.getRegion(),
101-
ParseArn.ACCOUNT_ID,
101+
ParseArn.ACCOUNT_ID.toString(),
102102
awsArn.getAccountId(),
103103
"resourceId", // TODO: make this one public too in Smithy
104104
awsArn.getResource());

aws/client/aws-client-rulesengine/src/main/resources/META-INF/resources/software.amazon.smithy.java.client.rulesengine.RulesExtension renamed to aws/client/aws-client-rulesengine/src/main/resources/META-INF/services/software.amazon.smithy.java.client.rulesengine.RulesExtension

File renamed without changes.

‎aws/client/aws-client-rulesengine/src/test/java/software/amazon/smithy/java/aws/client/rulesengine/RunnerTest.java‎

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎client/client-rulesengine/src/main/java/software/amazon/smithy/java/client/rulesengine/EndpointRulesPlugin.java‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@
55

66
package software.amazon.smithy.java.client.rulesengine;
77

8+
import java.util.Map;
89
import software.amazon.smithy.java.client.core.ClientConfig;
910
import software.amazon.smithy.java.client.core.ClientContext;
1011
import software.amazon.smithy.java.client.core.ClientPlugin;
12+
import software.amazon.smithy.java.context.Context;
1113
import software.amazon.smithy.java.core.schema.TraitKey;
14+
import software.amazon.smithy.java.logging.InternalLogger;
1215
import software.amazon.smithy.rulesengine.traits.ContextParamTrait;
1316
import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait;
1417
import software.amazon.smithy.rulesengine.traits.OperationContextParamsTrait;
@@ -19,6 +22,11 @@
1922
*/
2023
public final class EndpointRulesPlugin implements ClientPlugin {
2124

25+
private static final InternalLogger LOGGER = InternalLogger.getLogger(EndpointRulesPlugin.class);
26+
27+
public static final Context.Key<Map<String, Object>> ADDITIONAL_ENDPOINT_PARAMS = Context.key(
28+
"Additional endpoint parameters to pass to the rules engine");
29+
2230
public static final TraitKey<StaticContextParamsTrait> STATIC_CONTEXT_PARAMS_TRAIT =
2331
TraitKey.get(StaticContextParamsTrait.class);
2432

@@ -30,7 +38,7 @@ public final class EndpointRulesPlugin implements ClientPlugin {
3038
public static final TraitKey<EndpointRuleSetTrait> ENDPOINT_RULESET_TRAIT =
3139
TraitKey.get(EndpointRuleSetTrait.class);
3240

33-
private final RulesProgram program;
41+
private RulesProgram program;
3442

3543
private EndpointRulesPlugin(RulesProgram program) {
3644
this.program = program;
@@ -72,19 +80,31 @@ public RulesProgram getProgram() {
7280
public void configureClient(ClientConfig.Builder config) {
7381
// Only modify the endpoint resolver if it isn't set already or if CUSTOM_ENDPOINT is set,
7482
// and if a program was provided.
75-
if (config.endpointResolver() == null || config.context().get(ClientContext.CUSTOM_ENDPOINT) != null) {
76-
if (program != null) {
77-
applyResolver(program, config);
78-
} else if (config.service() != null) {
83+
boolean usePlugin = false;
84+
if (config.endpointResolver() == null) {
85+
usePlugin = true;
86+
LOGGER.debug("Trying to use EndpointRulesPlugin resolver because endpointResolver is null");
87+
} else if (config.context().get(ClientContext.CUSTOM_ENDPOINT) != null) {
88+
usePlugin = true;
89+
LOGGER.debug("Trying to use EndpointRulesPlugin resolver because CUSTOM_ENDPOINT is set");
90+
}
91+
92+
if (usePlugin) {
93+
if (program == null && config.service() != null) {
7994
var ruleset = config.service().schema().getTrait(ENDPOINT_RULESET_TRAIT);
8095
if (ruleset != null) {
81-
applyResolver(new RulesEngine().compile(ruleset.getEndpointRuleSet()), config);
96+
LOGGER.debug("Found endpoint rules traits on service: {}", config.service());
97+
program = new RulesEngine().compile(ruleset.getEndpointRuleSet());
8298
}
8399
}
100+
if (program != null) {
101+
applyResolver(program, config);
102+
}
84103
}
85104
}
86105

87106
private void applyResolver(RulesProgram applyProgram, ClientConfig.Builder config) {
88107
config.endpointResolver(new EndpointRulesResolver(applyProgram));
108+
LOGGER.debug("Applying EndpointRulesResolver to client: {}", config.service());
89109
}
90110
}

‎client/client-rulesengine/src/main/java/software/amazon/smithy/java/client/rulesengine/EndpointRulesResolver.java‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,18 @@
1111
import software.amazon.smithy.java.client.core.endpoint.Endpoint;
1212
import software.amazon.smithy.java.client.core.endpoint.EndpointResolver;
1313
import software.amazon.smithy.java.client.core.endpoint.EndpointResolverParams;
14+
import software.amazon.smithy.java.context.Context;
1415
import software.amazon.smithy.java.core.schema.ApiOperation;
1516
import software.amazon.smithy.java.core.schema.SerializableStruct;
17+
import software.amazon.smithy.java.logging.InternalLogger;
1618

1719
/**
1820
* Endpoint resolver that uses the endpoint rules engine.
1921
*/
2022
final class EndpointRulesResolver implements EndpointResolver {
2123

24+
private static final InternalLogger LOGGER = InternalLogger.getLogger(EndpointRulesResolver.class);
25+
2226
private final RulesProgram program;
2327
private final ContextProvider operationContextParams = new ContextProvider.OrchestratingProvider();
2428

@@ -29,16 +33,28 @@ final class EndpointRulesResolver implements EndpointResolver {
2933
@Override
3034
public CompletableFuture<Endpoint> resolveEndpoint(EndpointResolverParams params) {
3135
try {
32-
var endpointParams = createEndpointParams(params.operation(), params.inputValue());
36+
var operation = params.operation();
37+
var endpointParams = createEndpointParams(params.context(), operation, params.inputValue());
38+
LOGGER.debug("Resolving endpoint of {} using VM with params: {}", operation, endpointParams);
3339
return CompletableFuture.completedFuture(program.resolveEndpoint(params.context(), endpointParams));
3440
} catch (RulesEvaluationError e) {
3541
return CompletableFuture.failedFuture(e);
3642
}
3743
}
3844

39-
private Map<String, Object> createEndpointParams(ApiOperation<?, ?> operation, SerializableStruct input) {
45+
private Map<String, Object> createEndpointParams(
46+
Context context,
47+
ApiOperation<?, ?> operation,
48+
SerializableStruct input
49+
) {
4050
Map<String, Object> params = new HashMap<>();
4151
operationContextParams.addContext(operation, input, params);
52+
53+
var additionalParams = context.get(EndpointRulesPlugin.ADDITIONAL_ENDPOINT_PARAMS);
54+
if (additionalParams != null) {
55+
params.putAll(additionalParams);
56+
}
57+
4258
return params;
4359
}
4460
}

‎client/client-rulesengine/src/main/java/software/amazon/smithy/java/client/rulesengine/EndpointUtils.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@
2525
import software.amazon.smithy.rulesengine.language.evaluation.value.Value;
2626
import software.amazon.smithy.rulesengine.language.syntax.expressions.functions.ParseUrl;
2727

28-
final class EndpointUtils {
28+
public final class EndpointUtils {
2929

3030
private EndpointUtils() {}
3131

3232
// "The type of the value MUST be either a string, boolean or an array of string."
33-
static Object convertNode(Node value, boolean allowAllTypes) {
33+
public static Object convertNode(Node value, boolean allowAllTypes) {
3434
if (value instanceof StringNode s) {
3535
return s.getValue();
3636
} else if (value instanceof BooleanNode b) {
@@ -60,7 +60,7 @@ static Object convertNode(Node value, boolean allowAllTypes) {
6060
throw new RulesEvaluationError("Unsupported endpoint ruleset parameter: " + value);
6161
}
6262

63-
static Object convertNode(Node value) {
63+
public static Object convertNode(Node value) {
6464
return convertNode(value, false);
6565
}
6666

‎client/client-rulesengine/src/main/java/software/amazon/smithy/java/client/rulesengine/RulesCompiler.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ final class RulesCompiler {
9191
addRegister(param.getName().toString(), param.isRequired(), defaultValue, builtinValue);
9292
}
9393

94-
cse = performOptimizations ? CseOptimizer.apply(rules.getRules()) : Map.of();
94+
// cse = performOptimizations ? CseOptimizer.apply(rules.getRules()) : Map.of();
95+
performOptimizations = false;
96+
cse = Map.of();
9597
}
9698

9799
private byte addRegister(String name, boolean required, Object defaultValue, String builtin) {
@@ -106,7 +108,7 @@ private byte addRegister(String name, boolean required, Object defaultValue, Str
106108

107109
// Register scopes are tracking by flipping bits of a long. That means a max of 64 registers.
108110
// No real rules definition would have more than 64 registers.
109-
if (registry.size() > 64) {
111+
if (registry.size() > 255) {
110112
throw new RulesEvaluationError("Too many registers added to rules engine");
111113
}
112114

‎client/client-rulesengine/src/main/java/software/amazon/smithy/java/client/rulesengine/RulesEvaluationError.java‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,32 @@
99
* An error encountered while running the rules engine.
1010
*/
1111
public class RulesEvaluationError extends RuntimeException {
12+
13+
private final int position;
14+
1215
public RulesEvaluationError(String message) {
13-
super(message);
16+
this(message, -1);
1417
}
1518

1619
public RulesEvaluationError(String message, Throwable cause) {
17-
super(message, cause);
20+
this(message, -1, cause);
21+
}
22+
23+
public RulesEvaluationError(String message, int position) {
24+
super(createMessage(message, position));
25+
this.position = position;
26+
}
27+
28+
public RulesEvaluationError(String message, int position, Throwable cause) {
29+
super(createMessage(message, position), cause);
30+
this.position = position;
31+
}
32+
33+
private static String createMessage(String message, int position) {
34+
return message + (position == -1 ? "" : " (position " + position + ')');
35+
}
36+
37+
public int getPosition() {
38+
return position;
1839
}
1940
}

‎client/client-rulesengine/src/main/java/software/amazon/smithy/java/client/rulesengine/RulesProgram.java‎

Lines changed: 65 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,10 @@ public List<ParamDefinition> getParamDefinitions() {
264264
return result;
265265
}
266266

267+
private enum Show {
268+
CONST, FN, REGISTER
269+
}
270+
267271
@Override
268272
public String toString() {
269273
StringBuilder s = new StringBuilder();
@@ -309,28 +313,34 @@ public String toString() {
309313

310314
// Write the instructions.
311315
s.append("Instructions: (version=").append(-instructions[instructionOffset]).append(")\n");
316+
312317
// Skip version, param count, synthetic param count bytes.
313318
for (var i = instructionOffset + 3; i < instructionSize; i++) {
314319
s.append(" ");
315320
s.append(String.format("%03d", i));
316321
s.append(": ");
317322

318323
var skip = 0;
324+
Show show = null;
319325
var name = switch (instructions[i]) {
320326
case LOAD_CONST -> {
321327
skip = 1;
328+
show = Show.CONST;
322329
yield "LOAD_CONST";
323330
}
324331
case LOAD_CONST_W -> {
325332
skip = 2;
333+
show = Show.CONST;
326334
yield "LOAD_CONST_W";
327335
}
328336
case SET_REGISTER -> {
329337
skip = 1;
338+
show = Show.REGISTER;
330339
yield "SET_REGISTER";
331340
}
332341
case LOAD_REGISTER -> {
333342
skip = 1;
343+
show = Show.REGISTER;
334344
yield "LOAD_REGISTER";
335345
}
336346
case JUMP_IF_FALSEY -> {
@@ -341,6 +351,7 @@ public String toString() {
341351
case ISSET -> "ISSET";
342352
case TEST_REGISTER_ISSET -> {
343353
skip = 1;
354+
show = Show.REGISTER;
344355
yield "TEST_REGISTER_SET";
345356
}
346357
case RETURN_ERROR -> "RETURN_ERROR";
@@ -358,45 +369,54 @@ public String toString() {
358369
}
359370
case RESOLVE_TEMPLATE -> {
360371
skip = 2;
372+
show = Show.CONST;
361373
yield "RESOLVE_TEMPLATE";
362374
}
363375
case FN -> {
364376
skip = 1;
377+
show = Show.FN;
365378
yield "FN";
366379
}
367380
case GET_ATTR -> {
368381
skip = 2;
382+
show = Show.CONST;
369383
yield "GET_ATTR";
370384
}
371385
case IS_TRUE -> "IS_TRUE";
372386
case TEST_REGISTER_IS_TRUE -> {
373387
skip = 1;
388+
show = Show.REGISTER;
374389
yield "TEST_REGISTER_IS_TRUE";
375390
}
376391
case RETURN_VALUE -> "RETURN_VALUE";
377392
default -> "?" + instructions[i];
378393
};
379394

380-
switch (skip) {
381-
case 0 -> s.append(name);
382-
case 1 -> {
383-
s.append(String.format("%-22s ", name));
384-
if (instructions.length > i + 1) {
385-
s.append(instructions[i + 1]);
386-
} else {
387-
s.append("?");
395+
appendName(s, name);
396+
397+
int positionToShow = -1;
398+
if (skip == 1) {
399+
positionToShow = appendByte(s, i);
400+
i++;
401+
} else if (skip == 2) {
402+
positionToShow = appendShort(s, i);
403+
i += 2;
404+
}
405+
406+
if (positionToShow > -1 && show != null) {
407+
switch (show) {
408+
case CONST -> {
409+
s.append(" ");
410+
s.append(constantPool[positionToShow]);
388411
}
389-
i++;
390-
}
391-
default -> {
392-
// it's a two-byte unsigned short.
393-
s.append(String.format("%-22s ", name));
394-
if (instructions.length > i + 2) {
395-
s.append(EndpointUtils.bytesToShort(instructions, i + 1));
396-
} else {
397-
s.append("??");
412+
case FN -> {
413+
s.append(" ");
414+
s.append(functions[positionToShow].getFunctionName());
415+
}
416+
case REGISTER -> {
417+
s.append(" ");
418+
s.append(registerDefinitions[positionToShow].name());
398419
}
399-
i += 2;
400420
}
401421
}
402422

@@ -405,4 +425,31 @@ public String toString() {
405425

406426
return s.toString();
407427
}
428+
429+
private void appendName(StringBuilder s, String name) {
430+
s.append(String.format("%-22s ", name));
431+
}
432+
433+
private int appendByte(StringBuilder s, int i) {
434+
int result = -1;
435+
if (instructions.length > i + 1) {
436+
result = instructions[i + 1] & 0xFF;
437+
s.append(String.format("%-8d ", result));
438+
} else {
439+
s.append(" ??");
440+
}
441+
return result;
442+
}
443+
444+
private int appendShort(StringBuilder s, int i) {
445+
var result = -1;
446+
// it's a two-byte unsigned short.
447+
if (instructions.length > i + 2) {
448+
result = EndpointUtils.bytesToShort(instructions, i + 1);
449+
s.append(String.format("%-8d ", result));
450+
} else {
451+
s.append(" ??");
452+
}
453+
return result;
454+
}
408455
}

0 commit comments

Comments
 (0)