Skip to content

Commit

Permalink
fix(api): Do not cache server responses when refreshing API defs
Browse files Browse the repository at this point in the history
Also ignores any CLI or env parameters passed in when querying
for API definitions: only profile specified params are used.

Fixes #216
  • Loading branch information
wdullaer committed Nov 2, 2023
1 parent c4befd0 commit 13fe1aa
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 52 deletions.
19 changes: 8 additions & 11 deletions cli/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,13 @@ func Load(entrypoint string, root *cobra.Command) (API, error) {
return API{}, err
}

// For fetching specs, we apply a 24-hour cache time if no cache headers
// are set. So APIs can opt-in to caching if they want control, otherwise
// we try and do the right thing and not hit them too often. Localhost
// is never cached to make local development easier.
client := MinCachedTransport(24 * time.Hour).Client()
if viper.GetBool("rsh-no-cache") || req.URL.Hostname() == "localhost" {
client = &http.Client{Transport: InvalidateCachedTransport()}
}

httpResp, err := MakeRequest(req, WithClient(client))
// We already cache the parsed API specs, no need to cache the
// server response.
// We will almost never be in a situation where we don't want to use
// the parsed API cache, but do want to use a cached response from
// the server.
client := &http.Client{Transport: InvalidateCachedTransport()}
httpResp, err := MakeRequest(req, WithClient(client), IgnoreCLIParams())
if err != nil {
return API{}, err
}
Expand Down Expand Up @@ -249,7 +246,7 @@ func Load(entrypoint string, root *cobra.Command) (API, error) {
return API{}, err
}

resp, err := MakeRequest(req, WithClient(client))
resp, err := MakeRequest(req, WithClient(client), IgnoreCLIParams())
if err != nil {
return API{}, err
}
Expand Down
87 changes: 46 additions & 41 deletions cli/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,30 +71,40 @@ func fixAddress(addr string) string {
return addr
}

type requestOption struct {
client *http.Client
disableLog bool
ignoreStatus bool
type requestConfig struct {
client *http.Client
disableLog bool
ignoreStatus bool
ignoreCLIParams bool
}

type requestOption func(*requestConfig)

// WithClient sets the client to use for the request.
func WithClient(c *http.Client) requestOption {
return requestOption{
client: c,
return func(conf *requestConfig) {
conf.client = c
}
}

// WithoutLog disabled debug logging for the given request/response.
func WithoutLog() requestOption {
return requestOption{
disableLog: true,
return func(conf *requestConfig) {
conf.disableLog = true

Check warning on line 93 in cli/request.go

View check run for this annotation

Codecov / codecov/patch

cli/request.go#L92-L93

Added lines #L92 - L93 were not covered by tests
}
}

// IgnoreStatus ignores the response status code.
func IgnoreStatus() requestOption {
return requestOption{
ignoreStatus: true,
return func(conf *requestConfig) {
conf.ignoreStatus = true
}
}

// IgnoreCLIParams only applies the profile, but ignores commandline and env params
func IgnoreCLIParams() requestOption {
return func(conf *requestConfig) {
conf.ignoreCLIParams = true
}
}

Expand All @@ -103,6 +113,11 @@ func IgnoreStatus() requestOption {
// before sending it out on the wire. If verbose mode is enabled, it will
// print out both the request and response.
func MakeRequest(req *http.Request, options ...requestOption) (*http.Response, error) {
requestConf := &requestConfig{}
for _, opt := range options {
opt(requestConf)
}

name, config := findAPI(req.URL.String())

if config == nil {
Expand Down Expand Up @@ -134,25 +149,27 @@ func MakeRequest(req *http.Request, options ...requestOption) (*http.Response, e
}
}

// Allow env vars and commandline arguments to override config.
for _, h := range viper.GetStringSlice("rsh-header") {
parts := strings.SplitN(h, ":", 2)
value := ""
if len(parts) > 1 {
value = parts[1]
if !requestConf.ignoreCLIParams {
// Allow env vars and commandline arguments to override config.
for _, h := range viper.GetStringSlice("rsh-header") {
parts := strings.SplitN(h, ":", 2)
value := ""
if len(parts) > 1 {
value = parts[1]
}

req.Header.Add(parts[0], value)
}

req.Header.Add(parts[0], value)
}
for _, q := range viper.GetStringSlice("rsh-query") {
parts := strings.SplitN(q, "=", 2)
value := ""
if len(parts) > 1 {
value = parts[1]

Check warning on line 168 in cli/request.go

View check run for this annotation

Codecov / codecov/patch

cli/request.go#L165-L168

Added lines #L165 - L168 were not covered by tests
}

for _, q := range viper.GetStringSlice("rsh-query") {
parts := strings.SplitN(q, "=", 2)
value := ""
if len(parts) > 1 {
value = parts[1]
query.Add(parts[0], value)

Check warning on line 171 in cli/request.go

View check run for this annotation

Codecov / codecov/patch

cli/request.go#L171

Added line #L171 was not covered by tests
}

query.Add(parts[0], value)
}

// Save modified query string arguments.
Expand Down Expand Up @@ -243,28 +260,16 @@ func MakeRequest(req *http.Request, options ...requestOption) (*http.Response, e
client = &http.Client{Transport: InvalidateCachedTransport()}
}

log := true
setStatus := true
for _, option := range options {
if option.client != nil {
client = option.client
}

if option.disableLog {
log = false
}

if option.ignoreStatus {
setStatus = false
}
if requestConf.client != nil {
client = requestConf.client
}

resp, err := doRequestWithRetry(log, client, req)
resp, err := doRequestWithRetry(!requestConf.disableLog, client, req)
if err != nil {
return nil, err
}

if setStatus {
if !requestConf.ignoreStatus {
lastStatus = resp.StatusCode
}

Expand Down

0 comments on commit 13fe1aa

Please # to comment.