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())
+}