feat(repository): refactor query/mutation ports with capability-safe

interfaces - refactor: split UserRepository into AuthUserRepository and AdminUserRepository capabilities - refactor: split SessionRepository into AuthSessionRepository and repository-owned CLI/admin methods - refactor: replace generic Repository Update/Delete with operation-specific params and ownership predicates - refactor: replace CredentialRepository generic CRUD with passkey-specific methods - refactor: replace FileRepository generic Create/Update/Delete with UploadedFileParams, DirectoryParams, and owned soft-delete - refactor: remove repository fields from WebApp struct; repositories are now composition-time wiring only - feat: add domain error kinds ErrParentNotFound, ErrParentNotDir, ErrDirectoryNotEmpty, ErrInvalidMove - feat: add CredentialTypeAppPasskey constant - feat: add testutil.SetUserAdmin for test fixture setup that bypasses production service ports - test: add architecture test banning GORM Save in repository package - test: add capability interface contract tests ensuring each service receives the minimal interface - test: add blockingStorage helper for concurrent promotion tests - test: add preserved DSN parameter test for sqliteImmediateDSN - docs: update architecture decisions with repository write rules and capability separation - docs: update roadmap to clarify atomic single-use refresh sessions - docs: add -race test target to development docs
2026-07-16 12:24:36 +08:00
parent 6604ecb026
commit a18f96912d
30 changed files with 1259 additions and 394 deletions
@@ -3,11 +3,14 @@ package repository
 import (
 	"context"
 	"errors"
+	"path/filepath"
+	"sync"
 	"testing"

 	"gorm.io/driver/sqlite"
 	"gorm.io/gorm"

+	"github.com/dhao2001/mygo/internal/config"
 	"github.com/dhao2001/mygo/internal/model"
 )

@@ -25,6 +28,48 @@ func setupFileRepo(t *testing.T) FileRepository {
 	return NewFileRepository(db)
 }

