Desktop: Various improvements to Markdown import and export (#5290)

In preparation for #5224

.eslintignore

@@ -1248,6 +1248,8 @@ packages/lib/services/interop/InteropService_Importer_Jex.js.map
 packages/lib/services/interop/InteropService_Importer_Md.d.ts
 packages/lib/services/interop/InteropService_Importer_Md.js
 packages/lib/services/interop/InteropService_Importer_Md.js.map
+packages/lib/services/interop/InteropService_Importer_Md.test.js
+packages/lib/services/interop/InteropService_Importer_Md.test.js.map
 packages/lib/services/interop/InteropService_Importer_Raw.d.ts
 packages/lib/services/interop/InteropService_Importer_Raw.js
 packages/lib/services/interop/InteropService_Importer_Raw.js.map

.gitignore

@@ -1233,6 +1233,8 @@ packages/lib/services/interop/InteropService_Importer_Jex.js.map
 packages/lib/services/interop/InteropService_Importer_Md.d.ts
 packages/lib/services/interop/InteropService_Importer_Md.js
 packages/lib/services/interop/InteropService_Importer_Md.js.map
+packages/lib/services/interop/InteropService_Importer_Md.test.js
+packages/lib/services/interop/InteropService_Importer_Md.test.js.map
 packages/lib/services/interop/InteropService_Importer_Raw.d.ts
 packages/lib/services/interop/InteropService_Importer_Raw.js
 packages/lib/services/interop/InteropService_Importer_Raw.js.map

packages/app-cli/tests/MdToMd.ts

@@ -1,50 +0,0 @@
-const mdImporterService = require('@joplin/lib/services/interop/InteropService_Importer_Md').default;
-const Note = require('@joplin/lib/models/Note').default;
-import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils';
-const importer = new mdImporterService();
-
-
-describe('InteropService_Importer_Md: importLocalImages', function() {
-	beforeEach(async (done) => {
-		await setupDatabaseAndSynchronizer(1);
-		await switchClient(1);
-		done();
-	});
-	it('should import linked files and modify tags appropriately', async function() {
-		const tagNonExistentFile = '![does not exist](does_not_exist.png)';
-		const note = await importer.importFile(`${__dirname}/md_to_md/sample.md`, 'notebook');
-		const items = await Note.linkedItems(note.body);
-		expect(items.length).toBe(2);
-		const inexistentLinkUnchanged = note.body.includes(tagNonExistentFile);
-		expect(inexistentLinkUnchanged).toBe(true);
-	});
-	it('should only create 1 resource for duplicate links, all tags should be updated', async function() {
-		const note = await importer.importFile(`${__dirname}/md_to_md/sample-duplicate-links.md`, 'notebook');
-		const items = await Note.linkedItems(note.body);
-		expect(items.length).toBe(1);
-		const reg = new RegExp(items[0].id, 'g');
-		const matched = note.body.match(reg);
-		expect(matched.length).toBe(2);
-	});
-	it('should import linked files and modify tags appropriately when link is also in alt text', async function() {
-		const note = await importer.importFile(`${__dirname}/md_to_md/sample-link-in-alt-text.md`, 'notebook');
-		const items = await Note.linkedItems(note.body);
-		expect(items.length).toBe(1);
-	});
-	it('should passthrough unchanged if no links present', async function() {
-		const note = await importer.importFile(`${__dirname}/md_to_md/sample-no-links.md`, 'notebook');
-		const items = await Note.linkedItems(note.body);
-		expect(items.length).toBe(0);
-		expect(note.body).toContain('Unidentified vessel travelling at sub warp speed, bearing 235.7. Fluctuations in energy readings from it, Captain. All transporters off.');
-	});
-	it('should import linked image with special characters in name', async function() {
-		const note = await importer.importFile(`${__dirname}/md_to_md/sample-special-chars.md`, 'notebook');
-		const items = await Note.linkedItems(note.body);
-		expect(items.length).toBe(1);
-	});
-	it('should import resources for files', async function() {
-		const note = await importer.importFile(`${__dirname}/md_to_md/sample-files.md`, 'notebook');
-		const items = await Note.linkedItems(note.body);
-		expect(items.length).toBe(4);
-	});
-});

packages/app-cli/tests/md_to_md/sample-duplicate-links.md

@@ -1,2 +0,0 @@
-![link 1](../support/photo.jpg)
-![link 2](../support/photo.jpg)

packages/app-cli/tests/md_to_md/sample-files.md

@@ -1,9 +0,0 @@
-# Markdown file test
-
-![../support/photo.jpg](../support/photo.jpg)
-
-[welcome.pdf](../support/welcome.pdf)
-
-[sample.md](sample.md)
-
-[sample2.md](./sample.md)

packages/app-cli/tests/md_to_md/sample-link-in-alt-text.md

@@ -1,3 +0,0 @@
-# Markdown
-![../support/photo.jpg](../support/photo.jpg) should put resource link inside () not []
-![../support/photo.jpg](     ../support/photo.jpg   ) this case (spaces before/after link but within parens) is not currently covered

packages/app-cli/tests/md_to_md/sample-special-chars.md

@@ -1 +0,0 @@
-![link special chars](../support/photo-åäö.jpg)

packages/app-cli/tests/md_to_md/sample.md

@@ -1,13 +0,0 @@
-# Markdown
-
-lorem ipsum ![alt text here](../support/photo.jpg)
-- [ ] check!
-- [ ] boxes!
-
-![alt text here](../support/photo-two.jpg)ipsum lorem
-
-**strong text**
-![does not exist](does_not_exist.png) lorem ipsum
-
-**some directory**
-![a directory](../support) lorem ipsum

packages/app-cli/tests/support/test_notes/md/sample spaces.md

@@ -0,0 +1,3 @@
+# Test Spaces
+
+I hope this get's imported correctly!

packages/app-cli/tests/support/test_notes/md/sample-anchor-link.md

@@ -0,0 +1 @@
+[Section 1](./sample-no-links.md#markdown)

packages/app-cli/tests/support/test_notes/md/sample-cycles-a.md

@@ -0,0 +1,3 @@
+# Markdown file test
+
+[sample.md](sample-cycles-b.md)

packages/app-cli/tests/support/test_notes/md/sample-cycles-b.md

@@ -0,0 +1,4 @@
+# Markdown file test
+
+
+[sample.md](./sample-cycles-a.md)

packages/app-cli/tests/support/test_notes/md/sample-duplicate-links.md

@@ -0,0 +1,2 @@
+![link 1](../../photo.jpg)
+![link 2](../../photo.jpg)

packages/app-cli/tests/support/test_notes/md/sample-file-links.md

@@ -0,0 +1 @@
+![sample](file://../../photo.jpg)

packages/app-cli/tests/support/test_notes/md/sample-files.md

@@ -0,0 +1,9 @@
+# Markdown file test
+
+![../../photo.jpg](../../photo.jpg)
+
+[welcome.pdf](../../welcome.pdf)
+
+[sample.md](sample.md)
+
+[sample2.md](./sample.md)

packages/app-cli/tests/support/test_notes/md/sample-link-in-alt-text.md

@@ -0,0 +1,3 @@
+# Markdown
+![../../photo.jpg](../../photo.jpg) should put resource link inside () not []
+![../../photo.jpg](     ../../photo.jpg   ) this case (spaces before/after link but within parens) is not currently covered

packages/app-cli/tests/support/test_notes/md/sample-link-title.md

@@ -0,0 +1,3 @@
+![Alt text](../../photo.jpg "Scott Joplin")
+![Worst Case](<../../photo sample.jpg> "title")
+[Worst Case](<./sample spaces.md> "title")

packages/app-cli/tests/support/test_notes/md/sample-md

@@ -0,0 +1 @@
+I am here, but am I alive?

packages/app-cli/tests/support/test_notes/md/sample-no-extension.md

@@ -0,0 +1,3 @@
+# Some Title
+
+[link](./sample-md)

packages/app-cli/tests/support/test_notes/md/sample-special-chars.md

@@ -0,0 +1,4 @@
+![link special chars](../../photo-åäö.jpg)
+[sample photo](../../photo%20sample.jpg)
+[sample.md](./sample%20spaces.md)
+[sample special syntax](<../../photo sample.jpg>)

packages/app-cli/tests/support/test_notes/md/sample.html

@@ -0,0 +1,4 @@
+<img src="../../photo.jpg">
+<img src='../../photo-two.jpg'>
+<img src='does-not-exist' alt="../../photo.jpg">
+<a href="./sample-no-links.md">

packages/app-cli/tests/support/test_notes/md/sample.md

@@ -0,0 +1,13 @@
+# Markdown
+
+lorem ipsum ![alt text here](../../photo.jpg)
+- [ ] check!
+- [ ] boxes!
+
+![alt text here](../../photo-two.jpg)ipsum lorem
+
+**strong text**
+![does not exist](does_not_exist.png) lorem ipsum
+
+**some directory**
+![a directory](../..) lorem ipsum

packages/lib/fs-driver-base.ts

@@ -56,7 +56,7 @@ export default class FsDriverBase {
 		return output;
 	}
 
-	public async findUniqueFilename(name: string, reservedNames: string[] = null): Promise<string> {
+	public async findUniqueFilename(name: string, reservedNames: string[] = null, markdownSafe: boolean = false): Promise<string> {
 		if (reservedNames === null) {
 			reservedNames = [];
 		}
@@ -70,7 +70,11 @@ export default class FsDriverBase {
 			// Check if the filename does not exist in the filesystem and is not reserved
 			const exists = await this.exists(nameToTry) || reservedNames.includes(nameToTry);
 			if (!exists) return nameToTry;
+			if (!markdownSafe) {
 				nameToTry = `${nameNoExt} (${counter})${extension}`;
+			} else {
+				nameToTry = `${nameNoExt}-${counter}${extension}`;
+			}
 			counter++;
 			if (counter >= 1000) {
 				nameToTry = `${nameNoExt} (${new Date().getTime()})${extension}`;

packages/lib/htmlUtils.ts

@@ -45,18 +45,39 @@ class HtmlUtils {
 	}
 
 	// Returns the **encoded** URLs, so to be useful they should be decoded again before use.
-	public extractImageUrls(html: string) {
+	private extractUrls(regex: RegExp, html: string) {
 		if (!html) return [];
 
 		const output = [];
 		let matches;
-		while ((matches = imageRegex.exec(html))) {
+		while ((matches = regex.exec(html))) {
 			output.push(matches[2]);
 		}
 
 		return output.filter(url => !!url);
 	}
 
+	// Returns the **encoded** URLs, so to be useful they should be decoded again before use.
+	public extractImageUrls(html: string) {
+		return this.extractUrls(imageRegex, html);
+	}
+
+	// Returns the **encoded** URLs, so to be useful they should be decoded again before use.
+	public extractAnchorUrls(html: string) {
+		return this.extractUrls(anchorRegex, html);
+	}
+
+	// Returns the **encoded** URLs, so to be useful they should be decoded again before use.
+	public extractFileUrls(html: string) {
+		return this.extractImageUrls(html).concat(this.extractAnchorUrls(html));
+	}
+
+	public replaceResourceUrl(html: string, urlToReplace: string, id: string) {
+		const htmlLinkRegex = `(?<=(?:src|href)=["'])${urlToReplace}(?=["'])`;
+		const htmlReg = new RegExp(htmlLinkRegex, 'g');
+		return html.replace(htmlReg, `:/${id}`);
+	}
+
 	public replaceImageUrls(html: string, callback: Function) {
 		return this.processImageTags(html, (data: any) => {
 			const newSrc = callback(data.src);

packages/lib/markdownUtils.ts

@@ -100,6 +100,12 @@ const markdownUtils = {
 		return output;
 	},
 
+	replaceResourceUrl(md: string, urlToReplace: string, id: string) {
+		const linkRegex = `(?<=\\]\\()\\<?${urlToReplace}\\>?(?=.*\\))`;
+		const reg = new RegExp(linkRegex, 'g');
+		return md.replace(reg, `:/${id}`);
+	},
+
 	extractImageUrls(md: string) {
 		return markdownUtils.extractFileUrls(md,true);
 	},

packages/lib/services/interop/InteropService.test.ts

@@ -442,10 +442,10 @@ describe('services_InteropService', function() {
 		await service.export({ path: outDir, format: 'md' });
 
 		expect(await shim.fsDriver().exists(`${outDir}/folder1/生活.md`)).toBe(true);
-		expect(await shim.fsDriver().exists(`${outDir}/folder1/生活 (1).md`)).toBe(true);
-		expect(await shim.fsDriver().exists(`${outDir}/folder1/生活 (2).md`)).toBe(true);
+		expect(await shim.fsDriver().exists(`${outDir}/folder1/生活-1.md`)).toBe(true);
+		expect(await shim.fsDriver().exists(`${outDir}/folder1/生活-2.md`)).toBe(true);
 		expect(await shim.fsDriver().exists(`${outDir}/folder1/Untitled.md`)).toBe(true);
-		expect(await shim.fsDriver().exists(`${outDir}/folder1/Untitled (1).md`)).toBe(true);
+		expect(await shim.fsDriver().exists(`${outDir}/folder1/Untitled-1.md`)).toBe(true);
 		expect(await shim.fsDriver().exists(`${outDir}/folder1/salut, ça roule _.md`)).toBe(true);
 		expect(await shim.fsDriver().exists(`${outDir}/ジョプリン/ジョプリン.md`)).toBe(true);
 	}));

packages/lib/services/interop/InteropService.ts

@@ -53,7 +53,7 @@ export default class InteropService {
 				{
 					...defaultImportExportModule(ModuleType.Importer),
 					format: 'md',
-					fileExtensions: ['md', 'markdown', 'txt'],
+					fileExtensions: ['md', 'markdown', 'txt', 'html'],
 					sources: [FileSystemItem.File, FileSystemItem.Directory],
 					isNoteArchive: false, // Tells whether the file can contain multiple notes (eg. Enex or Jex format)
 					description: _('Markdown'),
@@ -401,10 +401,16 @@ export default class InteropService {
 			resourcePaths: {},
 		};
 
+		// Prepare to process each type before starting any
+		// This will allow exporters to operate on the full context
 		for (let typeOrderIndex = 0; typeOrderIndex < typeOrder.length; typeOrderIndex++) {
 			const type = typeOrder[typeOrderIndex];
 
 			await exporter.prepareForProcessingItemType(type, itemsToExport);
+		}
+
+		for (let typeOrderIndex = 0; typeOrderIndex < typeOrder.length; typeOrderIndex++) {
+			const type = typeOrder[typeOrderIndex];
 
 			for (let i = 0; i < itemsToExport.length; i++) {
 				const itemType = itemsToExport[i].type;

packages/lib/services/interop/InteropService_Exporter_Md.test.js

@@ -9,6 +9,7 @@ const Folder = require('../../models/Folder').default;
 const Resource = require('../../models/Resource').default;
 const Note = require('../../models/Note').default;
 const shim = require('../../shim').default;
+const { MarkupToHtml } = require('@joplin/renderer');
 
 describe('interop/InteropService_Exporter_Md', function() {
 
@@ -51,7 +52,7 @@ describe('interop/InteropService_Exporter_Md', function() {
 		queueExportItem(BaseModel.TYPE_RESOURCE, (await Note.linkedResourceIds(note1.body))[0]);
 
 		const folder2 = await Folder.save({ title: 'folder2' });
-		let note3 = await Note.save({ title: 'note3', parent_id: folder2.id });
+		let note3 = await Note.save({ title: 'note3', parent_id: folder2.id, markup_language: MarkupToHtml.MARKUP_LANGUAGE_HTML });
 		await shim.attachFileToNote(note3, `${supportDir}/photo.jpg`);
 		note3 = await Note.load(note3.id);
 		queueExportItem(BaseModel.TYPE_FOLDER, folder2.id);
@@ -67,7 +68,53 @@ describe('interop/InteropService_Exporter_Md', function() {
 		expect(Object.keys(exporter.context().notePaths).length).toBe(3, 'There should be 3 note paths in the context.');
 		expect(exporter.context().notePaths[note1.id]).toBe('folder1/note1.md');
 		expect(exporter.context().notePaths[note2.id]).toBe('folder1/note2.md');
-		expect(exporter.context().notePaths[note3.id]).toBe('folder2/note3.md');
+		expect(exporter.context().notePaths[note3.id]).toBe('folder2/note3.html');
+	}));
+
+	it('should create resource paths and add them to context', (async () => {
+		const exporter = new InteropService_Exporter_Md();
+		await exporter.init(exportDir());
+
+		const itemsToExport = [];
+		const queueExportItem = (itemType, itemOrId) => {
+			itemsToExport.push({
+				type: itemType,
+				itemOrId: itemOrId,
+			});
+		};
+
+		const folder1 = await Folder.save({ title: 'folder1' });
+		let note1 = await Note.save({ title: 'note1', parent_id: folder1.id });
+		const note2 = await Note.save({ title: 'note2', parent_id: folder1.id });
+		await shim.attachFileToNote(note1, `${supportDir}/photo.jpg`);
+		note1 = await Note.load(note1.id);
+		queueExportItem(BaseModel.TYPE_FOLDER, folder1.id);
+		queueExportItem(BaseModel.TYPE_NOTE, note1);
+		queueExportItem(BaseModel.TYPE_NOTE, note2);
+		queueExportItem(BaseModel.TYPE_RESOURCE, (await Note.linkedResourceIds(note1.body))[0]);
+		const resource1 = await Resource.load(itemsToExport[3].itemOrId);
+
+		const folder2 = await Folder.save({ title: 'folder2' });
+		let note3 = await Note.save({ title: 'note3', parent_id: folder2.id });
+		await shim.attachFileToNote(note3, `${supportDir}/photo.jpg`);
+		note3 = await Note.load(note3.id);
+		queueExportItem(BaseModel.TYPE_FOLDER, folder2.id);
+		queueExportItem(BaseModel.TYPE_NOTE, note3);
+		queueExportItem(BaseModel.TYPE_RESOURCE, (await Note.linkedResourceIds(note3.body))[0]);
+		const resource2 = await Resource.load(itemsToExport[6].itemOrId);
+
+		await exporter.processItem(Folder.modelType(), folder1);
+		await exporter.processItem(Folder.modelType(), folder2);
+		await exporter.prepareForProcessingItemType(BaseModel.TYPE_NOTE, itemsToExport);
+
+		await exporter.processResource(resource1, Resource.fullPath(resource1));
+		await exporter.processResource(resource2, Resource.fullPath(resource2));
+
+		expect(!exporter.context() && !(exporter.context().destResourcePaths || Object.keys(exporter.context().destResourcePaths).length)).toBe(false, 'Context should be empty before processing.');
+
+		expect(Object.keys(exporter.context().destResourcePaths).length).toBe(2, 'There should be 2 resource paths in the context.');
+		expect(exporter.context().destResourcePaths[resource1.id]).toBe(`${exportDir()}/_resources/photo.jpg`);
+		expect(exporter.context().destResourcePaths[resource2.id]).toBe(`${exportDir()}/_resources/photo-1.jpg`);
 	}));
 
 	it('should handle duplicate note names', (async () => {
@@ -94,7 +141,7 @@ describe('interop/InteropService_Exporter_Md', function() {
 
 		expect(Object.keys(exporter.context().notePaths).length).toBe(2, 'There should be 2 note paths in the context.');
 		expect(exporter.context().notePaths[note1.id]).toBe('folder1/note1.md');
-		expect(exporter.context().notePaths[note1_2.id]).toBe('folder1/note1 (1).md');
+		expect(exporter.context().notePaths[note1_2.id]).toBe('folder1/note1-1.md');
 	}));
 
 	it('should not override existing files', (async () => {
@@ -121,7 +168,7 @@ describe('interop/InteropService_Exporter_Md', function() {
 		await exporter.prepareForProcessingItemType(BaseModel.TYPE_NOTE, itemsToExport);
 
 		expect(Object.keys(exporter.context().notePaths).length).toBe(1, 'There should be 1 note paths in the context.');
-		expect(exporter.context().notePaths[note1.id]).toBe('folder1/note1 (1).md');
+		expect(exporter.context().notePaths[note1.id]).toBe('folder1/note1-1.md');
 	}));
 
 	it('should save resource files in _resource directory', (async () => {
@@ -157,8 +204,8 @@ describe('interop/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/photo.jpg`)).toBe(true, 'Resource file should be copied to _resources directory.');
+		expect(await shim.fsDriver().exists(`${exportDir()}/_resources/photo-1.jpg`)).toBe(true, 'Resource file should be copied to _resources directory.');
 	}));
 
 	it('should create folders in fs', (async () => {
@@ -255,23 +302,51 @@ describe('interop/InteropService_Exporter_Md', function() {
 		queueExportItem(BaseModel.TYPE_NOTE, note2);
 		const resource2 = await Resource.load((await Note.linkedResourceIds(note2.body))[0]);
 
+		let note3 = await Note.save({ title: 'note3', parent_id: folder2.id });
+		await shim.attachFileToNote(note3, `${supportDir}/photo.jpg`);
+		note3 = await Note.load(note3.id);
+		queueExportItem(BaseModel.TYPE_NOTE, note3);
+		const resource3 = await Resource.load((await Note.linkedResourceIds(note3.body))[0]);
+		note3 = await Note.save({ ...note3, body: `<img src=":/${resource3.id}" alt="alt">` });
+		note3 = await Note.load(note3.id);
+
+		let note4 = await Note.save({ title: 'note4', parent_id: folder2.id });
+		await shim.attachFileToNote(note4, `${supportDir}/photo.jpg`);
+		note4 = await Note.load(note4.id);
+		queueExportItem(BaseModel.TYPE_NOTE, note4);
+		const resource4 = await Resource.load((await Note.linkedResourceIds(note4.body))[0]);
+		note4 = await Note.save({ ...note4, body: `![](:/${resource4.id} "title")` });
+		note4 = await Note.load(note4.id);
+
 		await exporter.processItem(Folder.modelType(), folder1);
 		await exporter.processItem(Folder.modelType(), folder2);
 		await exporter.prepareForProcessingItemType(BaseModel.TYPE_NOTE, itemsToExport);
+		await exporter.processResource(resource1, Resource.fullPath(resource1));
+		await exporter.processResource(resource2, Resource.fullPath(resource2));
+		await exporter.processResource(resource3, Resource.fullPath(resource3));
+		await exporter.processResource(resource4, Resource.fullPath(resource3));
 		const context = {
 			resourcePaths: {},
 		};
 		context.resourcePaths[resource1.id] = 'resource1.jpg';
 		context.resourcePaths[resource2.id] = 'resource2.jpg';
+		context.resourcePaths[resource3.id] = 'resource3.jpg';
+		context.resourcePaths[resource4.id] = 'resource3.jpg';
 		exporter.updateContext(context);
 		await exporter.processItem(Note.modelType(), note1);
 		await exporter.processItem(Note.modelType(), note2);
+		await exporter.processItem(Note.modelType(), note3);
+		await exporter.processItem(Note.modelType(), note4);
 
 		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 note4_body = await shim.fsDriver().readFile(`${exportDir()}/${exporter.context().notePaths[note4.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.');
+		expect(note1_body).toContain('](../_resources/photo.jpg)', 'Resource id should be replaced with a relative path.');
+		expect(note2_body).toContain('](../../_resources/photo-1.jpg)', 'Resource id should be replaced with a relative path.');
+		expect(note3_body).toContain('<img src="../../_resources/photo-2.jpg" alt="alt">', 'Resource id should be replaced with a relative path.');
+		expect(note4_body).toContain('](../../_resources/photo-3.jpg "title")', 'Resource id should be replaced with a relative path.');
 	}));
 
 	it('should replace note ids with relative paths', (async () => {

packages/lib/services/interop/InteropService_Exporter_Md.ts

@@ -4,7 +4,9 @@ import shim from '../../shim';
 import markdownUtils from '../../markdownUtils';
 import Folder from '../../models/Folder';
 import Note from '../../models/Note';
+import { NoteEntity, ResourceEntity } from '../database/types';
 import { basename, dirname, friendlySafeFilename } from '../../path-utils';
+import { MarkupToHtml } from '@joplin/renderer';
 
 export default class InteropService_Exporter_Md extends InteropService_Exporter_Base {
 
@@ -29,7 +31,7 @@ export default class InteropService_Exporter_Md extends InteropService_Exporter_
 					output = `${pathPart}/${output}`;
 				} else {
 					output = `${friendlySafeFilename(item.title, null)}/${output}`;
-					if (findUniqueFilename) output = await shim.fsDriver().findUniqueFilename(output);
+					if (findUniqueFilename) output = await shim.fsDriver().findUniqueFilename(output, null, true);
 				}
 			}
 			if (!item.parent_id) return output;
@@ -46,7 +48,7 @@ export default class InteropService_Exporter_Md extends InteropService_Exporter_
 
 	async replaceResourceIdsByRelativePaths_(noteBody: string, relativePathToRoot: string) {
 		const linkedResourceIds = await Note.linkedResourceIds(noteBody);
-		const resourcePaths = this.context() && this.context().resourcePaths ? this.context().resourcePaths : {};
+		const resourcePaths = this.context() && this.context().destResourcePaths ? this.context().destResourcePaths : {};
 
 		const createRelativePath = function(resourcePath: string) {
 			return `${relativePathToRoot}_resources/${basename(resourcePath)}`;
@@ -83,17 +85,18 @@ export default class InteropService_Exporter_Md extends InteropService_Exporter_
 				notePaths: {},
 			};
 			for (let i = 0; i < itemsToExport.length; i++) {
-				const itemType = itemsToExport[i].type;
+				const it = itemsToExport[i].type;
 
-				if (itemType !== itemType) continue;
+				if (it !== itemType) continue;
 
 				const itemOrId = itemsToExport[i].itemOrId;
 				const note = typeof itemOrId === 'object' ? itemOrId : await Note.load(itemOrId);
 
 				if (!note) continue;
 
-				let notePath = `${await this.makeDirPath_(note, null, false)}${friendlySafeFilename(note.title, null)}.md`;
-				notePath = await shim.fsDriver().findUniqueFilename(`${this.destDir_}/${notePath}`, Object.values(context.notePaths));
+				const ext = note.markup_language === MarkupToHtml.MARKUP_LANGUAGE_HTML ? 'html' : 'md';
+				let notePath = `${await this.makeDirPath_(note, null, false)}${friendlySafeFilename(note.title, null)}.${ext}`;
+				notePath = await shim.fsDriver().findUniqueFilename(`${this.destDir_}/${notePath}`, Object.values(context.notePaths), true);
 				context.notePaths[note.id] = notePath;
 			}
 
@@ -107,6 +110,10 @@ export default class InteropService_Exporter_Md extends InteropService_Exporter_
 		}
 	}
 
+	private async getNoteExportContent_(modNote: NoteEntity) {
+		return await Note.replaceResourceInternalToExternalLinks(await Note.serialize(modNote, ['body']));
+	}
+
 	async processItem(_itemType: number, item: any) {
 		if ([BaseModel.TYPE_NOTE, BaseModel.TYPE_FOLDER].indexOf(item.type_) < 0) return;
 
@@ -124,15 +131,36 @@ export default class InteropService_Exporter_Md extends InteropService_Exporter_
 
 			const noteBody = await this.relaceLinkedItemIdsByRelativePaths_(item);
 			const modNote = Object.assign({}, item, { body: noteBody });
-			const noteContent = await Note.serializeForEdit(modNote);
+			const noteContent = await this.getNoteExportContent_(modNote);
 			await shim.fsDriver().mkdir(dirname(noteFilePath));
 			await shim.fsDriver().writeFile(noteFilePath, noteContent, 'utf-8');
 		}
 	}
 
-	async processResource(_resource: any, filePath: string) {
-		const destResourcePath = `${this.resourceDir_}/${basename(filePath)}`;
+	private async findReasonableFilename(resource: ResourceEntity, filePath: string) {
+		let fileName = basename(filePath);
+
+		if (resource.filename) {
+			fileName = resource.filename;
+		} else if (resource.title) {
+			fileName = friendlySafeFilename(resource.title);
+		}
+
+		// Fall back on the resource filename saved in the users resource folder
+		return fileName;
+	}
+
+	async processResource(resource: ResourceEntity, filePath: string) {
+		const context = this.context();
+		if (!context.destResourcePaths) context.destResourcePaths = {};
+
+		const fileName = await this.findReasonableFilename(resource, filePath);
+		let destResourcePath = `${this.resourceDir_}/${fileName}`;
+		destResourcePath = await shim.fsDriver().findUniqueFilename(destResourcePath, Object.values(context.destResourcePaths), true);
 		await shim.fsDriver().copy(filePath, destResourcePath);
+
+		context.destResourcePaths[resource.id] = destResourcePath;
+		this.updateContext(context);
 	}
 
 	async close() {}

packages/lib/services/interop/InteropService_Importer_Md.test.ts

@@ -0,0 +1,120 @@
+import InteropService_Importer_Md from '../../services/interop/InteropService_Importer_Md';
+import Note from '../../models/Note';
+import { setupDatabaseAndSynchronizer, supportDir, switchClient } from '../../testing/test-utils';
+import { MarkupToHtml } from '@joplin/renderer';
+
+
+describe('InteropService_Importer_Md: importLocalImages', function() {
+	async function importNote(path: string) {
+		const importer = new InteropService_Importer_Md();
+		importer.setMetadata({ fileExtensions: ['md', 'html'] });
+		return await importer.importFile(path, 'notebook');
+	}
+
+	beforeEach(async (done) => {
+		await setupDatabaseAndSynchronizer(1);
+		await switchClient(1);
+		done();
+	});
+	it('should import linked files and modify tags appropriately', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample.md`);
+
+		const tagNonExistentFile = '![does not exist](does_not_exist.png)';
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(2);
+		const inexistentLinkUnchanged = note.body.includes(tagNonExistentFile);
+		expect(inexistentLinkUnchanged).toBe(true);
+	});
+	it('should only create 1 resource for duplicate links, all tags should be updated', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-duplicate-links.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(1);
+		const reg = new RegExp(items[0].id, 'g');
+		const matched = note.body.match(reg);
+		expect(matched.length).toBe(2);
+	});
+	it('should import linked files and modify tags appropriately when link is also in alt text', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-link-in-alt-text.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(1);
+	});
+	it('should passthrough unchanged if no links present', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-no-links.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(0);
+		expect(note.body).toContain('Unidentified vessel travelling at sub warp speed, bearing 235.7. Fluctuations in energy readings from it, Captain. All transporters off.');
+	});
+	it('should import linked image with special characters in name', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-special-chars.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(3);
+		const noteIds = await Note.linkedNoteIds(note.body);
+		expect(noteIds.length).toBe(1);
+		const spaceSyntaxLeft = note.body.includes('<../../photo sample.jpg>');
+		expect(spaceSyntaxLeft).toBe(false);
+	});
+	it('should import resources and notes for files', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-files.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(3);
+		const noteIds = await Note.linkedNoteIds(note.body);
+		expect(noteIds.length).toBe(1);
+	});
+	it('should gracefully handle reference cycles in notes', async function() {
+		const importer = new InteropService_Importer_Md();
+		importer.setMetadata({ fileExtensions: ['md'] });
+		const noteA = await importer.importFile(`${supportDir}/test_notes/md/sample-cycles-a.md`, 'notebook');
+		const noteB = await importer.importFile(`${supportDir}/test_notes/md/sample-cycles-b.md`, 'notebook');
+
+		const noteAIds = await Note.linkedNoteIds(noteA.body);
+		expect(noteAIds.length).toBe(1);
+		const noteBIds = await Note.linkedNoteIds(noteB.body);
+		expect(noteBIds.length).toBe(1);
+		expect(noteAIds[0]).toEqual(noteB.id);
+		expect(noteBIds[0]).toEqual(noteA.id);
+	});
+	it('should not import resources from file:// links', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-file-links.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(0);
+		expect(note.body).toContain('![sample](file://../../photo.jpg)');
+	});
+	it('should attach resources that are missing the file extension', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-no-extension.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(1);
+	});
+	it('should attach resources that include anchor links', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-anchor-link.md`);
+
+		const itemIds = await Note.linkedItemIds(note.body);
+		expect(itemIds.length).toBe(1);
+		expect(note.body).toContain(`[Section 1](:/${itemIds[0]}#markdown)`);
+	});
+	it('should attach resources that include a title', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample-link-title.md`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(3);
+		const noteIds = await Note.linkedNoteIds(note.body);
+		expect(noteIds.length).toBe(1);
+	});
+	it('should import notes with html file extension as html', async function() {
+		const note = await importNote(`${supportDir}/test_notes/md/sample.html`);
+
+		const items = await Note.linkedItems(note.body);
+		expect(items.length).toBe(3);
+		const noteIds = await Note.linkedNoteIds(note.body);
+		expect(noteIds.length).toBe(1);
+		expect(note.markup_language).toBe(MarkupToHtml.MARKUP_LANGUAGE_HTML);
+		const preservedAlt = note.body.includes('alt="../../photo.jpg"');
+		expect(preservedAlt).toBe(true);
+	});
+});

packages/lib/services/interop/InteropService_Importer_Md.ts

@@ -4,14 +4,18 @@ import { _ } from '../../locale';
 import InteropService_Importer_Base from './InteropService_Importer_Base';
 import Folder from '../../models/Folder';
 import Note from '../../models/Note';
-const { basename, filename, rtrimSlashes, fileExtension, dirname } = require('../../path-utils');
+import { NoteEntity } from '../database/types';
+import { basename, filename, rtrimSlashes, fileExtension, dirname } from '../../path-utils';
 import shim from '../../shim';
 import markdownUtils from '../../markdownUtils';
+import htmlUtils from '../../htmlUtils';
 const { unique } = require('../../ArrayUtils');
 const { pregQuote } = require('../../string-utils-common');
-const { MarkupToHtml } = require('@joplin/renderer');
+import { MarkupToHtml } from '@joplin/renderer';
 
 export default class InteropService_Importer_Md extends InteropService_Importer_Base {
+	private importedNotes: Record<string, NoteEntity> = {};
+
 	async exec(result: ImportExportResult) {
 		let parentFolderId = null;
 
@@ -59,52 +63,100 @@ export default class InteropService_Importer_Md extends InteropService_Importer_
 		}
 	}
 
+	private trimAnchorLink(link: string) {
+		if (link.indexOf('#') <= 0) return link;
+
+		const splitted = link.split('#');
+		splitted.pop();
+		return splitted.join('#');
+	}
+
 	/**
 	 * Parse text for links, attempt to find local file, if found create Joplin resource
 	 * and update link accordingly.
 	 */
-	async importLocalFiles(filePath: string, md: string) {
+	async importLocalFiles(filePath: string, md: string, parentFolderId: string) {
 		let updated = md;
-		const fileLinks = unique(markdownUtils.extractFileUrls(md));
+		const markdownLinks = markdownUtils.extractFileUrls(md);
+		const htmlLinks = htmlUtils.extractFileUrls(md);
+		const fileLinks = unique(markdownLinks.concat(htmlLinks));
 		await Promise.all(fileLinks.map(async (encodedLink: string) => {
 			const link = decodeURI(encodedLink);
-			const attachmentPath = filename(`${dirname(filePath)}/${link}`, true);
-			const pathWithExtension = `${attachmentPath}.${fileExtension(link)}`;
+			// Handle anchor links appropriately
+			const trimmedLink = this.trimAnchorLink(link);
+			const attachmentPath = filename(`${dirname(filePath)}/${trimmedLink}`, true);
+			const pathWithExtension = `${attachmentPath}.${fileExtension(trimmedLink)}`;
 			const stat = await shim.fsDriver().stat(pathWithExtension);
 			const isDir = stat ? stat.isDirectory() : false;
 			if (stat && !isDir) {
+				const supportedFileExtension = this.metadata().fileExtensions;
+				const resolvedPath = shim.fsDriver().resolve(pathWithExtension);
+				let id: string = '';
+				// If the link looks like a note, then import it
+				if (supportedFileExtension.indexOf(fileExtension(trimmedLink).toLowerCase()) >= 0) {
+					// If the note hasn't been imported yet, do so now
+					if (!this.importedNotes[resolvedPath]) {
+						await this.importFile(resolvedPath, parentFolderId);
+					}
+
+					id = this.importedNotes[resolvedPath].id;
+				} else {
 					const resource = await shim.createResourceFromPath(pathWithExtension);
-				// NOTE: use ](link) in case the link also appears elsewhere, such as in alt text
-				const linkPatternEscaped = pregQuote(`](${link})`);
-				const reg = new RegExp(linkPatternEscaped, 'g');
-				updated = updated.replace(reg, `](:/${resource.id})`);
+					id = resource.id;
+				}
+
+				// The first is a normal link, the second is supports the <link> and [](<link with spaces>) syntax
+				// Only opening patterns are consider in order to cover all occurances
+				// We need to use the encoded link as well because some links (link's with spaces)
+				// will appear encoded in the source. Other links (unicode chars) will not
+				const linksToReplace = [this.trimAnchorLink(link), this.trimAnchorLink(encodedLink)];
+
+				for (let j = 0; j < linksToReplace.length; j++) {
+					const linkToReplace = pregQuote(linksToReplace[j]);
+
+					// Markdown links
+					updated = markdownUtils.replaceResourceUrl(updated, linkToReplace, id);
+
+					// HTML links
+					updated = htmlUtils.replaceResourceUrl(updated, linkToReplace, id);
+				}
 			}
 		}));
 		return updated;
 	}
 
 	async importFile(filePath: string, parentFolderId: string) {
-		const stat = await shim.fsDriver().stat(filePath);
-		if (!stat) throw new Error(`Cannot read ${filePath}`);
-		const title = filename(filePath);
-		const body = await shim.fsDriver().readFile(filePath);
-		let updatedBody;
-		try {
-			updatedBody = await this.importLocalFiles(filePath, body);
-		} catch (error) {
-			// console.error(`Problem importing links for file ${filePath}, error:\n ${error}`);
-		}
+		const resolvedPath = shim.fsDriver().resolve(filePath);
+		if (this.importedNotes[resolvedPath]) return this.importedNotes[resolvedPath];
+
+		const stat = await shim.fsDriver().stat(resolvedPath);
+		if (!stat) throw new Error(`Cannot read ${resolvedPath}`);
+		const ext = fileExtension(resolvedPath);
+		const title = filename(resolvedPath);
+		const body = await shim.fsDriver().readFile(resolvedPath);
 		const note = {
 			parent_id: parentFolderId,
 			title: title,
-			body: updatedBody || body,
+			body: body,
 			updated_time: stat.mtime.getTime(),
 			created_time: stat.birthtime.getTime(),
 			user_updated_time: stat.mtime.getTime(),
 			user_created_time: stat.birthtime.getTime(),
-			markup_language: MarkupToHtml.MARKUP_LANGUAGE_MARKDOWN,
+			markup_language: ext === 'html' ? MarkupToHtml.MARKUP_LANGUAGE_HTML : MarkupToHtml.MARKUP_LANGUAGE_MARKDOWN,
 		};
+		this.importedNotes[resolvedPath] = await Note.save(note, { autoTimestamp: false });
 
-		return Note.save(note, { autoTimestamp: false });
+		try {
+			const updatedBody = await this.importLocalFiles(resolvedPath, body, parentFolderId);
+			const updatedNote = {
+				...this.importedNotes[resolvedPath],
+				body: updatedBody || body,
+			};
+			this.importedNotes[resolvedPath] = await Note.save(updatedNote, { isNew: false });
+		} catch (error) {
+			// console.error(`Problem importing links for file ${resolvedPath}, error:\n ${error}`);
+		}
+
+		return this.importedNotes[resolvedPath];
 	}
 }