fix(remote): disambiguate AuthorID & AuthorPubKey use in logsync

Also! There's a horrible bug this refactoring uncovered in cmd/fetch_test.go: both peers in the fetch are writing to the _same username_, which is bad. Once this refactoring, the test was returning "not found" because we're doing a *get* by alias reference (formerly "test_peer/test_movies"), and post-fetch _two_ peers had the peername "test_peer". (The test used the same "NewTestRepo" func, which hardcoded the peername). There's actually no way to get the ref in this context.  We should be doing checks for ambiguous head references

cmd/cmd_test.go

@@ -239,7 +239,7 @@ func TestSaveRelativeBodyPath(t *testing.T) {
  t.Skip(err.Error())
  }
 
- r := NewTestRepoRoot(t, "qri_test_save_relative_body")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_relative_body")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -274,7 +274,7 @@ func TestRemoveOnlyTwoRevisions(t *testing.T) {
  t.Skip(err.Error())
  }
 
- r := NewTestRepoRoot(t, "qri_test_remove_only_two_revisions")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_remove_only_two_revisions")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -325,7 +325,7 @@ func TestRemoveAllRevisionsLongForm(t *testing.T) {
  t.Skip(err.Error())
  }
 
- r := NewTestRepoRoot(t, "qri_test_remove_only_one_revision")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_remove_only_one_revision")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -368,7 +368,7 @@ func TestRemoveAllRevisionsShortForm(t *testing.T) {
  t.Skip(err.Error())
  }
 
- r := NewTestRepoRoot(t, "qri_test_remove_only_one_revision")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_remove_only_one_revision")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -417,7 +417,7 @@ func TestSaveThenOverrideMetaComponent(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_then_override_meta")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_then_override_meta")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -458,7 +458,7 @@ func TestSaveTwoComponents(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_then_override_meta")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_then_override_meta")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -500,7 +500,7 @@ func TestSaveThenOverrideTransform(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_file_transform")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_file_transform")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -541,7 +541,7 @@ func TestSaveThenOverrideViz(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_file_transform")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_file_transform")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -582,7 +582,7 @@ func TestSaveThenOverrideMetaAndTransformAndViz(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_file_transform")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_file_transform")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -623,7 +623,7 @@ func TestSaveDatasetWithComponentError(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_then_override_meta")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_then_override_meta")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -653,7 +653,7 @@ func TestSaveConflictingComponents(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_then_override_meta")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_then_override_meta")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -689,7 +689,7 @@ func TestSaveTransformWithoutChanges(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_transform_same")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_transform_same")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -724,7 +724,7 @@ func TestTransformUsingGetBodyAndSetBody(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_save_transform_get_body")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_save_transform_get_body")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -765,7 +765,7 @@ func TestDiffRevisions(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_diff_revisions")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_diff_revisions")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -826,7 +826,7 @@ func TestDiffOnlyOneRevision(t *testing.T) {
  defer func() { dsfs.Timestamp = prev }()
  dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }
 
- r := NewTestRepoRoot(t, "qri_test_diff_only_one")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_diff_only_one")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -870,7 +870,7 @@ type TestRepoRoot struct {
 }
 
 // NewTestRepoRoot constructs the test repo and initializes everything as cheaply as possible.
