Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom cache object to be passed to client config #1036

Merged
merged 8 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions packages/js/client/src/PolywrapClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
PluginPackage,
RunOptions,
GetManifestOptions,
SimpleCache,
} from "@polywrap/core-js";
import { msgpackEncode, msgpackDecode } from "@polywrap/msgpack-js";
import { WrapManifest } from "@polywrap/wrap-manifest-types-js";
Expand All @@ -56,11 +57,7 @@ export interface PolywrapClientConfig<TUri extends Uri | string = string>
}

export class PolywrapClient implements Client {
// TODO: the Wrapper cache needs to be more like a routing table.
// It should help us keep track of what URI's map to what Wrappers,
// and handle cases where the are multiple jumps. For example, if
// A => B => C, then the cache should have A => C, and B => C.
private _wrapperCache: WrapperCache = new Map<string, Wrapper>();
private _wrapperCache: WrapperCache;
private _config: PolywrapClientConfig<Uri> = {
redirects: [],
plugins: [],
Expand Down Expand Up @@ -97,6 +94,14 @@ export class PolywrapClient implements Client {
uriResolvers: config.uriResolvers ?? [],
tracingEnabled: !!config.tracingEnabled,
};

if (config.wrapperCache) {
this._wrapperCache = config.wrapperCache;
}
}

if (!this._wrapperCache) {
this._wrapperCache = new SimpleCache();
}

if (!options?.noDefaults) {
Expand Down Expand Up @@ -486,9 +491,8 @@ export class PolywrapClient implements Client {

// Update cache for all URIs in the chain
if (cacheWrite && wrapper) {
for (const item of uriHistory.getResolutionPath().stack) {
this._wrapperCache.set(item.sourceUri.uri, wrapper);
}
const uris = uriHistory.getResolutionPath().stack.map((x) => x.sourceUri);
this._wrapperCache.set(uris, wrapper);
}

if (shouldClearContext) {
Expand Down
1 change: 1 addition & 0 deletions packages/js/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export * from "./PolywrapClient";
export * from "./createPolywrapClient";
export * from "./default-client-config";
export * from "./wasm";
export * from "./plugin";
export * from "@polywrap/core-js";
1 change: 1 addition & 0 deletions packages/js/client/src/plugin/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./PluginWrapper";
20 changes: 11 additions & 9 deletions packages/js/core/src/__tests__/resolveUri.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import {
SubscribeOptions,
Subscription,
PluginPackage,
SimpleCache,
GetManifestOptions,
} from "..";

import { WrapManifest } from "@polywrap/wrap-manifest-types-js";
Expand Down Expand Up @@ -106,7 +108,7 @@ describe("resolveUri", () => {
encoded: false
}),
getFile: (options: GetFileOptions, client: Client) => Promise.resolve(""),
getManifest: (client: Client) => Promise.resolve({} as WrapManifest)
getManifest: (options: GetManifestOptions, client: Client) => Promise.resolve({} as WrapManifest)
};
};

Expand Down Expand Up @@ -238,7 +240,7 @@ describe("resolveUri", () => {
new Uri("ens/test.eth"),
uriResolvers,
client(wrappers, plugins, interfaces),
new Map<string, Wrapper>(),
new SimpleCache(),
);

expect(result.wrapper).toBeTruthy();
Expand All @@ -260,7 +262,7 @@ describe("resolveUri", () => {
new Uri("my/something-different"),
uriResolvers,
client(wrappers, plugins, interfaces),
new Map<string, Wrapper>(),
new SimpleCache(),
);

expect(result.wrapper).toBeTruthy();
Expand All @@ -282,7 +284,7 @@ describe("resolveUri", () => {
new Uri("ens/ens"),
uriResolvers,
client(wrappers, plugins, interfaces),
new Map<string, Wrapper>(),
new SimpleCache(),
);

expect(result.wrapper).toBeTruthy();
Expand All @@ -304,7 +306,7 @@ describe("resolveUri", () => {
new Uri("my/something-different"),
uriResolvers,
client(wrappers, plugins, interfaces),
new Map<string, Wrapper>(),
new SimpleCache(),
);

expect(result.wrapper).toBeTruthy();
Expand Down Expand Up @@ -339,7 +341,7 @@ describe("resolveUri", () => {
new Uri("some/wrapper"),
uriResolvers,
client(wrappers, plugins, interfaces, circular),
new Map<string, Wrapper>(),
new SimpleCache(),
).catch((e: Error) =>
expect(e.message).toMatch(/Infinite loop while resolving URI/)
);
Expand All @@ -363,7 +365,7 @@ describe("resolveUri", () => {
new Uri("some/wrapper"),
uriResolvers,
client(wrappers, plugins, interfaces, missingFromProperty),
new Map<string, Wrapper>(),
new SimpleCache(),
).catch((e: Error) =>
expect(e.message).toMatch(
"Redirect missing the from property.\nEncountered while resolving wrap://some/wrapper"
Expand All @@ -387,7 +389,7 @@ describe("resolveUri", () => {
new Uri("some/wrapper"),
uriResolvers,
client(wrappers, pluginRegistrations, interfaces),
new Map<string, Wrapper>(),
new SimpleCache(),
);

expect(result.wrapper).toBeTruthy();
Expand Down Expand Up @@ -425,7 +427,7 @@ describe("resolveUri", () => {
plugins,
interfaces
),
new Map<string, Wrapper>(),
new SimpleCache(),
);

expect(resolvedUri).toEqual(uri);
Expand Down
23 changes: 23 additions & 0 deletions packages/js/core/src/cache/SimpleWrapperCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Uri, Wrapper, WrapperCache } from "../types";

export class SimpleCache implements WrapperCache {
private _map: Map<string, Wrapper> = new Map();

get(uri: Uri): Wrapper | undefined {
return this._map.get(uri.uri);
}

has(uri: Uri): boolean {
return this._map.has(uri.uri);
}

set(uris: Uri | Uri[], wrapper: Wrapper): void {
if (Array.isArray(uris)) {
for (const uri of uris) {
this._map.set(uri.uri, wrapper);
}
} else {
this._map.set(uris.uri, wrapper);
}
}
}
1 change: 1 addition & 0 deletions packages/js/core/src/cache/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./SimpleWrapperCache";
1 change: 1 addition & 0 deletions packages/js/core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from "./types";
export * from "./algorithms";
export * from "./cache";
export * from "./interfaces";
export * from "./uri-resolution/core";
export * from "./uri-resolution/resolvers";
Expand Down
2 changes: 2 additions & 0 deletions packages/js/core/src/types/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from "./";
import { UriResolver } from "../uri-resolution/core";
import { UriResolverHandler } from "./UriResolver";
import { WrapperCache } from "./WrapperCache";

import { WrapManifest } from "@polywrap/wrap-manifest-types-js";

Expand All @@ -20,6 +21,7 @@ export interface ClientConfig<TUri extends Uri | string = string> {
interfaces: InterfaceImplementations<TUri>[];
envs: Env<TUri>[];
uriResolvers: UriResolver[];
wrapperCache?: WrapperCache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on naming this "cache"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything LGTM!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the explicit nature of "wrapper cache", as there may be other things we want to cache in the future.

}

export interface Contextualized {
Expand Down
2 changes: 0 additions & 2 deletions packages/js/core/src/types/Wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,3 @@ export abstract class Wrapper implements Invocable {
client: Client
): Promise<WrapManifest>;
}

export type WrapperCache = Map<string, Wrapper>;
8 changes: 8 additions & 0 deletions packages/js/core/src/types/WrapperCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { Uri, Wrapper } from ".";

export interface WrapperCache {
get(uri: Uri): Wrapper | undefined;
has(uri: Uri): boolean;
set(uris: Uri[], wrapper: Wrapper): void;
set(uri: Uri, wrapper: Wrapper): void;
}
1 change: 1 addition & 0 deletions packages/js/core/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ export * from "./InterfaceImplementations";
export * from "./PluginRegistration";
export * from "./UriResolver";
export * from "./Workflow";
export * from "./WrapperCache";
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class CacheResolver implements UriResolver {
client: Client,
cache: WrapperCache
): Promise<UriResolutionResult> {
const wrapper = cache.get(uri.uri);
const wrapper = cache.get(uri);

return Promise.resolve({
uri: uri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class ExtendableUriResolver implements UriResolver {
const implementationsToLoad = new Queue<Uri>();

for (const implementationUri of implementationUris) {
if (!cache.has(implementationUri.uri)) {
if (!cache.has(implementationUri)) {
implementationsToLoad.enqueue(implementationUri);
}
}
Expand Down Expand Up @@ -148,7 +148,7 @@ export class ExtendableUriResolver implements UriResolver {
};
}
} else {
cache.set(implementationUri.uri, wrapper);
cache.set(implementationUri, wrapper);
loadedImplementations.push(implementationUri.uri);
failedAttempts = 0;
}
Expand Down