MWS: fix editing attachment tiddlers (#8455)

* fix breaking bug in image tiddler attachment

* fix comments

* fix code format

* refactor processIncomingTiddler flow

* remove whitespaces after if statements

* refactor attachment_blob persistence flow

* refactor process tiddler to support different attachments

* add tests for attachment

* add more attachement test cases

* working on adding instanbul for test coverage report

* code coverage report generation

* remove unnecessary packages

* fix comments

.gitignore

@@ -10,3 +10,4 @@ store/
 /playwright-report/
 /playwright/.cache/
 $__StoryList.tid
+/editions/test/test-store/*

editions/multiwikiserver/tiddlers/$__StoryList_1.tid

@@ -0,0 +1 @@
+

package.json

@@ -31,8 +31,10 @@
     "node": ">=0.8.2"
   },
   "scripts": {
-	"start": "node ./tiddlywiki.js ./editions/multiwikiserver --mws-load-plugin-bags --build load-mws-demo-data --mws-listen",
-    "test": "node ./tiddlywiki.js ./editions/test --verbose --version --build index && node ./tiddlywiki ./editions/multiwikiserver/ --build load-mws-demo-data --mws-listen --mws-test-server http://127.0.0.1:8080/ --quit",
+    "start": "node ./tiddlywiki.js ./editions/multiwikiserver --mws-load-plugin-bags --build load-mws-demo-data --mws-listen",
+    "build:test-edition": "node ./tiddlywiki.js ./editions/test --verbose --version --build index",
+    "test:multiwikiserver-edition": "node ./tiddlywiki.js ./editions/multiwikiserver/ --build load-mws-demo-data --mws-listen --mws-test-server http://127.0.0.1:8080/ --quit",
+    "test": "npm run build:test-edition && npm run test:multiwikiserver-edition",
     "lint:fix": "eslint . --fix",
     "lint": "eslint ."
   },

plugins/tiddlywiki/multiwikiserver/config/MultiWikiServerEnableAttachments.tid

@@ -1,2 +1,2 @@
 title: $:/config/MultiWikiServer/EnableAttachments
-text: no
+text: yes

plugins/tiddlywiki/multiwikiserver/modules/store/attachments.js

@@ -53,6 +53,7 @@ Saves an attachment to a file. Options include:
 text: text content (may be binary)
 type: MIME type of content
 reference: reference to use for debugging
+_canonical_uri: canonical uri of the content
 */
 AttachmentStore.prototype.saveAttachment = function(options) {
 	const path = require("path"),
@@ -69,6 +70,7 @@ AttachmentStore.prototype.saveAttachment = function(options) {
 	fs.writeFileSync(path.resolve(attachmentPath,dataFilename),options.text,contentTypeInfo.encoding);
 	// Save the meta.json file
 	fs.writeFileSync(path.resolve(attachmentPath,"meta.json"),JSON.stringify({
+		_canonical_uri: options._canonical_uri,
 		created: $tw.utils.stringifyDate(new Date()),
 		modified: $tw.utils.stringifyDate(new Date()),
 		contentHash: contentHash,
@@ -81,7 +83,7 @@ AttachmentStore.prototype.saveAttachment = function(options) {
 /*
 Adopts an attachment file into the store
 */
-AttachmentStore.prototype.adoptAttachment = function(incomingFilepath,type,hash) {
+AttachmentStore.prototype.adoptAttachment = function(incomingFilepath,type,hash,_canonical_uri) {
 	const path = require("path"),
 		fs = require("fs");
 	// Choose the best file extension for the attachment given its type
@@ -95,6 +97,7 @@ AttachmentStore.prototype.adoptAttachment = function(incomingFilepath,type,hash)
 	fs.renameSync(incomingFilepath,dataFilepath);
 	// Save the meta.json file
 	fs.writeFileSync(path.resolve(attachmentPath,"meta.json"),JSON.stringify({
+		_canonical_uri: _canonical_uri,
 		created: $tw.utils.stringifyDate(new Date()),
 		modified: $tw.utils.stringifyDate(new Date()),
 		contentHash: hash,
@@ -137,6 +140,42 @@ AttachmentStore.prototype.getAttachmentStream = function(attachment_name) {
 	return null;
 };
 
+/*
+Get the size of an attachment file given the contentHash.
+Returns the size in bytes, or null if the file doesn't exist.
+*/
+AttachmentStore.prototype.getAttachmentFileSize = function(contentHash) {
+	const path = require("path"),
+		fs = require("fs");
+	// Construct the path to the attachment directory
+	const attachmentPath = path.resolve(this.storePath, "files", contentHash);
+	// Read the meta.json file
+	const metaJsonPath = path.resolve(attachmentPath, "meta.json");
+	if(fs.existsSync(metaJsonPath) && fs.statSync(metaJsonPath).isFile()) {
+		const meta = $tw.utils.parseJSONSafe(fs.readFileSync(metaJsonPath, "utf8"), function() { return null; });
+		if(meta) {
+			const dataFilepath = path.resolve(attachmentPath, meta.filename);
+			// Check if the data file exists and return its size
+			if(fs.existsSync(dataFilepath) && fs.statSync(dataFilepath).isFile()) {
+				return fs.statSync(dataFilepath).size;
+			}
+		}
+	}
+	// Return null if the file doesn't exist or there was an error
+	return null;
+};
+
+AttachmentStore.prototype.getAttachmentMetadata = function(attachmentBlob) {
+	const path = require("path"),
+		fs = require("fs");
+	const attachmentPath = path.resolve(this.storePath, "files", attachmentBlob);
+	const metaJsonPath = path.resolve(attachmentPath, "meta.json");
+	if(fs.existsSync(metaJsonPath)) {
+		const metadata = JSON.parse(fs.readFileSync(metaJsonPath, "utf8"));
+		return metadata;
+	}
+	return null;
+};
 exports.AttachmentStore = AttachmentStore;
 
 })();

plugins/tiddlywiki/multiwikiserver/modules/store/sql-tiddler-database.js

@@ -539,6 +539,42 @@ SqlTiddlerDatabase.prototype.getRecipeBags = function(recipe_name) {
 	return rows.map(value => value.bag_name);
 };
 
+/*
+Get the attachment value of a bag, if any exist
+*/
+SqlTiddlerDatabase.prototype.getBagTiddlerAttachmentBlob = function(title,bag_name) {
+	const row = this.engine.runStatementGet(`
+		SELECT t.attachment_blob
+		FROM bags AS b
+		INNER JOIN tiddlers AS t ON b.bag_id = t.bag_id
+		WHERE t.title = $title AND b.bag_name = $bag_name AND t.is_deleted = FALSE
+	`, {
+		$title: title,
+		$bag_name: bag_name
+	});
+	return row ? row.attachment_blob : null;
+};
+
+/*
+Get the attachment value of a recipe, if any exist
+*/
+SqlTiddlerDatabase.prototype.getRecipeTiddlerAttachmentBlob = function(title,recipe_name) {
+	const row = this.engine.runStatementGet(`
+		SELECT t.attachment_blob
+		FROM bags AS b
+		INNER JOIN recipe_bags AS rb ON b.bag_id = rb.bag_id
+		INNER JOIN recipes AS r ON rb.recipe_id = r.recipe_id
+		INNER JOIN tiddlers AS t ON b.bag_id = t.bag_id
+		WHERE r.recipe_name = $recipe_name AND t.title = $title AND t.is_deleted = FALSE
+		ORDER BY rb.position DESC
+		LIMIT 1
+	`, {
+		$title: title,
+		$recipe_name: recipe_name
+	});
+	return row ? row.attachment_blob : null;
+};
+
 exports.SqlTiddlerDatabase = SqlTiddlerDatabase;
 
 })();

plugins/tiddlywiki/multiwikiserver/modules/store/sql-tiddler-store.js

@@ -143,30 +143,51 @@ SqlTiddlerStore.prototype.processOutgoingTiddler = function(tiddlerFields,tiddle
 
 /*
 */
-SqlTiddlerStore.prototype.processIncomingTiddler = function(tiddlerFields) {
-	let attachmentSizeLimit = $tw.utils.parseNumber(this.adminWiki.getTiddlerText("$:/config/MultiWikiServer/AttachmentSizeLimit"));
+SqlTiddlerStore.prototype.processIncomingTiddler = function(tiddlerFields, existing_attachment_blob, existing_canonical_uri) {
+  let attachmentSizeLimit = $tw.utils.parseNumber(this.adminWiki.getTiddlerText("$:/config/MultiWikiServer/AttachmentSizeLimit"));
 	if(attachmentSizeLimit < 100 * 1024) {
 		attachmentSizeLimit = 100 * 1024;
 	}
-	const attachmentsEnabled = this.adminWiki.getTiddlerText("$:/config/MultiWikiServer/EnableAttachments","yes") === "yes";
-	const contentTypeInfo = $tw.config.contentTypeInfo[tiddlerFields.type || "text/vnd.tiddlywiki"],
-		isBinary = !!contentTypeInfo && contentTypeInfo.encoding === "base64";
-	if(attachmentsEnabled && isBinary && tiddlerFields.text && tiddlerFields.text.length > attachmentSizeLimit) {
-		const attachment_blob = this.attachmentStore.saveAttachment({
-			text: tiddlerFields.text,
-			type: tiddlerFields.type,
-			reference: tiddlerFields.title
-		});
-		return {
-			tiddlerFields: Object.assign({},tiddlerFields,{text: undefined}),
-			attachment_blob: attachment_blob
-		};
-	} else {
-		return {
-			tiddlerFields: tiddlerFields,
-			attachment_blob: null
-		};
-	}
+  const attachmentsEnabled = this.adminWiki.getTiddlerText("$:/config/MultiWikiServer/EnableAttachments", "yes") === "yes";
+  const contentTypeInfo = $tw.config.contentTypeInfo[tiddlerFields.type || "text/vnd.tiddlywiki"];
+  const isBinary = !!contentTypeInfo && contentTypeInfo.encoding === "base64";
+
+  let shouldProcessAttachment = tiddlerFields.text && tiddlerFields.text.length <= attachmentSizeLimit;
+
+  if(existing_attachment_blob) {
+    const fileSize = this.attachmentStore.getAttachmentFileSize(existing_attachment_blob);
+    if(fileSize <= attachmentSizeLimit) {
+      const existingAttachmentMeta = this.attachmentStore.getAttachmentMetadata(existing_attachment_blob);
+      const hasCanonicalField = !!tiddlerFields._canonical_uri;
+      const skipAttachment = hasCanonicalField && (tiddlerFields._canonical_uri === (existingAttachmentMeta?._canonical_uri || existing_canonical_uri));
+      shouldProcessAttachment = !skipAttachment;
+    } else {
+      shouldProcessAttachment = false;
+    }
+  }
+
+  if(attachmentsEnabled && isBinary && shouldProcessAttachment) {
+    const attachment_blob = existing_attachment_blob || this.attachmentStore.saveAttachment({
+      text: tiddlerFields.text,
+      type: tiddlerFields.type,
+      reference: tiddlerFields.title,
+      _canonical_uri: tiddlerFields._canonical_uri
+    });
+    
+    if(tiddlerFields?._canonical_uri) {
+      delete tiddlerFields._canonical_uri;
+    }
+    
+    return {
+      tiddlerFields: Object.assign({}, tiddlerFields, { text: undefined }),
+      attachment_blob: attachment_blob
+    };
+  } else {
+    return {
+      tiddlerFields: tiddlerFields,
+      attachment_blob: existing_attachment_blob
+    };
+  }
 };
 
 SqlTiddlerStore.prototype.saveTiddlersFromPath = function(tiddler_files_path,bag_name) {
@@ -244,7 +265,12 @@ SqlTiddlerStore.prototype.createRecipe = function(recipe_name,bag_names,descript
 Returns {tiddler_id:}
 */
 SqlTiddlerStore.prototype.saveBagTiddler = function(incomingTiddlerFields,bag_name) {
-	const {tiddlerFields, attachment_blob} = this.processIncomingTiddler(incomingTiddlerFields);
+	let _canonical_uri;
+	const existing_attachment_blob = this.sqlTiddlerDatabase.getBagTiddlerAttachmentBlob(incomingTiddlerFields.title,bag_name)
+	if(existing_attachment_blob) {
+		_canonical_uri = `/bags/${$tw.utils.encodeURIComponentExtended(bag_name)}/tiddlers/${$tw.utils.encodeURIComponentExtended(incomingTiddlerFields.title)}/blob`
+	}
+	const {tiddlerFields, attachment_blob} = this.processIncomingTiddler(incomingTiddlerFields,existing_attachment_blob,_canonical_uri);
 	const result = this.sqlTiddlerDatabase.saveBagTiddler(tiddlerFields,bag_name,attachment_blob);
 	this.dispatchEvent("change");
 	return result;
@@ -261,7 +287,7 @@ type - content type of file as uploaded
 Returns {tiddler_id:}
 */
 SqlTiddlerStore.prototype.saveBagTiddlerWithAttachment = function(incomingTiddlerFields,bag_name,options) {
-	const attachment_blob = this.attachmentStore.adoptAttachment(options.filepath,options.type,options.hash);
+	const attachment_blob = this.attachmentStore.adoptAttachment(options.filepath,options.type,options.hash,options._canonical_uri);
 	if(attachment_blob) {
 		const result = this.sqlTiddlerDatabase.saveBagTiddler(incomingTiddlerFields,bag_name,attachment_blob);
 		this.dispatchEvent("change");
@@ -275,7 +301,8 @@ SqlTiddlerStore.prototype.saveBagTiddlerWithAttachment = function(incomingTiddle
 Returns {tiddler_id:,bag_name:}
 */
 SqlTiddlerStore.prototype.saveRecipeTiddler = function(incomingTiddlerFields,recipe_name) {
-	const {tiddlerFields, attachment_blob} = this.processIncomingTiddler(incomingTiddlerFields);
+	const existing_attachment_blob = this.sqlTiddlerDatabase.getRecipeTiddlerAttachmentBlob(incomingTiddlerFields.title,recipe_name)
+	const {tiddlerFields, attachment_blob} = this.processIncomingTiddler(incomingTiddlerFields,existing_attachment_blob,incomingTiddlerFields._canonical_uri);
 	const result = this.sqlTiddlerDatabase.saveRecipeTiddler(tiddlerFields,recipe_name,attachment_blob);
 	this.dispatchEvent("change");
 	return result;

plugins/tiddlywiki/multiwikiserver/modules/test-attachment.js

@@ -0,0 +1,175 @@
+/*\
+title: $:/plugins/tiddlywiki/multiwikiserver/modules/test-attachment.js
+type: application/javascript
+tags: [[$:/tags/test-spec]]
+
+Tests attachments.
+
+\*/
+var fs = require('fs');
+var path = require('path');
+var assert = require('assert');
+var AttachmentStore = require('$:/plugins/tiddlywiki/multiwikiserver/store/attachments.js').AttachmentStore;
+const {Buffer} = require('buffer');
+
+function generateFileWithSize(filePath, sizeInBytes) {
+  return new Promise((resolve, reject) => {
+    var buffer = Buffer.alloc(sizeInBytes);
+    for(var i = 0; i < sizeInBytes; i++) {
+      buffer[i] = Math.floor(Math.random() * 256);
+    }
+
+    fs.writeFile(filePath, buffer, (err) => {
+      if(err) {
+        console.error('Error writing file:', err);
+        reject(err);
+      } else {
+        console.log('File '+filePath+' generated with size '+sizeInBytes+' bytes');
+        fs.readFile(filePath, (err, data) => {
+          if(err) {
+            console.error('Error reading file:', err);
+            reject(err);
+          } else {
+            resolve(data);
+          }
+        });
+      }
+    });
+  });
+}
+
+(function() {
+'use strict';
+if($tw.node) {
+  describe('AttachmentStore', function() {
+    var storePath = './editions/test/test-store';
+    var attachmentStore = new AttachmentStore({ storePath: storePath });
+    var originalTimeout;
+
+    beforeAll(function() {
+      originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
+      jasmine.DEFAULT_TIMEOUT_INTERVAL = 50000;
+    });
+
+    afterAll(function() {
+      jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout;
+      fs.readdirSync(storePath).forEach(function(file) {
+        var filePath = path.join(storePath, file);
+        if(fs.lstatSync(filePath).isFile()) {
+          fs.unlinkSync(filePath);
+        } else if(fs.lstatSync(filePath).isDirectory()) {
+          fs.rmdirSync(filePath, { recursive: true });
+        }
+      });
+    });
+
+    it('isValidAttachmentName', function() {
+      expect(attachmentStore.isValidAttachmentName('abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890')).toBe(true);
+      expect(attachmentStore.isValidAttachmentName('invalid-name')).toBe(false);
+    });
+
+    it('saveAttachment', function() {
+      var options = {
+        text: 'Hello, World!',
+        type: 'text/plain',
+        reference: 'test-reference',
+      };
+      var contentHash = attachmentStore.saveAttachment(options);
+      assert.strictEqual(contentHash.length, 64);
+      assert.strictEqual(fs.existsSync(path.resolve(storePath, 'files', contentHash)), true);
+    });
+  
+    it('adoptAttachment', function() {
+      var incomingFilepath = path.resolve(storePath, 'incoming-file.txt');
+      fs.writeFileSync(incomingFilepath, 'Hello, World!');
+      var type = 'text/plain';
+      var hash = 'abcdef0123456789abcdef0123456789';
+      var _canonical_uri = 'test-canonical-uri';
+      attachmentStore.adoptAttachment(incomingFilepath, type, hash, _canonical_uri);
+      expect(fs.existsSync(path.resolve(storePath, 'files', hash))).toBe(true);
+    });
+  
+    it('getAttachmentStream', function() {
+      var options = {
+        text: 'Hello, World!',
+        type: 'text/plain',
+        filename: 'data.txt',
+      };
+      var contentHash = attachmentStore.saveAttachment(options);
+      var stream = attachmentStore.getAttachmentStream(contentHash);
+      expect(stream).not.toBeNull();
+      expect(stream.type).toBe('text/plain');
+    });
+
+    it('getAttachmentFileSize', function() {
+      var options = {
+        text: 'Hello, World!',
+        type: 'text/plain',
+        reference: 'test-reference',
+      };
+      var contentHash = attachmentStore.saveAttachment(options);
+      var fileSize = attachmentStore.getAttachmentFileSize(contentHash);
+      expect(fileSize).toBe(13);
+    });
+
+    it('getAttachmentMetadata', function() {
+      var options = {
+        text: 'Hello, World!',
+        type: 'text/plain',
+        filename: 'data.txt',
+      };
+      var contentHash = attachmentStore.saveAttachment(options);
+      var metadata = attachmentStore.getAttachmentMetadata(contentHash);
+      expect(metadata).not.toBeNull();
+      expect(metadata.type).toBe('text/plain');
+      expect(metadata.filename).toBe('data.txt');
+    });
+
+    it('saveAttachment large file', async function() {
+      var sizeInMB = 10
+      const file = await generateFileWithSize('./editions/test/test-store/large-file.txt', 1024 * 1024 * sizeInMB)
+      var options = {
+        text: file,
+        type: 'application/octet-stream',
+        reference: 'test-reference',
+      };
+      var contentHash = attachmentStore.saveAttachment(options);
+      assert.strictEqual(contentHash.length, 64);
+      assert.strictEqual(fs.existsSync(path.resolve(storePath, 'files', contentHash)), true);
+    });
+
+    it('saveAttachment multiple large files', async function() {
+      var sizeInMB = 10;
+      var numFiles = 5;
+      for (var i = 0; i < numFiles; i++) {
+        const file = await generateFileWithSize(`./editions/test/test-store/large-file-${i}.txt`, 1024 * 1024 * sizeInMB);
+        var options = {
+          text: file,
+          type: 'application/octet-stream',
+          reference: `test-reference-${i}`,
+        };
+        var contentHash = attachmentStore.saveAttachment(options);
+        assert.strictEqual(contentHash.length, 64);
+        assert.strictEqual(fs.existsSync(path.resolve(storePath, 'files', contentHash)), true);
+      }
+    });
+
+    it('getAttachmentStream multiple large files', async function() {
+      var sizeInMB = 10;
+      var numFiles = 5;
+      for (var i = 0; i < numFiles; i++) {
+        const file = await generateFileWithSize(`./editions/test/test-store/large-file-${i}.txt`, 1024 * 1024 * sizeInMB);
+        var options = {
+          text: file,
+          type: 'application/octet-stream',
+          reference: `test-reference-${i}`,
+        };
+        var contentHash = attachmentStore.saveAttachment(options);
+        var stream = attachmentStore.getAttachmentStream(contentHash);
+        assert.notStrictEqual(stream, null);
+        assert.strictEqual(stream.type, 'application/octet-stream');
+      }
+    });
+  });
+}
+})();