update test, cleanup

chilagrow · Sep 1, 2023 · beb6686 · beb6686
1 parent 79e435d
commit beb6686
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 56 deletions.
diff --git a/internal/backends/database.go b/internal/backends/database.go
@@ -202,8 +202,6 @@ type DatabaseStatsResult struct {
 }
 
 // Stats returns statistics about the database.
-// If collection is specified in *StatsParams.Collection, it returns statistics of only that collection.
-// Otherwise, it returns statistics of the entire database.
 //
 // Database may not exist; that's not an error.
 func (dbc *databaseContract) Stats(ctx context.Context, params *DatabaseStatsParams) (*DatabaseStatsResult, error) {

diff --git a/internal/backends/sqlite/collection_test.go b/internal/backends/sqlite/collection_test.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/FerretDB/FerretDB/internal/backends"
 	"github.com/FerretDB/FerretDB/internal/types"
+	"github.com/FerretDB/FerretDB/internal/util/must"
 	"github.com/FerretDB/FerretDB/internal/util/testutil"
 )
 
@@ -53,3 +54,41 @@ func TestInsert(t *testing.T) {
 	})
 	require.True(t, backends.ErrorCodeIs(err, backends.ErrorCodeInsertDuplicateID))
 }
+
+func TestCollectionStats(t *testing.T) {
+	t.Parallel()
+	ctx := testutil.Ctx(t)
+
+	b, err := NewBackend(&NewBackendParams{URI: "file:./?mode=memory", L: testutil.Logger(t)})
+	require.NoError(t, err)
+	t.Cleanup(b.Close)
+
+	db, err := b.Database(testutil.DatabaseName(t))
+	require.NoError(t, err)
+	t.Cleanup(db.Close)
+
+	cNames := []string{"collectionOne", "collectionTwo"}
+	for _, cName := range cNames {
+		err = db.CreateCollection(ctx, &backends.CreateCollectionParams{Name: cName})
+		require.NoError(t, err)
+	}
+
+	c, err := db.Collection(cNames[0])
+	require.NoError(t, err)
+
+	_, err = c.InsertAll(ctx, &backends.InsertAllParams{
+		Docs: []*types.Document{must.NotFail(types.NewDocument("_id", types.NewObjectID()))},
+	})
+	require.NoError(t, err)
+
+	dbStatsRes, err := db.Stats(ctx, new(backends.DatabaseStatsParams))
+	require.NoError(t, err)
+
+	res, err := c.Stats(ctx, new(backends.CollectionStatsParams))
+	require.NoError(t, err)
+	require.NotZero(t, res.SizeTotal)
+	require.Less(t, res.SizeTotal, dbStatsRes.SizeTotal)
+	require.NotZero(t, res.SizeCollection)
+	require.Less(t, res.SizeCollection, dbStatsRes.SizeCollections)
+	require.Equal(t, int64(1), res.CountObjects)
+}
diff --git a/internal/backends/sqlite/database.go b/internal/backends/sqlite/database.go
@@ -153,7 +153,7 @@ func (db *database) Stats(ctx context.Context, params *backends.DatabaseStatsPar
 	}, nil
 }
 
-// relationStats returns statistics about tables and countRows for the given list of collections.
+// relationStats returns statistics about tables and indexes for the given collections.
 func relationStats(ctx context.Context, db *fsql.DB, list []*metadata.Collection) (*stats, error) {
 	var err error
 

diff --git a/internal/backends/sqlite/database_test.go b/internal/backends/sqlite/database_test.go
@@ -20,24 +20,20 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/FerretDB/FerretDB/internal/backends"
-	"github.com/FerretDB/FerretDB/internal/backends/sqlite/metadata"
 	"github.com/FerretDB/FerretDB/internal/util/testutil"
 )
 
