From 219103a4e20c2bf0e29d0ee584d5b7eed6673a0f Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 16 Oct 2015 17:07:04 +0100 Subject: [PATCH 1/2] Yank out invite event from initialSync. Set stripped state events when recalculating invited rooms. --- lib/client.js | 26 +++++++++++++++----------- lib/models/room.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/lib/client.js b/lib/client.js index fa3e67846..65728223c 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1878,17 +1878,21 @@ function doInitialSync(client, historyLen) { data.rooms[i].state = []; } if (data.rooms[i].membership === "invite") { - // create fake invite state event (v1 sucks) - data.rooms[i].state.push({ - event_id: "$fake_" + room.roomId, - content: { - membership: "invite" - }, - state_key: client.credentials.userId, - user_id: data.rooms[i].inviter, - room_id: room.roomId, - type: "m.room.member" - }); + var inviteEvent = data.rooms[i].invite; + if (!inviteEvent) { + // fallback for servers which don't serve the invite key yet + inviteEvent = { + event_id: "$fake_" + room.roomId, + content: { + membership: "invite" + }, + state_key: client.credentials.userId, + user_id: data.rooms[i].inviter, + room_id: room.roomId, + type: "m.room.member" + }; + } + data.rooms[i].state.push(inviteEvent); } _processRoomEvents( diff --git a/lib/models/room.js b/lib/models/room.js index 3e398c50f..448e81a2c 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -6,6 +6,7 @@ var EventEmitter = require("events").EventEmitter; var RoomState = require("./room-state"); var RoomSummary = require("./room-summary"); +var MatrixEvent = require("./event").MatrixEvent; var utils = require("../utils"); /** @@ -209,6 +210,34 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { * @fires module:client~MatrixClient#event:"Room.name" */ Room.prototype.recalculate = function(userId) { + // set fake stripped state events if this is an invite room so logic remains + // consistent elsewhere. + var self = this; + var membershipEvent = this.currentState.getStateEvents( + "m.room.member", userId + ); + if (membershipEvent && membershipEvent.getContent().membership === "invite") { + var strippedStateEvents = membershipEvent.invite_room_state || []; + utils.forEach(strippedStateEvents, function(strippedEvent) { + var existingEvent = self.currentState.getStateEvents( + strippedEvent.type, strippedEvent.state_key + ); + if (!existingEvent) { + // set the fake stripped event instead + self.currentState.setStateEvents([new MatrixEvent({ + type: strippedEvent.type, + state_key: strippedEvent.state_key, + content: strippedEvent.content, + event_id: "$fake" + Date.now(), + room_id: self.roomId, + user_id: userId // technically a lie + })]); + } + }); + } + + + var oldName = this.name; this.name = calculateRoomName(this, userId); this.summary = new RoomSummary(this.roomId, { From 5ae87b7c95fe6cc20c8ff17f377a508329bd5737 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 16 Oct 2015 17:27:05 +0100 Subject: [PATCH 2/2] Bug fixes and unit tests --- lib/models/room.js | 2 +- spec/unit/room.spec.js | 308 +++++++++++++++++++++++------------------ 2 files changed, 178 insertions(+), 132 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index 448e81a2c..1f3df9a7a 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -217,7 +217,7 @@ Room.prototype.recalculate = function(userId) { "m.room.member", userId ); if (membershipEvent && membershipEvent.getContent().membership === "invite") { - var strippedStateEvents = membershipEvent.invite_room_state || []; + var strippedStateEvents = membershipEvent.event.invite_room_state || []; utils.forEach(strippedStateEvents, function(strippedEvent) { var existingEvent = self.currentState.getStateEvents( strippedEvent.type, strippedEvent.state_key diff --git a/spec/unit/room.spec.js b/spec/unit/room.spec.js index e0277a88f..ba3678592 100644 --- a/spec/unit/room.spec.js +++ b/spec/unit/room.spec.js @@ -337,7 +337,7 @@ describe("Room", function() { }); }); - describe("recalculate (Room Name)", function() { + describe("recalculate", function() { var stateLookup = { // event.type + "$" event.state_key : MatrixEvent }; @@ -404,149 +404,195 @@ describe("Room", function() { }); }); - it("should return the names of members in a private (invite join_rules)" + - " room if a room name and alias don't exist and there are >3 members.", - function() { - setJoinRule("invite"); - addMember(userA); - addMember(userB); - addMember(userC); - addMember(userD); - room.recalculate(userA); - var name = room.name; - // we expect at least 1 member to be mentioned - var others = [userB, userC, userD]; - var found = false; - for (var i = 0; i < others.length; i++) { - if (name.indexOf(others[i]) !== -1) { - found = true; - break; + describe("Room.recalculate => Stripped State Events", function() { + it("should set stripped state events as actual state events if the " + + "room is an invite room", function() { + var roomName = "flibble"; + + addMember(userA, "invite"); + stateLookup["m.room.member$" + userA].event.invite_room_state = [ + { + type: "m.room.name", + state_key: "", + content: { + name: roomName + } + } + ]; + + room.recalculate(userA); + expect(room.currentState.setStateEvents).toHaveBeenCalled(); + // first call, first arg (which is an array), first element in array + var fakeEvent = room.currentState.setStateEvents.calls[0].args[0][0]; + expect(fakeEvent.getContent()).toEqual({ + name: roomName + }); + }); + + it("should not clobber state events if it isn't an invite room", function() { + addMember(userA, "join"); + stateLookup["m.room.member$" + userA].event.invite_room_state = [ + { + type: "m.room.name", + state_key: "", + content: { + name: "flibble" + } + } + ]; + + room.recalculate(userA); + expect(room.currentState.setStateEvents).not.toHaveBeenCalled(); + }); + }); + + describe("Room.recalculate => Room Name", function() { + + it("should return the names of members in a private (invite join_rules)" + + " room if a room name and alias don't exist and there are >3 members.", + function() { + setJoinRule("invite"); + addMember(userA); + addMember(userB); + addMember(userC); + addMember(userD); + room.recalculate(userA); + var name = room.name; + // we expect at least 1 member to be mentioned + var others = [userB, userC, userD]; + var found = false; + for (var i = 0; i < others.length; i++) { + if (name.indexOf(others[i]) !== -1) { + found = true; + break; + } } - } - expect(found).toEqual(true, name); - }); + expect(found).toEqual(true, name); + }); - it("should return the names of members in a private (invite join_rules)" + - " room if a room name and alias don't exist and there are >2 members.", - function() { - setJoinRule("invite"); - addMember(userA); - addMember(userB); - addMember(userC); - room.recalculate(userA); - var name = room.name; - expect(name.indexOf(userB)).not.toEqual(-1, name); - expect(name.indexOf(userC)).not.toEqual(-1, name); - }); + it("should return the names of members in a private (invite join_rules)" + + " room if a room name and alias don't exist and there are >2 members.", + function() { + setJoinRule("invite"); + addMember(userA); + addMember(userB); + addMember(userC); + room.recalculate(userA); + var name = room.name; + expect(name.indexOf(userB)).not.toEqual(-1, name); + expect(name.indexOf(userC)).not.toEqual(-1, name); + }); - it("should return the names of members in a public (public join_rules)" + - " room if a room name and alias don't exist and there are >2 members.", - function() { - setJoinRule("public"); - addMember(userA); - addMember(userB); - addMember(userC); - room.recalculate(userA); - var name = room.name; - expect(name.indexOf(userB)).not.toEqual(-1, name); - expect(name.indexOf(userC)).not.toEqual(-1, name); - }); + it("should return the names of members in a public (public join_rules)" + + " room if a room name and alias don't exist and there are >2 members.", + function() { + setJoinRule("public"); + addMember(userA); + addMember(userB); + addMember(userC); + room.recalculate(userA); + var name = room.name; + expect(name.indexOf(userB)).not.toEqual(-1, name); + expect(name.indexOf(userC)).not.toEqual(-1, name); + }); - it("should show the other user's name for public (public join_rules)" + - " rooms if a room name and alias don't exist and it is a 1:1-chat.", - function() { - setJoinRule("public"); - addMember(userA); - addMember(userB); - room.recalculate(userA); - var name = room.name; - expect(name.indexOf(userB)).not.toEqual(-1, name); - }); + it("should show the other user's name for public (public join_rules)" + + " rooms if a room name and alias don't exist and it is a 1:1-chat.", + function() { + setJoinRule("public"); + addMember(userA); + addMember(userB); + room.recalculate(userA); + var name = room.name; + expect(name.indexOf(userB)).not.toEqual(-1, name); + }); - it("should show the other user's name for private " + - "(invite join_rules) rooms if a room name and alias don't exist and it" + - " is a 1:1-chat.", function() { - setJoinRule("invite"); - addMember(userA); - addMember(userB); - room.recalculate(userA); - var name = room.name; - expect(name.indexOf(userB)).not.toEqual(-1, name); - }); + it("should show the other user's name for private " + + "(invite join_rules) rooms if a room name and alias don't exist and it" + + " is a 1:1-chat.", function() { + setJoinRule("invite"); + addMember(userA); + addMember(userB); + room.recalculate(userA); + var name = room.name; + expect(name.indexOf(userB)).not.toEqual(-1, name); + }); - it("should show the other user's name for private" + - " (invite join_rules) rooms if you are invited to it.", function() { - setJoinRule("invite"); - addMember(userA, "invite"); - addMember(userB); - room.recalculate(userA); - var name = room.name; - expect(name.indexOf(userB)).not.toEqual(-1, name); - }); + it("should show the other user's name for private" + + " (invite join_rules) rooms if you are invited to it.", function() { + setJoinRule("invite"); + addMember(userA, "invite"); + addMember(userB); + room.recalculate(userA); + var name = room.name; + expect(name.indexOf(userB)).not.toEqual(-1, name); + }); - it("should show the room alias if one exists for private " + - "(invite join_rules) rooms if a room name doesn't exist.", function() { - var alias = "#room_alias:here"; - setJoinRule("invite"); - setAliases([alias, "#another:one"]); - room.recalculate(userA); - var name = room.name; - expect(name).toEqual(alias); - }); + it("should show the room alias if one exists for private " + + "(invite join_rules) rooms if a room name doesn't exist.", function() { + var alias = "#room_alias:here"; + setJoinRule("invite"); + setAliases([alias, "#another:one"]); + room.recalculate(userA); + var name = room.name; + expect(name).toEqual(alias); + }); - it("should show the room alias if one exists for public " + - "(public join_rules) rooms if a room name doesn't exist.", function() { - var alias = "#room_alias:here"; - setJoinRule("public"); - setAliases([alias, "#another:one"]); - room.recalculate(userA); - var name = room.name; - expect(name).toEqual(alias); - }); + it("should show the room alias if one exists for public " + + "(public join_rules) rooms if a room name doesn't exist.", function() { + var alias = "#room_alias:here"; + setJoinRule("public"); + setAliases([alias, "#another:one"]); + room.recalculate(userA); + var name = room.name; + expect(name).toEqual(alias); + }); - it("should show the room name if one exists for private " + - "(invite join_rules) rooms.", function() { - var roomName = "A mighty name indeed"; - setJoinRule("invite"); - setRoomName(roomName); - room.recalculate(userA); - var name = room.name; - expect(name).toEqual(roomName); - }); + it("should show the room name if one exists for private " + + "(invite join_rules) rooms.", function() { + var roomName = "A mighty name indeed"; + setJoinRule("invite"); + setRoomName(roomName); + room.recalculate(userA); + var name = room.name; + expect(name).toEqual(roomName); + }); - it("should show the room name if one exists for public " + - "(public join_rules) rooms.", function() { - var roomName = "A mighty name indeed"; - setJoinRule("public"); - setRoomName(roomName); - room.recalculate(userA); - var name = room.name; - expect(name).toEqual(roomName); - }); + it("should show the room name if one exists for public " + + "(public join_rules) rooms.", function() { + var roomName = "A mighty name indeed"; + setJoinRule("public"); + setRoomName(roomName); + room.recalculate(userA); + var name = room.name; + expect(name).toEqual(roomName); + }); - it("should show your name for private (invite join_rules) rooms if" + - " a room name and alias don't exist and it is a self-chat.", function() { - setJoinRule("invite"); - addMember(userA); - room.recalculate(userA); - var name = room.name; - expect(name).toEqual(userA); - }); + it("should show your name for private (invite join_rules) rooms if" + + " a room name and alias don't exist and it is a self-chat.", function() { + setJoinRule("invite"); + addMember(userA); + room.recalculate(userA); + var name = room.name; + expect(name).toEqual(userA); + }); - it("should show your name for public (public join_rules) rooms if a" + - " room name and alias don't exist and it is a self-chat.", function() { - setJoinRule("public"); - addMember(userA); - room.recalculate(userA); - var name = room.name; - expect(name).toEqual(userA); - }); + it("should show your name for public (public join_rules) rooms if a" + + " room name and alias don't exist and it is a self-chat.", function() { + setJoinRule("public"); + addMember(userA); + room.recalculate(userA); + var name = room.name; + expect(name).toEqual(userA); + }); + + it("should return '?' if there is no name, alias or members in the room.", + function() { + room.recalculate(userA); + var name = room.name; + expect(name).toEqual("?"); + }); - it("should return '?' if there is no name, alias or members in the room.", - function() { - room.recalculate(userA); - var name = room.name; - expect(name).toEqual("?"); }); }); });