From a2e108eac496c63d926100173b7ab45567459858 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Sep 2025 11:12:51 -0400 Subject: [PATCH] Make attachment storage paths relative in database with cross-platform support (#967) --- .../handlers/v1/v1_ctrl_items_attachments.go | 2 +- backend/go.sum | 1 - .../service_items_attachments_test.go | 4 +-- ...6000000_make_attachment_paths_relative.sql | 25 +++++++++++++++++++ ...6000000_make_attachment_paths_relative.sql | 25 +++++++++++++++++++ .../data/repo/repo_item_attachments.go | 23 +++++++++++------ 6 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 backend/internal/data/migrations/postgres/20250826000000_make_attachment_paths_relative.sql create mode 100644 backend/internal/data/migrations/sqlite3/20250826000000_make_attachment_paths_relative.sql diff --git a/backend/app/api/handlers/v1/v1_ctrl_items_attachments.go b/backend/app/api/handlers/v1/v1_ctrl_items_attachments.go index 27da77ba..e862f2cc 100644 --- a/backend/app/api/handlers/v1/v1_ctrl_items_attachments.go +++ b/backend/app/api/handlers/v1/v1_ctrl_items_attachments.go @@ -186,7 +186,7 @@ func (ctrl *V1Controller) handleItemAttachmentsHandler(w http.ResponseWriter, r log.Err(err).Msg("failed to open bucket") return validate.NewRequestError(err, http.StatusInternalServerError) } - file, err := bucket.NewReader(ctx, doc.Path, nil) + file, err := bucket.NewReader(ctx, ctrl.repo.Attachments.GetFullPath(doc.Path), nil) if err != nil { log.Err(err).Msg("failed to open file") return validate.NewRequestError(err, http.StatusInternalServerError) diff --git a/backend/go.sum b/backend/go.sum index b24c7400..fe5062e6 100644 --- a/backend/go.sum +++ b/backend/go.sum @@ -337,7 +337,6 @@ github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw= github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= github.com/ncruces/go-strftime v0.1.9 h1:bY0MQC28UADQmHmaF5dgpLmImcShSi2kHU9XLdhx/f4= github.com/ncruces/go-strftime v0.1.9/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls= -github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/olahol/melody v1.3.0 h1:n7UlKiQnxVrgxKoM0d7usZiN+Z0y2lVENtYLgKtXS6s= github.com/olahol/melody v1.3.0/go.mod h1:GgkTl6Y7yWj/HtfD48Q5vLKPVoZOH+Qqgfa7CvJgJM4= github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= diff --git a/backend/internal/core/services/service_items_attachments_test.go b/backend/internal/core/services/service_items_attachments_test.go index 7e15fac9..16560fd3 100644 --- a/backend/internal/core/services/service_items_attachments_test.go +++ b/backend/internal/core/services/service_items_attachments_test.go @@ -52,8 +52,8 @@ func TestItemService_AddAttachment(t *testing.T) { // Check that the file exists storedPath := afterAttachment.Attachments[0].Path - // {root}/{group}/{item}/{attachment} - assert.Equal(t, path.Join("/", tGroup.ID.String(), "documents"), path.Dir(storedPath)) + // path should now be relative: {group}/{documents} + assert.Equal(t, path.Join(tGroup.ID.String(), "documents"), path.Dir(storedPath)) // Check that the file contents are correct bts, err := os.ReadFile(path.Join(os.TempDir(), storedPath)) diff --git a/backend/internal/data/migrations/postgres/20250826000000_make_attachment_paths_relative.sql b/backend/internal/data/migrations/postgres/20250826000000_make_attachment_paths_relative.sql new file mode 100644 index 00000000..58221abc --- /dev/null +++ b/backend/internal/data/migrations/postgres/20250826000000_make_attachment_paths_relative.sql @@ -0,0 +1,25 @@ +-- +goose Up +-- Make attachment paths relative by removing the prefix path +-- This migration converts absolute paths to relative paths by finding the UUID/documents pattern + +-- Update Unix-style paths that contain "/documents/" by extracting the part starting from the UUID +-- The approach: find the "/documents/" substring, go back 37 characters (UUID + slash), +-- and extract from there to get "uuid/documents/hash" +UPDATE attachments +SET path = SUBSTRING(path FROM POSITION('/documents/' IN path) - 36) +WHERE path LIKE '%/documents/%' + AND POSITION('/documents/' IN path) > 36; + +-- Update Windows-style paths that contain "\documents\" by extracting the part starting from the UUID +-- Convert backslashes to forward slashes in the process for consistency +UPDATE attachments +SET path = REPLACE(SUBSTRING(path FROM POSITION('\documents\' IN path) - 36), '\', '/') +WHERE path LIKE '%\documents\%' + AND POSITION('\documents\' IN path) > 36; + +-- For paths that already look like relative paths (start with UUID), leave them unchanged +-- This handles cases where the migration might be run multiple times + +-- +goose Down +-- Note: This down migration cannot be safely implemented because we don't know +-- what the original prefix paths were. This is a one-way migration. \ No newline at end of file diff --git a/backend/internal/data/migrations/sqlite3/20250826000000_make_attachment_paths_relative.sql b/backend/internal/data/migrations/sqlite3/20250826000000_make_attachment_paths_relative.sql new file mode 100644 index 00000000..5c3c515f --- /dev/null +++ b/backend/internal/data/migrations/sqlite3/20250826000000_make_attachment_paths_relative.sql @@ -0,0 +1,25 @@ +-- +goose Up +-- Make attachment paths relative by removing the prefix path +-- This migration converts absolute paths to relative paths by finding the UUID/documents pattern + +-- Update Unix-style paths that contain "/documents/" by extracting the part starting from the UUID +-- The approach: find the "/documents/" substring, go back 37 characters (UUID + slash), +-- and extract from there to get "uuid/documents/hash" +UPDATE attachments +SET path = SUBSTR(path, INSTR(path, '/documents/') - 36) +WHERE path LIKE '%/documents/%' + AND INSTR(path, '/documents/') > 36; + +-- Update Windows-style paths that contain "\documents\" by extracting the part starting from the UUID +-- Convert backslashes to forward slashes in the process for consistency +UPDATE attachments +SET path = REPLACE(SUBSTR(path, INSTR(path, '\documents\') - 36), '\', '/') +WHERE path LIKE '%\documents\%' + AND INSTR(path, '\documents\') > 36; + +-- For paths that already look like relative paths (start with UUID), leave them unchanged +-- This handles cases where the migration might be run multiple times + +-- +goose Down +-- Note: This down migration cannot be safely implemented because we don't know +-- what the original prefix paths were. This is a one-way migration. \ No newline at end of file diff --git a/backend/internal/data/repo/repo_item_attachments.go b/backend/internal/data/repo/repo_item_attachments.go index 362c7f2c..554baee4 100644 --- a/backend/internal/data/repo/repo_item_attachments.go +++ b/backend/internal/data/repo/repo_item_attachments.go @@ -98,7 +98,15 @@ func ToItemAttachment(attachment *ent.Attachment) ItemAttachment { } func (r *AttachmentRepo) path(gid uuid.UUID, hash string) string { - return filepath.Join(r.storage.PrefixPath, gid.String(), "documents", hash) + return filepath.Join(gid.String(), "documents", hash) +} + +func (r *AttachmentRepo) fullPath(relativePath string) string { + return filepath.Join(r.storage.PrefixPath, relativePath) +} + +func (r *AttachmentRepo) GetFullPath(relativePath string) string { + return r.fullPath(relativePath) } func (r *AttachmentRepo) GetConnString() string { @@ -385,7 +393,7 @@ func (r *AttachmentRepo) Delete(ctx context.Context, gid uuid.UUID, itemId uuid. log.Err(err).Msg("failed to open bucket for thumbnail deletion") return err } - err = thumbBucket.Delete(ctx, thumb.Path) + err = thumbBucket.Delete(ctx, r.fullPath(thumb.Path)) if err != nil { return err } @@ -407,7 +415,7 @@ func (r *AttachmentRepo) Delete(ctx context.Context, gid uuid.UUID, itemId uuid. log.Err(err).Msg("failed to close bucket") } }(bucket) - err = bucket.Delete(ctx, doc.Path) + err = bucket.Delete(ctx, r.fullPath(doc.Path)) if err != nil { return err } @@ -475,7 +483,7 @@ func (r *AttachmentRepo) CreateThumbnail(ctx context.Context, groupId, attachmen } }(bucket) - origFile, err := bucket.Open(path) + origFile, err := bucket.Open(r.fullPath(path)) if err != nil { err := tx.Rollback() if err != nil { @@ -785,14 +793,15 @@ func (r *AttachmentRepo) UploadFile(ctx context.Context, itemGroup *ent.Group, d ContentType: contentType, ContentMD5: md5hash.Sum(nil), } - path := r.path(itemGroup.ID, fmt.Sprintf("%x", hashOut)) - err = bucket.WriteAll(ctx, path, contentBytes, options) + relativePath := r.path(itemGroup.ID, fmt.Sprintf("%x", hashOut)) + fullPath := r.fullPath(relativePath) + err = bucket.WriteAll(ctx, fullPath, contentBytes, options) if err != nil { log.Err(err).Msg("failed to write file to bucket") return "", err } - return path, nil + return relativePath, nil } func isImageFile(mimetype string) bool {