Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix/sharing: some more bugs with sharing and mirror file creation #161

Merged
merged 5 commits into from
Sep 8, 2020
+67 −37
Merged

Hotfix/sharing: some more bugs with sharing and mirror file creation #161

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62ef9c1
Hotfix/sharing: some more bugs with sharing and mirror file creation,…
jsonsivar Sep 8, 2020
26a4678
Hotfix/sharing: make paths immutable, fallback local ipfsaddr, fix pu…
jsonsivar Sep 8, 2020
b6c66b9
Hotfix/sharing: remove leftover logs
jsonsivar Sep 8, 2020
1742bb0
Hotfix/sharing: use different name for mirror bucket
jsonsivar Sep 8, 2020
c12046c
Hotfix/sharing: fix typo
jsonsivar Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .vscode/launch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"request": "launch",
"mode": "auto",
"program": "${workspaceFolder}/cmd/space-daemon",
"env": {},
"envFile": "${workspaceFolder}/.env",
"args": ["-dev=true"]
}
]
Expand Down
5 changes: 5 additions & 0 deletions config/map_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ func NewMap(envVal env.SpaceEnv, flags *Flags) Config {
configBool[Ipfsnode] = flags.Ipfsnode
}

// Temp fix until we move to viper
if configStr[Ipfsaddr] == "" {
configStr[Ipfsaddr] = "/ip4/127.0.0.1/tcp/5001"
}

c := mapConfig{
configStr: configStr,
configInt: configInt,
Expand Down
5 changes: 3 additions & 2 deletions core/env/file_env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package env

import (
"github.com/joho/godotenv"
syslog "log"
"os"
"strings"

"github.com/joho/godotenv"
)

type spaceEnv struct {
Expand Down Expand Up @@ -55,4 +56,4 @@ func (s spaceEnv) LogLevel() string {
}

return ll
}
}
4 changes: 2 additions & 2 deletions core/space/services/services_fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ func (s *Space) addFile(ctx context.Context, sourcePath string, targetPath strin
return domain.AddItemResult{}, err
}

