Code Review feedback

Issue 466

7 files changed, 64 insertions, 24 deletions

diff --git a/src/System.Web.Http.OData/HttpConfigurationExtensions.cs b/src/System.Web.Http.OData/HttpConfigurationExtensions.cs
index f0a0b39a..9516fcaa 100644
--- a/src/System.Web.Http.OData/HttpConfigurationExtensions.cs
+++ b/src/System.Web.Http.OData/HttpConfigurationExtensions.cs
@@ -134,13 +134,13 @@ namespace System.Web.Http
         }
 
         /// <summary>
-        /// Gets the <see cref="IODataActionResolver"/> on the configuration.
+        /// Gets the <see cref="IODataActionResolver"/> from the configuration.
         /// </summary>
         /// <remarks>
-        /// If not <see cref="IODataActionResolver"/> is configured this returns the <see cref="DefaultODataActionResolver"/> 
+        /// If an <see cref="IODataActionResolver"/> is not found, this method registers and returns a <see cref="DefaultODataActionResolver"/> 
         /// </remarks>
-        /// <param name="configuration">Configuration o check.</param>
-        /// <returns>Returns an <see cref="IODataActionResolver"/> for this configuration.</returns>
+        /// <param name="configuration">Configuration to check.</param>
+        /// <returns>Returns an <see cref="IODataActionResolver"/></returns>
         public static IODataActionResolver GetODataActionResolver(this HttpConfiguration configuration)
         {
             if (configuration == null)
@@ -149,7 +149,7 @@ namespace System.Web.Http
             }
 
             // returns one if user sets one, null otherwise
-            object result = configuration.Properties.GetOrAdd(ODataActionResolverKey, new DefaultODataActionResolver());
+            object result = configuration.Properties.GetOrAdd(ODataActionResolverKey, (key) => new DefaultODataActionResolver());
             return result as IODataActionResolver;
         }
 
