From f97331fbda15c184e2f8c889924ba20cc02ffcb1 Mon Sep 17 00:00:00 2001 From: George Fu Date: Mon, 2 Sep 2024 19:14:27 +0000 Subject: [PATCH] feat(util-endpoint): add endpoint ruleset cache --- .changeset/moody-fishes-buy.md | 5 + .../src/cache/EndpointCache.spec.ts | 53 ++++++++ .../util-endpoints/src/cache/EndpointCache.ts | 62 ++++++++++ packages/util-endpoints/src/index.ts | 1 + scripts/build-generated-test-packages.js | 44 +++++-- .../endpointsV2/EndpointsV2Generator.java | 52 ++++---- .../endpointsV2/RuleSetParameterFinder.java | 101 ++++++++++++++++ .../RuleSetParameterFinderTest.java | 114 ++++++++++++++++++ 8 files changed, 398 insertions(+), 34 deletions(-) create mode 100644 .changeset/moody-fishes-buy.md create mode 100644 packages/util-endpoints/src/cache/EndpointCache.spec.ts create mode 100644 packages/util-endpoints/src/cache/EndpointCache.ts create mode 100644 smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinderTest.java diff --git a/.changeset/moody-fishes-buy.md b/.changeset/moody-fishes-buy.md new file mode 100644 index 00000000000..8b03b8cf2c9 --- /dev/null +++ b/.changeset/moody-fishes-buy.md @@ -0,0 +1,5 @@ +--- +"@smithy/util-endpoints": minor +--- + +add endpoint ruleset cache diff --git a/packages/util-endpoints/src/cache/EndpointCache.spec.ts b/packages/util-endpoints/src/cache/EndpointCache.spec.ts new file mode 100644 index 00000000000..e8adef282d6 --- /dev/null +++ b/packages/util-endpoints/src/cache/EndpointCache.spec.ts @@ -0,0 +1,53 @@ +import { EndpointCache } from "./EndpointCache"; + +describe(EndpointCache.name, () => { + const endpoint1: any = {}; + const endpoint2: any = {}; + + it("should store and retrieve items", () => { + const cache = new EndpointCache({ + size: 50, + }); + + expect(cache.get({ A: "b", B: "b" }, () => endpoint1)).toBe(endpoint1); + expect(cache.get({ A: "b", B: "b" }, () => endpoint1)).toBe(endpoint1); + expect(cache.get({ A: "b", B: "b", C: "c" }, () => endpoint1)).toBe(endpoint1); + expect(cache.get({ A: "b", B: "b", C: "c" }, () => endpoint1)).toBe(endpoint1); + expect(cache.get({ A: "b", B: "b", C: "cc" }, () => endpoint2)).toBe(endpoint2); + expect(cache.get({ A: "b", B: "b", C: "cc" }, () => endpoint2)).toBe(endpoint2); + + expect(cache.size()).toEqual(3); + }); + + it("should accept a custom parameter list", () => { + const cache = new EndpointCache({ + size: 50, + params: ["A", "B"], + }); + + expect(cache.get({ A: "b", B: "b" }, () => endpoint1)).toBe(endpoint1); + expect(cache.get({ A: "b", B: "b", C: "c" }, () => endpoint1)).toBe(endpoint1); + expect(cache.get({ A: "b", B: "b", C: "cc" }, () => endpoint2)).toBe(endpoint1); + + expect(cache.size()).toEqual(1); + }); + + it("should be an LRU cache", () => { + const cache = new EndpointCache({ + size: 5, + params: ["A", "B"], + }); + + for (let i = 0; i < 50; ++i) { + cache.get({ A: "b", B: "b" + i }, () => endpoint1); + } + + const size = cache.size(); + expect(size).toBeLessThan(16); + expect(cache.get({ A: "b", B: "b49" }, () => endpoint2)).toBe(endpoint1); + expect(cache.size()).toEqual(size); + + expect(cache.get({ A: "b", B: "b1" }, () => endpoint2)).toBe(endpoint2); + expect(cache.size()).toEqual(size + 1); + }); +}); diff --git a/packages/util-endpoints/src/cache/EndpointCache.ts b/packages/util-endpoints/src/cache/EndpointCache.ts new file mode 100644 index 00000000000..9eefb8289cc --- /dev/null +++ b/packages/util-endpoints/src/cache/EndpointCache.ts @@ -0,0 +1,62 @@ +import type { EndpointParams, EndpointV2 } from "@smithy/types"; + +/** + * @internal + * + * Cache for endpoint ruleSet resolution. + */ +export class EndpointCache { + private capacity: number = 50; + private data = new Map(); + private parameters: string[] = []; + + /** + * @param [params] - list of params to consider as part of the cache key. + * + * If the params list is not populated, all object keys will be considered. + * This may be out of order depending on how the object is created and arrives to this class. + */ + public constructor({ size, params }: { size?: number; params?: string[] }) { + this.capacity = size ?? 50; + if (params) { + this.parameters = params; + } + } + + /** + * @param endpointParams - query for endpoint. + * @param resolver - provider of the value if not present. + * @returns endpoint corresponding to the query. + */ + public get(endpointParams: EndpointParams, resolver: () => EndpointV2): EndpointV2 { + const key = this.hash(endpointParams); + if (!this.data.has(key)) { + if (this.data.size > this.capacity + 10) { + const keys = this.data.keys(); + let i = 0; + while (true) { + const { value, done } = keys.next(); + this.data.delete(value); + if (done || ++i > 10) { + break; + } + } + } + this.data.set(key, resolver()); + } + return this.data.get(key)!; + } + + public size() { + return this.data.size; + } + + private hash(endpointParams: EndpointParams): string { + let buffer = ""; + const params = this.parameters.length ? this.parameters : Object.keys(endpointParams); + for (const param of params) { + buffer += endpointParams[param] ?? "" + "|"; + } + return buffer; + } +} diff --git a/packages/util-endpoints/src/index.ts b/packages/util-endpoints/src/index.ts index 07c6fd3874e..c39ed2b760d 100644 --- a/packages/util-endpoints/src/index.ts +++ b/packages/util-endpoints/src/index.ts @@ -1,3 +1,4 @@ +export * from "./cache/EndpointCache"; export * from "./lib/isIpAddress"; export * from "./lib/isValidHostLabel"; export * from "./utils/customEndpointFunctions"; diff --git a/scripts/build-generated-test-packages.js b/scripts/build-generated-test-packages.js index 4797a7aa884..37230a0a2ec 100644 --- a/scripts/build-generated-test-packages.js +++ b/scripts/build-generated-test-packages.js @@ -5,6 +5,8 @@ */ const path = require("node:path"); +const fs = require("node:fs"); + const { spawnProcess } = require("./utils/spawn-process"); const root = path.join(__dirname, ".."); @@ -15,16 +17,6 @@ const codegenTestDir = path.join(testProjectDir, "build", "smithyprojections", " const weatherClientDir = path.join(codegenTestDir, "source", "typescript-client-codegen"); -const releasedClientDir = path.join( - testProjectDir, - "released-version-test", - "build", - "smithyprojections", - "released-version-test", - "source", - "typescript-codegen" -); - // Build generic legacy auth client for integration tests const weatherLegacyAuthClientDir = path.join(codegenTestDir, "client-legacy-auth", "typescript-client-codegen"); @@ -54,7 +46,27 @@ const buildAndCopyToNodeModules = async (packageName, codegenDir, nodeModulesDir // as its own package. await spawnProcess("touch", ["yarn.lock"], { cwd: codegenDir }); await spawnProcess("yarn", { cwd: codegenDir }); + const smithyPackages = path.join(__dirname, "..", "packages"); + const node_modules = path.join(codegenDir, "node_modules"); + const localSmithyPkgs = fs.readdirSync(smithyPackages); + + for (const smithyPkg of localSmithyPkgs) { + if (!fs.existsSync(path.join(smithyPackages, smithyPkg, "dist-cjs"))) { + continue; + } + await Promise.all( + ["dist-cjs", "dist-types", "dist-es", "package.json"].map((folder) => + spawnProcess("cp", [ + "-r", + path.join(smithyPackages, smithyPkg, folder), + path.join(node_modules, "@smithy", smithyPkg), + ]) + ) + ); + } + await spawnProcess("yarn", ["build"], { cwd: codegenDir }); + // Optionally, after building the package, it's packed and copied to node_modules so that // it can be used in integration tests by other packages within the monorepo. if (nodeModulesDir != undefined) { @@ -89,6 +101,18 @@ const buildAndCopyToNodeModules = async (packageName, codegenDir, nodeModulesDir httpBearerAuthClientDir, nodeModulesDir ); + // TODO(released-version-test): Test released version of smithy-typescript codegenerators, but currently is not working + /* + const releasedClientDir = path.join( + testProjectDir, + "released-version-test", + "build", + "smithyprojections", + "released-version-test", + "source", + "typescript-codegen" + ); + */ // await buildAndCopyToNodeModules("released", releasedClientDir, undefined); })(); diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java index 9db0a0529bc..81cde408eff 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/EndpointsV2Generator.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import software.amazon.smithy.codegen.core.SymbolDependency; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.ObjectNode; @@ -79,6 +80,7 @@ public List getDependencies() { private final EndpointRuleSetTrait endpointRuleSetTrait; private final ServiceShape service; private final TypeScriptSettings settings; + private final RuleSetParameterFinder ruleSetParameterFinder; public EndpointsV2Generator( TypeScriptDelegator delegator, @@ -90,6 +92,7 @@ public EndpointsV2Generator( this.settings = settings; endpointRuleSetTrait = service.getTrait(EndpointRuleSetTrait.class) .orElseThrow(() -> new RuntimeException("service missing EndpointRuleSetTrait")); + ruleSetParameterFinder = new RuleSetParameterFinder(service); } @Override @@ -114,8 +117,6 @@ private void generateEndpointParameters() { "export interface ClientInputEndpointParameters {", "}", () -> { - RuleSetParameterFinder ruleSetParameterFinder = new RuleSetParameterFinder(service); - Map clientInputParams = ruleSetParameterFinder.getClientContextParams(); //Omit Endpoint params that should not be a part of the ClientInputEndpointParameters interface Map builtInParams = ruleSetParameterFinder.getBuiltInParams(); @@ -164,10 +165,9 @@ private void generateEndpointParameters() { writer.openBlock( "export const commonParams = {", "} as const", () -> { - RuleSetParameterFinder parameterFinder = new RuleSetParameterFinder(service); Set paramNames = new HashSet<>(); - parameterFinder.getClientContextParams().forEach((name, type) -> { + ruleSetParameterFinder.getClientContextParams().forEach((name, type) -> { if (!paramNames.contains(name)) { writer.write( "$L: { type: \"clientContextParams\", name: \"$L\" },", @@ -176,7 +176,7 @@ private void generateEndpointParameters() { paramNames.add(name); }); - parameterFinder.getBuiltInParams().forEach((name, type) -> { + ruleSetParameterFinder.getBuiltInParams().forEach((name, type) -> { if (!paramNames.contains(name)) { writer.write( "$L: { type: \"builtInParams\", name: \"$L\" },", @@ -222,25 +222,29 @@ private void generateEndpointResolver() { Paths.get(".", CodegenUtils.SOURCE_FOLDER, ENDPOINT_FOLDER, ENDPOINT_RULESET_FILE.replace(".ts", ""))); - writer.openBlock( - "export const defaultEndpointResolver = ", - "", - () -> { - writer.openBlock( - "(endpointParams: EndpointParameters, context: { logger?: Logger } = {}): EndpointV2 => {", - "};", - () -> { - writer.openBlock( - "return resolveEndpoint(ruleSet, {", - "});", - () -> { - writer.write("endpointParams: endpointParams as EndpointParams,"); - writer.write("logger: context.logger,"); - } - ); - } - ); - } + writer.addImport("EndpointCache", null, TypeScriptDependency.UTIL_ENDPOINTS); + writer.write(""" + const cache = new EndpointCache({ + size: 50, + params: [$L] + }); + """, + ruleSetParameterFinder.getEffectiveParams() + .stream().collect(Collectors.joining("\",\n \"", "\"", "\"")) + ); + + writer.write( + """ + export const defaultEndpointResolver = ( + endpointParams: EndpointParameters, + context: { logger?: Logger } = {} + ): EndpointV2 => { + return cache.get(endpointParams as EndpointParams, () => resolveEndpoint(ruleSet, { + endpointParams: endpointParams as EndpointParams, + logger: context.logger, + })); + }; + """ ); } ); diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java index 44831958b97..5b8d76d3467 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinder.java @@ -15,9 +15,17 @@ package software.amazon.smithy.typescript.codegen.endpointsV2; +import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Queue; +import java.util.Set; +import java.util.TreeSet; +import java.util.regex.Pattern; import java.util.stream.Collectors; import software.amazon.smithy.model.node.ArrayNode; import software.amazon.smithy.model.node.Node; @@ -29,6 +37,14 @@ import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeType; +import software.amazon.smithy.rulesengine.language.Endpoint; +import software.amazon.smithy.rulesengine.language.EndpointRuleSet; +import software.amazon.smithy.rulesengine.language.syntax.expressions.Expression; +import software.amazon.smithy.rulesengine.language.syntax.rule.Condition; +import software.amazon.smithy.rulesengine.language.syntax.rule.EndpointRule; +import software.amazon.smithy.rulesengine.language.syntax.rule.ErrorRule; +import software.amazon.smithy.rulesengine.language.syntax.rule.Rule; +import software.amazon.smithy.rulesengine.language.syntax.rule.TreeRule; import software.amazon.smithy.rulesengine.traits.ClientContextParamsTrait; import software.amazon.smithy.rulesengine.traits.ContextParamTrait; import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait; @@ -38,6 +54,9 @@ @SmithyInternalApi public class RuleSetParameterFinder { + + public static final Pattern URL_PARAMETERS = Pattern.compile("\\{(\\w+)[}#]"); + private final ServiceShape service; private final EndpointRuleSetTrait ruleset; @@ -48,6 +67,88 @@ public RuleSetParameterFinder(ServiceShape service) { ); } + /** + * It's possible for a parameter to pass validation, i.e. exist in the modeled shapes + * and be used in endpoint tests, but have no actual effect on endpoint resolution. + * + * @return the list of endpoint parameters that are actually used in endpoint resolution. + */ + public List getEffectiveParams() { + Set effectiveParams = new TreeSet<>(); + EndpointRuleSet endpointRuleSet = ruleset.getEndpointRuleSet(); + Set initialParams = new HashSet<>(); + + endpointRuleSet.getParameters().forEach(parameter -> { + initialParams.add(parameter.getName().getName().getValue()); + }); + + Queue ruleQueue = new LinkedList<>(endpointRuleSet.getRules()); + Queue conditionQueue = new LinkedList<>(); + Queue argQueue = new LinkedList<>(); + + while (!ruleQueue.isEmpty() || !conditionQueue.isEmpty() || !argQueue.isEmpty()) { + while (!argQueue.isEmpty()) { + ObjectNode arg = argQueue.poll(); + Optional ref = arg.getMember("ref"); + if (ref.isPresent()) { + String refName = ref.get().expectStringNode().getValue(); + if (initialParams.contains(refName)) { + effectiveParams.add(refName); + } + } + Optional argv = arg.getMember("argv"); + if (argv.isPresent()) { + ArrayNode nestedArgv = argv.get().expectArrayNode(); + for (Node nestedArg : nestedArgv) { + if (nestedArg.isObjectNode()) { + argQueue.add(nestedArg.expectObjectNode()); + } + } + } + } + + while (!conditionQueue.isEmpty()) { + Condition condition = conditionQueue.poll(); + ArrayNode argv = condition.toNode() + .expectObjectNode() + .expectArrayMember("argv"); + for (Node arg : argv) { + if (arg.isObjectNode()) { + argQueue.add(arg.expectObjectNode()); + } + } + } + + Rule rule = ruleQueue.poll(); + if (null == rule) { + continue; + } + List conditions = rule.getConditions(); + conditionQueue.addAll(conditions); + if (rule instanceof TreeRule treeRule) { + ruleQueue.addAll(treeRule.getRules()); + } else if (rule instanceof EndpointRule endpointRule) { + Endpoint endpoint = endpointRule.getEndpoint(); + Expression url = endpoint.getUrl(); + String urlString = url.toString(); + + URL_PARAMETERS + .matcher(urlString) + .results().forEach(matchResult -> { + if (matchResult.groupCount() >= 1) { + if (initialParams.contains(matchResult.group(1))) { + effectiveParams.add(matchResult.group(1)); + } + } + }); + } else if (rule instanceof ErrorRule errorRule) { + // no additional use of endpoint parameters in error rules. + } + } + + return new ArrayList<>(effectiveParams); + } + /** * TODO(endpointsv2) From definitions in EndpointRuleSet.parameters, or * TODO(endpointsv2) are they from the closed set? diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinderTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinderTest.java new file mode 100644 index 00000000000..9a7c4529080 --- /dev/null +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/endpointsV2/RuleSetParameterFinderTest.java @@ -0,0 +1,114 @@ +package software.amazon.smithy.typescript.codegen.endpointsV2; + +import java.util.List; +import java.util.Optional; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.rulesengine.language.EndpointRuleSet; +import software.amazon.smithy.rulesengine.traits.EndpointRuleSetTrait; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class RuleSetParameterFinderTest { + + Node ruleSet = Node.parse(""" + { + "version": "1.0", + "parameters": { + "BasicParameter": { + "required": false, + "documentation": "...", + "type": "String" + }, + "NestedParameter": { + "required": true, + "documentation": "...", + "type": "Boolean" + }, + "UrlOnlyParameter": { + "required": true, + "documentation": "...", + "type": "String" + }, + "UnusedParameter": { + "required": false, + "documentation": "...", + "type": "String" + } + }, + "rules": [ + { + "conditions": [ + { + "fn": "isSet", + "argv": [ + { + "ref": "BasicParameter" + } + ] + } + ], + "rules": [ + { + "conditions": [ + { + "fn": "booleanEquals", + "argv": [ + { + "fn": "booleanEquals", + "argv": [ + { + "fn": "booleanEquals", + "argv": [ + { + "fn": "booleanEquals", + "argv": [ + { + "ref": "NestedParameter" + }, + true + ] + }, + true + ] + }, + true + ] + }, + true + ] + } + ], + "endpoint": { + "url": "https://www.{BasicParameter}.{UrlOnlyParameter}.com", + "properties": {}, + "headers": {} + }, + "type": "endpoint" + } + ], + "type": "tree" + } + ] + } + """); + + @Test + void getEffectiveParams(@Mock ServiceShape serviceShape, @Mock EndpointRuleSetTrait endpointRuleSetTrait) { + EndpointRuleSet endpointRuleSet = EndpointRuleSet.fromNode(ruleSet); + when(serviceShape.getTrait(EndpointRuleSetTrait.class)).thenReturn(Optional.of(endpointRuleSetTrait)); + when(endpointRuleSetTrait.getEndpointRuleSet()).thenReturn(endpointRuleSet); + + RuleSetParameterFinder subject = new RuleSetParameterFinder(serviceShape); + + List effectiveParams = subject.getEffectiveParams(); + + assertEquals(List.of("BasicParameter", "NestedParameter", "UrlOnlyParameter"), effectiveParams); + } +} \ No newline at end of file