Skip to content

Commit

Permalink
fix(registry): update peername and dsrefs on registy signup name change
Browse files Browse the repository at this point in the history
  • Loading branch information
b5 committed Jan 16, 2020
1 parent d4ac19c commit 8bc151c
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 4 deletions.
47 changes: 45 additions & 2 deletions base/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,15 @@ func ModifyDatasetRef(ctx context.Context, r repo.Repo, current, new *repo.Datas
}
// attempt to canonicalize the new reference
err = repo.CanonicalizeDatasetRef(r, new)
if err == nil && isRename {
if err == nil {
// successful canonicalization on rename is an error
return fmt.Errorf("dataset '%s/%s' already exists", new.Peername, new.Name)
if isRename {
// TODO (b5) - this isn't entirely true. If the provided ref has an absolute path, it'll clean up
// the new var, which won't error, but *will* destroy the rename alias. We need more sophisticated
// name resolution functions. This test should only be checking if the *alias* portion of the
// name already exists for a rename
return fmt.Errorf("dataset '%s/%s' already exists", new.Peername, new.Name)
}
} else if err != repo.ErrNotFound {
log.Debug(err.Error())
return fmt.Errorf("error with new reference: %s", err.Error())
Expand All @@ -109,3 +115,40 @@ func ModifyDatasetRef(ctx context.Context, r repo.Repo, current, new *repo.Datas

return nil
}

// ModifyRepoUsername performs all tasks necessary to switch a username
// (formerly: peername) for a local repo, and must be called when a username is
// changed
// TODO (b5) - make this transactional
func ModifyRepoUsername(ctx context.Context, r repo.Repo, book *logbook.Book, from, to string) error {
log.Debugf("change peername: %s -> %s", from, to)
// TODO (b5) - we need to immidiately update all dataset references in the refstore on rename
// because we currently rely on dsref as our source of canonicalization.
// Many places in our codebase call repo.CanonicalizeDatasetRef with an alias reference
// before doing anything, which means if the refence there is off, Canonicalize will overwrite
// with the prior, incorrect name, and cause not-found errors in place that are properly tracking
// updates, (like logbook). This is hacky & ugly, but helps us understand how to redesign dsrefs
if refs, err := r.References(0, 10000000); err == nil {
for _, ref := range refs {
if ref.Peername == from {
update := repo.DatasetRef{
Peername: to,
Name: ref.Name,
Path: ref.Path,
ProfileID: ref.ProfileID,
FSIPath: ref.FSIPath,
}

if err = r.DeleteRef(ref); err != nil {
return err
}
if err = r.PutRef(update); err != nil {
return err
}
}
}
}

// we also need to update the logbook
return book.WriteAuthorRename(ctx, to)
}
2 changes: 2 additions & 0 deletions lib/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,11 @@ func (r *LogRequests) Logbook(p *RefListParams, res *[]LogEntry) error {
if err != nil {
return err
}

if err = repo.CanonicalizeDatasetRef(r.node.Repo, &ref); err != nil {
return err
}
log.Debugf("%v", ref)

book := r.node.Repo.Logbook()
*res, err = book.LogEntries(ctx, repo.ConvertToDsref(ref), p.Offset, p.Limit)
Expand Down
17 changes: 16 additions & 1 deletion lib/registry.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package lib

import (
"context"

"github.com/qri-io/qri/base"
"github.com/qri-io/qri/config"
"github.com/qri-io/qri/registry"
"github.com/qri-io/qri/repo/profile"
Expand Down Expand Up @@ -35,7 +38,7 @@ func (m RegistryClientMethods) CreateProfile(p *RegistryProfile, ok *bool) (err
return err
}

log.Errorf("create profile response: %v", pro)
log.Debugf("create profile response: %v", pro)
*p = *pro

return m.updateConfig(pro)
Expand Down Expand Up @@ -75,13 +78,25 @@ func (m RegistryClientMethods) configChanges(pro *registry.Profile) *config.Conf
}

func (m RegistryClientMethods) updateConfig(pro *registry.Profile) error {
ctx := context.TODO()
cfg := m.configChanges(pro)

// TODO (b5) - this should be automatically done by m.inst.ChangeConfig
repoPro, err := profile.NewProfile(cfg.Profile)
if err != nil {
return err
}

// TODO (b5) - this is the lowest level place I could find to monitor for
// profile name changes, not sure this makes the most sense to have this here.
// we should consider a separate track for any change that affects the peername,
// it should always be verified by any set registry before saving
if cfg.Profile.Peername != m.inst.cfg.Profile.Peername {
if err := base.ModifyRepoUsername(ctx, m.inst.Repo(), m.inst.logbook, m.inst.cfg.Profile.Peername, cfg.Profile.Peername); err != nil {
return err
}
}

if err := m.inst.Repo().SetProfile(repoPro); err != nil {
return err
}
Expand Down
5 changes: 4 additions & 1 deletion logbook/logbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ func (book Book) Log(ctx context.Context, id string) (*oplog.Log, error) {
// branch
// ...
func (book Book) UserDatasetRef(ctx context.Context, ref dsref.Ref) (*oplog.Log, error) {
log.Debugf("UserDatasetRef: %s", ref)
if ref.Username == "" {
return nil, fmt.Errorf("logbook: reference Username is required")
}
Expand Down Expand Up @@ -563,8 +564,9 @@ func (book Book) UserDatasetRef(ctx context.Context, ref dsref.Ref) (*oplog.Log,
// access control changes are kept in the dataset log
//
// currently all logs are hardcoded to only accept one branch name. This
// function always returns
// function will always retunr a singl branch
func (book Book) DatasetRef(ctx context.Context, ref dsref.Ref) (*oplog.Log, error) {
log.Debugf("DatasetRef: %s", ref)
if ref.Username == "" {
return nil, fmt.Errorf("logbook: ref.Username is required")
}
Expand All @@ -581,6 +583,7 @@ func (book Book) DatasetRef(ctx context.Context, ref dsref.Ref) (*oplog.Log, err
// currently all logs are hardcoded to only accept one branch name. This
// function always returns
func (book Book) BranchRef(ctx context.Context, ref dsref.Ref) (*oplog.Log, error) {
log.Debugf("BranchRef: %s", ref)
if ref.Username == "" {
return nil, fmt.Errorf("logbook: ref.Username is required")
}
Expand Down

0 comments on commit 8bc151c

Please sign in to comment.