doc: cleanup and improve for recently moved pkgs (#354)

* chore(atomicx): review docs and add usage example

* chore(fsx): improve docs, return value, add examples

* fix(kvstore): correct typo and add example

* fix(multierror): add basic example

* doc: revamp ooapi documentation

CONTRIBUTING.md

@@ -57,7 +57,9 @@ run `go mod tidy` to minimize such changes.
 
 ## Implementation requirements
 
-Please, use `./internal/atomicx` rather than `atomic/sync`.
+- use `./internal/atomicx` rather than `atomic/sync`
+
+- use `./internal/fsx.OpenFile` when you need to open a file
 
 Do now use `os/exec`, use `x/sys/execabs`.
 

internal/atomicx/atomicx.go

@@ -13,9 +13,6 @@
 // 32 bit platform, but that's difficult to do correctly. This package
 // provides an easier-to-use interface. We use allocated
 // structures protected by a mutex that encapsulate a int64 value.
-//
-// While there we also added support for atomic float64 operations, again
-// by using structures protected by a mutex variable.
 package atomicx
 
 import "sync"

internal/atomicx/example_test.go

@@ -0,0 +1,14 @@
+package atomicx_test
+
+import (
+	"fmt"
+
+	"github.com/ooni/probe-cli/v3/internal/atomicx"
+)
+
+func Example_typicalUsage() {
+	v := &atomicx.Int64{}
+	v.Add(1)
+	fmt.Printf("%d\n", v.Load())
+	// Output: 1
+}

internal/fsx/example_test.go

@@ -0,0 +1,34 @@
+package fsx_test
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"log"
+	"path/filepath"
+	"syscall"
+
+	"github.com/ooni/probe-cli/v3/internal/fsx"
+)
+
+func ExampleOpenFile_openingDir() {
+	filep, err := fsx.OpenFile("testdata")
+	if !errors.Is(err, syscall.ENOENT) {
+		log.Fatal("unexpected error", err)
+	}
+	if filep != nil {
+		log.Fatal("expected nil fp")
+	}
+}
+
+func ExampleOpenFile_openingFile() {
+	filep, err := fsx.OpenFile(filepath.Join("testdata", "testfile.txt"))
+	if err != nil {
+		log.Fatal("unexpected error", err)
+	}
+	data, err := io.ReadAll(filep)
+	if err != nil {
+		log.Fatal("unexpected error", err)
+	}
+	fmt.Printf("%d\n", len(data))
+}

internal/fsx/fsx.go

