Skip to content

Commit

Permalink
Initial ideas to fix httprc API
Browse files Browse the repository at this point in the history
  • Loading branch information
lestrrat committed Jan 31, 2024
1 parent bfca88e commit 8bb4b7e
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 41 deletions.
15 changes: 5 additions & 10 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,10 @@ package httprc
import (
"context"
"fmt"
"net/http"
"sync"
"time"
)

// HTTPClient defines the interface required for the HTTP client
// used within the fetcher.
type HTTPClient interface {
Get(string) (*http.Response, error)
}

// Cache represents a cache that stores resources locally, while
// periodically refreshing the contents based on HTTP header values
// and/or user-supplied hints.
Expand Down Expand Up @@ -65,18 +58,20 @@ func NewCache(ctx context.Context, options ...CacheOption) *Cache {
switch option.Ident() {
case identRefreshWindow{}:
refreshWindow = option.Value().(time.Duration)
case identFetcherWorkerCount{}, identWhitelist{}:
fetcherOptions = append(fetcherOptions, option)
case identErrSink{}:
errSink = option.Value().(ErrSink)
default:
if fo, ok := option.(FetcherOption); ok {
fetcherOptions = append(fetcherOptions, fo)
}
}
}

if refreshWindow < time.Second {
refreshWindow = defaultRefreshWindow
}

fetch := NewFetcher(ctx, fetcherOptions...)
fetch := newFetcher(ctx, fetcherOptions...)
queue := newQueue(ctx, refreshWindow, fetch, errSink)

