Fix attachment interop between win and nix

browser/main/lib/dataApi/attachmentManagement.js

@@ -10,6 +10,7 @@ import i18n from 'browser/lib/i18n'
 
 const STORAGE_FOLDER_PLACEHOLDER = ':storage'
 const DESTINATION_FOLDER = 'attachments'
+const PATH_SEPARATORS = escapeStringRegexp(path.posix.sep) + escapeStringRegexp(path.win32.sep)
 
 /**
  * @description
@@ -74,6 +75,8 @@ function createAttachmentDestinationFolder (destinationStoragePath, noteKey) {
   }
 }
 
+// TODO: This function does not have unit test. Is it working?
+// TODO: Can we rewrite it to use markdownContent instead of renderedHTML? That way we can keep one function
 /**
  * @description Moves attachments from the old location ('/images') to the new one ('/attachments/noteKey)
  * @param renderedHTML HTML of the current note
@@ -106,7 +109,10 @@ function migrateAttachments (renderedHTML, storagePath, noteKey) {
  * @returns {String} postprocessed HTML in which all :storage references are mapped to the actual paths.
  */
 function fixLocalURLS (renderedHTML, storagePath) {
-  return renderedHTML.replace(new RegExp(mdurl.encode(path.sep), 'g'), path.sep).replace(new RegExp('/?' + STORAGE_FOLDER_PLACEHOLDER, 'g'), 'file:///' + path.join(storagePath, DESTINATION_FOLDER))
+  return renderedHTML.replace(new RegExp('/?' + STORAGE_FOLDER_PLACEHOLDER + ".*?\"", 'g'), function (match) {
+    var encoded_path_separators = new RegExp(mdurl.encode(path.win32.sep) + '|' + mdurl.encode(path.posix.sep), 'g')
+    return match.replace(encoded_path_separators, path.sep).replace(new RegExp('/?' + STORAGE_FOLDER_PLACEHOLDER, 'g'), 'file:///' + path.join(storagePath, DESTINATION_FOLDER))
+  });
 }
 
 /**
@@ -186,12 +192,23 @@ function handlePastImageEvent (codeEditor, storageKey, noteKey, dataTransferItem
 }
 
 /**
- * @description Returns all attachment paths of the given markdown
+* @description Returns all attachment paths of the given markdown
- * @param {String} markdownContent content in which the attachment paths should be found
+* @param {String} markdownContent content in which the attachment paths should be found
+* @returns {String[]} Array of the relative paths (starting with :storage) of the attachments of the given markdown
+*/
+function getAttachmentsInMarkdownContent (markdownContent) {
+  const preparedInput = markdownContent.replace(new RegExp('[' + PATH_SEPARATORS + ']', 'g'), path.sep)
+  const regexp = new RegExp('/?' + STORAGE_FOLDER_PLACEHOLDER + '(' + escapeStringRegexp(path.sep) + ')' + '?([a-zA-Z0-9]|-)*' + '(' + escapeStringRegexp(path.sep) + ')' + '([a-zA-Z0-9]|\\.)+(\\.[a-zA-Z0-9]+)?', 'g')
+  return preparedInput.match(regexp)
+}
+
+/**
+ * @description Returns all attachment paths of the given renderedHTML
+ * @param {String} renderedHTML content in which the attachment paths should be found
  * @returns {String[]} Array of the relative paths (starting with :storage) of the attachments of the given markdown
  */
-function getAttachmentsInContent (markdownContent) {
+function getAttachmentsInContent (renderedHTML) {
-  const preparedInput = markdownContent.replace(new RegExp(mdurl.encode(path.sep), 'g'), path.sep)
+  const preparedInput = renderedHTML.replace(new RegExp(mdurl.encode(path.sep), 'g'), path.sep)
   const regexp = new RegExp('/?' + STORAGE_FOLDER_PLACEHOLDER + '(' + escapeStringRegexp(path.sep) + '|/)' + '?([a-zA-Z0-9]|-)*' + '(' + escapeStringRegexp(path.sep) + '|/)' + '([a-zA-Z0-9]|\\.)+(\\.[a-zA-Z0-9]+)?', 'g')
   return preparedInput.match(regexp)
 }
@@ -203,7 +220,7 @@ function getAttachmentsInContent (markdownContent) {
  * @returns {String[]} Absolute paths of the referenced attachments
  */
 function getAbsolutePathsOfAttachmentsInContent (markdownContent, storagePath) {
-  const temp = getAttachmentsInContent(markdownContent) || []
+  const temp = getAttachmentsInMarkdownContent(markdownContent) || []
   const result = []
   for (const relativePath of temp) {
     result.push(relativePath.replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER, 'g'), path.join(storagePath, DESTINATION_FOLDER)))
@@ -239,7 +256,8 @@ function moveAttachments (oldPath, newPath, noteKey, newNoteKey, noteContent) {
  */
 function replaceNoteKeyWithNewNoteKey (noteContent, oldNoteKey, newNoteKey) {
   if (noteContent) {
-    return noteContent.replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + oldNoteKey, 'g'), path.join(STORAGE_FOLDER_PLACEHOLDER, newNoteKey))
+    const preparedInput = noteContent.replace(new RegExp('[' + PATH_SEPARATORS + ']', 'g'), path.sep)
+    return preparedInput.replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + oldNoteKey, 'g'), path.join(STORAGE_FOLDER_PLACEHOLDER, newNoteKey))
   }
   return noteContent
 }
