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)
 	}
 }