diff --git a/apps/backend/internal/catalog/http_integration_test.go b/apps/backend/internal/catalog/http_integration_test.go index d226cb5..e5ecbf9 100644 --- a/apps/backend/internal/catalog/http_integration_test.go +++ b/apps/backend/internal/catalog/http_integration_test.go @@ -301,8 +301,31 @@ func TestHTTP_CatalogEndpoints(t *testing.T) { } }) - // Note: Directory rename test skipped due to VFS bug where it tries to - // resolve blob keys for directories which don't have blobs. + t.Run("rename directory", func(t *testing.T) { + root, err := env.vfs.FindNode(ctx, env.db, env.rootDirID.String(), env.scope) + if err != nil { + t.Fatalf("FindNode root: %v", err) + } + renameDir, err := env.vfs.CreateDirectory(ctx, env.db, root.ID, "to-rename", env.scope) + if err != nil { + t.Fatalf("CreateDirectory to-rename: %v", err) + } + + var resp struct { + Kind string `json:"kind"` + ID string `json:"id"` + Name string `json:"name"` + } + body := map[string]string{"name": "renamed-dir"} + doJSONAuth(t, env.app, http.MethodPatch, driveURL(fmt.Sprintf("/directories/%s", renameDir.PublicID)), env.accessToken, body, http.StatusOK, &resp) + + if resp.Kind != "directory" { + t.Errorf("expected kind=directory, got %q", resp.Kind) + } + if resp.Name != "renamed-dir" { + t.Errorf("expected name=renamed-dir, got %q", resp.Name) + } + }) // File tests t.Run("fetch file by ID", func(t *testing.T) { diff --git a/apps/backend/internal/virtualfs/flat_key_resolver.go b/apps/backend/internal/virtualfs/flat_key_resolver.go index 8da128c..5ffdb2c 100644 --- a/apps/backend/internal/virtualfs/flat_key_resolver.go +++ b/apps/backend/internal/virtualfs/flat_key_resolver.go @@ -45,3 +45,9 @@ func (r *FlatKeyResolver) ResolveBulkMoveOps(ctx context.Context, db bun.IDB, no } return nil, nil } + +// ResolveRenameOp returns nil for flat key storage since blob keys are UUIDs +// and don't change when nodes are renamed. +func (r *FlatKeyResolver) ResolveRenameOp(ctx context.Context, db bun.IDB, node *Node, newName string) (*BlobMoveOp, error) { + return nil, nil +} diff --git a/apps/backend/internal/virtualfs/hierarchical_key_resolver.go b/apps/backend/internal/virtualfs/hierarchical_key_resolver.go index 8f3f1b8..87dcd23 100644 --- a/apps/backend/internal/virtualfs/hierarchical_key_resolver.go +++ b/apps/backend/internal/virtualfs/hierarchical_key_resolver.go @@ -77,3 +77,34 @@ func (r *HierarchicalKeyResolver) ResolveBulkMoveOps(ctx context.Context, db bun return ops, nil } + +// ResolveRenameOp returns the blob move operation for renaming a node. +// For hierarchical storage, both files and directories need blob moves since +// the key is path-based. os.Rename handles directory moves atomically. +func (r *HierarchicalKeyResolver) ResolveRenameOp(ctx context.Context, db bun.IDB, node *Node, newName string) (*BlobMoveOp, error) { + oldPath, err := buildNodeAbsolutePathString(ctx, db, node) + if err != nil { + return nil, err + } + + parentPath, err := buildPathFromNodeID(ctx, db, node.ParentID) + if err != nil { + return nil, err + } + + var newPath string + if parentPath == "" { + newPath = newName + } else { + newPath = fmt.Sprintf("%s/%s", parentPath, newName) + } + + oldKey := blob.Key(fmt.Sprintf("%s/%s", node.DriveID, oldPath)) + newKey := blob.Key(fmt.Sprintf("%s/%s", node.DriveID, newPath)) + + return &BlobMoveOp{ + Node: node, + OldKey: oldKey, + NewKey: newKey, + }, nil +} diff --git a/apps/backend/internal/virtualfs/key_resolver.go b/apps/backend/internal/virtualfs/key_resolver.go index bac15cb..b0235b7 100644 --- a/apps/backend/internal/virtualfs/key_resolver.go +++ b/apps/backend/internal/virtualfs/key_resolver.go @@ -20,6 +20,12 @@ type BlobKeyResolver interface { // Returns ErrBulkMoveRequiresSameParent if nodes don't all share the same parent. // Returns nil, nil if no blob moves are needed (e.g., flat key storage where keys are UUIDs). ResolveBulkMoveOps(ctx context.Context, db bun.IDB, nodes []*Node, newParentID uuid.UUID) ([]BlobMoveOp, error) + + // ResolveRenameOp returns the blob move operation needed when renaming a node. + // Returns nil, nil if no blob move is needed (e.g., flat key storage where keys are UUIDs). + // For hierarchical storage, returns a single move op for both files and directories + // (os.Rename handles directories atomically). + ResolveRenameOp(ctx context.Context, db bun.IDB, node *Node, newName string) (*BlobMoveOp, error) } type DeletionPlan struct { diff --git a/apps/backend/internal/virtualfs/vfs.go b/apps/backend/internal/virtualfs/vfs.go index 64dc814..1dc68b3 100644 --- a/apps/backend/internal/virtualfs/vfs.go +++ b/apps/backend/internal/virtualfs/vfs.go @@ -598,7 +598,8 @@ func (vfs *VirtualFS) RenameNode(ctx context.Context, db bun.IDB, node *Node, na return ErrAccessDenied } - oldKey, err := vfs.keyResolver.Resolve(ctx, db, node) + // Resolve rename operation before updating DB (needs old path) + renameOp, err := vfs.keyResolver.ResolveRenameOp(ctx, db, node, name) if err != nil { return err } @@ -617,22 +618,18 @@ func (vfs *VirtualFS) RenameNode(ctx context.Context, db bun.IDB, node *Node, na return err } - newKey, err := vfs.keyResolver.Resolve(ctx, db, node) - if err != nil { - return err - } - - if oldKey != newKey { - err = vfs.blobStore.Move(ctx, oldKey, newKey) + // Execute blob move if needed + if renameOp != nil { + err = vfs.blobStore.Move(ctx, renameOp.OldKey, renameOp.NewKey) if err != nil { return err } if vfs.keyResolver.ShouldPersistKey() { - node.BlobKey = newKey + node.BlobKey = renameOp.NewKey _, err = db.NewUpdate().Model(node). WherePK(). - Set("blob_key = ?", newKey). + Set("blob_key = ?", renameOp.NewKey). Exec(ctx) if err != nil { return err