@@ -277,7 +295,7 @@ function deleteAttachmentsNotPresentInNote (markdownContent, storageKey, noteKey
   }
   const targetStorage = findStorage.findStorage(storageKey)
   const attachmentFolder = path.join(targetStorage.path, DESTINATION_FOLDER, noteKey)
-  const attachmentsInNote = getAttachmentsInContent(markdownContent)
+  const attachmentsInNote = getAttachmentsInMarkdownContent(markdownContent)
   const attachmentsInNoteOnlyFileNames = []
   if (attachmentsInNote) {
     for (let i = 0; i < attachmentsInNote.length; i++) {
@@ -348,7 +366,7 @@ function generateFileNotFoundMarkdown () {
  */
 function isAttachmentLink (text) {
   if (text) {
-    return text.match(new RegExp('.*\\[.*\\]\\( *' + escapeStringRegexp(STORAGE_FOLDER_PLACEHOLDER) + escapeStringRegexp(path.sep) + '.*\\).*', 'gi')) != null
+    return text.match(new RegExp('.*\\[.*\\]\\( *' + escapeStringRegexp(STORAGE_FOLDER_PLACEHOLDER) + '[' + PATH_SEPARATORS + ']' + '.*\\).*', 'gi')) != null
   }
   return false
 }
@@ -364,7 +382,7 @@ function isAttachmentLink (text) {
 function handleAttachmentLinkPaste (storageKey, noteKey, linkText) {
   if (storageKey != null && noteKey != null && linkText != null) {
     const storagePath = findStorage.findStorage(storageKey).path
-    const attachments = getAttachmentsInContent(linkText) || []
+    const attachments = getAttachmentsInMarkdownContent(linkText) || []
     const replaceInstructions = []
     const copies = []
     for (const attachment of attachments) {
@@ -373,13 +391,13 @@ function handleAttachmentLinkPaste (storageKey, noteKey, linkText) {
         sander.exists(absPathOfAttachment)
           .then((fileExists) => {
             if (!fileExists) {
-              const fileNotFoundRegexp = new RegExp('!?' + escapeStringRegexp('[') + '[\\w|\\d|\\s|\\.]*\\]\\(\\s*' + STORAGE_FOLDER_PLACEHOLDER + '[\\w|\\d|\\-|' + escapeStringRegexp(path.sep) + ']*' + escapeStringRegexp(path.basename(absPathOfAttachment)) + escapeStringRegexp(')'))
+              const fileNotFoundRegexp = new RegExp('!?' + escapeStringRegexp('[') + '[\\w|\\d|\\s|\\.]*\\]\\(\\s*' + STORAGE_FOLDER_PLACEHOLDER + '[\\w|\\d|\\-|' + PATH_SEPARATORS + ']*' + escapeStringRegexp(path.basename(absPathOfAttachment)) + escapeStringRegexp(')'))
               replaceInstructions.push({regexp: fileNotFoundRegexp, replacement: this.generateFileNotFoundMarkdown()})
               return Promise.resolve()
             }
             return this.copyAttachment(absPathOfAttachment, storageKey, noteKey)
               .then((fileName) => {
-                const replaceLinkRegExp = new RegExp(escapeStringRegexp('(') + ' *' + STORAGE_FOLDER_PLACEHOLDER + '[\\w|\\d|\\-|' + escapeStringRegexp(path.sep) + ']*' + escapeStringRegexp(path.basename(absPathOfAttachment)) + ' *' + escapeStringRegexp(')'))
+                const replaceLinkRegExp = new RegExp(escapeStringRegexp('(') + ' *' + STORAGE_FOLDER_PLACEHOLDER + '[\\w|\\d|\\-|' + PATH_SEPARATORS + ']*' + escapeStringRegexp(path.basename(absPathOfAttachment)) + ' *' + escapeStringRegexp(')'))
                 replaceInstructions.push({
                   regexp: replaceLinkRegExp,
                   replacement: '(' + path.join(STORAGE_FOLDER_PLACEHOLDER, noteKey, fileName) + ')'
@@ -409,6 +427,7 @@ module.exports = {
   handleAttachmentDrop,
   handlePastImageEvent,
   getAttachmentsInContent,
+  getAttachmentsInMarkdownContent,
   getAbsolutePathsOfAttachmentsInContent,
   removeStorageAndNoteReferences,
   deleteAttachmentFolder,

tests/dataApi/attachmentManagement.test.js

@@ -169,6 +169,43 @@ it('should replace the all ":storage" path with the actual storage path', functi
   expect(actual).toEqual(expectedOutput)
 })
 
+it('should replace the ":storage" path with the actual storage path when they have different path separators', function () {
+  const storageFolder = systemUnderTest.DESTINATION_FOLDER
+  const testInput =
+    '<html>\n' +
+    '    <head>\n' +
+    '        //header\n' +
+    '    </head>\n' +
+    '    <body data-theme="default">\n' +
+    '        <h2 data-line="0" id="Headline">Headline</h2>\n' +
+    '        <p data-line="2">\n' +
+    '            <img src=":storage' + mdurl.encode(path.win32.sep) + '0.6r4zdgc22xp.png" alt="dummyImage.png" >\n' +
+    '        </p>\n' +
+    '        <p data-line="4">\n' +
+    '            <a href=":storage' + mdurl.encode(path.posix.sep) + '0.q2i4iw0fyx.pdf">dummyPDF.pdf</a>\n' +
+    '        </p>\n' +
+    '    </body>\n' +
+    '</html>'
+  const storagePath = '<<dummyStoragePath>>'
+  const expectedOutput =
+    '<html>\n' +
+    '    <head>\n' +
+    '        //header\n' +
+    '    </head>\n' +
+    '    <body data-theme="default">\n' +
+    '        <h2 data-line="0" id="Headline">Headline</h2>\n' +
+    '        <p data-line="2">\n' +
+    '            <img src="file:///' + storagePath + path.sep + storageFolder + path.sep + '0.6r4zdgc22xp.png" alt="dummyImage.png" >\n' +
+    '        </p>\n' +
+    '        <p data-line="4">\n' +
+    '            <a href="file:///' + storagePath + path.sep + storageFolder + path.sep + '0.q2i4iw0fyx.pdf">dummyPDF.pdf</a>\n' +
+    '        </p>\n' +
+    '    </body>\n' +
+    '</html>'
+  const actual = systemUnderTest.fixLocalURLS(testInput, storagePath)
+  expect(actual).toEqual(expectedOutput)
+})
+
 it('should test that generateAttachmentMarkdown works correct both with previews and without', function () {
   const fileName = 'fileName'
   const path = 'path'
@@ -204,6 +241,18 @@ it('should test that getAttachmentsInContent finds all attachments', function ()
   expect(actual).toEqual(expect.arrayContaining(expected))
 })
 
+it('should test that getAttachmentsInMarkdownContent finds all attachments when they have different path separators', function () {
+  const testInput = '"# Test\n' +
+    '\n' +
+    '![Screenshot1](:storage' + path.win32.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.win32.sep + '0.3b88d0dc.png)\n' +
+    '![Screenshot2](:storage' + path.posix.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.posix.sep + '2cb8875c.pdf)\n' +
+    '![Screenshot3](:storage' + path.win32.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.posix.sep + 'bbf49b02.jpg)"'
+
+  const actual = systemUnderTest.getAttachmentsInMarkdownContent(testInput)
+  const expected = [':storage' + path.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.sep + '0.3b88d0dc.png', ':storage' + path.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.sep + '2cb8875c.pdf', ':storage' + path.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.sep + 'bbf49b02.jpg']
+  expect(actual).toEqual(expect.arrayContaining(expected))
+})
+
 it('should test that getAbsolutePathsOfAttachmentsInContent returns all absolute paths', function () {
   const dummyStoragePath = 'dummyStoragePath'
   const testInput =