Standardise content type handling in MSC3089 createFile() and createNewVersion() (#2014)

* Provide cross platform compatible versions of createFile() and createNewVersion()

The exist implementations are deprecated as they only work in a browser and support a different type of contents from MatrixClient.uploadContent()

* Fix MSC3089 content upload meta data in NodeJS runtime

* Break unstable createFile() and createNewVersion() instead of deprecating

Test using NodeJS types instead of mocked browser Blob

* chore: remove incorrect comment

Author: hughns

spec/MockBlob.ts

@@ -244,8 +244,7 @@ describe("MSC3089Branch", () => {
 
     it('should create new versions of itself', async () => {
         const canaryName = "canary";
-        const fileContents = "contents go here";
-        const canaryContents = Uint8Array.from(Array.from(fileContents).map((_, i) => fileContents.charCodeAt(i)));
+        const canaryContents = "contents go here";
         const canaryFile = {} as IEncryptedFile;
         const canaryAddl = { canary: true };
         indexEvent.getContent = () => ({ active: true, retained: true });

spec/unit/models/MSC3089Branch.spec.ts

@@ -24,7 +24,6 @@ import {
     TreePermissions,
 } from "../../../src/models/MSC3089TreeSpace";
 import { DEFAULT_ALPHABET } from "../../../src/utils";
-import { MockBlob } from "../../MockBlob";
 import { MatrixError } from "../../../src/http-api";
 
 describe("MSC3089TreeSpace", () => {
@@ -887,12 +886,8 @@ describe("MSC3089TreeSpace", () => {
         const fileName = "My File.txt";
         const fileContents = "This is a test file";
 
-        // Mock out Blob for the test environment
-        (<any>global).Blob = MockBlob;
-
-        const uploadFn = jest.fn().mockImplementation((contents: Blob, opts: any) => {
-            expect(contents).toBeInstanceOf(Blob);
-            expect(contents.size).toEqual(fileContents.length);
+        const uploadFn = jest.fn().mockImplementation((contents: Buffer, opts: any) => {
+            expect(contents.length).toEqual(fileContents.length);
             expect(opts).toMatchObject({
                 includeFilename: false,
                 onlyContentUri: true, // because the tests rely on this - we shouldn't really be testing  for this.
@@ -930,7 +925,7 @@ describe("MSC3089TreeSpace", () => {
             });
         client.sendStateEvent = sendStateFn;
 
-        const buf = Uint8Array.from(Array.from(fileContents).map((_, i) => fileContents.charCodeAt(i)));
+        const buf = Buffer.from(fileContents);
 
         // We clone the file info just to make sure it doesn't get mutated for the test.
         const result = await tree.createFile(fileName, buf, Object.assign({}, fileInfo), { metadata: true });
@@ -951,12 +946,8 @@ describe("MSC3089TreeSpace", () => {
         const fileName = "My File.txt";
         const fileContents = "This is a test file";
 
-        // Mock out Blob for the test environment
-        (<any>global).Blob = MockBlob;
-
-        const uploadFn = jest.fn().mockImplementation((contents: Blob, opts: any) => {
-            expect(contents).toBeInstanceOf(Blob);
-            expect(contents.size).toEqual(fileContents.length);
+        const uploadFn = jest.fn().mockImplementation((contents: Buffer, opts: any) => {
+            expect(contents.length).toEqual(fileContents.length);
             expect(opts).toMatchObject({
                 includeFilename: false,
                 onlyContentUri: true, // because the tests rely on this - we shouldn't really be testing  for this.
@@ -997,7 +988,7 @@ describe("MSC3089TreeSpace", () => {
             });
         client.sendStateEvent = sendStateFn;
 
-        const buf = Uint8Array.from(Array.from(fileContents).map((_, i) => fileContents.charCodeAt(i)));
+        const buf = Buffer.from(fileContents);
 
         // We clone the file info just to make sure it doesn't get mutated for the test.
         const result = await tree.createFile(fileName, buf, Object.assign({}, fileInfo), { "m.new_content": true });

spec/unit/models/MSC3089TreeSpace.spec.ts

@@ -18,6 +18,7 @@ import { MatrixClient } from "../client";
 import { IEncryptedFile, RelationType, UNSTABLE_MSC3089_BRANCH } from "../@types/event";
 import { IContent, MatrixEvent } from "./event";
 import { MSC3089TreeSpace } from "./MSC3089TreeSpace";
+import type { ReadStream } from "fs";
 
 /**
  * Represents a [MSC3089](https://github.com/matrix-org/matrix-doc/pull/3089) branch - a reference
@@ -142,16 +143,16 @@ export class MSC3089Branch {
     }
 
     /**
-     * Creates a new version of this file.
+     * Creates a new version of this file with contents in a type that is compatible with MatrixClient.uploadContent().
      * @param {string} name The name of the file.
-     * @param {ArrayBuffer} encryptedContents The encrypted contents.
+     * @param {File | String | Buffer | ReadStream | Blob} encryptedContents The encrypted contents.
      * @param {Partial<IEncryptedFile>} info The encrypted file information.
      * @param {IContent} additionalContent Optional event content fields to include in the message.
      * @returns {Promise<void>} Resolves when uploaded.
      */
     public async createNewVersion(
         name: string,
-        encryptedContents: ArrayBuffer,
+        encryptedContents: File | String | Buffer | ReadStream | Blob,
         info: Partial<IEncryptedFile>,
         additionalContent?: IContent,
     ): Promise<void> {

src/models/MSC3089Branch.ts

@@ -31,6 +31,7 @@ import { MSC3089Branch } from "./MSC3089Branch";
 import promiseRetry from "p-retry";
 import { isRoomSharedHistory } from "../crypto/algorithms/megolm";
 import { ISendEventResponse } from "../@types/requests";
+import type { ReadStream } from "fs";
 
 /**
  * The recommended defaults for a tree space's power levels. Note that this
@@ -449,21 +450,23 @@ export class MSC3089TreeSpace {
 
     /**
      * Creates (uploads) a new file to this tree. The file must have already been encrypted for the room.
+     * The file contents are in a type that is compatible with MatrixClient.uploadContent().
      * @param {string} name The name of the file.
-     * @param {ArrayBuffer} encryptedContents The encrypted contents.
+     * @param {File | String | Buffer | ReadStream | Blob} encryptedContents The encrypted contents.
      * @param {Partial<IEncryptedFile>} info The encrypted file information.
      * @param {IContent} additionalContent Optional event content fields to include in the message.
      * @returns {Promise<ISendEventResponse>} Resolves to the file event's sent response.
      */
     public async createFile(
         name: string,
-        encryptedContents: ArrayBuffer,
+        encryptedContents: File | String | Buffer | ReadStream | Blob,
         info: Partial<IEncryptedFile>,
         additionalContent?: IContent,
     ): Promise<ISendEventResponse> {
-        const mxc = await this.client.uploadContent(new Blob([encryptedContents]), {
+        const mxc = await this.client.uploadContent(encryptedContents, {
             includeFilename: false,
             onlyContentUri: true,
+            rawResponse: false, // make this explicit otherwise behaviour is different on browser vs NodeJS
         });
         info.url = mxc;