Improve tests and fix the bugs

.golangci.yml

@@ -133,6 +133,7 @@ linters:
     rules:
       - linters:
           - dupl
+          - exhaustruct
           - goconst
           - lll
           - mnd

cmd/gateway/helpers.go

@@ -51,8 +51,8 @@ func withConfig(action func(c *cli.Context, cfg config.Config) error) cli.Action
 	}
 }
 
-func newMigrator(cfg config.PostgresConfig) (*migrator.Migrator, error) { //nolint:gocritic
-	dbConn, err := pgdb.ConnectToPg(cfg)
+func newMigrator(ctx context.Context, cfg config.PostgresConfig) (*migrator.Migrator, error) { //nolint:gocritic
+	dbConn, err := pgdb.ConnectToPg(ctx, cfg)
 	if err != nil {
 		return nil, fmt.Errorf("connect to postgres: %w", err)
 	}
@@ -85,7 +85,7 @@ func withApp(
 
 		logger.Info("start")
 
-		migratorInst, err := newMigrator(cfg.Storage)
+		migratorInst, err := newMigrator(ctx, cfg.Storage)
 		if err != nil {
 			return fmt.Errorf("create new migrator: %w", err)
 		}
@@ -94,7 +94,7 @@ func withApp(
 			return fmt.Errorf("up all migrations: %w", err)
 		}
 
-		dbConnWrite, err := pgdb.ConnectToPg(cfg.Storage)
+		dbConnWrite, err := pgdb.ConnectToPg(ctx, cfg.Storage)
 		if err != nil {
 			return fmt.Errorf("connect to db: %w", err)
 		}

cmd/gateway/main.go

@@ -155,8 +155,8 @@ func cmdGenerateModels(_ *cli.Context, cfg config.Config) error { //nolint:gocri
 	return nil
 }
 
-func cmdMigrateUp(_ *cli.Context, cfg config.Config) error { //nolint:gocritic
-	migratorInst, err := newMigrator(cfg.Storage)
+func cmdMigrateUp(c *cli.Context, cfg config.Config) error { //nolint:gocritic
+	migratorInst, err := newMigrator(c.Context, cfg.Storage)
 	if err != nil {
 		return fmt.Errorf("create new migrator: %w", err)
 	}
@@ -168,8 +168,8 @@ func cmdMigrateUp(_ *cli.Context, cfg config.Config) error { //nolint:gocritic
 	return nil
 }
 
-func cmdMigrateDownOne(_ *cli.Context, cfg config.Config) error { //nolint:gocritic
-	migratorInst, err := newMigrator(cfg.Storage)
+func cmdMigrateDownOne(c *cli.Context, cfg config.Config) error { //nolint:gocritic
+	migratorInst, err := newMigrator(c.Context, cfg.Storage)
 	if err != nil {
 		return fmt.Errorf("create new migrator: %w", err)
 	}
@@ -181,8 +181,8 @@ func cmdMigrateDownOne(_ *cli.Context, cfg config.Config) error { //nolint:gocri
 	return nil
 }
 
-func cmdMigrateCreateNew(_ *cli.Context, cfg config.Config) error { //nolint:gocritic
-	migratorInst, err := newMigrator(cfg.Storage)
+func cmdMigrateCreateNew(c *cli.Context, cfg config.Config) error { //nolint:gocritic
+	migratorInst, err := newMigrator(c.Context, cfg.Storage)
 	if err != nil {
 		return fmt.Errorf("create new migrator: %w", err)
 	}

frontend/src/components/QueryResultsView.svelte

@@ -17,7 +17,13 @@
 		status = "",
 		error = ""
 	} = $props();
-	console.log(meta)
+
+	const displayMeta = $derived(meta ?? {
+		rows_count: "",
+		columns_count: "",
+		vectors_count: "",
+		trace: {records: []}
+	});
 	const panelClass =
 		"rounded-xl border border-zinc-700/90 bg-zinc-900/90 shadow-[0_18px_42px_rgb(0_0_0_/_0.28)] backdrop-blur-xl";
 	const chipClass = "rounded-md border border-zinc-700/80 bg-zinc-800/90";
@@ -44,10 +50,10 @@
 				<Chip title="Created" value={createdAt}/>
 				<Chip title="Target" value={targetID}/>
 				<Chip title="User" value={userID}/>
-				<Chip title="Rows" value={meta.rows_count}/>
-				<Chip title="Columns" value={meta.columns_count}/>
-				<Chip title="Vectors" value={meta.vectors_count}/>
-				{#each meta.trace.records as record}
+				<Chip title="Rows" value={displayMeta.rows_count}/>
+				<Chip title="Columns" value={displayMeta.columns_count}/>
+				<Chip title="Vectors" value={displayMeta.vectors_count}/>
+				{#each displayMeta.trace.records as record}
 					<Chip title="{record.name} (ms)" value={record.duration/1000000}/>
 				{/each}
 			</div>

internal/app/service.go

@@ -46,8 +46,9 @@ import (
 )
 
 var (
-	ErrNotFound  = errors.New("not found")
-	ErrForbidden = errors.New("forbidden")
+	ErrNotFound        = errors.New("not found")
+	ErrForbidden       = errors.New("forbidden")
+	errInvalidArgument = errors.New("invalid argument")
 )
 
 type storedQueryResultPayload struct {
@@ -507,7 +508,7 @@ func (s *Service) ListAllBookmarks(ctx context.Context, uid config.UserID) ([]st
 
 func (s *Service) ListRecentQueries(ctx context.Context, uid config.UserID, limit int64) ([]structs.Query, error) {
 	if limit <= 0 {
-		limit = 50
+		return nil, fmt.Errorf("limit must be positive: %w", errInvalidArgument)
 	}
 
 	items, err := s.opts.storage.ListQueryResultsByUser(s.opts.storage.Conn(ctx), uid, limit)
@@ -518,9 +519,11 @@ func (s *Service) ListRecentQueries(ctx context.Context, uid config.UserID, limi
 	out := make([]structs.Query, 0, len(items))
 	for i := range items {
 		item := &items[i]
-		var payload storedQueryResultPayload
-		if err := json.Unmarshal(item.Response, &payload); err != nil {
-			continue
+		if len(item.Response) != 0 {
+			var payload storedQueryResultPayload
+			if err := json.Unmarshal(item.Response, &payload); err != nil {
+				continue
+			}
 		}
 
 		out = append(out, structs.Query{
@@ -545,10 +548,10 @@ func (s *Service) ListAdminRequests(
 	}
 
 	if page <= 0 {
-		page = 1
+		return nil, false, fmt.Errorf("page must be positive: %w", errInvalidArgument)
 	}
 	if pageSize <= 0 {
-		pageSize = 50
+		return nil, false, fmt.Errorf("page size must be positive: %w", errInvalidArgument)
 	}
 
 	offset := (page - 1) * pageSize

internal/app/service_bookmarks_test.go

@@ -0,0 +1,218 @@
+// Database Gateway provides access to servers with ACL for safe and restricted database interactions.
+// Copyright (C) 2024  Kirill Zhuravlev
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package app //nolint:exhaustruct,testpackage
+
+import (
+	"context"
+	"errors"
+	"sync"
+	"testing"
+
+	"github.com/go-jet/jet/v2/qrm"
+	"github.com/kazhuravlev/database-gateway/internal/config"
+	"github.com/kazhuravlev/database-gateway/internal/storage"
+	"github.com/kazhuravlev/database-gateway/internal/storage/jetgen/model"
+	"github.com/kazhuravlev/database-gateway/internal/structs"
+	"github.com/kazhuravlev/database-gateway/internal/uuid6"
+	"github.com/stretchr/testify/require"
+)
+
+var errBookmarkStorageFailed = errors.New("storage failed")
+
+type fakeBookmarkStorage struct {
+	mu              sync.Mutex
+	bookmarkInserts []storage.InsertBookmarkReq
+	insertErr       error
+}
+
+func (*fakeBookmarkStorage) Conn(context.Context) qrm.DB { //nolint:ireturn
+	return nil
+}
+
+func (*fakeBookmarkStorage) InsertQueryResults(qrm.DB, storage.InsertQueryResultsReq) error {
+	return nil
+}
+
+func (*fakeBookmarkStorage) SetQueryResultsState(qrm.DB, storage.SetQueryResultsStateReq) error {
+	return nil
+}
+
+func (*fakeBookmarkStorage) GetQueryResultsByID(qrm.DB, uuid6.UUID) (*model.QueryResults, error) {
+	return nil, storage.ErrNotFound
+}
+
+func (*fakeBookmarkStorage) ListQueryResultsByUser(
+	qrm.DB,
+	config.UserID,
+	int64,
+) ([]storage.QueryResult, error) {
+	return nil, nil
+}
+
+func (*fakeBookmarkStorage) ListQueryResults(qrm.DB, int64, int64) ([]storage.QueryResult, error) {
+	return nil, nil
+}
+
+func (s *fakeBookmarkStorage) InsertBookmark(_ qrm.DB, req storage.InsertBookmarkReq) error { //nolint:gocritic
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.bookmarkInserts = append(s.bookmarkInserts, req)
+	if s.insertErr != nil {
+		return s.insertErr
+	}
+
+	return nil
+}
+
+func (*fakeBookmarkStorage) DeleteBookmark(qrm.DB, config.UserID, uuid6.UUID) error {
+	return nil
+}
+
+func (*fakeBookmarkStorage) ListBookmarks(qrm.DB, config.UserID, config.TargetID) ([]storage.Bookmark, error) {
+	return nil, nil
+}
+
+func (*fakeBookmarkStorage) ListBookmarksByUser(qrm.DB, config.UserID) ([]storage.Bookmark, error) {
+	return nil, nil
+}
+
+func (s *fakeBookmarkStorage) insertedBookmarks() []storage.InsertBookmarkReq {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	return append([]storage.InsertBookmarkReq(nil), s.bookmarkInserts...)
+}
+
+func TestServiceAddBookmarkTrimsTitleAndQuery(t *testing.T) {
+	t.Parallel()
+
+	store := new(fakeBookmarkStorage)
+	svc := newBookmarkTestService(t, store, bookmarkAllowPolicy)
+	user := structs.User{ID: config.UserID("alice@example.com"), Username: "alice", Role: config.RoleUser}
+
+	err := svc.AddBookmark(context.Background(), user, config.TargetID("pg-1"), "  Important query  ", "\n select 1 \t")
+
+	require.NoError(t, err)
+	inserted := store.insertedBookmarks()
+	require.Len(t, inserted, 1)
+	require.NotEmpty(t, inserted[0].ID.S())
+	require.Equal(t, user.ID, inserted[0].UserID)
+	require.Equal(t, config.TargetID("pg-1"), inserted[0].TargetID)
+	require.Equal(t, "Important query", inserted[0].Title)
+	require.Equal(t, "select 1", inserted[0].Query)
+	require.False(t, inserted[0].CreatedAt.IsZero())
+}
+
+func TestServiceAddBookmarkValidatesEmptyTitleAndQuery(t *testing.T) {
+	t.Parallel()
+
+	user := structs.User{ID: config.UserID("alice@example.com"), Username: "alice", Role: config.RoleUser}
+	testCases := []struct {
+		name  string
+		title string
+		query string
+	}{
+		{name: "empty title", title: "", query: "select 1"},
+		{name: "blank title", title: " \t", query: "select 1"},
+		{name: "empty query", title: "Important query", query: ""},
+		{name: "blank query", title: "Important query", query: " \n"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			store := new(fakeBookmarkStorage)
+			svc := newBookmarkTestService(t, store, bookmarkAllowPolicy)
+
+			err := svc.AddBookmark(context.Background(), user, config.TargetID("pg-1"), tc.title, tc.query)
+
+			require.ErrorContains(t, err, "title and query are required")
+			require.Empty(t, store.insertedBookmarks())
+		})
+	}
+}
+
+func TestServiceAddBookmarkValidatesTargetAccess(t *testing.T) {
+	t.Parallel()
+
+	store := new(fakeBookmarkStorage)
+	svc := newBookmarkTestService(t, store, `
+package gateway
+
+default allow_target := false
+default allow_query := false
+`)
+	user := structs.User{ID: config.UserID("alice@example.com"), Username: "alice", Role: config.RoleUser}
+
+	err := svc.AddBookmark(context.Background(), user, config.TargetID("pg-1"), "Important query", "select 1")
+
+	require.ErrorIs(t, err, ErrNotFound)
+	require.ErrorContains(t, err, "validate target access")
+	require.Empty(t, store.insertedBookmarks())
+}
+
+func TestServiceAddBookmarkPropagatesStorageError(t *testing.T) {
+	t.Parallel()
+
+	insertErr := errBookmarkStorageFailed
+	store := &fakeBookmarkStorage{insertErr: insertErr}
+	svc := newBookmarkTestService(t, store, bookmarkAllowPolicy)
+	user := structs.User{ID: config.UserID("alice@example.com"), Username: "alice", Role: config.RoleUser}
+
+	err := svc.AddBookmark(context.Background(), user, config.TargetID("pg-1"), "Important query", "select 1")
+
+	require.ErrorIs(t, err, insertErr)
+	require.ErrorContains(t, err, "insert bookmark")
+	require.Len(t, store.insertedBookmarks(), 1)
+}
+
+const bookmarkAllowPolicy = `
+package gateway
+
+default allow_target := false
+default allow_query := false
+
+allow_target if {
+	"role:user" in input.subjects
+	input.target == "pg-1"
+}
+`
+
+func newBookmarkTestService(t *testing.T, store *fakeBookmarkStorage, policy string) *Service {
+	t.Helper()
+
+	return &Service{
+		opts: Options{
+			targets: []config.Target{
+				{
+					ID:            "pg-1",
+					Description:   "main",
+					Tags:          []string{"prod"},
+					Type:          "postgres",
+					DefaultSchema: "public",
+					Tables:        []config.TargetTable{{Table: "public.clients", Fields: []string{"id"}}},
+				},
+			},
+			authorizer: mustAuthorizer(t, policy),
+			storage:    store,
+		},
+		connsMu: new(sync.RWMutex),
+		conns:   nil,
+	}
+}