Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: only define ErrNoAvailableTestHelpers once #1583

Merged
merged 54 commits into from
May 2, 2024
Merged
Changes from 1 commit
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
75ef7fd
refactor: consolidate httpx and httpapi
bassosimone Apr 22, 2024
f9210ec
refactor to make testing the whole package easier
bassosimone Apr 23, 2024
587290c
Merge branch 'master' into issue/2700
bassosimone Apr 23, 2024
af394c2
Merge branch 'master' into issue/2700
bassosimone Apr 23, 2024
c6f2f5a
Merge branch 'master' into issue/2700
bassosimone Apr 23, 2024
68c9779
Merge branch 'issue/2700' of github.com:ooni/probe-cli into issue/2700
bassosimone Apr 23, 2024
57e29da
Merge branch 'master' into issue/2700
bassosimone Apr 23, 2024
5c953f0
x
bassosimone Apr 23, 2024
e03e810
x
bassosimone Apr 23, 2024
a6046fd
x
bassosimone Apr 23, 2024
341fcf2
x
bassosimone Apr 23, 2024
8c34524
x
bassosimone Apr 23, 2024
4b464ff
try to entirely remove httpx usages
bassosimone Apr 23, 2024
6d57184
fix: make sure there is nil safety
bassosimone Apr 23, 2024
9c2a226
oxford comma: yes/no?
bassosimone Apr 23, 2024
1123b4e
x
bassosimone Apr 23, 2024
d421d24
fix: unit test needs to be adapted
bassosimone Apr 24, 2024
67e0a10
chore: improve testing for cloudflare IP lookup
bassosimone Apr 24, 2024
a69d981
chore: improve the ubuntu IP lookup tests
bassosimone Apr 24, 2024
cd25c56
Merge branch 'master' into issue/2700
bassosimone Apr 24, 2024
642ae5c
x
bassosimone Apr 24, 2024
548e6bc
doc: document oonirun/v2_test.go tests
bassosimone Apr 24, 2024
40db0e5
Merge branch 'master' into issue/2700
bassosimone Apr 24, 2024
4cf3566
start improving probeservices tests
bassosimone Apr 24, 2024
e736e42
Merge branch 'master' into issue/2700
bassosimone Apr 26, 2024
e8471c4
x
bassosimone Apr 26, 2024
aa1c836
Merge branch 'master' into issue/2700
bassosimone Apr 26, 2024
08e81a9
x
bassosimone Apr 26, 2024
fa74b48
x
bassosimone Apr 26, 2024
a7e748f
x
bassosimone Apr 26, 2024
87146cc
x
bassosimone Apr 26, 2024
dac7b8f
x
bassosimone Apr 26, 2024
04b0071
Merge branch 'master' into issue/2700
bassosimone Apr 26, 2024
79d1fee
Merge branch 'master' into issue/2700
bassosimone Apr 29, 2024
88b399d
Merge branch 'master' into issue/2700
bassosimone Apr 29, 2024
de23e7d
x
bassosimone Apr 29, 2024
9d87673
Merge branch 'master' into issue/2700
bassosimone Apr 29, 2024
a436f1e
x
bassosimone Apr 29, 2024
08f8ca9
Merge branch 'master' into issue/2700
bassosimone Apr 29, 2024
25140f3
x
bassosimone Apr 29, 2024
1bbe0b7
chore: write tests for oonicollector.go
bassosimone Apr 30, 2024
6707d61
Merge branch 'master' into issue/2700
bassosimone Apr 30, 2024
4ddd507
Merge branch 'master' into issue/2700
bassosimone Apr 30, 2024
c453ee2
x
bassosimone Apr 30, 2024
ad3d84f
Merge branch 'master' into issue/2700
bassosimone May 2, 2024
28d64f1
feat(probeservices): use httpclientx for check-in
bassosimone May 2, 2024
2107750
cleanup: remove check-in from ooapi
bassosimone May 2, 2024
c2c8ebf
Merge branch 'master' into issue/2700
bassosimone May 2, 2024
36610a8
feat: start moving TH call into engine/session.go
bassosimone May 2, 2024
b7ccf2f
Merge branch 'master' into issue/2700
bassosimone May 2, 2024
b94a8b8
x
bassosimone May 2, 2024
5f1994c
x
bassosimone May 2, 2024
6e16369
Merge branch 'master' into issue/2700
bassosimone May 2, 2024
8400bde
x
bassosimone May 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: start moving TH call into engine/session.go
bassosimone committed May 2, 2024
commit 36610a8153b479bf09f8b19e957aaf9bd9c42ffd
32 changes: 32 additions & 0 deletions internal/engine/session.go
Original file line number Diff line number Diff line change
@@ -13,8 +13,10 @@ import (
"github.com/ooni/probe-cli/v3/internal/enginelocate"
"github.com/ooni/probe-cli/v3/internal/enginenetx"
"github.com/ooni/probe-cli/v3/internal/engineresolver"
"github.com/ooni/probe-cli/v3/internal/httpapi"
"github.com/ooni/probe-cli/v3/internal/kvstore"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/ooapi"
"github.com/ooni/probe-cli/v3/internal/platform"
"github.com/ooni/probe-cli/v3/internal/probeservices"
"github.com/ooni/probe-cli/v3/internal/runtimex"
@@ -688,4 +690,34 @@ func (s *Session) MaybeLookupLocationContext(ctx context.Context) error {
return nil
}

// CallWebConnectivityTestHelper implements [model.EngineExperimentSession].
func (s *Session) CallWebConnectivityTestHelper(ctx context.Context,
creq *model.THRequest, testhelpers []model.OOAPIService) (*model.THResponse, int, error) {
// handle the case where there are no available web connectivity test helpers
if len(testhelpers) <= 0 {
return nil, 0, model.ErrNoAvailableTestHelpers
}

// initialize a sequence caller for invoking the THs in FIFO order
seqCaller := httpapi.NewSequenceCaller(
ooapi.NewDescriptorTH(creq),
httpapi.NewEndpointList(s.DefaultHTTPClient(), s.Logger(), s.UserAgent(), testhelpers...)...,
)

// issue the composed call proper and obtain a response and an index or an error
cresp, idx, err := seqCaller.Call(ctx)

// handle the case where all test helpers failed
if err != nil {
return nil, 0, err
}

// apply some sanity checks to the results
runtimex.Assert(idx >= 0 && idx < len(testhelpers), "idx out of bounds")
runtimex.Assert(cresp != nil, "out is nil")

// return the results to the web connectivity caller
return cresp, idx, nil
}

var _ model.ExperimentSession = &Session{}
8 changes: 1 addition & 7 deletions internal/experiment/webconnectivity/control.go
Original file line number Diff line number Diff line change
@@ -4,10 +4,8 @@ import (
"context"

"github.com/ooni/probe-cli/v3/internal/geoipx"
"github.com/ooni/probe-cli/v3/internal/httpapi"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/ooapi"
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

@@ -24,12 +22,8 @@ type (
func Control(
ctx context.Context, sess model.ExperimentSession,
testhelpers []model.OOAPIService, creq ControlRequest) (ControlResponse, *model.OOAPIService, error) {
seqCaller := httpapi.NewSequenceCaller(
ooapi.NewDescriptorTH(&creq),
httpapi.NewEndpointList(sess.DefaultHTTPClient(), sess.Logger(), sess.UserAgent(), testhelpers...)...,
)
sess.Logger().Infof("control for %s...", creq.HTTPRequest)
out, idx, err := seqCaller.Call(ctx)
out, idx, err := sess.CallWebConnectivityTestHelper(ctx, &creq, testhelpers)
sess.Logger().Infof("control for %s... %+v", creq.HTTPRequest, model.ErrorToStringOrOK(err))
if err != nil {
// make sure error is wrapped
10 changes: 1 addition & 9 deletions internal/experiment/webconnectivitylte/control.go
Original file line number Diff line number Diff line change
@@ -8,11 +8,9 @@ import (
"time"

"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity"
"github.com/ooni/probe-cli/v3/internal/httpapi"
"github.com/ooni/probe-cli/v3/internal/logx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/ooapi"
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

@@ -109,14 +107,8 @@ func (c *Control) Run(parentCtx context.Context) {
c.TestHelpers,
)

// create an httpapi sequence caller
seqCaller := httpapi.NewSequenceCaller(
ooapi.NewDescriptorTH(creq),
httpapi.NewEndpointList(c.Session.DefaultHTTPClient(), c.Logger, c.Session.UserAgent(), c.TestHelpers...)...,
)

// issue the control request and wait for the response
cresp, idx, err := seqCaller.Call(opCtx)
cresp, idx, err := c.Session.CallWebConnectivityTestHelper(opCtx, creq, c.TestHelpers)
if err != nil {
// make sure error is wrapped
err = netxlite.NewTopLevelGenericErrWrapper(err)
6 changes: 6 additions & 0 deletions internal/legacy/mockable/mockable.go
Original file line number Diff line number Diff line change
@@ -38,6 +38,12 @@ type Session struct {
MockableUserAgent string
}

// CallWebConnectivityTestHelper implements [model.EngineExperimentSession].
func (sess *Session) CallWebConnectivityTestHelper(
ctx context.Context, request *model.THRequest, ths []model.OOAPIService) (*model.THResponse, int, error) {
panic("not implemented")
}

// GetTestHelpersByName implements ExperimentSession.GetTestHelpersByName
func (sess *Session) GetTestHelpersByName(name string) ([]model.OOAPIService, bool) {
services, okay := sess.MockableTestHelpers[name]
5 changes: 5 additions & 0 deletions internal/mocks/session.go
Original file line number Diff line number Diff line change
@@ -58,6 +58,11 @@ type Session struct {
config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error)
}

func (sess *Session) CallWebConnectivityTestHelper(ctx context.Context,
req *model.THRequest, ths []model.OOAPIService) (*model.THResponse, int, error) {
panic("not implemented")
}

func (sess *Session) GetTestHelpersByName(name string) ([]model.OOAPIService, bool) {
return sess.MockGetTestHelpersByName(name)
}
20 changes: 20 additions & 0 deletions internal/model/experiment.go
Original file line number Diff line number Diff line change
@@ -7,14 +7,34 @@ package model

import (
"context"
"errors"
)

// ErrNoAvailableTestHelpers is emitted when there are no available test helpers.
var ErrNoAvailableTestHelpers = errors.New("no available helpers")

// ExperimentSession is the experiment's view of a session.
type ExperimentSession interface {
// CallWebConnectivityTestHelper invokes the Web Connectivity test helper with the
// given request object and the given list of available test helpers.
//
// If the list of test helpers is empty this function immediately returns nil, zero,
// and the [ErrNoAvailableTestHelpers] error to the caller.
//
// In case of any other failure, this function returns nil, zero, and an error
//
// On success, it returns the response, the used TH index, and nil.
//
// Note that the returned error won't be wrapped, so you need to wrap it yourself.
CallWebConnectivityTestHelper(
ctx context.Context, request *THRequest, ths []OOAPIService) (*THResponse, int, error)

// GetTestHelpersByName returns a list of test helpers with the given name.
GetTestHelpersByName(name string) ([]OOAPIService, bool)

// DefaultHTTPClient returns the default HTTPClient used by the session.
//
// Deprecated: Web Connectivity should use CallWebConnectivityTestHelper instead.
DefaultHTTPClient() HTTPClient

// FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error.