Mirroristas/CommunitySolidServer
2
0
Fork 0
mirror of https://github.com/CommunitySolidServer/CommunitySolidServer.git synced 2024-10-03 14:55:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Remove the cache from the ChainedConverter

Browse Source
This commit is contained in:
Joachim Van Herwegen 2021-07-23 11:55:46 +02:00
parent bd10256e59
commit fe8d579c72
2 changed files with 13 additions and 194 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
src/storage/conversion/ChainedConverter.ts
View File

@ -31,67 +31,6 @@ type MatchedPath = {
weight: number;
};
/**
* An LRU cache for storing `ConversionPath`s.
*/
class LruPathCache {
private readonly maxSize: number;
// Contents are ordered from least to most recently used
private readonly paths: ConversionPath[] = [];
public constructor(maxSize: number) {
this.maxSize = maxSize;
}
/**
* Add the given path to the cache as most recently used.
*/
public add(path: ConversionPath): void {
this.paths.push(path);
if (this.paths.length > this.maxSize) {
this.paths.shift();
}
}
/**
* Find a path that can convert the given type to the given preferences.
* Note that this finds the first matching path in the cache,
* not the best one, should there be multiple results.
* In practice this should almost never be the case though.
*/
public find(inType: string, outPreferences: ValuePreferences): MatchedPath | undefined {
// Last element is most recently used so has more chance of being the correct one
for (let i = this.paths.length - 1; i >= 0; --i) {
const path = this.paths[i];
// Check if `inType` matches the input and `outPreferences` the output types of the path
const match = this.getMatchedPath(inType, outPreferences, path);
if (match) {
// Set matched path to most recent result in the cache
this.paths.splice(i, 1);
this.paths.push(path);
return match;
}
}
}
/**
* Calculates the weights and exact types when using the given path on the given type and preferences.
* Undefined if there is no match
*/
private getMatchedPath(inType: string, outPreferences: ValuePreferences, path: ConversionPath):
MatchedPath | undefined {
const inWeight = getTypeWeight(inType, path.inTypes);
if (inWeight === 0) {
return;
}
const outMatch = getBestPreference(path.outTypes, outPreferences);
if (!outMatch) {
return;
}
return { path, inType, outType: outMatch.value, weight: inWeight * outMatch.weight };
}
}
/**
* A meta converter that takes an array of other converters as input.
* It chains these converters by finding a path of converters
@ -99,39 +38,36 @@ class LruPathCache {
* In case there are multiple paths, the one with the highest weight gets found.
* Will error in case no path can be found.
*
* Generated paths get stored in an internal cache for later re-use on similar requests.
* Note that due to this caching `RepresentationConverter`s
* that change supported input/output types at runtime are not supported,
* unless cache size is set to 0.
*
* This is not a TypedRepresentationConverter since the supported output types
* might depend on what is the input content-type.
*
* This converter should be the last in a WaterfallHandler if there are multiple,
* since it will try to convert any representation with a content-type.
*
* Some suggestions on how this class can be even more optimized should this ever be needed in the future.
* Most of these decrease computation time at the cost of more memory.
* - Subpaths that are generated could also be cached.
* - When looking for the next step, cached paths could also be considered.
* - The algorithm could start on both ends of a possible path and work towards the middle.
* - When creating a path, store the list of unused converters instead of checking every step.
* - Caching: https://github.com/solid/community-server/issues/832
*/
export class ChainedConverter extends RepresentationConverter {
protected readonly logger = getLoggerFor(this);
private readonly converters: TypedRepresentationConverter[];
private readonly cache: LruPathCache;
public constructor(converters: TypedRepresentationConverter[], maxCacheSize = 50) {
public constructor(converters: TypedRepresentationConverter[]) {
super();
if (converters.length === 0) {
throw new Error('At least 1 converter is required.');
}
this.converters = [ ...converters ];
this.cache = new LruPathCache(maxCacheSize);
}
public async canHandle(input: RepresentationConverterArgs): Promise<void> {
// Will cache the path if found, and error if not
await this.findPath(input);
const type = input.representation.metadata.contentType;
if (!type) {
throw new BadRequestHttpError('Missing Content-Type header.');
}
}
public async handle(input: RepresentationConverterArgs): Promise<Representation> {
@ -156,24 +92,15 @@ export class ChainedConverter extends RepresentationConverter {
return path.converters.slice(-1)[0].handle(args);
}
public async handleSafe(input: RepresentationConverterArgs): Promise<Representation> {
// This way we don't run `findPath` twice, even though it would be cached for the second call
return this.handle(input);
}
private isMatchedPath(path: unknown): path is MatchedPath {
return typeof (path as MatchedPath).path === 'object';
}
/**
* Finds a conversion path that can handle the given input,
* either in the cache or by generating a new one.
* Finds a conversion path that can handle the given input.
*/
private async findPath(input: RepresentationConverterArgs): Promise<MatchedPath | ValuePreference> {
const type = input.representation.metadata.contentType;
if (!type) {
throw new BadRequestHttpError('Missing Content-Type header.');
}
const type = input.representation.metadata.contentType!;
const preferences = cleanPreferences(input.preferences.type);
const weight = getTypeWeight(type, preferences);
@ -182,15 +109,7 @@ export class ChainedConverter extends RepresentationConverter {
return { value: type, weight };
}
// Use a cached solution if we have one.
// Note that it's possible that a better one could be generated.
// But this is usually highly unlikely.
let match = this.cache.find(type, preferences);
if (!match) {
match = await this.generatePath(type, preferences);
this.cache.add(match.path);
}
return match;
return this.generatePath(type, preferences);
}
/**

102
test/unit/storage/conversion/ChainedConverter.test.ts
View File

@ -77,7 +77,7 @@ describe('A ChainedConverter', (): void => {
const converter = new ChainedConverter(converters);
args.representation.metadata.contentType = 'b/b';
await expect(converter.canHandle(args)).rejects
await expect(converter.handle(args)).rejects
.toThrow('No conversion path could be made from b/b to x/x,x/*,internal/*.');
});
@ -218,104 +218,4 @@ describe('A ChainedConverter', (): void => {
expect(converter.handle).toHaveBeenCalledTimes(1);
expect(converter.handle).toHaveBeenLastCalledWith(args);
});
it('caches paths for re-use.', async(): Promise<void> => {
const converters = [
new DummyConverter({ 'a/a': 0.8 }, { 'b/b': 0.9 }),
new DummyConverter({ 'b/b': 0.8 }, { 'x/x': 1 }),
];
const converter = new ChainedConverter(converters);
let result = await converter.handle(args);
expect(result.metadata.contentType).toBe('x/x');
jest.spyOn(converters[0], 'getInputTypes');
jest.spyOn(converters[0], 'getOutputTypes');
result = await converter.handle(args);
expect(result.metadata.contentType).toBe('x/x');
expect(converters[0].getInputTypes).toHaveBeenCalledTimes(0);
expect(converters[0].getOutputTypes).toHaveBeenCalledTimes(0);
});
it('removes unused paths from the cache.', async(): Promise<void> => {
const converters = [
new DummyConverter({ 'a/a': 0.8 }, { 'b/b': 0.9 }),
new DummyConverter({ 'b/b': 0.8 }, { 'x/x': 1 }),
new DummyConverter({ 'c/c': 0.8 }, { 'b/b': 0.9 }),
];
// Cache size 1
const converter = new ChainedConverter(converters, 1);
let result = await converter.handle(args);
expect(result.metadata.contentType).toBe('x/x');
// Should remove previous path (which contains converter 0)
args.representation.metadata.contentType = 'c/c';
result = await converter.handle(args);
expect(result.metadata.contentType).toBe('x/x');
jest.spyOn(converters[0], 'getInputTypes');
jest.spyOn(converters[0], 'getOutputTypes');
args.representation.metadata.contentType = 'a/a';
result = await converter.handle(args);
expect(result.metadata.contentType).toBe('x/x');
expect(converters[0].getInputTypes).not.toHaveBeenCalledTimes(0);
expect(converters[0].getOutputTypes).not.toHaveBeenCalledTimes(0);
});
it('keeps the most recently used paths in the cache.', async(): Promise<void> => {
const converters = [
new DummyConverter({ 'a/a': 1 }, { 'd/d': 1 }),
new DummyConverter({ 'b/b': 1 }, { 'd/d': 1 }),
new DummyConverter({ 'c/c': 1 }, { 'd/d': 1 }),
new DummyConverter({ 'd/d': 1 }, { 'x/x': 1 }),
];
// Cache size 2
const converter = new ChainedConverter(converters, 2);
// Caches path 0
await converter.handle(args);
// Caches path 1
args.representation.metadata.contentType = 'b/b';
await converter.handle(args);
// Reset path 0 in cache
args.representation.metadata.contentType = 'a/a';
await converter.handle(args);
// Caches path 2 and removes 1
args.representation.metadata.contentType = 'c/c';
await converter.handle(args);
jest.spyOn(converters[0], 'getInputTypes');
jest.spyOn(converters[1], 'getInputTypes');
jest.spyOn(converters[2], 'getInputTypes');
// Path 0 and 2 should be cached now
args.representation.metadata.contentType = 'a/a';
await converter.handle(args);
expect(converters[0].getInputTypes).toHaveBeenCalledTimes(0);
args.representation.metadata.contentType = 'c/c';
await converter.handle(args);
expect(converters[2].getInputTypes).toHaveBeenCalledTimes(0);
args.representation.metadata.contentType = 'b/b';
await converter.handle(args);
expect(converters[1].getInputTypes).not.toHaveBeenCalledTimes(0);
});
it('does not use cached paths that match content-type but not preferences.', async(): Promise<void> => {
const converters = [
new DummyConverter({ 'a/a': 1 }, { 'b/b': 1 }),
new DummyConverter({ 'b/b': 1 }, { 'x/x': 1 }),
new DummyConverter({ 'a/a': 1 }, { 'c/c': 1 }),
new DummyConverter({ 'c/c': 1 }, { 'y/y': 1 }),
];
const converter = new ChainedConverter(converters);
// Cache a-b-x path
await converter.handle(args);
// Generate new a-c-y path
args.preferences.type = { 'y/y': 1 };
const result = await converter.handle(args);
expect(result.metadata.contentType).toBe('y/y');
});
});