1
0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-07-31 15:24:23 +03:00

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
This commit is contained in:
Hugh Nimmo-Smith
2021-11-30 09:02:41 +00:00
committed by GitHub
parent 7dcee2eb40
commit db9936e07c
5 changed files with 17 additions and 50 deletions

View File

@ -1,27 +0,0 @@
/*
Copyright 2021 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
export class MockBlob {
private contents: number[] = [];
public constructor(private parts: ArrayLike<number>[]) {
parts.forEach(p => Array.from(p).forEach(e => this.contents.push(e)));
}
public get size(): number {
return this.contents.length;
}
}

View File

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

View File

@ -24,7 +24,6 @@ import {
TreePermissions, TreePermissions,
} from "../../../src/models/MSC3089TreeSpace"; } from "../../../src/models/MSC3089TreeSpace";
import { DEFAULT_ALPHABET } from "../../../src/utils"; import { DEFAULT_ALPHABET } from "../../../src/utils";
import { MockBlob } from "../../MockBlob";
import { MatrixError } from "../../../src/http-api"; import { MatrixError } from "../../../src/http-api";
describe("MSC3089TreeSpace", () => { describe("MSC3089TreeSpace", () => {
@ -887,12 +886,8 @@ describe("MSC3089TreeSpace", () => {
const fileName = "My File.txt"; const fileName = "My File.txt";
const fileContents = "This is a test file"; const fileContents = "This is a test file";
// Mock out Blob for the test environment const uploadFn = jest.fn().mockImplementation((contents: Buffer, opts: any) => {
(<any>global).Blob = MockBlob; expect(contents.length).toEqual(fileContents.length);
const uploadFn = jest.fn().mockImplementation((contents: Blob, opts: any) => {
expect(contents).toBeInstanceOf(Blob);
expect(contents.size).toEqual(fileContents.length);
expect(opts).toMatchObject({ expect(opts).toMatchObject({
includeFilename: false, includeFilename: false,
onlyContentUri: true, // because the tests rely on this - we shouldn't really be testing for this. 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; 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. // 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 }); const result = await tree.createFile(fileName, buf, Object.assign({}, fileInfo), { metadata: true });
@ -951,12 +946,8 @@ describe("MSC3089TreeSpace", () => {
const fileName = "My File.txt"; const fileName = "My File.txt";
const fileContents = "This is a test file"; const fileContents = "This is a test file";
// Mock out Blob for the test environment const uploadFn = jest.fn().mockImplementation((contents: Buffer, opts: any) => {
(<any>global).Blob = MockBlob; expect(contents.length).toEqual(fileContents.length);
const uploadFn = jest.fn().mockImplementation((contents: Blob, opts: any) => {
expect(contents).toBeInstanceOf(Blob);
expect(contents.size).toEqual(fileContents.length);
expect(opts).toMatchObject({ expect(opts).toMatchObject({
includeFilename: false, includeFilename: false,
onlyContentUri: true, // because the tests rely on this - we shouldn't really be testing for this. 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; 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. // 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 }); const result = await tree.createFile(fileName, buf, Object.assign({}, fileInfo), { "m.new_content": true });

View File

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

View File

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