Addressing code review feedback for HttpError support in OData and EnableQuerySupport

5 files changed, 89 insertions, 70 deletions

diff --git a/src/System.Web.Http.OData/HttpConfigurationExtensions.cs b/src/System.Web.Http.OData/HttpConfigurationExtensions.cs
index fec2bf6d..b092667b 100644
--- a/src/System.Web.Http.OData/HttpConfigurationExtensions.cs
+++ b/src/System.Web.Http.OData/HttpConfigurationExtensions.cs
@@ -183,16 +183,6 @@ namespace System.Web.Http
         }
 
         /// <summary>
-        /// Enables query support for actions with an <see cref="IQueryable" /> or <see cref="IQueryable{T} "/> return type.
-        /// </summary>
-        /// <param name="configuration">The server configuration.</param>
-        /// <param name="resultLimit">The maximum number of results to return.</param>
-        public static void EnableQuerySupport(this HttpConfiguration configuration, int resultLimit)
-        {
-            configuration.EnableQuerySupport(new QueryableAttribute() { ResultLimit = resultLimit });
-        }
-
-        /// <summary>
         /// Enables query support for actions with an <see cref="IQueryable" /> or <see cref="IQueryable{T}" /> return type.
         /// </summary>
         /// <param name="configuration">The server configuration.</param>
diff --git a/src/System.Web.Http.OData/HttpErrorExtensions.cs b/src/System.Web.Http.OData/HttpErrorExtensions.cs
new file mode 100644
index 00000000..e92043c1
--- /dev/null
+++ b/src/System.Web.Http.OData/HttpErrorExtensions.cs
@@ -0,0 +1,79 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
+
+using System.Diagnostics.Contracts;
+using Microsoft.Data.OData;
+
+namespace System.Web.Http
+{
+    /// <summary>
+    /// Provides extension methods for the <see cref="HttpError"/> class.
+    /// </summary>
+    public static class HttpErrorExtensions
+    {
+        private const string MessageKey = "Message";
+        private const string MessageLanguageKey = "MessageLanguage";
+        private const string ErrorCodeKey = "ErrorCode";
+        private const string ExceptionMessageKey = "ExceptionMessage";
+        private const string ExceptionTypeKey = "ExceptionType";
+        private const string StackTraceKey = "StackTrace";
+        private const string InnerExceptionKey = "InnerException";
+
+        /// <summary>
+        /// Converts the <paramref name="httpError"/> to an <see cref="ODataError"/>.
+        /// </summary>
+        /// <param name="httpError">The <see cref="HttpError"/> instance to convert.</param>
+        /// <returns>The converted <see cref="ODataError"/></returns>
+        public static ODataError ToODataError(this HttpError httpError)
+        {
+            if (httpError == null)
+            {
+                throw Error.ArgumentNull("httpError");
+            }
+
+            return new ODataError()
+            {
+                Message = httpError.GetPropertyValue<string>(MessageKey),
+                MessageLanguage = httpError.GetPropertyValue<string>(MessageLanguageKey),
+                ErrorCode = httpError.GetPropertyValue<string>(ErrorCodeKey),
+                InnerError = httpError.ToODataInnerError()
+            };
+        }
+
+        private static ODataInnerError ToODataInnerError(this HttpError httpError)
+        {
+            string innerErrorMessage = httpError.GetPropertyValue<string>(ExceptionMessageKey);
+            if (innerErrorMessage == null)
+            {
+                return null;
+            }
+            else
+            {
+                ODataInnerError innerError = new ODataInnerError();
+                innerError.Message = innerErrorMessage;
+                innerError.TypeName = httpError.GetPropertyValue<string>(ExceptionTypeKey);
+                innerError.StackTrace = httpError.GetPropertyValue<string>(StackTraceKey);
+                HttpError innerExceptionError = httpError.GetPropertyValue<HttpError>(InnerExceptionKey);
+                if (innerExceptionError != null)
+                {
+                    innerError.InnerError = innerExceptionError.ToODataInnerError();
+                }
+                return innerError;
+            }
+        }
+
+        private static TValue GetPropertyValue<TValue>(this HttpError httpError, string key)
+        {
+            Contract.Assert(httpError != null);
+
+            object value;
+            if (httpError.TryGetValue(key, out value))
+            {
+                if (value is TValue)
+                {
+                    return (TValue)value;
+                }
+            }
+            return default(TValue);
+        }
+    }
+}
diff --git a/src/System.Web.Http.OData/OData/Formatter/Serialization/ODataErrorSerializer.cs b/src/System.Web.Http.OData/OData/Formatter/Serialization/ODataErrorSerializer.cs
index c2edb874..a50345ad 100644
--- a/src/System.Web.Http.OData/OData/Formatter/Serialization/ODataErrorSerializer.cs
+++ b/src/System.Web.Http.OData/OData/Formatter/Serialization/ODataErrorSerializer.cs
@@ -1,6 +1,5 @@
 // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information.
 
