feat(remote): populate LogPushFinalCheck dataset reference

logbook/logbook.go

@@ -402,7 +402,8 @@ func (book *Book) WriteCronJobRan(ctx context.Context, number int64, ref dsref.R
  return book.save(ctx)
 }
 
-// Log gets a log for a given dsref
+// Log gets a log for a given dsref. The returned log is an exact reference,
+// refering one and only one dataset
 func (book Book) Log(ref dsref.Ref) (*oplog.Log, error) {
  if ref.Username == "" {
  return nil, fmt.Errorf("logbook: reference Username is required")
@@ -433,6 +434,30 @@ func (book Book) LogBytes(log *oplog.Log) ([]byte, error) {
  return log.SignedFlatbufferBytes(book.pk)
 }
 
+// DsrefAliasForLog parses log data into a dataset alias reference, populating
+// only the username and name components of a dataset.
+// the passed in oplog must refer unambiguously to a dataset or branch.
+// book.Log() returns exact log references
+func DsrefAliasForLog(log *oplog.Log) (dsref.Ref, error) {
+ ref := dsref.Ref{}
+ if log == nil {
+ return ref, fmt.Errorf("logbook: log is required")
+ }
+ if log.Model() != nameModel {
+ return ref, fmt.Errorf("logbook: log doesn't descibe a dataset")
+ }
+ if len(log.Logs) != 1 {
+ return ref, fmt.Errorf("logbook: ambiguous dataset reference")
+ }
+
+ ref = dsref.Ref{
+ Username: log.Name(),
+ Name: log.Logs[0].Name(),
+ }
+
+ return ref, nil
+}
+
 // MergeLog adds a log to the logbook, merging with any existing log data
 func (book *Book) MergeLog(ctx context.Context, sender oplog.Author, lg *oplog.Log) error {
 

logbook/logbook_test.go

@@ -282,6 +282,55 @@ func TestLogBytes(t *testing.T) {
  }
 }
 
+func TestDsRefAliasForLog(t *testing.T) {
+ tr, cleanup := newTestRunner(t)
+ defer cleanup()
+
+ tr.WriteWorldBankExample(t)
+ tr.WriteRenameExample(t)
+ egRef := tr.RenameRef()
+ log, err := tr.Book.Log(egRef)
+ if err != nil {
+ t.Error(err)
+ }
+
+ if _, err := DsrefAliasForLog(nil); err == nil {
+ t.Error("expected nil ref to error")
+ }
+
+ wrongModelLog, err := tr.Book.bk.Log(userModel, tr.Username)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := DsrefAliasForLog(wrongModelLog); err == nil {
+ t.Error("expected converting log of wrong model to error")
+ }
+
+ ambiguousLog, err := tr.Book.bk.Log(nameModel, tr.Username)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := DsrefAliasForLog(ambiguousLog); err == nil {
+ t.Error("expected converting ambiguous logs to error")
+ }
+
+ ref, err := DsrefAliasForLog(log)
+ if err != nil {
+ t.Error(err)
+ }
+
+ expect := dsref.Ref{
+ Username: egRef.Username,
+ Name: egRef.Name,
+ }
+
+ if diff := cmp.Diff(expect, ref); diff != "" {
+ t.Errorf("result mismatch. (-want +got):\n%s", diff)
+ }
+}
+
 func TestBookRawLog(t *testing.T) {
  tr, cleanup := newTestRunner(t)
  defer cleanup()

logbook/logsync/logsync.go

@@ -185,7 +185,6 @@ func (lsync *Logsync) put(ctx context.Context, author oplog.Author, r io.Reader)
  }
 
  if lsync.pushPreCheck != nil {
- // TODO (b5) - need to populate path
  if err := lsync.pushPreCheck(ctx, author, dsref.Ref{}, nil); err != nil {
  return err
  }
@@ -205,8 +204,11 @@ func (lsync *Logsync) put(ctx context.Context, author oplog.Author, r io.Reader)
  }
 
  if lsync.pushFinalCheck != nil {
- // TODO (b5) - need to populate path
- if err := lsync.pushFinalCheck(ctx, author, dsref.Ref{}, lg); err != nil {
+ ref, err := logbook.DsrefAliasForLog(lg)
+ if err != nil {
+ return err
+ }
+ if err := lsync.pushFinalCheck(ctx, author, ref, lg); err != nil {
  return err
  }
  }
@@ -259,7 +261,6 @@ func (lsync *Logsync) del(ctx context.Context, sender oplog.Author, ref dsref.Re
  }
 
  if lsync.removePreCheck != nil {
- // TODO (b5) - need to populate path
  if err := lsync.removePreCheck(ctx, sender, ref, nil); err != nil {
  return err
  }

remote/remote_test.go

@@ -41,6 +41,18 @@ func TestDatasetPullPushDeleteHTTP(t *testing.T) {
  }
  }
 
+ requireLogAndRefCallCheck := func(t *testing.T, s string) Hook {
+ return func(ctx context.Context, pid profile.ID, ref repo.DatasetRef) error {
+ if ref.String() == "" {
+ t.Errorf("hook %s expected reference to be populated", s)
+ }
+ if l, ok := OplogFromContext(ctx); !ok {
+ t.Errorf("hook %s expected log to be in context. got: %v", s, l)
+ }
+ return callCheck(s)(ctx, pid, ref)
+ }
+ }
+
  opts := func(o *Options) {
  o.DatasetPushPreCheck = callCheck("DatasetPushPreCheck")
  o.DatasetPushFinalCheck = callCheck("DatasetPushFinalCheck")
@@ -49,7 +61,7 @@ func TestDatasetPullPushDeleteHTTP(t *testing.T) {
  o.DatasetRemoved = callCheck("DatasetRemoved")
 
  o.LogPushPreCheck = callCheck("LogPushPreCheck")
- o.LogPushFinalCheck = requireLogCallCheck(t, "LogPushFinalCheck")
+ o.LogPushFinalCheck = requireLogAndRefCallCheck(t, "LogPushFinalCheck")
  o.LogPushed = requireLogCallCheck(t, "LogPushed")
  o.LogPullPreCheck = callCheck("LogPullPreCheck")
  o.LogPulled = callCheck("LogPulled")
@@ -113,7 +125,7 @@ func TestDatasetPullPushDeleteHTTP(t *testing.T) {
  t.Error(err)
  }
 
- expect := []string{
+ expectHooksCallOrder := []string{
  "LogPullPreCheck",
  "LogPulled",
  "DatasetPulled",
@@ -128,7 +140,7 @@ func TestDatasetPullPushDeleteHTTP(t *testing.T) {
  "DatasetRemoved",
  }
 
- if diff := cmp.Diff(expect, hooksCalled); diff != "" {
+ if diff := cmp.Diff(expectHooksCallOrder, hooksCalled); diff != "" {
  t.Errorf("result mismatch (-want +got):\n%s", diff)
  }
 }