From 83edb6bb58cb8fa6b8bb4ae8c9d9457e45a2219e Mon Sep 17 00:00:00 2001 From: Abhishek Shroff Date: Wed, 11 Jun 2025 00:30:12 +0530 Subject: [PATCH] [server][core] Improve RestoreDeleted --- server/internal/api/v1/trash/routes.go | 2 +- server/internal/command/fs/trash/restore.go | 4 +- server/internal/core/core.go | 2 +- server/internal/core/resource_create.go | 62 +++++++++------ server/internal/core/resource_delete.go | 86 +++++---------------- 5 files changed, 63 insertions(+), 93 deletions(-) diff --git a/server/internal/api/v1/trash/routes.go b/server/internal/api/v1/trash/routes.go index 4f7ef141..cd6ec522 100644 --- a/server/internal/api/v1/trash/routes.go +++ b/server/internal/api/v1/trash/routes.go @@ -71,7 +71,7 @@ func handleRestoreRequest(c *gin.Context) { if err != nil { panic(err) } - r, _, _, err = fs.RestoreDeleted(r, target, name, autoRename) + r, err = fs.RestoreDeleted(r, target, name, autoRename) if err != nil { panic(err) } diff --git a/server/internal/command/fs/trash/restore.go b/server/internal/command/fs/trash/restore.go index 4a248295..6522739c 100644 --- a/server/internal/command/fs/trash/restore.go +++ b/server/internal/command/fs/trash/restore.go @@ -36,7 +36,7 @@ func setupRestoreCommand() *cobra.Command { p, _ := cmd.Flags().GetString("parent") name, _ := cmd.Flags().GetString("name") autoRename, _ := cmd.Flags().GetBool("auto-rename") - r, items, size, err := f.RestoreDeleted(r, p, name, autoRename) + r, err = f.RestoreDeleted(r, p, name, autoRename) if err != nil { fmt.Println("cannot restore '" + id.String() + "': " + err.Error()) os.Exit(1) @@ -47,7 +47,7 @@ func setupRestoreCommand() *cobra.Command { fmt.Println("cannot get path: " + err.Error()) os.Exit(1) } - fmt.Printf("Restored %s (%d items, %s)\n", path, items, common.FormatSize(int(size))) + fmt.Println("Restored " + path) } }, } diff --git a/server/internal/core/core.go b/server/internal/core/core.go index 9dfc3ff5..fae2e3a3 100644 --- a/server/internal/core/core.go +++ b/server/internal/core/core.go @@ -68,7 +68,7 @@ type FileSystem interface { // resource_delete.go Delete(r Resource) (Resource, error) DeleteForever(Resource) error - RestoreDeleted(r Resource, parentPathOrUUID string, name string, autoRename bool) (res Resource, count int, size int64, err error) + RestoreDeleted(r Resource, parentPathOrUUID string, name string, autoRename bool) (res Resource, err error) // trash.go TrashList(cursor string, n uint) ([]Resource, string, error) diff --git a/server/internal/core/resource_create.go b/server/internal/core/resource_create.go index 7737f048..21f8e554 100644 --- a/server/internal/core/resource_create.go +++ b/server/internal/core/resource_create.go @@ -162,30 +162,18 @@ func (f filesystem) createResource( err = ErrResourceNameConflict } case ResourceBindConflictResolutionRename: - ext := path.Ext(name) - basename := name[:len(name)-len(ext)] - counter := 1 - for { - name := fmt.Sprintf("%s (%d)%s", basename, counter, ext) - err = f.runInTx(func(f filesystem) error { - res, err = f.insertResource( - id, - parent, - name, - dir, - permissions, - ) - return err - }) - if err != nil { - if !strings.Contains(err.Error(), "unique_member_resource_name") { - return - } - counter++ - } else { - created = true - return - } + var name string + if name, err = f.detectNameConflict(parent, name, true); err != nil { + return + } else { + res, err = f.insertResource( + id, + parent, + name, + dir, + permissions, + ) + return } case ResourceBindConflictResolutionOverwrite: var rID uuid.UUID @@ -291,6 +279,32 @@ func (f filesystem) createResourceVersion(id, versionID uuid.UUID, size int64, m return err } +func (f filesystem) detectNameConflict(parentID uuid.UUID, name string, autoRename bool) (string, error) { + if _, _, err := f.childResourceIDByName(parentID, name); err != nil { + // No name conflict. Good to go! + if errors.Is(err, ErrResourceNotFound) { + return name, nil + } + return "", err + } else if !autoRename { + return "", ErrResourceNameConflict + } + + ext := path.Ext(name) + basename := name[:len(name)-len(ext)] + counter := 1 + for { + name = fmt.Sprintf("%s (%d)%s", basename, counter, ext) + if _, _, err := f.childResourceIDByName(parentID, name); err == nil { + counter++ + } else if errors.Is(err, ErrResourceNotFound) { + return name, nil + } else { + return "", err + } + } +} + // iteratorForCreateResources implements pgx.CopyFromSource. type iteratorForCreateResources struct { rows []CreateResourcesParams diff --git a/server/internal/core/resource_delete.go b/server/internal/core/resource_delete.go index d7ed089a..a38cd0dd 100644 --- a/server/internal/core/resource_delete.go +++ b/server/internal/core/resource_delete.go @@ -1,8 +1,6 @@ package core import ( - "fmt" - "path" "time" "codeberg.org/shroff/phylum/server/internal/db" @@ -156,16 +154,17 @@ func scanID(row pgx.CollectableRow) (uuid.UUID, error) { // - Parent must not be deleted // - Parent must have write permission // - No name conflict with exiting resource -func (f filesystem) RestoreDeleted(r Resource, parentPathOrUUID string, name string, autoRename bool) (res Resource, count int, size int64, err error) { - var p Resource +func (f filesystem) RestoreDeleted(r Resource, parentPathOrUUID string, name string, autoRename bool) (res Resource, err error) { + // Locate parent + var parent Resource if parentPathOrUUID == "" { if r.parentID.Valid { - p, err = f.ResourceByID(r.parentID.Bytes) + parent, err = f.ResourceByID(r.parentID.Bytes) } else { err = ErrResourceNotFound } } else { - p, err = f.ResourceByPathWithRoot(parentPathOrUUID) + parent, err = f.ResourceByPathWithRoot(parentPathOrUUID) } if err == ErrResourceNotFound { err = ErrParentNotFound @@ -174,11 +173,12 @@ func (f filesystem) RestoreDeleted(r Resource, parentPathOrUUID string, name str return } - if p.deleted.Valid { + // Make sure parent is not deleted and has write permissions + if parent.deleted.Valid { err = ErrParentDeleted return } - if !p.hasPermission(PermissionWrite) { + if !parent.hasPermission(PermissionWrite) { err = ErrInsufficientPermissions return } @@ -186,24 +186,7 @@ func (f filesystem) RestoreDeleted(r Resource, parentPathOrUUID string, name str if name == "" { name = r.name } - _, _, err = f.childResourceIDByName(p.id, name) - if autoRename && err == nil { - ext := path.Ext(name) - basename := name[:len(name)-len(ext)] - counter := 1 - for { - name = fmt.Sprintf("%s (%d)%s", basename, counter, ext) - if _, _, err = f.childResourceIDByName(p.id, name); err == nil { - counter++ - } else { - break - } - } - } - if err != ErrResourceNotFound { - if err == nil { - err = ErrResourceNameConflict - } + if name, err = f.detectNameConflict(parent.id, name, autoRename); err != nil { return } @@ -212,22 +195,25 @@ func (f filesystem) RestoreDeleted(r Resource, parentPathOrUUID string, name str if _, err := f.db.Exec(q, args...); err != nil { return err } - if p.id != r.parentID.Bytes || r.name != name { - if err := f.updateResourceNameParent(r.ID(), name, pgtype.UUID{Bytes: p.id, Valid: true}); err != nil { + + if parent.id != r.parentID.Bytes || r.name != name { + if err := f.updateResourceNameParent(r.ID(), name, pgtype.UUID{Bytes: parent.id, Valid: true}); err != nil { return err } else { r.name = name - r.parentID = pgtype.UUID{Bytes: p.id, Valid: true} + r.parentID = pgtype.UUID{Bytes: parent.id, Valid: true} r.visibleParent = r.parentID } } - if del, err := f.markNotDeleted(r.id); err != nil { + _, _, s := selectResourceTree(r.id, false, false, false) + query, params, _ := s.Update().Set( + goqu.Record{ + "modified": goqu.L("NOW()"), + "deleted": nil, + }).ToSQL() + + if _, err := f.db.Exec(query, params...); err != nil { return err - } else { - count = len(del) - for _, l := range del { - size += l - } } return f.recomputePermissions(r.id) @@ -236,33 +222,3 @@ func (f filesystem) RestoreDeleted(r Resource, parentPathOrUUID string, name str res = r return } - -func (f filesystem) markNotDeleted(id uuid.UUID) ([]int64, error) { - r, _, s := selectResourceTree(id, false, false, false) - q := s.Update().Set( - goqu.Record{ - "modified": goqu.L("NOW()"), - "deleted": nil, - }). - Returning(r.Col("content_length")) - - query, params, _ := q.ToSQL() - - rows, err := f.db.Query(query, params...) - if err != nil { - return nil, err - } - defer rows.Close() - var items []int64 - for rows.Next() { - var content_length int64 - if err := rows.Scan(&content_length); err != nil { - return nil, err - } - items = append(items, content_length) - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -}