From acd4ffff359c8168993767cca786cd91f9ea6a67 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 4 Jun 2021 11:39:00 +0200 Subject: [PATCH] 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 | 4 +++- internal/atomicx/atomicx.go | 3 --- internal/atomicx/example_test.go | 14 +++++++++++ internal/fsx/example_test.go | 34 +++++++++++++++++++++++++++ internal/fsx/fsx.go | 14 +++++++---- internal/kvstore/example_test.go | 27 ++++++++++++++++++++++ internal/kvstore/memory.go | 2 +- internal/multierror/example_test.go | 16 +++++++++++++ internal/ooapi/client.go | 19 +++++++++------ internal/ooapi/dependencies.go | 22 ++++++++++++++---- internal/ooapi/doc.go | 7 ++---- internal/ooapi/example_test.go | 36 +++++++++++++++++++++++++++++ 12 files changed, 172 insertions(+), 26 deletions(-) create mode 100644 internal/atomicx/example_test.go create mode 100644 internal/fsx/example_test.go create mode 100644 internal/kvstore/example_test.go create mode 100644 internal/multierror/example_test.go create mode 100644 internal/ooapi/example_test.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9634de3..5fbe70a 100644 --- a/CONTRIBUTING.md +++ b/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`. diff --git a/internal/atomicx/atomicx.go b/internal/atomicx/atomicx.go index 33ddf1a..9e0cb8a 100644 --- a/internal/atomicx/atomicx.go +++ b/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" diff --git a/internal/atomicx/example_test.go b/internal/atomicx/example_test.go new file mode 100644 index 0000000..f4cfbe4 --- /dev/null +++ b/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 +} diff --git a/internal/fsx/example_test.go b/internal/fsx/example_test.go new file mode 100644 index 0000000..8457858 --- /dev/null +++ b/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)) +} diff --git a/internal/fsx/fsx.go b/internal/fsx/fsx.go index 89b888f..b2f9dcd 100644 --- a/internal/fsx/fsx.go +++ b/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( - "input path points to a directory: %w", syscall.EISDIR) + return nil, &os.PathError{ + Op: "openFile", + Path: pathname, + Err: syscall.EISDIR, + } } return file, nil } diff --git a/internal/kvstore/example_test.go b/internal/kvstore/example_test.go new file mode 100644 index 0000000..579a64b --- /dev/null +++ b/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 +} diff --git a/internal/kvstore/memory.go b/internal/kvstore/memory.go index 9129e5e..88f81db 100644 --- a/internal/kvstore/memory.go +++ b/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() diff --git a/internal/multierror/example_test.go b/internal/multierror/example_test.go new file mode 100644 index 0000000..bbb178d --- /dev/null +++ b/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 +} diff --git a/internal/ooapi/client.go b/internal/ooapi/client.go index 4a9137f..693531f 100644 --- a/internal/ooapi/client.go +++ b/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 - GobCodec GobCodec // optional - HTTPClient HTTPClient // optional - JSONCodec JSONCodec // optional - KVStore KVStore // mandatory - RequestMaker RequestMaker // optional - UserAgent string // optional + // KVStore is the MANDATORY key-value store. You can use + // the kvstore.Memory{} struct for an in-memory store. + KVStore KVStore + + // The following fields are optional. When they are empty + // we will fallback to sensible defaults. + BaseURL string + GobCodec GobCodec + HTTPClient HTTPClient + JSONCodec JSONCodec + RequestMaker RequestMaker + UserAgent string } diff --git a/internal/ooapi/dependencies.go b/internal/ooapi/dependencies.go index 2abf27e..9a66aa5 100644 --- a/internal/ooapi/dependencies.go +++ b/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) diff --git a/internal/ooapi/doc.go b/internal/ooapi/doc.go index 0117043..79a47de 100644 --- a/internal/ooapi/doc.go +++ b/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 diff --git a/internal/ooapi/example_test.go b/internal/ooapi/example_test.go new file mode 100644 index 0000000..4cb6eb2 --- /dev/null +++ b/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: + if resp == nil { + log.Fatal("expected non-nil response") + } +}