From 6e42acba687083d772790c3bc84e3e8f9f9d31da Mon Sep 17 00:00:00 2001 From: Christopher F Date: Wed, 20 Jul 2016 17:20:37 -0400 Subject: [PATCH 1/6] Add Autoload to Module Attribute [UNTESTED] Adds an optional parameter to the Module attribute, "autoload", which defaults to true. Specifies whether or not the assembly crawler should load this module. --- src/Discord.Net.Commands/Attributes/ModuleAttribute.cs | 7 +++++-- src/Discord.Net.Commands/CommandService.cs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs b/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs index 59e6a6aca..57f525389 100644 --- a/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs +++ b/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs @@ -6,13 +6,16 @@ namespace Discord.Commands public class ModuleAttribute : Attribute { public string Prefix { get; } - public ModuleAttribute() + public bool Autoload { get; } + public ModuleAttribute(bool autoload = true) { Prefix = null; + Autoload = autoload; } - public ModuleAttribute(string prefix) + public ModuleAttribute(string prefix, bool autoload = true) { Prefix = prefix; + Autoload = autoload; } } } diff --git a/src/Discord.Net.Commands/CommandService.cs b/src/Discord.Net.Commands/CommandService.cs index f762ae366..9fd8aecc9 100644 --- a/src/Discord.Net.Commands/CommandService.cs +++ b/src/Discord.Net.Commands/CommandService.cs @@ -174,7 +174,7 @@ namespace Discord.Commands { var typeInfo = type.GetTypeInfo(); var moduleAttr = typeInfo.GetCustomAttribute(); - if (moduleAttr != null) + if (moduleAttr != null && moduleAttr.Autoload) { var moduleInstance = ReflectionUtils.CreateObject(typeInfo); modules.Add(LoadInternal(moduleInstance, moduleAttr, typeInfo)); From bbe51012cf91fd81a50c894971ad5244bfa308ef Mon Sep 17 00:00:00 2001 From: Christopher F Date: Wed, 20 Jul 2016 17:50:52 -0400 Subject: [PATCH 2/6] Add Dependency Map, Update Assembly Crawler [Untested] Assembly Crawler will now accept constructors matching: new(), new(CommandService), new(IDependencyMap). Add IDependencyMap Add DependencyMap --- src/Discord.Net.Commands/CommandService.cs | 4 +-- .../Dependencies/DependencyMap.cs | 27 ++++++++++++++++++++ .../Dependencies/IDependencyMap.cs | 13 ++++++++++ src/Discord.Net.Commands/Module.cs | 2 +- src/Discord.Net.Commands/ReflectionUtils.cs | 29 ++++++++++++++++------ 5 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 src/Discord.Net.Commands/Dependencies/DependencyMap.cs create mode 100644 src/Discord.Net.Commands/Dependencies/IDependencyMap.cs diff --git a/src/Discord.Net.Commands/CommandService.cs b/src/Discord.Net.Commands/CommandService.cs index 9fd8aecc9..d0dfeaeb9 100644 --- a/src/Discord.Net.Commands/CommandService.cs +++ b/src/Discord.Net.Commands/CommandService.cs @@ -164,7 +164,7 @@ namespace Discord.Commands return loadedModule; } - public async Task> LoadAssembly(Assembly assembly) + public async Task> LoadAssembly(Assembly assembly, IDependencyMap dependencyMap = null) { var modules = ImmutableArray.CreateBuilder(); await _moduleLock.WaitAsync().ConfigureAwait(false); @@ -176,7 +176,7 @@ namespace Discord.Commands var moduleAttr = typeInfo.GetCustomAttribute(); if (moduleAttr != null && moduleAttr.Autoload) { - var moduleInstance = ReflectionUtils.CreateObject(typeInfo); + var moduleInstance = ReflectionUtils.CreateObject(typeInfo, this, dependencyMap); modules.Add(LoadInternal(moduleInstance, moduleAttr, typeInfo)); } } diff --git a/src/Discord.Net.Commands/Dependencies/DependencyMap.cs b/src/Discord.Net.Commands/Dependencies/DependencyMap.cs new file mode 100644 index 000000000..20d9a60c5 --- /dev/null +++ b/src/Discord.Net.Commands/Dependencies/DependencyMap.cs @@ -0,0 +1,27 @@ +using System; +using System.Collections.Generic; +using System.Reflection; + +namespace Discord.Commands +{ + public class DependencyMap : IDependencyMap + { + private Dictionary map; + + public T Get() where T : class + { + var t = typeof(T); + if (!map.ContainsKey(t)) + throw new KeyNotFoundException($"The dependency map does not contain \"{t.FullName}\""); + return map[t] as T; + } + + public void Add(T obj) + { + var t = typeof(T); + if (map.ContainsKey(t)) + throw new InvalidOperationException($"The dependency map already contains \"{t.FullName}\""); + map.Add(t, obj); + } + } +} diff --git a/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs b/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs new file mode 100644 index 000000000..fb2710795 --- /dev/null +++ b/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs @@ -0,0 +1,13 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +namespace Discord.Commands +{ + public interface IDependencyMap + { + T Get() where T : class; + void Add(T obj); + } +} diff --git a/src/Discord.Net.Commands/Module.cs b/src/Discord.Net.Commands/Module.cs index ea6e29c28..b884832bc 100644 --- a/src/Discord.Net.Commands/Module.cs +++ b/src/Discord.Net.Commands/Module.cs @@ -43,7 +43,7 @@ namespace Discord.Commands nextGroupPrefix = groupPrefix + groupAttrib.Prefix ?? type.Name; else nextGroupPrefix = groupPrefix; - SearchClass(ReflectionUtils.CreateObject(type), commands, type, nextGroupPrefix); + SearchClass(ReflectionUtils.CreateObject(type, Service), commands, type, nextGroupPrefix); } } } diff --git a/src/Discord.Net.Commands/ReflectionUtils.cs b/src/Discord.Net.Commands/ReflectionUtils.cs index 28672a06f..98f1989be 100644 --- a/src/Discord.Net.Commands/ReflectionUtils.cs +++ b/src/Discord.Net.Commands/ReflectionUtils.cs @@ -6,18 +6,33 @@ namespace Discord.Commands { internal class ReflectionUtils { - internal static object CreateObject(TypeInfo typeInfo) + internal static object CreateObject(TypeInfo typeInfo, CommandService commands, IDependencyMap map = null) { - var constructor = typeInfo.DeclaredConstructors.Where(x => x.GetParameters().Length == 0).FirstOrDefault(); - if (constructor == null) - throw new InvalidOperationException($"Failed to find a valid constructor for \"{typeInfo.FullName}\""); + if (typeInfo.DeclaredConstructors.Count() > 1) + throw new InvalidOperationException($"Found too many constructors for \"{typeInfo.FullName}\""); + var constructor = typeInfo.DeclaredConstructors.FirstOrDefault(); try { - return constructor.Invoke(null); + if (constructor.GetParameters().Length == 0) + return constructor.Invoke(null); + else if (constructor.GetParameters().Length > 1) + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); + var parameter = constructor.GetParameters().FirstOrDefault(); + if (parameter == null) + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); + if (parameter.GetType() == typeof(CommandService)) + return constructor.Invoke(new object[1] { commands }); + else if (parameter is IDependencyMap) + { + if (map == null) throw new InvalidOperationException($"The constructor for \"{typeInfo.FullName}\" requires a Dependency Map."); + return constructor.Invoke(new object[1] { map }); + } + else + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); } - catch (Exception ex) + catch { - throw new InvalidOperationException($"Failed to create \"{typeInfo.FullName}\"", ex); + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); } } } From e266fa8b32d928d1bdcb4f81f7a0ff489b460835 Mon Sep 17 00:00:00 2001 From: Christopher F Date: Wed, 20 Jul 2016 18:04:18 -0400 Subject: [PATCH 3/6] Cleaned up bugs in DependencyMap and ReflectionUtils --- src/Discord.Net.Commands/Dependencies/DependencyMap.cs | 5 +++++ src/Discord.Net.Commands/ReflectionUtils.cs | 14 +++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Discord.Net.Commands/Dependencies/DependencyMap.cs b/src/Discord.Net.Commands/Dependencies/DependencyMap.cs index 20d9a60c5..4495a906b 100644 --- a/src/Discord.Net.Commands/Dependencies/DependencyMap.cs +++ b/src/Discord.Net.Commands/Dependencies/DependencyMap.cs @@ -8,6 +8,11 @@ namespace Discord.Commands { private Dictionary map; + public DependencyMap() + { + map = new Dictionary(); + } + public T Get() where T : class { var t = typeof(T); diff --git a/src/Discord.Net.Commands/ReflectionUtils.cs b/src/Discord.Net.Commands/ReflectionUtils.cs index 98f1989be..d1eeccb4c 100644 --- a/src/Discord.Net.Commands/ReflectionUtils.cs +++ b/src/Discord.Net.Commands/ReflectionUtils.cs @@ -16,23 +16,23 @@ namespace Discord.Commands if (constructor.GetParameters().Length == 0) return constructor.Invoke(null); else if (constructor.GetParameters().Length > 1) - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Found too many parameters)"); var parameter = constructor.GetParameters().FirstOrDefault(); if (parameter == null) - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); - if (parameter.GetType() == typeof(CommandService)) + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (No valid parameters)"); + if (parameter.ParameterType == typeof(CommandService)) return constructor.Invoke(new object[1] { commands }); - else if (parameter is IDependencyMap) + else if (parameter.ParameterType == typeof(IDependencyMap)) { if (map == null) throw new InvalidOperationException($"The constructor for \"{typeInfo.FullName}\" requires a Dependency Map."); return constructor.Invoke(new object[1] { map }); } else - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Invalid Parameter Type: \"{parameter.ParameterType.FullName}\")"); } - catch + catch (Exception e) { - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\""); + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Error invoking constructor)"); } } } From f7455c389b4e70e15ee74c406c976f053f07700a Mon Sep 17 00:00:00 2001 From: Finite Reality Date: Thu, 21 Jul 2016 00:23:49 +0100 Subject: [PATCH 4/6] Improve DI system --- .../Attributes/ModuleAttribute.cs | 10 ++--- src/Discord.Net.Commands/CommandService.cs | 2 +- .../Dependencies/DependencyMap.cs | 10 +++-- .../Dependencies/IDependencyMap.cs | 1 + src/Discord.Net.Commands/ReflectionUtils.cs | 45 +++++++++++++--------- 5 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs b/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs index 57f525389..ec04041e8 100644 --- a/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs +++ b/src/Discord.Net.Commands/Attributes/ModuleAttribute.cs @@ -6,16 +6,16 @@ namespace Discord.Commands public class ModuleAttribute : Attribute { public string Prefix { get; } - public bool Autoload { get; } - public ModuleAttribute(bool autoload = true) + public bool AutoLoad { get; set; } + public ModuleAttribute() { Prefix = null; - Autoload = autoload; + AutoLoad = true; } - public ModuleAttribute(string prefix, bool autoload = true) + public ModuleAttribute(string prefix) { Prefix = prefix; - Autoload = autoload; + AutoLoad = true; } } } diff --git a/src/Discord.Net.Commands/CommandService.cs b/src/Discord.Net.Commands/CommandService.cs index d0dfeaeb9..46c5aaa39 100644 --- a/src/Discord.Net.Commands/CommandService.cs +++ b/src/Discord.Net.Commands/CommandService.cs @@ -174,7 +174,7 @@ namespace Discord.Commands { var typeInfo = type.GetTypeInfo(); var moduleAttr = typeInfo.GetCustomAttribute(); - if (moduleAttr != null && moduleAttr.Autoload) + if (moduleAttr != null && moduleAttr.AutoLoad) { var moduleInstance = ReflectionUtils.CreateObject(typeInfo, this, dependencyMap); modules.Add(LoadInternal(moduleInstance, moduleAttr, typeInfo)); diff --git a/src/Discord.Net.Commands/Dependencies/DependencyMap.cs b/src/Discord.Net.Commands/Dependencies/DependencyMap.cs index 4495a906b..db4d20984 100644 --- a/src/Discord.Net.Commands/Dependencies/DependencyMap.cs +++ b/src/Discord.Net.Commands/Dependencies/DependencyMap.cs @@ -13,12 +13,16 @@ namespace Discord.Commands map = new Dictionary(); } - public T Get() where T : class + public object Get(Type t) { - var t = typeof(T); if (!map.ContainsKey(t)) throw new KeyNotFoundException($"The dependency map does not contain \"{t.FullName}\""); - return map[t] as T; + return map[t]; + } + + public T Get() where T : class + { + return Get(typeof(T)) as T; } public void Add(T obj) diff --git a/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs b/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs index fb2710795..859cff09b 100644 --- a/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs +++ b/src/Discord.Net.Commands/Dependencies/IDependencyMap.cs @@ -7,6 +7,7 @@ namespace Discord.Commands { public interface IDependencyMap { + object Get(Type t); T Get() where T : class; void Add(T obj); } diff --git a/src/Discord.Net.Commands/ReflectionUtils.cs b/src/Discord.Net.Commands/ReflectionUtils.cs index d1eeccb4c..4603563d8 100644 --- a/src/Discord.Net.Commands/ReflectionUtils.cs +++ b/src/Discord.Net.Commands/ReflectionUtils.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -6,33 +7,39 @@ namespace Discord.Commands { internal class ReflectionUtils { - internal static object CreateObject(TypeInfo typeInfo, CommandService commands, IDependencyMap map = null) + internal static object CreateObject(TypeInfo typeInfo, CommandService service, IDependencyMap map = null) { if (typeInfo.DeclaredConstructors.Count() > 1) throw new InvalidOperationException($"Found too many constructors for \"{typeInfo.FullName}\""); + var constructor = typeInfo.DeclaredConstructors.FirstOrDefault(); + + if (constructor == null) + throw new InvalidOperationException($"Found no constructor for \"{typeInfo.FullName}\""); + + object[] parameters; + try + { + // TODO: probably change this ternary into something sensible + parameters = constructor.GetParameters() + .Select(x => x.ParameterType == typeof(CommandService) ? service : map.Get(x.ParameterType)).ToArray(); + } + catch (KeyNotFoundException ex) // tried to inject an invalid dependency + { + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (could not provide parameter)", ex); + } + catch (NullReferenceException ex) // tried to find a dependency + { + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (type requires dependency injection)", ex); + } + try { - if (constructor.GetParameters().Length == 0) - return constructor.Invoke(null); - else if (constructor.GetParameters().Length > 1) - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Found too many parameters)"); - var parameter = constructor.GetParameters().FirstOrDefault(); - if (parameter == null) - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (No valid parameters)"); - if (parameter.ParameterType == typeof(CommandService)) - return constructor.Invoke(new object[1] { commands }); - else if (parameter.ParameterType == typeof(IDependencyMap)) - { - if (map == null) throw new InvalidOperationException($"The constructor for \"{typeInfo.FullName}\" requires a Dependency Map."); - return constructor.Invoke(new object[1] { map }); - } - else - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Invalid Parameter Type: \"{parameter.ParameterType.FullName}\")"); + return constructor.Invoke(parameters); } - catch (Exception e) + catch (Exception ex) { - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Error invoking constructor)"); + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Error invoking constructor)", ex); } } } From 39d8e3c159606757cd6e58793ff6e06fb78167de Mon Sep 17 00:00:00 2001 From: Finite Reality Date: Thu, 21 Jul 2016 01:44:14 +0100 Subject: [PATCH 5/6] Allow users to get IDependencyMap if they follow a strict format --- src/Discord.Net.Commands/ReflectionUtils.cs | 45 ++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/Discord.Net.Commands/ReflectionUtils.cs b/src/Discord.Net.Commands/ReflectionUtils.cs index 4603563d8..dab5dfda8 100644 --- a/src/Discord.Net.Commands/ReflectionUtils.cs +++ b/src/Discord.Net.Commands/ReflectionUtils.cs @@ -17,25 +17,50 @@ namespace Discord.Commands if (constructor == null) throw new InvalidOperationException($"Found no constructor for \"{typeInfo.FullName}\""); - object[] parameters; - try + object[] arguments = null; + + ParameterInfo[] parameters = constructor.GetParameters(); + + // TODO: can this logic be made better/cleaner? + if (parameters.Length == 1) { - // TODO: probably change this ternary into something sensible - parameters = constructor.GetParameters() - .Select(x => x.ParameterType == typeof(CommandService) ? service : map.Get(x.ParameterType)).ToArray(); + if (parameters[0].ParameterType == typeof(IDependencyMap)) + { + if (map != null) + arguments = new object[] { map }; + else + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (an IDependencyMap is required)"); + } } - catch (KeyNotFoundException ex) // tried to inject an invalid dependency + else if (parameters.Length == 2) { - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (could not provide parameter)", ex); + if (parameters[0].ParameterType == typeof(CommandService) && parameters[1].ParameterType == typeof(IDependencyMap)) + if (map != null) + arguments = new object[] { service, map }; + else + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (an IDependencyMap is required)"); } - catch (NullReferenceException ex) // tried to find a dependency + + if (arguments == null) { - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (type requires dependency injection)", ex); + try + { + // TODO: probably change this ternary into something sensible? + arguments = parameters.Select(x => x.ParameterType == typeof(CommandService) ? service : map.Get(x.ParameterType)).ToArray(); + } + catch (KeyNotFoundException ex) // tried to inject an invalid dependency + { + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (could not provide parameter)", ex); + } + catch (NullReferenceException ex) // tried to find a dependency + { + throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (an IDependencyMap is required)", ex); + } } try { - return constructor.Invoke(parameters); + return constructor.Invoke(arguments); } catch (Exception ex) { From b546ba919b13614ffb0a7c9bc925a7ea6488a0cb Mon Sep 17 00:00:00 2001 From: Finite Reality Date: Thu, 21 Jul 2016 02:49:27 +0100 Subject: [PATCH 6/6] Simplify exception --- src/Discord.Net.Commands/ReflectionUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Discord.Net.Commands/ReflectionUtils.cs b/src/Discord.Net.Commands/ReflectionUtils.cs index dab5dfda8..62c77ff64 100644 --- a/src/Discord.Net.Commands/ReflectionUtils.cs +++ b/src/Discord.Net.Commands/ReflectionUtils.cs @@ -64,7 +64,7 @@ namespace Discord.Commands } catch (Exception ex) { - throw new InvalidOperationException($"Could not find a valid constructor for \"{typeInfo.FullName}\" (Error invoking constructor)", ex); + throw new InvalidOperationException($"Could not create \"{typeInfo.FullName}\"", ex); } } }