-func TestStats(t *testing.T) {
+func TestDatabaseStats(t *testing.T) {
 	t.Parallel()
 	ctx := testutil.Ctx(t)
 
-	r, err := metadata.NewRegistry("file:./?mode=memory", testutil.Logger(t))
+	b, err := NewBackend(&NewBackendParams{URI: "file:./?mode=memory", L: testutil.Logger(t)})
 	require.NoError(t, err)
-	t.Cleanup(r.Close)
+	t.Cleanup(b.Close)
 
-	dbName := t.Name()
-	db := newDatabase(r, dbName)
-
-	t.Cleanup(func() {
-		db.Close()
-	})
+	db, err := b.Database(testutil.DatabaseName(t))
+	require.NoError(t, err)
+	t.Cleanup(db.Close)
 
 	t.Run("NonExistingDatabase", func(t *testing.T) {
 		var res *backends.DatabaseStatsResult
@@ -46,38 +42,19 @@ func TestStats(t *testing.T) {
 		require.Equal(t, new(backends.DatabaseStatsResult), res)
 	})
 
-	collectionOne := "collectionOne"
-	err = db.CreateCollection(ctx, &backends.CreateCollectionParams{Name: collectionOne})
-	require.NoError(t, err)
-	require.NotNil(t, db)
-
-	collectionTwo := "collectionTwo"
-	err = db.CreateCollection(ctx, &backends.CreateCollectionParams{Name: collectionTwo})
-	require.NoError(t, err)
-	require.NotNil(t, db)
-
-	t.Cleanup(func() {
-		r.DatabaseDrop(ctx, dbName)
-	})
-
-	var dbStatsRes *backends.DatabaseStatsResult
-	t.Run("Database", func(t *testing.T) {
-		dbStatsRes, err = db.Stats(ctx, new(backends.DatabaseStatsParams))
+	cNames := []string{"collectionOne", "collectionTwo"}
+	for _, cName := range cNames {
+		err = db.CreateCollection(ctx, &backends.CreateCollectionParams{Name: cName})
 		require.NoError(t, err)
-		require.NotZero(t, dbStatsRes.SizeTotal)
-		require.NotZero(t, dbStatsRes.CountCollections)
-		require.NotZero(t, dbStatsRes.SizeCollections)
-		require.Zero(t, dbStatsRes.CountObjects)
-	})
+		require.NotNil(t, db)
+	}
 
-	t.Run("Collection", func(t *testing.T) {
-		c := newCollection(r, dbName, collectionOne)
-		res, err := c.Stats(ctx, new(backends.CollectionStatsParams))
+	t.Run("DatabaseWithCollections", func(t *testing.T) {
+		res, err := db.Stats(ctx, new(backends.DatabaseStatsParams))
 		require.NoError(t, err)
 		require.NotZero(t, res.SizeTotal)
-		require.Less(t, res.SizeTotal, dbStatsRes.SizeTotal)
-		require.NotZero(t, res.SizeCollection)
-		require.Less(t, res.SizeCollection, dbStatsRes.SizeCollections)
+		require.NotZero(t, len(cNames))
+		require.NotZero(t, res.SizeCollections)
 		require.Zero(t, res.CountObjects)
 	})
 }
diff --git a/internal/handlers/pg/msg_collstats.go b/internal/handlers/pg/msg_collstats.go
@@ -92,6 +92,8 @@ func (h *Handler) MsgCollStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 		pairs = append(pairs, "avgObjSize", stats.SizeCollection/stats.CountObjects)
 	}
 
+	// MongoDB uses "numbers" that could be int32 or int64,
+	// FerretDB always returns int64 for simplicity.
 	pairs = append(pairs,
 		"storageSize", stats.SizeCollection/scale,
 		"nindexes", stats.CountIndexes,

diff --git a/internal/handlers/sqlite/msg_aggregate.go b/internal/handlers/sqlite/msg_aggregate.go
@@ -69,9 +69,9 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 	// handle collection-agnostic pipelines ({aggregate: 1})
 	// TODO https://github.com/FerretDB/FerretDB/issues/1890
 	var ok bool
-	var collectionName string
+	var cName string
 
-	if collectionName, ok = collectionParam.(string); !ok {
+	if cName, ok = collectionParam.(string); !ok {
 		return nil, commonerrors.NewCommandErrorMsgWithArgument(
 			commonerrors.ErrFailedToParse,
 			"Invalid command format: the 'aggregate' field must specify a collection name or 1",
@@ -82,18 +82,18 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 	db, err := h.b.Database(dbName)
 	if err != nil {
 		if backends.ErrorCodeIs(err, backends.ErrorCodeDatabaseNameIsInvalid) {
-			msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", dbName, collectionName)
+			msg := fmt.Sprintf("Invalid namespace specified '%s.%s'", dbName, cName)
 			return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, document.Command())
 		}
 
 		return nil, lazyerrors.Error(err)
 	}
 	defer db.Close()
 
-	c, err := db.Collection(collectionName)
+	c, err := db.Collection(cName)
 	if err != nil {
 		if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionNameIsInvalid) {
-			msg := fmt.Sprintf("Invalid collection name: %s", collectionName)
+			msg := fmt.Sprintf("Invalid collection name: %s", cName)
 			return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, document.Command())
 		}
 
@@ -268,7 +268,7 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 		statistics := stages.GetStatistics(collStatsDocuments)
 
 		iter, err = processStagesStats(ctx, closer, &stagesStatsParams{
-			c, dbName, collectionName, statistics, collStatsDocuments,
+			c, dbName, cName, statistics, collStatsDocuments,
 		})
 	}
 
@@ -282,7 +282,7 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 	cursor := h.cursors.NewCursor(ctx, &cursor.NewParams{
 		Iter:       iterator.WithClose(iter, closer.Close),
 		DB:         dbName,
-		Collection: collectionName,
+		Collection: cName,
 		Username:   username,
 	})
 
@@ -312,7 +312,7 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 			"cursor", must.NotFail(types.NewDocument(
 				"firstBatch", firstBatch,
 				"id", cursorID,
-				"ns", dbName+"."+collectionName,
+				"ns", dbName+"."+cName,
 			)),
 			"ok", float64(1),
 		))},
@@ -350,11 +350,11 @@ func processStagesDocuments(ctx context.Context, closer *iterator.MultiCloser, p
 
 // stagesStatsParams contains the parameters for processStagesStats.
 type stagesStatsParams struct {
-	c              backends.Collection
-	dbName         string
-	collectionName string
-	statistics     map[stages.Statistic]struct{}
-	stages         []aggregations.Stage
+	c          backends.Collection
+	dbName     string
+	cName      string
+	statistics map[stages.Statistic]struct{}
+	stages     []aggregations.Stage
 }
 
 // processStagesStats retrieves the statistics from the database and then processes them through the stages.
@@ -375,7 +375,7 @@ func processStagesStats(ctx context.Context, closer *iterator.MultiCloser, p *st
 	}
 
 	doc := must.NotFail(types.NewDocument(
-		"ns", p.dbName+"."+p.collectionName,
+		"ns", p.dbName+"."+p.cName,
 		"host", host,
 		"localTime", time.Now().UTC().Format(time.RFC3339),
 	))
@@ -387,7 +387,7 @@ func processStagesStats(ctx context.Context, closer *iterator.MultiCloser, p *st
 		if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionDoesNotExist) {
 			return nil, commonerrors.NewCommandErrorMsgWithArgument(
 				commonerrors.ErrNamespaceNotFound,
-				fmt.Sprintf("ns not found: %s.%s", p.dbName, p.collectionName),
+				fmt.Sprintf("ns not found: %s.%s", p.dbName, p.cName),
 				"aggregate",
 			)
 		}

diff --git a/internal/handlers/sqlite/msg_collstats.go b/internal/handlers/sqlite/msg_collstats.go
@@ -78,6 +78,11 @@ func (h *Handler) MsgCollStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 	}
 
 	stats, err := c.Stats(ctx, new(backends.CollectionStatsParams))
+	if backends.ErrorCodeIs(err, backends.ErrorCodeCollectionDoesNotExist) {
+		stats = new(backends.CollectionStatsResult)
+		err = nil
+	}
+
 	if err != nil {
 		return nil, lazyerrors.Error(err)
 	}
@@ -93,6 +98,8 @@ func (h *Handler) MsgCollStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs
 		pairs = append(pairs, "avgObjSize", stats.SizeCollection/stats.CountObjects)
 	}
 
+	// MongoDB uses "numbers" that could be int32 or int64,
+	// FerretDB always returns int64 for simplicity.
 	pairs = append(pairs,
 		"storageSize", stats.SizeCollection/scale,
 		"nindexes", stats.CountIndexes,