Desktop: Fixed issue with attachment paths being invalid when user has spaces in home directory path. Also fixed race condition in test units that could make them randomly fail.

This commit is contained in:
Laurent Cozic
2020-12-09 12:53:47 +00:00
parent 46ed1304f2
commit dc3e9178ef
12 changed files with 131 additions and 67 deletions
+3
View File
@@ -910,6 +910,9 @@ packages/lib/errorUtils.js.map
packages/lib/eventManager.d.ts
packages/lib/eventManager.js
packages/lib/eventManager.js.map
packages/lib/fs-driver-base.d.ts
packages/lib/fs-driver-base.js
packages/lib/fs-driver-base.js.map
packages/lib/fs-driver-node.d.ts
packages/lib/fs-driver-node.js
packages/lib/fs-driver-node.js.map
+3
View File
@@ -899,6 +899,9 @@ packages/lib/errorUtils.js.map
packages/lib/eventManager.d.ts
packages/lib/eventManager.js
packages/lib/eventManager.js.map
packages/lib/fs-driver-base.d.ts
packages/lib/fs-driver-base.js
packages/lib/fs-driver-base.js.map
packages/lib/fs-driver-node.d.ts
packages/lib/fs-driver-node.js
packages/lib/fs-driver-node.js.map
+1 -1
View File
@@ -1,2 +1,2 @@
data/
test data/
export/
+6 -3
View File
@@ -1,6 +1,7 @@
import Setting from '@joplin/lib/models/Setting';
import BaseModel from '@joplin/lib/BaseModel';
import shim from '@joplin/lib/shim';
import markdownUtils from '@joplin/lib/markdownUtils';
const { sortedIds, createNTestNotes, setupDatabaseAndSynchronizer, switchClient, checkThrowAsync } = require('./test-utils.js');
const Folder = require('@joplin/lib/models/Folder.js');
const Note = require('@joplin/lib/models/Note.js');
@@ -217,6 +218,8 @@ describe('models_Note', function() {
const t1 = r1.updated_time;
const t2 = r2.updated_time;
const resourceDirE = markdownUtils.escapeLinkUrl(resourceDir);
const testCases = [
[
false,
@@ -241,17 +244,17 @@ describe('models_Note', function() {
[
true,
`![](:/${r1.id})`,
`![](file://${resourceDir}/${r1.id}.jpg?t=${t1})`,
`![](file://${resourceDirE}/${r1.id}.jpg?t=${t1})`,
],
[
true,
`![](:/${r1.id}) ![](:/${r1.id}) ![](:/${r2.id})`,
`![](file://${resourceDir}/${r1.id}.jpg?t=${t1}) ![](file://${resourceDir}/${r1.id}.jpg?t=${t1}) ![](file://${resourceDir}/${r2.id}.jpg?t=${t2})`,
`![](file://${resourceDirE}/${r1.id}.jpg?t=${t1}) ![](file://${resourceDirE}/${r1.id}.jpg?t=${t1}) ![](file://${resourceDirE}/${r2.id}.jpg?t=${t2})`,
],
[
true,
`![](:/${r3.id})`,
`![](file://${resourceDir}/${r3.id}.pdf)`,
`![](file://${resourceDirE}/${r3.id}.pdf)`,
],
];
@@ -2,8 +2,7 @@ import InteropService from '@joplin/lib/services/interop/InteropService';
import { CustomExportContext, CustomImportContext, Module, ModuleType } from '@joplin/lib/services/interop/types';
import shim from '@joplin/lib/shim';
const { fileContentEqual, setupDatabaseAndSynchronizer, switchClient, checkThrowAsync } = require('./test-utils.js');
const { fileContentEqual, setupDatabaseAndSynchronizer, switchClient, checkThrowAsync, exportDir } = require('./test-utils.js');
const Folder = require('@joplin/lib/models/Folder.js');
const Note = require('@joplin/lib/models/Note.js');
const Tag = require('@joplin/lib/models/Tag.js');
@@ -11,10 +10,6 @@ const Resource = require('@joplin/lib/models/Resource.js');
const fs = require('fs-extra');
const ArrayUtils = require('@joplin/lib/ArrayUtils');
function exportDir() {
return `${__dirname}/export`;
}
async function recreateExportDir() {
const dir = exportDir();
await fs.remove(dir);
@@ -2,7 +2,7 @@
const fs = require('fs-extra');
const { setupDatabaseAndSynchronizer, switchClient } = require('./test-utils.js');
const { setupDatabaseAndSynchronizer, switchClient, exportDir } = require('./test-utils.js');
const InteropService_Exporter_Md = require('@joplin/lib/services/interop/InteropService_Exporter_Md').default;
const BaseModel = require('@joplin/lib/BaseModel').default;
const Folder = require('@joplin/lib/models/Folder.js');
@@ -10,29 +10,27 @@ const Resource = require('@joplin/lib/models/Resource.js');
const Note = require('@joplin/lib/models/Note.js');
const shim = require('@joplin/lib/shim').default;
const exportDir = `${__dirname}/export`;
describe('services_InteropService_Exporter_Md', function() {
beforeEach(async (done) => {
await setupDatabaseAndSynchronizer(1);
await switchClient(1);
await fs.remove(exportDir);
await fs.mkdirp(exportDir);
await fs.remove(exportDir());
await fs.mkdirp(exportDir());
done();
});
it('should create resources directory', (async () => {
const service = new InteropService_Exporter_Md();
await service.init(exportDir);
await service.init(exportDir());
expect(await shim.fsDriver().exists(`${exportDir}/_resources/`)).toBe(true);
expect(await shim.fsDriver().exists(`${exportDir()}/_resources/`)).toBe(true);
}));
it('should create note paths and add them to context', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -74,7 +72,7 @@ describe('services_InteropService_Exporter_Md', function() {
it('should handle duplicate note names', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -101,7 +99,7 @@ describe('services_InteropService_Exporter_Md', function() {
it('should not override existing files', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -118,7 +116,7 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.processItem(Folder.modelType(), folder1);
// Create a file with the path of note1 before processing note1
await shim.fsDriver().writeFile(`${exportDir}/folder1/note1.md`, 'Note content', 'utf-8');
await shim.fsDriver().writeFile(`${exportDir()}/folder1/note1.md`, 'Note content', 'utf-8');
await exporter.prepareForProcessingItemType(BaseModel.TYPE_NOTE, itemsToExport);
@@ -128,7 +126,7 @@ describe('services_InteropService_Exporter_Md', function() {
it('should save resource files in _resource directory', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -159,13 +157,13 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.processResource(resource1, Resource.fullPath(resource1));
await exporter.processResource(resource2, Resource.fullPath(resource2));
expect(await shim.fsDriver().exists(`${exportDir}/_resources/${Resource.filename(resource1)}`)).toBe(true, 'Resource file should be copied to _resources directory.');
expect(await shim.fsDriver().exists(`${exportDir}/_resources/${Resource.filename(resource2)}`)).toBe(true, 'Resource file should be copied to _resources directory.');
expect(await shim.fsDriver().exists(`${exportDir()}/_resources/${Resource.filename(resource1)}`)).toBe(true, 'Resource file should be copied to _resources directory.');
expect(await shim.fsDriver().exists(`${exportDir()}/_resources/${Resource.filename(resource2)}`)).toBe(true, 'Resource file should be copied to _resources directory.');
}));
it('should create folders in fs', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -189,14 +187,14 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.prepareForProcessingItemType(BaseModel.TYPE_NOTE, itemsToExport);
await exporter.processItem(Note.modelType(), note2);
expect(await shim.fsDriver().exists(`${exportDir}/folder1`)).toBe(true, 'Folder should be created in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir}/folder1/folder2`)).toBe(true, 'Folder should be created in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir}/folder1/folder3`)).toBe(true, 'Folder should be created in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir()}/folder1`)).toBe(true, 'Folder should be created in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir()}/folder1/folder2`)).toBe(true, 'Folder should be created in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir()}/folder1/folder3`)).toBe(true, 'Folder should be created in filesystem.');
}));
it('should save notes in fs', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -226,14 +224,14 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.processItem(Note.modelType(), note2);
await exporter.processItem(Note.modelType(), note3);
expect(await shim.fsDriver().exists(`${exportDir}/${exporter.context().notePaths[note1.id]}`)).toBe(true, 'File should be saved in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir}/${exporter.context().notePaths[note2.id]}`)).toBe(true, 'File should be saved in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir}/${exporter.context().notePaths[note3.id]}`)).toBe(true, 'File should be saved in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir()}/${exporter.context().notePaths[note1.id]}`)).toBe(true, 'File should be saved in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir()}/${exporter.context().notePaths[note2.id]}`)).toBe(true, 'File should be saved in filesystem.');
expect(await shim.fsDriver().exists(`${exportDir()}/${exporter.context().notePaths[note3.id]}`)).toBe(true, 'File should be saved in filesystem.');
}));
it('should replace resource ids with relative paths', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -269,8 +267,8 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.processItem(Note.modelType(), note1);
await exporter.processItem(Note.modelType(), note2);
const note1_body = await shim.fsDriver().readFile(`${exportDir}/${exporter.context().notePaths[note1.id]}`);
const note2_body = await shim.fsDriver().readFile(`${exportDir}/${exporter.context().notePaths[note2.id]}`);
const note1_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note1.id]}`);
const note2_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note2.id]}`);
expect(note1_body).toContain('](../_resources/resource1.jpg)', 'Resource id should be replaced with a relative path.');
expect(note2_body).toContain('](../../_resources/resource2.jpg)', 'Resource id should be replaced with a relative path.');
@@ -278,7 +276,7 @@ describe('services_InteropService_Exporter_Md', function() {
it('should replace note ids with relative paths', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -318,9 +316,9 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.processItem(Note.modelType(), note2);
await exporter.processItem(Note.modelType(), note3);
const note1_body = await shim.fsDriver().readFile(`${exportDir}/${exporter.context().notePaths[note1.id]}`);
const note2_body = await shim.fsDriver().readFile(`${exportDir}/${exporter.context().notePaths[note2.id]}`);
const note3_body = await shim.fsDriver().readFile(`${exportDir}/${exporter.context().notePaths[note3.id]}`);
const note1_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note1.id]}`);
const note2_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note2.id]}`);
const note3_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note3.id]}`);
expect(note1_body).toContain('](../folder3/note3.md)', 'Note id should be replaced with a relative path.');
expect(note2_body).toContain('](../../folder3/note3.md)', 'Resource id should be replaced with a relative path.');
@@ -330,7 +328,7 @@ describe('services_InteropService_Exporter_Md', function() {
it('should url encode relative note links', (async () => {
const exporter = new InteropService_Exporter_Md();
await exporter.init(exportDir);
await exporter.init(exportDir());
const itemsToExport = [];
const queueExportItem = (itemType, itemOrId) => {
@@ -351,7 +349,7 @@ describe('services_InteropService_Exporter_Md', function() {
await exporter.processItem(Note.modelType(), note1);
await exporter.processItem(Note.modelType(), note2);
const note2_body = await shim.fsDriver().readFile(`${exportDir}/${exporter.context().notePaths[note2.id]}`);
const note2_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note2.id]}`);
expect(note2_body).toContain('[link](../folder%20with%20space1/note1%20name%20with%20space.md)', 'Whitespace in URL should be encoded');
}));
});
+11 -2
View File
@@ -100,7 +100,11 @@ FileApiDriverLocal.fsDriver_ = fsDriver;
const logDir = `${__dirname}/../tests/logs`;
const baseTempDir = `${__dirname}/../tests/tmp/${suiteName_}`;
const dataDir = `${__dirname}/data/${suiteName_}`;
// We add a space in the data directory path as that will help uncover
// various space-in-path issues.
const dataDir = `${__dirname}/test data/${suiteName_}`;
fs.mkdirpSync(logDir, 0o755);
fs.mkdirpSync(baseTempDir, 0o755);
fs.mkdirpSync(dataDir);
@@ -302,6 +306,11 @@ async function setupDatabase(id: number = null, options: any = null) {
await loadKeychainServiceAndSettings(options.keychainEnabled ? KeychainServiceDriver : KeychainServiceDriverDummy);
}
function exportDir(id: number = null) {
if (id === null) id = currentClient_;
return `${dataDir}/export`;
}
function resourceDirName(id: number = null) {
if (id === null) id = currentClient_;
return `resources-${id}`;
@@ -783,4 +792,4 @@ class TestApp extends BaseApplication {
}
}
module.exports = { newPluginService, newPluginScript, synchronizerStart, afterEachCleanUp, syncTargetName, setSyncTargetName, syncDir, createTempDir, isNetworkSyncTarget, kvStore, expectThrow, logger, expectNotThrow, resourceService, resourceFetcher, tempFilePath, allSyncTargetItemsEncrypted, msleep, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, checkThrow, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, currentClientId, id, ids, sortedIds, at, createNTestNotes, createNTestFolders, createNTestTags, TestApp };
module.exports = { exportDir, newPluginService, newPluginScript, synchronizerStart, afterEachCleanUp, syncTargetName, setSyncTargetName, syncDir, createTempDir, isNetworkSyncTarget, kvStore, expectThrow, logger, expectNotThrow, resourceService, resourceFetcher, tempFilePath, allSyncTargetItemsEncrypted, msleep, setupDatabase, revisionService, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync, checkThrow, encryptionService, loadEncryptionMasterKey, fileContentEqual, decryptionWorker, currentClientId, id, ids, sortedIds, at, createNTestNotes, createNTestFolders, createNTestTags, TestApp };
+1 -1
View File
@@ -1,5 +1,5 @@
const RNFS = require('react-native-fs');
const FsDriverBase = require('@joplin/lib/fs-driver-base');
const FsDriverBase = require('@joplin/lib/fs-driver-base').default;
const RNFetchBlob = require('rn-fetch-blob').default;
class FsDriverRN extends FsDriverBase {
@@ -1,15 +1,48 @@
const { filename, fileExtension } = require('./path-utils');
const time = require('./time').default;
const Setting = require('./models/Setting').default;
import time from './time';
import Setting from './models/Setting';
import { filename, fileExtension } from './path-utils';
const md5 = require('md5');
class FsDriverBase {
async isDirectory(path) {
export interface Stat {
birthtime: number;
mtime: number;
isDirectory(): boolean;
path: string;
size: number;
}
export interface ReadDirStatsOptions {
recursive: boolean;
}
export default class FsDriverBase {
public async stat(_path: string): Promise<Stat> {
throw new Error('Not implemented');
}
public async readDirStats(_path: string, _options: ReadDirStatsOptions = null): Promise<Stat[]> {
throw new Error('Not implemented');
}
public async exists(_path: string): Promise<boolean> {
throw new Error('Not implemented');
}
public async remove(_path: string): Promise<void> {
throw new Error('Not implemented');
}
public async isDirectory(path: string) {
const stat = await this.stat(path);
return !stat ? false : stat.isDirectory();
}
async readDirStatsHandleRecursion_(basePath, stat, output, options) {
public async writeFile(_path: string, _content: string, _encoding: string = 'base64'): Promise<void> {
throw new Error('Not implemented');
}
protected async readDirStatsHandleRecursion_(basePath: string, stat: Stat, output: Stat[], options: ReadDirStatsOptions): Promise<Stat[]> {
if (options.recursive && stat.isDirectory()) {
const subPath = `${basePath}/${stat.path}`;
const subStats = await this.readDirStats(subPath, options);
@@ -23,7 +56,7 @@ class FsDriverBase {
return output;
}
async findUniqueFilename(name, reservedNames = null) {
public async findUniqueFilename(name: string, reservedNames: string[] = null): Promise<string> {
if (reservedNames === null) {
reservedNames = [];
}
@@ -47,7 +80,7 @@ class FsDriverBase {
}
}
async removeAllThatStartWith(dirPath, filenameStart) {
public async removeAllThatStartWith(dirPath: string, filenameStart: string) {
if (!filenameStart || !dirPath) throw new Error('dirPath and filenameStart cannot be empty');
const stats = await this.readDirStats(dirPath);
@@ -59,7 +92,7 @@ class FsDriverBase {
}
}
async waitTillExists(path, timeout = 10000) {
public async waitTillExists(path: string, timeout: number = 10000) {
const startTime = Date.now();
while (true) {
@@ -72,7 +105,7 @@ class FsDriverBase {
// TODO: move out of here and make it part of joplin-renderer
// or assign to option using .bind(fsDriver())
async cacheCssToFile(cssStrings) {
public async cacheCssToFile(cssStrings: string[]) {
const cssString = Array.isArray(cssStrings) ? cssStrings.join('\n') : cssStrings;
const cssFilePath = `${Setting.value('tempDir')}/${md5(escape(cssString))}.css`;
if (!(await this.exists(cssFilePath))) {
@@ -84,6 +117,5 @@ class FsDriverBase {
mime: 'text/css',
};
}
}
module.exports = FsDriverBase;
}
+3 -3
View File
@@ -1,8 +1,8 @@
import { resolve as nodeResolve } from 'path';
import FsDriverBase, { Stat } from './fs-driver-base';
import time from './time';
const fs = require('fs-extra');
const time = require('./time').default;
const FsDriverBase = require('./fs-driver-base');
export default class FsDriverNode extends FsDriverBase {
@@ -125,7 +125,7 @@ export default class FsDriverNode extends FsDriverBase {
throw this.fsErrorToJsError_(error);
}
let output = [];
let output: Stat[] = [];
for (let i = 0; i < items.length; i++) {
const item = items[i];
const stat = await this.stat(`${path}/${item}`);
+16 -1
View File
@@ -171,7 +171,7 @@ class Note extends BaseItem {
useAbsolutePaths: false,
}, options);
const pathsToTry = [];
let pathsToTry = [];
if (options.useAbsolutePaths) {
pathsToTry.push(`file://${Setting.value('resourceDir')}`);
pathsToTry.push(`file://${shim.pathRelativeToCwd(Setting.value('resourceDir'))}`);
@@ -179,6 +179,21 @@ class Note extends BaseItem {
pathsToTry.push(Resource.baseRelativeDirectoryPath());
}
// We support both the escaped and unescaped versions because both
// of those paths are valid:
//
// [](file://C:/I like spaces in paths/abcdefg.jpg)
// [](file://C:/I%20like%20spaces%20in%20paths/abcdefg.jpg)
//
// https://discourse.joplinapp.org/t/12986/4
const temp = [];
for (const p of pathsToTry) {
temp.push(p);
temp.push(markdownUtils.escapeLinkUrl(p));
}
pathsToTry = temp;
this.logger().debug('replaceResourceExternalToInternalLinks', 'options:', options, 'pathsToTry:', pathsToTry);
for (const basePath of pathsToTry) {
@@ -1,11 +1,17 @@
import { _ } from '../../locale';
const InteropService_Exporter_Base = require('./InteropService_Exporter_Base').default;
const InteropService_Exporter_Raw = require('./InteropService_Exporter_Raw').default;
import InteropService_Exporter_Base from './InteropService_Exporter_Base';
import InteropService_Exporter_Raw from './InteropService_Exporter_Raw';
import shim from '../../shim';
const fs = require('fs-extra');
const shim = require('../../shim').default;
export default class InteropService_Exporter_Jex extends InteropService_Exporter_Base {
async init(destPath: string) {
private tempDir_: string;
private destPath_: string;
private rawExporter_: InteropService_Exporter_Raw;
public async init(destPath: string) {
if (await shim.fsDriver().isDirectory(destPath)) throw new Error(`Path is a directory: ${destPath}`);
this.tempDir_ = await this.temporaryDirectory_(false);
@@ -14,15 +20,15 @@ export default class InteropService_Exporter_Jex extends InteropService_Exporter
await this.rawExporter_.init(this.tempDir_);
}
async processItem(itemType: number, item: any) {
public async processItem(itemType: number, item: any) {
return this.rawExporter_.processItem(itemType, item);
}
async processResource(resource: any, filePath: string) {
public async processResource(resource: any, filePath: string) {
return this.rawExporter_.processResource(resource, filePath);
}
async close() {
public async close() {
const stats = await shim.fsDriver().readDirStats(this.tempDir_, { recursive: true });
const filePaths = stats.filter((a: any) => !a.isDirectory()).map((a: any) => a.path);