From e25b41ef8b4eba2ebb5bc6e41ecba769c3017497 Mon Sep 17 00:00:00 2001 From: david <58231312+davseby@users.noreply.github.com> Date: Sun, 16 Aug 2026 12:52:09 +0300 Subject: [PATCH] add tests to email, buildinfo, storage packages --- server/core/internal/buildinfo/buildinfo.go | 18 +- .../core/internal/buildinfo/buildinfo_test.go | 143 ++++++ .../core/internal/db/document_search_job.go | 6 +- .../internal/db/document_search_job_test.go | 11 +- .../internal/document/hook/manager/manager.go | 21 +- .../document/hook/manager/manager_test.go | 10 +- server/core/internal/document/summary.go | 13 +- server/core/internal/document/summary_test.go | 8 + .../internal/search/manager/0mock_db_test.go | 30 +- .../core/internal/search/manager/manager.go | 25 +- .../internal/search/manager/manager_test.go | 45 +- server/core/internal/storage/client_test.go | 436 ++++++++++++++++++ server/core/internal/storage/object_test.go | 262 +++++++++++ server/core/pkg/cryptoutil/cryptoutil.go | 3 + server/core/pkg/httpserver/handler_test.go | 53 +++ server/core/pkg/logutil/logutil_test.go | 25 + server/core/pkg/metricutil/metricutil_test.go | 5 + server/core/pkg/metricutil/runtime.go | 49 +- server/core/pkg/metricutil/runtime_test.go | 74 +++ server/core/pkg/metricutil/util_test.go | 15 + server/core/pkg/redkit/value_store_test.go | 91 +++- server/core/pkg/strutil/nanoid.go | 1 + server/core/pkg/testutil/testutil_test.go | 136 ++++++ 23 files changed, 1365 insertions(+), 115 deletions(-) create mode 100644 server/core/internal/buildinfo/buildinfo_test.go create mode 100644 server/core/internal/storage/client_test.go create mode 100644 server/core/internal/storage/object_test.go create mode 100644 server/core/pkg/metricutil/util_test.go diff --git a/server/core/internal/buildinfo/buildinfo.go b/server/core/internal/buildinfo/buildinfo.go index 187a1c9e..d72f4872 100644 --- a/server/core/internal/buildinfo/buildinfo.go +++ b/server/core/internal/buildinfo/buildinfo.go @@ -28,23 +28,25 @@ var ( func init() { //nolint:gochecknoinits // we need to prepare build info logutil.ShowCritical = IsDevEnv() - // only strings can be inserted via ldflags which means - // some information needs to be parsed. Malformed injected values - // fall back to zero defaults instead of failing the boot. + _info.Version, _info.Timestamp = parseBuildValues(_version, _timestampString) +} - version, err := semver.ParseTolerant(_version) +// parseBuildValues parses the injected version and timestamp values. +// Only strings can be inserted via ldflags which means some +// information needs to be parsed. Malformed injected values fall back +// to zero defaults instead of failing the boot. +func parseBuildValues(rawVersion, rawTimestamp string) (semver.Version, time.Time) { + version, err := semver.ParseTolerant(rawVersion) if err != nil { version = semver.Version{} } - _info.Version = version - - timestamp, err := timeutil.Parse(_timestampString) + timestamp, err := timeutil.Parse(rawTimestamp) if err != nil { timestamp = time.Time{} } - _info.Timestamp = timestamp + return version, timestamp } // BuildInfo contains information about the executable. diff --git a/server/core/internal/buildinfo/buildinfo_test.go b/server/core/internal/buildinfo/buildinfo_test.go new file mode 100644 index 00000000..f3be21b4 --- /dev/null +++ b/server/core/internal/buildinfo/buildinfo_test.go @@ -0,0 +1,143 @@ +package buildinfo + +import ( + "testing" + "time" + + "github.com/blang/semver/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + +func Test_parseBuildValues(t *testing.T) { + cc := map[string]struct { + RawVersion string + RawTimestamp string + Version semver.Version + Timestamp time.Time + }{ + "Malformed version": { + RawVersion: "not-a-version", + RawTimestamp: "2006-12-09T19:00:00Z", + Version: semver.Version{}, + Timestamp: time.Date(2006, 12, 9, 19, 0, 0, 0, time.UTC), + }, + "Malformed timestamp": { + RawVersion: "1.2.3", + RawTimestamp: "not-a-timestamp", + Version: semver.MustParse("1.2.3"), + Timestamp: time.Time{}, + }, + "Successful parsing": { + RawVersion: "v1.2.3-rc.1+name", + RawTimestamp: "2021-01-02T03:04:05Z", + Version: semver.MustParse("1.2.3-rc.1+name"), + Timestamp: time.Date(2021, 1, 2, 3, 4, 5, 0, time.UTC), + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + version, timestamp := parseBuildValues(c.RawVersion, c.RawTimestamp) + assert.Equal(t, c.Version, version) + assert.Equal(t, c.Timestamp, timestamp) + }) + } +} + +func Test_BuildInfo_String(t *testing.T) { + t.Parallel() + + b := BuildInfo{ + Name: "oxynote_core", + Version: semver.MustParse("1.2.3"), + Commit: "abc1234", + Timestamp: time.Date(2021, 1, 2, 3, 4, 0, 0, time.UTC), + } + + assert.Equal( + t, + "oxynote_core 1.2.3 (abc1234; 2021-01-02 03:04 UTC)", + b.String(), + ) +} + +func Test_BuildInfo_VersionName(t *testing.T) { + cc := map[string]struct { + Version semver.Version + Result string + }{ + "No build metadata": { + Version: semver.MustParse("1.2.3"), + Result: "unknown", + }, + "Build metadata present": { + Version: semver.MustParse("1.2.3+name.extra"), + Result: "name", + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + b := BuildInfo{Version: c.Version} + assert.Equal(t, c.Result, b.VersionName()) + }) + } +} + +func Test_BuildInfo_PlainVersion(t *testing.T) { + t.Parallel() + + b := BuildInfo{Version: semver.MustParse("1.2.3-rc.1+name")} + assert.Equal(t, "1.2.3-rc.1", b.PlainVersion()) +} + +func Test_BuildInfo_FormattedTimestamp(t *testing.T) { + t.Parallel() + + b := BuildInfo{Timestamp: time.Date(2021, 1, 2, 3, 4, 0, 0, time.UTC)} + assert.Equal(t, "2021-01-02 03:04 UTC", b.FormattedTimestamp()) +} + +func Test_EnvName(t *testing.T) { + t.Parallel() + + assert.Equal(t, "OXYNOTE_CORE_FOO", EnvName("FOO")) +} + +func Test_Getenv(t *testing.T) { + t.Setenv("OXYNOTE_CORE_TEST_KEY", "test-value") + + assert.Equal(t, "test-value", Getenv("TEST_KEY")) +} + +func Test_Full(t *testing.T) { + // no t.Parallel: Test_IsDevEnv mutates the shared _info variable. + full := Full() + + require.NotZero(t, full) + assert.Equal(t, _info, full) +} + +func Test_IsDevEnv(t *testing.T) { + // the environment name lives in the package-level _info variable, + // so the false branch temporarily mutates it. No t.Parallel to + // keep other tests from observing the mutation. + assert.True(t, IsDevEnv()) + + env := _info.Env + defer func() { _info.Env = env }() + + _info.Env = "prod" + + assert.False(t, IsDevEnv()) +} diff --git a/server/core/internal/db/document_search_job.go b/server/core/internal/db/document_search_job.go index bb91dc3c..e24e0fee 100644 --- a/server/core/internal/db/document_search_job.go +++ b/server/core/internal/db/document_search_job.go @@ -20,12 +20,14 @@ func (a *agent) InsertDocumentSearchJob(ctx context.Context, blockDiff search.Bl return err } -// FetchDocumentSearchJobs retrieves a batch of document search jobs. -func (a *agent) FetchDocumentSearchJobs(ctx context.Context, limit int64) ([]search.DocumentSearchJob, error) { +// FetchDocumentSearchJobs retrieves a batch of document search jobs with IDs +// greater than the given offset ID. +func (a *agent) FetchDocumentSearchJobs(ctx context.Context, offsetID, limit int64) ([]search.DocumentSearchJob, error) { q, args := a.builder.Select( "id", "block_diff", ).From("document_search_jobs"). + Where(sq.Gt{"id": offsetID}). OrderBy("id ASC"). Limit(uint64(limit)). MustSql() diff --git a/server/core/internal/db/document_search_job_test.go b/server/core/internal/db/document_search_job_test.go index 10b1de51..6d78e87e 100644 --- a/server/core/internal/db/document_search_job_test.go +++ b/server/core/internal/db/document_search_job_test.go @@ -137,21 +137,26 @@ func Test_agent_FetchDocumentSearchJobs(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - res, err := db.FetchDocumentSearchJobs(ctx, 10) + res, err := db.FetchDocumentSearchJobs(ctx, 0, 10) require.Error(t, err) assert.Nil(t, res) // success - no jobs - res, err = db.FetchDocumentSearchJobs(context.Background(), 10) + res, err = db.FetchDocumentSearchJobs(context.Background(), 0, 10) require.NoError(t, err) assert.Empty(t, res) // success - limited batch in id order jobs := prepDocumentSearchJobs(t, db, 3, nil) - res, err = db.FetchDocumentSearchJobs(context.Background(), 2) + res, err = db.FetchDocumentSearchJobs(context.Background(), 0, 2) assert.NoError(t, err) assert.Equal(t, jobs[:2], res) + + // success - offset skips earlier jobs + res, err = db.FetchDocumentSearchJobs(context.Background(), jobs[0].ID, 10) + assert.NoError(t, err) + assert.Equal(t, jobs[1:], res) } func Test_agent_DeleteDocumentSearchJob(t *testing.T) { diff --git a/server/core/internal/document/hook/manager/manager.go b/server/core/internal/document/hook/manager/manager.go index a4ffac8a..88f04c9f 100644 --- a/server/core/internal/document/hook/manager/manager.go +++ b/server/core/internal/document/hook/manager/manager.go @@ -86,28 +86,20 @@ func (m *Manager) Start(ctx context.Context) { } } -// documentBranchKey is the cache key for the documents map in ProcessingState. -type documentBranchKey struct { - // ID is the document ID. - ID xid.ID - - // BranchID is the document branch ID. - BranchID null.Value[xid.ID] -} - // ProcessingState holds the state during hook processing. type ProcessingState struct { // OffsetID is the last processed hook ID. OffsetID xid.ID - // Documents caches documents by (ID, branchID) to avoid redundant fetches. - Documents map[documentBranchKey]*document.Document + // Documents caches documents by branch ID to avoid redundant fetches; + // a branch belongs to exactly one document. + Documents map[xid.ID]*document.Document } // processHooks processes document hooks in a paginated manner. func (m *Manager) processHooks(ctx context.Context) error { //nolint:gocognit // this method is complex, however, it's well-structured ps := &ProcessingState{ - Documents: make(map[documentBranchKey]*document.Document), + Documents: make(map[xid.ID]*document.Document), } for { @@ -206,10 +198,7 @@ func (m *Manager) ensureHook( state *ProcessingState, h *hook.Hook, ) bool { - key := documentBranchKey{ - ID: h.DocumentID, - BranchID: h.BranchID, - } + key := h.BranchID.V doc, ok := state.Documents[key] if !ok { diff --git a/server/core/internal/document/hook/manager/manager_test.go b/server/core/internal/document/hook/manager/manager_test.go index 4f878b49..b776f89c 100644 --- a/server/core/internal/document/hook/manager/manager_test.go +++ b/server/core/internal/document/hook/manager/manager_test.go @@ -244,12 +244,10 @@ func Test_Manager_processHooks(t *testing.T) { }, "Documents are fetched once per branch": { Hooks: func(t *testing.T) []hook.Hook { - // hooks on the same branch share a document. - first := stubHook(t, branchID, time.Now().Add(time.Hour), time.Now()) - second := stubHook(t, branchID, time.Now().Add(time.Hour), time.Now()) - second.DocumentID = first.DocumentID - - return []hook.Hook{first, second} + return []hook.Hook{ + stubHook(t, branchID, time.Now().Add(time.Hour), time.Now()), + stubHook(t, branchID, time.Now().Add(time.Hour), time.Now()), + } }, Doc: stubDocument(), Checks: checks( diff --git a/server/core/internal/document/summary.go b/server/core/internal/document/summary.go index 9f5a2e21..c95c3649 100644 --- a/server/core/internal/document/summary.go +++ b/server/core/internal/document/summary.go @@ -14,7 +14,8 @@ import ( // Summaries is a collection of Summary objects. type Summaries []Summary -// Swap swaps the elements in the Summaries slice. +// Swap returns a copy of the Summaries slice with the summary moved to the +// given sort index. The receiver is left untouched. func (ss Summaries) Swap(id xid.ID, sortIndex int) (Summaries, error) { if sortIndex < 0 || sortIndex >= len(ss) { return nil, errutil.New( @@ -31,12 +32,14 @@ func (ss Summaries) Swap(id xid.ID, sortIndex int) (Summaries, error) { return nil, errutil.ErrNotFound } - sliceutil.Move(ss, index, sortIndex) + nss := slices.Clone(ss) + sliceutil.Move(nss, index, sortIndex) - return ss, nil + return nss, nil } -// Remove removes a summary from the Summaries slice by its id. +// Remove returns a copy of the Summaries slice without the summary carrying +// the given id. The receiver is left untouched. func (ss Summaries) Remove(id xid.ID) (Summaries, error) { index := slices.IndexFunc(ss, func(d Summary) bool { return d.ID == id @@ -45,7 +48,7 @@ func (ss Summaries) Remove(id xid.ID) (Summaries, error) { return nil, errutil.ErrNotFound } - return append(ss[:index], ss[index+1:]...), nil + return slices.Delete(slices.Clone(ss), index, index+1), nil } // Summary represents summary of the document. diff --git a/server/core/internal/document/summary_test.go b/server/core/internal/document/summary_test.go index b05a0c9c..3d2adb77 100644 --- a/server/core/internal/document/summary_test.go +++ b/server/core/internal/document/summary_test.go @@ -36,6 +36,9 @@ func Test_Summaries_Swap(t *testing.T) { res[1].DocumentName, res[2].DocumentName, }) + + // the receiver is left untouched. + assert.Equal(t, "a", ss[0].DocumentName) }) t.Run("Out-of-range index fails", func(t *testing.T) { @@ -74,6 +77,11 @@ func Test_Summaries_Remove(t *testing.T) { require.Len(t, res, 2) assert.Equal(t, "a", res[0].DocumentName) assert.Equal(t, "c", res[1].DocumentName) + + // the receiver is left untouched. + require.Len(t, ss, 3) + assert.Equal(t, "b", ss[1].DocumentName) + assert.Equal(t, "c", ss[2].DocumentName) }) t.Run("Unknown ID fails with not found", func(t *testing.T) { diff --git a/server/core/internal/search/manager/0mock_db_test.go b/server/core/internal/search/manager/0mock_db_test.go index 01b2c7ed..f2a08663 100644 --- a/server/core/internal/search/manager/0mock_db_test.go +++ b/server/core/internal/search/manager/0mock_db_test.go @@ -23,7 +23,7 @@ var _ DB = &DBMock{} // DeleteDocumentSearchJobFunc: func(ctx context.Context, id int64) error { // panic("mock out the DeleteDocumentSearchJob method") // }, -// FetchDocumentSearchJobsFunc: func(ctx context.Context, limit int64) ([]search.DocumentSearchJob, error) { +// FetchDocumentSearchJobsFunc: func(ctx context.Context, offsetID int64, limit int64) ([]search.DocumentSearchJob, error) { // panic("mock out the FetchDocumentSearchJobs method") // }, // } @@ -37,7 +37,7 @@ type DBMock struct { DeleteDocumentSearchJobFunc func(ctx context.Context, id int64) error // FetchDocumentSearchJobsFunc mocks the FetchDocumentSearchJobs method. - FetchDocumentSearchJobsFunc func(ctx context.Context, limit int64) ([]search.DocumentSearchJob, error) + FetchDocumentSearchJobsFunc func(ctx context.Context, offsetID int64, limit int64) ([]search.DocumentSearchJob, error) // calls tracks calls to the methods. calls struct { @@ -52,6 +52,8 @@ type DBMock struct { FetchDocumentSearchJobs []struct { // Ctx is the ctx argument value. Ctx context.Context + // OffsetID is the offsetID argument value. + OffsetID int64 // Limit is the limit argument value. Limit int64 } @@ -100,13 +102,15 @@ func (mock *DBMock) DeleteDocumentSearchJobCalls() []struct { } // FetchDocumentSearchJobs calls FetchDocumentSearchJobsFunc. -func (mock *DBMock) FetchDocumentSearchJobs(ctx context.Context, limit int64) ([]search.DocumentSearchJob, error) { +func (mock *DBMock) FetchDocumentSearchJobs(ctx context.Context, offsetID int64, limit int64) ([]search.DocumentSearchJob, error) { callInfo := struct { - Ctx context.Context - Limit int64 + Ctx context.Context + OffsetID int64 + Limit int64 }{ - Ctx: ctx, - Limit: limit, + Ctx: ctx, + OffsetID: offsetID, + Limit: limit, } mock.lockFetchDocumentSearchJobs.Lock() mock.calls.FetchDocumentSearchJobs = append(mock.calls.FetchDocumentSearchJobs, callInfo) @@ -118,7 +122,7 @@ func (mock *DBMock) FetchDocumentSearchJobs(ctx context.Context, limit int64) ([ ) return documentSearchJobsOut, errOut } - return mock.FetchDocumentSearchJobsFunc(ctx, limit) + return mock.FetchDocumentSearchJobsFunc(ctx, offsetID, limit) } // FetchDocumentSearchJobsCalls gets all the calls that were made to FetchDocumentSearchJobs. @@ -126,12 +130,14 @@ func (mock *DBMock) FetchDocumentSearchJobs(ctx context.Context, limit int64) ([ // // len(mockedDB.FetchDocumentSearchJobsCalls()) func (mock *DBMock) FetchDocumentSearchJobsCalls() []struct { - Ctx context.Context - Limit int64 + Ctx context.Context + OffsetID int64 + Limit int64 } { var calls []struct { - Ctx context.Context - Limit int64 + Ctx context.Context + OffsetID int64 + Limit int64 } mock.lockFetchDocumentSearchJobs.RLock() calls = mock.calls.FetchDocumentSearchJobs diff --git a/server/core/internal/search/manager/manager.go b/server/core/internal/search/manager/manager.go index 0256cf70..56931b32 100644 --- a/server/core/internal/search/manager/manager.go +++ b/server/core/internal/search/manager/manager.go @@ -58,21 +58,24 @@ func (m *Manager) Start(ctx context.Context) { } } -// processJobs processes document search jobs in a paginated manner. +// processJobs processes document search jobs in a paginated manner. Failed +// jobs stay in the database and are retried on the next processing interval. func (m *Manager) processJobs(ctx context.Context) error { + var offsetID int64 + for { if err := ctx.Err(); err != nil { return err } - jobs, err := m.db.FetchDocumentSearchJobs(ctx, _processingBatch) + jobs, err := m.db.FetchDocumentSearchJobs(ctx, offsetID, _processingBatch) if err != nil { return fmt.Errorf("fetching paginated document search jobs: %w", err) } - var deleted int - for _, job := range jobs { + offsetID = job.ID + err := m.searchGateway.ReplaceDocumentBlocks(ctx, job.BlockDiff) if err != nil { m.log.With("job_id", job.ID). @@ -87,17 +90,10 @@ func (m *Manager) processJobs(ctx context.Context) error { m.log.With("job_id", job.ID). With("error", err). Error("deleting document search job") - - continue } - - deleted++ } - // the fetch has no offset, so without progress the next pass - // would see the same failing jobs again — leave them for the - // next processing interval instead of spinning on them. - if len(jobs) < _processingBatch || deleted == 0 { + if len(jobs) < _processingBatch { break } } @@ -110,8 +106,9 @@ func (m *Manager) processJobs(ctx context.Context) error { //go:generate ../../../scripts/codegen/mock -t internal DB db type DB interface { // FetchDocumentSearchJobs should retrieve a batch of document search - // jobs, limited to the specified number of jobs. - FetchDocumentSearchJobs(ctx context.Context, limit int64) ([]search.DocumentSearchJob, error) + // jobs with IDs greater than the given offset ID, limited to the + // specified number of jobs. + FetchDocumentSearchJobs(ctx context.Context, offsetID, limit int64) ([]search.DocumentSearchJob, error) // DeleteDocumentSearchJob should delete the given document search job from the database. DeleteDocumentSearchJob(ctx context.Context, id int64) error diff --git a/server/core/internal/search/manager/manager_test.go b/server/core/internal/search/manager/manager_test.go index 657dbcc5..074e5e19 100644 --- a/server/core/internal/search/manager/manager_test.go +++ b/server/core/internal/search/manager/manager_test.go @@ -10,12 +10,12 @@ import ( "github.com/stretchr/testify/require" ) -// jobs builds n sequential search jobs. -func jobs(n int) []search.DocumentSearchJob { +// jobs builds n sequential search jobs with IDs starting at start. +func jobs(start, n int) []search.DocumentSearchJob { jj := make([]search.DocumentSearchJob, 0, n) for i := range n { - jj = append(jj, search.DocumentSearchJob{ID: int64(i + 1)}) + jj = append(jj, search.DocumentSearchJob{ID: int64(start + i)}) } return jj @@ -73,23 +73,29 @@ func Test_Manager_processJobs(t *testing.T) { ), }, "Jobs are processed and deleted": { - Batches: [][]search.DocumentSearchJob{jobs(3)}, + Batches: [][]search.DocumentSearchJob{jobs(1, 3)}, Checks: checks( hasError(false), wasReplaceCalled(3), wasDeleteCalled(3), ), }, - "Full batches keep paginating": { - Batches: [][]search.DocumentSearchJob{jobs(_processingBatch), jobs(2)}, + "Full batches keep paginating past the last job ID": { + Batches: [][]search.DocumentSearchJob{jobs(1, _processingBatch), jobs(101, 2)}, Checks: checks( hasError(false), wasReplaceCalled(_processingBatch+2), wasDeleteCalled(_processingBatch+2), + func(t *testing.T, db *DBMock, _ *SearchGatewayMock, _ error) { + ff := db.FetchDocumentSearchJobsCalls() + require.Len(t, ff, 2) + assert.Equal(t, int64(0), ff[0].OffsetID) + assert.Equal(t, int64(_processingBatch), ff[1].OffsetID) + }, ), }, - "Replace failure keeps the job for the next pass": { - Batches: [][]search.DocumentSearchJob{jobs(2)}, + "Replace failure keeps the job for the next interval": { + Batches: [][]search.DocumentSearchJob{jobs(1, 2)}, ReplaceErr: assert.AnError, Checks: checks( hasError(false), @@ -97,19 +103,26 @@ func Test_Manager_processJobs(t *testing.T) { wasDeleteCalled(0), ), }, - "Persistently failing full batch terminates": { - // without the no-progress guard this would refetch the - // same failing batch forever. - Batches: [][]search.DocumentSearchJob{jobs(_processingBatch), jobs(_processingBatch)}, + "Persistently failing full batches advance and terminate": { + // the offset moves past failing jobs, so a wall of + // failures cannot refetch the same batch forever. + Batches: [][]search.DocumentSearchJob{ + jobs(1, _processingBatch), + jobs(101, _processingBatch), + nil, + }, ReplaceErr: assert.AnError, Checks: checks( hasError(false), - wasReplaceCalled(_processingBatch), + wasReplaceCalled(2*_processingBatch), wasDeleteCalled(0), ), }, - "Delete failure does not count as progress": { - Batches: [][]search.DocumentSearchJob{jobs(_processingBatch), jobs(_processingBatch)}, + "Delete failure still advances the offset": { + Batches: [][]search.DocumentSearchJob{ + jobs(1, _processingBatch), + nil, + }, DeleteErr: assert.AnError, Checks: checks( hasError(false), @@ -126,7 +139,7 @@ func Test_Manager_processJobs(t *testing.T) { fetches := 0 db := &DBMock{ - FetchDocumentSearchJobsFunc: func(context.Context, int64) ([]search.DocumentSearchJob, error) { + FetchDocumentSearchJobsFunc: func(context.Context, int64, int64) ([]search.DocumentSearchJob, error) { if fetches >= len(tc.Batches) { if len(tc.Batches) == 0 { return nil, assert.AnError diff --git a/server/core/internal/storage/client_test.go b/server/core/internal/storage/client_test.go new file mode 100644 index 00000000..9b60947e --- /dev/null +++ b/server/core/internal/storage/client_test.go @@ -0,0 +1,436 @@ +package storage + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync" + "testing" + + "github.com/oxynote/oxynote/server/core/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + +func Test_NewClient(t *testing.T) { + cc := map[string]struct { + Fake *fakeS3 + URL string + Err error + }{ + "Error returned by setupClient": { + URL: "://invalid", + Err: assert.AnError, + }, + "Successful creation": { + Fake: &fakeS3{bucketExists: true}, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + opts := Options{ + Bucket: "test-bucket", + URL: c.URL, + AccessKey: "access-key", + SecretKey: "secret-key", + } + + if c.Fake != nil { + opts.URL = newFakeS3Server(t, c.Fake) + } + + client, err := NewClient(context.Background(), opts) + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + return + } + + require.NotNil(t, client) + assert.NotNil(t, client.client) + assert.Equal(t, "test-bucket", client.bucket) + }) + } +} + +func Test_setupClient(t *testing.T) { + cc := map[string]struct { + Fake *fakeS3 + URL string + Created bool + Err error + }{ + "Invalid URL": { + URL: "://invalid", + Err: assert.AnError, + }, + "Error returned by minio.New": { + URL: "http://", + Err: assert.AnError, + }, + "Error returned by BucketExists": { + Fake: &fakeS3{bucketExists: true, failBucket: true}, + Err: assert.AnError, + }, + "Error returned by MakeBucket": { + Fake: &fakeS3{failMakeBucket: true}, + Err: assert.AnError, + }, + "Bucket created when missing": { + Fake: &fakeS3{}, + Created: true, + }, + "Existing bucket": { + Fake: &fakeS3{bucketExists: true}, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + opts := Options{ + Bucket: "test-bucket", + URL: c.URL, + AccessKey: "access-key", + SecretKey: "secret-key", + } + + if c.Fake != nil { + opts.URL = newFakeS3Server(t, c.Fake) + } + + mc, err := setupClient(context.Background(), opts) + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + return + } + + require.NotNil(t, mc) + + if c.Created { + assert.True(t, c.Fake.bucketExists) + } + }) + } +} + +// prepClient creates a storage client backed by the provided fake S3 +// server, marking the fake's bucket as existing so that setup succeeds. +func prepClient(t *testing.T, f *fakeS3) *Client { + t.Helper() + + f.bucketExists = true + + client, err := NewClient(context.Background(), Options{ + Bucket: "test-bucket", + URL: newFakeS3Server(t, f), + AccessKey: "access-key", + SecretKey: "secret-key", + }) + require.NoError(t, err) + + return client +} + +// newFakeS3Server starts an HTTP test server backed by the provided +// fake S3 implementation and returns its URL. +func newFakeS3Server(t *testing.T, f *fakeS3) string { + t.Helper() + + srv := httptest.NewServer(f) + t.Cleanup(srv.Close) + + return srv.URL +} + +// fakeS3 is a minimal in-memory S3 implementation covering only the +// requests the minio client performs for the storage package: bucket +// location lookup, bucket creation, multipart upload, object stat, +// object retrieval and object removal. +type fakeS3 struct { + mu sync.Mutex + + // bucketExists indicates whether the bucket location lookup + // should report the bucket as present. + bucketExists bool + + // objects maps completed object keys to their data. + objects map[string][]byte + + // contentTypes maps completed object keys to their content type. + contentTypes map[string]string + + // parts accumulates uploaded multipart data per object key. + parts map[string][]byte + + // failBucket forces the bucket existence probe to fail. + failBucket bool + + // failMakeBucket forces bucket creation to fail. + failMakeBucket bool + + // failUpload forces multipart upload initiation to fail. + failUpload bool + + // failStat forces object stat requests to fail. + failStat bool + + // failDelete forces object removal to fail. + failDelete bool +} + +// ServeHTTP dispatches the S3 REST requests issued by the minio client. +func (f *fakeS3) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + + if f.objects == nil { + f.objects = make(map[string][]byte) + f.contentTypes = make(map[string]string) + f.parts = make(map[string][]byte) + } + + segments := strings.SplitN(strings.TrimPrefix(r.URL.Path, "/"), "/", 2) + + var key string + + if len(segments) > 1 { + key = segments[1] + } + + query := r.URL.Query() + + switch { + case key == "" && r.Method == http.MethodGet && query.Has("location"): + f.handleLocation(w) + case key == "" && r.Method == http.MethodHead: + f.handleHeadBucket(w) + case key == "" && r.Method == http.MethodPut: + f.handleMakeBucket(w) + case r.Method == http.MethodPost && query.Has("uploads"): + f.handleInitiateUpload(w, r, key) + case r.Method == http.MethodPut && query.Get("uploadId") != "": + f.handleUploadPart(w, r, key) + case r.Method == http.MethodPost && query.Get("uploadId") != "": + f.handleCompleteUpload(w, key) + case r.Method == http.MethodHead: + f.handleStat(w, key) + case r.Method == http.MethodGet: + f.handleGet(w, key) + case r.Method == http.MethodDelete: + f.handleDelete(w, key) + default: + w.WriteHeader(http.StatusNotImplemented) + } +} + +// handleLocation serves the bucket location lookup that backs the +// minio client's BucketExists call. +func (f *fakeS3) handleLocation(w http.ResponseWriter) { + if !f.bucketExists { + writeS3Error(w, http.StatusNotFound, "NoSuchBucket") + + return + } + + fmt.Fprint(w, ``) //nolint:errcheck // test server response errors are irrelevant +} + +// handleHeadBucket serves the bucket existence probe. +func (f *fakeS3) handleHeadBucket(w http.ResponseWriter) { + if f.failBucket { + w.WriteHeader(http.StatusForbidden) + + return + } + + if !f.bucketExists { + w.WriteHeader(http.StatusNotFound) + + return + } + + w.WriteHeader(http.StatusOK) +} + +// handleMakeBucket serves bucket creation. +func (f *fakeS3) handleMakeBucket(w http.ResponseWriter) { + if f.failMakeBucket { + writeS3Error(w, http.StatusForbidden, "AccessDenied") + + return + } + + f.bucketExists = true + + w.WriteHeader(http.StatusOK) +} + +// handleInitiateUpload serves multipart upload initiation, recording +// the object's content type provided by the client. +func (f *fakeS3) handleInitiateUpload(w http.ResponseWriter, r *http.Request, key string) { + if f.failUpload { + writeS3Error(w, http.StatusForbidden, "AccessDenied") + + return + } + + f.contentTypes[key] = r.Header.Get("Content-Type") + + fmt.Fprintf( //nolint:errcheck,gosec // test server response errors and input taint are irrelevant + w, + `%stest-upload`, + key, + ) +} + +// handleUploadPart accumulates a single multipart upload part. +func (f *fakeS3) handleUploadPart(w http.ResponseWriter, r *http.Request, key string) { + f.parts[key] = append(f.parts[key], readS3Body(r)...) + + w.Header().Set("ETag", `"part-etag"`) + w.WriteHeader(http.StatusOK) +} + +// handleCompleteUpload finalizes a multipart upload by promoting the +// accumulated parts to a stored object. +func (f *fakeS3) handleCompleteUpload(w http.ResponseWriter, key string) { + f.objects[key] = f.parts[key] + + delete(f.parts, key) + + fmt.Fprintf( //nolint:errcheck,gosec // test server response errors and input taint are irrelevant + w, + `test-bucket%s"test-etag"`, + key, + ) +} + +// handleStat serves object metadata requests. +func (f *fakeS3) handleStat(w http.ResponseWriter, key string) { + if f.failStat { + w.WriteHeader(http.StatusForbidden) + + return + } + + data, ok := f.objects[key] + if !ok { + w.WriteHeader(http.StatusNotFound) + + return + } + + writeS3ObjectHeaders(w, data, f.contentTypes[key]) + w.WriteHeader(http.StatusOK) +} + +// handleGet serves object data requests. +func (f *fakeS3) handleGet(w http.ResponseWriter, key string) { + data, ok := f.objects[key] + if !ok { + writeS3Error(w, http.StatusNotFound, "NoSuchKey") + + return + } + + writeS3ObjectHeaders(w, data, f.contentTypes[key]) + w.Write(data) //nolint:errcheck,gosec // test server response errors are irrelevant +} + +// handleDelete serves object removal requests. +func (f *fakeS3) handleDelete(w http.ResponseWriter, key string) { + if f.failDelete { + writeS3Error(w, http.StatusForbidden, "AccessDenied") + + return + } + + delete(f.objects, key) + + w.WriteHeader(http.StatusNoContent) +} + +// writeS3ObjectHeaders writes the object metadata headers the minio +// client requires to build stat results. +func writeS3ObjectHeaders(w http.ResponseWriter, data []byte, contentType string) { + w.Header().Set("ETag", `"test-etag"`) + w.Header().Set("Content-Type", contentType) + w.Header().Set("Content-Length", strconv.Itoa(len(data))) + w.Header().Set("Last-Modified", "Mon, 02 Jan 2006 15:04:05 GMT") +} + +// writeS3Error writes an S3-style XML error response. +func writeS3Error(w http.ResponseWriter, status int, code string) { + w.WriteHeader(status) + fmt.Fprintf( //nolint:errcheck // test server response errors are irrelevant + w, + `%s%s`, + code, + code, + ) +} + +// readS3Body reads a request body, decoding the aws-chunked streaming +// signature framing the minio client uses for unknown-size uploads. +// Each frame is a ";chunk-signature=" header line +// followed by the payload, both CRLF-terminated; a zero-size frame +// ends the stream. +func readS3Body(r *http.Request) []byte { + raw, err := io.ReadAll(r.Body) + if err != nil { + return nil + } + + if !strings.HasPrefix(r.Header.Get("X-Amz-Content-Sha256"), "STREAMING-") { + return raw + } + + var out []byte + + for { + idx := bytes.Index(raw, []byte("\r\n")) + if idx < 0 { + return out + } + + header := string(raw[:idx]) + raw = raw[idx+2:] + + if semi := strings.Index(header, ";"); semi >= 0 { + header = header[:semi] + } + + length, err := parseHexLength(header) + if err != nil || length == 0 { + return out + } + + out = append(out, raw[:length]...) + raw = raw[length+2:] + } +} + +// parseHexLength parses a hexadecimal chunk length. +func parseHexLength(val string) (int, error) { + var length int + + _, err := fmt.Sscanf(val, "%x", &length) + + return length, err +} diff --git a/server/core/internal/storage/object_test.go b/server/core/internal/storage/object_test.go new file mode 100644 index 00000000..82f33aa1 --- /dev/null +++ b/server/core/internal/storage/object_test.go @@ -0,0 +1,262 @@ +package storage + +import ( + "bytes" + "context" + "io" + "strings" + "testing" + + "github.com/oxynote/oxynote/server/core/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// _testPNG is a data prefix carrying the PNG magic bytes so content +// type sniffing detects image/png. +var _testPNG = append([]byte("\x89PNG\r\n\x1a\n"), bytes.Repeat([]byte{1}, 1024)...) + +// _testJPEG is a data prefix carrying the JPEG magic bytes so content +// type sniffing detects image/jpeg. +var _testJPEG = append([]byte("\xff\xd8\xff"), bytes.Repeat([]byte{2}, 1024)...) + +// _testWebP is a data prefix carrying the WebP magic bytes so content +// type sniffing detects image/webp. +var _testWebP = append([]byte("RIFF\x00\x00\x00\x00WEBPVP8 "), bytes.Repeat([]byte{3}, 1024)...) + +func Test_Client_Upload(t *testing.T) { + cc := map[string]struct { + Fake *fakeS3 + Reader io.Reader + Data []byte + ContentType string + Err error + }{ + "Error returned by reader": { + Fake: &fakeS3{}, + Reader: &errReader{}, + Err: assert.AnError, + }, + "Invalid content type": { + Fake: &fakeS3{}, + Reader: strings.NewReader("plain text data"), + Err: ErrInvalidContentType, + }, + "Size limit exceeded": { + Fake: &fakeS3{}, + Reader: io.MultiReader( + bytes.NewReader(_testPNG), + bytes.NewReader(bytes.Repeat([]byte{4}, _maxObjectSize)), + ), + Err: assert.AnError, + }, + "Error returned by PutObject": { + Fake: &fakeS3{failUpload: true}, + Reader: bytes.NewReader(_testPNG), + Err: assert.AnError, + }, + "Successful PNG upload": { + Fake: &fakeS3{}, + Reader: bytes.NewReader(_testPNG), + Data: _testPNG, + ContentType: "image/png", + }, + "Successful JPEG upload": { + Fake: &fakeS3{}, + Reader: bytes.NewReader(_testJPEG), + Data: _testJPEG, + ContentType: "image/jpeg", + }, + "Successful WebP upload": { + Fake: &fakeS3{}, + Reader: bytes.NewReader(_testWebP), + Data: _testWebP, + ContentType: "image/webp", + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + client := prepClient(t, c.Fake) + + err := client.Upload(context.Background(), "folder", "object-id", c.Reader) + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + return + } + + assert.Equal(t, c.Data, c.Fake.objects["folder/object-id"]) + assert.Equal(t, c.ContentType, c.Fake.contentTypes["folder/object-id"]) + }) + } +} + +func Test_Client_Retrieve(t *testing.T) { + cc := map[string]struct { + Fake *fakeS3 + Bucket string + Found bool + Err error + }{ + "Error returned by GetObject": { + Fake: &fakeS3{}, + Bucket: "..", + Err: assert.AnError, + }, + "Error returned by Stat": { + Fake: &fakeS3{failStat: true}, + Err: assert.AnError, + }, + "Object not found": { + Fake: &fakeS3{}, + }, + "Successful retrieval": { + Fake: &fakeS3{ + objects: map[string][]byte{"folder/object-id": _testPNG}, + contentTypes: map[string]string{"folder/object-id": "image/png"}, + }, + Found: true, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + client := prepClient(t, c.Fake) + + if c.Bucket != "" { + client.bucket = c.Bucket + } + + info, found, err := client.Retrieve(context.Background(), "folder", "object-id") + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + return + } + + assert.Equal(t, c.Found, found) + + if !c.Found { + assert.Nil(t, info) + + return + } + + require.NotNil(t, info) + + body, err := io.ReadAll(info.Body) + require.NoError(t, err) + require.NoError(t, info.Body.Close()) + + assert.Equal(t, _testPNG, body) + assert.Equal(t, "test-etag", info.ETag) + assert.Equal(t, "image/png", info.ContentType) + }) + } +} + +func Test_Client_Delete(t *testing.T) { + cc := map[string]struct { + Fake *fakeS3 + Err error + }{ + "Error returned by RemoveObject": { + Fake: &fakeS3{ + objects: map[string][]byte{"folder/object-id": _testPNG}, + failDelete: true, + }, + Err: assert.AnError, + }, + "Successful deletion": { + Fake: &fakeS3{ + objects: map[string][]byte{"folder/object-id": _testPNG}, + }, + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + client := prepClient(t, c.Fake) + + err := client.Delete(context.Background(), "folder", "object-id") + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + return + } + + assert.NotContains(t, c.Fake.objects, "folder/object-id") + }) + } +} + +func Test_newLimitedReader(t *testing.T) { + t.Parallel() + + r := strings.NewReader("data") + lr := newLimitedReader(r, 10) + + require.NotNil(t, lr) + assert.Equal(t, r, lr.r) + assert.EqualValues(t, 10, lr.limit) + assert.Zero(t, lr.read) +} + +func Test_limitedReader_Read(t *testing.T) { + cc := map[string]struct { + Reader io.Reader + Limit int64 + Result []byte + Err error + }{ + "Error returned by reader": { + Reader: &errReader{}, + Limit: 10, + Err: assert.AnError, + }, + "Limit exceeded": { + Reader: strings.NewReader("oversized data"), + Limit: 4, + Err: ErrSizeLimitExceeded, + }, + "Read within limit": { + Reader: strings.NewReader("data"), + Limit: 10, + Result: []byte("data"), + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + lr := newLimitedReader(c.Reader, c.Limit) + + buf := make([]byte, 64) + + n, err := lr.Read(buf) + testutil.AssertEqualError(t, c.Err, err) + + if err != nil { + return + } + + assert.Equal(t, c.Result, buf[:n]) + }) + } +} + +// errReader is a reader that always fails. +type errReader struct{} + +// Read returns a read failure. +func (er *errReader) Read(_ []byte) (int, error) { + return 0, assert.AnError +} diff --git a/server/core/pkg/cryptoutil/cryptoutil.go b/server/core/pkg/cryptoutil/cryptoutil.go index bd1a5b2a..b8aa7d3f 100644 --- a/server/core/pkg/cryptoutil/cryptoutil.go +++ b/server/core/pkg/cryptoutil/cryptoutil.go @@ -26,6 +26,7 @@ func EncryptText(text string, key []byte) (string, error) { // https://en.wikipedia.org/wiki/Galois/Counter_Mode. gcm, err := cipher.NewGCM(c) if err != nil { + // NOCOV: cipher.NewGCM cannot fail with a valid AES block. return "", err } @@ -36,6 +37,7 @@ func EncryptText(text string, key []byte) (string, error) { // Populates our nonce with a cryptographically secure // random sequence. if _, err = io.ReadFull(rand.Reader, nonce); err != nil { + // NOCOV: crypto/rand failures cannot be simulated in tests. return "", err } @@ -71,6 +73,7 @@ func DecryptText(encryptedText string, key []byte) (string, error) { gcm, err := cipher.NewGCM(c) if err != nil { + // NOCOV: cipher.NewGCM cannot fail with a valid AES block. return "", err } diff --git a/server/core/pkg/httpserver/handler_test.go b/server/core/pkg/httpserver/handler_test.go index 78bee405..18dd1a1f 100644 --- a/server/core/pkg/httpserver/handler_test.go +++ b/server/core/pkg/httpserver/handler_test.go @@ -248,6 +248,33 @@ func Test_DecodeForm(t *testing.T) { req.URL.RawQuery = q.Encode() assert.NoError(t, DecodeForm(req, &v2)) assert.Equal(t, []xid.ID{id}, v2.IDs) + + v3 := struct { + ID xid.ID + Time time.Time + }{} + tm := time.Date(2021, 1, 2, 3, 4, 5, 0, time.UTC) + + req = httptest.NewRequest("GET", "http://test.com/", http.NoBody) + q = req.URL.Query() + q.Add("id", "invalid") + req.URL.RawQuery = q.Encode() + assert.Equal(t, ErrInvalidForm, DecodeForm(req, &v3)) + + req = httptest.NewRequest("GET", "http://test.com/", http.NoBody) + q = req.URL.Query() + q.Add("time", "invalid") + req.URL.RawQuery = q.Encode() + assert.Equal(t, ErrInvalidForm, DecodeForm(req, &v3)) + + req = httptest.NewRequest("GET", "http://test.com/", http.NoBody) + q = req.URL.Query() + q.Add("id", id.String()) + q.Add("time", tm.Format(time.RFC3339Nano)) + req.URL.RawQuery = q.Encode() + assert.NoError(t, DecodeForm(req, &v3)) + assert.Equal(t, id, v3.ID) + assert.Equal(t, tm, v3.Time) } func Test_NotFound(t *testing.T) { @@ -282,6 +309,32 @@ func Test_Recoverer(t *testing.T) { assert.Contains(t, rec.Body.String(), strings.ToLower(http.StatusText(http.StatusInternalServerError))) } +func Test_ExtractNamedID(t *testing.T) { + req := httptest.NewRequest("GET", "http://test.com/", http.NoBody) + id, err := ExtractNamedID(req, "id") + assert.Zero(t, id) + assert.Equal(t, errutil.ErrNotFound, err) + + ctx := chi.NewRouteContext() + ctx.URLParams.Add("id", "invalid") + req = req.WithContext(context.WithValue(context.Background(), + chi.RouteCtxKey, ctx)) + + id, err = ExtractNamedID(req, "id") + assert.Zero(t, id) + assert.Equal(t, errutil.ErrNotFound, err) + + expID := xid.New() + ctx = chi.NewRouteContext() + ctx.URLParams.Add("id", expID.String()) + req = req.WithContext(context.WithValue(context.Background(), + chi.RouteCtxKey, ctx)) + + id, err = ExtractNamedID(req, "id") + assert.Equal(t, expID, id) + assert.NoError(t, err) +} + func Test_ExtractParam(t *testing.T) { req := httptest.NewRequest("GET", "http://test.com/", http.NoBody) val, err := ExtractParam(req, "key") diff --git a/server/core/pkg/logutil/logutil_test.go b/server/core/pkg/logutil/logutil_test.go index 35f1764b..37757cad 100644 --- a/server/core/pkg/logutil/logutil_test.go +++ b/server/core/pkg/logutil/logutil_test.go @@ -203,6 +203,31 @@ func Test_Recover(t *testing.T) { ShowCritical = false } +func Test_processRecoveryValue(t *testing.T) { + // the nil plan branch inside the returned closure is reachable + // only through a direct call, since Recover replaces a nil plan + // before invoking this helper. + out, b := testutil.NewBuffer() + log := slog.New(slog.NewJSONHandler(out, &slog.HandlerOptions{ + ReplaceAttr: func(_ []string, a slog.Attr) slog.Attr { + if a.Key == "time" { + return slog.Attr{} + } + + return a + }, + })) + + processRecoveryValue(log, nil)("error") + + require.NoError(t, out.Flush()) + assert.JSONEq(t, `{ + "level":"ERROR", + "error":"internal error", + "msg":"recovered from panic" + }`, b.String()) +} + func Test_NewRecoveryPlan(t *testing.T) { p := NewRecoveryPlan("hello") require.NotNil(t, p) diff --git a/server/core/pkg/metricutil/metricutil_test.go b/server/core/pkg/metricutil/metricutil_test.go index f127e4fa..115c5118 100644 --- a/server/core/pkg/metricutil/metricutil_test.go +++ b/server/core/pkg/metricutil/metricutil_test.go @@ -5,8 +5,13 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" + "go.uber.org/goleak" ) +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + func Test_New(t *testing.T) { reg := prometheus.NewRegistry() f := NewFactory("test", reg) diff --git a/server/core/pkg/metricutil/runtime.go b/server/core/pkg/metricutil/runtime.go index 0bf7cf8a..afd05e4d 100644 --- a/server/core/pkg/metricutil/runtime.go +++ b/server/core/pkg/metricutil/runtime.go @@ -236,26 +236,7 @@ func (f *factory) CollectRuntimeMetrics(ctx context.Context, dur time.Duration) metrics.memory.NextGC.Set(float64(memStats.NextGC)) metrics.memory.NumGC.Set(float64(memStats.NumGC - numGC)) - i := numGC % uint32(len(memStats.PauseNs)) - ii := memStats.NumGC % uint32(len(memStats.PauseNs)) - - if memStats.NumGC-numGC >= uint32(len(memStats.PauseNs)) { - for i = range uint32(len(memStats.PauseNs)) { - metrics.memory.Pause.Observe(float64(memStats.PauseNs[i] * _nanosecondsPerSecond)) - } - } else { - if i > ii { - for ; i < uint32(len(memStats.PauseNs)); i++ { - metrics.memory.Pause.Observe(float64(memStats.PauseNs[i] * _nanosecondsPerSecond)) - } - - i = 0 - } - - for ; i < ii; i++ { - metrics.memory.Pause.Observe(float64(memStats.PauseNs[i] * _nanosecondsPerSecond)) - } - } + observeGCPauses(metrics.memory.Pause, &memStats, numGC) frees = memStats.Frees lookups = memStats.Lookups @@ -271,3 +252,31 @@ func (f *factory) CollectRuntimeMetrics(ctx context.Context, dur time.Duration) } } } + +// observeGCPauses observes the garbage collection pause durations +// recorded since the previous collection cycle, unwinding the +// runtime's circular pause buffer. +func observeGCPauses(pause Observer, memStats *runtime.MemStats, prevNumGC uint32) { + i := prevNumGC % uint32(len(memStats.PauseNs)) + ii := memStats.NumGC % uint32(len(memStats.PauseNs)) + + if memStats.NumGC-prevNumGC >= uint32(len(memStats.PauseNs)) { + for i = range uint32(len(memStats.PauseNs)) { + pause.Observe(float64(memStats.PauseNs[i] * _nanosecondsPerSecond)) + } + + return + } + + if i > ii { + for ; i < uint32(len(memStats.PauseNs)); i++ { + pause.Observe(float64(memStats.PauseNs[i] * _nanosecondsPerSecond)) + } + + i = 0 + } + + for ; i < ii; i++ { + pause.Observe(float64(memStats.PauseNs[i] * _nanosecondsPerSecond)) + } +} diff --git a/server/core/pkg/metricutil/runtime_test.go b/server/core/pkg/metricutil/runtime_test.go index 1f8406fb..200de72f 100644 --- a/server/core/pkg/metricutil/runtime_test.go +++ b/server/core/pkg/metricutil/runtime_test.go @@ -2,6 +2,7 @@ package metricutil import ( "context" + "runtime" "testing" "time" @@ -46,3 +47,76 @@ func Test_CollectRuntimeMetrics(t *testing.T) { f.CollectRuntimeMetrics(ctx, time.Millisecond*50) }) } + +func Test_observeGCPauses(t *testing.T) { + // the runtime records pauses in a 256-entry circular buffer, so + // the case data is expressed through buffer indexes. + buffer := func(vals map[int]uint64) [256]uint64 { + var pp [256]uint64 + + for i, v := range vals { + pp[i] = v + } + + return pp + } + + cc := map[string]struct { + PauseNs [256]uint64 + NumGC uint32 + PrevNumGC uint32 + Result []float64 + }{ + "No new garbage collections": { + PauseNs: buffer(map[int]uint64{3: 1}), + NumGC: 3, + PrevNumGC: 3, + }, + "Pauses within a single window": { + PauseNs: buffer(map[int]uint64{2: 1, 3: 2, 4: 3}), + NumGC: 5, + PrevNumGC: 2, + Result: []float64{1e9, 2e9, 3e9}, + }, + "Pauses wrapping the buffer": { + PauseNs: buffer(map[int]uint64{254: 1, 255: 2, 0: 3, 1: 4}), + NumGC: 258, + PrevNumGC: 254, + Result: []float64{1e9, 2e9, 3e9, 4e9}, + }, + "Full buffer of pauses": { + PauseNs: buffer(map[int]uint64{0: 1}), + NumGC: 300, + PrevNumGC: 0, + Result: append( + []float64{1e9}, + make([]float64, 255)..., + ), + }, + } + + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() + + ob := &observerStub{} + memStats := &runtime.MemStats{ + NumGC: c.NumGC, + PauseNs: c.PauseNs, + } + + observeGCPauses(ob, memStats, c.PrevNumGC) + assert.Equal(t, c.Result, ob.values) + }) + } +} + +// observerStub records observed values for assertions. +type observerStub struct { + values []float64 +} + +// Observe records the observed value. +func (ob *observerStub) Observe(value float64) { + ob.values = append(ob.values, value) +} diff --git a/server/core/pkg/metricutil/util_test.go b/server/core/pkg/metricutil/util_test.go new file mode 100644 index 00000000..4b93846a --- /dev/null +++ b/server/core/pkg/metricutil/util_test.go @@ -0,0 +1,15 @@ +package metricutil + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_histogramBuckets(t *testing.T) { + // custom buckets take precedence + assert.Equal(t, []float64{1, 2}, histogramBuckets([]float64{1, 2})) + + // package default is used when no custom buckets are given + assert.Equal(t, _defaultHistogramBuckets, histogramBuckets(nil)) +} diff --git a/server/core/pkg/redkit/value_store_test.go b/server/core/pkg/redkit/value_store_test.go index b95ad0aa..351c2daa 100644 --- a/server/core/pkg/redkit/value_store_test.go +++ b/server/core/pkg/redkit/value_store_test.go @@ -249,23 +249,88 @@ func Test_ValueStore_Get(t *testing.T) { } func Test_ValueStore_Delete(t *testing.T) { - conn := redigomock.NewConn() - conn.Command("DEL", "test").Expect(int64(1)) + cc := map[string]struct { + Cancelled bool + Conn func() (*redigomock.Conn, func(*testing.T)) + Err error + }{ + "Cancelled context": { + Cancelled: true, + Conn: func() (*redigomock.Conn, func(*testing.T)) { + conn := redigomock.NewConn() - vs := &ValueStore[any]{ - pool: &redis.Pool{ - Dial: func() (redis.Conn, error) { - return conn, nil + return conn, func(t *testing.T) { + err := conn.ExpectationsWereMet() + assert.NoError(t, err) + } + }, + Err: assert.AnError, + }, + "DEL returns an error": { + Conn: func() (*redigomock.Conn, func(*testing.T)) { + conn := redigomock.NewConn() + conn.Command("DEL", "test").ExpectError(assert.AnError) + + return conn, func(t *testing.T) { + err := conn.ExpectationsWereMet() + assert.NoError(t, err) + } + }, + Err: assert.AnError, + }, + "Successfully deleted element": { + Conn: func() (*redigomock.Conn, func(*testing.T)) { + conn := redigomock.NewConn() + conn.Command("DEL", "test").Expect(int64(1)) + + return conn, func(t *testing.T) { + err := conn.ExpectationsWereMet() + assert.NoError(t, err) + } }, - Wait: true, - MaxActive: 10, }, - expireAfter: time.Hour * 168, } - err := vs.Delete(context.Background(), "test") - require.NoError(t, err) + for cn, c := range cc { + t.Run(cn, func(t *testing.T) { + t.Parallel() - err = conn.ExpectationsWereMet() - require.NoError(t, err) + conn, check := c.Conn() + + vs := &ValueStore[any]{ + pool: &redis.Pool{ + Dial: func() (redis.Conn, error) { + return conn, nil + }, + Wait: true, + MaxActive: 10, + }, + expireAfter: time.Hour * 168, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + if c.Cancelled { + cancel() + } + + err := vs.Delete(ctx, "test") + + check(t) + + if c.Err != nil { + if errors.Is(c.Err, assert.AnError) { + assert.Error(t, err) + return + } + + assert.Equal(t, c.Err, err) + + return + } + + assert.NoError(t, err) + }) + } } diff --git a/server/core/pkg/strutil/nanoid.go b/server/core/pkg/strutil/nanoid.go index 5fdba875..2ea58a48 100644 --- a/server/core/pkg/strutil/nanoid.go +++ b/server/core/pkg/strutil/nanoid.go @@ -16,6 +16,7 @@ func NanoID() string { _, err := rand.Read(bytes) if err != nil { + // NOCOV: crypto/rand failures cannot be simulated in tests. panic(err) } diff --git a/server/core/pkg/testutil/testutil_test.go b/server/core/pkg/testutil/testutil_test.go index f366a2b5..affb708c 100644 --- a/server/core/pkg/testutil/testutil_test.go +++ b/server/core/pkg/testutil/testutil_test.go @@ -1,10 +1,14 @@ package testutil import ( + "context" + "errors" "testing" "time" + "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/goleak" ) @@ -12,6 +16,95 @@ func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } +func Test_AddChiCtx(t *testing.T) { + // fresh context - a chi route context is created + ctx := AddChiCtx(context.Background(), "key", "value") + + rctx, ok := ctx.Value(chi.RouteCtxKey).(*chi.Context) + require.True(t, ok) + assert.Equal(t, "value", rctx.URLParam("key")) + + // existing context - the chi route context is reused + ctx2 := AddChiCtx(ctx, "key2", "value2") + + assert.Equal(t, ctx, ctx2) + assert.Equal(t, "value", rctx.URLParam("key")) + assert.Equal(t, "value2", rctx.URLParam("key2")) +} + +func Test_AssertEqualError(t *testing.T) { + exp := errors.New("expected error") + + // success - no error expected and none occurred + nt := &testing.T{} + AssertEqualError(nt, nil, nil) + assert.False(t, nt.Failed()) + + // error - no error expected but one occurred + nt = &testing.T{} + AssertEqualError(nt, nil, assert.AnError) + assert.True(t, nt.Failed()) + + // success - generic error expected and one occurred + nt = &testing.T{} + AssertEqualError(nt, assert.AnError, errors.New("error")) + assert.False(t, nt.Failed()) + + // error - generic error expected but none occurred + nt = &testing.T{} + AssertEqualError(nt, assert.AnError, nil) + assert.True(t, nt.Failed()) + + // success - concrete error expected and matched + nt = &testing.T{} + AssertEqualError(nt, exp, exp) + assert.False(t, nt.Failed()) + + // error - concrete error expected but a different one occurred + nt = &testing.T{} + AssertEqualError(nt, exp, errors.New("other error")) + assert.True(t, nt.Failed()) +} + +func Test_RequireEqualError(t *testing.T) { + // require assertions stop the calling goroutine via FailNow on + // failure, so every call runs in its own goroutine. + run := func(exp, err error) *testing.T { + nt := &testing.T{} + done := make(chan struct{}) + + go func() { + defer close(done) + + RequireEqualError(nt, exp, err) //nolint:testifylint // the helper's require behavior is under test + }() + + <-done + + return nt + } + + exp := errors.New("expected error") + + // success - no error expected and none occurred + assert.False(t, run(nil, nil).Failed()) + + // error - no error expected but one occurred + assert.True(t, run(nil, assert.AnError).Failed()) + + // success - generic error expected and one occurred + assert.False(t, run(assert.AnError, errors.New("error")).Failed()) + + // error - generic error expected but none occurred + assert.True(t, run(assert.AnError, nil).Failed()) + + // success - concrete error expected and matched + assert.False(t, run(exp, exp).Failed()) + + // error - concrete error expected but a different one occurred + assert.True(t, run(exp, errors.New("other error")).Failed()) +} + func Test_FilterEqual(t *testing.T) { type object struct { Name string @@ -36,6 +129,14 @@ func Test_FilterEqual(t *testing.T) { object{CreatedAt: time.Date(2021, 1, 2, 0, 0, 0, 0, time.UTC).Local()}, ) assert.NoError(t, err) + + // success - unexported fields are compared as well + type hidden struct { + name string + } + + err = FilterEqual(hidden{name: "a"}, hidden{name: "a"}) + assert.NoError(t, err) } func Test_AssertFilterEqual(t *testing.T) { @@ -51,3 +152,38 @@ func Test_AssertFilterEqual(t *testing.T) { AssertFilterEqual(nt, 1, 1) assert.False(t, nt.Failed()) } + +func Test_MockHTTP(t *testing.T) { + client, mt := MockHTTP() + + require.NotNil(t, client) + require.NotNil(t, mt) + assert.Same(t, mt, client.Transport) +} + +func Test_NewBuffer(t *testing.T) { + w, b := NewBuffer() + + require.NotNil(t, w) + require.NotNil(t, b) + assert.NotNil(t, w.out) +} + +func Test_Writer_Write(t *testing.T) { + w, _ := NewBuffer() + + n, err := w.Write([]byte("data")) + require.NoError(t, err) + assert.Equal(t, 4, n) +} + +func Test_Writer_Flush(t *testing.T) { + w, b := NewBuffer() + + _, err := w.Write([]byte("data")) + require.NoError(t, err) + require.Empty(t, b.String()) + + require.NoError(t, w.Flush()) + assert.Equal(t, "data", b.String()) +}