return &Cache{
Expand Down
33 changes: 21 additions & 12 deletions fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,20 @@ import (
"sync"
)

// HTTPClient defines the interface required for the HTTP client
// used within the fetcher.
type HTTPClient interface {
Do(*http.Request) (*http.Response, error)
}

type fetchRequest struct {
mu sync.RWMutex

// client contains the HTTP Client that can be used to make a
// request. By setting a custom *http.Client, you can for example
// provide a custom http.Transport
//
// If not specified, http.DefaultClient will be used.
// If not specified, whatever specified when Cache is created
client HTTPClient

wl Whitelist
Expand Down Expand Up @@ -48,12 +54,7 @@ type fetcher struct {
requests chan *fetchRequest
}

type Fetcher interface {
Fetch(context.Context, string, ...FetchOption) (*http.Response, error)
fetch(context.Context, *fetchRequest) (*http.Response, error)
}

func NewFetcher(ctx context.Context, options ...FetcherOption) Fetcher {
func newFetcher(ctx context.Context, options ...FetcherOption) *fetcher {
var nworkers int
var wl Whitelist
for _, option := range options {
Expand All @@ -80,7 +81,7 @@ func NewFetcher(ctx context.Context, options ...FetcherOption) Fetcher {
}

func (f *fetcher) Fetch(ctx context.Context, u string, options ...FetchOption) (*http.Response, error) {
var client HTTPClient
var client HTTPClient = http.DefaultClient
var wl Whitelist
for _, option := range options {
//nolint:forcetypeassert
Expand Down Expand Up @@ -139,9 +140,6 @@ LOOP:
req.mu.RLock()
reply := req.reply
client := req.client
if client == nil {
client = http.DefaultClient
}
url := req.url
reqwl := req.wl
req.mu.RUnlock()
Expand All @@ -168,8 +166,19 @@ LOOP:
}

// The body is handled by the consumer of the fetcher
httpreq, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
r := &fetchResult{
err: err,
}
if err := r.reply(ctx, reply); err != nil {
break LOOP
}
continue LOOP
}

//nolint:bodyclose
res, err := client.Get(url)
res, err := client.Do(httpreq)
r := &fetchResult{
res: res,
err: err,
Expand Down
10 changes: 8 additions & 2 deletions options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ interfaces:
comment: |
RegisterOption desribes options that can be passed to `(httprc.Cache).Register()`
- name: CacheOption
methods:
- cacheOption
comment: |
CacheOption desribes options that can be passed to `New()`
- name: FetcherOption
methods:
- cacheOption
- fetcherOption
comment: |
FetcherOption describes options that can be passed to `(httprc.Fetcher).NewFetcher()`
- name: FetcherCacheOption
methods:
- cacheOption
- fetcherOption
- name: FetchOption
comment: |
FetchOption describes options that can be passed to `(httprc.Fetcher).Fetch()`
Expand Down Expand Up @@ -51,7 +57,7 @@ options:
to the fetched resource. The `Transform()` method is only called if the HTTP request
returns a `200 OK` status.
- ident: HTTPClient
interface: FetchRegisterOption
interface: FetchFetcherRegisterOption
argument_type: HTTPClient
comment: |
WithHTTPClient specififes the HTTP Client object that should be used to fetch
Expand Down
10 changes: 5 additions & 5 deletions options_gen.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// This file is auto-generated by github.com/lestrrat-go/option/cmd/genoptions. DO NOT EDIT
// This file is auto-generated by internal/cmd/genoptions/main.go. DO NOT EDIT

package httprc

Expand Down Expand Up @@ -68,14 +68,14 @@ func (*fetchRegisterOption) registerOption() {}
// FetcherOption describes options that can be passed to `(httprc.Fetcher).NewFetcher()`
type FetcherOption interface {
Option
cacheOption()
fetcherOption()
}

type fetcherOption struct {
Option
}

func (*fetcherOption) cacheOption() {}
func (*fetcherOption) fetcherOption() {}

// RegisterOption desribes options that can be passed to `(httprc.Cache).Register()`
type RegisterOption interface {
Expand Down Expand Up @@ -146,8 +146,8 @@ func WithFetcherWorkerCount(v int) FetcherOption {
// WithHTTPClient specififes the HTTP Client object that should be used to fetch
// the resource. For example, if you need an `*http.Client` instance that requires
// special TLS or Authorization setup, you might want to pass it using this option.
func WithHTTPClient(v HTTPClient) FetchRegisterOption {
return &fetchRegisterOption{option.New(identHTTPClient{}, v)}
func WithHTTPClient(v HTTPClient) FetchFetcherRegisterOption {
return &fetchFetcherRegisterOption{option.New(identHTTPClient{}, v)}
}

// WithMinRefreshInterval specifies the minimum refresh interval to be used.
Expand Down
4 changes: 2 additions & 2 deletions queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ type queue struct {
mu sync.RWMutex
registry map[string]*entry
windowSize time.Duration
fetch Fetcher
fetch *fetcher
fetchCond *sync.Cond
fetchQueue []*rqentry

Expand All @@ -146,7 +146,7 @@ func (cf clockFunc) Now() time.Time {
return cf()
}

func newQueue(ctx context.Context, window time.Duration, fetch Fetcher, errSink ErrSink) *queue {
func newQueue(ctx context.Context, window time.Duration, fetch *fetcher, errSink ErrSink) *queue {
fetchLocker := &sync.Mutex{}
rq := &queue{
windowSize: window,
Expand Down
15 changes: 5 additions & 10 deletions queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,12 @@ import (

// Sanity check for the queue portion

type dummyFetcher struct {
type dummyClient struct {
srv *httptest.Server
}

func (f *dummyFetcher) Fetch(_ context.Context, _ string, _ ...FetchOption) (*http.Response, error) {
panic("unimplemented")
}

// URLs must be for f.srv
func (f *dummyFetcher) fetch(_ context.Context, fr *fetchRequest) (*http.Response, error) {
//nolint:noctx
return f.srv.Client().Get(fr.url)
func (c *dummyClient) Do(req *http.Request) (*http.Response, error) {
return c.srv.Client().Do(req)
}

type noErrorSink struct {
Expand All @@ -44,7 +38,8 @@ func TestQueue(t *testing.T) {
}))
defer srv.Close()

q := newQueue(ctx, 15*time.Minute, &dummyFetcher{srv: srv}, &noErrorSink{t: t})
f := newFetcher(ctx, WithHTTPClient(&dummyClient{srv: srv}))
q := newQueue(ctx, 15*time.Minute, f, &noErrorSink{t: t})

base := time.Now()
q.clock = clockFunc(func() time.Time {
Expand Down

0 comments on commit 8bb4b7e

Please sign in to comment.