if s.tc.IsBucketBackup(ctx, b.Slug()) && !s.tc.IsMirrorFile(ctx, targetPath, b.Slug()) {
if s.tc.IsBucketBackup(ctx, b.Slug()) && !s.tc.IsMirrorFile(ctx, targetPathBucket, b.Slug()) {
f.Seek(0, io.SeekStart)

_, _, err = s.tc.UploadFileToHub(ctx, b, targetPathBucket, f)
Expand All @@ -502,7 +502,7 @@ func (s *Space) addFile(ctx context.Context, sourcePath string, targetPath strin
return domain.AddItemResult{}, err
}

_, err = s.tc.MarkMirrorFileBackup(ctx, targetPath, b.Slug())
_, err = s.tc.MarkMirrorFileBackup(ctx, targetPathBucket, b.Slug())
if err != nil {
log.Error(fmt.Sprintf("error creating mirror file Path=%s BucketSlug=%s", targetPathBucket, b.Key()), err)
return domain.AddItemResult{}, err
Expand Down
42 changes: 28 additions & 14 deletions core/space/services/services_sharing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import (

"github.com/FleekHQ/space-daemon/log"
crypto "github.com/libp2p/go-libp2p-crypto"
"github.com/pkg/errors"

"github.com/textileio/dcrypto"

"github.com/pkg/errors"

"github.com/FleekHQ/space-daemon/core/space/domain"
t "github.com/FleekHQ/space-daemon/core/textile"
"github.com/ipfs/go-cid"
)

Expand Down Expand Up @@ -229,24 +229,35 @@ func (s *Space) OpenSharedFile(ctx context.Context, hash, password, filename str
func (s *Space) ShareFilesViaPublicKey(ctx context.Context, paths []domain.FullPath, pubkeys []crypto.PubKey) error {
m := s.tc.GetModel()

var enckeys [][]byte
enhancedPaths := make([]domain.FullPath, len(paths))
enckeys := make([][]byte, len(paths))
for i, path := range paths {
ep := domain.FullPath{
DbId: path.DbId,
Bucket: path.Bucket,
Path: path.Path,
BucketKey: path.BucketKey,
}

// this handles personal bucket since for shared-with-me files
// the dbid will be preset
if path.DbId == "" {
if ep.DbId == "" {
b, err := s.tc.GetDefaultBucket(ctx)
if err != nil {
return err
}

bs, err := m.FindBucket(ctx, b.GetData().Name)
bs, err := m.FindBucket(ctx, b.Slug())
if err != nil {
return err
}
path.DbId = bs.RemoteBucketKey

log.Info("incoming dbid nil, updating")
log.Info("using new remote db id: " + bs.RemoteDbID)
dmerrill6 marked this conversation as resolved.
Show resolved Hide resolved
ep.DbId = bs.RemoteDbID
}

if path.Bucket == "" {
if ep.Bucket == "" || ep.Bucket == t.GetDefaultBucketSlug() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do shared with me files have ep.Bucket == ""? Because if that's the case it might unintendedly fall into this block for shared-with-me files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So shared with me files will always have a bucket name. But the weird part is that the mirror bucket name is also called personal so it would erroneous fall into the second clause. I made both clauses so that it would default to the personal bucket if it was empty or if personal was supplied explicitly. For now I'm thinking to keep it clean we can change the name of the mirrored buckets to something like shared so it doesn't fall into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maurycy do you see any issues with this idea? Like if I used personal_mirror for the mirror bucket names. As long as we are not referring the mirror bucket name hardcoded as personal elsewhere that should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsonsivar

Not sure if I understand. Do you mean adding a prefix, eg: shared_, personal_?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1742bb0. Mirror buckets now use a different name so it wouldn't fall into this slot unless it was actually a personal bucket (or empty in which case it is personal).

b, err := s.tc.GetDefaultBucket(ctx)
if err != nil {
return err
Expand All @@ -255,28 +266,31 @@ func (s *Space) ShareFilesViaPublicKey(ctx context.Context, paths []domain.FullP
if err != nil {
return err
}
path.Bucket = b.Slug()
path.BucketKey = bs.RemoteBucketKey

enckeys[i] = bs.EncryptionKey
ep.Bucket = b.Slug()
ep.BucketKey = bs.RemoteBucketKey
enckeys = append(enckeys, bs.EncryptionKey)
} else {
r, err := m.FindReceivedFile(ctx, path)
if err != nil {
return err
}
enckeys[i] = r.EncryptionKey
ep.Bucket = r.Bucket
ep.BucketKey = r.BucketKey
enckeys = append(enckeys, r.EncryptionKey)
}

enhancedPaths[i] = ep
}

err := s.tc.ShareFilesViaPublicKey(ctx, paths, pubkeys, enckeys)
err := s.tc.ShareFilesViaPublicKey(ctx, enhancedPaths, pubkeys, enckeys)
if err != nil {
return err
}

for _, pk := range pubkeys {

d := &domain.Invitation{
ItemPaths: paths,
ItemPaths: enhancedPaths,
Keys: enckeys,
}

Expand Down
1 change: 1 addition & 0 deletions core/textile/buckd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func (tb *TextileBuckd) Start(ctx context.Context) error {
addrAPIProxy := cmd.AddrFromStr("/ip4/127.0.0.1/tcp/3007")
addrThreadsHost := cmd.AddrFromStr("/ip4/0.0.0.0/tcp/4006")
// TODO: replace with local blockstore

addrIpfsAPI := cmd.AddrFromStr(IpfsAddr)

addrGatewayHost := cmd.AddrFromStr("/ip4/127.0.0.1/tcp/8006")
Expand Down
6 changes: 5 additions & 1 deletion core/textile/bucket_factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func (tc *textileClient) createBucket(ctx context.Context, bucketSlug string) (B

// We store the bucket in a meta thread so that we can later fetch a list of all buckets
log.Debug("Bucket " + bucketSlug + " created. Storing metadata.")
schema, err := m.CreateBucket(ctx, bucketSlug, dbID.String())
schema, err := m.CreateBucket(ctx, bucketSlug, utils.CastDbIDToString(*dbID))
jsonsivar marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -333,3 +333,7 @@ func (tc *textileClient) IsBucketBackup(ctx context.Context, bucketSlug string)

return bucketSchema.Backup
}

func GetDefaultBucketSlug() string {
return defaultPersonalBucketSlug
}
11 changes: 8 additions & 3 deletions core/textile/model/mirror_file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type MirrorBucketSchema struct {
const mirrorFileModelName = "MirrorFile"

var errMirrorFileNotFound = errors.New("Mirror file not found")
var errMirrorFileAlreadyExists = errors.New("Mirror file already exists")

func (m *model) CreateMirrorBucket(ctx context.Context, bucketSlug string, mirrorBucket *MirrorBucketSchema) (*BucketSchema, error) {
metaCtx, metaDbID, err := m.initBucketModel(ctx)
Expand Down Expand Up @@ -71,12 +72,12 @@ func (m *model) FindMirrorFileByPathAndBucketSlug(ctx context.Context, path, buc
}

if rawMirrorFiles == nil {
return &MirrorFileSchema{}, nil
return nil, nil
}

mirror_files := rawMirrorFiles.([]*MirrorFileSchema)
if len(mirror_files) == 0 {
return &MirrorFileSchema{}, nil
return nil, nil
}

log.Debug("Model.FindMirrorFileByPathAndBucketSlug: returning mirror file with dbid " + mirror_files[0].DbID)
Expand All @@ -89,11 +90,15 @@ func (m *model) CreateMirrorFile(ctx context.Context, mirrorFile *domain.MirrorF
return nil, err
}

_, err = m.FindMirrorFileByPathAndBucketSlug(ctx, mirrorFile.Path, mirrorFile.BucketSlug)
mf, err := m.FindMirrorFileByPathAndBucketSlug(ctx, mirrorFile.Path, mirrorFile.BucketSlug)
if err != nil {
return nil, err
}

if mf != nil {
return nil, errMirrorFileAlreadyExists
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsonsivar

I wonder: should we return err or just return mf, nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will leave this as-is for now and can update later since it's working at least. If we need a create or get type usage if this later we can always update.

}

newInstance := &MirrorFileSchema{
Path: mirrorFile.Path,
BucketSlug: mirrorFile.BucketSlug,
Expand Down
10 changes: 4 additions & 6 deletions core/textile/sharing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/FleekHQ/space-daemon/core/space/domain"
"github.com/FleekHQ/space-daemon/log"
crypto "github.com/libp2p/go-libp2p-crypto"
"github.com/textileio/go-threads/core/thread"
"github.com/textileio/textile/buckets"
)

Expand All @@ -25,13 +26,10 @@ func (tc *textileClient) ShareFilesViaPublicKey(ctx context.Context, paths []dom
}

log.Info("Adding roles for pth: " + pth.Path)
var roles map[string]buckets.Role
roles := make(map[string]buckets.Role)
for _, pk := range pubkeys {
pkb, err := pk.Bytes()
if err != nil {
return err
}
roles[string(pkb)] = buckets.Writer
tpk := thread.NewLibp2pPubKey(pk)
roles[tpk.String()] = buckets.Writer
}

err := tc.hb.PushPathAccessRoles(ctx, pth.BucketKey, pth.Path, roles)
Expand Down