BrandonRomano / wrecker

A golang request builder for JSON APIs
MIT License
24 stars 0 forks source link

Pluggable encoder/decoders #30

Open benpate opened 9 years ago

benpate commented 9 years ago

Currently, Wrecker is hard coded to use one of two body encodings: "application/x-www-form-urlencoded" or "application/json" depending on the kind of parameters posted.

I'm using another library to handle XML-RPC encoding and would love to be able to use Wrecker to handle the transport. This simplifies a ton of duplicated code, and lets me use all the good stuff that's going in to Wrecker recently, like the debugging interceptor.

To support this, we'd just need to make a place for the encode/decode functions in the Request (or possibly to global client variable). If this is not defined, then we fall back to a default function. Here's a rough idea:

// Encoder reads from an io.Reader and returns: 1) the encoded 
// HTTP Body, 2) the Content-Type, and 3) a rarely-used error variable.
type Encoder function(io.Reader) (string, string, error)

// Decoder reads from an io.Reader and either: 1) populates the 
// interface{} argument with the un-marshaled data, or 2) returns 
// an error explaining why it can't.
type Decoder function(io.Reader, interface{}) error

We'd then make default Encoders/Decoders to handle the current use cases, and applications would be free to provide their own implementations where needed.

This is definitely related to #15 - adding a ContentType function - so it probably makes sense to handle them in one go. We'll need to decide what is the thing that determines which Encoder is used. Is it the Content-Type or is it the presence/absence of parameters? I'm thinking that Content-Type should be the primary determinant, but it should fall back to sniffing parameters if none is present. So, our default would look something like this:

if r.Encoder == nil {
    if r.ContentType == "" {
        if r.Body != nil {
            r.ContentType = "application/json"
        } else {
            r.ContentType = "x-form-urlencoded"
        }
    }

    switch r.ContentType {
    case "application/json":
        r.Encoder = encoder.JSON

    case "application/xml":
        r.Encoder = encoder.XML

    case "x-www-form-urlencoded":
        r.Encoder = encoder.FormURL

    case "x-form-multipart":
        r.Encoder = encoder.FormMultipart
    }
}

Since Wrecker is becoming a huge part of my current project, I'm happy to do this. But, I'd really like an "OK" before I go too far down the path on this, because I don't want to take the project too far off track from where you see it headed.

BrandonRomano commented 9 years ago

From a quick glance I think this is a great idea... Will make the library more flexible while also not muddying up the default interface.

I took the weekend off from doing any work, and hopefully tonight I'll be able to catch up on all these issues + I'll give you better responses for everything there!