Skip to content

Commit

Permalink
Create dedicated type for node attributes, based on map[string][]byte
Browse files Browse the repository at this point in the history
  • Loading branch information
aduffeck committed Mar 13, 2023
1 parent 719e536 commit 371c1d7
Show file tree
Hide file tree
Showing 20 changed files with 291 additions and 248 deletions.
2 changes: 1 addition & 1 deletion pkg/storage/utils/decomposedfs/decomposedfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference)

if fs.o.TreeTimeAccounting || fs.o.TreeSizeAccounting {
// mark the home node as the end of propagation
if err = n.SetXattr(prefixes.PropagationAttr, "1"); err != nil {
if err = n.SetXattrString(prefixes.PropagationAttr, "1"); err != nil {
appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not mark node to propagate")

// FIXME: This does not return an error at all, but results in a severe situation that the
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/utils/decomposedfs/grants.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (fs *Decomposedfs) ListGrants(ctx context.Context, ref *provider.Reference)
return nil, errtypes.NotFound(f)
}
log := appctx.GetLogger(ctx)
var attrs map[string]string
var attrs node.Attributes
if attrs, err = grantNode.Xattrs(); err != nil {
log.Error().Err(err).Msg("error listing attributes")
return nil, err
Expand Down Expand Up @@ -312,7 +312,7 @@ func (fs *Decomposedfs) storeGrant(ctx context.Context, n *node.Node, g *provide
// set the grant
e := ace.FromGrant(g)
principal, value := e.Marshal()
if err := n.SetXattr(prefixes.GrantPrefix+principal, string(value)); err != nil {
if err := n.SetXattr(prefixes.GrantPrefix+principal, value); err != nil {
appctx.GetLogger(ctx).Error().Err(err).
Str("principal", principal).Msg("Could not set grant for principal")
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/utils/decomposedfs/grants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ var _ = Describe("Grants", func() {
Path: "/dir1",
})
Expect(err).ToNot(HaveOccurred())
attr, err := n.Xattr(prefixes.GrantUserAcePrefix + grant.Grantee.GetUserId().OpaqueId)
attr, err := n.XattrString(prefixes.GrantUserAcePrefix + grant.Grantee.GetUserId().OpaqueId)
Expect(err).ToNot(HaveOccurred())
Expect(attr).To(Equal(fmt.Sprintf("\x00t=A:f=:p=rw:c=%s:e=0\n", o.GetOpaqueId()))) // NOTE: this tests ace package
})
Expand Down
21 changes: 6 additions & 15 deletions pkg/storage/utils/decomposedfs/lookup/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"os"
"path/filepath"
"strconv"
"strings"

provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
Expand Down Expand Up @@ -59,14 +58,10 @@ func (lu *Lookup) MetadataBackend() metadata.Backend {

// ReadBlobSizeAttr reads the blobsize from the xattrs
func (lu *Lookup) ReadBlobSizeAttr(path string) (int64, error) {
attr, err := lu.metadataBackend.Get(path, prefixes.BlobsizeAttr)
blobSize, err := lu.metadataBackend.GetInt64(path, prefixes.BlobsizeAttr)
if err != nil {
return 0, errors.Wrapf(err, "error reading blobsize xattr")
}
blobSize, err := strconv.ParseInt(attr, 10, 64)
if err != nil {
return 0, errors.Wrapf(err, "invalid blobsize xattr format")
}
return blobSize, nil
}

Expand All @@ -76,22 +71,18 @@ func (lu *Lookup) ReadBlobIDAttr(path string) (string, error) {
if err != nil {
return "", errors.Wrapf(err, "error reading blobid xattr")
}
return attr, nil
return string(attr), nil
}

// TypeFromPath returns the type of the node at the given path
func (lu *Lookup) TypeFromPath(path string) provider.ResourceType {
// Try to read from xattrs
typeAttr, err := lu.metadataBackend.Get(path, prefixes.TypeAttr)
t := provider.ResourceType_RESOURCE_TYPE_INVALID
typeAttr, err := lu.metadataBackend.GetInt64(path, prefixes.TypeAttr)
if err == nil {
typeInt, err := strconv.ParseInt(typeAttr, 10, 32)
if err != nil {
return t
}
return provider.ResourceType(typeInt)
return provider.ResourceType(int32(typeAttr))
}

t := provider.ResourceType_RESOURCE_TYPE_INVALID
// Fall back to checking on disk
fi, err := os.Lstat(path)
if err != nil {
Expand Down Expand Up @@ -317,7 +308,7 @@ func (lu *Lookup) CopyMetadataWithSourceLock(source, target string, filter func(
return err
}

newAttrs := make(map[string]string, 0)
newAttrs := make(map[string][]byte, 0)
for attrName, val := range attrs {
if filter == nil || filter(attrName) {
newAttrs[attrName] = val
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/utils/decomposedfs/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (fs *Decomposedfs) SetArbitraryMetadata(ctx context.Context, ref *provider.
}
for k, v := range md.Metadata {
attrName := prefixes.MetadataPrefix + k
if err = n.SetXattr(attrName, v); err != nil {
if err = n.SetXattrString(attrName, v); err != nil {
errs = append(errs, errors.Wrap(err, "Decomposedfs: could not set metadata attribute "+attrName+" to "+k))
}
}
Expand Down
39 changes: 20 additions & 19 deletions pkg/storage/utils/decomposedfs/metadata/ini_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,23 @@ func NewIniBackend(rootPath string, o options.CacheOptions) IniBackend {
}

// All reads all extended attributes for a node
func (b IniBackend) All(path string) (map[string]string, error) {
func (b IniBackend) All(path string) (map[string][]byte, error) {
path = b.MetadataPath(path)

return b.loadMeta(path)
}

// Get an extended attribute value for the given key
func (b IniBackend) Get(path, key string) (string, error) {
func (b IniBackend) Get(path, key string) ([]byte, error) {
path = b.MetadataPath(path)

attribs, err := b.loadMeta(path)
if err != nil {
return "", err
return []byte{}, err
}
val, ok := attribs[key]
if !ok {
return "", &xattr.Error{Op: "ini.get", Path: path, Name: key, Err: xattr.ENOATTR}
return []byte{}, &xattr.Error{Op: "ini.get", Path: path, Name: key, Err: xattr.ENOATTR}
}
return val, nil
}
Expand All @@ -94,7 +94,7 @@ func (b IniBackend) GetInt64(path, key string) (int64, error) {
if !ok {
return 0, &xattr.Error{Op: "ini.get", Path: path, Name: key, Err: xattr.ENOATTR}
}
i, err := strconv.ParseInt(val, 10, 64)
i, err := strconv.ParseInt(string(val), 10, 64)
if err != nil {
return 0, err
}
Expand All @@ -118,12 +118,12 @@ func (b IniBackend) List(path string) ([]string, error) {
}

// Set sets one attribute for the given path
func (b IniBackend) Set(path, key, val string) error {
return b.SetMultiple(path, map[string]string{key: val}, true)
func (b IniBackend) Set(path, key string, val []byte) error {
return b.SetMultiple(path, map[string][]byte{key: val}, true)
}

// SetMultiple sets a set of attribute for the given path
func (b IniBackend) SetMultiple(path string, attribs map[string]string, acquireLock bool) error {
func (b IniBackend) SetMultiple(path string, attribs map[string][]byte, acquireLock bool) error {
return b.saveIni(path, attribs, nil, acquireLock)
}

Expand All @@ -132,7 +132,7 @@ func (b IniBackend) Remove(path, key string) error {
return b.saveIni(path, nil, []string{key}, true)
}

func (b IniBackend) saveIni(path string, setAttribs map[string]string, deleteAttribs []string, acquireLock bool) error {
func (b IniBackend) saveIni(path string, setAttribs map[string][]byte, deleteAttribs []string, acquireLock bool) error {
var (
f readWriteCloseSeekTruncater
err error
Expand Down Expand Up @@ -199,8 +199,8 @@ func (b IniBackend) saveIni(path string, setAttribs map[string]string, deleteAtt
return b.metaCache.PushToCache(b.cacheKey(path), iniAttribs)
}

func (b IniBackend) loadMeta(path string) (map[string]string, error) {
var attribs map[string]string
func (b IniBackend) loadMeta(path string) (map[string][]byte, error) {
var attribs map[string][]byte
err := b.metaCache.PullFromCache(b.cacheKey(path), &attribs)
if err == nil {
return attribs, err
Expand Down Expand Up @@ -295,30 +295,31 @@ func needsEncoding(s []byte) bool {
return false
}

func encodeAttribs(attribs map[string]string) map[string]string {
func encodeAttribs(attribs map[string][]byte) map[string]string {
encAttribs := map[string]string{}
for key, val := range attribs {
if needsEncoding([]byte(val)) {
encAttribs["base64:"+key] = base64.StdEncoding.EncodeToString([]byte(val))
if needsEncoding(val) {
encAttribs["base64:"+key] = base64.StdEncoding.EncodeToString(val)
} else {
encAttribs[key] = val
encAttribs[key] = string(val)
}
}
return encAttribs
}

func decodeAttribs(iniFile *ini.File) (map[string]string, error) {
decodedAttributes := map[string]string{}
func decodeAttribs(iniFile *ini.File) (map[string][]byte, error) {
decodedAttributes := map[string][]byte{}
for key, val := range iniFile.Section("").KeysHash() {
if strings.HasPrefix(key, "base64:") {
key = strings.TrimPrefix(key, "base64:")
valBytes, err := base64.StdEncoding.DecodeString(val)
if err != nil {
return nil, err
}
val = string(valBytes)
decodedAttributes[key] = valBytes
} else {
decodedAttributes[key] = []byte(val)
}
decodedAttributes[key] = val
}
return decodedAttributes, nil
}
16 changes: 8 additions & 8 deletions pkg/storage/utils/decomposedfs/metadata/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ var errUnconfiguredError = errors.New("no metadata backend configured. Bailing o

// Backend defines the interface for file attribute backends
type Backend interface {
All(path string) (map[string]string, error)
Get(path, key string) (string, error)
All(path string) (map[string][]byte, error)
Get(path, key string) ([]byte, error)
GetInt64(path, key string) (int64, error)
List(path string) (attribs []string, err error)
Set(path, key, val string) error
SetMultiple(path string, attribs map[string]string, acquireLock bool) error
Set(path, key string, val []byte) error
SetMultiple(path string, attribs map[string][]byte, acquireLock bool) error
Remove(path, key string) error

Purge(path string) error
Expand All @@ -42,10 +42,10 @@ type Backend interface {
type NullBackend struct{}

// All reads all extended attributes for a node
func (NullBackend) All(path string) (map[string]string, error) { return nil, errUnconfiguredError }
func (NullBackend) All(path string) (map[string][]byte, error) { return nil, errUnconfiguredError }

// Get an extended attribute value for the given key
func (NullBackend) Get(path, key string) (string, error) { return "", errUnconfiguredError }
func (NullBackend) Get(path, key string) ([]byte, error) { return []byte{}, errUnconfiguredError }

// GetInt64 reads a string as int64 from the xattrs
func (NullBackend) GetInt64(path, key string) (int64, error) { return 0, errUnconfiguredError }
Expand All @@ -55,10 +55,10 @@ func (NullBackend) GetInt64(path, key string) (int64, error) { return 0, errUnco
func (NullBackend) List(path string) ([]string, error) { return nil, errUnconfiguredError }

// Set sets one attribute for the given path
func (NullBackend) Set(path string, key string, val string) error { return errUnconfiguredError }
func (NullBackend) Set(path string, key string, val []byte) error { return errUnconfiguredError }

// SetMultiple sets a set of attribute for the given path
func (NullBackend) SetMultiple(path string, attribs map[string]string, acquireLock bool) error {
func (NullBackend) SetMultiple(path string, attribs map[string][]byte, acquireLock bool) error {
return errUnconfiguredError
}

Expand Down
40 changes: 20 additions & 20 deletions pkg/storage/utils/decomposedfs/metadata/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ var _ = Describe("Backend", func() {

Describe("Set", func() {
It("sets an attribute", func() {
err := backend.Set(file, "foo", "bar")
err := backend.Set(file, "foo", []byte("bar"))
Expect(err).ToNot(HaveOccurred())

content, err := os.ReadFile(metafile)
Expand All @@ -73,9 +73,9 @@ var _ = Describe("Backend", func() {
})

It("updates an attribute", func() {
err := backend.Set(file, "foo", "bar")
err := backend.Set(file, "foo", []byte("bar"))
Expect(err).ToNot(HaveOccurred())
err = backend.Set(file, "foo", "baz")
err = backend.Set(file, "foo", []byte("baz"))
Expect(err).ToNot(HaveOccurred())

content, err := os.ReadFile(metafile)
Expand All @@ -84,14 +84,14 @@ var _ = Describe("Backend", func() {
})

It("encodes where needed", func() {
err := backend.Set(file, "user.ocis.cs.foo", "bar")
err := backend.Set(file, "user.ocis.cs.foo", []byte("bar"))
Expect(err).ToNot(HaveOccurred())

content, err := os.ReadFile(metafile)
Expect(err).ToNot(HaveOccurred())
Expect(string(content)).To(Equal("user.ocis.cs.foo = bar\n"))

err = backend.Set(file, "user.ocis.cs.foo", string([]byte{200, 201, 202}))
err = backend.Set(file, "user.ocis.cs.foo", []byte{200, 201, 202})
Expect(err).ToNot(HaveOccurred())

content, err = os.ReadFile(metafile)
Expand All @@ -100,14 +100,14 @@ var _ = Describe("Backend", func() {
})

It("doesn't encode already encoded attributes", func() {
err := backend.Set(file, "user.ocis.cs.foo", string([]byte{200, 201, 202}))
err := backend.Set(file, "user.ocis.cs.foo", []byte{200, 201, 202})
Expect(err).ToNot(HaveOccurred())

content, err := os.ReadFile(metafile)
Expect(err).ToNot(HaveOccurred())
Expect(string(content)).To(Equal("`base64:user.ocis.cs.foo` = yMnK\n"))

err = backend.Set(file, "user.something", "doesn'tmatter")
err = backend.Set(file, "user.something", []byte("doesn'tmatter"))
Expect(err).ToNot(HaveOccurred())

content, err = os.ReadFile(metafile)
Expand All @@ -119,7 +119,7 @@ var _ = Describe("Backend", func() {
_, err := backend.Get(file, "foo")
Expect(err).To(HaveOccurred())

err = backend.Set(file, "foo", "")
err = backend.Set(file, "foo", []byte{})
Expect(err).ToNot(HaveOccurred())

v, err := backend.Get(file, "foo")
Expand All @@ -130,7 +130,7 @@ var _ = Describe("Backend", func() {

Describe("SetMultiple", func() {
It("sets attributes", func() {
err := backend.SetMultiple(file, map[string]string{"foo": "bar", "baz": "qux"}, true)
err := backend.SetMultiple(file, map[string][]byte{"foo": []byte("bar"), "baz": []byte("qux")}, true)
Expect(err).ToNot(HaveOccurred())

content, err := os.ReadFile(metafile)
Expand All @@ -140,9 +140,9 @@ var _ = Describe("Backend", func() {
})

It("updates an attribute", func() {
err := backend.Set(file, "foo", "bar")
err := backend.Set(file, "foo", []byte("bar"))
Expect(err).ToNot(HaveOccurred())
err = backend.SetMultiple(file, map[string]string{"foo": "bar", "baz": "qux"}, true)
err = backend.SetMultiple(file, map[string][]byte{"foo": []byte("bar"), "baz": []byte("qux")}, true)
Expect(err).ToNot(HaveOccurred())

content, err := os.ReadFile(metafile)
Expand All @@ -152,11 +152,11 @@ var _ = Describe("Backend", func() {
})

It("encodes where needed", func() {
err := backend.SetMultiple(file, map[string]string{
"user.ocis.something.foo": "bar",
"user.ocis.cs.foo": string([]byte{200, 201, 202}),
"user.ocis.md.foo": string([]byte{200, 201, 202}),
"user.ocis.grant.foo": "bar",
err := backend.SetMultiple(file, map[string][]byte{
"user.ocis.something.foo": []byte("bar"),
"user.ocis.cs.foo": []byte{200, 201, 202},
"user.ocis.md.foo": []byte{200, 201, 202},
"user.ocis.grant.foo": []byte("bar"),
}, true)
Expect(err).ToNot(HaveOccurred())

Expand All @@ -180,7 +180,7 @@ var _ = Describe("Backend", func() {
Expect(err).ToNot(HaveOccurred())
Expect(len(v)).To(Equal(2))
Expect(v["foo"]).To(Equal("123"))
Expect(v["bar"]).To(Equal("baz"))
Expect(v["bar"]).To(Equal([]byte("baz")))
})

It("returns an empty map", func() {
Expand All @@ -197,7 +197,7 @@ var _ = Describe("Backend", func() {

v, err := backend.List(file)
Expect(err).ToNot(HaveOccurred())
Expect(v).To(ConsistOf("foo", "bar"))
Expect(v).To(ConsistOf("foo", []byte("bar")))
})

It("returns an empty list", func() {
Expand All @@ -214,7 +214,7 @@ var _ = Describe("Backend", func() {

v, err := backend.Get(file, "foo")
Expect(err).ToNot(HaveOccurred())
Expect(v).To(Equal("bar"))
Expect(v).To(Equal([]byte("bar")))
})

It("returns an error on unknown attributes", func() {
Expand Down Expand Up @@ -246,7 +246,7 @@ var _ = Describe("Backend", func() {

v, err := backend.Get(file, "foo")
Expect(err).ToNot(HaveOccurred())
Expect(v).To(Equal("bar"))
Expect(v).To(Equal([]byte("bar")))

err = backend.Remove(file, "foo")
Expect(err).ToNot(HaveOccurred())
Expand Down
Loading

0 comments on commit 371c1d7

Please sign in to comment.