From 5974e693d6c7405eaa10a86a4526fc5a1dc4c74c Mon Sep 17 00:00:00 2001 From: felk Date: Mon, 20 Sep 2021 20:49:36 +0200 Subject: [PATCH 1/3] split CommandProcessor into interface and implementation --- TPP.Core/Commands/CommandProcessor.cs | 10 +++++++++- TPP.Core/Commands/Definitions/HelpCommand.cs | 4 ++-- TPP.Core/Modes/ModeBase.cs | 4 ++-- TPP.Core/Setups.cs | 6 +++--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/TPP.Core/Commands/CommandProcessor.cs b/TPP.Core/Commands/CommandProcessor.cs index 1b1050f9..644bdea5 100644 --- a/TPP.Core/Commands/CommandProcessor.cs +++ b/TPP.Core/Commands/CommandProcessor.cs @@ -10,11 +10,19 @@ namespace TPP.Core.Commands { + public interface ICommandProcessor + { + public Task Process(string commandName, IImmutableList args, Message message); + public Command? FindCommand(string commandName); + public void InstallCommand(Command command); + public void UninstallCommand(params string[] commandOrAlias); + } + /// /// The command processor can be configured using instances to have commands, /// which then get executed using the method. /// - public class CommandProcessor + public class CommandProcessor : ICommandProcessor { /// /// maximum execution time for a command before a warning is logged. diff --git a/TPP.Core/Commands/Definitions/HelpCommand.cs b/TPP.Core/Commands/Definitions/HelpCommand.cs index beb962f5..128d137f 100644 --- a/TPP.Core/Commands/Definitions/HelpCommand.cs +++ b/TPP.Core/Commands/Definitions/HelpCommand.cs @@ -11,8 +11,8 @@ public class HelpCommand Description = "Get general help, or info on a specific command like: \"!help balance\"" }; - private readonly CommandProcessor _commandProcessor; - public HelpCommand(CommandProcessor commandProcessor) => _commandProcessor = commandProcessor; + private readonly ICommandProcessor _commandProcessor; + public HelpCommand(ICommandProcessor commandProcessor) => _commandProcessor = commandProcessor; private CommandResult Execute(CommandContext context) { diff --git a/TPP.Core/Modes/ModeBase.cs b/TPP.Core/Modes/ModeBase.cs index 4682b6cf..3d680932 100644 --- a/TPP.Core/Modes/ModeBase.cs +++ b/TPP.Core/Modes/ModeBase.cs @@ -25,7 +25,7 @@ public sealed class ModeBase : IDisposable private readonly ILogger _logger; private readonly IImmutableDictionary _chats; private readonly IImmutableDictionary _commandResponders; - private readonly IImmutableDictionary _commandProcessors; + private readonly IImmutableDictionary _commandProcessors; private readonly IImmutableDictionary _moderators; private readonly IImmutableDictionary _advertisePollsWorkers; private readonly IMessagequeueRepo _messagequeueRepo; @@ -105,7 +105,7 @@ public ModeBase( public void InstallAdditionalCommand(Command command) { - foreach (CommandProcessor commandProcessor in _commandProcessors.Values) + foreach (ICommandProcessor commandProcessor in _commandProcessors.Values) commandProcessor.InstallCommand(command); } diff --git a/TPP.Core/Setups.cs b/TPP.Core/Setups.cs index 780d3e62..62e4be7a 100644 --- a/TPP.Core/Setups.cs +++ b/TPP.Core/Setups.cs @@ -59,7 +59,7 @@ public static ArgsParser SetUpArgsParser(IUserRepo userRepo, PokedexData pokedex return argsParser; } - public static CommandProcessor SetUpCommandProcessor( + public static ICommandProcessor SetUpCommandProcessor( ILoggerFactory loggerFactory, ArgsParser argsParser, Databases databases, @@ -68,7 +68,7 @@ public static CommandProcessor SetUpCommandProcessor( IChatModeChanger chatModeChanger, IImmutableSet knownSpecies) { - var commandProcessor = new CommandProcessor( + ICommandProcessor commandProcessor = new CommandProcessor( loggerFactory.CreateLogger(), databases.CommandLogger, argsParser); @@ -180,7 +180,7 @@ public static (WebsocketBroadcastServer, OverlayConnection) SetUpOverlayServer( } private static void SetUpDynamicCommands( - ILogger logger, CommandProcessor commandProcessor, IResponseCommandRepo responseCommandRepo) + ILogger logger, ICommandProcessor commandProcessor, IResponseCommandRepo responseCommandRepo) { IImmutableList commands = responseCommandRepo.GetCommands().Result; From 862a1f4ba063b043e2454752715573dfe1795cdc Mon Sep 17 00:00:00 2001 From: felk Date: Mon, 20 Sep 2021 20:50:08 +0200 Subject: [PATCH 2/3] CommandProcessor: implement per-user load factor limits --- TPP.Core/Commands/CommandProcessor.cs | 60 ++++++++++++++- TPP.Core/Setups.cs | 2 +- .../Commands/CommandProcessorTest.cs | 77 ++++++++++++++++--- 3 files changed, 125 insertions(+), 14 deletions(-) diff --git a/TPP.Core/Commands/CommandProcessor.cs b/TPP.Core/Commands/CommandProcessor.cs index 644bdea5..d16963db 100644 --- a/TPP.Core/Commands/CommandProcessor.cs +++ b/TPP.Core/Commands/CommandProcessor.cs @@ -5,7 +5,10 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using NodaTime; using TPP.ArgsParsing; +using TPP.Core.Utils; +using TPP.Model; using TPP.Persistence; namespace TPP.Core.Commands @@ -32,16 +35,45 @@ public class CommandProcessor : ICommandProcessor private readonly ILogger _logger; private readonly ICommandLogger _commandLogger; private readonly ArgsParser _argsParser; + private readonly IClock _clock; + private readonly Dictionary _commands = new(); + private readonly float _maxLoadFactor; + private readonly Duration _maxLoadFactorTimeframe; + private readonly float _additionalLoadFactorAtHighThreshold; + private Dictionary> _loadsPerUser = new(); + + /// + /// Create a new command processor instance + /// + /// logger + /// command logger + /// args parser instance + /// clock + /// maximum load factor before commands are silently dropped + /// timeframe for which the load factor is computed + /// + /// additional load to add to the load factor when a user is at their maximum load capacity. + /// It is linearly interpolated from 0 when there are no messages within the timeframe, + /// up to the supplied number multiplier when at the maximum amount of messages within the timeframe. + /// This is to have the load factor be more effective against continuous spam than sporadic bursts. public CommandProcessor( ILogger logger, ICommandLogger commandLogger, - ArgsParser argsParser) + ArgsParser argsParser, + IClock clock, + float maxLoadFactor = 200f, + Duration? maxLoadFactorTimeframe = null, + float additionalLoadFactorAtHighThreshold = 2f) { _logger = logger; _commandLogger = commandLogger; _argsParser = argsParser; + _clock = clock; + _maxLoadFactor = maxLoadFactor; + _maxLoadFactorTimeframe = maxLoadFactorTimeframe ?? Duration.FromMinutes(10); + _additionalLoadFactorAtHighThreshold = additionalLoadFactorAtHighThreshold; } public void InstallCommand(Command command) @@ -77,8 +109,34 @@ public void UninstallCommand(params string[] commandOrAlias) public Command? FindCommand(string commandName) => _commands.TryGetValue(commandName.ToLower(), out Command command) ? command : null; + private float CheckAndUpdateLoadFactorForUser(User user) + { + _loadsPerUser = _loadsPerUser + .Where(kvp => kvp.Value.Count > 0) + .ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + if (!_loadsPerUser.TryGetValue(user, out TtlQueue? loads)) + { + loads = new TtlQueue(_maxLoadFactorTimeframe, _clock); + _loadsPerUser[user] = loads; + } + float sum = loads.Sum(); + float ratioFilled = Math.Min(1, sum / _maxLoadFactor); + float toAdd = 1 + ratioFilled * _additionalLoadFactorAtHighThreshold; + loads.Enqueue(toAdd); + return sum + toAdd; + } + public async Task Process(string commandName, IImmutableList args, Message message) { + float loadFactor = CheckAndUpdateLoadFactorForUser(message.User); + if (loadFactor > _maxLoadFactor) + { + _logger.LogDebug( + "command '{Command}' from user {User} ignored because load factor is {LoadFactor} " + + "for timeframe {Duration}, which is above the maximum of {MaxLoadFactor}", + commandName, message.User, loadFactor, _maxLoadFactorTimeframe, _maxLoadFactor); + return new CommandResult(); + } if (!_commands.TryGetValue(commandName.ToLower(), out Command command)) { _logger.LogDebug("unknown command '{Command}'", commandName); diff --git a/TPP.Core/Setups.cs b/TPP.Core/Setups.cs index 62e4be7a..33532590 100644 --- a/TPP.Core/Setups.cs +++ b/TPP.Core/Setups.cs @@ -70,7 +70,7 @@ public static ICommandProcessor SetUpCommandProcessor( { ICommandProcessor commandProcessor = new CommandProcessor( loggerFactory.CreateLogger(), - databases.CommandLogger, argsParser); + databases.CommandLogger, argsParser, SystemClock.Instance); IEnumerable commands = new[] { diff --git a/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs b/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs index 42a6e630..acdde0d4 100644 --- a/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs +++ b/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs @@ -20,19 +20,20 @@ public class CommandProcessorTest private readonly ILogger _nullLogger = new NullLogger(); private readonly Mock _commandLoggerMock = new(); private readonly ImmutableList _noArgs = ImmutableList.Empty; - private readonly User _mockUser = new User( + private static User MockUser() => new( id: Guid.NewGuid().ToString(), name: "MockUser", twitchDisplayName: "☺MockUser", simpleName: "mockuser", color: null, firstActiveAt: Instant.FromUnixTimeSeconds(0), lastActiveAt: Instant.FromUnixTimeSeconds(0), lastMessageAt: null, pokeyen: 0, tokens: 0); + private readonly User _mockUser = MockUser(); - private Message MockMessage(string text = "") - => new Message(_mockUser, text, MessageSource.Chat, string.Empty); + private Message MockMessage(string text = "") => new(_mockUser, text, MessageSource.Chat, string.Empty); [Test] public async Task TestUnknownCommand() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); CommandResult? result = await commandProcessor.Process("unknown", _noArgs, MockMessage()); @@ -44,7 +45,8 @@ public async Task TestUnknownCommand() public async Task TestLogSlowCommand() { var loggerMock = new Mock>(); - var commandProcessor = new CommandProcessor(loggerMock.Object, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + loggerMock.Object, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("slow", async _ => { await Task.Delay(TimeSpan.FromMilliseconds(1050)); @@ -62,7 +64,8 @@ public async Task TestLogSlowCommand() public async Task TestCommandThrowsError() { var loggerMock = new Mock>(); - var commandProcessor = new CommandProcessor(loggerMock.Object, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + loggerMock.Object, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("broken", _ => throw new InvalidOperationException("this command is busted!"))); @@ -78,7 +81,8 @@ public async Task TestCommandThrowsError() [Test] public async Task TestCaseInsensitive() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("MiXeD", CommandUtils.StaticResponse("Hi!"))); foreach (string command in ImmutableList.Create("MiXeD", "mixed", "MIXED")) @@ -91,7 +95,8 @@ public async Task TestCaseInsensitive() [Test] public async Task TestAliases() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("main", CommandUtils.StaticResponse("Hi!")) { Aliases = new[] { "alias1", "alias2" } }); @@ -105,7 +110,8 @@ public async Task TestAliases() [Test] public void InstallConflictName() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("a", CommandUtils.StaticResponse("Hi!"))); ArgumentException ex = Assert.Throws(() => commandProcessor @@ -116,7 +122,8 @@ public void InstallConflictName() [Test] public void InstallConflictAlias() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("a", CommandUtils.StaticResponse("Hi!")) { Aliases = new[] { "x" } }); @@ -128,7 +135,8 @@ public void InstallConflictAlias() [Test] public void InstallConflictNameVsAlias() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("a", CommandUtils.StaticResponse("Hi!")) { Aliases = new[] { "b" } }); @@ -140,7 +148,8 @@ public void InstallConflictNameVsAlias() [Test] public async Task TestPermissions() { - var commandProcessor = new CommandProcessor(_nullLogger, _commandLoggerMock.Object, new ArgsParser()); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("opsonly", CommandUtils.StaticResponse("you are an operator")).WithCondition( canExecute: ctx => IsOperator(ctx.Message.User), ersatzResult: new CommandResult { Response = "Only operators can use that command" })); @@ -158,5 +167,49 @@ bool IsOperator(User user) => CommandResult? opResult = await commandProcessor.Process("opsonly", _noArgs, new Message(op, "", MessageSource.Chat, "")); Assert.That(opResult?.Response, Is.EqualTo("you are an operator")); } + + [Test] + public async Task MaxCommandsPerUser() + { + Mock clockMock = new(); + var commandProcessor = new CommandProcessor( + _nullLogger, _commandLoggerMock.Object, new ArgsParser(), clockMock.Object, + maxLoadFactor: 6, maxLoadFactorTimeframe: Duration.FromSeconds(10), + additionalLoadFactorAtHighThreshold: 6); + + commandProcessor.InstallCommand(new Command("foo", + _ => Task.FromResult(new CommandResult {Response = "yes!"}))); + + clockMock.Setup(clock => clock.GetCurrentInstant()).Returns(Instant.FromUnixTimeSeconds(0)); + CommandResult? resultOk1 = await commandProcessor.Process( + "foo", ImmutableList.Create(""), new Message(_mockUser, "", MessageSource.Chat, "")); + + // has +1 additional load factor because the load factor is already at 1/6, which * 6 additional load is 1 + // result is a total load of 3 + clockMock.Setup(clock => clock.GetCurrentInstant()).Returns(Instant.FromUnixTimeSeconds(5)); + CommandResult? resultOk2 = await commandProcessor.Process( + "foo", ImmutableList.Create(""), new Message(_mockUser, "", MessageSource.Chat, "")); + + // at 50% load already. this gets rejected and adds an additional +3 load (50% of additional 6 load) + // result is a total load of 7 + clockMock.Setup(clock => clock.GetCurrentInstant()).Returns(Instant.FromUnixTimeSeconds(10)); + CommandResult? resultNo = await commandProcessor.Process( + "foo", ImmutableList.Create(""), new Message(_mockUser, "", MessageSource.Chat, "")); + + // make sure this is per-user + CommandResult? resultOkOtherUser = await commandProcessor.Process( + "foo", ImmutableList.Create(""), new Message(MockUser(), "", MessageSource.Chat, "")); + + // letting everything so far expire lets the user use commands again + clockMock.Setup(clock => clock.GetCurrentInstant()).Returns(Instant.FromUnixTimeSeconds(21)); + CommandResult? resultOk3 = await commandProcessor.Process( + "foo", ImmutableList.Create(""), new Message(_mockUser, "", MessageSource.Chat, "")); + + Assert.That(resultOk1?.Response, Is.EqualTo("yes!")); + Assert.That(resultOk2?.Response, Is.EqualTo("yes!")); + Assert.That(resultNo?.Response, Is.Null); + Assert.That(resultOkOtherUser?.Response, Is.EqualTo("yes!")); + Assert.That(resultOk3?.Response, Is.EqualTo("yes!")); + } } } From e87d729536ec2948d34226e532358a782718113f Mon Sep 17 00:00:00 2001 From: felk Date: Mon, 20 Sep 2021 20:47:33 +0200 Subject: [PATCH 3/3] fix formatting in CommandProcessorTest --- .../Commands/CommandProcessorTest.cs | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs b/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs index acdde0d4..924f6a0d 100644 --- a/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs +++ b/tests/TPP.Core.Tests/Commands/CommandProcessorTest.cs @@ -73,7 +73,7 @@ public async Task TestCommandThrowsError() Assert.That(result?.Response, Is.EqualTo("An error occurred.")); string errorTextRegex = @"^An exception occured while executing command 'broken'\. " + - $@"User: {Regex.Escape(_mockUser.ToString())}, Original text: bla$"; + $@"User: {Regex.Escape(_mockUser.ToString())}, Original text: bla$"; loggerMock.VerifyLog(logger => logger.LogError( new InvalidOperationException("this command is busted!"), It.IsRegex(errorTextRegex))); } @@ -98,7 +98,9 @@ public async Task TestAliases() var commandProcessor = new CommandProcessor( _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("main", CommandUtils.StaticResponse("Hi!")) - { Aliases = new[] { "alias1", "alias2" } }); + { + Aliases = new[] { "alias1", "alias2" } + }); foreach (string command in ImmutableList.Create("main", "alias1", "ALIAS2")) { @@ -126,7 +128,9 @@ public void InstallConflictAlias() _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("a", CommandUtils.StaticResponse("Hi!")) - { Aliases = new[] { "x" } }); + { + Aliases = new[] { "x" } + }); ArgumentException ex = Assert.Throws(() => commandProcessor .InstallCommand(new Command("b", CommandUtils.StaticResponse("Hi!")) { Aliases = new[] { "X" } }))!; Assert.That(ex.Message, Is.EqualTo("The alias 'x' conflicts with: a(x): ")); @@ -139,7 +143,9 @@ public void InstallConflictNameVsAlias() _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); commandProcessor.InstallCommand(new Command("a", CommandUtils.StaticResponse("Hi!")) - { Aliases = new[] { "b" } }); + { + Aliases = new[] { "b" } + }); ArgumentException ex = Assert.Throws(() => commandProcessor .InstallCommand(new Command("b", CommandUtils.StaticResponse("Hi!")) { Aliases = new[] { "x" } }))!; Assert.That(ex.Message, Is.EqualTo("The command name 'b' conflicts with: a(b): ")); @@ -150,21 +156,24 @@ public async Task TestPermissions() { var commandProcessor = new CommandProcessor( _nullLogger, _commandLoggerMock.Object, new ArgsParser(), Mock.Of()); - commandProcessor.InstallCommand(new Command("opsonly", CommandUtils.StaticResponse("you are an operator")).WithCondition( - canExecute: ctx => IsOperator(ctx.Message.User), - ersatzResult: new CommandResult { Response = "Only operators can use that command" })); + commandProcessor.InstallCommand( + new Command("opsonly", CommandUtils.StaticResponse("you are an operator")).WithCondition( + canExecute: ctx => IsOperator(ctx.Message.User), + ersatzResult: new CommandResult { Response = "Only operators can use that command" })); bool IsOperator(User user) => user.Roles.Contains(Role.Operator); - User op = new User( - id: Guid.NewGuid().ToString(), - name: "operator", twitchDisplayName: "operator", simpleName: "mockoperator", color: null, - firstActiveAt: Instant.FromUnixTimeSeconds(0), lastActiveAt: Instant.FromUnixTimeSeconds(0), - lastMessageAt: null, pokeyen: 0, tokens: 0, roles: new HashSet { Role.Operator }); - - CommandResult? userResult = await commandProcessor.Process("opsonly", _noArgs, new Message(_mockUser, "", MessageSource.Chat, "")); + User op = new( + id: Guid.NewGuid().ToString(), + name: "operator", twitchDisplayName: "operator", simpleName: "mockoperator", color: null, + firstActiveAt: Instant.FromUnixTimeSeconds(0), lastActiveAt: Instant.FromUnixTimeSeconds(0), + lastMessageAt: null, pokeyen: 0, tokens: 0, roles: new HashSet { Role.Operator }); + + CommandResult? userResult = await commandProcessor.Process( + "opsonly", _noArgs, new Message(_mockUser, "", MessageSource.Chat, "")); Assert.That(userResult?.Response, Is.EqualTo("Only operators can use that command")); - CommandResult? opResult = await commandProcessor.Process("opsonly", _noArgs, new Message(op, "", MessageSource.Chat, "")); + CommandResult? opResult = await commandProcessor.Process( + "opsonly", _noArgs, new Message(op, "", MessageSource.Chat, "")); Assert.That(opResult?.Response, Is.EqualTo("you are an operator")); } @@ -178,7 +187,7 @@ public async Task MaxCommandsPerUser() additionalLoadFactorAtHighThreshold: 6); commandProcessor.InstallCommand(new Command("foo", - _ => Task.FromResult(new CommandResult {Response = "yes!"}))); + _ => Task.FromResult(new CommandResult { Response = "yes!" }))); clockMock.Setup(clock => clock.GetCurrentInstant()).Returns(Instant.FromUnixTimeSeconds(0)); CommandResult? resultOk1 = await commandProcessor.Process(