fix: Prevent errors during migration for invalid JSON

This commit is contained in:
Joachim Van Herwegen 2023-11-17 15:35:49 +01:00
parent 01623e0b5c
commit 2f928bd2d4
4 changed files with 24 additions and 6 deletions

View File

@ -1,5 +1,6 @@
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
import { JsonResourceStorage } from '../../storage/keyvalue/JsonResourceStorage';
import { createErrorMessage } from '../../util/errors/ErrorUtil';
import { isContainerIdentifier } from '../../util/PathUtil';
import { readableToString } from '../../util/StreamUtil';
import { LDP } from '../../util/Vocabularies';
@ -34,9 +35,14 @@ export class SingleContainerJsonStorage<T> extends JsonResourceStorage<T> {
continue;
}
const json = JSON.parse(await readableToString(document.data)) as T;
const key = this.identifierToKey(documentId);
try {
const json = JSON.parse(await readableToString(document.data)) as T;
yield [ key, json ];
} catch (error: unknown) {
this.logger.error(`Unable to parse ${path}. You should probably delete this resource manually. Error: ${
createErrorMessage(error)}`);
}
}
}
}

View File

@ -0,0 +1 @@
this is not valid JSON

View File

@ -63,6 +63,8 @@ describe('A server migrating from v6', (): void => {
// Setup resources should have been migrated
const setupDir = await readdir(joinFilePath(rootFilePath, '.internal/setup/'));
expect(setupDir).toEqual([
// Invalid JSON file was not deleted, only error was logged. Just in case its data needs to be saved.
'aW52YWxpZFJlc291cmNl$.json',
'current-base-url$.json',
'current-server-version$.json',
'setupCompleted-2.0$.json',

View File

@ -19,6 +19,7 @@ describe('A SingleContainerJsonStorage', (): void => {
if (isContainerIdentifier(id)) {
const metadata = new RepresentationMetadata(id);
metadata.add(LDP.terms.contains, 'http://example.com/.internal/accounts/foo');
metadata.add(LDP.terms.contains, 'http://example.com/.internal/accounts/bad');
metadata.add(LDP.terms.contains, 'http://example.com/.internal/accounts/bar/');
metadata.add(LDP.terms.contains, 'http://example.com/.internal/accounts/baz');
metadata.add(LDP.terms.contains, 'http://example.com/.internal/accounts/unknown');
@ -27,14 +28,17 @@ describe('A SingleContainerJsonStorage', (): void => {
if (id.path.endsWith('unknown')) {
throw new NotFoundHttpError();
}
return new BasicRepresentation(`{ "id": "${id.path}" }`, 'text/plain');
if (id.path.endsWith('bad')) {
return new BasicRepresentation(`invalid JSON`, 'application/json');
}
return new BasicRepresentation(`{ "id": "${id.path}" }`, 'application/json');
}),
} satisfies Partial<ResourceStore> as any;
storage = new SingleContainerJsonStorage(store, baseUrl, container);
});
it('only iterates over the documents in the base container.', async(): Promise<void> => {
it('only iterates over the valid documents in the base container.', async(): Promise<void> => {
const entries = [];
for await (const entry of storage.entries()) {
entries.push(entry);
@ -43,7 +47,7 @@ describe('A SingleContainerJsonStorage', (): void => {
[ 'foo', { id: 'http://example.com/.internal/accounts/foo' }],
[ 'baz', { id: 'http://example.com/.internal/accounts/baz' }],
]);
expect(store.getRepresentation).toHaveBeenCalledTimes(4);
expect(store.getRepresentation).toHaveBeenCalledTimes(5);
expect(store.getRepresentation).toHaveBeenNthCalledWith(
1,
{ path: 'http://example.com/.internal/accounts/' },
@ -56,11 +60,16 @@ describe('A SingleContainerJsonStorage', (): void => {
);
expect(store.getRepresentation).toHaveBeenNthCalledWith(
3,
{ path: 'http://example.com/.internal/accounts/baz' },
{ path: 'http://example.com/.internal/accounts/bad' },
{ type: { 'application/json': 1 }},
);
expect(store.getRepresentation).toHaveBeenNthCalledWith(
4,
{ path: 'http://example.com/.internal/accounts/baz' },
{ type: { 'application/json': 1 }},
);
expect(store.getRepresentation).toHaveBeenNthCalledWith(
5,
{ path: 'http://example.com/.internal/accounts/unknown' },
{ type: { 'application/json': 1 }},
);