aofei / air

An ideally refined web framework for Go.
https://pkg.go.dev/github.com/aofei/air
MIT License
441 stars 37 forks source link

Msgpack Support #14

Closed SaulDoesCode closed 5 years ago

SaulDoesCode commented 5 years ago

Initial pr for msgpack support, hang tight a bit, I'll add some more commits just now.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 355


Changes Missing Coverage Covered Lines Changed/Added Lines %
binder.go 0 2 0.0%
<!-- Total: 0 2 0.0% -->
Files with Coverage Reduction New Missed Lines %
router.go 9 47.38%
<!-- Total: 9 -->
Totals Coverage Status
Change from base Build 354: -0.03%
Covered Lines: 726
Relevant Lines: 2410

💛 - Coveralls
coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 355


Changes Missing Coverage Covered Lines Changed/Added Lines %
binder.go 0 2 0.0%
<!-- Total: 0 2 0.0% -->
Files with Coverage Reduction New Missed Lines %
router.go 9 47.38%
<!-- Total: 9 -->
Totals Coverage Status
Change from base Build 354: -0.03%
Covered Lines: 726
Relevant Lines: 2410

💛 - Coveralls
coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 356


Changes Missing Coverage Covered Lines Changed/Added Lines %
binder.go 0 2 0.0%
response.go 0 13 0.0%
<!-- Total: 0 15 0.0% -->
Totals Coverage Status
Change from base Build 354: -0.2%
Covered Lines: 726
Relevant Lines: 2423

💛 - Coveralls
SaulDoesCode commented 5 years ago

Ok basic MsgPack support is there.

just a thought I have it as res.WriteMsgpack, do you think it should be res.WriteMsgPack instead?

aofei commented 5 years ago

I prefer the Response#WriteMsgpack().

Msgpack -> MessagePack

Protobuf -> ProtocolBuffers

SaulDoesCode commented 5 years ago

https://stackoverflow.com/questions/6888900/mime-type-for-msgpack

We can add support for both, since msgpack is a lesser known format and people will have varying opinions.

SaulDoesCode commented 5 years ago

From StackOverflow

From Wikipedia : According to RFC 6838 (published in January 2013 : http://tools.ietf.org/html/rfc6838), any use of types in the "x." tree is strongly discouraged. Media types with names beginning with "x-" are no longer considered to be members of this tree since January 2013.

Then use directly application/msgpack

However, if the mime-type is still a concern we could add in the following, just to give people the benefit of the doubt:

switch mt {
case "application/json":
    err = json.NewDecoder(r.Body).Decode(v)
case "application/msgpack", "application/x-msgpack":
    err = msgpack.NewDecoder(r.Body).Decode(v)
case "application/xml":
    err = xml.NewDecoder(r.Body).Decode(v)
case "application/x-www-form-urlencoded", "multipart/form-data":
    err = b.bindParams(v, r.Params())
default:
    return errors.New("unsupported media type")
}

Not sure how to go about it with responses though

aofei commented 5 years ago

case "application/msgpack", "application/x-msgpack":, I think we should do this.