@@ -2,7 +2,6 @@
 package fsx
 
 import (
-	"fmt"
 	"io/fs"
 	"os"
 	"syscall"
@@ -10,7 +9,11 @@ import (
 
 // OpenFile is a wrapper for os.OpenFile that ensures that
 // we're opening a file rather than a directory. If you are
-// opening a directory, this func will return an error.
+// opening a directory, this func returns an *os.PathError
+// error with Err set to syscall.EISDIR.
+//
+// As mentioned in CONTRIBUTING.md, this is the function
+// you SHOULD be using when opening files.
 func OpenFile(pathname string) (fs.File, error) {
 	return openWithFS(filesystem{}, pathname)
 }
@@ -28,8 +31,11 @@ func openWithFS(fs fs.FS, pathname string) (fs.File, error) {
 	}
 	if info.IsDir() {
 		file.Close()
-		return nil, fmt.Errorf(
+		return nil, &os.PathError{
-			"input path points to a directory: %w", syscall.EISDIR)
+			Op:   "openFile",
+			Path: pathname,
+			Err:  syscall.EISDIR,
+		}
 	}
 	return file, nil
 }

internal/kvstore/example_test.go

@@ -0,0 +1,27 @@
+package kvstore_test
+
+import (
+	"errors"
+	"fmt"
+	"log"
+	"reflect"
+
+	"github.com/ooni/probe-cli/v3/internal/kvstore"
+)
+
+func ExampleMemory() {
+	kvs := &kvstore.Memory{}
+	if _, err := kvs.Get("akey"); !errors.Is(err, kvstore.ErrNoSuchKey) {
+		log.Fatal("unexpected error", err)
+	}
+	val := []byte("value")
+	if err := kvs.Set("akey", val); err != nil {
+		log.Fatal("unexpected error", err)
+	}
+	out, err := kvs.Get("akey")
+	if err != nil {
+		log.Fatal("unexpected error", err)
+	}
+	fmt.Printf("%+v\n", reflect.DeepEqual(val, out))
+	// Output: true
+}

internal/kvstore/memory.go

@@ -30,7 +30,7 @@ func (kvs *Memory) Get(key string) ([]byte, error) {
 	return value, nil
 }
 
-// Set sets a key into the key-value store
+// Set sets a key into the key-value store.
 func (kvs *Memory) Set(key string, value []byte) error {
 	kvs.mu.Lock()
 	defer kvs.mu.Unlock()

internal/multierror/example_test.go

@@ -0,0 +1,16 @@
+package multierror_test
+
+import (
+	"errors"
+	"fmt"
+
+	"github.com/ooni/probe-cli/v3/internal/multierror"
+)
+
+func ExampleUnion() {
+	root := errors.New("some error")
+	me := multierror.New(root)
+	check := errors.Is(me, root)
+	fmt.Printf("%+v\n", check)
+	// Output: true
+}

internal/ooapi/client.go

@@ -3,11 +3,16 @@ package ooapi
 // Client is a client for speaking with the OONI API. Make sure you
 // fill in the mandatory fields.
 type Client struct {
-	BaseURL      string       // optional
+	// KVStore is the MANDATORY key-value store. You can use
-	GobCodec     GobCodec     // optional
+	// the kvstore.Memory{} struct for an in-memory store.
-	HTTPClient   HTTPClient   // optional
+	KVStore KVStore
-	JSONCodec    JSONCodec    // optional
+
-	KVStore      KVStore      // mandatory
+	// The following fields are optional. When they are empty
-	RequestMaker RequestMaker // optional
+	// we will fallback to sensible defaults.
-	UserAgent    string       // optional
+	BaseURL      string
+	GobCodec     GobCodec
+	HTTPClient   HTTPClient
+	JSONCodec    JSONCodec
+	RequestMaker RequestMaker
+	UserAgent    string
 }

internal/ooapi/dependencies.go

@@ -6,7 +6,9 @@ import (
 	"net/http"
 )
 
-// JSONCodec is a JSON encoder and decoder.
+// JSONCodec is a JSON encoder and decoder. Generally, we use a
+// default JSONCodec in Client. This is the interface to implement
+// if you want to override such a default.
 type JSONCodec interface {
 	// Encode encodes v as a serialized JSON byte slice.
 	Encode(v interface{}) ([]byte, error)
@@ -15,7 +17,9 @@ type JSONCodec interface {
 	Decode(b []byte, v interface{}) error
 }
 
-// RequestMaker makes an HTTP request.
+// RequestMaker makes an HTTP request. Generally, we use a
+// default RequestMaker in Client. This is the interface to implement
+// if you want to override such a default.
 type RequestMaker interface {
 	// NewRequest creates a new HTTP request.
 	NewRequest(ctx context.Context, method, URL string, body io.Reader) (*http.Request, error)
@@ -29,13 +33,19 @@ type templateExecutor interface {
 	Execute(tmpl string, v interface{}) (string, error)
 }
 
-// HTTPClient is the interface of a generic HTTP client.
+// HTTPClient is the interface of a generic HTTP client. The
+// stdlib's http.Client implements this interface. We use
+// http.DefaultClient as the default HTTPClient used by Client.
+// Consumers of this package typically provide a custom HTTPClient
+// with additional functionality (e.g., DoH, circumvention).
 type HTTPClient interface {
 	// Do should work like http.Client.Do.
 	Do(req *http.Request) (*http.Response, error)
 }
 
-// GobCodec is a Gob encoder and decoder.
+// GobCodec is a Gob encoder and decoder. Generally, we use a
+// default GobCodec in Client. This is the interface to implement
+// if you want to override such a default.
 type GobCodec interface {
 	// Encode encodes v as a serialized gob byte slice.
 	Encode(v interface{}) ([]byte, error)
@@ -44,7 +54,9 @@ type GobCodec interface {
 	Decode(b []byte, v interface{}) error
 }
 
-// KVStore is a key-value store.
+// KVStore is a key-value store. This is the interface the
+// client expect for the key-value store used to save persistent
+// state (typically on the file system).
 type KVStore interface {
 	// Get gets a value from the key-value store.
 	Get(key string) ([]byte, error)

internal/ooapi/doc.go

@@ -15,9 +15,6 @@
 // perform the login. If an API uses caching, we will
 // automatically use the cache.
 //
-// See the example describing auto-login for more information
-// on how to use auto-login.
-//
 // Design
 //
 // Most of the code in this package is auto-generated from the
@@ -46,11 +43,11 @@
 //
 // Architecture
 //
-// The ./apimodel package contains the definition of request
+// The ./apimodel sub-package contains the definition of request
 // and response messages. We rely on tagging to specify how
 // we should encode and decode messages.
 //
-// The ./internal/generator contains code to generate most
+// The ./internal/generator sub-package contains code to generate most
 // code in this package. In particular, the spec.go file is
 // the specification of the APIs.
 package ooapi

internal/ooapi/example_test.go

@@ -0,0 +1,36 @@
+package ooapi_test
+
+import (
+	"context"
+	"fmt"
+	"log"
+
+	"github.com/ooni/probe-cli/v3/internal/kvstore"
+	"github.com/ooni/probe-cli/v3/internal/ooapi"
+	"github.com/ooni/probe-cli/v3/internal/ooapi/apimodel"
+)
+
+func ExampleClient() {
+	clnt := &ooapi.Client{
+		KVStore: &kvstore.Memory{},
+	}
+	ctx := context.Background()
+	resp, err := clnt.CheckIn(ctx, &apimodel.CheckInRequest{
+		Charging:        false,
+		OnWiFi:          false,
+		Platform:        "linux",
+		ProbeASN:        "AS30722",
+		ProbeCC:         "IT",
+		RunType:         "timed",
+		SoftwareName:    "miniooni",
+		SoftwareVersion: "0.1.0-dev",
+		WebConnectivity: apimodel.CheckInRequestWebConnectivity{
+			CategoryCodes: []string{"NEWS"},
+		},
+	})
+	fmt.Printf("%+v\n", err)
+	// Output: <nil>
+	if resp == nil {
+		log.Fatal("expected non-nil response")
+	}
+}