#339 Reduce IFeatureCollection surface area.

Author: Tratcher

src/Microsoft.AspNet.Http.Features/FeatureCollection.cs

@@ -4,15 +4,16 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Framework.Internal;
 
 namespace Microsoft.AspNet.Http.Features
 {
     public class FeatureCollection : IFeatureCollection
     {
-        private readonly IFeatureCollection _defaults;
+        private static KeyComparer FeatureKeyComparer = new FeatureCollection.KeyComparer();
         private readonly IDictionary<Type, object> _featureByFeatureType = new Dictionary<Type, object>();
-        private readonly object _containerSync = new object();
+        private readonly IFeatureCollection _defaults;
         private volatile int _containerRevision;
 
         public FeatureCollection()
@@ -24,143 +25,78 @@ public FeatureCollection(IFeatureCollection defaults)
             _defaults = defaults;
         }
 
-        public object GetInterface()
+        public virtual int Revision
         {
-            return GetInterface(null);
+            get { return _containerRevision + (_defaults?.Revision ?? 0); }
         }
 
-        public object GetInterface([NotNull] Type type)
-        {
-            object feature;
-            if (_featureByFeatureType.TryGetValue(type, out feature))
-            {
-                return feature;
-            }
+        public bool IsReadOnly { get { return false; } }
 
-            if (_defaults != null && _defaults.TryGetValue(type, out feature))
-            {
-                return feature;
-            }
-            return null;
-        }
-
-        void SetInterface([NotNull] Type type, object feature)
+        public object this[[NotNull] Type key]
         {
-            if (feature == null)
-            {
-                Remove(type);
-                return;
-            }
-
-            lock (_containerSync)
-            {
-                _featureByFeatureType[type] = feature;
-                _containerRevision++;
-            }
+            get { return Get(key); }
+            set { Set(key, value); }
         }
 
-        public virtual int Revision
+        public object Get([NotNull] Type key)
         {
-            get { return _containerRevision; }
+            object result;
+            return _featureByFeatureType.TryGetValue(key, out result) ? result : _defaults?.Get(key);
         }
 
-        public void Dispose()
+        public void Set([NotNull] Type key, object value)
         {
-        }
+            if (value == null)
+            {
+                if (_featureByFeatureType.Remove(key))
+                {
+                    _containerRevision++;
+                }
+                return;
+            }
 
-        public IEnumerator<KeyValuePair<Type, object>> GetEnumerator()
-        {
-            throw new NotImplementedException();
+            _featureByFeatureType[key] = value;
+            _containerRevision++;
         }
 
         IEnumerator IEnumerable.GetEnumerator()
         {
             return GetEnumerator();
         }
 
-        public void Add(KeyValuePair<Type, object> item)
-        {
-            SetInterface(item.Key, item.Value);
-        }
-
-        public void Clear()
-        {
-            throw new NotImplementedException();
-        }
-
-        public bool Contains(KeyValuePair<Type, object> item)
-        {
-            object value;
-            return TryGetValue(item.Key, out value) && Equals(item.Value, value);
-        }
-
-        public void CopyTo(KeyValuePair<Type, object>[] array, int arrayIndex)
-        {
-            throw new NotImplementedException();
-        }
-
-        public bool Remove(KeyValuePair<Type, object> item)
-        {
-            return Contains(item) && Remove(item.Key);
-        }
-
-        public int Count
-        {
-            get { throw new NotImplementedException(); }
-        }
-
-        public bool IsReadOnly
-        {
-            get { return false; }
-        }
-
-        public bool ContainsKey([NotNull] Type key)
-        {
-            return GetInterface(key) != null;
-        }
-
-        public void Add([NotNull] Type key, [NotNull] object value)
+        public IEnumerator<KeyValuePair<Type, object>> GetEnumerator()
         {
-            if (ContainsKey(key))
+            foreach (var pair in _featureByFeatureType)
             {
-                throw new ArgumentException();
+                yield return pair;
             }
-            SetInterface(key, value);
-        }
 
-        public bool Remove([NotNull] Type key)
-        {
-            lock (_containerSync)
+            if (_defaults != null)
             {
-                if (_featureByFeatureType.Remove(key))
+                // Don't return features masked by the wrapper.
+                foreach (var pair in _defaults.Except(_featureByFeatureType, FeatureKeyComparer))
                 {
-                    _containerRevision++;
-                    return true;
+                    yield return pair;
                 }
-                return false;
             }
         }
 
-        public bool TryGetValue([NotNull] Type key, out object value)
-        {
-            value = GetInterface(key);
-            return value != null;
-        }
-
-        public object this[Type key]
+        public virtual void Dispose()
         {
-            get { return GetInterface(key); }
-            set { SetInterface(key, value); }
+            _defaults?.Dispose();
         }
 
-        public ICollection<Type> Keys
+        private class KeyComparer : IEqualityComparer<KeyValuePair<Type, object>>
         {
-            get { throw new NotImplementedException(); }
-        }
+            public bool Equals(KeyValuePair<Type, object> x, KeyValuePair<Type, object> y)
+            {
+                return x.Key.Equals(y.Key);
+            }
 
-        public ICollection<object> Values
-        {
-            get { throw new NotImplementedException(); }
+            public int GetHashCode(KeyValuePair<Type, object> obj)
+            {
+                throw new NotImplementedException();
+            }
         }
     }
 }

