Skip to content

Commit

Permalink
Merge pull request #920 from matrix-org/bwindels/message-editing-loca…
Browse files Browse the repository at this point in the history
…l-echo

Local echo for m.replace relations
  • Loading branch information
bwindels authored May 16, 2019
2 parents 79d2574 + 1f2a701 commit fbf5352
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 63 deletions.
10 changes: 4 additions & 6 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,11 +153,6 @@ function keyFromRecoverySession(session, decryptionKey) {
* via `EventTimelineSet#getRelationsForEvent`.
* This feature is currently unstable and the API may change without notice.
*
* @param {boolean} [opts.unstableClientRelationReplacements = false]
* Optional. Set to true to enable client-side handling of m.replace event relations,
* exposed through the `Room.replaceEvent` event.
* This feature is currently unstable and the API may change without notice.
*
* @param {Array} [opts.verificationMethods] Optional. The verification method
* that the application can handle. Each element should be an item from {@link
* module:crypto~verificationMethods verificationMethods}, or a class that
Expand Down Expand Up @@ -224,7 +219,6 @@ function MatrixClient(opts) {
this.urlPreviewCache = {};
this._notifTimelineSet = null;
this.unstableClientRelationAggregation = !!opts.unstableClientRelationAggregation;
this.unstableClientRelationReplacements = !!opts.unstableClientRelationReplacements;

this._crypto = null;
this._cryptoStore = opts.cryptoStore;
Expand Down Expand Up @@ -4168,6 +4162,10 @@ function _PojoToMatrixEventMapper(client) {
]);
event.attemptDecryption(client._crypto);
}
const room = client.getRoom(event.getRoomId());
if (room) {
room.reEmitter.reEmit(event, ["Event.replaced"]);
}
return event;
}
return mapper;
Expand Down
31 changes: 31 additions & 0 deletions src/models/event-timeline-set.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ EventTimelineSet.prototype.addEventToTimeline = function(event, timeline,
timeline.addEvent(event, toStartOfTimeline);
this._eventIdToTimeline[eventId] = timeline;

this.setRelationsTarget(event);
this.aggregateRelations(event);

const data = {
Expand Down Expand Up @@ -708,6 +709,34 @@ EventTimelineSet.prototype.getRelationsForEvent = function(
return relationsWithRelType[eventType];
};

/**
* Set an event as the target event if any Relations exist for it already
*
* @param {MatrixEvent} event
* The event to check as relation target.
*/
EventTimelineSet.prototype.setRelationsTarget = function(event) {
if (!this._unstableClientRelationAggregation) {
return;
}

const relationsForEvent = this._relations[event.getId()];
if (!relationsForEvent) {
return;
}
// don't need it for non m.replace relations for now
const relationsWithRelType = relationsForEvent["m.replace"];
if (!relationsWithRelType) {
return;
}
// only doing replacements for messages for now (e.g. edits)
const relationsWithEventType = relationsWithRelType["m.room.message"];

if (relationsWithEventType) {
relationsWithEventType.setTargetEvent(event);
}
};

/**
* Add relation events to the relevant relation collection.
*
Expand Down Expand Up @@ -747,6 +776,7 @@ EventTimelineSet.prototype.aggregateRelations = function(event) {
relationsWithRelType = relationsForEvent[relationType] = {};
}
let relationsWithEventType = relationsWithRelType[eventType];

if (!relationsWithEventType) {
relationsWithEventType = relationsWithRelType[eventType] = new Relations(
relationType,
Expand All @@ -755,6 +785,7 @@ EventTimelineSet.prototype.aggregateRelations = function(event) {
);
const relatesToEvent = this.findEventById(relatesToEventId);
if (relatesToEvent) {
relationsWithEventType.setTargetEvent(relatesToEvent);
relatesToEvent.emit("Event.relationsCreated", relationType, eventType);
}
}
Expand Down
34 changes: 22 additions & 12 deletions src/models/event.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,11 @@ utils.extend(module.exports.MatrixEvent.prototype, {
* @return {Object} The event content JSON, or an empty object.
*/
getContent: function() {
return this._clearEvent.content || this.event.content || {};
if (this._replacingEvent) {
return this._replacingEvent.getContent()["m.new_content"] || {};
} else {
return this._clearEvent.content || this.event.content || {};
}
},

/**
Expand Down Expand Up @@ -664,6 +668,7 @@ utils.extend(module.exports.MatrixEvent.prototype, {

this.emit("Event.beforeRedaction", this, redaction_event);

this._replacingEvent = null;
// we attempt to replicate what we would see from the server if
// the event had been redacted before we saw it.
//
Expand Down Expand Up @@ -786,24 +791,29 @@ utils.extend(module.exports.MatrixEvent.prototype, {
/**
* Set an event that replaces the content of this event, through an m.replace relation.
*
* @param {MatrixEvent} newEvent the event with the replacing content.
* @param {MatrixEvent?} newEvent the event with the replacing content, if any.
*/
makeReplaced(newEvent) {
if (this.isRedacted()) {
return;
}
// ignore previous replacements
if (this._replacingEvent && this._replacingEvent.getTs() > newEvent.getTs()) {
return;
if (this._replacingEvent !== newEvent) {
this._replacingEvent = newEvent;
this.emit("Event.replaced", this);
}
if (newEvent.isBeingDecrypted()) {
throw new Error("Trying to replace event when " +
"new content hasn't been decrypted yet");
},

/**
* Returns the status of the event, or the replacing event in case `makeReplace` has been called.
*
* @return {EventStatus}
*/
replacementOrOwnStatus() {
if (this._replacingEvent) {
return this._replacingEvent.status;
} else {
return this.status;
}
const oldContent = this.getContent();
const newContent = newEvent.getContent()["m.new_content"];
Object.assign(oldContent, newContent);
this._replacingEvent = newEvent;
},

/**
Expand Down
65 changes: 61 additions & 4 deletions src/models/relations.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export default class Relations extends EventEmitter {
this._annotationsByKey = {};
this._annotationsBySender = {};
this._sortedAnnotationsByKey = [];
this._targetEvent = null;
}

/**
Expand Down Expand Up @@ -77,12 +78,14 @@ export default class Relations extends EventEmitter {
event.on("Event.status", this._onEventStatus);
}

this._relations.add(event);

if (this.relationType === "m.annotation") {
this._addAnnotationToAggregation(event);
} else if (this.relationType === "m.replace" && this._targetEvent) {
this._targetEvent.makeReplaced(this.getLastReplacement());
}

this._relations.add(event);

event.on("Event.beforeRedaction", this._onBeforeRedaction);

this.emit("Relations.add", event);
Expand Down Expand Up @@ -113,12 +116,14 @@ export default class Relations extends EventEmitter {
return;
}

this._relations.delete(event);

if (this.relationType === "m.annotation") {
this._removeAnnotationFromAggregation(event);
} else if (this.relationType === "m.replace" && this._targetEvent) {
this._targetEvent.makeReplaced(this.getLastReplacement());
}

this._relations.delete(event);

this.emit("Relations.remove", event);
}

Expand Down Expand Up @@ -226,9 +231,13 @@ export default class Relations extends EventEmitter {
return;
}

this._relations.delete(redactedEvent);

if (this.relationType === "m.annotation") {
// Remove the redacted annotation from aggregation by key
this._removeAnnotationFromAggregation(redactedEvent);
} else if (this.relationType === "m.replace" && this._targetEvent) {
this._targetEvent.makeReplaced(this.getLastReplacement());
}

redactedEvent.removeListener("Event.beforeRedaction", this._onBeforeRedaction);
Expand Down Expand Up @@ -277,4 +286,52 @@ export default class Relations extends EventEmitter {

return this._annotationsBySender;
}

/**
* Returns the most recent (and allowed) m.replace relation, if any.
*
* This is currently only supported for the m.replace relation type,
* once the target event is known, see `addEvent`.
*
* @return {MatrixEvent?}
*/
getLastReplacement() {
if (this.relationType !== "m.replace") {
// Aggregating on last only makes sense for this relation type
return null;
}
if (!this._targetEvent) {
// Don't know which replacements to accept yet.
// This method shouldn't be called before the original
// event is known anyway.
return null;
}
return this.getRelations().reduce((last, event) => {
if (event.getSender() !== this._targetEvent.getSender()) {
return last;
}
if (last && last.getTs() > event.getTs()) {
return last;
}
return event;
}, null);
}

/*
* @param {MatrixEvent} targetEvent the event the relations are related to.
*/
setTargetEvent(event) {
if (this._targetEvent) {
return;
}
this._targetEvent = event;
if (this.relationType === "m.replace") {
const replacement = this.getLastReplacement();
// this is the initial update, so only call it if we already have something
// to not emit Event.replaced needlessly
if (replacement) {
this._targetEvent.makeReplaced(replacement);
}
}
}
}
52 changes: 13 additions & 39 deletions src/models/room.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ function synthesizeReceipt(userId, event, receiptType) {
* via `EventTimelineSet#getRelationsForEvent`.
* This feature is currently unstable and the API may change without notice.
*
* @param {boolean} [opts.unstableClientRelationReplacements = false]
* Optional. Set to true to enable client-side handling of m.replace event relations,
* exposed through the `Room.replaceEvent` event.
* This feature is currently unstable and the API may change without notice.
*
* @prop {string} roomId The ID of this room.
* @prop {string} name The human-readable display name for this room.
* @prop {Array<MatrixEvent>} timeline The live event timeline for this room,
Expand Down Expand Up @@ -1032,25 +1027,6 @@ Room.prototype._addLiveEvent = function(event, duplicateStrategy) {
// this may be needed to trigger an update.
}


if (this._opts.unstableClientRelationReplacements && event.isRelation("m.replace")) {
const relatesTo = event.getRelation();
const replacedId = relatesTo && relatesTo.event_id;
const replacedEvent = this.getUnfilteredTimelineSet().findEventById(replacedId);
if (replacedEvent && event.getSender() === replacedEvent.getSender()) {
const doAndEmitReplacement = () => {
replacedEvent.makeReplaced(event);
this.emit("Room.replaceEvent", replacedEvent, this);
};

if (event.isBeingDecrypted()) {
event.once("Event.decrypted", doAndEmitReplacement);
} else {
doAndEmitReplacement();
}
}
}

if (event.getUnsigned().transaction_id) {
const existingEvent = this._txnToEvent[event.getUnsigned().transaction_id];
if (existingEvent) {
Expand Down Expand Up @@ -1102,10 +1078,6 @@ Room.prototype._addLiveEvent = function(event, duplicateStrategy) {
* unique transaction id.
*/
Room.prototype.addPendingEvent = function(event, txnId) {
if (event.isRelation("m.replace")) {
return;
}

if (event.status !== EventStatus.SENDING) {
throw new Error("addPendingEvent called on an event with status " +
event.status);
Expand Down Expand Up @@ -1134,19 +1106,21 @@ Room.prototype.addPendingEvent = function(event, txnId) {
}
this._pendingEventList.push(event);

// For pending events, add them to the relations collection immediately.
// (The alternate case below already covers this as part of adding to
// the timeline set.)
// TODO: We should consider whether this means it would be a better
// design to lift the relations handling up to the room instead.
for (let i = 0; i < this._timelineSets.length; i++) {
const timelineSet = this._timelineSets[i];
if (timelineSet.getFilter()) {
if (this._filter.filterRoomTimeline([event]).length) {
if (event.isRelation()) {
// For pending events, add them to the relations collection immediately.
// (The alternate case below already covers this as part of adding to
// the timeline set.)
// TODO: We should consider whether this means it would be a better
// design to lift the relations handling up to the room instead.
for (let i = 0; i < this._timelineSets.length; i++) {
const timelineSet = this._timelineSets[i];
if (timelineSet.getFilter()) {
if (this._filter.filterRoomTimeline([event]).length) {
timelineSet.aggregateRelations(event);
}
} else {
timelineSet.aggregateRelations(event);
}
} else {
timelineSet.aggregateRelations(event);
}
}
} else {
Expand Down
2 changes: 0 additions & 2 deletions src/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,12 @@ SyncApi.prototype.createRoom = function(roomId) {
const {
timelineSupport,
unstableClientRelationAggregation,
unstableClientRelationReplacements,
} = client;
const room = new Room(roomId, client, client.getUserId(), {
lazyLoadMembers: this.opts.lazyLoadMembers,
pendingEventOrdering: this.opts.pendingEventOrdering,
timelineSupport,
unstableClientRelationAggregation,
unstableClientRelationReplacements,
});
client.reEmitter.reEmit(room, ["Room.name", "Room.timeline", "Room.redaction",
"Room.receipt", "Room.tags",
Expand Down

0 comments on commit fbf5352

Please sign in to comment.