improve GetQuota behavior in decomposedfs (#2666)

* improve GetQuota behavior in decomposedfs

* use 0 as a way to represent unlimited quota

changelog/unreleased/quota-improvements.md

@@ -0,0 +1,7 @@
+Change: Improve quota handling
+
+GetQuota now returns 0 when no quota was set instead of the disk size.
+Also added a new return value for the remaining space which will either be quota - used bytes or if no quota was set the free disk size.
+
+https://github.com/owncloud/ocis/issues/3233
+https://github.com/cs3org/reva/pull/2666

internal/grpc/services/storageprovider/storageprovider.go

@@ -29,6 +29,7 @@ import (
 
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
+	typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
@@ -1044,7 +1045,7 @@ func (s *service) CreateSymlink(ctx context.Context, req *provider.CreateSymlink
 }
 
 func (s *service) GetQuota(ctx context.Context, req *provider.GetQuotaRequest) (*provider.GetQuotaResponse, error) {
-	total, used, err := s.storage.GetQuota(ctx, req.Ref)
+	total, used, remaining, err := s.storage.GetQuota(ctx, req.Ref)
 	if err != nil {
 		var st *rpc.Status
 		switch err.(type) {
@@ -1067,6 +1068,14 @@ func (s *service) GetQuota(ctx context.Context, req *provider.GetQuotaRequest) (
 	}
 
 	res := &provider.GetQuotaResponse{
+		Opaque: &typesv1beta1.Opaque{
+			Map: map[string]*typesv1beta1.OpaqueEntry{
+				"remaining": {
+					Decoder: "plain",
+					Value:   []byte(strconv.FormatUint(remaining, 10)),
+				},
+			},
+		},
 		Status:     status.NewOK(ctx),
 		TotalBytes: total,
 		UsedBytes:  used,

pkg/storage/fs/nextcloud/nextcloud.go

@@ -692,21 +692,25 @@ func (nc *StorageDriver) ListGrants(ctx context.Context, ref *provider.Reference
 }
 
 // GetQuota as defined in the storage.FS interface
-func (nc *StorageDriver) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
+func (nc *StorageDriver) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
 	log := appctx.GetLogger(ctx)
 	log.Info().Msg("GetQuota")
 
 	_, respBody, err := nc.do(ctx, Action{"GetQuota", ""})
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 
 	var respMap map[string]interface{}
 	err = json.Unmarshal(respBody, &respMap)
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
-	return uint64(respMap["totalBytes"].(float64)), uint64(respMap["usedBytes"].(float64)), err
+
+	total := uint64(respMap["totalBytes"].(float64))
+	used := uint64(respMap["usedBytes"].(float64))
+	remaining := total - used
+	return total, used, remaining, err
 }
 
 // CreateReference as defined in the storage.FS interface

pkg/storage/fs/nextcloud/nextcloud_test.go

@@ -875,10 +875,11 @@ var _ = Describe("Nextcloud", func() {
 		It("calls the GetQuota endpoint", func() {
 			nc, called, teardown := setUpNextcloudServer()
 			defer teardown()
-			maxBytes, maxFiles, err := nc.GetQuota(ctx, nil)
+			maxBytes, maxFiles, remaining, err := nc.GetQuota(ctx, nil)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(maxBytes).To(Equal(uint64(456)))
 			Expect(maxFiles).To(Equal(uint64(123)))
+			Expect(remaining).To(Equal(uint64(333)))
 			checkCalled(called, `POST /apps/sciencemesh/~tester/api/storage/GetQuota `)
 		})
 	})

pkg/storage/fs/owncloudsql/owncloudsql_unix.go

@@ -69,16 +69,17 @@ func calcEtag(ctx context.Context, fi os.FileInfo) string {
 	return strings.Trim(etag, "\"")
 }
 
-func (fs *owncloudsqlfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
+func (fs *owncloudsqlfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
 	// TODO quota of which storage space?
 	// we could use the logged in user, but when a user has access to multiple storages this falls short
 	// for now return quota of root
 	stat := syscall.Statfs_t{}
 	err := syscall.Statfs(fs.toInternalPath(ctx, "/"), &stat)
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 	total := stat.Blocks * uint64(stat.Bsize)                // Total data blocks in filesystem
 	used := (stat.Blocks - stat.Bavail) * uint64(stat.Bsize) // Free blocks available to unprivileged user
-	return total, used, nil
+	remaining := total - used
+	return total, used, remaining, nil
 }

pkg/storage/fs/owncloudsql/owncloudsql_windows.go

@@ -56,7 +56,7 @@ func calcEtag(ctx context.Context, fi os.FileInfo) string {
 	return fmt.Sprintf("\"%s\"", strings.Trim(etag, "\""))
 }
 
-func (fs *owncloudsqlfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
+func (fs *owncloudsqlfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
 	// TODO quota of which storage space?
 	// we could use the logged in user, but when a user has access to multiple storages this falls short
 	// for now return quota of root
@@ -72,5 +72,5 @@ func (fs *owncloudsqlfs) GetQuota(ctx context.Context, ref *provider.Reference)
 	}
 
 	used := total - free
-	return total, used, nil
+	return total, used, free, nil
 }

pkg/storage/fs/s3/s3.go

@@ -268,8 +268,8 @@ func (fs *s3FS) UpdateGrant(ctx context.Context, ref *provider.Reference, g *pro
 	return errtypes.NotSupported("s3: operation not supported")
 }
 
-func (fs *s3FS) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
-	return 0, 0, nil
+func (fs *s3FS) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
+	return 0, 0, 0, nil
 }
 
 func (fs *s3FS) SetArbitraryMetadata(ctx context.Context, ref *provider.Reference, md *provider.ArbitraryMetadata) error {

pkg/storage/storage.go

@@ -53,7 +53,7 @@ type FS interface {
 	RemoveGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error
 	UpdateGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error
 	ListGrants(ctx context.Context, ref *provider.Reference) ([]*provider.Grant, error)
-	GetQuota(ctx context.Context, ref *provider.Reference) ( /*TotalBytes*/ uint64 /*UsedBytes*/, uint64, error)
+	GetQuota(ctx context.Context, ref *provider.Reference) ( /*TotalBytes*/ uint64 /*UsedBytes*/, uint64 /*RemainingBytes*/, uint64, error)
 	CreateReference(ctx context.Context, path string, targetURI *url.URL) error
 	Shutdown(ctx context.Context) error
 	SetArbitraryMetadata(ctx context.Context, ref *provider.Reference, md *provider.ArbitraryMetadata) error

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -148,58 +148,67 @@ func (fs *Decomposedfs) Shutdown(ctx context.Context) error {
 
 // GetQuota returns the quota available
 // TODO Document in the cs3 should we return quota or free space?
-func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) (total uint64, inUse uint64, err error) {
+func (fs *Decomposedfs) GetQuota(ctx context.Context, ref *provider.Reference) (total uint64, inUse uint64, remaining uint64, err error) {
 	var n *node.Node
 	if ref == nil {
 		err = errtypes.BadRequest("no space given")
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 	if n, err = fs.lu.NodeFromResource(ctx, ref); err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 
 	if !n.Exists {
 		err = errtypes.NotFound(filepath.Join(n.ParentID, n.Name))
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 
 	rp, err := fs.p.AssemblePermissions(ctx, n)
 	switch {
 	case err != nil:
-		return 0, 0, errtypes.InternalError(err.Error())
+		return 0, 0, 0, errtypes.InternalError(err.Error())
 	case !rp.GetQuota:
-		return 0, 0, errtypes.PermissionDenied(n.ID)
+		return 0, 0, 0, errtypes.PermissionDenied(n.ID)
 	}
 
 	ri, err := n.AsResourceInfo(ctx, &rp, []string{"treesize", "quota"}, true)
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 
 	quotaStr := node.QuotaUnknown
 	if ri.Opaque != nil && ri.Opaque.Map != nil && ri.Opaque.Map["quota"] != nil && ri.Opaque.Map["quota"].Decoder == "plain" {
 		quotaStr = string(ri.Opaque.Map["quota"].Value)
 	}
 
-	avail, err := node.GetAvailableSize(n.InternalPath())
+	remaining, err = node.GetAvailableSize(n.InternalPath())
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
-	total = avail + ri.Size
 
-	switch {
-	case quotaStr == node.QuotaUncalculated, quotaStr == node.QuotaUnknown, quotaStr == node.QuotaUnlimited:
-	// best we can do is return current total
-	// TODO indicate unlimited total? -> in opaque data?
+	switch quotaStr {
+	case node.QuotaUncalculated, node.QuotaUnknown:
+		// best we can do is return current total
+		// TODO indicate unlimited total? -> in opaque data?
+	case node.QuotaUnlimited:
+		total = 0
 	default:
-		if quota, err := strconv.ParseUint(quotaStr, 10, 64); err == nil {
-			if total > quota {
-				total = quota
+		total, err = strconv.ParseUint(quotaStr, 10, 64)
+		if err != nil {
+			return 0, 0, 0, err
+		}
+
+		if total <= remaining {
+			// Prevent overflowing
+			if ri.Size >= total {
+				remaining = 0
+			} else {
+				remaining = total - ri.Size
 			}
 		}
 	}
 
-	return total, ri.Size, nil
+	return total, ri.Size, remaining, nil
 }
 
 // CreateHome creates a new home node for the given user

pkg/storage/utils/decomposedfs/node/node.go

@@ -58,9 +58,9 @@ const (
 	UserShareType    = "0"
 	QuotaKey         = "quota"
 
+	QuotaUnlimited    = "0"
 	QuotaUncalculated = "-1"
 	QuotaUnknown      = "-2"
-	QuotaUnlimited    = "-3"
 
 	// TrashIDDelimiter represents the characters used to separate the nodeid and the deletion time.
 	TrashIDDelimiter    = ".T."

pkg/storage/utils/decomposedfs/recycle_test.go

@@ -79,7 +79,7 @@ var _ = Describe("Recycle", func() {
 			})
 
 			It("they do not count towards the quota anymore", func() {
-				_, used, err := env.Fs.GetQuota(env.Ctx, &provider.Reference{ResourceId: env.SpaceRootRes})
+				_, used, _, err := env.Fs.GetQuota(env.Ctx, &provider.Reference{ResourceId: env.SpaceRootRes})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(used).To(Equal(uint64(0)))
 			})
@@ -296,10 +296,11 @@ var _ = Describe("Recycle", func() {
 				err = env.Fs.RestoreRecycleItem(env.Ctx, &provider.Reference{ResourceId: projectID}, items[0].Key, "/", nil)
 				Expect(err).ToNot(HaveOccurred())
 
-				max, used, err := env.Fs.GetQuota(env.Ctx, &provider.Reference{ResourceId: projectID})
+				max, used, remaining, err := env.Fs.GetQuota(env.Ctx, &provider.Reference{ResourceId: projectID})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(max).To(Equal(uint64(2000)))
 				Expect(used).To(Equal(uint64(3234)))
+				Expect(remaining).To(Equal(uint64(0)))
 			})
 
 		})

pkg/storage/utils/eosfs/eosfs.go

@@ -1029,29 +1029,31 @@ func (fs *eosfs) CreateStorageSpace(ctx context.Context, req *provider.CreateSto
 	return nil, errtypes.NotSupported("unimplemented: CreateStorageSpace")
 }
 
-func (fs *eosfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
+func (fs *eosfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
 	u, err := getUser(ctx)
 	if err != nil {
-		return 0, 0, errors.Wrap(err, "eosfs: no user in ctx")
+		return 0, 0, 0, errors.Wrap(err, "eosfs: no user in ctx")
 	}
 	// lightweight accounts don't have quota nodes, so we're passing an empty string as path
 	auth, err := fs.getUserAuth(ctx, u, "")
 	if err != nil {
-		return 0, 0, errors.Wrap(err, "eosfs: error getting uid and gid for user")
+		return 0, 0, 0, errors.Wrap(err, "eosfs: error getting uid and gid for user")
 	}
 
 	rootAuth, err := fs.getRootAuth(ctx)
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 
 	qi, err := fs.c.GetQuota(ctx, auth.Role.UID, rootAuth, fs.conf.QuotaNode)
 	if err != nil {
 		err := errors.Wrap(err, "eosfs: error getting quota")
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 
-	return qi.AvailableBytes, qi.UsedBytes, nil
+	remaining := qi.AvailableBytes - qi.UsedBytes
+
+	return qi.AvailableBytes, qi.UsedBytes, remaining, nil
 }
 
 func (fs *eosfs) GetHome(ctx context.Context) (string, error) {

pkg/storage/utils/localfs/localfs_unix.go

@@ -67,16 +67,17 @@ func calcEtag(ctx context.Context, fi os.FileInfo) string {
 	return fmt.Sprintf("\"%s\"", strings.Trim(etag, "\""))
 }
 
-func (fs *localfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
+func (fs *localfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
 	// TODO quota of which storage space?
 	// we could use the logged in user, but when a user has access to multiple storages this falls short
 	// for now return quota of root
 	stat := syscall.Statfs_t{}
 	err := syscall.Statfs(fs.wrap(ctx, "/"), &stat)
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 	total := stat.Blocks * uint64(stat.Bsize)                // Total data blocks in filesystem
 	used := (stat.Blocks - stat.Bavail) * uint64(stat.Bsize) // Free blocks available to unprivileged user
-	return total, used, nil
+	remaining := stat.Bavail * uint64(stat.Bsize)
+	return total, used, remaining, nil
 }

pkg/storage/utils/localfs/localfs_windows.go

@@ -56,7 +56,7 @@ func calcEtag(ctx context.Context, fi os.FileInfo) string {
 	return fmt.Sprintf("\"%s\"", strings.Trim(etag, "\""))
 }
 
-func (fs *localfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, error) {
+func (fs *localfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint64, uint64, uint64, error) {
 	// TODO quota of which storage space?
 	// we could use the logged in user, but when a user has access to multiple storages this falls short
 	// for now return quota of root
@@ -72,5 +72,5 @@ func (fs *localfs) GetQuota(ctx context.Context, ref *provider.Reference) (uint6
 	}
 
 	used := total - free
-	return total, used, nil
+	return total, used, free, nil
 }