From 8ce69e802d22322386e2e17ee27a26a41933d660 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 13 Nov 2014 11:55:02 +0000 Subject: [PATCH 1/5] SYWEB-152: Migrate IRC command logic to commands-service. --- syweb/webclient/app.js | 1 + .../components/matrix/commands-service.js | 164 ++++++++++++++++ syweb/webclient/index.html | 1 + syweb/webclient/room/room-controller.js | 182 ++---------------- 4 files changed, 180 insertions(+), 168 deletions(-) create mode 100644 syweb/webclient/components/matrix/commands-service.js diff --git a/syweb/webclient/app.js b/syweb/webclient/app.js index 35190a71f..f9d5c18ec 100644 --- a/syweb/webclient/app.js +++ b/syweb/webclient/app.js @@ -33,6 +33,7 @@ var matrixWebClient = angular.module('matrixWebClient', [ 'notificationService', 'recentsService', 'modelService', + 'commandsService', 'infinite-scroll', 'ui.bootstrap', 'monospaced.elastic' diff --git a/syweb/webclient/components/matrix/commands-service.js b/syweb/webclient/components/matrix/commands-service.js new file mode 100644 index 000000000..3c516ad1e --- /dev/null +++ b/syweb/webclient/components/matrix/commands-service.js @@ -0,0 +1,164 @@ +/* +Copyright 2014 OpenMarket Ltd + +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. +*/ + +'use strict'; + +/* +This service contains logic for parsing and performing IRC style commands. +*/ +angular.module('commandsService', []) +.factory('commandsService', ['$q', '$location', 'matrixService', 'modelService', function($q, $location, matrixService, modelService) { + + // create a rejected promise with the given message + var reject = function(msg) { + var deferred = $q.defer(); + deferred.reject({ + data: { + error: msg + } + }); + return deferred.promise; + }; + + // Change your nickname + var doNick = function(room_id, args) { + if (args) { + return matrixService.setDisplayName(args); + } + return reject("Usage: /nick "); + }; + + // Join a room + var doJoin = function(room_id, args) { + if (args) { + var matches = args.match(/^(\S+)$/); + if (matches) { + var room_alias = matches[1]; + $location.url("room/" + room_alias); + // NB: We don't need to actually do the join, since that happens + // automatically if we are not joined onto a room already when + // the page loads. + return reject("Joining "+room_alias); + } + } + return reject("Usage: /join "); + }; + + // Kick a user from the room with an optional reason + var doKick = function(room_id, args) { + if (args) { + var matches = args.match(/^(\S+?)( +(.*))?$/); + if (matches) { + return matrixService.kick(room_id, matches[1], matches[3]); + } + } + return reject("Usage: /kick []"); + }; + + // Ban a user from the room with an optional reason + var doBan = function(room_id, args) { + if (args) { + var matches = args.match(/^(\S+?)( +(.*))?$/); + if (matches) { + return matrixService.ban(room_id, matches[1], matches[3]); + } + } + return reject("Usage: /ban []"); + }; + + // Unban a user from the room + var doUnban = function(room_id, args) { + if (args) { + var matches = args.match(/^(\S+)$/); + if (matches) { + // Reset the user membership to "leave" to unban him + return matrixService.unban(room_id, matches[1]); + } + } + return reject("Usage: /unban "); + }; + + // Define the power level of a user + var doOp = function(room_id, args) { + if (args) { + var matches = args.match(/^(\S+?)( +(\d+))?$/); + var powerLevel = 50; // default power level for op + if (matches) { + var user_id = matches[1]; + if (matches.length === 4 && undefined !== matches[3]) { + powerLevel = parseInt(matches[3]); + } + if (powerLevel !== NaN) { + var powerLevelEvent = modelService.getRoom(room_id).current_room_state.state("m.room.power_levels"); + return matrixService.setUserPowerLevel(room_id, user_id, powerLevel, powerLevelEvent); + } + } + } + return reject("Usage: /op []"); + }; + + // Reset the power level of a user + var doDeop = function(room_id, args) { + if (args) { + var matches = args.match(/^(\S+)$/); + if (matches) { + var powerLevelEvent = modelService.getRoom(room_id).current_room_state.state("m.room.power_levels"); + return matrixService.setUserPowerLevel(room_id, args, undefined, powerLevelEvent); + } + } + return reject("Usage: /deop "); + }; + + + var commands = { + "nick": doNick, + "join": doJoin, + "kick": doKick, + "ban": doBan, + "unban": doUnban, + "op": doOp, + "deop": doDeop + }; + + return { + + /** + * Process the given text for commands and perform them. + * @param {String} roomId The room in which the input was performed. + * @param {String} input The raw text input by the user. + * @return {Promise} A promise of the pending command, or null if the + * input is not a command. + */ + processInput: function(roomId, input) { + // trim any trailing whitespace, as it can confuse the parser for + // IRC-style commands + input = input.replace(/\s+$/, ""); + if (input[0] === "/" && input[1] !== "/") { + var bits = input.match(/^(\S+?)( +(.*))?$/); + var cmd = bits[1].substring(1); + var args = bits[3]; + if (commands[cmd]) { + return commands[cmd](roomId, args); + } + return reject("Unrecognised IRC-style command: " + cmd); + } + return null; // not a command + } + + }; + +}]); + diff --git a/syweb/webclient/index.html b/syweb/webclient/index.html index 4bca320e7..fb3c3f528 100644 --- a/syweb/webclient/index.html +++ b/syweb/webclient/index.html @@ -45,6 +45,7 @@ + diff --git a/syweb/webclient/room/room-controller.js b/syweb/webclient/room/room-controller.js index 667020170..bea0db575 100644 --- a/syweb/webclient/room/room-controller.js +++ b/syweb/webclient/room/room-controller.js @@ -15,8 +15,8 @@ limitations under the License. */ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'angular-peity']) -.controller('RoomController', ['$modal', '$filter', '$scope', '$timeout', '$routeParams', '$location', '$rootScope', 'matrixService', 'mPresence', 'eventHandlerService', 'mFileUpload', 'matrixPhoneService', 'MatrixCall', 'notificationService', 'modelService', 'recentsService', - function($modal, $filter, $scope, $timeout, $routeParams, $location, $rootScope, matrixService, mPresence, eventHandlerService, mFileUpload, matrixPhoneService, MatrixCall, notificationService, modelService, recentsService) { +.controller('RoomController', ['$modal', '$filter', '$scope', '$timeout', '$routeParams', '$location', '$rootScope', 'matrixService', 'mPresence', 'eventHandlerService', 'mFileUpload', 'matrixPhoneService', 'MatrixCall', 'notificationService', 'modelService', 'recentsService', 'commandsService', + function($modal, $filter, $scope, $timeout, $routeParams, $location, $rootScope, matrixService, mPresence, eventHandlerService, mFileUpload, matrixPhoneService, MatrixCall, notificationService, modelService, recentsService, commandsService) { 'use strict'; var MESSAGES_PER_PAGINATION = 30; var THUMBNAIL_SIZE = 320; @@ -435,172 +435,18 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a // Store the command in the history history.push(input); - var promise; - var cmd; - var args; + var promise = commandsService.processInput($scope.room_id, input); var echo = false; + var isEmote = input.indexOf("/me ") === 0; - // Check for IRC style commands first - // trim any trailing whitespace, as it can confuse the parser for IRC-style commands - input = input.replace(/\s+$/, ""); - - if (input[0] === "/" && input[1] !== "/") { - var bits = input.match(/^(\S+?)( +(.*))?$/); - cmd = bits[1]; - args = bits[3]; - - console.log("cmd: " + cmd + ", args: " + args); - - switch (cmd) { - case "/me": - promise = matrixService.sendEmoteMessage($scope.room_id, args); - echo = true; - break; - - case "/nick": - // Change user display name - if (args) { - promise = matrixService.setDisplayName(args); - } - else { - $scope.feedback = "Usage: /nick "; - } - break; - - case "/join": - // Join a room - if (args) { - var matches = args.match(/^(\S+)$/); - if (matches) { - var room_alias = matches[1]; - if (room_alias.indexOf(':') == -1) { - // FIXME: actually track the :domain style name of our homeserver - // with or without port as is appropriate and append it at this point - } - - var room_id = modelService.getAliasToRoomIdMapping(room_alias); - console.log("joining " + room_alias + " id=" + room_id); - if ($scope.room) { // TODO actually check that you = join - // don't send a join event for a room you're already in. - $location.url("room/" + room_alias); - } - else { - promise = matrixService.joinAlias(room_alias).then( - function(response) { - // TODO: factor out the common housekeeping whenever we try to join a room or alias - matrixService.roomState(response.room_id).then( - function(response) { - eventHandlerService.handleEvents(response.data, false, true); - }, - function(error) { - $scope.feedback = "Failed to get room state for: " + response.room_id; - } - ); - $location.url("room/" + room_alias); - }, - function(error) { - $scope.feedback = "Can't join room: " + JSON.stringify(error.data); - } - ); - } - } - } - else { - $scope.feedback = "Usage: /join "; - } - break; - - case "/kick": - // Kick a user from the room with an optional reason - if (args) { - var matches = args.match(/^(\S+?)( +(.*))?$/); - if (matches) { - promise = matrixService.kick($scope.room_id, matches[1], matches[3]); - } - } - - if (!promise) { - $scope.feedback = "Usage: /kick []"; - } - break; - - case "/ban": - // Ban a user from the room with an optional reason - if (args) { - var matches = args.match(/^(\S+?)( +(.*))?$/); - if (matches) { - promise = matrixService.ban($scope.room_id, matches[1], matches[3]); - } - } - - if (!promise) { - $scope.feedback = "Usage: /ban []"; - } - break; - - case "/unban": - // Unban a user from the room - if (args) { - var matches = args.match(/^(\S+)$/); - if (matches) { - // Reset the user membership to "leave" to unban him - promise = matrixService.unban($scope.room_id, matches[1]); - } - } - - if (!promise) { - $scope.feedback = "Usage: /unban "; - } - break; - - case "/op": - // Define the power level of a user - if (args) { - var matches = args.match(/^(\S+?)( +(\d+))?$/); - var powerLevel = 50; // default power level for op - if (matches) { - var user_id = matches[1]; - if (matches.length === 4 && undefined !== matches[3]) { - powerLevel = parseInt(matches[3]); - } - if (powerLevel !== NaN) { - var powerLevelEvent = $scope.room.current_room_state.state("m.room.power_levels"); - promise = matrixService.setUserPowerLevel($scope.room_id, user_id, powerLevel, powerLevelEvent); - } - } - } - - if (!promise) { - $scope.feedback = "Usage: /op []"; - } - break; - - case "/deop": - // Reset the power level of a user - if (args) { - var matches = args.match(/^(\S+)$/); - if (matches) { - var powerLevelEvent = $scope.room.current_room_state.state("m.room.power_levels"); - promise = matrixService.setUserPowerLevel($scope.room_id, args, undefined, powerLevelEvent); - } - } - - if (!promise) { - $scope.feedback = "Usage: /deop "; - } - break; - - default: - $scope.feedback = ("Unrecognised IRC-style command: " + cmd); - break; - } - } - - // By default send this as a message unless it's an IRC-style command - if (!promise && !cmd) { - // Make the request - promise = matrixService.sendTextMessage($scope.room_id, input); + if (!promise) { // not a non-echoable command echo = true; + if (isEmote) { + promise = matrixService.sendEmoteMessage($scope.room_id, input.substring(4)); + } + else { + promise = matrixService.sendTextMessage($scope.room_id, input); + } } if (echo) { @@ -608,8 +454,8 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a // To do so, create a minimalist fake text message event and add it to the in-memory list of room messages var echoMessage = { content: { - body: (cmd === "/me" ? args : input), - msgtype: (cmd === "/me" ? "m.emote" : "m.text"), + body: (isEmote ? input.substring(4) : input), + msgtype: (isEmote ? "m.emote" : "m.text"), }, origin_server_ts: new Date().getTime(), // fake a timestamp room_id: $scope.room_id, @@ -642,7 +488,7 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a } }, function(error) { - $scope.feedback = "Request failed: " + error.data.error; + $scope.feedback = error.data.error; if (echoMessage) { // Mark the message as unsent for the rest of the page life From 51802854562351a04f8cfd248a9aefb25a5508e7 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 13 Nov 2014 11:58:28 +0000 Subject: [PATCH 2/5] SYWEB-152: Unbreak /me --- syweb/webclient/room/room-controller.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/syweb/webclient/room/room-controller.js b/syweb/webclient/room/room-controller.js index bea0db575..7094a703f 100644 --- a/syweb/webclient/room/room-controller.js +++ b/syweb/webclient/room/room-controller.js @@ -435,9 +435,13 @@ angular.module('RoomController', ['ngSanitize', 'matrixFilter', 'mFileInput', 'a // Store the command in the history history.push(input); - var promise = commandsService.processInput($scope.room_id, input); - var echo = false; var isEmote = input.indexOf("/me ") === 0; + var promise; + if (!isEmote) { + promise = commandsService.processInput($scope.room_id, input); + } + var echo = false; + if (!promise) { // not a non-echoable command echo = true; From 0a699df5e8fcb18098a1e53c60798de4bf51eef7 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 13 Nov 2014 12:33:43 +0000 Subject: [PATCH 3/5] Wipe the selected room ID on the home screen. --- syweb/webclient/home/home-controller.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/syweb/webclient/home/home-controller.js b/syweb/webclient/home/home-controller.js index 6a3c07929..a9538a030 100644 --- a/syweb/webclient/home/home-controller.js +++ b/syweb/webclient/home/home-controller.js @@ -17,8 +17,8 @@ limitations under the License. 'use strict'; angular.module('HomeController', ['matrixService', 'eventHandlerService', 'RecentsController']) -.controller('HomeController', ['$scope', '$location', 'matrixService', 'eventHandlerService', 'modelService', - function($scope, $location, matrixService, eventHandlerService, modelService) { +.controller('HomeController', ['$scope', '$location', 'matrixService', 'eventHandlerService', 'modelService', 'recentsService', + function($scope, $location, matrixService, eventHandlerService, modelService, recentsService) { $scope.config = matrixService.config(); $scope.public_rooms = []; @@ -46,6 +46,8 @@ angular.module('HomeController', ['matrixService', 'eventHandlerService', 'Recen $scope.newChat = { user: "" }; + + recentsService.setSelectedRoomId(undefined); var refresh = function() { From 11da8d0dffd330bc699b83972a78c9d20e7e5071 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 13 Nov 2014 16:34:51 +0200 Subject: [PATCH 4/5] remove nganimate dependency as it seems to feature disproportionately highly in the FF profiler, and removing it seems to have stopped my FF stalling for seconds on end --- syweb/webclient/app-controller.js | 8 +++++--- syweb/webclient/app.js | 1 - syweb/webclient/index.html | 2 ++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/syweb/webclient/app-controller.js b/syweb/webclient/app-controller.js index bbcf4ab5f..0c419a546 100644 --- a/syweb/webclient/app-controller.js +++ b/syweb/webclient/app-controller.js @@ -21,18 +21,20 @@ limitations under the License. 'use strict'; angular.module('MatrixWebClientController', ['matrixService', 'mPresence', 'eventStreamService']) -.controller('MatrixWebClientController', ['$scope', '$location', '$rootScope', '$timeout', '$animate', 'matrixService', 'mPresence', 'eventStreamService', 'eventHandlerService', 'matrixPhoneService', 'modelService', - function($scope, $location, $rootScope, $timeout, $animate, matrixService, mPresence, eventStreamService, eventHandlerService, matrixPhoneService, modelService) { +.controller('MatrixWebClientController', ['$scope', '$location', '$rootScope', '$timeout', 'matrixService', 'mPresence', 'eventStreamService', 'eventHandlerService', 'matrixPhoneService', 'modelService', + function($scope, $location, $rootScope, $timeout, matrixService, mPresence, eventStreamService, eventHandlerService, matrixPhoneService, modelService) { // Check current URL to avoid to display the logout button on the login page $scope.location = $location.path(); +/* // disable nganimate for the local and remote video elements because ngAnimate appears // to be buggy and leaves animation classes on the video elements causing them to show // when they should not (their animations are pure CSS3) $animate.enabled(false, angular.element('#localVideo')); $animate.enabled(false, angular.element('#remoteVideo')); - +*/ + // Update the location state when the ng location changed $rootScope.$on('$routeChangeSuccess', function (event, current, previous) { $scope.location = $location.path(); diff --git a/syweb/webclient/app.js b/syweb/webclient/app.js index f9d5c18ec..9e5b85820 100644 --- a/syweb/webclient/app.js +++ b/syweb/webclient/app.js @@ -16,7 +16,6 @@ limitations under the License. var matrixWebClient = angular.module('matrixWebClient', [ 'ngRoute', - 'ngAnimate', 'MatrixWebClientController', 'LoginController', 'RegisterController', diff --git a/syweb/webclient/index.html b/syweb/webclient/index.html index fb3c3f528..6211ecd62 100644 --- a/syweb/webclient/index.html +++ b/syweb/webclient/index.html @@ -16,7 +16,9 @@ + From cadcc6cabed8e6c679fc5cf58bb3809f8f1465ba Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Thu, 13 Nov 2014 14:35:53 +0000 Subject: [PATCH 5/5] Add commands-service unit tests. --- .../test/unit/commands-service.spec.js | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 syweb/webclient/test/unit/commands-service.spec.js diff --git a/syweb/webclient/test/unit/commands-service.spec.js b/syweb/webclient/test/unit/commands-service.spec.js new file mode 100644 index 000000000..142044f15 --- /dev/null +++ b/syweb/webclient/test/unit/commands-service.spec.js @@ -0,0 +1,143 @@ +describe('CommandsService', function() { + var scope; + var roomId = "!dlwifhweu:localhost"; + + var testPowerLevelsEvent, testMatrixServicePromise; + + var matrixService = { // these will be spyed on by jasmine, hence stub methods + setDisplayName: function(args){}, + kick: function(args){}, + ban: function(args){}, + unban: function(args){}, + setUserPowerLevel: function(args){} + }; + + var modelService = { + getRoom: function(roomId) { + return { + room_id: roomId, + current_room_state: { + events: { + "m.room.power_levels": testPowerLevelsEvent + }, + state: function(type, key) { + return key ? this.events[type+key] : this.events[type]; + } + } + }; + } + }; + + + // helper function for asserting promise outcomes + NOTHING = "[Promise]"; + RESOLVED = "[Resolved promise]"; + REJECTED = "[Rejected promise]"; + var expectPromise = function(promise, expects) { + var value = NOTHING; + promise.then(function(result) { + value = RESOLVED; + }, function(fail) { + value = REJECTED; + }); + scope.$apply(); + expect(value).toEqual(expects); + }; + + // setup the service and mocked dependencies + beforeEach(function() { + + // set default mock values + testPowerLevelsEvent = { + content: { + default: 50 + }, + user_id: "@foo:bar", + room_id: roomId + } + + // mocked dependencies + module(function ($provide) { + $provide.value('matrixService', matrixService); + $provide.value('modelService', modelService); + }); + + // tested service + module('commandsService'); + }); + + beforeEach(inject(function($rootScope, $q) { + scope = $rootScope; + testMatrixServicePromise = $q.defer(); + })); + + it('should reject a no-arg "/nick".', inject( + function(commandsService) { + var promise = commandsService.processInput(roomId, "/nick"); + expectPromise(promise, REJECTED); + })); + + it('should be able to set a /nick with multiple words.', inject( + function(commandsService) { + spyOn(matrixService, 'setDisplayName').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/nick Bob Smith"); + expect(matrixService.setDisplayName).toHaveBeenCalledWith("Bob Smith"); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /kick a user without a reason.', inject( + function(commandsService) { + spyOn(matrixService, 'kick').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/kick @bob:matrix.org"); + expect(matrixService.kick).toHaveBeenCalledWith(roomId, "@bob:matrix.org", undefined); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /kick a user with a reason.', inject( + function(commandsService) { + spyOn(matrixService, 'kick').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/kick @bob:matrix.org he smells"); + expect(matrixService.kick).toHaveBeenCalledWith(roomId, "@bob:matrix.org", "he smells"); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /ban a user without a reason.', inject( + function(commandsService) { + spyOn(matrixService, 'ban').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/ban @bob:matrix.org"); + expect(matrixService.ban).toHaveBeenCalledWith(roomId, "@bob:matrix.org", undefined); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /ban a user with a reason.', inject( + function(commandsService) { + spyOn(matrixService, 'ban').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/ban @bob:matrix.org he smells"); + expect(matrixService.ban).toHaveBeenCalledWith(roomId, "@bob:matrix.org", "he smells"); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /unban a user.', inject( + function(commandsService) { + spyOn(matrixService, 'unban').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/unban @bob:matrix.org"); + expect(matrixService.unban).toHaveBeenCalledWith(roomId, "@bob:matrix.org"); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /op a user.', inject( + function(commandsService) { + spyOn(matrixService, 'setUserPowerLevel').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/op @bob:matrix.org 50"); + expect(matrixService.setUserPowerLevel).toHaveBeenCalledWith(roomId, "@bob:matrix.org", 50, testPowerLevelsEvent); + expect(promise).toBe(testMatrixServicePromise); + })); + + it('should be able to /deop a user.', inject( + function(commandsService) { + spyOn(matrixService, 'setUserPowerLevel').and.returnValue(testMatrixServicePromise); + var promise = commandsService.processInput(roomId, "/deop @bob:matrix.org"); + expect(matrixService.setUserPowerLevel).toHaveBeenCalledWith(roomId, "@bob:matrix.org", undefined, testPowerLevelsEvent); + expect(promise).toBe(testMatrixServicePromise); + })); +});