+func createFileRecord(ctx context.Context, repo FileRepository, file *model.File) error {
+	var err error
+	if file.IsDir {
+		_, err = repo.CreateDirectory(ctx, DirectoryParams{
+			ID: file.ID, UserID: file.UserID, ParentID: file.ParentID, Name: file.Name,
+		})
+	} else {
+		_, err = repo.CreateUploadedFile(ctx, UploadedFileParams{
+			ID: file.ID, UserID: file.UserID, ParentID: file.ParentID, Name: file.Name,
+			Size: file.Size, MimeType: file.MimeType, StoragePath: file.StoragePath, Hash: file.Hash,
+		})
+	}
+	if err != nil {
+		return err
+	}
+	if file.Status == model.StatusUserDeleted {
+		return repo.SoftDeleteOwned(ctx, file.UserID, file.ID)
+	}
+	return nil
+}
+
+func setupConcurrentFileRepo(t *testing.T) FileRepository {
+	t.Helper()
+	db, err := Open(config.DatabaseConfig{
+		Driver: "sqlite3",
+		SQLite: config.SQLiteConfig{Path: filepath.Join(t.TempDir(), "concurrency.db")},
+	})
+	if err != nil {
+		t.Fatalf("open db: %v", err)
+	}
+	if err := AutoMigrate(db); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	t.Cleanup(func() {
+		sqlDB, err := db.DB()
+		if err == nil {
+			_ = sqlDB.Close()
+		}
+	})
+	return NewFileRepository(db)
+}
+
 func TestFileRepository_Create(t *testing.T) {
 	repo := setupFileRepo(t)
 	ctx := context.Background()
@@ -37,7 +82,7 @@ func TestFileRepository_Create(t *testing.T) {
 		Status: model.StatusActive,
 	}

-	if err := repo.Create(ctx, file); err != nil {
+	if err := createFileRecord(ctx, repo, file); err != nil {
 		t.Fatalf("Create = %v", err)
 	}
 }
@@ -52,7 +97,7 @@ func TestFileRepository_FindByID(t *testing.T) {
 		Name:   "test.txt",
 		Status: model.StatusActive,
 	}
-	if err := repo.Create(ctx, file); err != nil {
+	if err := createFileRecord(ctx, repo, file); err != nil {
 		t.Fatalf("Create = %v", err)
 	}

@@ -85,7 +130,7 @@ func TestFileRepository_FindByUserID(t *testing.T) {
 		{ID: "f-3", UserID: "user-2", Name: "c.txt", Status: model.StatusActive},
 	}
 	for _, f := range files {
-		if err := repo.Create(ctx, f); err != nil {
+		if err := createFileRecord(ctx, repo, f); err != nil {
 			t.Fatalf("Create = %v", err)
 		}
 	}
@@ -107,13 +152,16 @@ func TestFileRepository_FindByParentID(t *testing.T) {
 	ctx := context.Background()

 	parentID := "dir-1"
+	if err := createFileRecord(ctx, repo, &model.File{ID: parentID, UserID: "user-1", Name: "dir", Status: model.StatusActive, IsDir: true}); err != nil {
+		t.Fatalf("Create parent = %v", err)
+	}
 	files := []*model.File{
 		{ID: "f-1", UserID: "user-1", ParentID: &parentID, Name: "a.txt", Status: model.StatusActive},
 		{ID: "f-2", UserID: "user-1", ParentID: &parentID, Name: "b.txt", Status: model.StatusActive},
 		{ID: "f-3", UserID: "user-1", Name: "c.txt", Status: model.StatusActive},
 	}
 	for _, f := range files {
-		if err := repo.Create(ctx, f); err != nil {
+		if err := createFileRecord(ctx, repo, f); err != nil {
 			t.Fatalf("Create = %v", err)
 		}
 	}
@@ -132,12 +180,15 @@ func TestFileRepository_FindByParentIDNull(t *testing.T) {
 	ctx := context.Background()

 	parentID := "dir-1"
+	if err := createFileRecord(ctx, repo, &model.File{ID: parentID, UserID: "user-1", Name: "dir", Status: model.StatusActive, IsDir: true}); err != nil {
+		t.Fatalf("Create parent = %v", err)
+	}
 	files := []*model.File{
 		{ID: "f-1", UserID: "user-1", ParentID: &parentID, Name: "a.txt", Status: model.StatusActive},
 		{ID: "f-2", UserID: "user-1", Name: "root.txt", Status: model.StatusActive},
 	}
 	for _, f := range files {
-		if err := repo.Create(ctx, f); err != nil {
+		if err := createFileRecord(ctx, repo, f); err != nil {
 			t.Fatalf("Create = %v", err)
 		}
 	}
@@ -146,8 +197,8 @@ func TestFileRepository_FindByParentIDNull(t *testing.T) {
 	if err != nil {
 		t.Fatalf("FindByParentID(nil) = %v", err)
 	}
-	if len(children) != 1 {
-		t.Errorf("len(children) = %d, want 1", len(children))
+	if len(children) != 2 {
+		t.Errorf("len(children) = %d, want 2", len(children))
 	}
 }

@@ -155,14 +206,26 @@ func TestFileRepository_Update(t *testing.T) {
 	repo := setupFileRepo(t)
 	ctx := context.Background()

-	file := &model.File{ID: "file-1", UserID: "user-1", Name: "original.txt", Status: model.StatusActive}
-	if err := repo.Create(ctx, file); err != nil {
+	file := &model.File{
+		ID:          "file-1",
+		UserID:      "user-1",
+		Name:        "original.txt",
+		Size:        1024,
+		MimeType:    "text/plain",
+		StoragePath: "data/user-1/file-1",
+		Hash:        "original-hash",
+		Status:      model.StatusActive,
+	}
+	if err := createFileRecord(ctx, repo, file); err != nil {
 		t.Fatalf("Create = %v", err)
 	}
+	before, err := repo.FindByID(ctx, file.ID)
+	if err != nil {
+		t.Fatalf("FindByID before update = %v", err)
+	}

-	file.Name = "renamed.txt"
-	file.Size = 2048
-	if err := repo.Update(ctx, file); err != nil {
+	newName := "renamed.txt"
+	if _, err := repo.UpdateOwnedMetadata(ctx, FileMetadataUpdate{FileID: file.ID, UserID: file.UserID, NewName: &newName}); err != nil {
 		t.Fatalf("Update = %v", err)
 	}

@@ -173,8 +236,11 @@ func TestFileRepository_Update(t *testing.T) {
 	if found.Name != "renamed.txt" {
 		t.Errorf("name = %q, want %q", found.Name, "renamed.txt")
 	}
-	if found.Size != 2048 {
-		t.Errorf("size = %d, want %d", found.Size, 2048)
+	if found.UserID != before.UserID || found.Status != before.Status || found.IsDir != before.IsDir ||
+		found.Size != before.Size || found.MimeType != before.MimeType ||
+		found.StoragePath != before.StoragePath || found.Hash != before.Hash ||
+		!found.CreatedAt.Equal(before.CreatedAt) {
+		t.Fatalf("metadata update changed protected fields: before=%+v after=%+v", before, found)
 	}
 }

@@ -183,11 +249,11 @@ func TestFileRepository_Delete(t *testing.T) {
 	ctx := context.Background()

 	file := &model.File{ID: "file-1", UserID: "user-1", Name: "test.txt", Status: model.StatusActive}
-	if err := repo.Create(ctx, file); err != nil {
+	if err := createFileRecord(ctx, repo, file); err != nil {
 		t.Fatalf("Create = %v", err)
 	}

-	if err := repo.Delete(ctx, "file-1"); err != nil {
+	if err := repo.SoftDeleteOwned(ctx, "user-1", "file-1"); err != nil {
 		t.Fatalf("Delete = %v", err)
 	}

@@ -207,11 +273,11 @@ func TestFileRepository_SoftDelete(t *testing.T) {
 		Name:   "test.txt",
 		Status: model.StatusActive,
 	}
-	if err := repo.Create(ctx, file); err != nil {
+	if err := createFileRecord(ctx, repo, file); err != nil {
 		t.Fatalf("Create = %v", err)
 	}

-	if err := repo.Delete(ctx, "file-1"); err != nil {
+	if err := repo.SoftDeleteOwned(ctx, "user-1", "file-1"); err != nil {
 		t.Fatalf("Delete = %v", err)
 	}

@@ -238,10 +304,10 @@ func TestFileRepository_StatusFilter(t *testing.T) {
 		Status: model.StatusUserDeleted,
 	}

-	if err := repo.Create(ctx, activeFile); err != nil {
+	if err := createFileRecord(ctx, repo, activeFile); err != nil {
 		t.Fatalf("Create active = %v", err)
 	}
-	if err := repo.Create(ctx, deletedFile); err != nil {
+	if err := createFileRecord(ctx, repo, deletedFile); err != nil {
 		t.Fatalf("Create deleted = %v", err)
 	}

@@ -270,17 +336,17 @@ func TestFileRepository_DeleteReturnsNotFoundAfterDeletion(t *testing.T) {
 		Name:   "test.txt",
 		Status: model.StatusActive,
 	}
-	if err := repo.Create(ctx, file); err != nil {
+	if err := createFileRecord(ctx, repo, file); err != nil {
 		t.Fatalf("Create = %v", err)
 	}

-	if err := repo.Delete(ctx, "file-1"); err != nil {
+	if err := repo.SoftDeleteOwned(ctx, "user-1", "file-1"); err != nil {
 		t.Fatalf("first Delete = %v", err)
 	}
-	if err := repo.Delete(ctx, "file-1"); !errors.Is(err, model.ErrNotFound) {
+	if err := repo.SoftDeleteOwned(ctx, "user-1", "file-1"); !errors.Is(err, model.ErrNotFound) {
 		t.Fatalf("second Delete = %v, want ErrNotFound", err)
 	}
-	if err := repo.Delete(ctx, "missing-file"); !errors.Is(err, model.ErrNotFound) {
+	if err := repo.SoftDeleteOwned(ctx, "user-1", "missing-file"); !errors.Is(err, model.ErrNotFound) {
 		t.Fatalf("missing Delete = %v, want ErrNotFound", err)
 	}
 }
@@ -302,10 +368,10 @@ func TestFileRepository_StatusFilterCount(t *testing.T) {
 		Status: model.StatusUserDeleted,
 	}

-	if err := repo.Create(ctx, activeFile); err != nil {
+	if err := createFileRecord(ctx, repo, activeFile); err != nil {
 		t.Fatalf("Create active = %v", err)
 	}
-	if err := repo.Create(ctx, deletedFile); err != nil {
+	if err := createFileRecord(ctx, repo, deletedFile); err != nil {
 		t.Fatalf("Create deleted = %v", err)
 	}

@@ -317,3 +383,160 @@ func TestFileRepository_StatusFilterCount(t *testing.T) {
 		t.Errorf("total = %d, want 1 (soft-deleted excluded from count)", total)
 	}
 }
+
+func TestFileRepository_UpdateCannotReviveDeletedFile(t *testing.T) {
+	repo := setupFileRepo(t)
+	ctx := context.Background()
+	file := &model.File{ID: "file-1", UserID: "user-1", Name: "before.txt", Status: model.StatusActive}
+	if err := createFileRecord(ctx, repo, file); err != nil {
+		t.Fatalf("Create = %v", err)
+	}
+	if err := repo.SoftDeleteOwned(ctx, file.UserID, file.ID); err != nil {
+		t.Fatalf("SoftDeleteOwned = %v", err)
+	}
+
+	newName := "after.txt"
+	_, err := repo.UpdateOwnedMetadata(ctx, FileMetadataUpdate{FileID: file.ID, UserID: file.UserID, NewName: &newName})
+	if !errors.Is(err, model.ErrNotFound) {
+		t.Fatalf("UpdateOwnedMetadata = %v, want ErrNotFound", err)
+	}
+	if _, err := repo.FindByID(ctx, file.ID); !errors.Is(err, model.ErrNotFound) {
+		t.Fatalf("deleted file became active again: %v", err)
+	}
+}
+
+func TestFileRepository_ConcurrentUpdateAndDeleteCannotReviveFile(t *testing.T) {
+	repo := setupConcurrentFileRepo(t)
+	ctx := context.Background()
+	if _, err := repo.CreateUploadedFile(ctx, UploadedFileParams{
+		ID: "file-1", UserID: "user-1", Name: "before.txt", StoragePath: "data/user-1/file-1",
+	}); err != nil {
+		t.Fatalf("CreateUploadedFile = %v", err)
+	}
+
+	newName := "after.txt"
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	wg.Add(2)
+	var updateErr, deleteErr error
+	go func() {
+		defer wg.Done()
+		<-start
+		_, updateErr = repo.UpdateOwnedMetadata(ctx, FileMetadataUpdate{
+			FileID: "file-1", UserID: "user-1", NewName: &newName,
+		})
+	}()
+	go func() {
+		defer wg.Done()
+		<-start
+		deleteErr = repo.SoftDeleteOwned(ctx, "user-1", "file-1")
+	}()
+	close(start)
+	wg.Wait()
+
+	if deleteErr != nil {
+		t.Fatalf("SoftDeleteOwned = %v, want success", deleteErr)
+	}
+	if updateErr != nil && !errors.Is(updateErr, model.ErrNotFound) {
+		t.Fatalf("UpdateOwnedMetadata = %v, want success or ErrNotFound", updateErr)
+	}
+	if _, err := repo.FindByID(ctx, "file-1"); !errors.Is(err, model.ErrNotFound) {
+		t.Fatalf("concurrent update revived deleted file: %v", err)
+	}
+}
+
+func TestFileRepository_CreateAndParentDeleteAreSerialized(t *testing.T) {
+	repo := setupConcurrentFileRepo(t)
+	ctx := context.Background()
+	dir, err := repo.CreateDirectory(ctx, DirectoryParams{ID: "dir-1", UserID: "user-1", Name: "dir"})
+	if err != nil {
+		t.Fatalf("CreateDirectory = %v", err)
+	}
+
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	wg.Add(2)
+	var createErr, deleteErr error
+	go func() {
+		defer wg.Done()
+		<-start
+		_, createErr = repo.CreateUploadedFile(ctx, UploadedFileParams{
+			ID: "child-1", UserID: "user-1", ParentID: &dir.ID, Name: "child.txt", StoragePath: "data/user-1/child-1",
+		})
+	}()
+	go func() {
+		defer wg.Done()
+		<-start
+		deleteErr = repo.SoftDeleteOwned(ctx, "user-1", dir.ID)
+	}()
+	close(start)
+	wg.Wait()
+
+	switch {
+	case createErr == nil:
+		if !errors.Is(deleteErr, model.ErrDirectoryNotEmpty) {
+			t.Fatalf("create succeeded but delete error = %v, want ErrDirectoryNotEmpty", deleteErr)
+		}
+		if _, err := repo.FindByID(ctx, dir.ID); err != nil {
+			t.Fatalf("parent missing after child creation: %v", err)
+		}
+	case deleteErr == nil:
+		if !errors.Is(createErr, model.ErrParentNotFound) {
+			t.Fatalf("delete succeeded but create error = %v, want ErrParentNotFound", createErr)
+		}
+		if _, err := repo.FindByID(ctx, "child-1"); !errors.Is(err, model.ErrNotFound) {
+			t.Fatalf("active orphan child exists: %v", err)
+		}
+	default:
+		t.Fatalf("neither operation succeeded: create=%v delete=%v", createErr, deleteErr)
+	}
+}
+
+func TestFileRepository_MoveAndTargetDeleteAreSerialized(t *testing.T) {
+	repo := setupConcurrentFileRepo(t)
+	ctx := context.Background()
+	target, err := repo.CreateDirectory(ctx, DirectoryParams{ID: "target-1", UserID: "user-1", Name: "target"})
+	if err != nil {
+		t.Fatalf("CreateDirectory = %v", err)
+	}
+	if _, err := repo.CreateUploadedFile(ctx, UploadedFileParams{ID: "file-1", UserID: "user-1", Name: "file.txt", StoragePath: "data/user-1/file-1"}); err != nil {
+		t.Fatalf("CreateUploadedFile = %v", err)
+	}
+
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	wg.Add(2)
+	var moveErr, deleteErr error
+	go func() {
+		defer wg.Done()
+		<-start
+		_, moveErr = repo.UpdateOwnedMetadata(ctx, FileMetadataUpdate{FileID: "file-1", UserID: "user-1", NewParentID: &target.ID})
+	}()
+	go func() {
+		defer wg.Done()
+		<-start
+		deleteErr = repo.SoftDeleteOwned(ctx, "user-1", target.ID)
+	}()
+	close(start)
+	wg.Wait()
+
+	switch {
+	case moveErr == nil:
+		if !errors.Is(deleteErr, model.ErrDirectoryNotEmpty) {
+			t.Fatalf("move succeeded but delete error = %v, want ErrDirectoryNotEmpty", deleteErr)
+		}
+	case deleteErr == nil:
+		if !errors.Is(moveErr, model.ErrParentNotFound) {
+			t.Fatalf("delete succeeded but move error = %v, want ErrParentNotFound", moveErr)
+		}
+		file, err := repo.FindByID(ctx, "file-1")
+		if err != nil {
+			t.Fatalf("find source file: %v", err)
+		}
+		if file.ParentID != nil {
+			t.Fatalf("failed move changed source parent to %v", *file.ParentID)
+		}
+	default:
+		t.Fatalf("neither operation succeeded: move=%v delete=%v", moveErr, deleteErr)
+	}
+}