Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update MSC3912 implementation to use with_rel_type instead of with_relations #3420

Merged
merged 7 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 30 additions & 31 deletions spec/unit/matrix-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
UNSTABLE_MSC3088_ENABLED,
UNSTABLE_MSC3088_PURPOSE,
UNSTABLE_MSC3089_TREE_SUBTYPE,
MSC3912_RELATION_BASED_REDACTIONS_PROP,
} from "../../src/@types/event";
import { MEGOLM_ALGORITHM } from "../../src/crypto/olmlib";
import { Crypto } from "../../src/crypto";
Expand Down Expand Up @@ -181,9 +180,7 @@ describe("MatrixClient", function () {
data: SYNC_DATA,
};

const unstableFeatures: Record<string, boolean> = {
"org.matrix.msc3440.stable": true,
};
let unstableFeatures: Record<string, boolean> = {};

// items are popped off when processed and block if no items left.
let httpLookups: HttpLookup[] = [];
Expand Down Expand Up @@ -342,6 +339,12 @@ describe("MatrixClient", function () {
store.getClientOptions = jest.fn().mockReturnValue(Promise.resolve(null));
store.storeClientOptions = jest.fn().mockReturnValue(Promise.resolve(null));
store.isNewlyCreated = jest.fn().mockReturnValue(Promise.resolve(true));

// set unstableFeatures to a defined state before each test
unstableFeatures = {
"org.matrix.msc3440.stable": true,
};
Comment on lines +344 to +346
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unstableFeatures = {
"org.matrix.msc3440.stable": true,
};
unstableFeatures["org.matrix.msc3440.stable"] = true;

(though I'm not sure why it is being moved at all)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets unstableFeatures to a defined state before each test. Another test sets an additional property on it:

unstableFeatures["org.matrix.msc3912"] = true;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

→ will add a comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see.


makeClient();

// set reasonable working defaults
Expand Down Expand Up @@ -1373,18 +1376,18 @@ describe("MatrixClient", function () {
await client.redactEvent(roomId, eventId, txnId, { reason });
});

describe("when calling with with_relations", () => {
describe("when calling with 'with_rel_types'", () => {
const eventId = "$event42:example.org";

it("should raise an error if server has no support for relation based redactions", async () => {
it("should raise an error if the server has no support for relation based redactions", async () => {
// load supported features
await client.getVersions();

const txnId = client.makeTxnId();

expect(() => {
client.redactEvent(roomId, eventId, txnId, {
with_relations: [RelationType.Reference],
with_rel_types: [RelationType.Reference],
});
}).toThrow(
new Error(
Expand All @@ -1394,34 +1397,30 @@ describe("MatrixClient", function () {
);
});

describe("and the server supports relation based redactions (unstable)", () => {
beforeEach(async () => {
unstableFeatures["org.matrix.msc3912"] = true;
// load supported features
await client.getVersions();
});
it("and the server has unstable support for relation based redactions, it should send 'org.matrix.msc3912.with_relations' in the request body", async () => {
unstableFeatures["org.matrix.msc3912"] = true;
// load supported features
await client.getVersions();

it("should send with_relations in the request body", async () => {
const txnId = client.makeTxnId();
const txnId = client.makeTxnId();

httpLookups = [
{
method: "PUT",
path:
`/rooms/${encodeURIComponent(roomId)}/redact/${encodeURIComponent(eventId)}` +
`/${encodeURIComponent(txnId)}`,
expectBody: {
reason: "redaction test",
[MSC3912_RELATION_BASED_REDACTIONS_PROP.unstable!]: [RelationType.Reference],
},
data: { event_id: eventId },
httpLookups = [
{
method: "PUT",
path:
`/rooms/${encodeURIComponent(roomId)}/redact/${encodeURIComponent(eventId)}` +
`/${encodeURIComponent(txnId)}`,
expectBody: {
reason: "redaction test",
["org.matrix.msc3912.with_relations"]: ["m.reference"],
},
];
data: { event_id: eventId },
},
];

await client.redactEvent(roomId, eventId, txnId, {
reason: "redaction test",
with_relations: [RelationType.Reference],
});
await client.redactEvent(roomId, eventId, txnId, {
reason: "redaction test",
with_rel_types: [RelationType.Reference],
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/@types/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,11 @@ export const UNSTABLE_MSC3089_BRANCH = new UnstableValue("m.branch", "org.matrix
export const UNSTABLE_MSC2716_MARKER = new UnstableValue("m.room.marker", "org.matrix.msc2716.marker");

/**
* Name of the "with_relations" request property for relation based redactions.
* Name of the request property for relation based redactions.
* {@link https://github.com/matrix-org/matrix-spec-proposals/pull/3912}
*/
export const MSC3912_RELATION_BASED_REDACTIONS_PROP = new UnstableValue(
"with_relations",
"with_rel_types",
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
"org.matrix.msc3912.with_relations",
);

Expand Down
5 changes: 2 additions & 3 deletions src/@types/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,16 @@ export interface IJoinRoomOpts {
export interface IRedactOpts {
reason?: string;
/**
* Whether events related to the redacted event should be redacted.
*
* If specified, then any events which relate to the event being redacted with
* any of the relationship types listed will also be redacted.
* Provide a "*" list item to tell the server to redact relations of any type.
*
* <b>Raises an Error if the server does not support it.</b>
* Check for server-side support before using this param with
* <code>client.canSupport.get(Feature.RelationBasedRedactions)</code>.
* {@link https://github.com/matrix-org/matrix-spec-proposals/pull/3912}
*/
with_relations?: Array<RelationType | string>;
with_rel_types?: Array<RelationType | "*">;
}

export interface ISendEventResponse {
Expand Down
40 changes: 18 additions & 22 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4594,10 +4594,10 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

/**
* @param txnId - transaction id. One will be made up if not supplied.
* @param opts - Options to pass on, may contain `reason` and `with_relations` (MSC3912)
* @param opts - Redact options
* @returns Promise which resolves: TODO
* @returns Rejects: with an error response.
* @throws Error if called with `with_relations` (MSC3912) but the server does not support it.
* @throws Error if called with `with_rel_types` (MSC3912) but the server does not support it.
* Callers should check whether the server supports MSC3912 via `MatrixClient.canSupport`.
*/
public redactEvent(
Expand Down Expand Up @@ -4627,34 +4627,30 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
threadId = null;
}
const reason = opts?.reason;
const content: IContent = { reason };

if (
opts?.with_relations &&
this.canSupport.get(Feature.RelationBasedRedactions) === ServerSupport.Unsupported
) {
throw new Error(
"Server does not support relation based redactions " +
`roomId ${roomId} eventId ${eventId} txnId: ${txnId} threadId ${threadId}`,
);
}
if (opts?.with_rel_types !== undefined) {
if (this.canSupport.get(Feature.RelationBasedRedactions) === ServerSupport.Unsupported) {
throw new Error(
"Server does not support relation based redactions " +
`roomId ${roomId} eventId ${eventId} txnId: ${txnId} threadId ${threadId}`,
);
}

const withRelations = opts?.with_relations
? {
[this.canSupport.get(Feature.RelationBasedRedactions) === ServerSupport.Stable
? MSC3912_RELATION_BASED_REDACTIONS_PROP.stable!
: MSC3912_RELATION_BASED_REDACTIONS_PROP.unstable!]: opts?.with_relations,
}
: {};
const withRelTypesPropName =
this.canSupport.get(Feature.RelationBasedRedactions) === ServerSupport.Stable
? MSC3912_RELATION_BASED_REDACTIONS_PROP.stable!
: MSC3912_RELATION_BASED_REDACTIONS_PROP.unstable!;

content[withRelTypesPropName] = opts.with_rel_types;
}

return this.sendCompleteEvent(
roomId,
threadId,
{
type: EventType.RoomRedaction,
content: {
...withRelations,
reason,
},
content,
redacts: eventId,
},
txnId as string,
Expand Down