Merge pull request #4897 from kobergj/RemainingQuota

Calculate remaining Quota correctly

changelog/unreleased/fix-activitylog-issues.md

@@ -0,0 +1,5 @@
+Enhancement: Fix remaining quota calculation
+
+Remaining quota should only be total - used and not take disk space into account.
+
+https://github.com/cs3org/reva/pull/4897

pkg/storage/fs/ocis/blobstore/blobstore.go

@@ -116,11 +116,6 @@ func (bs *Blobstore) Delete(node *node.Node) error {
 	return nil
 }
 
-// GetAvailableSize returns the available size in the blobstore in bytes
-func (bs *Blobstore) GetAvailableSize(n *node.Node) (uint64, error) {
-	return node.GetAvailableSize(n.InternalPath())
-}
-
 // List lists all blobs in the Blobstore
 func (bs *Blobstore) List() ([]*node.Node, error) {
 	dirs, err := filepath.Glob(filepath.Join(bs.root, "spaces", "*", "*", "blobs", "*", "*", "*", "*", "*"))

pkg/storage/fs/posix/blobstore/blobstore.go

@@ -88,8 +88,3 @@ func (bs *Blobstore) Download(node *node.Node) (io.ReadCloser, error) {
 func (bs *Blobstore) Delete(node *node.Node) error {
 	return nil
 }
-
-// GetAvailableSize returns the available size in the blobstore in bytes
-func (bs *Blobstore) GetAvailableSize(n *node.Node) (uint64, error) {
-	return node.GetAvailableSize(n.InternalPath())
-}

pkg/storage/fs/posix/posix.go

@@ -126,7 +126,6 @@ func New(m map[string]interface{}, stream events.Stream) (storage.FS, error) {
 	aspects := aspects.Aspects{
 		Lookup:            lu,
 		Tree:              tp,
-		Blobstore:         bs,
 		Permissions:       p,
 		EventStream:       stream,
 		UserMapper:        um,

pkg/storage/fs/posix/testhelpers/helpers.go

@@ -178,7 +178,6 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) {
 	)
 
 	bs := &treemocks.Blobstore{}
-	bs.On("GetAvailableSize", mock.Anything).Return(uint64(1000000000), nil)
 	tree, err := tree.New(lu, bs, um, &trashbin.Trashbin{}, o, nil, store.Create())
 	if err != nil {
 		return nil, err
@@ -190,7 +189,6 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) {
 	aspects := aspects.Aspects{
 		Lookup:      lu,
 		Tree:        tree,
-		Blobstore:   bs,
 		Permissions: permissions.NewPermissions(pmock, permissionsSelector),
 		Trashbin:    tb,
 	}

pkg/storage/fs/s3ng/blobstore/blobstore.go

@@ -29,7 +29,6 @@ import (
 
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/lookup"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node"
-	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/tree"
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
 	"github.com/pkg/errors"
@@ -129,12 +128,6 @@ func (bs *Blobstore) Delete(node *node.Node) error {
 	return nil
 }
 
-// GetAvailableSize returns the available size in the blobstore in bytes
-func (bs *Blobstore) GetAvailableSize(n *node.Node) (uint64, error) {
-	// S3 doen't have a concept of available size
-	return 0, tree.ErrSizeUnlimited
-}
-
 // List lists all blobs in the Blobstore
 func (bs *Blobstore) List() ([]*node.Node, error) {
 	ch := bs.client.ListObjects(context.Background(), bs.bucket, minio.ListObjectsOptions{Recursive: true})

pkg/storage/utils/decomposedfs/aspects/aspects.go

@@ -23,15 +23,13 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/permissions"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/trashbin"
-	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/tree"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/usermapper"
 )
 
 // Aspects holds dependencies for handling aspects of the decomposedfs
 type Aspects struct {
 	Lookup            node.PathLookup
 	Tree              node.Tree
-	Blobstore         tree.Blobstore
 	Trashbin          trashbin.Trashbin
 	Permissions       permissions.Permissions
 	EventStream       events.Stream

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -22,7 +22,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"math"
 	"net/url"
 	"path"
 	"path/filepath"
@@ -113,7 +112,6 @@ type SessionStore interface {
 type Decomposedfs struct {
 	lu           node.PathLookup
 	tp           node.Tree
-	bs           tree.Blobstore
 	trashbin     trashbin.Trashbin
 	o            *options.Options
 	p            permissions.Permissions
@@ -164,7 +162,6 @@ func NewDefault(m map[string]interface{}, bs tree.Blobstore, es events.Stream) (
 	aspects := aspects.Aspects{
 		Lookup:            lu,
 		Tree:              tp,
-		Blobstore:         bs,
 		Permissions:       permissions.NewPermissions(node.NewPermissions(lu), permissionsSelector),
 		EventStream:       es,
 		DisableVersioning: o.DisableVersioning,
@@ -226,7 +223,6 @@ func New(o *options.Options, aspects aspects.Aspects) (storage.FS, error) {
 
 	fs := &Decomposedfs{
 		tp:              aspects.Tree,
-		bs:              aspects.Blobstore,
 		lu:              aspects.Lookup,
 		trashbin:        aspects.Trashbin,
 		o:               o,
@@ -602,20 +598,12 @@ func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) (
 		quotaStr = string(ri.Opaque.Map["quota"].Value)
 	}
 
-	remaining, err = fs.bs.GetAvailableSize(n)
-	switch {
-	case errors.Is(err, tree.ErrSizeUnlimited):
-		remaining = math.MaxUint64
-	case err != nil:
-		return 0, 0, 0, err
-	}
-
-	return fs.calculateTotalUsedRemaining(quotaStr, ri.Size, remaining)
+	return fs.calculateTotalUsedRemaining(quotaStr, ri.Size)
 }
 
-func (fs *Decomposedfs) calculateTotalUsedRemaining(quotaStr string, inUse, remaining uint64) (uint64, uint64, uint64, error) {
+func (fs *Decomposedfs) calculateTotalUsedRemaining(quotaStr string, inUse uint64) (uint64, uint64, uint64, error) {
 	var err error
-	var total uint64
+	var total, remaining uint64
 	switch quotaStr {
 	case node.QuotaUncalculated, node.QuotaUnknown:
 		// best we can do is return current total
@@ -628,14 +616,10 @@ func (fs *Decomposedfs) calculateTotalUsedRemaining(quotaStr string, inUse, rema
 			return 0, 0, 0, err
 		}
 
-		if total <= remaining {
-			// Prevent overflowing
-			if inUse >= total {
-				remaining = 0
-			} else {
-				remaining = total - inUse
-			}
+		if total > inUse {
+			remaining = total - inUse
 		}
+
 	}
 	return total, inUse, remaining, nil
 }

pkg/storage/utils/decomposedfs/decomposedfs_test.go

@@ -58,7 +58,6 @@ var _ = Describe("Decomposed", func() {
 	Describe("NewDefault", func() {
 		It("works", func() {
 			bs := &treemocks.Blobstore{}
-			bs.On("GetAvailableSize", mock.Anything).Return(uint64(1000000000), nil)
 			_, err := decomposedfs.NewDefault(map[string]interface{}{
 				"root":           env.Root,
 				"permissionssvc": "any",

pkg/storage/utils/decomposedfs/spaces.go

@@ -46,7 +46,6 @@ import (
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/metadata/prefixes"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/node"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/permissions"
-	"github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/tree"
 	"github.com/cs3org/reva/v2/pkg/storage/utils/templates"
 	"github.com/cs3org/reva/v2/pkg/storagespace"
 	"github.com/cs3org/reva/v2/pkg/utils"
@@ -1030,22 +1029,13 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 		quotaStr = quotaInOpaque
 	}
 
-	remaining, err := fs.bs.GetAvailableSize(n)
-	switch {
-	case errors.Is(err, tree.ErrSizeUnlimited):
-		remaining = math.MaxUint64
-	case err != nil:
-		return nil, err
-	}
-	total, used, remaining, err := fs.calculateTotalUsedRemaining(quotaStr, space.GetRootInfo().GetSize(), remaining)
+	total, used, remaining, err := fs.calculateTotalUsedRemaining(quotaStr, space.GetRootInfo().GetSize())
 	if err != nil {
 		return nil, err
 	}
 	space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "quota.total", strconv.FormatUint(total, 10))
 	space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "quota.used", strconv.FormatUint(used, 10))
-	if remaining != math.MaxUint64 {
-		space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "quota.remaining", strconv.FormatUint(remaining, 10))
-	}
+	space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "quota.remaining", strconv.FormatUint(remaining, 10))
 
 	return space, nil
 }

pkg/storage/utils/decomposedfs/testhelpers/helpers.go

@@ -175,7 +175,6 @@ func NewTestEnv(config map[string]interface{}) (*TestEnv, error) {
 	aspects := aspects.Aspects{
 		Lookup:      lu,
 		Tree:        tree,
-		Blobstore:   bs,
 		Permissions: permissions.NewPermissions(pmock, permissionsSelector),
 		Trashbin:    &decomposedfs.DecomposedfsTrashbin{},
 	}
@@ -291,7 +290,6 @@ func (t *TestEnv) CreateTestStorageSpace(typ string, quota *providerv1beta1.Quot
 	if typ == "personal" {
 		owner = t.Owner
 	}
-	t.Blobstore.On("GetAvailableSize", mock.Anything).Return(uint64(1000000000), nil)
 	space, err := t.Fs.CreateStorageSpace(t.Ctx, &providerv1beta1.CreateStorageSpaceRequest{
 		Owner: owner,
 		Type:  typ,

pkg/storage/utils/decomposedfs/tree/tree.go

@@ -48,10 +48,7 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
-var (
-	tracer           trace.Tracer
-	ErrSizeUnlimited = errors.New("blobstore size unlimited")
-)
+var tracer trace.Tracer
 
 func init() {
 	tracer = otel.Tracer("github.com/cs3org/reva/pkg/storage/utils/decomposedfs/tree")
@@ -62,7 +59,6 @@ type Blobstore interface {
 	Upload(node *node.Node, source string) error
 	Download(node *node.Node) (io.ReadCloser, error)
 	Delete(node *node.Node) error
-	GetAvailableSize(node *node.Node) (uint64, error)
 }
 
 // Tree manages a hierarchical tree

pkg/storage/utils/decomposedfs/upload_async_test.go

@@ -153,7 +153,6 @@ var _ = Describe("Async file uploads", Ordered, func() {
 			},
 		)
 		bs = &treemocks.Blobstore{}
-		bs.On("GetAvailableSize", mock.Anything).Return(uint64(1000000000), nil)
 
 		// create space uses CheckPermission endpoint
 		cs3permissionsclient.On("CheckPermission", mock.Anything, mock.Anything, mock.Anything).Return(&cs3permissions.CheckPermissionResponse{
@@ -177,7 +176,6 @@ var _ = Describe("Async file uploads", Ordered, func() {
 		aspects := aspects.Aspects{
 			Lookup:      lu,
 			Tree:        tree,
-			Blobstore:   bs,
 			Permissions: permissions.NewPermissions(pmock, permissionsSelector),
 			EventStream: stream.Chan{pub, con},
 			Trashbin:    &DecomposedfsTrashbin{},

pkg/storage/utils/decomposedfs/upload_test.go

@@ -136,13 +136,11 @@ var _ = Describe("File uploads", func() {
 			AddGrant: true,
 		}, nil).Times(1)
 		var err error
-		bs.On("GetAvailableSize", mock.Anything).Return(uint64(1000000000), nil).Times(1)
 		tree := tree.New(lu, bs, o, store.Create())
 
 		aspects := aspects.Aspects{
 			Lookup:      lu,
 			Tree:        tree,
-			Blobstore:   bs,
 			Permissions: permissions.NewPermissions(pmock, permissionsSelector),
 			Trashbin:    &decomposedfs.DecomposedfsTrashbin{},
 		}