Merge pull request #922 from Tom94/prevent-dependency-pollution

Prevent dependency pollution up the scene graph

osu.Framework/Allocation/DependencyContainer.cs

@@ -13,21 +13,21 @@ namespace osu.Framework.Allocation
     /// <summary>
     /// Hierarchically caches dependencies and can inject those automatically into types registered for dependency injection.
     /// </summary>
-    public class DependencyContainer
+    public class DependencyContainer : IReadOnlyDependencyContainer
     {
         private delegate object ObjectActivator(DependencyContainer dc, object instance);
 
         private readonly ConcurrentDictionary<Type, ObjectActivator> activators = new ConcurrentDictionary<Type, ObjectActivator>();
         private readonly ConcurrentDictionary<Type, object> cache = new ConcurrentDictionary<Type, object>();
         private readonly HashSet<Type> cacheable = new HashSet<Type>();
 
-        private readonly DependencyContainer parentContainer;
+        private readonly IReadOnlyDependencyContainer parentContainer;
 
         /// <summary>
         /// Create a new DependencyContainer instance.
         /// </summary>
         /// <param name="parent">An optional parent container which we should use as a fallback for cache lookups.</param>
-        public DependencyContainer(DependencyContainer parent = null)
+        public DependencyContainer(IReadOnlyDependencyContainer parent = null)
         {
             parentContainer = parent;
             Cache(this);
@@ -70,7 +70,7 @@ private void register(Type type, bool lazy)
                 var parameters = initializer.GetParameters().Select(p => p.ParameterType)
                                             .Select(t => new Func<object>(() =>
                                             {
-                                                var val = get(t);
+                                                var val = Get(t);
                                                 if (val == null && !permitNull)
                                                 {
                                                     throw new InvalidOperationException(
@@ -132,13 +132,20 @@ public T Cache<T>(T instance = null, bool overwrite = false, bool lazy = false)
             return instance;
         }
 
-        private object get(Type type)
+        /// <summary>
+        /// Retrieves a cached dependency of <paramref name="type"/> if it exists. If not, then the parent
+        /// <see cref="IReadOnlyDependencyContainer"/> is recursively queried. If no parent contains
+        /// <paramref name="type"/>, then null is returned.
+        /// </summary>
+        /// <param name="type">The dependency type to query for.</param>
+        /// <returns>The requested dependency, or null if not found.</returns>
+        public object Get(Type type)
         {
             object ret;
             if (cache.TryGetValue(type, out ret))
                 return ret;
 
-            return parentContainer?.get(type);
+            return parentContainer?.Get(type);
 
             //we don't ever want to instantiate for now, as this breaks expectations when using permitNull.
             //need to revisit this when/if it is required.
@@ -155,20 +162,20 @@ private object get(Type type)
         /// </summary>
         public T Get<T>(bool autoRegister = true, bool lazy = false) where T : class
         {
-            T instance = (T)get(typeof(T));
+            T instance = (T)Get(typeof(T));
             if (autoRegister && instance == null)
             {
                 Register<T>(lazy);
-                instance = (T)get(typeof(T));
+                instance = (T)Get(typeof(T));
             }
             return instance;
         }
 
         /// <summary>
-        /// Injects dependencies for the given instance.
+        /// Injects dependencies into the given instance.
         /// </summary>
-        /// <typeparam name="T">The type of the instance to inject dependencies for.</typeparam>
-        /// <param name="instance">The instance to inject dependencies for.</param>
+        /// <typeparam name="T">The type of the instance to inject dependencies into.</typeparam>
+        /// <param name="instance">The instance to inject dependencies into.</param>
         /// <param name="autoRegister">True if the instance should be automatically registered as injectable if it isn't already.</param>
         /// <param name="lazy">True if the dependencies should be initialized lazily.</param>
         public void Inject<T>(T instance, bool autoRegister = true, bool lazy = false) where T : class

osu.Framework/Allocation/IReadOnlyDependencyContainer.cs

@@ -0,0 +1,30 @@
+// Copyright (c) 2007-2017 ppy Pty Ltd <contact@ppy.sh>.
+// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu-framework/master/LICENCE
+
+using System;
+
+namespace osu.Framework.Allocation
+{
+    /// <summary>
+    /// Read-only interface into a dependency container capable of injective and retrieving dependencies based
+    /// on types.
+    /// </summary>
+    public interface IReadOnlyDependencyContainer
+    {
+        /// <summary>
+        /// Retrieves a cached dependency of <paramref name="type"/> if it exists and null otherwise.
+        /// </summary>
+        /// <param name="type">The dependency type to query for.</param>
+        /// <returns>The requested dependency, or null if not found.</returns>
+        object Get(Type type);
+
+        /// <summary>
+        /// Injects dependencies into the given instance.
+        /// </summary>
+        /// <typeparam name="T">The type of the instance to inject dependencies into.</typeparam>
+        /// <param name="instance">The instance to inject dependencies into.</param>
+        /// <param name="autoRegister">True if the instance should be automatically registered as injectable if it isn't already.</param>
+        /// <param name="lazy">True if the dependencies should be initialized lazily.</param>
+        void Inject<T>(T instance, bool autoRegister = true, bool lazy = false) where T : class;
+    }
+}

osu.Framework/Game.cs

@@ -95,6 +95,11 @@ public virtual void SetHost(GameHost host)
             host.Deactivated += () => IsActive = false;
         }
 
+        private DependencyContainer dependencies;
+
+        protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) =>
+            dependencies = new DependencyContainer(base.CreateLocalDependencies(parent));
+
         [BackgroundDependencyLoader]
         private void load(FrameworkConfigManager config)
         {
@@ -104,7 +109,7 @@ private void load(FrameworkConfigManager config)
 
             Textures = new TextureStore(new RawTextureLoaderStore(new NamespacedResourceStore<byte[]>(Resources, @"Textures")));
             Textures.AddStore(new RawTextureLoaderStore(new OnlineStore()));
-            Dependencies.Cache(Textures);
+            dependencies.Cache(Textures);
 
             var tracks = new ResourceStore<byte[]>(Resources);
             tracks.AddStore(new NamespacedResourceStore<byte[]>(Resources, @"Tracks"));
@@ -114,7 +119,7 @@ private void load(FrameworkConfigManager config)
             samples.AddStore(new NamespacedResourceStore<byte[]>(Resources, @"Samples"));
             samples.AddStore(new OnlineStore());
 
-            Audio = Dependencies.Cache(new AudioManager(
+            Audio = dependencies.Cache(new AudioManager(
                 tracks,
                 samples)
             {
@@ -130,13 +135,13 @@ private void load(FrameworkConfigManager config)
             config.BindWith(FrameworkSetting.VolumeMusic, Audio.VolumeTrack);
 
             Shaders = new ShaderManager(new NamespacedResourceStore<byte[]>(Resources, @"Shaders"));
-            Dependencies.Cache(Shaders);
+            dependencies.Cache(Shaders);
 
             Fonts = new FontStore(new GlyphStore(Resources, @"Fonts/OpenSans"))
             {
                 ScaleAdjust = 100
             };
-            Dependencies.Cache(Fonts);
+            dependencies.Cache(Fonts);
         }
 
         protected override void LoadComplete()

osu.Framework/Graphics/Drawable.cs

@@ -148,22 +148,22 @@ internal Task LoadAsync(Game game, Drawable target, Action onLoaded = null)
         /// Create a local dependency container which will be used by ourselves and all our nested children.
         /// If not overridden, the load-time parent's dependency tree will be used.
         /// </summary>
-        /// <param name="parent">The parent <see cref="DependencyContainer"/> which should be passed through if we want fallback lookups to work.</param>
+        /// <param name="parent">The parent <see cref="IReadOnlyDependencyContainer"/> which should be passed through if we want fallback lookups to work.</param>
         /// <returns>A new dependency container to be stored for this Drawable.</returns>
-        protected virtual DependencyContainer CreateLocalDependencies(DependencyContainer parent) => parent;
+        protected virtual IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => parent;
 
         /// <summary>
         /// Contains all dependencies that can be injected into this Drawable using <see cref="BackgroundDependencyLoader"/>.
         /// Add or override dependencies by calling <see cref="DependencyContainer.Cache{T}(T, bool, bool)"/>.
         /// </summary>
-        protected DependencyContainer Dependencies { get; private set; }
+        protected IReadOnlyDependencyContainer Dependencies { get; private set; }
 
         /// <summary>
         /// Loads this drawable, including the gathering of dependencies and initialisation of required resources.
         /// </summary>
         /// <param name="clock">The clock we should use by default.</param>
-        /// <param name="dependencies">The dependency tree we will inherit by default. May be extended via <see cref="CreateLocalDependencies(DependencyContainer)"/></param>
-        internal void Load(IFrameBasedClock clock, DependencyContainer dependencies)
+        /// <param name="dependencies">The dependency tree we will inherit by default. May be extended via <see cref="CreateLocalDependencies(IReadOnlyDependencyContainer)"/></param>
+        internal void Load(IFrameBasedClock clock, IReadOnlyDependencyContainer dependencies)
         {
             // Blocks when loading from another thread already.
             lock (loadLock)

osu.Framework/osu.Framework.csproj

@@ -82,6 +82,7 @@
     </Reference>
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="Allocation\IReadOnlyDependencyContainer.cs" />
     <Compile Include="Audio\AudioComponent.cs" />
     <Compile Include="Audio\IBassAudio.cs" />
     <Compile Include="Audio\IHasPitchAdjust.cs" />