refactor(oohelperd): flatten package hierarchy (#834)

In https://github.com/ooni/probe-cli/pull/832's initial diff, I
mentioned it would be cool to flatten oohelperd's hier.

I'm doing this now, and just for the master branch.

This diff is mostly a mechanical refactoring with very light
and apparently rather safe manual changes.

internal/cmd/oohelperd/dns.go

@@ -1,4 +1,8 @@
-package webconnectivity
+package main
+
+//
+// DNS measurements
+//
 
 import (
 	"context"
@@ -13,20 +17,27 @@ import (
 // newfailure is a convenience shortcut to save typing
 var newfailure = tracex.NewFailure
 
-// CtrlDNSResult is the result of the DNS check performed by
+// ctrlDNSResult is the result of the DNS check performed by
 // the Web Connectivity test helper.
-type CtrlDNSResult = webconnectivity.ControlDNSResult
+type ctrlDNSResult = webconnectivity.ControlDNSResult
 
-// DNSConfig configures the DNS check.
+// dnsConfig configures the DNS check.
-type DNSConfig struct {
+type dnsConfig struct {
-	Domain      string
+	// Domain is the MANDATORY domain to resolve.
+	Domain string
+
+	// NewResolver is the MANDATORY factory to create a new resolver.
 	NewResolver func() model.Resolver
-	Out         chan CtrlDNSResult
+
-	Wg          *sync.WaitGroup
+	// Out is the channel where we publish the results.
+	Out chan ctrlDNSResult
+
+	// Wg allows to synchronize with the parent.
+	Wg *sync.WaitGroup
 }
 
-// DNSDo performs the DNS check.
+// dnsDo performs the DNS check.
-func DNSDo(ctx context.Context, config *DNSConfig) {
+func dnsDo(ctx context.Context, config *dnsConfig) {
 	defer config.Wg.Done()
 	reso := config.NewResolver()
 	defer reso.CloseIdleConnections()
@@ -35,7 +46,7 @@ func DNSDo(ctx context.Context, config *DNSConfig) {
 		addrs = []string{} // fix: the old test helper did that
 	}
 	failure := dnsMapFailure(newfailure(err))
-	config.Out <- CtrlDNSResult{Failure: failure, Addrs: addrs}
+	config.Out <- ctrlDNSResult{Failure: failure, Addrs: addrs}
 }
 
 // dnsMapFailure attempts to map netxlite failures to the strings

internal/cmd/oohelperd/dns_test.go

@@ -1,4 +1,4 @@
-package webconnectivity
+package main
 
 import (
 	"context"
@@ -67,7 +67,7 @@ func Test_dnsMapFailure(t *testing.T) {
 func TestDNSDo(t *testing.T) {
 	t.Run("returns non-nil addresses list on nxdomin", func(t *testing.T) {
 		ctx := context.Background()
-		config := &DNSConfig{
+		config := &dnsConfig{
 			Domain: "antani.ooni.org",
 			NewResolver: func() model.Resolver {
 				return &mocks.Resolver{
@@ -83,7 +83,7 @@ func TestDNSDo(t *testing.T) {
 			Wg:  &sync.WaitGroup{},
 		}
 		config.Wg.Add(1)
-		DNSDo(ctx, config)
+		dnsDo(ctx, config)
 		config.Wg.Wait()
 		resp := <-config.Out
 		if resp.Addrs == nil || len(resp.Addrs) != 0 {

internal/cmd/oohelperd/fake_test.go

@@ -1,4 +1,4 @@
-package webconnectivity
+package main
 
 import (
 	"context"

internal/cmd/oohelperd/handler.go

@@ -1,4 +1,8 @@
-package webconnectivity
+package main
+
+//
+// HTTP handler
+//
 
 import (
 	"encoding/json"
@@ -12,15 +16,25 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/version"
 )
 
-// Handler implements the Web Connectivity test helper HTTP API.
+// handler implements the Web Connectivity test helper HTTP API.
-type Handler struct {
+type handler struct {
+	// MaxAcceptableBody is the MANDATORY maximum acceptable response body.
 	MaxAcceptableBody int64
-	NewClient         func() model.HTTPClient
+
-	NewDialer         func() model.Dialer
+	// NewClient is the MANDATORY factory to create a new HTTPClient.
-	NewResolver       func() model.Resolver
+	NewClient func() model.HTTPClient
+
+	// NewDialer is the MANDATORY factory to create a new Dialer.
+	NewDialer func() model.Dialer
+
+	// NewResolver is the MANDATORY factory for creating a new resolver.
+	NewResolver func() model.Resolver
 }
 
-func (h Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+var _ http.Handler = &handler{}
+
+// ServeHTTP implements http.Handler.ServeHTTP.
+func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	w.Header().Add("Server", fmt.Sprintf(
 		"oohelperd/%s ooniprobe-engine/%s", version.Version, version.Version,
 	))
@@ -34,19 +48,18 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		w.WriteHeader(400)
 		return
 	}
-	var creq CtrlRequest
+	var creq ctrlRequest
 	if err := json.Unmarshal(data, &creq); err != nil {
 		w.WriteHeader(400)
 		return
 	}
-	measureConfig := MeasureConfig(h)
+	cresp, err := measure(req.Context(), h, &creq)
-	cresp, err := Measure(req.Context(), measureConfig, &creq)
 	if err != nil {
 		w.WriteHeader(400)
 		return
 	}
 	// We assume that the following call cannot fail because it's a
-	// clearly serializable data structure.
+	// clearly-serializable data structure.
 	data, err = json.Marshal(cresp)
 	runtimex.PanicOnError(err, "json.Marshal failed")
 	w.Header().Add("Content-Type", "application/json")

internal/cmd/oohelperd/handler_test.go

@@ -1,4 +1,4 @@
-package webconnectivity
+package main
 
 import (
 	"context"
@@ -50,7 +50,7 @@ const requestWithoutDomainName = `{
 }`
 
 func TestWorkingAsIntended(t *testing.T) {
-	handler := Handler{
+	handler := &handler{
 		MaxAcceptableBody: 1 << 24,
 		NewClient: func() model.HTTPClient {
 			return http.DefaultClient
@@ -148,7 +148,7 @@ func TestWorkingAsIntended(t *testing.T) {
 
 func TestHandlerWithRequestBodyReadingError(t *testing.T) {
 	expected := errors.New("mocked error")
-	handler := Handler{MaxAcceptableBody: 1 << 24}
+	handler := handler{MaxAcceptableBody: 1 << 24}
 	rw := NewFakeResponseWriter()
 	req := &http.Request{
 		Method: "POST",

internal/cmd/oohelperd/http.go

@@ -1,4 +1,8 @@
-package webconnectivity
+package main
+
+//
+// HTTP measurements
+//
 
 import (
 	"context"
@@ -13,26 +17,37 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/tracex"
 )
 
-// CtrlHTTPResponse is the result of the HTTP check performed by
+// ctrlHTTPResponse is the result of the HTTP check performed by
 // the Web Connectivity test helper.
-type CtrlHTTPResponse = webconnectivity.ControlHTTPRequestResult
+type ctrlHTTPResponse = webconnectivity.ControlHTTPRequestResult
 
-// HTTPConfig configures the HTTP check.
+// httpConfig configures the HTTP check.
-type HTTPConfig struct {
+type httpConfig struct {
-	Headers           map[string][]string
+	// Headers is OPTIONAL and contains the request headers we should set.
+	Headers map[string][]string
+
+	// MaxAcceptableBody is MANDATORY and specifies the maximum acceptable body size.
 	MaxAcceptableBody int64
-	NewClient         func() model.HTTPClient
+
-	Out               chan CtrlHTTPResponse
+	// NewClient is the MANDATORY factory to create a new client.
-	URL               string
+	NewClient func() model.HTTPClient
-	Wg                *sync.WaitGroup
+
+	// Out is the MANDATORY channel where we'll post results.
+	Out chan ctrlHTTPResponse
+
+	// URL is the MANDATORY URL to measure.
+	URL string
+
+	// Wg is MANDATORY and allows synchronizing with parent.
+	Wg *sync.WaitGroup
 }
 
-// HTTPDo performs the HTTP check.
+// httpDo performs the HTTP check.
-func HTTPDo(ctx context.Context, config *HTTPConfig) {
+func httpDo(ctx context.Context, config *httpConfig) {
 	defer config.Wg.Done()
 	req, err := http.NewRequestWithContext(ctx, "GET", config.URL, nil)
 	if err != nil {
-		config.Out <- CtrlHTTPResponse{ // fix: emit -1 like the old test helper does
+		config.Out <- ctrlHTTPResponse{ // fix: emit -1 like the old test helper does
 			BodyLength: -1,
 			Failure:    httpMapFailure(err),
 			StatusCode: -1,
@@ -54,7 +69,7 @@ func HTTPDo(ctx context.Context, config *HTTPConfig) {
 	defer clnt.CloseIdleConnections()
 	resp, err := clnt.Do(req)
 	if err != nil {
-		config.Out <- CtrlHTTPResponse{ // fix: emit -1 like old test helper does
+		config.Out <- ctrlHTTPResponse{ // fix: emit -1 like old test helper does
 			BodyLength: -1,
 			Failure:    httpMapFailure(err),
 			StatusCode: -1,
@@ -69,7 +84,7 @@ func HTTPDo(ctx context.Context, config *HTTPConfig) {
 	}
 	reader := &io.LimitedReader{R: resp.Body, N: config.MaxAcceptableBody}
 	data, err := netxlite.ReadAllContext(ctx, reader)
-	config.Out <- CtrlHTTPResponse{
+	config.Out <- ctrlHTTPResponse{
 		BodyLength: int64(len(data)),
 		Failure:    httpMapFailure(err),
 		StatusCode: int64(resp.StatusCode),

internal/cmd/oohelperd/http_test.go

@@ -1,4 +1,4 @@
-package webconnectivity
+package main
 
 import (
 	"context"
@@ -15,9 +15,9 @@ import (
 func TestHTTPDoWithInvalidURL(t *testing.T) {
 	ctx := context.Background()
 	wg := new(sync.WaitGroup)
-	httpch := make(chan CtrlHTTPResponse, 1)
+	httpch := make(chan ctrlHTTPResponse, 1)
 	wg.Add(1)
-	go HTTPDo(ctx, &HTTPConfig{
+	go httpDo(ctx, &httpConfig{
 		Headers:           nil,
 		MaxAcceptableBody: 1 << 24,
 		NewClient: func() model.HTTPClient {
@@ -39,9 +39,9 @@ func TestHTTPDoWithHTTPTransportFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
 	wg := new(sync.WaitGroup)
-	httpch := make(chan CtrlHTTPResponse, 1)
+	httpch := make(chan ctrlHTTPResponse, 1)
 	wg.Add(1)
-	go HTTPDo(ctx, &HTTPConfig{
+	go httpDo(ctx, &httpConfig{
 		Headers:           nil,
 		MaxAcceptableBody: 1 << 24,
 		NewClient: func() model.HTTPClient {

internal/cmd/oohelperd/main.go

@@ -1,4 +1,4 @@
-// Command oohelperd contains the Web Connectivity test helper.
+// Command oohelperd implements the Web Connectivity test helper.
 package main
 
 import (
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal/webconnectivity"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
@@ -27,7 +26,7 @@ func init() {
 	srvctx, srvcancel = context.WithCancel(context.Background())
 }
 
-func newresolver() model.Resolver {
+func newResolver() model.Resolver {
 	// Implementation note: pin to a specific resolver so we don't depend upon the
 	// default resolver configured by the box. Also, use an encrypted transport thus
 	// we're less vulnerable to any policy implemented by the box's provider.
@@ -54,15 +53,15 @@ func main() {
 
 func testableMain() {
 	mux := http.NewServeMux()
-	mux.Handle("/", webconnectivity.Handler{
+	mux.Handle("/", &handler{
 		MaxAcceptableBody: maxAcceptableBody,
 		NewClient: func() model.HTTPClient {
-			return netxlite.NewHTTPClientWithResolver(log.Log, newresolver())
+			return netxlite.NewHTTPClientWithResolver(log.Log, newResolver())
 		},
 		NewDialer: func() model.Dialer {
-			return netxlite.NewDialerWithResolver(log.Log, newresolver())
+			return netxlite.NewDialerWithResolver(log.Log, newResolver())
 		},
-		NewResolver: newresolver,
+		NewResolver: newResolver,
 	})
 	srv := &http.Server{Addr: *endpoint, Handler: mux}
 	srvwg.Add(1)

internal/cmd/oohelperd/measure.go

@@ -1,4 +1,8 @@
-package webconnectivity
+package main
+
+//
+// Top-level measurement algorithm
+//
 
 import (
 	"context"
@@ -7,60 +11,54 @@ import (
 	"sync"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity"
-	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
 type (
-	// CtrlRequest is the request sent to the test helper
+	// ctrlRequest is the request sent to the test helper
-	CtrlRequest = webconnectivity.ControlRequest
+	ctrlRequest = webconnectivity.ControlRequest
 
-	// CtrlResponse is the response from the test helper
+	// ctrlResponse is the response from the test helper
-	CtrlResponse = webconnectivity.ControlResponse
+	ctrlResponse = webconnectivity.ControlResponse
 )
 
-// MeasureConfig contains configuration for Measure.
+// measure performs the measurement described by the request and
-type MeasureConfig struct {
-	MaxAcceptableBody int64
-	NewClient         func() model.HTTPClient
-	NewDialer         func() model.Dialer
-	NewResolver       func() model.Resolver
-}
-
-// Measure performs the measurement described by the request and
 // returns the corresponding response or an error.
-func Measure(ctx context.Context, config MeasureConfig, creq *CtrlRequest) (*CtrlResponse, error) {
+func measure(ctx context.Context, config *handler, creq *ctrlRequest) (*ctrlResponse, error) {
 	// parse input for correctness
 	URL, err := url.Parse(creq.HTTPRequest)
 	if err != nil {
 		return nil, err
 	}
+	wg := &sync.WaitGroup{}
+
 	// dns: start
-	wg := new(sync.WaitGroup)
+	dnsch := make(chan ctrlDNSResult, 1)
-	dnsch := make(chan CtrlDNSResult, 1)
 	if net.ParseIP(URL.Hostname()) == nil {
 		wg.Add(1)
-		go DNSDo(ctx, &DNSConfig{
+		go dnsDo(ctx, &dnsConfig{
 			Domain:      URL.Hostname(),
 			NewResolver: config.NewResolver,
 			Out:         dnsch,
 			Wg:          wg,
 		})
 	}
+
 	// tcpconnect: start
-	tcpconnch := make(chan TCPResultPair, len(creq.TCPConnect))
+	tcpconnch := make(chan tcpResultPair, len(creq.TCPConnect))
 	for _, endpoint := range creq.TCPConnect {
 		wg.Add(1)
-		go TCPDo(ctx, &TCPConfig{
+		go tcpDo(ctx, &tcpConfig{
 			Endpoint:  endpoint,
 			NewDialer: config.NewDialer,
 			Out:       tcpconnch,
 			Wg:        wg,
 		})
 	}
+
 	// http: start
-	httpch := make(chan CtrlHTTPResponse, 1)
+	httpch := make(chan ctrlHTTPResponse, 1)
 	wg.Add(1)
-	go HTTPDo(ctx, &HTTPConfig{
+	go httpDo(ctx, &httpConfig{
 		Headers:           creq.HTTPRequestHeaders,
 		MaxAcceptableBody: config.MaxAcceptableBody,
 		NewClient:         config.NewClient,
@@ -68,25 +66,28 @@ func Measure(ctx context.Context, config MeasureConfig, creq *CtrlRequest) (*Ctr
 		URL:               creq.HTTPRequest,
 		Wg:                wg,
 	})
+
 	// wait for measurement steps to complete
 	wg.Wait()
+
 	// assemble response
-	cresp := new(CtrlResponse)
+	cresp := new(ctrlResponse)
 	select {
 	case cresp.DNS = <-dnsch:
 	default:
 		// we need to emit a non-nil Addrs to match exactly
 		// the behavior of the legacy TH
-		cresp.DNS = CtrlDNSResult{
+		cresp.DNS = ctrlDNSResult{
 			Failure: nil,
 			Addrs:   []string{},
 		}
 	}
 	cresp.HTTPRequest = <-httpch
-	cresp.TCPConnect = make(map[string]CtrlTCPResult)
+	cresp.TCPConnect = make(map[string]ctrlTCPResult)
 	for len(cresp.TCPConnect) < len(creq.TCPConnect) {
 		tcpconn := <-tcpconnch
 		cresp.TCPConnect[tcpconn.Endpoint] = tcpconn.Result
 	}
+
 	return cresp, nil
 }

internal/cmd/oohelperd/tcpconnect.go

@@ -1,4 +1,8 @@
-package webconnectivity
+package main
+
+//
+// TCP connect measurements
+//
 
 import (
 	"context"
@@ -9,25 +13,35 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
 
-// CtrlTCPResult is the result of the TCP check performed by the test helper.
+// ctrlTCPResult is the result of the TCP check performed by the test helper.
-type CtrlTCPResult = webconnectivity.ControlTCPConnectResult
+type ctrlTCPResult = webconnectivity.ControlTCPConnectResult
 
-// TCPResultPair contains the endpoint and the corresponding result.
+// tcpResultPair contains the endpoint and the corresponding result.
-type TCPResultPair struct {
+type tcpResultPair struct {
+	// Endpoint is the endpoint we measured.
 	Endpoint string
-	Result   CtrlTCPResult
+
+	// Result contains the results.
+	Result ctrlTCPResult
 }
 
-// TCPConfig configures the TCP connect check.
+// tcpConfig configures the TCP connect check.
-type TCPConfig struct {
+type tcpConfig struct {
-	Endpoint  string
+	// Endpoint is the MANDATORY endpoint to connect to.
+	Endpoint string
+
+	// NewDialer is the MANDATORY factory for creating a new dialer.
 	NewDialer func() model.Dialer
-	Out       chan TCPResultPair
+
-	Wg        *sync.WaitGroup
+	// Out is the MANDATORY where we'll post the TCP measurement results.
+	Out chan tcpResultPair
+
+	// Wg is MANDATORY and is used to sync with the parent.
+	Wg *sync.WaitGroup
 }
 
-// TCPDo performs the TCP check.
+// tcpDo performs the TCP check.
-func TCPDo(ctx context.Context, config *TCPConfig) {
+func tcpDo(ctx context.Context, config *tcpConfig) {
 	defer config.Wg.Done()
 	dialer := config.NewDialer()
 	defer dialer.CloseIdleConnections()
@@ -35,9 +49,9 @@ func TCPDo(ctx context.Context, config *TCPConfig) {
 	if conn != nil {
 		conn.Close()
 	}
-	config.Out <- TCPResultPair{
+	config.Out <- tcpResultPair{
 		Endpoint: config.Endpoint,
-		Result: CtrlTCPResult{
+		Result: ctrlTCPResult{
 			Failure: tcpMapFailure(newfailure(err)),
 			Status:  err == nil,
 		},

internal/cmd/oohelperd/tcpconnect_test.go

@@ -1,4 +1,4 @@
-package webconnectivity
+package main
 
 import (
 	"testing"