From d6eb6bc54a0138112ea16203d0e7df0925499055 Mon Sep 17 00:00:00 2001 From: Gerard Smit Date: Wed, 21 Oct 2020 14:55:03 +0200 Subject: [PATCH] Implemented EventListener priorities (#62) * Improved EventManager Now it actually will order the event listeners on their priority. Also I've cleaned up the registering of temporary event listeners. * Ignore new Impostor.Api library while loading plugins * Fixed Dockerfile --- Dockerfile | 13 +- .../Attributes/EventListenerAttribute.cs | 5 - .../Events/Managers/IEventManager.cs | 9 - src/Impostor.Server/Events/EventHandler.cs | 7 +- src/Impostor.Server/Events/EventManager.cs | 62 +++---- .../Register/IRegisteredEventListener.cs | 15 ++ .../InvokedRegisteredEventListener.cs | 27 +++ .../{ => Register}/RegisteredEventListener.cs | 5 +- .../Events/Register/TemporaryEventRegister.cs | 60 +++++++ .../WrappedRegisteredEventListener.cs | 27 +++ .../Events/TemporaryEventRegister.cs | 79 -------- src/Impostor.Server/Plugins/PluginLoader.cs | 5 +- .../Events/EventManagerTests.cs | 168 ++++++++++++++++++ 13 files changed, 336 insertions(+), 146 deletions(-) create mode 100644 src/Impostor.Server/Events/Register/IRegisteredEventListener.cs create mode 100644 src/Impostor.Server/Events/Register/InvokedRegisteredEventListener.cs rename src/Impostor.Server/Events/{ => Register}/RegisteredEventListener.cs (98%) create mode 100644 src/Impostor.Server/Events/Register/TemporaryEventRegister.cs create mode 100644 src/Impostor.Server/Events/Register/WrappedRegisteredEventListener.cs delete mode 100644 src/Impostor.Server/Events/TemporaryEventRegister.cs create mode 100644 src/Impostor.Tests/Events/EventManagerTests.cs diff --git a/Dockerfile b/Dockerfile index 49fddc9..3066607 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,10 +8,8 @@ WORKDIR /source # Copy csproj and restore. COPY src/Impostor.Server/Impostor.Server.csproj ./src/Impostor.Server/Impostor.Server.csproj -COPY src/Impostor.Server.Api/Impostor.Server.Api.csproj ./src/Impostor.Server.Api/Impostor.Server.Api.csproj -COPY src/Impostor.Server.Hazel/Impostor.Server.Hazel.csproj ./src/Impostor.Server.Hazel/Impostor.Server.Hazel.csproj -COPY src/Impostor.Shared/Impostor.Shared.csproj ./src/Impostor.Shared/Impostor.Shared.csproj -COPY submodules/Hazel-Networking/Hazel/Hazel.csproj ./submodules/Hazel-Networking/Hazel/Hazel.csproj +COPY src/Impostor.Api/Impostor.Api.csproj ./src/Impostor.Api/Impostor.Api.csproj +COPY src/Impostor.Hazel/Impostor.Hazel.csproj ./src/Impostor.Hazel/Impostor.Hazel.csproj RUN case "$TARGETARCH" in \ amd64) NETCORE_PLATFORM='linux-x64';; \ @@ -20,13 +18,10 @@ RUN case "$TARGETARCH" in \ *) echo "unsupported architecture"; exit 1 ;; \ esac && \ dotnet restore -r "$NETCORE_PLATFORM" ./src/Impostor.Server/Impostor.Server.csproj && \ - dotnet restore -r "$NETCORE_PLATFORM" ./src/Impostor.Server.Api/Impostor.Server.Api.csproj && \ - dotnet restore -r "$NETCORE_PLATFORM" ./src/Impostor.Server.Hazel/Impostor.Server.Hazel.csproj && \ - dotnet restore -r "$NETCORE_PLATFORM" ./src/Impostor.Shared/Impostor.Shared.csproj && \ - dotnet restore -r "$NETCORE_PLATFORM" ./submodules/Hazel-Networking/Hazel/Hazel.csproj + dotnet restore -r "$NETCORE_PLATFORM" ./src/Impostor.Api/Impostor.Api.csproj && \ + dotnet restore -r "$NETCORE_PLATFORM" ./src/Impostor.Hazel/Impostor.Hazel.csproj # Copy everything else. -COPY submodules/. ./submodules/ COPY src/. ./src/ RUN case "$TARGETARCH" in \ amd64) NETCORE_PLATFORM='linux-x64';; \ diff --git a/src/Impostor.Api/Events/Attributes/EventListenerAttribute.cs b/src/Impostor.Api/Events/Attributes/EventListenerAttribute.cs index a373946..b31d2d1 100644 --- a/src/Impostor.Api/Events/Attributes/EventListenerAttribute.cs +++ b/src/Impostor.Api/Events/Attributes/EventListenerAttribute.cs @@ -30,10 +30,5 @@ namespace Impostor.Api.Events /// If set to true, the listener will be called regardless of the . /// public bool IgnoreCancelled { get; set; } - - /// - /// The order of the priority. - /// - public int PriorityOrder { get; set; } = 100; } } \ No newline at end of file diff --git a/src/Impostor.Api/Events/Managers/IEventManager.cs b/src/Impostor.Api/Events/Managers/IEventManager.cs index a9d433b..07a7f7c 100644 --- a/src/Impostor.Api/Events/Managers/IEventManager.cs +++ b/src/Impostor.Api/Events/Managers/IEventManager.cs @@ -5,15 +5,6 @@ namespace Impostor.Api.Events.Managers { public interface IEventManager { - /// - /// Register a temporary event listener. - /// - /// Event callback. - /// Disposable that unregisters the callback from the event manager. - /// Type of the event. - IDisposable Register(Func callback) - where TEvent : IEvent; - /// /// Register a temporary event listener. /// diff --git a/src/Impostor.Server/Events/EventHandler.cs b/src/Impostor.Server/Events/EventHandler.cs index e4582ca..190f7f3 100644 --- a/src/Impostor.Server/Events/EventHandler.cs +++ b/src/Impostor.Server/Events/EventHandler.cs @@ -1,10 +1,11 @@ using Impostor.Api.Events; +using Impostor.Server.Events.Register; namespace Impostor.Server.Events { internal readonly struct EventHandler { - public EventHandler(IEventListener o, RegisteredEventListener listener) + public EventHandler(IEventListener o, IRegisteredEventListener listener) { Object = o; Listener = listener; @@ -12,9 +13,9 @@ namespace Impostor.Server.Events public IEventListener Object { get; } - public RegisteredEventListener Listener { get; } + public IRegisteredEventListener Listener { get; } - public void Deconstruct(out IEventListener o, out RegisteredEventListener listener) + public void Deconstruct(out IEventListener o, out IRegisteredEventListener listener) { o = Object; listener = Listener; diff --git a/src/Impostor.Server/Events/EventManager.cs b/src/Impostor.Server/Events/EventManager.cs index 05a3e70..504fcd0 100644 --- a/src/Impostor.Server/Events/EventManager.cs +++ b/src/Impostor.Server/Events/EventManager.cs @@ -6,30 +6,20 @@ using System.Reflection; using System.Threading.Tasks; using Impostor.Api.Events; using Impostor.Api.Events.Managers; +using Impostor.Server.Events.Register; using Microsoft.Extensions.DependencyInjection; namespace Impostor.Server.Events { internal class EventManager : IEventManager { - private readonly ConcurrentDictionary _temporaryEventListeners; + private readonly ConcurrentDictionary _temporaryEventListeners; private readonly IServiceProvider _serviceProvider; public EventManager(IServiceProvider serviceProvider) { _serviceProvider = serviceProvider; - _temporaryEventListeners = new ConcurrentDictionary(); - } - - /// - public IDisposable Register(Func callback) - where TEvent : IEvent - { - var register = (TemporaryEventRegister) _temporaryEventListeners.GetOrAdd( - typeof(TEvent), - _ => new TemporaryEventRegister()); - - return register.Add(callback); + _temporaryEventListeners = new ConcurrentDictionary(); } /// @@ -41,17 +31,23 @@ namespace Impostor.Server.Events throw new ArgumentNullException(nameof(listener)); } - var registerMethod = typeof(EventManager).GetMethod(nameof(RegisterListenerImpl), BindingFlags.Instance | BindingFlags.NonPublic); - var methods = RegisteredEventListener.FromType(listener.GetType()); - var disposes = new IDisposable[methods.Count]; + var eventListeners = RegisteredEventListener.FromType(listener.GetType()); + var disposes = new IDisposable[eventListeners.Count]; - for (var i = 0; i < methods.Count; i++) + foreach (var eventListener in eventListeners) { - var method = methods[i]; + IRegisteredEventListener wrappedEventListener = new WrappedRegisteredEventListener(eventListener, listener); + + if (invoker != null) + { + wrappedEventListener = new InvokedRegisteredEventListener(wrappedEventListener, invoker); + } + + var register = _temporaryEventListeners.GetOrAdd( + wrappedEventListener.EventType, + _ => new TemporaryEventRegister()); - disposes[i] = (IDisposable) registerMethod! - .MakeGenericMethod(method.EventType) - .Invoke(this, new object[] { listener, method, invoker }); + register.Add(wrappedEventListener); } return new MultiDisposable(disposes); @@ -74,15 +70,11 @@ namespace Impostor.Server.Events try { - foreach (var (handler, eventListener) in GetHandlers(scope.ServiceProvider)) + foreach (var (handler, eventListener) in GetHandlers(scope.ServiceProvider) + .OrderByDescending(e => e.Listener.Priority)) { await eventListener.InvokeAsync(handler, @event, scope.ServiceProvider); } - - if (_temporaryEventListeners.TryGetValue(typeof(T), out var cb)) - { - await ((TemporaryEventRegister) cb).CallAsync(scope.ServiceProvider, @event); - } } finally { @@ -95,7 +87,7 @@ namespace Impostor.Server.Events /// /// Current service provider. /// The event listeners. - private static IEnumerable GetHandlers(IServiceProvider services) + private IEnumerable GetHandlers(IServiceProvider services) where TEvent : IEvent { foreach (var handler in services.GetServices()) @@ -112,14 +104,14 @@ namespace Impostor.Server.Events yield return new EventHandler(handler, eventHandler); } } - } - private IDisposable RegisterListenerImpl(object obj, RegisteredEventListener listener, Func, Task> invoker = null) - where TEvent : IEvent - { - return invoker == null - ? Register((provider, @event) => listener.InvokeAsync(obj, @event, provider)) - : Register((provider, @event) => new ValueTask(invoker(() => listener.InvokeAsync(obj, @event, provider).AsTask()))); + if (_temporaryEventListeners.TryGetValue(typeof(TEvent), out var cb)) + { + foreach (var eventListener in cb.GetEventListeners()) + { + yield return new EventHandler(null, eventListener); + } + } } } } \ No newline at end of file diff --git a/src/Impostor.Server/Events/Register/IRegisteredEventListener.cs b/src/Impostor.Server/Events/Register/IRegisteredEventListener.cs new file mode 100644 index 0000000..479a3f6 --- /dev/null +++ b/src/Impostor.Server/Events/Register/IRegisteredEventListener.cs @@ -0,0 +1,15 @@ +using System; +using System.Threading.Tasks; +using Impostor.Api.Events; + +namespace Impostor.Server.Events.Register +{ + internal interface IRegisteredEventListener + { + Type EventType { get; } + + EventPriority Priority { get; } + + ValueTask InvokeAsync(object eventHandler, object @event, IServiceProvider provider); + } +} \ No newline at end of file diff --git a/src/Impostor.Server/Events/Register/InvokedRegisteredEventListener.cs b/src/Impostor.Server/Events/Register/InvokedRegisteredEventListener.cs new file mode 100644 index 0000000..a21c3b1 --- /dev/null +++ b/src/Impostor.Server/Events/Register/InvokedRegisteredEventListener.cs @@ -0,0 +1,27 @@ +using System; +using System.Threading.Tasks; +using Impostor.Api.Events; + +namespace Impostor.Server.Events.Register +{ + internal class InvokedRegisteredEventListener : IRegisteredEventListener + { + private readonly IRegisteredEventListener _innerObject; + private readonly Func, Task> _invoker; + + public InvokedRegisteredEventListener(IRegisteredEventListener innerObject, Func, Task> invoker) + { + _innerObject = innerObject; + _invoker = invoker; + } + + public Type EventType => _innerObject.EventType; + + public EventPriority Priority => _innerObject.Priority; + + public ValueTask InvokeAsync(object eventHandler, object @event, IServiceProvider provider) + { + return new ValueTask(_invoker(() => _innerObject.InvokeAsync(eventHandler, @event, provider).AsTask())); + } + } +} \ No newline at end of file diff --git a/src/Impostor.Server/Events/RegisteredEventListener.cs b/src/Impostor.Server/Events/Register/RegisteredEventListener.cs similarity index 98% rename from src/Impostor.Server/Events/RegisteredEventListener.cs rename to src/Impostor.Server/Events/Register/RegisteredEventListener.cs index 80df9d8..0c49a07 100644 --- a/src/Impostor.Server/Events/RegisteredEventListener.cs +++ b/src/Impostor.Server/Events/Register/RegisteredEventListener.cs @@ -8,9 +8,9 @@ using System.Threading.Tasks; using Impostor.Api.Events; using Microsoft.Extensions.DependencyInjection; -namespace Impostor.Server.Events +namespace Impostor.Server.Events.Register { - internal class RegisteredEventListener + internal class RegisteredEventListener : IRegisteredEventListener { private static readonly ConcurrentDictionary Instances = new ConcurrentDictionary(); private readonly Func _invoker; @@ -21,7 +21,6 @@ namespace Impostor.Server.Events EventType = eventType; _eventListenerType = eventListenerType; Priority = attribute.Priority; - PriorityOrder = attribute.PriorityOrder; IgnoreCancelled = attribute.IgnoreCancelled; Method = method.GetFriendlyName(showParameters: false); _invoker = CreateInvoker(method, attribute.IgnoreCancelled); diff --git a/src/Impostor.Server/Events/Register/TemporaryEventRegister.cs b/src/Impostor.Server/Events/Register/TemporaryEventRegister.cs new file mode 100644 index 0000000..52cf629 --- /dev/null +++ b/src/Impostor.Server/Events/Register/TemporaryEventRegister.cs @@ -0,0 +1,60 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using Impostor.Api.Events; + +namespace Impostor.Server.Events.Register +{ + internal class TemporaryEventRegister + { + private readonly ConcurrentDictionary _callbacks; + private int _idLast; + + public TemporaryEventRegister() + { + _callbacks = new ConcurrentDictionary(); + } + + public IEnumerable GetEventListeners() + { + return _callbacks.Select(i => i.Value); + } + + public IDisposable Add(IRegisteredEventListener callback) + { + var id = Interlocked.Increment(ref _idLast); + + if (!_callbacks.TryAdd(id, callback)) + { + Debug.Fail("Failed to register the event listener"); + } + + return new UnregisterEvent(this, id); + } + + private void Remove(int id) + { + _callbacks.TryRemove(id, out _); + } + + private class UnregisterEvent : IDisposable + { + private readonly TemporaryEventRegister _register; + private readonly int _id; + + public UnregisterEvent(TemporaryEventRegister register, int id) + { + _register = register; + _id = id; + } + + public void Dispose() + { + _register.Remove(_id); + } + } + } +} \ No newline at end of file diff --git a/src/Impostor.Server/Events/Register/WrappedRegisteredEventListener.cs b/src/Impostor.Server/Events/Register/WrappedRegisteredEventListener.cs new file mode 100644 index 0000000..dd668c5 --- /dev/null +++ b/src/Impostor.Server/Events/Register/WrappedRegisteredEventListener.cs @@ -0,0 +1,27 @@ +using System; +using System.Threading.Tasks; +using Impostor.Api.Events; + +namespace Impostor.Server.Events.Register +{ + internal class WrappedRegisteredEventListener : IRegisteredEventListener + { + private readonly IRegisteredEventListener _innerObject; + private readonly object _object; + + public WrappedRegisteredEventListener(IRegisteredEventListener innerObject, object o) + { + _innerObject = innerObject; + _object = o; + } + + public Type EventType => _innerObject.EventType; + + public EventPriority Priority => _innerObject.Priority; + + public ValueTask InvokeAsync(object eventHandler, object @event, IServiceProvider provider) + { + return _innerObject.InvokeAsync(_object, @event, provider); + } + } +} \ No newline at end of file diff --git a/src/Impostor.Server/Events/TemporaryEventRegister.cs b/src/Impostor.Server/Events/TemporaryEventRegister.cs deleted file mode 100644 index 8bcb643..0000000 --- a/src/Impostor.Server/Events/TemporaryEventRegister.cs +++ /dev/null @@ -1,79 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using Impostor.Api.Events; - -namespace Impostor.Server.Events -{ - internal class TemporaryEventRegister - where T : IEvent - { - private readonly SemaphoreSlim semaphoreSlim = new SemaphoreSlim(1, 1); - private readonly List> _callbacks = new List>(); - - public async ValueTask CallAsync(IServiceProvider provider, T @event) - { - await semaphoreSlim.WaitAsync(); - - try - { - foreach (var callback in _callbacks) - { - await callback.Invoke(provider, @event); - } - } - finally - { - semaphoreSlim.Release(); - } - } - - public IDisposable Add(Func callback) - { - semaphoreSlim.Wait(); - - try - { - _callbacks.Add(callback); - } - finally - { - semaphoreSlim.Release(); - } - - return new UnregisterEvent(this, callback); - } - - private void Remove(Func callback) - { - semaphoreSlim.Wait(); - - try - { - _callbacks.Remove(callback); - } - finally - { - semaphoreSlim.Release(); - } - } - - private class UnregisterEvent : IDisposable - { - private readonly TemporaryEventRegister _register; - private readonly Func _callback; - - public UnregisterEvent(TemporaryEventRegister register, Func callback) - { - _register = register; - _callback = callback; - } - - public void Dispose() - { - _register.Remove(_callback); - } - } - } -} \ No newline at end of file diff --git a/src/Impostor.Server/Plugins/PluginLoader.cs b/src/Impostor.Server/Plugins/PluginLoader.cs index 8a1bcfc..2b3985d 100644 --- a/src/Impostor.Server/Plugins/PluginLoader.cs +++ b/src/Impostor.Server/Plugins/PluginLoader.cs @@ -30,8 +30,7 @@ namespace Impostor.Server.Plugins var matcher = new Matcher(StringComparison.OrdinalIgnoreCase); matcher.AddInclude("*.dll"); - matcher.AddExclude("Impostor.Server.Api.dll"); - matcher.AddExclude("Impostor.Shared.dll"); + matcher.AddExclude("Impostor.Api.dll"); RegisterAssemblies(pluginPaths, matcher, assemblyInfos, true); RegisterAssemblies(libraryPaths, matcher, assemblyInfos, false); @@ -53,7 +52,7 @@ namespace Impostor.Server.Plugins var plugins = assemblies .SelectMany(a => a.GetTypes()) - .Where(typeof(IPlugin).IsAssignableFrom) + .Where(t => typeof(IPlugin).IsAssignableFrom(t) && t.IsClass && !t.IsAbstract) .Select(Activator.CreateInstance) .Cast() .ToList(); diff --git a/src/Impostor.Tests/Events/EventManagerTests.cs b/src/Impostor.Tests/Events/EventManagerTests.cs new file mode 100644 index 0000000..08b33f8 --- /dev/null +++ b/src/Impostor.Tests/Events/EventManagerTests.cs @@ -0,0 +1,168 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using Impostor.Api.Events; +using Impostor.Api.Events.Managers; +using Impostor.Server.Events; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Impostor.Tests.Events +{ + public class EventManagerTests + { + public static readonly IEnumerable TestModes = new [] + { + new object[] { TestMode.Service }, + new object[] { TestMode.Temporary } + }; + + [Theory] + [MemberData(nameof(TestModes))] + public async ValueTask CallEvent(TestMode mode) + { + var listener = new EventListener(); + var eventManager = CreatEventManager(mode, listener); + + await eventManager.CallAsync(new SetValueEvent(1)); + + Assert.Equal(1, listener.Value); + } + + [Theory] + [MemberData(nameof(TestModes))] + public async Task CallPriority(TestMode mode) + { + var listener = new PriorityEventListener(); + var eventManager = CreatEventManager(mode, listener); + + await eventManager.CallAsync(new SetValueEvent(1)); + + Assert.Equal(new [] + { + EventPriority.Monitor, + EventPriority.Highest, + EventPriority.High, + EventPriority.Normal, + EventPriority.Low, + EventPriority.Lowest + }, listener.Priorities); + } + + [Theory] + [MemberData(nameof(TestModes))] + public async ValueTask CancelEvent(TestMode mode) + { + var listener = new EventListener(); + var eventManager = CreatEventManager( + mode, + new CancelAtHighEventListener(), + listener + ); + + await eventManager.CallAsync(new SetValueEvent(1)); + + Assert.Equal(0, listener.Value); + } + + [Theory] + [MemberData(nameof(TestModes))] + public async Task CancelPriority(TestMode mode) + { + var listener = new PriorityEventListener(); + var eventManager = CreatEventManager( + mode, + new CancelAtHighEventListener(), + listener + ); + + await eventManager.CallAsync(new SetValueEvent(1)); + + Assert.Equal(new [] + { + EventPriority.Monitor, + EventPriority.Highest + }, listener.Priorities); + } + + private static IEventManager CreatEventManager(TestMode mode, params IEventListener[] listeners) + { + var services = new ServiceCollection(); + services.AddSingleton(); + + if (mode == TestMode.Service) + { + foreach (var listener in listeners) + { + services.AddSingleton(listener); + } + } + + var eventManager = services.BuildServiceProvider().GetRequiredService(); + + if (mode == TestMode.Temporary) + { + foreach (var listener in listeners) + { + eventManager.RegisterListener(listener); + } + } + + return eventManager; + } + + public enum TestMode + { + Service, + Temporary + } + + public class SetValueEvent : IEventCancelable + { + public SetValueEvent(int value) + { + Value = value; + } + + public int Value { get; } + + public bool IsCancelled { get; set; } + } + + private class CancelAtHighEventListener : IEventListener + { + [EventListener(Priority = EventPriority.High)] + public void OnSetCalled(SetValueEvent e) => e.IsCancelled = true; + } + + private class EventListener : IEventListener + { + public int Value { get; private set; } + + [EventListener] + public void OnSetCalled(SetValueEvent e) => Value = e.Value; + } + + private class PriorityEventListener : IEventListener + { + public List Priorities { get; } = new List(); + + [EventListener(EventPriority.Lowest)] + public void OnLowest(SetValueEvent e) => Priorities.Add(EventPriority.Lowest); + + [EventListener(EventPriority.Low)] + public void OnLow(SetValueEvent e) => Priorities.Add(EventPriority.Low); + + [EventListener] + public void OnNormal(SetValueEvent e) => Priorities.Add(EventPriority.Normal); + + [EventListener(EventPriority.High)] + public void OnHigh(SetValueEvent e) => Priorities.Add(EventPriority.High); + + [EventListener(EventPriority.Highest)] + public void OnHighest(SetValueEvent e) => Priorities.Add(EventPriority.Highest); + + [EventListener(EventPriority.Monitor)] + public void OnMonitor(SetValueEvent e) => Priorities.Add(EventPriority.Monitor); + } + } +} \ No newline at end of file -- 2.39.5