From a768023365dc088058c1ac659c3976fdb3ff0dda Mon Sep 17 00:00:00 2001 From: b5 Date: Fri, 18 Oct 2019 10:54:11 -0400 Subject: [PATCH] feat(remote): populate LogPushFinalCheck dataset reference --- logbook/logbook.go | 27 ++++++++++++++++++++- logbook/logbook_test.go | 49 ++++++++++++++++++++++++++++++++++++++ logbook/logsync/logsync.go | 9 +++---- remote/remote_test.go | 18 +++++++++++--- 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/logbook/logbook.go b/logbook/logbook.go index 8c3c7fdd7..6ec3d7a39 100644 --- a/logbook/logbook.go +++ b/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 { diff --git a/logbook/logbook_test.go b/logbook/logbook_test.go index 3982ebbd1..1ce0a8c63 100644 --- a/logbook/logbook_test.go +++ b/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() diff --git a/logbook/logsync/logsync.go b/logbook/logsync/logsync.go index 97dbdf691..48a735b71 100644 --- a/logbook/logsync/logsync.go +++ b/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 } diff --git a/remote/remote_test.go b/remote/remote_test.go index c9a802ba1..fd63f0753 100644 --- a/remote/remote_test.go +++ b/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) } }