Skip to content

Commit

Permalink
refactor(probeservices): use httpclientx for check-in (#1580)
Browse files Browse the repository at this point in the history
Closes ooni/probe#2724.

While there (1) make sure we test the check-in API call writes into the
kvstore as a side effect; (2) remove parts of ooapi that have now become
unused due to the change inside this pull request.
  • Loading branch information
bassosimone committed May 2, 2024
1 parent a628af2 commit 66ff45e
Show file tree
Hide file tree
Showing 7 changed files with 256 additions and 146 deletions.
8 changes: 4 additions & 4 deletions internal/checkincache/checkincache.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

// checkInFlagsState is the state created by check-in flags.
const checkInFlagsState = "checkinflags.state"
// CheckInFlagsState is the state created by check-in flags.
const CheckInFlagsState = "checkinflags.state"

// checkInFlagsWrapper is the struct wrapping the check-in flags.
//
Expand All @@ -37,13 +37,13 @@ func Store(kvStore model.KeyValueStore, resp *model.OOAPICheckInResult) error {
}
data, err := json.Marshal(wrapper)
runtimex.PanicOnError(err, "json.Marshal unexpectedly failed")
return kvStore.Set(checkInFlagsState, data)
return kvStore.Set(CheckInFlagsState, data)
}

// GetFeatureFlag returns the value of a check-in feature flag. In case of any
// error this function will always return a false value.
func GetFeatureFlag(kvStore model.KeyValueStore, name string) bool {
data, err := kvStore.Get(checkInFlagsState)
data, err := kvStore.Get(CheckInFlagsState)
if err != nil {
return false // as documented
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checkincache/checkincache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestStore(t *testing.T) {
t.Fatal(err)
}
var wrapper checkInFlagsWrapper
data, err := memstore.Get(checkInFlagsState)
data, err := memstore.Get(CheckInFlagsState)
if err != nil {
t.Fatal(err)
}
Expand Down
39 changes: 0 additions & 39 deletions internal/ooapi/checkin.go

This file was deleted.

79 changes: 0 additions & 79 deletions internal/ooapi/checkin_test.go

This file was deleted.

30 changes: 22 additions & 8 deletions internal/probeservices/checkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import (
"context"

"github.com/ooni/probe-cli/v3/internal/checkincache"
"github.com/ooni/probe-cli/v3/internal/httpapi"
"github.com/ooni/probe-cli/v3/internal/httpclientx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/ooapi"
"github.com/ooni/probe-cli/v3/internal/urlx"
)

// CheckIn function is called by probes asking if there are tests to be run
Expand All @@ -17,17 +17,31 @@ import (
// or an explanatory error, in case of failure.
func (c Client) CheckIn(
ctx context.Context, config model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) {
// prepare endpoint and descriptor for the API call
epnt := c.newHTTPAPIEndpoint()
desc := ooapi.NewDescriptorCheckIn(&config)
// construct the URL to use
URL, err := urlx.ResolveReference(c.BaseURL, "/api/v1/check-in", "")
if err != nil {
return nil, err
}

// issue the API call
resp, err := httpclientx.PostJSON[*model.OOAPICheckInConfig, *model.OOAPICheckInResult](
ctx, URL, &config, &httpclientx.Config{
Authorization: "", // not needed
Client: c.HTTPClient,
Host: c.Host,
Logger: c.Logger,
UserAgent: c.UserAgent,
})

// issue the API call and handle failures
resp, err := httpapi.Call(ctx, desc, epnt)
// handle the case of error
if err != nil {
return nil, err
}

// make sure we track selected parts of the response
// make sure we track selected parts of the response and ignore
// the error because OONI Probe would also work without this caching
// it would only work more poorly, but it does not seem worth it
// crippling it entirely if we cannot write into the kvstore
_ = checkincache.Store(c.KVStore, resp)
return resp, nil
}

0 comments on commit 66ff45e

Please sign in to comment.