-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Public HTTP Response class #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| // | ||
| // Copyright 2012 Microsoft Corporation | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.WindowsAzure.ServiceLayer.Http; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.WindowsAzure.ServiceLayer.UnitTests.HttpTests | ||
| { | ||
| /// <summary> | ||
| /// HTTP pipeline tests. | ||
| /// </summary> | ||
| public class PipelineTests | ||
| { | ||
| /// <summary> | ||
| /// Tests passing invalid arguments in the request's constructor. | ||
| /// </summary> | ||
| [Fact] | ||
| public void InvalidArgsInRequestConstructor() | ||
| { | ||
| Uri validUri = new Uri("http://microsoft.com"); | ||
| Assert.Throws<ArgumentNullException>(() => new HttpRequest(null, validUri)); | ||
| Assert.Throws<ArgumentNullException>(() => new HttpRequest("PUT", null)); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests passing invalid arguments in the response's constructor. | ||
| /// </summary> | ||
| [Fact] | ||
| public void InvalidArgsInResponseConstructor() | ||
| { | ||
| HttpRequest validRequest = new HttpRequest("PUT", new Uri("http://microsoft.com")); | ||
| Assert.Throws<ArgumentNullException>(() => new HttpResponse(null, 200)); | ||
| Assert.Throws<ArgumentOutOfRangeException>(() => new HttpResponse(validRequest, -5)); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| // | ||
| // Copyright 2012 Microsoft Corporation | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
| // | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
|
|
||
| using NetHttpResponseMessage = System.Net.Http.HttpResponseMessage; | ||
|
|
||
| namespace Microsoft.WindowsAzure.ServiceLayer.Http | ||
| { | ||
| /// <summary> | ||
| /// Represents an HTTP response message. | ||
| /// server. | ||
| /// </summary> | ||
| public sealed class HttpResponse | ||
| { | ||
| /// <summary> | ||
| /// Gets the request that lead to this response. | ||
| /// </summary> | ||
| public HttpRequest Request { get; private set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the status code of the HTTP response. | ||
| /// </summary> | ||
| public int StatusCode { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Tells whether the HTTP response was successful. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gets whether the HTTP request was successful. However, I am not sure this logic should really be build into an accessor. Could this logic be left for the app to handle it?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been done for consistency with System.Net.Http.HttpResponseMessage class, which exposes both StatusCode and IsSuccessStatusCode. I was using this property with the .Net response class, and adding it here let me minimize changes. |
||
| /// </summary> | ||
| public bool IsSuccessStatusCode { get { return StatusCode >= 200 && StatusCode < 299; } } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the reason phrase which typically is sent by servers | ||
| /// together with the status code. | ||
| /// </summary> | ||
| public string ReasonPhrase { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the content of the response. | ||
| /// </summary> | ||
| public HttpContent Content { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the collection of HTTP response headers. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gets a dictionary of HTTP response headers.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am using sentences from MSDN article on System.Net.Http.HttpResponseMessage class. |
||
| /// </summary> | ||
| public IDictionary<string, string> Headers { get; private set; } | ||
|
|
||
| /// <summary> | ||
| /// Initializes the response object. | ||
| /// </summary> | ||
| /// <param name="originalRequest">Request that initiated the response.</param> | ||
| /// <param name="statusCode">Status code of the HTTP response.</param> | ||
| public HttpResponse(HttpRequest originalRequest, int statusCode) | ||
| { | ||
| if (originalRequest == null) | ||
| { | ||
| throw new ArgumentNullException("originalRequest"); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about do a range validation for the statusCode here and throw OutOfRange exception if the statusCode is out of range? Have u thought about leveraging on existing HttpStatusCode enum? http://msdn.microsoft.com/en-us/library/system.net.httpstatuscode.aspx
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| if (!Enum.IsDefined(typeof(System.Net.HttpStatusCode), statusCode)) | ||
| { | ||
| throw new ArgumentOutOfRangeException("statusCode"); | ||
| } | ||
|
|
||
| Request = originalRequest; | ||
| StatusCode = statusCode; | ||
| Headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes the response object. | ||
| /// </summary> | ||
| /// <param name="originalRequest">Request that initiated the response.</param> | ||
| /// <param name="response">.Net HTTP response.</param> | ||
| internal HttpResponse(HttpRequest originalRequest, NetHttpResponseMessage response) | ||
| { | ||
| Debug.Assert(originalRequest != null); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Debug code shouldn't be in product code.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assertions are valuable tools for detecting errors. |
||
| Debug.Assert(response != null); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Debug code shouldn't be in product code. |
||
|
|
||
| Request = originalRequest; | ||
| StatusCode = (int)response.StatusCode; | ||
| ReasonPhrase = response.ReasonPhrase; | ||
| Content = HttpContent.CreateFromResponse(response); | ||
| Headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember FxCop used to complain about this. FxCop recommends to create the dictionary by default, each time we need to renew the dictionary, we just call Headers.Clear(). What does FxCop say this time?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It currently complains about many things, but not about this one. I have decided to instantiate the object myself instead of letting users create their own dictionaries because I wanted to control which string comparer is used for keys. With my design keys are case-insensitive, and users cannot change that. |
||
|
|
||
| foreach (KeyValuePair<string, IEnumerable<string>> item in response.Headers) | ||
| { | ||
| string valueString = string.Join(string.Empty, item.Value); | ||
| Headers.Add(item.Key, valueString); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,10 +16,9 @@ | |
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Net; | ||
| using System.Net.Http; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.WindowsAzure.ServiceLayer.Http; | ||
|
|
||
| namespace Microsoft.WindowsAzure.ServiceLayer | ||
| { | ||
|
|
@@ -31,7 +30,7 @@ internal class WindowsAzureServiceException: WindowsAzureException | |
| /// <summary> | ||
| /// Gets the HTTP status code. | ||
| /// </summary> | ||
| internal HttpStatusCode StatusCode { get; private set; } | ||
| internal int StatusCode { get; private set; } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the reason here that we drop the usage of HttpStatusCode?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wasn't used anyway. I am planning to improve error handling soon, however. |
||
| /// <summary> | ||
| /// Gets the reason string fore th exception. | ||
|
|
@@ -42,7 +41,7 @@ internal class WindowsAzureServiceException: WindowsAzureException | |
| /// Constructor. | ||
| /// </summary> | ||
| /// <param name="response">Source HTTP response.</param> | ||
| internal WindowsAzureServiceException(HttpResponseMessage response) | ||
| internal WindowsAzureServiceException(HttpResponse response) | ||
| { | ||
| StatusCode = response.StatusCode; | ||
| ReasonPhrase = response.ReasonPhrase; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The identiation of Licensed seems to be off...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a copy/paste; all files have the same problem. I may eventually fix that, but it's definitely not my highest priority.