-using System.Diagnostics.Contracts;
 using System.Web.Http.OData.Properties;
 using Microsoft.Data.OData;
 
@@ -8,14 +7,6 @@ namespace System.Web.Http.OData.Formatter.Serialization
 {
     internal class ODataErrorSerializer : ODataSerializer
     {
-        private const string MessageKey = "Message";
-        private const string MessageLanguageKey = "MessageLanguage";
-        private const string ErrorCodeKey = "ErrorCode";
-        private const string ExceptionMessageKey = "ExceptionMessage";
-        private const string ExceptionTypeKey = "ExceptionType";
-        private const string StackTraceKey = "StackTrace";
-        private const string InnerExceptionKey = "InnerException";
-
         public ODataErrorSerializer()
             : base(ODataPayloadKind.Error)
         {
@@ -39,64 +30,16 @@ namespace System.Web.Http.OData.Formatter.Serialization
                 HttpError httpError = graph as HttpError;
                 if (httpError == null)
                 {
-                    throw Error.InvalidOperation(SRResources.ErrorTypeMustBeODataErrorOrHttpError, graph.GetType().Name);
+                    throw Error.InvalidOperation(SRResources.ErrorTypeMustBeODataErrorOrHttpError, graph.GetType().FullName);
                 }
                 else
                 {
-                    oDataError = ConvertToODataError(httpError);
+                    oDataError = httpError.ToODataError();
                 }
             }
 
             bool includeDebugInformation = oDataError.InnerError != null;
             messageWriter.WriteError(oDataError, includeDebugInformation);
         }
-
-        internal static ODataError ConvertToODataError(HttpError httpError)
-        {
-            return new ODataError()
-            {
-                Message = GetPropertyValue<string>(httpError, MessageKey),
-                MessageLanguage = GetPropertyValue<string>(httpError, MessageLanguageKey),
-                ErrorCode = GetPropertyValue<string>(httpError, ErrorCodeKey),
-                InnerError = ConvertToODataInnerError(httpError)
-            };
-        }
-
-        private static ODataInnerError ConvertToODataInnerError(HttpError httpError)
-        {
-            string innerErrorMessage = GetPropertyValue<string>(httpError, ExceptionMessageKey);
-            if (innerErrorMessage == null)
-            {
-                return null;
-            }
-            else
-            {
-                ODataInnerError innerError = new ODataInnerError();
-                innerError.Message = innerErrorMessage;
-                innerError.TypeName = GetPropertyValue<string>(httpError, ExceptionTypeKey);
-                innerError.StackTrace = GetPropertyValue<string>(httpError, StackTraceKey);
-                HttpError innerExceptionError = GetPropertyValue<HttpError>(httpError, InnerExceptionKey);
-                if (innerExceptionError != null)
-                {
-                    innerError.InnerError = ConvertToODataInnerError(innerExceptionError);
-                }
-                return innerError;
-            }
-        }
-
-        private static TValue GetPropertyValue<TValue>(HttpError httpError, string key)
-        {
-            Contract.Assert(httpError != null);
-
-            object value;
-            if (httpError.TryGetValue(key, out value))
-            {
-                if (value is TValue)
-                {
-                    return (TValue)value;
-                }
-            }
-            return default(TValue);
-        }
     }
 }
diff --git a/src/System.Web.Http.OData/System.Web.Http.OData.csproj b/src/System.Web.Http.OData/System.Web.Http.OData.csproj
index 2079ba41..666ab162 100644
--- a/src/System.Web.Http.OData/System.Web.Http.OData.csproj
+++ b/src/System.Web.Http.OData/System.Web.Http.OData.csproj
@@ -104,6 +104,7 @@
       <Link>Common\Error.cs</Link>
     </Compile>
     <Compile Include="GlobalSuppressions.cs" />
+    <Compile Include="HttpErrorExtensions.cs" />
     <Compile Include="HttpRequestMessageExtensions.cs" />
     <Compile Include="OData\Builder\ActionLinkBuilder.cs" />
     <Compile Include="OData\Builder\BindableProcedureFinder.cs" />
diff --git a/src/System.Web.Http/HttpError.cs b/src/System.Web.Http/HttpError.cs
index c6b5326a..04e11be5 100644
--- a/src/System.Web.Http/HttpError.cs
+++ b/src/System.Web.Http/HttpError.cs
@@ -229,7 +229,13 @@ namespace System.Web.Http
             get { return GetPropertyValue<HttpError>(InnerExceptionKey); }
         }
 
-        private TValue GetPropertyValue<TValue>(string key)
+        /// <summary>
+        /// Gets a particular property value from this error instance.
+        /// </summary>
+        /// <typeparam name="TValue">The type of the property.</typeparam>
+        /// <param name="key">The name of the error property.</param>
+        /// <returns>The value of the error property.</returns>
+        public TValue GetPropertyValue<TValue>(string key)
         {
             TValue value;
             if (this.TryGetValue(key, out value))