-func NewTestRepoRoot(t *testing.T, prefix string) TestRepoRoot {
+func NewTestRepoRoot(t *testing.T, peername, prefix string) TestRepoRoot {
  rootPath, err := ioutil.TempDir("", prefix)
  if err != nil {
  t.Fatal(err)
@@ -896,7 +896,7 @@ func NewTestRepoRoot(t *testing.T, prefix string) TestRepoRoot {
  }
  // Create empty config.yaml into the test repo.
  cfg := config.DefaultConfigForTesting().Copy()
- cfg.Profile.Peername = "test_peer"
+ cfg.Profile.Peername = peername
  err = cfg.WriteToFile(filepath.Join(qriPath, "config.yaml"))
  if err != nil {
  t.Fatal(err)

cmd/fetch_test.go

@@ -7,7 +7,7 @@ import (
 )
 
 func TestFetchCommand(t *testing.T) {
- r := NewTestRepoRoot(t, "qri_test_fetch_a")
+ r := NewTestRepoRoot(t, "peer_a", "qri_test_fetch_a")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())
@@ -25,7 +25,7 @@ func TestFetchCommand(t *testing.T) {
  }
 
  cmdR = r.CreateCommandRunner(ctx)
- if err = executeCommand(cmdR, "qri log test_peer/test_movies"); err != nil {
+ if err = executeCommand(cmdR, "qri log peer_a/test_movies"); err != nil {
  t.Fatal(err)
  }
 
@@ -50,11 +50,11 @@ func TestFetchCommand(t *testing.T) {
  // TODO (b5) - this is horrible. we should block on a channel receive for connectedness
  time.Sleep(time.Second * 5)
 
- b := NewTestRepoRoot(t, "qri_test_fetch_b")
+ b := NewTestRepoRoot(t, "peer_b", "qri_test_fetch_b")
  defer b.Delete()
 
  cmdBr := b.CreateCommandRunner(ctx)
- if err = executeCommand(cmdBr, "qri log test_peer/test_movies"); err == nil {
+ if err = executeCommand(cmdBr, "qri log peer_b/test_movies"); err == nil {
  t.Fatal("expected fetch on non-existant log to error")
  }
 
@@ -64,7 +64,7 @@ func TestFetchCommand(t *testing.T) {
  }
 
  cmdBr = b.CreateCommandRunner(ctx)
- if err = executeCommand(cmdBr, "qri fetch test_peer/test_movies --remote a_node"); err != nil {
+ if err = executeCommand(cmdBr, "qri fetch peer_a/test_movies --remote a_node"); err != nil {
  t.Fatal(err)
  }
 
@@ -77,7 +77,7 @@ func TestFetchCommand(t *testing.T) {
  t.Logf("%s", text)
 
  cmdBr = b.CreateCommandRunner(ctx)
- if err = executeCommand(cmdBr, "qri log test_peer/test_movies"); err != nil {
+ if err = executeCommand(cmdBr, "qri log peer_a/test_movies"); err != nil {
  t.Fatal(err)
  }
 

cmd/fsi_integration_test.go

@@ -30,7 +30,7 @@ type FSITestRunner struct {
 
 // NewFSITestRunner returns a new FSITestRunner.
 func NewFSITestRunner(t *testing.T, testName string) *FSITestRunner {
- root := NewTestRepoRoot(t, testName)
+ root := NewTestRepoRoot(t, "test_peer", testName)
 
  fr := FSITestRunner{}
  fr.RepoRoot = &root

cmd/log_test.go

@@ -6,7 +6,7 @@ import (
 )
 
 func TestLogbookCommand(t *testing.T) {
- r := NewTestRepoRoot(t, "qri_test_logbook")
+ r := NewTestRepoRoot(t, "test_peer", "qri_test_logbook")
  defer r.Delete()
 
  ctx, done := context.WithCancel(context.Background())

logbook/logbook.go

@@ -115,9 +115,10 @@ func NewBook(pk crypto.PrivKey, username string, fs qfs.Filesystem, location str
  return book, err
  }
  return nil, err
- } else {
- // TODO (b5) verify username integrity on load
  }
+ // else {
+ // TODO (b5) verify username integrity on load
+ // }
 
  return book, nil
 }

logbook/logsync/p2p_test.go

@@ -2,7 +2,6 @@ package logsync
 
 import (
  "context"
- "fmt"
  "testing"
 
  libp2p "github.com/libp2p/go-libp2p"
@@ -45,7 +44,6 @@ func TestP2PLogsync(t *testing.T) {
  if err != nil {
  t.Error(err)
  }
- fmt.Printf("removing dataset: %s\n", tr.A.ActivePeerID())
  if err := pull.Do(tr.Ctx); err != nil {
  t.Error(err)
  }

logbook/oplog/log.go

@@ -392,7 +392,8 @@ func (lg Log) Name() string {
  return lg.name
 }
 
-// Log fetches a log by ID, checking all descendants for a match
+// Log fetches a log by ID, checking the current log and all descendants for a
+// an exact match
 func (lg *Log) Log(id string) (*Log, error) {
  if lg.ID() == id {
  return lg, nil
@@ -423,6 +424,7 @@ func (lg *Log) HeadRef(names ...string) (*Log, error) {
 
 // AddChild appends a log as a direct descendant of this log
 func (lg *Log) AddChild(l *Log) {
+ l.parent = lg
  lg.Logs = append(lg.Logs, l)
 }
 

logbook/oplog/log_test.go

@@ -7,6 +7,7 @@ import (
  "testing"
 
  "github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp/cmpopts"
  crypto "github.com/libp2p/go-libp2p-core/crypto"
  "github.com/qri-io/qri/logbook/oplog/logfb"
 )
@@ -61,7 +62,11 @@ func TestBookFlatbuffer(t *testing.T) {
  t.Fatalf("unmarshalling flatbuffer bytes: %s", err.Error())
  }
 
- if diff := cmp.Diff(book, got, allowUnexported, cmp.Comparer(comparePrivKeys)); diff != "" {
+ // TODO (b5) - need to ignore log.parent here. causes a stack overflow in cmp.Diff
+ // we should file an issue with a test that demonstrates the error
+ ignoreCircularPointers := cmpopts.IgnoreUnexported(Log{})
+
+ if diff := cmp.Diff(book, got, allowUnexported, cmp.Comparer(comparePrivKeys), ignoreCircularPointers); diff != "" {
  t.Errorf("result mismatch (-want +got):\n%s", diff)
  }
 }