fix(server): upload assets outside transaction

reearth · Mar 14, 2023 · afc7e23 · afc7e23
1 parent 4b164b2
commit afc7e23
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 51 deletions.
diff --git a/server/internal/usecase/interactor/asset.go b/server/internal/usecase/interactor/asset.go
@@ -48,55 +48,53 @@ func (i *Asset) Create(ctx context.Context, inp interfaces.CreateAssetParam, ope
 	if inp.File == nil {
 		return nil, interfaces.ErrFileNotIncluded
 	}
-	return Run1(
-		ctx, operator, i.repos,
-		Usecase().
-			WithWritableWorkspaces(inp.WorkspaceID).
-			Transaction(),
-		func(ctx context.Context) (*asset.Asset, error) {
-			ws, err := i.repos.Workspace.FindByID(ctx, inp.WorkspaceID)
-			if err != nil {
-				return nil, err
-			}
 
-			url, size, err := i.gateways.File.UploadAsset(ctx, inp.File)
-			if err != nil {
-				return nil, err
-			}
+	ws, err := i.repos.Workspace.FindByID(ctx, inp.WorkspaceID)
+	if err != nil {
+		return nil, err
+	}
 
-			// enforce policy
-			if policyID := operator.Policy(ws.Policy()); policyID != nil {
-				p, err := i.repos.Policy.FindByID(ctx, *policyID)
-				if err != nil {
-					return nil, err
-				}
-				s, err := i.repos.Asset.TotalSizeByWorkspace(ctx, ws.ID())
-				if err != nil {
-					return nil, err
-				}
-				if err := p.EnforceAssetStorageSize(s + size); err != nil {
-					_ = i.gateways.File.RemoveAsset(ctx, url)
-					return nil, err
-				}
-			}
+	if !operator.IsWritableWorkspace(ws.ID()) {
+		return nil, interfaces.ErrOperationDenied
+	}
 
-			a, err := asset.New().
-				NewID().
-				Workspace(inp.WorkspaceID).
-				Name(path.Base(inp.File.Path)).
-				Size(size).
-				URL(url.String()).
-				Build()
-			if err != nil {
-				return nil, err
-			}
+	url, size, err := i.gateways.File.UploadAsset(ctx, inp.File)
+	if err != nil {
+		return nil, err
+	}
 
-			if err := i.repos.Asset.Save(ctx, a); err != nil {
-				return nil, err
-			}
+	// enforce policy
+	if policyID := operator.Policy(ws.Policy()); policyID != nil {
+		p, err := i.repos.Policy.FindByID(ctx, *policyID)
+		if err != nil {
+			return nil, err
+		}
+		s, err := i.repos.Asset.TotalSizeByWorkspace(ctx, ws.ID())
+		if err != nil {
+			return nil, err
+		}
+		if err := p.EnforceAssetStorageSize(s + size); err != nil {
+			_ = i.gateways.File.RemoveAsset(ctx, url)
+			return nil, err
+		}
+	}
+
+	a, err := asset.New().
+		NewID().
+		Workspace(inp.WorkspaceID).
+		Name(path.Base(inp.File.Path)).
+		Size(size).
+		URL(url.String()).
+		Build()
+	if err != nil {
+		return nil, err
+	}
+
+	if err := i.repos.Asset.Save(ctx, a); err != nil {
+		return nil, err
+	}
 
-			return a, nil
-		})
+	return a, nil
 }
 
 func (i *Asset) Remove(ctx context.Context, aid id.AssetID, operator *usecase.Operator) (result id.AssetID, err error) {

diff --git a/server/internal/usecase/interactor/asset_test.go b/server/internal/usecase/interactor/asset_test.go
@@ -16,7 +16,6 @@ import (
 	"github.com/reearth/reearth/server/pkg/file"
 	"github.com/reearth/reearth/server/pkg/id"
 	"github.com/reearth/reearth/server/pkg/workspace"
-	"github.com/reearth/reearthx/usecasex"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 )
@@ -30,13 +29,10 @@ func TestAsset_Create(t *testing.T) {
 
 	mfs := afero.NewMemMapFs()
 	f, _ := fs.NewFile(mfs, "")
-
-	transaction := &usecasex.NopTransaction{}
 	uc := &Asset{
 		repos: &repo.Container{
-			Asset:       memory.NewAsset(),
-			Workspace:   memory.NewWorkspaceWith(ws),
-			Transaction: transaction,
+			Asset:     memory.NewAsset(),
+			Workspace: memory.NewWorkspaceWith(ws),
 		},
 		gateways: &gateway.Container{
 			File: f,
@@ -70,7 +66,6 @@ func TestAsset_Create(t *testing.T) {
 
 	assert.NoError(t, err)
 	assert.Equal(t, want, res)
-	assert.True(t, transaction.IsCommitted())
 	a, _ := uc.repos.Asset.FindByID(ctx, aid)
 	assert.Equal(t, want, a)
 }