diff --git a/backend/internal/core/services/service_items_attachments.go b/backend/internal/core/services/service_items_attachments.go index 4085d90d..4ce0decc 100644 --- a/backend/internal/core/services/service_items_attachments.go +++ b/backend/internal/core/services/service_items_attachments.go @@ -50,6 +50,7 @@ func (svc *ItemService) AttachmentAdd(ctx Context, itemID uuid.UUID, filename st _, err = svc.repo.Attachments.Create(ctx, itemID, repo.ItemCreateAttachment{Title: filename, Content: file}, attachmentType, primary) if err != nil { log.Err(err).Msg("failed to create attachment") + return repo.ItemOut{}, err } return svc.repo.Items.GetOneByGroup(ctx, ctx.GID, itemID) diff --git a/backend/internal/core/services/service_items_attachments_test.go b/backend/internal/core/services/service_items_attachments_test.go index 16560fd3..8fb5d155 100644 --- a/backend/internal/core/services/service_items_attachments_test.go +++ b/backend/internal/core/services/service_items_attachments_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/sysadminsmedia/homebox/backend/internal/data/repo" + "github.com/sysadminsmedia/homebox/backend/internal/sys/config" ) func TestItemService_AddAttachment(t *testing.T) { @@ -60,3 +61,53 @@ func TestItemService_AddAttachment(t *testing.T) { require.NoError(t, err) assert.Equal(t, contents, string(bts)) } + +func TestItemService_AddAttachment_InvalidStorage(t *testing.T) { + // Create a service with an invalid storage path to simulate the issue + svc := &ItemService{ + repo: tRepos, + filepath: "/nonexistent/path/that/should/not/exist", + } + + // Create a temporary repo with invalid storage config + invalidRepos := repo.New(tClient, tbus, config.Storage{ + PrefixPath: "/", + ConnString: "file:///nonexistent/directory/that/does/not/exist", + }, "mem://{{ .Topic }}", config.Thumbnail{ + Enabled: false, + Width: 0, + Height: 0, + }) + + svc.repo = invalidRepos + + loc, err := invalidRepos.Locations.Create(context.Background(), tGroup.ID, repo.LocationCreate{ + Description: "test", + Name: "test-invalid", + }) + require.NoError(t, err) + assert.NotNil(t, loc) + + itmC := repo.ItemCreate{ + Name: fk.Str(10), + Description: fk.Str(10), + LocationID: loc.ID, + } + + itm, err := invalidRepos.Items.Create(context.Background(), tGroup.ID, itmC) + require.NoError(t, err) + assert.NotNil(t, itm) + t.Cleanup(func() { + err := invalidRepos.Items.Delete(context.Background(), itm.ID) + require.NoError(t, err) + }) + + contents := fk.Str(1000) + reader := strings.NewReader(contents) + + // Attempt to add attachment with invalid storage - should return an error + _, err = svc.AttachmentAdd(tCtx, itm.ID, "testfile.txt", "attachment", false, reader) + + // This should return an error now (after the fix) + assert.Error(t, err, "AttachmentAdd should return an error when storage is invalid") +} diff --git a/backend/internal/data/repo/repo_item_attachments.go b/backend/internal/data/repo/repo_item_attachments.go index eb6f8391..0cd2feed 100644 --- a/backend/internal/data/repo/repo_item_attachments.go +++ b/backend/internal/data/repo/repo_item_attachments.go @@ -218,9 +218,8 @@ func (r *AttachmentRepo) Create(ctx context.Context, itemID uuid.UUID, doc ItemC // Upload the file to the storage bucket path, err := r.UploadFile(ctx, itemGroup, doc) if err != nil { - err := tx.Rollback() - if err != nil { - return nil, err + if rollbackErr := tx.Rollback(); rollbackErr != nil { + return nil, rollbackErr } return nil, err }