src/Microsoft.AspNet.Http.Features/FeatureReference.cs

@@ -18,23 +18,19 @@ private FeatureReference(T feature, int revision)
 
         public T Fetch(IFeatureCollection features)
         {
-            if (_revision == features.Revision) return _feature;
-            object value;
-            if (features.TryGetValue(typeof(T), out value))
+            if (_revision == features.Revision)
             {
-                _feature = (T)value;
-            }
-            else
-            {
-                _feature = default(T);
+                return _feature;
             }
+            _feature = (T)features[typeof(T)];
             _revision = features.Revision;
             return _feature;
         }
 
         public T Update(IFeatureCollection features, T feature)
         {
-            features[typeof(T)] = _feature = feature;
+            features[typeof(T)] = feature;
+            _feature = feature;
             _revision = features.Revision;
             return feature;
         }

src/Microsoft.AspNet.Http.Features/IFeatureCollection.cs

@@ -3,11 +3,42 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.Framework.Internal;
 
 namespace Microsoft.AspNet.Http.Features
 {
-    public interface IFeatureCollection : IDictionary<Type, object>, IDisposable
+    public interface IFeatureCollection : IEnumerable<KeyValuePair<Type, object>>, IDisposable
     {
+        /// <summary>
+        /// Indicates if the collection can be modified.
+        /// </summary>
+        bool IsReadOnly { get; }
+
+        /// <summary>
+        /// Incremented for each modification and can be used verify cached results.
+        /// </summary>
         int Revision { get; }
+
+        /// <summary>
+        /// Gets or sets a given feature. Setting a null value removes the feature.
+        /// </summary>
+        /// <param name="key"></param>
+        /// <returns>The requested feature, or null if it is not present.</returns>
+        object this[[NotNull] Type key] { get; set; }
+
+        /// <summary>
+        /// Gets the requested feature, or null if it is not present.
+        /// </summary>
+        /// <param name="key"></param>
+        /// <returns></returns>
+        object Get([NotNull] Type key);
+
+        /// <summary>
+        /// Sets the given feature, overriding the existing feature if any. If a null value is
+        /// provided then any existing feature is removed.
+        /// </summary>
+        /// <param name="key"></param>
+        /// <param name="value"></param>
+        void Set([NotNull] Type key, object value);
     }
 }

src/Microsoft.AspNet.Http.Features/project.json

@@ -13,6 +13,7 @@
         "dnxcore50": {
             "dependencies": {
                 "System.Collections": "4.0.10-beta-*",
+                "System.Linq": "4.0.0-beta-*",
                 "System.Net.Primitives": "4.0.10-beta-*",
                 "System.Net.WebSockets": "4.0.0-beta-*",
                 "System.Runtime.Extensions": "4.0.10-beta-*",

src/Microsoft.AspNet.Http/DefaultHttpContext.cs

@@ -172,8 +172,7 @@ public override void Dispose()
 
         public override object GetFeature(Type type)
         {
-            object value;
-            return _features.TryGetValue(type, out value) ? value : null;
+            return _features[type];
         }
 
         public override void SetFeature(Type type, object instance)