-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
upgraded to web api 2.2 and replace msgpack with msgpack.cli #4
base: master
Are you sure you want to change the base?
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 |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<packages> | ||
<package id="NUnit.Runners" version="2.6.2" /> | ||
<package id="NUnit.Runners" version="2.6.4" /> | ||
</packages> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,14 @@ | ||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Net; | ||
using System.Net.Http; | ||
using System.Net.Http.Formatting; | ||
using System.Net.Http.Headers; | ||
using MsgPack; | ||
using System.Threading.Tasks; | ||
using MsgPack.Serialization; | ||
|
||
namespace WebApiContrib.Formatting.MsgPack | ||
{ | ||
|
@@ -14,7 +18,7 @@ namespace WebApiContrib.Formatting.MsgPack | |
/// <remarks>Converted from Filip W.'s MessagePackMediaTypeFormatter.</remarks> | ||
/// <see href="http://msgpack.org/"/> | ||
/// <see href="http://www.strathweb.com/2012/09/boost-up-your-asp-net-web-api-with-messagepack/"/> | ||
public class MessagePackMediaTypeFormatter : BufferedMediaTypeFormatter | ||
public class MessagePackMediaTypeFormatter : MediaTypeFormatter | ||
{ | ||
private const string mediaType = "application/x-msgpack"; | ||
|
||
|
@@ -27,7 +31,7 @@ private static bool IsAllowedType(Type t) | |
return true; | ||
|
||
return false; | ||
}; | ||
} | ||
|
||
public MessagePackMediaTypeFormatter() | ||
{ | ||
|
@@ -37,29 +41,29 @@ public MessagePackMediaTypeFormatter() | |
public override bool CanReadType(Type type) | ||
{ | ||
if (type == null) | ||
throw new ArgumentNullException("type is null"); | ||
throw new ArgumentNullException("type"); | ||
|
||
return IsAllowedType(type); | ||
} | ||
|
||
public override bool CanWriteType(Type type) | ||
{ | ||
if (type == null) | ||
throw new ArgumentNullException("type is null"); | ||
throw new ArgumentNullException("type"); | ||
|
||
return IsAllowedType(type); | ||
} | ||
|
||
public override object ReadFromStream(Type type, System.IO.Stream readStream, System.Net.Http.HttpContent content, IFormatterLogger formatterLogger) | ||
public override Task<object> ReadFromStreamAsync(Type type, Stream stream, HttpContent content, IFormatterLogger formatterLogger) | ||
{ | ||
if (content.Headers != null && content.Headers.ContentLength == 0) | ||
return null; | ||
|
||
object result; | ||
try | ||
{ | ||
var packer = new CompiledPacker(packPrivateField: false); | ||
result = packer.Unpack(type, readStream); | ||
var packer = SerializationContext.Default.GetSerializer(type); | ||
result = packer.Unpack(stream); | ||
} | ||
catch (Exception ex) | ||
{ | ||
|
@@ -70,23 +74,35 @@ public override object ReadFromStream(Type type, System.IO.Stream readStream, Sy | |
result = GetDefaultValueForType(type); | ||
} | ||
|
||
return result; | ||
return Task.FromResult(result); | ||
} | ||
|
||
public override void WriteToStream(Type type, object value, System.IO.Stream writeStream, System.Net.Http.HttpContent content) | ||
public override Task WriteToStreamAsync(Type type, object value, Stream stream, HttpContent content, TransportContext transportContext) | ||
{ | ||
if (type == null) | ||
throw new ArgumentNullException("type is null"); | ||
throw new ArgumentNullException("type"); | ||
|
||
if (writeStream == null) | ||
throw new ArgumentNullException("writeStream is null"); | ||
if (stream == null) | ||
throw new ArgumentNullException("stream"); | ||
|
||
if (typeof(IEnumerable).IsAssignableFrom(type)) | ||
value = (value as IEnumerable<object>).ToList(); | ||
|
||
var packer = new CompiledPacker(packPrivateField: false); | ||
byte[] buffer = packer.Pack(value); | ||
writeStream.Write(buffer, 0, buffer.Length); | ||
var tcs = new TaskCompletionSource<object>(); | ||
|
||
try | ||
{ | ||
var packer = SerializationContext.Default.GetSerializer(type); | ||
packer.Pack(stream, value); | ||
|
||
tcs.SetResult(null); | ||
} | ||
catch (Exception ex) | ||
{ | ||
tcs.SetException(ex); | ||
} | ||
|
||
return tcs.Task; | ||
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. Wouldn't the result be the same using a synchronous approach but without having to use the 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 is. Please see my response above. |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<packages> | ||
<package id="Microsoft.AspNet.WebApi.Client" version="4.0.20710.0" targetFramework="net40-Client" /> | ||
<package id="Microsoft.Net.Http" version="2.0.20710.0" targetFramework="net40-Client" /> | ||
<package id="MsgPack" version="0.1.0.2011042300" targetFramework="net40-Client" /> | ||
<package id="Newtonsoft.Json" version="4.5.11" targetFramework="net40-Client" /> | ||
<package id="Microsoft.AspNet.WebApi.Client" version="5.2.3" targetFramework="net45" /> | ||
<package id="MsgPack.Cli" version="0.5.10" targetFramework="net45" /> | ||
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. @panesofglass - MsgPack.Cli is added here |
||
<package id="Newtonsoft.Json" version="6.0.8" targetFramework="net45" /> | ||
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. Where's the new version of 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 replaced MsgPack with MsgPack.Cli. The MsgPack package has not been updated since October of 2011 while MsgPack.Cli is under active development. 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. Right, but I don't see the 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. See commit 91ae605 |
||
</packages> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<packages> | ||
<package id="Microsoft.AspNet.WebApi.Client" version="4.0.20710.0" targetFramework="net40-Client" /> | ||
<package id="Microsoft.Net.Http" version="2.0.20710.0" targetFramework="net40-Client" /> | ||
<package id="Newtonsoft.Json" version="4.5.11" targetFramework="net40-Client" /> | ||
<package id="NUnit" version="2.6.2" targetFramework="net40-Client" /> | ||
<package id="Should" version="1.1.12.0" targetFramework="net40-Client" /> | ||
<package id="Microsoft.AspNet.WebApi.Client" version="5.2.3" targetFramework="net45" /> | ||
<package id="Newtonsoft.Json" version="6.0.8" targetFramework="net45" /> | ||
<package id="NUnit" version="2.6.4" targetFramework="net45" /> | ||
<package id="Should" version="1.1.20" targetFramework="net45" /> | ||
</packages> |
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.
Everything appears to be synchronous in your implementation below. Why switch from
BufferedMediaTypeFormatter
?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.
Personal preference rather than a compelling technical reason I'm afraid. Given that reasoning, I'll switch it back if you feel strongly about it.
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 fine. We don't need the buffering, and the result is pretty straightforward. Thanks!