diff --git a/src/System.Web.Http.OData/OData/DefaultODataActionResolver.cs b/src/System.Web.Http.OData/OData/DefaultODataActionResolver.cs
index abfd6331..f3127946 100644
--- a/src/System.Web.Http.OData/OData/DefaultODataActionResolver.cs
+++ b/src/System.Web.Http.OData/OData/DefaultODataActionResolver.cs
@@ -16,8 +16,14 @@ namespace System.Web.Http.OData
     {
         public IEdmFunctionImport Resolve(ODataDeserializerContext context)
         {
-            Contract.Assert(context.Request != null);
-            Contract.Assert(context.Model != null);
+            if (context == null)
+            {
+                throw Error.ArgumentNull("context");
+            }
+            else if (context.Request == null || context.Request.RequestUri == null)
+            {
+                throw Error.InvalidOperation(SRResources.DefaultODataActionResolverRequirementsNotSatisfied);
+            }
 
             string actionName = null;
             string containerName = null;
@@ -26,7 +32,9 @@ namespace System.Web.Http.OData
             string lastSegment = context.Request.RequestUri.Segments.Last();
             string[] nameParts = lastSegment.Split('.');
 
-            IEnumerable<IEdmFunctionImport> matchingActionsQuery = context.Model.EntityContainers().Single().FunctionImports();
+            IEdmEntityContainer[] entityContainers = context.Model.EntityContainers().ToArray();
+            Contract.Assert(entityContainers.Length == 1);
+            IEnumerable<IEdmFunctionImport> matchingActionsQuery = entityContainers[0].FunctionImports();
 
             if (nameParts.Length == 1)
             {
@@ -51,11 +59,11 @@ namespace System.Web.Http.OData
 
             if (possibleMatches.Length == 0)
             {
-                throw Error.InvalidOperation(SRResources.ActionNotFound, actionName);
+                throw Error.InvalidOperation(SRResources.ActionNotFound, actionName, context.Request.RequestUri.AbsoluteUri);
             }
-            if (possibleMatches.Length > 1)
+            else if (possibleMatches.Length > 1)
             {
-                throw Error.InvalidOperation(SRResources.ActionResolutionFailed, actionName);
+                throw Error.InvalidOperation(SRResources.ActionResolutionFailed, actionName, context.Request.RequestUri.AbsoluteUri);
             }
             return possibleMatches[0];
         }
diff --git a/src/System.Web.Http.OData/OData/Formatter/ODataFormatterParameterBinding.cs b/src/System.Web.Http.OData/OData/Formatter/ODataFormatterParameterBinding.cs
index 6645d2c4..58d4615b 100644
--- a/src/System.Web.Http.OData/OData/Formatter/ODataFormatterParameterBinding.cs
+++ b/src/System.Web.Http.OData/OData/Formatter/ODataFormatterParameterBinding.cs
@@ -21,6 +21,10 @@ namespace System.Web.Http.OData.Formatter
         public ODataFormatterParameterBinding(HttpParameterDescriptor descriptor, ODataMediaTypeFormatter formatter)
             : base(descriptor)
         {
+            if (formatter == null)
+            {
+                throw Error.ArgumentNull("formatter");
+            }
             _formatter = formatter;
         }
 
diff --git a/src/System.Web.Http.OData/OData/Formatter/ODataParameterBindingAttribute.cs b/src/System.Web.Http.OData/OData/Formatter/ODataParameterBindingAttribute.cs
index 7f4b170a..831a8c86 100644
--- a/src/System.Web.Http.OData/OData/Formatter/ODataParameterBindingAttribute.cs
+++ b/src/System.Web.Http.OData/OData/Formatter/ODataParameterBindingAttribute.cs
@@ -1,18 +1,24 @@
 // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
 
 using System.Web.Http.Controllers;
+using System.Web.Http.OData.Properties;
 
 namespace System.Web.Http.OData.Formatter
 {
     /// <summary>
-    /// This attribute insures that The ODataFormatterParameterBinding is used.
+    /// This attribute ensures that The ODataFormatterParameterBinding is used.
     /// </summary>
     [AttributeUsage(AttributeTargets.Class | AttributeTargets.Parameter, Inherited = true, AllowMultiple = false)]
     public sealed class ODataParameterBindingAttribute : ParameterBindingAttribute
     {
         public override HttpParameterBinding GetBinding(HttpParameterDescriptor parameter)
         {
-            return new ODataFormatterParameterBinding(parameter, parameter.Configuration.GetODataFormatter());
+            ODataMediaTypeFormatter formatter = parameter.Configuration.GetODataFormatter();
+            if (formatter == null)
+            {
+                throw Error.InvalidOperation(SRResources.NoODataMediaTypeFormatterFound);
+            }
+            return new ODataFormatterParameterBinding(parameter, formatter);
         }
     }
 }
diff --git a/src/System.Web.Http.OData/Properties/SRResources.Designer.cs b/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
index 0b26e87a..0fbec7aa 100644
--- a/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
+++ b/src/System.Web.Http.OData/Properties/SRResources.Designer.cs
@@ -1,7 +1,7 @@
 //------------------------------------------------------------------------------
 // <auto-generated>
 //     This code was generated by a tool.
-//     Runtime Version:4.0.30319.17929
+//     Runtime Version:4.0.30319.18003
 //
 //     Changes to this file may cause incorrect behavior and will be lost if
 //     the code is regenerated.
@@ -97,7 +97,7 @@ namespace System.Web.Http.OData.Properties {
         }
         
         /// <summary>
-        ///   Looks up a localized string similar to Action &apos;{0}&apos; not found..
+        ///   Looks up a localized string similar to Action &apos;{0}&apos; was not found for RequestUri &apos;{1}&apos;..
         /// </summary>
         internal static string ActionNotFound {
             get {
@@ -106,7 +106,7 @@ namespace System.Web.Http.OData.Properties {
         }
         
         /// <summary>
-        ///   Looks up a localized string similar to Ambiguous request. Multiple action overloads called &apos;{0}&apos; found..
+        ///   Looks up a localized string similar to Ambiguous request. Found multiple action overloads called &apos;{0}&apos; for RequestUri &apos;{1}&apos;..
         /// </summary>
         internal static string ActionResolutionFailed {
             get {
@@ -250,6 +250,15 @@ namespace System.Web.Http.OData.Properties {
         }
         
         /// <summary>
+        ///   Looks up a localized string similar to DefaultODataActionResolver requires an ODataDeserializerContext with a non null Request and RequestUri..
+        /// </summary>
+        internal static string DefaultODataActionResolverRequirementsNotSatisfied {
+            get {
+                return ResourceManager.GetString("DefaultODataActionResolverRequirementsNotSatisfied", resourceCulture);
+            }
+        }
+        
+        /// <summary>
         ///   Looks up a localized string similar to The feed self link could not be generated for the route named &apos;{0}&apos;..
         /// </summary>
         internal static string DefaultRouteMissingOrIncorrect {
@@ -673,6 +682,15 @@ namespace System.Web.Http.OData.Properties {
         }
         
         /// <summary>
+        ///   Looks up a localized string similar to The ODataParameterBindingAttribute requires that an ODataMediaTypeFormatter be registered with the HttpConfiguration..
+        /// </summary>
+        internal static string NoODataMediaTypeFormatterFound {
+            get {
+                return ResourceManager.GetString("NoODataMediaTypeFormatterFound", resourceCulture);
+            }
+        }
+        
+        /// <summary>
         ///   Looks up a localized string similar to Collections cannot contain null elements..
         /// </summary>
         internal static string NullElementInCollection {
diff --git a/src/System.Web.Http.OData/Properties/SRResources.resx b/src/System.Web.Http.OData/Properties/SRResources.resx
index 99dd6934..d6a69098 100644
--- a/src/System.Web.Http.OData/Properties/SRResources.resx
+++ b/src/System.Web.Http.OData/Properties/SRResources.resx
@@ -409,9 +409,15 @@
     <value>The OData formatter does not support writing client requests. This formatter instance must have an associated request.</value>
   </data>
   <data name="ActionNotFound" xml:space="preserve">
-    <value>Action '{0}' not found.</value>
+    <value>Action '{0}' was not found for RequestUri '{1}'.</value>
   </data>
   <data name="ActionResolutionFailed" xml:space="preserve">
-    <value>Ambiguous request. Multiple action overloads called '{0}' found.</value>
+    <value>Ambiguous request. Found multiple action overloads called '{0}' for RequestUri '{1}'.</value>
+  </data>
+  <data name="DefaultODataActionResolverRequirementsNotSatisfied" xml:space="preserve">
+    <value>DefaultODataActionResolver requires an ODataDeserializerContext with a non null Request and RequestUri.</value>
+  </data>
+  <data name="NoODataMediaTypeFormatterFound" xml:space="preserve">
+    <value>The ODataParameterBindingAttribute requires that an ODataMediaTypeFormatter be registered with the HttpConfiguration.</value>
   </data>
 </root>
\ No newline at end of file
diff --git a/test/System.Web.Http.OData.Test/OData/DefaultODataActionResolverTest.cs b/test/System.Web.Http.OData.Test/OData/DefaultODataActionResolverTest.cs
index 06b8b5ed..36ada9af 100644
--- a/test/System.Web.Http.OData.Test/OData/DefaultODataActionResolverTest.cs
+++ b/test/System.Web.Http.OData.Test/OData/DefaultODataActionResolverTest.cs
@@ -51,25 +51,23 @@ namespace System.Web.Http.OData
         [Fact]
         public void Throws_InvalidOperation_when_action_not_found()
         {
-            string invalidUrl = "http://server/service/MissingOperation";
             IODataActionResolver resolver = new DefaultODataActionResolver();
-            ODataDeserializerContext context = new ODataDeserializerContext { Request = GetPostRequest(invalidUrl), Model = GetModel() };
+            ODataDeserializerContext context = new ODataDeserializerContext { Request = GetPostRequest("http://server/service/MissingOperation"), Model = GetModel() };
             Assert.Throws<InvalidOperationException>(() =>
             {
                 IEdmFunctionImport action = resolver.Resolve(context);
-            }, "Action 'MissingOperation' not found.");
+            },  "Action 'MissingOperation' was not found for RequestUri 'http://server/service/MissingOperation'.");
         }
 
         [Fact]
         public void Throws_InvalidOperation_when_multiple_overloads_found()
         {
-            string invalidUrl = "http://server/service/Vehicles/Container.Car(8)/Park";
             IODataActionResolver resolver = new DefaultODataActionResolver();
-            ODataDeserializerContext context = new ODataDeserializerContext { Request = GetPostRequest(invalidUrl), Model = GetModel() };
+            ODataDeserializerContext context = new ODataDeserializerContext { Request = GetPostRequest("http://server/service/Vehicles/Container.Car(8)/Park"), Model = GetModel() };
             InvalidOperationException ioe = Assert.Throws<InvalidOperationException>(() =>
             {
                 IEdmFunctionImport action = resolver.Resolve(context);
-            }, "Ambiguous request. Multiple action overloads called 'Park' found.");
+            }, "Ambiguous request. Found multiple action overloads called 'Park' for RequestUri 'http://server/service/Vehicles/Container.Car(8)/Park'.");
         }
 
         [Fact]