HeavyHorst / easykv

very simple Key/Value store abstraction library
MIT License
12 stars 7 forks source link

add optional headers to file backend #7

Closed andreykaipov closed 5 years ago

andreykaipov commented 5 years ago

First step before https://github.com/HeavyHorst/remco/issues/34.

I didn't want to change the logic of GetValues, so I opted for a custom transport that will attach any optional headers for us. Alternatively, I was thinking maybe we could split out the remote file functionality out of file into its own http backend? It doesn't feel necessary right now, but I feel if more options are ever added, it might be.

Advice on how I might write some tests for this would be appreciated! I looked through the other backends, and noticed their options aren't really tested either though, so idk if you're cool with just this.

HeavyHorst commented 5 years ago

Thanks! I like the solution with the custom transport.

We could use the httptest package to test this. Here is an example from https://golang.org/pkg/net/http/httptest/#Server:

package main

import (
    "fmt"
    "io/ioutil"
    "log"
    "net/http"
    "net/http/httptest"
)

func main() {
    ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        fmt.Fprintln(w, "Hello, client")
    }))
    defer ts.Close()

    res, err := http.Get(ts.URL)
    if err != nil {
        log.Fatal(err)
    }
    greeting, err := ioutil.ReadAll(res.Body)
    res.Body.Close()
    if err != nil {
        log.Fatal(err)
    }

    fmt.Printf("%s", greeting)
}

We could create a handler that returns the http headers as json and test if they match our provided ones.

andreykaipov commented 5 years ago

Added a test to have it parse its own headers. That's a pretty cool idea. Thank you! :-) Coverage for the file backend went up a bit too

HeavyHorst commented 5 years ago

LGTM! Thanks again.