Skip to content

Commit

Permalink
Merge branch 'main' into versions/4.0.0
Browse files Browse the repository at this point in the history
# Conflicts:
#	test/unit/storage/accessors/FileDataAccessor.test.ts
  • Loading branch information
joachimvh committed Apr 15, 2022
2 parents 4f7eec3 + 30799f6 commit e651999
Show file tree
Hide file tree
Showing 16 changed files with 150 additions and 95 deletions.
1 change: 1 addition & 0 deletions config/quota-file.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"files-scs:config/app/main/default.json",
"files-scs:config/app/init/default.json",
"files-scs:config/app/setup/required.json",
"files-scs:config/app/variables/default.json",
"files-scs:config/http/handler/default.json",
"files-scs:config/http/middleware/websockets.json",
"files-scs:config/http/server-factory/websockets.json",
Expand Down
1 change: 1 addition & 0 deletions config/sparql-file-storage.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"files-scs:config/app/main/default.json",
"files-scs:config/app/init/default.json",
"files-scs:config/app/setup/required.json",
"files-scs:config/app/variables/default.json",
"files-scs:config/http/handler/default.json",
"files-scs:config/http/middleware/websockets.json",
"files-scs:config/http/server-factory/websockets.json",
Expand Down
5 changes: 2 additions & 3 deletions src/init/SeededPodInitializer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { promises as fsPromises } from 'fs';
import { readJson } from 'fs-extra';
import type { RegistrationManager } from '../identity/interaction/email-password/util/RegistrationManager';
import { getLoggerFor } from '../logging/LogUtil';
import { Initializer } from './Initializer';
Expand All @@ -23,8 +23,7 @@ export class SeededPodInitializer extends Initializer {
if (!this.configFilePath) {
return;
}
const configText = await fsPromises.readFile(this.configFilePath, 'utf8');
const configuration: NodeJS.Dict<unknown>[] = JSON.parse(configText);
const configuration = await readJson(this.configFilePath, 'utf8');

let count = 0;
for await (const input of configuration) {
Expand Down
9 changes: 4 additions & 5 deletions src/storage/accessors/AtomicFileDataAccessor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { mkdirSync, promises as fsPromises } from 'fs';
import type { Readable } from 'stream';
import { ensureDirSync, rename, unlink } from 'fs-extra';
import { v4 } from 'uuid';
import type { RepresentationMetadata } from '../../http/representation/RepresentationMetadata';
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
Expand All @@ -20,8 +20,7 @@ export class AtomicFileDataAccessor extends FileDataAccessor implements AtomicDa
public constructor(resourceMapper: FileIdentifierMapper, rootFilePath: string, tempFilePath: string) {
super(resourceMapper);
this.tempFilePath = joinFilePath(rootFilePath, tempFilePath);
// Cannot use fsPromises in constructor
mkdirSync(this.tempFilePath, { recursive: true });
ensureDirSync(this.tempFilePath);
}

/**
Expand All @@ -45,12 +44,12 @@ export class AtomicFileDataAccessor extends FileDataAccessor implements AtomicDa
await this.verifyExistingExtension(link);

// When no quota errors occur move the file to its desired location
await fsPromises.rename(tempFilePath, link.filePath);
await rename(tempFilePath, link.filePath);
} catch (error: unknown) {
// Delete the data already written
try {
if ((await this.getStats(tempFilePath)).isFile()) {
await fsPromises.unlink(tempFilePath);
await unlink(tempFilePath);
}
} catch {
throw error;
Expand Down
62 changes: 17 additions & 45 deletions src/storage/accessors/FileDataAccessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Stats } from 'fs';
import { createWriteStream, createReadStream, promises as fsPromises } from 'fs';
import type { Readable } from 'stream';
import type { Stats } from 'fs-extra';
import { ensureDir, remove, stat, lstat, createWriteStream, createReadStream, opendir } from 'fs-extra';
import type { Quad } from 'rdf-js';
import type { Representation } from '../../http/representation/Representation';
import { RepresentationMetadata } from '../../http/representation/RepresentationMetadata';
Expand Down Expand Up @@ -96,7 +96,7 @@ export class FileDataAccessor implements DataAccessor {
// Delete the metadata if there was an error writing the file
if (wroteMetadata) {
const metaLink = await this.resourceMapper.mapUrlToFilePath(identifier, true);
await fsPromises.unlink(metaLink.filePath);
await remove(metaLink.filePath);
}
throw error;
}
Expand All @@ -107,14 +107,7 @@ export class FileDataAccessor implements DataAccessor {
*/
public async writeContainer(identifier: ResourceIdentifier, metadata: RepresentationMetadata): Promise<void> {
const link = await this.resourceMapper.mapUrlToFilePath(identifier, false);
try {
await fsPromises.mkdir(link.filePath, { recursive: true });
} catch (error: unknown) {
// Don't throw if directory already exists
if (!isSystemError(error) || error.code !== 'EEXIST') {
throw error;
}
}
await ensureDir(link.filePath);

await this.writeMetadata(link, metadata);
}
Expand All @@ -123,23 +116,16 @@ export class FileDataAccessor implements DataAccessor {
* Removes the corresponding file/folder (and metadata file).
*/
public async deleteResource(identifier: ResourceIdentifier): Promise<void> {
const metaLink = await this.resourceMapper.mapUrlToFilePath(identifier, true);
await remove(metaLink.filePath);

const link = await this.resourceMapper.mapUrlToFilePath(identifier, false);
const stats = await this.getStats(link.filePath);

try {
const metaLink = await this.resourceMapper.mapUrlToFilePath(identifier, true);
await fsPromises.unlink(metaLink.filePath);
} catch (error: unknown) {
// Ignore if it doesn't exist
if (!isSystemError(error) || error.code !== 'ENOENT') {
throw error;
}
}

if (!isContainerIdentifier(identifier) && stats.isFile()) {
await fsPromises.unlink(link.filePath);
await remove(link.filePath);
} else if (isContainerIdentifier(identifier) && stats.isDirectory()) {
await fsPromises.rmdir(link.filePath);
await remove(link.filePath);
} else {
throw new NotFoundHttpError();
}
Expand All @@ -155,7 +141,7 @@ export class FileDataAccessor implements DataAccessor {
*/
protected async getStats(path: string): Promise<Stats> {
try {
return await fsPromises.stat(path);
return await stat(path);
} catch (error: unknown) {
if (isSystemError(error) && error.code === 'ENOENT') {
throw new NotFoundHttpError('', { cause: error });
Expand Down Expand Up @@ -216,14 +202,7 @@ export class FileDataAccessor implements DataAccessor {

// Delete (potentially) existing metadata file if no metadata needs to be stored
} else {
try {
await fsPromises.unlink(metadataLink.filePath);
} catch (error: unknown) {
// Metadata file doesn't exist so nothing needs to be removed
if (!isSystemError(error) || error.code !== 'ENOENT') {
throw error;
}
}
await remove(metadataLink.filePath);
wroteMetadata = false;
}
return wroteMetadata;
Expand Down Expand Up @@ -255,7 +234,7 @@ export class FileDataAccessor implements DataAccessor {
const metadataLink = await this.resourceMapper.mapUrlToFilePath(identifier, true);

// Check if the metadata file exists first
await fsPromises.lstat(metadataLink.filePath);
await lstat(metadataLink.filePath);

const readMetadataStream = guardStream(createReadStream(metadataLink.filePath));
return await parseQuads(readMetadataStream, { format: metadataLink.contentType, baseIRI: identifier.path });
Expand All @@ -274,7 +253,7 @@ export class FileDataAccessor implements DataAccessor {
* @param link - Path related metadata.
*/
private async* getChildMetadata(link: ResourceLink): AsyncIterableIterator<RepresentationMetadata> {
const dir = await fsPromises.opendir(link.filePath);
const dir = await opendir(link.filePath);

// For every child in the container we want to generate specific metadata
for await (const entry of dir) {
Expand Down Expand Up @@ -345,17 +324,10 @@ export class FileDataAccessor implements DataAccessor {
* @param link - ResourceLink corresponding to the new resource data.
*/
protected async verifyExistingExtension(link: ResourceLink): Promise<void> {
try {
// Delete the old file with the (now) wrong extension
const oldLink = await this.resourceMapper.mapUrlToFilePath(link.identifier, false);
if (oldLink.filePath !== link.filePath) {
await fsPromises.unlink(oldLink.filePath);
}
} catch (error: unknown) {
// Ignore it if the file didn't exist yet and couldn't be unlinked
if (!isSystemError(error) || error.code !== 'ENOENT') {
throw error;
}
// Delete the old file with the (now) wrong extension
const oldLink = await this.resourceMapper.mapUrlToFilePath(link.identifier, false);
if (oldLink.filePath !== link.filePath) {
await remove(oldLink.filePath);
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/storage/keyvalue/JsonFileStorage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { promises as fsPromises } from 'fs';
import { writeJson, readJson } from 'fs-extra';
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
import { isSystemError } from '../../util/errors/SystemError';
import type { ReadWriteLocker } from '../../util/locking/ReadWriteLocker';
Expand Down Expand Up @@ -70,8 +70,7 @@ export class JsonFileStorage implements KeyValueStorage<string, unknown> {
return this.locker.withWriteLock(this.lockIdentifier, async(): Promise<T> => {
const json = await this.getJson();
const result = updateFn(json);
const updatedText = JSON.stringify(json, null, 2);
await fsPromises.writeFile(this.filePath, updatedText, 'utf8');
await writeJson(this.filePath, json, { encoding: 'utf8', spaces: 2 });
return result;
});
}
Expand All @@ -81,8 +80,7 @@ export class JsonFileStorage implements KeyValueStorage<string, unknown> {
*/
private async getJson(): Promise<NodeJS.Dict<unknown>> {
try {
const text = await fsPromises.readFile(this.filePath, 'utf8');
return JSON.parse(text);
return await readJson(this.filePath, 'utf8');
} catch (error: unknown) {
if (isSystemError(error) && error.code === 'ENOENT') {
return {};
Expand Down
13 changes: 7 additions & 6 deletions test/unit/init/SeededPodInitializer.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { promises as fsPromises } from 'fs';
import { writeJson } from 'fs-extra';
import type { RegistrationManager } from '../../../src/identity/interaction/email-password/util/RegistrationManager';
import { SeededPodInitializer } from '../../../src/init/SeededPodInitializer';
import { mockFs } from '../../util/Util';
import { mockFileSystem } from '../../util/Util';

jest.mock('fs');
jest.mock('fs-extra');

describe('A SeededPodInitializer', (): void => {
const dummyConfig = JSON.stringify([
const dummyConfig = [
{
podName: 'example',
email: 'hello@example.com',
Expand All @@ -17,7 +18,7 @@ describe('A SeededPodInitializer', (): void => {
email: 'hello2@example.com',
password: '123abc',
},
]);
];
let registrationManager: RegistrationManager;
let configFilePath: string | null;

Expand All @@ -28,8 +29,8 @@ describe('A SeededPodInitializer', (): void => {
register: jest.fn(),
} as any;

mockFs('/');
await fsPromises.writeFile(configFilePath, dummyConfig);
mockFileSystem('/');
await writeJson(configFilePath, dummyConfig);
});

it('does not generate any accounts or pods if no config file is specified.', async(): Promise<void> => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/pods/generate/TemplatedResourcesGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
import { ensureTrailingSlash, trimTrailingSlashes } from '../../../../src/util/PathUtil';
import { readableToString } from '../../../../src/util/StreamUtil';
import { HandlebarsTemplateEngine } from '../../../../src/util/templates/HandlebarsTemplateEngine';
import { mockFs } from '../../../util/Util';
import { mockFileSystem } from '../../../util/Util';

const { namedNode } = DataFactory;

Expand Down Expand Up @@ -55,7 +55,7 @@ describe('A TemplatedResourcesGenerator', (): void => {
const webId = 'http://alice/#profile';

beforeEach(async(): Promise<void> => {
cache = mockFs(rootFilePath);
cache = mockFileSystem(rootFilePath);
});

it('fills in a template with the given options.', async(): Promise<void> => {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/quota/PodQuotaStrategy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError';
import type { IdentifierStrategy } from '../../../src/util/identifiers/IdentifierStrategy';
import { SingleRootIdentifierStrategy } from '../../../src/util/identifiers/SingleRootIdentifierStrategy';
import { PIM, RDF } from '../../../src/util/Vocabularies';
import { mockFs } from '../../util/Util';
import { mockFileSystem } from '../../util/Util';

jest.mock('fs');

Expand All @@ -24,7 +24,7 @@ describe('PodQuotaStrategy', (): void => {

beforeEach((): void => {
jest.restoreAllMocks();
mockFs(rootFilePath, new Date());
mockFileSystem(rootFilePath, new Date());
mockSize = { amount: 2000, unit: UNIT_BYTES };
identifierStrategy = new SingleRootIdentifierStrategy(base);
mockReporter = {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/quota/QuotaStrategy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { UNIT_BYTES } from '../../../src/storage/size-reporter/Size';
import type { Size } from '../../../src/storage/size-reporter/Size';
import type { SizeReporter } from '../../../src/storage/size-reporter/SizeReporter';
import { guardedStreamFrom, pipeSafely } from '../../../src/util/StreamUtil';
import { mockFs } from '../../util/Util';
import { mockFileSystem } from '../../util/Util';

jest.mock('fs');

Expand All @@ -26,7 +26,7 @@ describe('A QuotaStrategy', (): void => {

beforeEach((): void => {
jest.restoreAllMocks();
mockFs(rootFilePath, new Date());
mockFileSystem(rootFilePath, new Date());
mockSize = { amount: 2000, unit: UNIT_BYTES };
mockReporter = {
getSize: jest.fn().mockResolvedValue({ unit: mockSize.unit, amount: 50 }),
Expand Down
19 changes: 10 additions & 9 deletions test/unit/storage/accessors/AtomicFileDataAccessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import { APPLICATION_OCTET_STREAM } from '../../../../src/util/ContentTypes';
import type { Guarded } from '../../../../src/util/GuardedStream';
import { guardedStreamFrom } from '../../../../src/util/StreamUtil';
import { CONTENT_TYPE } from '../../../../src/util/Vocabularies';
import { mockFs } from '../../../util/Util';
import { mockFileSystem } from '../../../util/Util';

jest.mock('fs');
jest.mock('fs-extra');

describe('AtomicFileDataAccessor', (): void => {
const rootFilePath = 'uploads';
Expand All @@ -20,7 +21,7 @@ describe('AtomicFileDataAccessor', (): void => {
let data: Guarded<Readable>;

beforeEach(async(): Promise<void> => {
cache = mockFs(rootFilePath, new Date());
cache = mockFileSystem(rootFilePath, new Date());
accessor = new AtomicFileDataAccessor(
new ExtensionBasedMapper(base, rootFilePath),
rootFilePath,
Expand Down Expand Up @@ -59,38 +60,38 @@ describe('AtomicFileDataAccessor', (): void => {
data.emit('error', new Error('error'));
return null;
});
jest.requireMock('fs').promises.stat = jest.fn((): any => ({
jest.requireMock('fs-extra').stat = jest.fn((): any => ({
isFile: (): boolean => false,
}));
await expect(accessor.writeDocument({ path: `${base}res.ttl` }, data, metadata)).rejects.toThrow('error');
});

it('should throw when renaming / moving the file goes wrong.', async(): Promise<void> => {
jest.requireMock('fs').promises.rename = jest.fn((): any => {
jest.requireMock('fs-extra').rename = jest.fn((): any => {
throw new Error('error');
});
jest.requireMock('fs').promises.stat = jest.fn((): any => ({
jest.requireMock('fs-extra').stat = jest.fn((): any => ({
isFile: (): boolean => true,
}));
await expect(accessor.writeDocument({ path: `${base}res.ttl` }, data, metadata)).rejects.toThrow('error');
});

it('should (on error) not unlink the temp file if it does not exist.', async(): Promise<void> => {
jest.requireMock('fs').promises.rename = jest.fn((): any => {
jest.requireMock('fs-extra').rename = jest.fn((): any => {
throw new Error('error');
});
jest.requireMock('fs').promises.stat = jest.fn((): any => ({
jest.requireMock('fs-extra').stat = jest.fn((): any => ({
isFile: (): boolean => false,
}));
await expect(accessor.writeDocument({ path: `${base}res.ttl` }, data, metadata)).rejects.toThrow('error');
});

it('should throw when renaming / moving the file goes wrong and the temp file does not exist.',
async(): Promise<void> => {
jest.requireMock('fs').promises.rename = jest.fn((): any => {
jest.requireMock('fs-extra').rename = jest.fn((): any => {
throw new Error('error');
});
jest.requireMock('fs').promises.stat = jest.fn();
jest.requireMock('fs-extra').stat = jest.fn();
await expect(accessor.writeDocument({ path: `${base}res.ttl` }, data, metadata)).rejects.toThrow('error');
});
});
Expand Down

0 comments on commit e651999

Please sign in to comment.