zmb3 / spotify

A Go wrapper for the Spotify Web API
Apache License 2.0
1.34k stars 284 forks source link

V2 #145

Closed strideynet closed 2 years ago

strideynet commented 3 years ago

V2 should include the following breaking changes:

strideynet commented 3 years ago

Hey @zmb3 , having some general thoughts that might be easier to discuss in IRC/Slack/Discord. Is there any platforms I can reach you on?

zmb3 commented 3 years ago

@strideynet you bet. I’m @zmb3 on Gophers slack.

zmb3 commented 3 years ago

@strideynet we should remove the AppVeyor CI as part of this work too. The Github actions are easier to maintain and we really don't do anything OS-specific so AppVeyor's Windows support is not adding vaue.

strideynet commented 3 years ago

@zmb3 Yeah! I can't find the config for that, but Github Actions is much better integrated with github.

zmb3 commented 3 years ago

I deleted it from AppVeyor, so we just need to remove the badge from the README, and might as well delete the .travis.yml file too.

strideynet commented 3 years ago

Perfect.

nzoschke commented 3 years ago

Just wanted to say this is working well for me. I wanted to make a client that is wrapped in a local disk / memcache. I already have an access token handy from a saved OAuth flow so it looks like:

type CacheTransport struct {
    Dir       string
    Transport http.RoundTripper
}

func (t *CacheTransport) RoundTrip(req *http.Request) (*http.Response, error) {
    h := sha1.New()
    h.Write([]byte(req.URL.String()))
    bs := h.Sum(nil)

    filename := filepath.Join(t.Dir, fmt.Sprintf("%x", bs))
    if fileExists(filename) {
        bs, err := ioutil.ReadFile(filename)
        if err != nil {
            return nil, err
        }

        return &http.Response{
            Body:       ioutil.NopCloser(bytes.NewBuffer(bs)),
            Status:     http.StatusText(http.StatusOK),
            StatusCode: http.StatusOK,
        }, nil
    }

    res, err := t.Transport.RoundTrip(req)
    if err != nil {
        return res, err
    }

    bs, err = ioutil.ReadAll(res.Body)
    if err != nil {
        return res, err
    }

    if err := ioutil.WriteFile(filename, bs, 0644); err != nil {
        return res, err
    }

    res.Body = ioutil.NopCloser(bytes.NewBuffer(bs))

    return res, nil
}

auth := spotifyauth.New("", "").Client(context.Background(), &oauth2.Token{
    AccessToken: tok.AccessToken,
    TokenType:   tok.TokenType,
})

tr, err := NewCacheTransport(auth.Transport)
if err != nil {
    return spotify.Client{}, xerrors.Errorf(": %w", err)
}
auth.Transport = tr

c := spotify.New(spotify.HTTPClientOpt(auth), spotify.RetryOpt(true))

I found this impossible to do with V1, so +1 on this restructuring.

strideynet commented 3 years ago

Hey @nzoschke. Glad this is working great for you! Feel free to send any Qs or thoughts my way, really looking to get the dev experience sweet on this 👍

strideynet commented 3 years ago

I've updated the Authenticator New() to take functional options as well. This stops the redundant empty string values when using Client Credentials or PKCE.

conradludgate commented 2 years ago

I'd love to start using this in my project but I would like to wait for it to be merged first. Can I help in anyway?

strideynet commented 2 years ago

@conradludgate, I'm actually hoping to do some work on this tomorrow. I'll let you know then if theres anything that comes to mind.