mirror of
https://github.com/CommunitySolidServer/CommunitySolidServer.git
synced 2024-10-03 14:55:10 +00:00
refactor: Rename resourceExists to hasResource
The function was also moved to the smaller interface ResourceSet.
This commit is contained in:
parent
2ae5924dde
commit
4404fa07d9
@ -21,6 +21,8 @@ The following changes are relevant for v3 custom configs that replaced certain f
|
||||
These changes are relevant if you wrote custom modules for the server that depend on existing interfaces.
|
||||
- The output of `parseContentType` in `HeaderUtil` was changed to include parameters.
|
||||
- `PermissionReader`s take an additional `modes` parameter as input.
|
||||
- The `ResourceStore` function `resourceExists` has been renamed to `hasResource`
|
||||
and has been moved to a separate `ResourceSet` interface.
|
||||
|
||||
## v3.0.0
|
||||
### New features
|
||||
|
@ -42,7 +42,7 @@ export class PatchOperationHandler extends OperationHandler {
|
||||
// RFC7231, §4.3.4: If the target resource does not have a current representation and the
|
||||
// PUT successfully creates one, then the origin server MUST inform the
|
||||
// user agent by sending a 201 (Created) response.
|
||||
const exists = await this.store.resourceExists(operation.target, operation.conditions);
|
||||
const exists = await this.store.hasResource(operation.target);
|
||||
await this.store.modifyResource(operation.target, operation.body as Patch, operation.conditions);
|
||||
if (exists) {
|
||||
return new ResetResponseDescription();
|
||||
|
@ -38,7 +38,7 @@ export class PutOperationHandler extends OperationHandler {
|
||||
}
|
||||
// A more efficient approach would be to have the server return metadata indicating if a resource was new
|
||||
// See https://github.com/solid/community-server/issues/632
|
||||
const exists = await this.store.resourceExists(operation.target, operation.conditions);
|
||||
const exists = await this.store.hasResource(operation.target);
|
||||
await this.store.setRepresentation(operation.target, operation.body, operation.conditions);
|
||||
if (exists) {
|
||||
return new ResetResponseDescription();
|
||||
|
@ -358,6 +358,7 @@ export * from './storage/PassthroughStore';
|
||||
export * from './storage/PatchingStore';
|
||||
export * from './storage/ReadOnlyStore';
|
||||
export * from './storage/RepresentationConvertingStore';
|
||||
export * from './storage/ResourceSet';
|
||||
export * from './storage/ResourceStore';
|
||||
export * from './storage/RoutingResourceStore';
|
||||
|
||||
|
@ -28,7 +28,7 @@ export class GeneratedPodManager implements PodManager {
|
||||
*/
|
||||
public async createPod(identifier: ResourceIdentifier, settings: PodSettings, overwrite: boolean): Promise<void> {
|
||||
this.logger.info(`Creating pod ${identifier.path}`);
|
||||
if (!overwrite && await this.store.resourceExists(identifier)) {
|
||||
if (!overwrite && await this.store.hasResource(identifier)) {
|
||||
throw new ConflictHttpError(`There already is a resource at ${identifier.path}`);
|
||||
}
|
||||
|
||||
|
@ -11,7 +11,7 @@ import type { ResourceStore } from './ResourceStore';
|
||||
*/
|
||||
/* eslint-disable @typescript-eslint/no-unused-vars */
|
||||
export class BaseResourceStore implements ResourceStore {
|
||||
public async resourceExists(identifier: ResourceIdentifier, conditions?: Conditions): Promise<boolean> {
|
||||
public async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
throw new NotImplementedHttpError();
|
||||
}
|
||||
|
||||
|
@ -81,7 +81,7 @@ export class DataAccessorBasedStore implements ResourceStore {
|
||||
this.auxiliaryStrategy = auxiliaryStrategy;
|
||||
}
|
||||
|
||||
public async resourceExists(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
public async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
try {
|
||||
this.validateIdentifier(identifier);
|
||||
await this.accessor.getMetadata(identifier);
|
||||
@ -530,7 +530,7 @@ export class DataAccessorBasedStore implements ResourceStore {
|
||||
// Make sure we don't already have a resource with this exact name (or with differing trailing slash)
|
||||
const withSlash = { path: ensureTrailingSlash(newID.path) };
|
||||
const withoutSlash = { path: trimTrailingSlashes(newID.path) };
|
||||
if (await this.resourceExists(withSlash) || await this.resourceExists(withoutSlash)) {
|
||||
if (await this.hasResource(withSlash) || await this.hasResource(withoutSlash)) {
|
||||
newID = this.createURI(container, isContainer);
|
||||
}
|
||||
|
||||
|
@ -34,9 +34,9 @@ export class LockingResourceStore implements AtomicResourceStore {
|
||||
this.auxiliaryStrategy = auxiliaryStrategy;
|
||||
}
|
||||
|
||||
public async resourceExists(identifier: ResourceIdentifier, conditions?: Conditions): Promise<boolean> {
|
||||
public async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
return this.locks.withReadLock(this.getLockIdentifier(identifier),
|
||||
async(): Promise<boolean> => this.source.resourceExists(identifier, conditions));
|
||||
async(): Promise<boolean> => this.source.hasResource(identifier));
|
||||
}
|
||||
|
||||
public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
|
||||
|
@ -19,8 +19,8 @@ export class MonitoringStore<T extends ResourceStore = ResourceStore>
|
||||
this.source = source;
|
||||
}
|
||||
|
||||
public async resourceExists(identifier: ResourceIdentifier, conditions?: Conditions): Promise<boolean> {
|
||||
return this.source.resourceExists(identifier, conditions);
|
||||
public async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
return this.source.hasResource(identifier);
|
||||
}
|
||||
|
||||
public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
|
||||
|
@ -17,8 +17,8 @@ export class PassthroughStore<T extends ResourceStore = ResourceStore> implement
|
||||
this.source = source;
|
||||
}
|
||||
|
||||
public async resourceExists(identifier: ResourceIdentifier, conditions?: Conditions): Promise<boolean> {
|
||||
return this.source.resourceExists(identifier, conditions);
|
||||
public async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
return this.source.hasResource(identifier);
|
||||
}
|
||||
|
||||
public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
|
||||
|
14
src/storage/ResourceSet.ts
Normal file
14
src/storage/ResourceSet.ts
Normal file
@ -0,0 +1,14 @@
|
||||
import type { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
|
||||
|
||||
/**
|
||||
* A set containing resources.
|
||||
*/
|
||||
export interface ResourceSet {
|
||||
/**
|
||||
* Check if a resource exists in this ResourceSet.
|
||||
* @param identifier - Identifier of resource to check.
|
||||
*
|
||||
* @returns A promise resolving if the resource already exists.
|
||||
*/
|
||||
hasResource: (identifier: ResourceIdentifier) => Promise<boolean>;
|
||||
}
|
@ -3,6 +3,7 @@ import type { Representation } from '../http/representation/Representation';
|
||||
import type { RepresentationPreferences } from '../http/representation/RepresentationPreferences';
|
||||
import type { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
|
||||
import type { Conditions } from './Conditions';
|
||||
import type { ResourceSet } from './ResourceSet';
|
||||
|
||||
/**
|
||||
* A ResourceStore represents a collection of resources.
|
||||
@ -15,16 +16,7 @@ import type { Conditions } from './Conditions';
|
||||
* ResourceStores are also responsible for taking auxiliary resources into account
|
||||
* should those be relevant to the store.
|
||||
*/
|
||||
export interface ResourceStore {
|
||||
|
||||
/**
|
||||
* Check if a resource exists.
|
||||
* @param identifier - Identifier of resource to check.
|
||||
*
|
||||
* @returns A promise resolving if the resource already exists
|
||||
*/
|
||||
resourceExists: (identifier: ResourceIdentifier, conditions?: Conditions) => Promise<boolean>;
|
||||
|
||||
export interface ResourceStore extends ResourceSet {
|
||||
/**
|
||||
* Retrieves a representation of a resource.
|
||||
* @param identifier - Identifier of the resource to read.
|
||||
|
@ -20,9 +20,9 @@ export class RoutingResourceStore implements ResourceStore {
|
||||
this.rule = rule;
|
||||
}
|
||||
|
||||
public async resourceExists(identifier: ResourceIdentifier, conditions?: Conditions):
|
||||
public async hasResource(identifier: ResourceIdentifier):
|
||||
Promise<boolean> {
|
||||
return (await this.getStore(identifier)).resourceExists(identifier, conditions);
|
||||
return (await this.getStore(identifier)).hasResource(identifier);
|
||||
}
|
||||
|
||||
public async getRepresentation(identifier: ResourceIdentifier, preferences: RepresentationPreferences,
|
||||
|
@ -42,7 +42,7 @@ export class JsonResourceStorage implements KeyValueStorage<string, unknown> {
|
||||
|
||||
public async has(key: string): Promise<boolean> {
|
||||
const identifier = this.createIdentifier(key);
|
||||
return await this.source.resourceExists(identifier);
|
||||
return await this.source.hasResource(identifier);
|
||||
}
|
||||
|
||||
public async set(key: string, value: unknown): Promise<this> {
|
||||
|
@ -39,7 +39,7 @@ export class ConvertingRouterRule extends RouterRule {
|
||||
entry.supportChecker.supports({ identifier, representation }));
|
||||
} else {
|
||||
// No content-type given so we can only check if one of the stores has data for the identifier
|
||||
store = await this.findStore(async(entry): Promise<boolean> => entry.store.resourceExists(identifier));
|
||||
store = await this.findStore(async(entry): Promise<boolean> => entry.store.hasResource(identifier));
|
||||
}
|
||||
return store;
|
||||
}
|
||||
|
@ -19,7 +19,7 @@ describe('A PatchOperationHandler', (): void => {
|
||||
operation = { method: 'PATCH', target: { path: 'http://test.com/foo' }, body, conditions, preferences: {}};
|
||||
|
||||
store = {
|
||||
resourceExists: jest.fn(),
|
||||
hasResource: jest.fn(),
|
||||
modifyResource: jest.fn(),
|
||||
} as any;
|
||||
|
||||
@ -47,7 +47,7 @@ describe('A PatchOperationHandler', (): void => {
|
||||
});
|
||||
|
||||
it('returns the correct response if the resource already exists.', async(): Promise<void> => {
|
||||
store.resourceExists.mockResolvedValueOnce(true);
|
||||
store.hasResource.mockResolvedValueOnce(true);
|
||||
const result = await handler.handle({ operation });
|
||||
expect(store.modifyResource).toHaveBeenCalledTimes(1);
|
||||
expect(store.modifyResource).toHaveBeenLastCalledWith(operation.target, body, conditions);
|
||||
|
@ -18,7 +18,7 @@ describe('A PutOperationHandler', (): void => {
|
||||
body = new BasicRepresentation('', 'text/turtle');
|
||||
operation = { method: 'PUT', target: { path: 'http://test.com/foo' }, body, conditions, preferences: {}};
|
||||
store = {
|
||||
resourceExists: jest.fn(),
|
||||
hasResource: jest.fn(),
|
||||
setRepresentation: jest.fn(),
|
||||
} as any;
|
||||
|
||||
@ -46,7 +46,7 @@ describe('A PutOperationHandler', (): void => {
|
||||
});
|
||||
|
||||
it('returns the correct response if the resource already exists.', async(): Promise<void> => {
|
||||
store.resourceExists.mockResolvedValueOnce(true);
|
||||
store.hasResource.mockResolvedValueOnce(true);
|
||||
const result = await handler.handle({ operation });
|
||||
expect(store.setRepresentation).toHaveBeenCalledTimes(1);
|
||||
expect(store.setRepresentation).toHaveBeenLastCalledWith(operation.target, body, conditions);
|
||||
|
@ -20,7 +20,7 @@ describe('A GeneratedPodManager', (): void => {
|
||||
};
|
||||
store = {
|
||||
setRepresentation: jest.fn(),
|
||||
resourceExists: jest.fn(),
|
||||
hasResource: jest.fn(),
|
||||
} as any;
|
||||
generatorData = [
|
||||
{ identifier: { path: '/path/' }, representation: '/' as any },
|
||||
@ -36,7 +36,7 @@ describe('A GeneratedPodManager', (): void => {
|
||||
});
|
||||
|
||||
it('throws an error if the generate identifier is not available.', async(): Promise<void> => {
|
||||
store.resourceExists.mockResolvedValueOnce(true);
|
||||
store.hasResource.mockResolvedValueOnce(true);
|
||||
const result = manager.createPod({ path: `${base}user/` }, settings, false);
|
||||
await expect(result).rejects.toThrow(`There already is a resource at ${base}user/`);
|
||||
await expect(result).rejects.toThrow(ConflictHttpError);
|
||||
@ -52,7 +52,7 @@ describe('A GeneratedPodManager', (): void => {
|
||||
});
|
||||
|
||||
it('allows overwriting when enabled.', async(): Promise<void> => {
|
||||
store.resourceExists.mockResolvedValueOnce(true);
|
||||
store.hasResource.mockResolvedValueOnce(true);
|
||||
await expect(manager.createPod({ path: `${base}${settings.login}/` }, settings, true)).resolves.toBeUndefined();
|
||||
|
||||
expect(store.setRepresentation).toHaveBeenCalledTimes(3);
|
||||
|
@ -25,7 +25,7 @@ describe('A BaseResourceStore', (): void => {
|
||||
await expect(store.modifyResource(any, any)).rejects.toThrow(NotImplementedHttpError);
|
||||
});
|
||||
|
||||
it('errors on resourceExists.', async(): Promise<void> => {
|
||||
await expect(store.resourceExists(any)).rejects.toThrow(NotImplementedHttpError);
|
||||
it('errors on hasResource.', async(): Promise<void> => {
|
||||
await expect(store.hasResource(any)).rejects.toThrow(NotImplementedHttpError);
|
||||
});
|
||||
});
|
||||
|
@ -726,13 +726,13 @@ describe('A DataAccessorBasedStore', (): void => {
|
||||
describe('resource Exists', (): void => {
|
||||
it('should return false when the resource does not exist.', async(): Promise<void> => {
|
||||
const resourceID = { path: `${root}resource` };
|
||||
await expect(store.resourceExists(resourceID)).resolves.toBeFalsy();
|
||||
await expect(store.hasResource(resourceID)).resolves.toBeFalsy();
|
||||
});
|
||||
|
||||
it('should return true when the resource exists.', async(): Promise<void> => {
|
||||
const resourceID = { path: `${root}resource` };
|
||||
accessor.data[resourceID.path] = representation;
|
||||
await expect(store.resourceExists(resourceID)).resolves.toBeTruthy();
|
||||
await expect(store.hasResource(resourceID)).resolves.toBeTruthy();
|
||||
});
|
||||
|
||||
it('should rethrow any unexpected errors from validateIdentifier.', async(): Promise<void> => {
|
||||
@ -741,7 +741,7 @@ describe('A DataAccessorBasedStore', (): void => {
|
||||
accessor.getMetadata = jest.fn(async(): Promise<any> => {
|
||||
throw new Error('error');
|
||||
});
|
||||
await expect(store.resourceExists(resourceID)).rejects.toThrow('error');
|
||||
await expect(store.hasResource(resourceID)).rejects.toThrow('error');
|
||||
accessor.getMetadata = originalMetaData;
|
||||
});
|
||||
});
|
||||
|
@ -39,7 +39,7 @@ describe('A LockingResourceStore', (): void => {
|
||||
setRepresentation: jest.fn((): any => addOrder('setRepresentation')),
|
||||
deleteResource: jest.fn((): any => addOrder('deleteResource')),
|
||||
modifyResource: jest.fn((): any => addOrder('modifyResource')),
|
||||
resourceExists: jest.fn((): any => addOrder('resourceExists')),
|
||||
hasResource: jest.fn((): any => addOrder('hasResource')),
|
||||
};
|
||||
|
||||
timeoutTrigger = new EventEmitter();
|
||||
@ -287,12 +287,12 @@ describe('A LockingResourceStore', (): void => {
|
||||
expect(order).toEqual([ 'lock read', 'useless get', 'timeout', 'unlock read' ]);
|
||||
});
|
||||
|
||||
it('resourceExists should only acquire and release the read lock.', async(): Promise<void> => {
|
||||
await store.resourceExists(subjectId);
|
||||
it('hasResource should only acquire and release the read lock.', async(): Promise<void> => {
|
||||
await store.hasResource(subjectId);
|
||||
expect(locker.withReadLock).toHaveBeenCalledTimes(1);
|
||||
expect(locker.withWriteLock).toHaveBeenCalledTimes(0);
|
||||
expect(source.resourceExists).toHaveBeenCalledTimes(1);
|
||||
expect(source.resourceExists).toHaveBeenLastCalledWith(subjectId, undefined);
|
||||
expect(order).toEqual([ 'lock read', 'resourceExists', 'unlock read' ]);
|
||||
expect(source.hasResource).toHaveBeenCalledTimes(1);
|
||||
expect(source.hasResource).toHaveBeenLastCalledWith(subjectId);
|
||||
expect(order).toEqual([ 'lock read', 'hasResource', 'unlock read' ]);
|
||||
});
|
||||
});
|
||||
|
@ -19,7 +19,7 @@ describe('A MonitoringStore', (): void => {
|
||||
setRepresentation: jest.fn(async(): Promise<any> => modified),
|
||||
deleteResource: jest.fn(async(): Promise<any> => modified),
|
||||
modifyResource: jest.fn(async(): Promise<any> => modified),
|
||||
resourceExists: jest.fn(async(): Promise<any> => undefined),
|
||||
hasResource: jest.fn(async(): Promise<any> => undefined),
|
||||
};
|
||||
store = new MonitoringStore(source);
|
||||
changedCallback = jest.fn();
|
||||
@ -106,9 +106,9 @@ describe('A MonitoringStore', (): void => {
|
||||
expect(changedCallback).toHaveBeenCalledWith({ path: 'http://example.org/modified/2' });
|
||||
});
|
||||
|
||||
it('calls resourceExists directly from the source.', async(): Promise<void> => {
|
||||
await expect(store.resourceExists({ path: 'http://example.org/foo/bar' })).resolves.toBeUndefined();
|
||||
expect(source.resourceExists).toHaveBeenCalledTimes(1);
|
||||
expect(source.resourceExists).toHaveBeenLastCalledWith({ path: 'http://example.org/foo/bar' }, undefined);
|
||||
it('calls hasResource directly from the source.', async(): Promise<void> => {
|
||||
await expect(store.hasResource({ path: 'http://example.org/foo/bar' })).resolves.toBeUndefined();
|
||||
expect(source.hasResource).toHaveBeenCalledTimes(1);
|
||||
expect(source.hasResource).toHaveBeenLastCalledWith({ path: 'http://example.org/foo/bar' });
|
||||
});
|
||||
});
|
||||
|
@ -14,7 +14,7 @@ describe('A PassthroughStore', (): void => {
|
||||
setRepresentation: jest.fn(async(): Promise<any> => 'set'),
|
||||
deleteResource: jest.fn(async(): Promise<any> => 'delete'),
|
||||
modifyResource: jest.fn(async(): Promise<any> => 'modify'),
|
||||
resourceExists: jest.fn(async(): Promise<any> => 'exists'),
|
||||
hasResource: jest.fn(async(): Promise<any> => 'exists'),
|
||||
};
|
||||
|
||||
store = new PassthroughStore(source);
|
||||
@ -50,9 +50,9 @@ describe('A PassthroughStore', (): void => {
|
||||
expect(source.modifyResource).toHaveBeenLastCalledWith({ path: 'modifyPath' }, {}, undefined);
|
||||
});
|
||||
|
||||
it('calls resourceExists directly from the source.', async(): Promise<void> => {
|
||||
await expect(store.resourceExists({ path: 'existsPath' })).resolves.toBe('exists');
|
||||
expect(source.resourceExists).toHaveBeenCalledTimes(1);
|
||||
expect(source.resourceExists).toHaveBeenLastCalledWith({ path: 'existsPath' }, undefined);
|
||||
it('calls hasResource directly from the source.', async(): Promise<void> => {
|
||||
await expect(store.hasResource({ path: 'existsPath' })).resolves.toBe('exists');
|
||||
expect(source.hasResource).toHaveBeenCalledTimes(1);
|
||||
expect(source.hasResource).toHaveBeenLastCalledWith({ path: 'existsPath' });
|
||||
});
|
||||
});
|
||||
|
@ -17,7 +17,7 @@ describe('A RoutingResourceStore', (): void => {
|
||||
setRepresentation: jest.fn(),
|
||||
modifyResource: jest.fn(),
|
||||
deleteResource: jest.fn(),
|
||||
resourceExists: jest.fn(),
|
||||
hasResource: jest.fn(),
|
||||
};
|
||||
|
||||
rule = new StaticAsyncHandler(true, source);
|
||||
@ -60,10 +60,10 @@ describe('A RoutingResourceStore', (): void => {
|
||||
expect(source.deleteResource).toHaveBeenLastCalledWith(identifier, 'conditions');
|
||||
});
|
||||
|
||||
it('calls resourceExists on the resulting store.', async(): Promise<void> => {
|
||||
await expect(store.resourceExists(identifier)).resolves.toBeUndefined();
|
||||
expect(source.resourceExists).toHaveBeenCalledTimes(1);
|
||||
expect(source.resourceExists).toHaveBeenLastCalledWith(identifier, undefined);
|
||||
it('calls hasResource on the resulting store.', async(): Promise<void> => {
|
||||
await expect(store.hasResource(identifier)).resolves.toBeUndefined();
|
||||
expect(source.hasResource).toHaveBeenCalledTimes(1);
|
||||
expect(source.hasResource).toHaveBeenLastCalledWith(identifier);
|
||||
});
|
||||
|
||||
it('throws a 404 if there is no body and no store was found.', async(): Promise<void> => {
|
||||
|
@ -19,7 +19,7 @@ describe('A JsonResourceStorage', (): void => {
|
||||
beforeEach(async(): Promise<void> => {
|
||||
const data: Record<string, string> = { };
|
||||
store = {
|
||||
async resourceExists(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
async hasResource(identifier: ResourceIdentifier): Promise<boolean> {
|
||||
return Boolean(data[identifier.path]);
|
||||
},
|
||||
async getRepresentation(identifier: ResourceIdentifier): Promise<Representation> {
|
||||
|
@ -39,18 +39,18 @@ describe('A ConvertingRouterRule', (): void => {
|
||||
});
|
||||
|
||||
it('checks if the stores contain the identifier if there is no data.', async(): Promise<void> => {
|
||||
store1.resourceExists = jest.fn().mockImplementationOnce((): any => true);
|
||||
store1.hasResource = jest.fn().mockImplementationOnce((): any => true);
|
||||
await expect(rule.handle({ identifier: { path: 'identifier' }})).resolves.toBe(store1);
|
||||
expect(store1.resourceExists).toHaveBeenCalledTimes(1);
|
||||
expect(store1.hasResource).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('returns the defaultStore if no other store has the resource.', async(): Promise<void> => {
|
||||
store1.resourceExists = jest.fn().mockImplementationOnce((): any => false);
|
||||
store1.hasResource = jest.fn().mockImplementationOnce((): any => false);
|
||||
await expect(rule.handle({ identifier: { path: 'identifier' }})).resolves.toBe(defaultStore);
|
||||
});
|
||||
|
||||
it('throws the error if a store had a non-404 error.', async(): Promise<void> => {
|
||||
store1.resourceExists = jest.fn().mockRejectedValueOnce(new InternalServerError());
|
||||
store1.hasResource = jest.fn().mockRejectedValueOnce(new InternalServerError());
|
||||
await expect(rule.handle({ identifier: { path: 'identifier' }})).rejects.toThrow(InternalServerError);
|
||||
});
|
||||
});
|
||||
|
Loading…
x
Reference in New Issue
Block a user