-
Notifications
You must be signed in to change notification settings - Fork 191
[Perf] Add ContentLength to IHeaderDictionary #757
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,14 @@ | ||
[ | ||
{ | ||
"OldTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewMemberId": "System.Nullable<System.Int64> get_ContentLength()", | ||
"Kind": "Addition" | ||
}, | ||
{ | ||
"OldTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewMemberId": "System.Void set_ContentLength(System.Nullable<System.Int64> value)", | ||
"Kind": "Addition" | ||
} | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
[ | ||
{ | ||
"OldTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewMemberId": "System.Nullable<System.Int64> get_ContentLength()", | ||
"Kind": "Addition" | ||
}, | ||
{ | ||
"OldTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewTypeId": "public interface Microsoft.AspNetCore.Http.IHeaderDictionary : System.Collections.Generic.IDictionary<System.String, Microsoft.Extensions.Primitives.StringValues>", | ||
"NewMemberId": "System.Void set_ContentLength(System.Nullable<System.Int64> value)", | ||
"Kind": "Addition" | ||
} | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
using System.Collections; | ||
using System.Collections.Generic; | ||
using Microsoft.Extensions.Primitives; | ||
using Microsoft.Net.Http.Headers; | ||
|
||
namespace Microsoft.AspNetCore.Http | ||
{ | ||
|
@@ -97,6 +98,34 @@ StringValues IDictionary<string, StringValues>.this[string key] | |
set { this[key] = value; } | ||
} | ||
|
||
public long? ContentLength | ||
{ | ||
get | ||
{ | ||
long value; | ||
var rawValue = this[HeaderNames.ContentLength]; | ||
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. So the value in the string is being parsed every time from the string? Don't we want to store this as a long somewhere that's being kept in sync with the value stored in the string dictionary? 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. Yes, this is the same as before. Kestrel and WebListener will provide more optimized versions of IHeaderDictionary. I didn't optimize this one because it's primarily used in tests. 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. Ah yes, I should have read your first comment more carefully before looking at the code. |
||
if (rawValue.Count == 1 && | ||
!string.IsNullOrWhiteSpace(rawValue[0]) && | ||
HeaderUtilities.TryParseInt64(new StringSegment(rawValue[0]).Trim(), out value)) | ||
{ | ||
return value; | ||
} | ||
|
||
return null; | ||
} | ||
set | ||
{ | ||
if (value.HasValue) | ||
{ | ||
this[HeaderNames.ContentLength] = HeaderUtilities.FormatInt64(value.Value); | ||
} | ||
else | ||
{ | ||
this.Remove(HeaderNames.ContentLength); | ||
} | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Gets the number of elements contained in the <see cref="HeaderDictionary" />;. | ||
/// </summary> | ||
|
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.
Looks like duplication here and in
DictionaryStringValuesWrapper
. Should we keep this in a helper method like before and just call it from the getter and setter of theContentLength
properties.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.
I thought about it but the implementation is actually different, their internal stores work a little differently.