diff --git a/api/datasets.go b/api/datasets.go index 398ddd2df..643fbf1b4 100644 --- a/api/datasets.go +++ b/api/datasets.go @@ -380,10 +380,9 @@ func (h *DatasetHandlers) diffHandler(w http.ResponseWriter, r *http.Request) { return } - res, err := h.Diff(r.Context(), params) + res, _, err := h.inst.Dispatch(r.Context(), "dataset.diff", params) if err != nil { - fmt.Println(err) - util.WriteErrResponse(w, http.StatusInternalServerError, fmt.Errorf("error generating diff: %s", err.Error())) + util.RespondWithError(w, err) return } diff --git a/config/test/config.go b/config/test/config.go index 00206314e..271ad808d 100644 --- a/config/test/config.go +++ b/config/test/config.go @@ -1,6 +1,7 @@ package test import ( + "github.com/qri-io/qfs" testkeys "github.com/qri-io/qri/auth/key/test" "github.com/qri-io/qri/config" ) @@ -19,7 +20,25 @@ func DefaultConfigForTesting() *config.Config { return cfg } -// DefaultProfileForTesting constructs a profile with precomputed keys, only used for testing. +// DefaultMemConfigForTesting constructs a config with precomputed keys & settings for an in memory repo & fs, only used for testing. +func DefaultMemConfigForTesting() *config.Config { + cfg := DefaultConfigForTesting().Copy() + + // add settings for in-mem filesystem + cfg.Filesystems = []qfs.Config{ + {Type: "mem"}, + {Type: "local"}, + {Type: "http"}, + } + + // add settings for in mem repo + cfg.Repo.Type = "mem" + + return cfg +} + +// DefaultProfileForTesting constructs a profile with precompted keys, only used for testing. +// TODO (b5): move this into a new profile/test package func DefaultProfileForTesting() *config.ProfilePod { kd := testkeys.GetKeyData(0) pro := config.DefaultProfile() diff --git a/lib/datasets_test.go b/lib/datasets_test.go index ba8d5f749..84d46cb81 100644 --- a/lib/datasets_test.go +++ b/lib/datasets_test.go @@ -794,14 +794,14 @@ func TestRenameNoHistory(t *testing.T) { if err != nil { t.Fatal(err) } - - if refstr != "peer/rename_no_history" { - t.Errorf("init returned bad refstring %q", refstr) + username := tr.MustOwner(t).Peername + expect := fmt.Sprintf("%s/rename_no_history", username) + if refstr != expect { + t.Errorf("dataset init refstring mismatch, expected %q, got %q", expect, refstr) } // Read .qri-ref file, it contains the reference this directory is linked to actual := tr.MustReadFile(t, filepath.Join(workDir, ".qri-ref")) - expect := "peer/rename_no_history" if diff := cmp.Diff(expect, actual); diff != "" { t.Errorf("qri list (-want +got):\n%s", diff) } @@ -818,7 +818,7 @@ func TestRenameNoHistory(t *testing.T) { // Read .qri-ref file, it contains the new reference name actual = tr.MustReadFile(t, filepath.Join(workDir, ".qri-ref")) - expect = "peer/rename_second_name" + expect = fmt.Sprintf("%s/rename_second_name", username) if diff := cmp.Diff(expect, actual); diff != "" { t.Errorf("qri list (-want +got):\n%s", diff) } diff --git a/lib/diff.go b/lib/diff.go index 01869da89..e2b7eb1b9 100644 --- a/lib/diff.go +++ b/lib/diff.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/qri-io/dataset/tabular" "github.com/qri-io/deepdiff" "github.com/qri-io/qfs" "github.com/qri-io/qri/base/component" @@ -117,16 +118,42 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse return nil, err } } + got, _, err := m.inst.Dispatch(ctx, dispatchMethodName(m, "diff"), p) + if res, ok := got.(*DiffResponse); ok { + return res, err + } + return nil, dispatchReturnError(got, err) +} - res := &DiffResponse{} +func schemaDiff(ctx context.Context, left, right *component.BodyComponent) ([]*Delta, *DiffStat, error) { + dd := deepdiff.New() + if left.Format == ".csv" && right.Format == ".csv" { + left, _, err := tabular.ColumnsFromJSONSchema(left.InferredSchema) + if err != nil { + return nil, nil, err + } - if m.inst.http != nil { - err := m.inst.http.Call(ctx, AEDiff, p, &res) + right, _, err := tabular.ColumnsFromJSONSchema(right.InferredSchema) if err != nil { - return nil, err + return nil, nil, err } - return res, nil + + return dd.StatDiff(ctx, left.Titles(), right.Titles()) + } + return dd.StatDiff(ctx, left.InferredSchema, right.InferredSchema) +} + +// assume a non-empty string, which isn't a dataset reference, is a file +func isFilePath(text string) bool { + if text == "" { + return false } + return !dsref.IsRefString(text) +} + +// Diff computes the diff of two source +func (datasetImpl) Diff(scope scope, p *DiffParams) (*DiffResponse, error) { + res := &DiffResponse{} diffMode, err := p.diffMode() if err != nil { @@ -147,13 +174,13 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse return nil, err } - res.Schema, res.SchemaStat, err = schemaDiff(ctx, leftComp, rightComp) + res.Schema, res.SchemaStat, err = schemaDiff(scope.Context(), leftComp, rightComp) if err != nil { return nil, err } dd := deepdiff.New() - res.Diff, res.Stat, err = dd.StatDiff(ctx, leftData, rightData) + res.Diff, res.Stat, err = dd.StatDiff(scope.Context(), leftData, rightData) if err != nil { return nil, err } @@ -161,11 +188,8 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse } // Left side of diff loaded into a component - parseResolveLoad, err := m.inst.NewParseResolveLoadFunc(p.Remote) - if err != nil { - return nil, err - } - ds, err := parseResolveLoad(ctx, p.LeftSide) + parseResolveLoad := scope.ParseResolveFunc() + ds, err := parseResolveLoad(scope.Context(), p.LeftSide) if err != nil { if errors.Is(err, dsref.ErrNoHistory) { return nil, qerr.New(err, fmt.Sprintf("dataset %s has no versions, nothing to diff against", p.LeftSide)) @@ -176,7 +200,7 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse // calling ds.DropDerivedValues is overzealous. investigate the right solution ds.Name = "" ds.Peername = "" - leftComp := component.ConvertDatasetToComponents(ds, m.inst.repo.Filesystem()) + leftComp := component.ConvertDatasetToComponents(ds, scope.Filesystem()) // Right side of diff laoded into a component var rightComp component.Component @@ -188,7 +212,7 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse if err != nil { return nil, err } - err = component.ExpandListedComponents(rightComp, m.inst.repo.Filesystem()) + err = component.ExpandListedComponents(rightComp, scope.Filesystem()) if err != nil { return nil, err } @@ -205,13 +229,13 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse if ds.PreviousPath == "" { return nil, fmt.Errorf("dataset has only one version, nothing to diff against") } - ds, err = dsfs.LoadDataset(ctx, m.inst.repo.Filesystem(), ds.PreviousPath) + ds, err = dsfs.LoadDataset(scope.Context(), scope.Filesystem(), ds.PreviousPath) if err != nil { return nil, err } - leftComp = component.ConvertDatasetToComponents(ds, m.inst.repo.Filesystem()) + leftComp = component.ConvertDatasetToComponents(ds, scope.Filesystem()) case DatasetRefDiffMode: - ds, err = parseResolveLoad(ctx, p.RightSide) + ds, err = parseResolveLoad(scope.Context(), p.RightSide) if err != nil { return nil, err } @@ -219,7 +243,7 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse // calling ds.DropDerivedValues is overzealous. investigate the right solution ds.Name = "" ds.Peername = "" - rightComp = component.ConvertDatasetToComponents(ds, m.inst.repo.Filesystem()) + rightComp = component.ConvertDatasetToComponents(ds, scope.Filesystem()) } // If in an FSI linked working directory, drop derived values, since the user is not @@ -299,61 +323,9 @@ func (m *DatasetMethods) Diff(ctx context.Context, p *DiffParams) (*DiffResponse } dd := deepdiff.New() - res.Diff, res.Stat, err = dd.StatDiff(ctx, leftData, rightData) + res.Diff, res.Stat, err = dd.StatDiff(scope.Context(), leftData, rightData) if err != nil { return nil, err } return res, nil } - -func schemaDiff(ctx context.Context, left, right *component.BodyComponent) ([]*Delta, *DiffStat, error) { - dd := deepdiff.New() - if left.Format == ".csv" && right.Format == ".csv" { - left, err := terribleHackToGetHeaderRow(left.InferredSchema) - if err != nil { - return nil, nil, err - } - - right, err := terribleHackToGetHeaderRow(right.InferredSchema) - if err != nil { - return nil, nil, err - } - - return dd.StatDiff(ctx, left, right) - } - return dd.StatDiff(ctx, left.InferredSchema, right.InferredSchema) -} - -// TODO (b5) - this is terrible. We need better logic error handling for -// jsonschemas describing CSV data. We're relying too heavily on the schema -// being well-formed -func terribleHackToGetHeaderRow(sch map[string]interface{}) ([]string, error) { - if itemObj, ok := sch["items"].(map[string]interface{}); ok { - if itemArr, ok := itemObj["items"].([]interface{}); ok { - titles := make([]string, len(itemArr)) - for i, f := range itemArr { - if field, ok := f.(map[string]interface{}); ok { - if title, ok := field["title"].(string); ok { - titles[i] = title - } - } - } - return titles, nil - } - } - log.Debug("that terrible hack to detect header row & types just failed") - return nil, fmt.Errorf("nope") -} - -// assume a non-empty string, which isn't a dataset reference, is a file -func isFilePath(text string) bool { - if text == "" { - return false - } - return !dsref.IsRefString(text) -} - -// Diff computes the diff of two source -func (datasetImpl) Diff(scope scope, p *DiffParams) (*DiffResponse, error) { - return nil, fmt.Errorf("not yet implemented") -} diff --git a/lib/diff_test.go b/lib/diff_test.go index 841748aeb..4cfcb0c78 100644 --- a/lib/diff_test.go +++ b/lib/diff_test.go @@ -220,7 +220,7 @@ func TestDiff(t *testing.T) { // compared with cmp.Diff. // TODO(dustmop): Would be better if Diff only returned the changes, instead of things that // stay the same, since the delta in this case is pretty small. - expect := `{"stat":{"leftNodes":102,"rightNodes":112,"leftWeight":3814,"rightWeight":4170,"inserts":51,"deletes":25},"diff":[["-","bodyPath","/mem/Qmc7AoCfFVW5xe8qhyjNYewSgBHFubp6yLM3mfBzQp7iTr"],["+","bodyPath","/mem/QmYuVj1JvALB9Au5DNcVxGLMcWCDBUfbKCN3QbpvissSC4"],[" ","commit",null,[[" ","author",{"id":"QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt"}],["-","message","created dataset from body.csv"],["+","message","created dataset from body_more.csv"],["-","path","/mem/QmPuJT13PENgG6yQQsC5SxRickT8CfBGdP27Q1DFzYbR1U"],["+","path","/mem/QmQm7BGbSbjXSXJopZG23j1ruAYHNPMGk5rsbsvaJ2AKH3"],[" ","qri","cm:0"],["-","signature","aO6TIie3VfmC8fvzZL4BulTUYd98nEMzvXrsw8Ght3Tt4vDEaZ58v1iogI1BHVX4lVLlVIJn2F5JK1vZEGB/Du8Y9qvZi1VPvZIT4kRBG4KcGezNHQmiQidk9GgnuqgVVW+wPgDyAwrP+FD2B/3U+UXTeqUKiTeaYEiS+8odqnBWkA0lNjkIROVz3Q7bOmKUZePzvCDONv4zugpLgKSPNuEbAv8qf2rVZLzrOiA7C1z1dh1kgrfv+RNgrNwWL6PmdfebE2ND5kr9X6qesZamUXyrK97Zb/KDZwEOsDPUQlQYdXlzvi0yx+o2V7vnwbkUN6PO9ytmOsh3egL7TTJH1A=="],["+","signature","M1RDiczU7mze3siJoi5BCi73oV7c0bXisPXYlZD11fvXeCCiu+NYvUMcDIOB5u/k/CNn4zi51NX503mxy05wS6I7D5FDl6GME9qyiky8KY5vDobgFBq93ht+p+/arTo2G0a0FdWgaQf9c0YWid1xTbBlZ2ED558AZFBnH7QHhtZRc5YDRPVAEMP/0R9pgDsM4lfo52cOfIxmg7cRkcjfvJ6pkTRPMDNu/M1ZvveF0fq5sMMJ+8joA0Kh9IkLU2CiwxfkFKcQi2fDcixjpFoikyRqQbLYATMRDrB+s18eSMMO5AKMp41WonBGBaAv+j0RGEONtJBaN7pmQU5od3Y9Tw=="],["-","timestamp","2001-01-01T01:01:01.000000001Z"],["+","timestamp","2001-01-01T01:02:01.000000001Z"],["-","title","created dataset from body.csv"],["+","title","created dataset from body_more.csv"]]],["-","path","/mem/QmQqnpYhA7UfcyCLkupkd35VLY8AL9XSQEtY5QhhLvV5jj"],["+","path","/mem/QmS6G5QpWtdHGeQMT3Nn2pqmdvQYJPSN8L7DyxVcmqu342"],[" ","qri","ds:0"],[" ","stats",null,[["-","path","/mem/QmVvv9vBHLsYbDYzG1G931Pi58gTPdVE4hcUsxb6rAU8S9"],["+","path","/mem/QmcLRdnni9jpvhACHN9LmUsDuT2iiqpEhQ1ZqxdLK5ELr6"],[" ","qri","sa:0"],[" ","stats",null,[[" ",0,null,[["-","count",5],["+","count",7],[" ","frequencies",null,[[" ","chatham",1],[" ","chicago",1],["+","los angeles",1],["+","mexico city",1],[" ","new york",1],[" ","raleigh",1],[" ","toronto",1]]],["-","maxLength",8],["+","maxLength",11],[" ","minLength",7],[" ","type","string"],["-","unique",5],["+","unique",7]]],[" ",1,null,[["-","count",5],["+","count",7],[" ","histogram",null,[[" ","bins",null,[["+",0,35000],[" ",1,250000],["+",2,300000],["+",3,3990000],[" ",4,8500000],["+",5,50000000],["+",6,70000000],["+",7,70000001]]],[" ","frequencies",null,[["+",0,1],["+",1,1],["+",2,1],["+",3,1],["+",4,1],["+",5,1],["+",6,1]]]]],["-","max",50000000],["+","max",70000000],["-","mean",11817000],["+","mean",19010714.285714287],["-","median",300000],["+","median",3990000],[" ","min",35000],[" ","type","numeric"]]],[" ",2,null,[["-","count",5],["+","count",7],[" ","histogram",null,[[" ","bins",null,[["+",0,28.6],["+",1,42.7],["+",2,44.4],["+",3,50.65],[" ",4,55.5],["+",5,65.25],[" ",6,66.25]]],[" ","frequencies",null,[["+",0,1],["+",1,1],["+",2,2],["+",3,1],["+",4,1],["+",5,1]]]]],[" ","max",65.25],["-","mean",52.04],["+","mean",47.357142857142854],[" ","median",50.65],["-","min",44.4],["+","min",28.6],[" ","type","numeric"]]],[" ",3,null,[["-","count",5],["+","count",7],["-","falseCount",1],["+","falseCount",2],["-","trueCount",4],["+","trueCount",5],[" ","type","boolean"]]]]]]],[" ","structure",null,[["-","checksum","/mem/Qmc7AoCfFVW5xe8qhyjNYewSgBHFubp6yLM3mfBzQp7iTr"],["+","checksum","/mem/QmYuVj1JvALB9Au5DNcVxGLMcWCDBUfbKCN3QbpvissSC4"],[" ","depth",2],["-","entries",5],["+","entries",7],[" ","format","csv"],[" ","formatConfig",{"headerRow":true,"lazyQuotes":true}],["-","length",155],["+","length",217],["-","path","/mem/QmX3HjmvFGYXavQiPqpJvAZZ14J1DNPjCCGEzEy9NgZq2J"],["+","path","/mem/QmNpWHSFo8xwNQyamsaYAqUKu7Nzd3J1a4MukyNVf4J1xt"],[" ","qri","st:0"],[" ","schema",{"items":{"items":[{"title":"city","type":"string"},{"title":"pop","type":"integer"},{"title":"avg_age","type":"number"},{"title":"in_usa","type":"boolean"}],"type":"array"},"type":"array"}]]]]}` + expect := `{"stat":{"leftNodes":102,"rightNodes":112,"leftWeight":3814,"rightWeight":4170,"inserts":51,"deletes":25},"diff":[["-","bodyPath","/mem/Qmc7AoCfFVW5xe8qhyjNYewSgBHFubp6yLM3mfBzQp7iTr"],["+","bodyPath","/mem/QmYuVj1JvALB9Au5DNcVxGLMcWCDBUfbKCN3QbpvissSC4"],[" ","commit",null,[[" ","author",{"id":"QmeL2mdVka1eahKENjehK6tBxkkpk5dNQ1qMcgWi7Hrb4B"}],["-","message","created dataset from body.csv"],["+","message","created dataset from body_more.csv"],["-","path","/mem/QmTKLXhW5CHD9a8o4UwWo6EJ4X7Y2h6h2jsDRKw5tU54jS"],["+","path","/mem/QmQRf5y4L6Hn37a1bf1nkWYDh9wUeUcs6XKoi5v2v2zKjV"],[" ","qri","cm:0"],["-","signature","fYfAWIzpiArVi5g+Ls9dA2KLdcSqbqqBQTF/QVFAVA4IZ01T3gJVwDot6scb7twaM6tX2uihynMz6n1xsZi/x7TZFg7VZlACV/RRG93iyW2OcaKlvXgzqgW4HpNvEdaUvN5l59nRtf10lfoL8jN1SVKI6vlhtZtT0ETLliNA83JEfBCwrWk0ftf/lKFBcbLUv0sI0x8km2gV7fTvAHOeTjiCO5Ya+2ID9AI8TN8AMEAltRkyEQMDankrHg8wEnOwadipZM2/qn0wsZae7b/n2T1JNJPs78tVjC95/7AW+JEqrlhtGpwqX2d5t4vbSMvC1vB/gf8nRwh9PVIaz9ZoTg=="],["+","signature","ibX3sNj08nbcKES2aGXkVk+ZMKgmZEdtnB8Q5be4v4y9DhRrRWUbR2R3XwOTs0vfv3wMcsHqp9jY/v7k/HNnNsCqg0PBiYGdXkvEWr2sjqJbG6A+0x5Bkqzkxf2FhrUqzqx4wMvJYZsgwSf7UDWNh/A/1wybt8jR7P2tT7IWLKbwaWJWhG5nk2NvlkL3nQIVLBApJaYCQhjolAp0n2q82jgD9x1dzs2qgV3oSyVqBdWCCg9GY/q9QuqmQHM/wyxJajXo8gGV0LDgPh39sZfqRqQjJ8koszCTiYl1b9np41GBFkKBn0KRIvSGJrE62NS87YkIi4wOib66XjoY8jaA9w=="],["-","timestamp","2001-01-01T01:01:01.000000001Z"],["+","timestamp","2001-01-01T01:02:01.000000001Z"],["-","title","created dataset from body.csv"],["+","title","created dataset from body_more.csv"]]],["-","path","/mem/QmbTMgT154t4NnP4H2FXBPcec7D85HrJKnBmZKWvRsYCtS"],["+","path","/mem/QmXQ6L58wzoG57QkLGp6iQtd24PGbnnRk9kPvaE1QPXzcq"],[" ","qri","ds:0"],[" ","stats",null,[["-","path","/mem/QmVvv9vBHLsYbDYzG1G931Pi58gTPdVE4hcUsxb6rAU8S9"],["+","path","/mem/QmcLRdnni9jpvhACHN9LmUsDuT2iiqpEhQ1ZqxdLK5ELr6"],[" ","qri","sa:0"],[" ","stats",null,[[" ",0,null,[["-","count",5],["+","count",7],[" ","frequencies",null,[[" ","chatham",1],[" ","chicago",1],["+","los angeles",1],["+","mexico city",1],[" ","new york",1],[" ","raleigh",1],[" ","toronto",1]]],["-","maxLength",8],["+","maxLength",11],[" ","minLength",7],[" ","type","string"],["-","unique",5],["+","unique",7]]],[" ",1,null,[["-","count",5],["+","count",7],[" ","histogram",null,[[" ","bins",null,[["+",0,35000],[" ",1,250000],["+",2,300000],["+",3,3990000],[" ",4,8500000],["+",5,50000000],["+",6,70000000],["+",7,70000001]]],[" ","frequencies",null,[["+",0,1],["+",1,1],["+",2,1],["+",3,1],["+",4,1],["+",5,1],["+",6,1]]]]],["-","max",50000000],["+","max",70000000],["-","mean",11817000],["+","mean",19010714.285714287],["-","median",300000],["+","median",3990000],[" ","min",35000],[" ","type","numeric"]]],[" ",2,null,[["-","count",5],["+","count",7],[" ","histogram",null,[[" ","bins",null,[["+",0,28.6],["+",1,42.7],["+",2,44.4],["+",3,50.65],[" ",4,55.5],["+",5,65.25],[" ",6,66.25]]],[" ","frequencies",null,[["+",0,1],["+",1,1],["+",2,2],["+",3,1],["+",4,1],["+",5,1]]]]],[" ","max",65.25],["-","mean",52.04],["+","mean",47.357142857142854],[" ","median",50.65],["-","min",44.4],["+","min",28.6],[" ","type","numeric"]]],[" ",3,null,[["-","count",5],["+","count",7],["-","falseCount",1],["+","falseCount",2],["-","trueCount",4],["+","trueCount",5],[" ","type","boolean"]]]]]]],[" ","structure",null,[["-","checksum","/mem/Qmc7AoCfFVW5xe8qhyjNYewSgBHFubp6yLM3mfBzQp7iTr"],["+","checksum","/mem/QmYuVj1JvALB9Au5DNcVxGLMcWCDBUfbKCN3QbpvissSC4"],[" ","depth",2],["-","entries",5],["+","entries",7],[" ","format","csv"],[" ","formatConfig",{"headerRow":true,"lazyQuotes":true}],["-","length",155],["+","length",217],["-","path","/mem/QmX3HjmvFGYXavQiPqpJvAZZ14J1DNPjCCGEzEy9NgZq2J"],["+","path","/mem/QmNpWHSFo8xwNQyamsaYAqUKu7Nzd3J1a4MukyNVf4J1xt"],[" ","qri","st:0"],[" ","schema",{"items":{"items":[{"title":"city","type":"string"},{"title":"pop","type":"integer"},{"title":"avg_age","type":"number"},{"title":"in_usa","type":"boolean"}],"type":"array"},"type":"array"}]]]]}` if diff := cmp.Diff(expect, output); diff != "" { t.Errorf("output mismatch (-want +got):\n%s", diff) } diff --git a/lib/fsi_test.go b/lib/fsi_test.go index 58e1d1844..39adc59d7 100644 --- a/lib/fsi_test.go +++ b/lib/fsi_test.go @@ -206,9 +206,9 @@ func TestDscacheCheckout(t *testing.T) { actual := run.NiceifyTempDirs(cache.VerboseString(false)) expect := dstest.Template(t, `Dscache: Dscache.Users: - 0) user=peer profileID={{ .profileID }} + 0) user=default_profile_for_testing profileID={{ .profileID }} Dscache.Refs: - 0) initID = vrh4iurbzeyx42trlddzvtoiqevmy2d3mxex4ojd4mxv7cudhlwq + 0) initID = brblgyckelk7fsmt7bgxq6grjaslaey7z32wvq3dzcvtl2hvgy3q profileID = {{ .profileID }} topIndex = 1 cursorIndex = 1 @@ -219,8 +219,8 @@ func TestDscacheCheckout(t *testing.T) { headRef = {{ .headRef }} fsiPath = /tmp/cities_ds `, map[string]string{ - "profileID": "QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt", - "headRef": "/mem/QmQqnpYhA7UfcyCLkupkd35VLY8AL9XSQEtY5QhhLvV5jj", + "profileID": "QmeL2mdVka1eahKENjehK6tBxkkpk5dNQ1qMcgWi7Hrb4B", + "headRef": "/mem/QmbTMgT154t4NnP4H2FXBPcec7D85HrJKnBmZKWvRsYCtS", }) if diff := cmp.Diff(expect, actual); diff != "" { t.Errorf("result mismatch (-want +got):%s\n", diff) @@ -258,9 +258,9 @@ func TestDscacheInit(t *testing.T) { actual := run.NiceifyTempDirs(cache.VerboseString(false)) expect := dstest.Template(t, `Dscache: Dscache.Users: - 0) user=peer profileID={{ .profileID }} + 0) user=default_profile_for_testing profileID={{ .profileID }} Dscache.Refs: - 0) initID = vrh4iurbzeyx42trlddzvtoiqevmy2d3mxex4ojd4mxv7cudhlwq + 0) initID = brblgyckelk7fsmt7bgxq6grjaslaey7z32wvq3dzcvtl2hvgy3q profileID = {{ .profileID }} topIndex = 1 cursorIndex = 1 @@ -269,7 +269,7 @@ func TestDscacheInit(t *testing.T) { bodyRows = 5 commitTime = 978310861 headRef = {{ .citiesHeadRef }} - 1) initID = ekwzgcu4s4o4xchsoip3oa3j45ko5n7pybtizgvsbudojbhxuita + 1) initID = bin777fera6wcqzthjzmitfipd5p5myda55qgau2eirba4ynu7ia profileID = {{ .profileID }} topIndex = 0 cursorIndex = 0 @@ -277,8 +277,8 @@ func TestDscacheInit(t *testing.T) { commitTime = -62135596800 fsiPath = /tmp/json_body `, map[string]string{ - "profileID": "QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt", - "citiesHeadRef": "/mem/QmQqnpYhA7UfcyCLkupkd35VLY8AL9XSQEtY5QhhLvV5jj", + "profileID": "QmeL2mdVka1eahKENjehK6tBxkkpk5dNQ1qMcgWi7Hrb4B", + "citiesHeadRef": "/mem/QmbTMgT154t4NnP4H2FXBPcec7D85HrJKnBmZKWvRsYCtS", }) if diff := cmp.Diff(expect, actual); diff != "" { diff --git a/lib/lib.go b/lib/lib.go index ffeef5be1..436c6d242 100644 --- a/lib/lib.go +++ b/lib/lib.go @@ -75,9 +75,11 @@ type InstanceOptions struct { node *p2p.QriNode repo repo.Repo qfs *muxfs.Mux + dscache *dscache.Dscache regclient *regclient.Client logbook *logbook.Book profiles profile.Store + bus event.Bus logAll bool remoteMockClient bool @@ -294,6 +296,22 @@ func OptProfiles(pros profile.Store) Option { } } +// OptBus overrides the configured `event.Bus` with a manually provided one +func OptBus(bus event.Bus) Option { + return func(o *InstanceOptions) error { + o.bus = bus + return nil + } +} + +// OptDscache overrides the configured `dscache.Dscache` with a manually provided one +func OptDscache(dscache *dscache.Dscache) Option { + return func(o *InstanceOptions) error { + o.dscache = dscache + return nil + } +} + // NewInstance creates a new Qri Instance, if no Option funcs are provided, // New uses a default set of Option funcs. Any Option functions passed to this // function must check whether their fields are nil or not. @@ -376,8 +394,9 @@ func NewInstance(ctx context.Context, repoPath string, opts ...Option) (qri *Ins streams: o.Streams, registry: o.regclient, logbook: o.logbook, + dscache: o.dscache, profiles: o.profiles, - bus: event.NewBus(ctx), + bus: o.bus, appCtx: ctx, } qri = inst @@ -420,6 +439,10 @@ func NewInstance(ctx context.Context, repoPath string, opts ...Option) (qri *Ins } } + if inst.bus == nil { + inst.bus = newEventBus(ctx) + } + if o.eventHandler != nil && o.events != nil { inst.bus.SubscribeTypes(o.eventHandler, o.events...) } @@ -465,6 +488,14 @@ func NewInstance(ctx context.Context, repoPath string, opts ...Option) (qri *Ins inst.registry = newRegClient(ctx, cfg) } + if inst.dscache == nil { + inst.dscache, err = newDscache(ctx, inst.qfs, inst.bus, pro.Peername, inst.repoPath) + if err != nil { + log.Error("initalizing dscache:", err.Error()) + return nil, fmt.Errorf("newDsache: %w", err) + } + } + if inst.repo == nil { if inst.repo, err = buildrepo.New(ctx, inst.repoPath, cfg, func(o *buildrepo.Options) { o.Bus = inst.bus @@ -491,13 +522,6 @@ func NewInstance(ctx context.Context, repoPath string, opts ...Option) (qri *Ins } } - if inst.dscache == nil { - inst.dscache, err = newDscache(ctx, inst.qfs, inst.bus, pro.Peername, inst.repoPath) - if err != nil { - return nil, fmt.Errorf("newDsache: %w", err) - } - } - if inst.node == nil { var localResolver dsref.Resolver localResolver, err = inst.resolverForMode("local") @@ -663,11 +687,12 @@ func NewInstanceFromConfigAndNodeAndBus(ctx context.Context, cfg *config.Config, cancel: cancel, doneCh: make(chan struct{}), - cfg: cfg, - node: node, - dscache: dc, - logbook: r.Logbook(), - appCtx: ctx, + cfg: cfg, + node: node, + dscache: dc, + logbook: r.Logbook(), + profiles: r.Profiles(), + appCtx: ctx, } inst.RegisterMethods() @@ -936,6 +961,10 @@ func (inst *Instance) activeProfile(ctx context.Context) (pro *profile.Profile, return inst.profiles.Owner(), nil } + if pro == nil { + return nil, fmt.Errorf("no active profile") + } + return pro, err } diff --git a/lib/scope.go b/lib/scope.go index 8efefa715..413a88903 100644 --- a/lib/scope.go +++ b/lib/scope.go @@ -130,7 +130,7 @@ func (s *scope) ParseAndResolveRefWithWorkingDir(ctx context.Context, refstr, so // ParseResolveFunc returns a function that can parse a ref, then resolve and load it func (s *scope) ParseResolveFunc() dsref.ParseResolveLoad { - return NewParseResolveLoadFunc("", s.inst.defaultResolver(), s.inst) + return NewParseResolveLoadFunc(s.ActiveProfile().Peername, s.inst.defaultResolver(), s.inst) } // Profiles accesses the profile store diff --git a/lib/test_runner_test.go b/lib/test_runner_test.go index 93846789a..319aaa583 100644 --- a/lib/test_runner_test.go +++ b/lib/test_runner_test.go @@ -15,16 +15,12 @@ import ( "github.com/qri-io/qri/base/dsfs" testcfg "github.com/qri-io/qri/config/test" "github.com/qri-io/qri/dsref" - "github.com/qri-io/qri/event" "github.com/qri-io/qri/logbook" - "github.com/qri-io/qri/p2p" "github.com/qri-io/qri/profile" - testrepo "github.com/qri-io/qri/repo/test" ) type testRunner struct { Ctx context.Context - Profile *profile.Profile Instance *Instance Pwd string TmpDir string @@ -54,34 +50,35 @@ func newTestRunner(t *testing.T) *testRunner { t.Fatal(err) } - bus := event.NewBus(ctx) - // A temporary directory for doing filesystem work. tmpDir, err := ioutil.TempDir("", "lib_test_runner") if err != nil { t.Fatal(err) } - mr, err := testrepo.NewEmptyTestRepo(bus) - if err != nil { - t.Fatalf("error allocating test repo: %s", err.Error()) - } - localResolver := dsref.SequentialResolver(mr.Dscache(), mr) - node, err := p2p.NewQriNode(mr, testcfg.DefaultP2PForTesting(), bus, localResolver) - if err != nil { - t.Fatal(err.Error()) + qriPath := filepath.Join(tmpDir, "qri") + if err := os.Mkdir(qriPath, os.ModePerm); err != nil { + t.Fatal(err) } + cfg := testcfg.DefaultMemConfigForTesting() - inst := NewInstanceFromConfigAndNodeAndBus(ctx, testcfg.DefaultConfigForTesting(), node, bus) - // Assign the repoPath, as some tests inspect it in order to save files in the repository. - // Not assigning a path will cause them to use the current directory, and then save - // into the sourcetree. - inst.repoPath = tmpDir + // ensure we create a mock registry client for testing + key := InstanceContextKey("RemoteClient") + ctx = context.WithValue(ctx, key, "mock") + // create new instance! + inst, err := NewInstance( + ctx, + // NewInstance requires a qriPath, even if the repo & all stores are in mem + qriPath, + OptConfig(cfg), + ) + if err != nil { + t.Fatal(err) + } return &testRunner{ Ctx: ctx, // TODO (b5) - move test profile creation into testRunner constructor - Profile: testPeerProfile, Instance: inst, TmpDir: tmpDir, Pwd: pwd, @@ -97,6 +94,14 @@ func (tr *testRunner) Delete() { os.RemoveAll(tr.TmpDir) } +func (tr *testRunner) MustOwner(t *testing.T) *profile.Profile { + owner, err := tr.Instance.activeProfile(tr.Ctx) + if err != nil { + t.Fatal(err) + } + return owner +} + func (tr *testRunner) MustReadFile(t *testing.T, filename string) string { data, err := ioutil.ReadFile(filename) if err != nil { @@ -153,8 +158,12 @@ func (tr *testRunner) MustSaveFromBody(t *testing.T, dsName, bodyFilename string if !dsref.IsValidName(dsName) { t.Fatalf("invalid dataset name: %q", dsName) } + pro, err := tr.Instance.activeProfile(tr.Ctx) + if err != nil { + t.Fatalf("error getting active profile: %s", err) + } p := SaveParams{ - Ref: fmt.Sprintf("peer/%s", dsName), + Ref: fmt.Sprintf("%s/%s", pro.Peername, dsName), BodyPath: bodyFilename, } res, err := tr.Instance.Dataset().Save(tr.Ctx, &p) diff --git a/repo/buildrepo/build.go b/repo/buildrepo/build.go index acdbd0b88..85c226e17 100644 --- a/repo/buildrepo/build.go +++ b/repo/buildrepo/build.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + golog "github.com/ipfs/go-log" "github.com/qri-io/qfs" "github.com/qri-io/qfs/muxfs" "github.com/qri-io/qri/auth/key" @@ -19,6 +20,8 @@ import ( fsrepo "github.com/qri-io/qri/repo/fs" ) +var log = golog.Logger("buildrepo") + // Options provides additional fields to new type Options struct { Profiles profile.Store @@ -43,26 +46,31 @@ func New(ctx context.Context, path string, cfg *config.Config, opts ...func(o *O var err error if o.Keystore == nil { + log.Debug("buildrepo.New: creating keystore") if o.Keystore, err = key.NewStore(cfg); err != nil { return nil, err } } if o.Profiles == nil { + log.Debug("buildrepo.New: creating profiles") if o.Profiles, err = profile.NewStore(cfg, o.Keystore); err != nil { return nil, err } } if o.Filesystem == nil { + log.Debug("buildrepo.New: creating filesystem") if o.Filesystem, err = NewFilesystem(ctx, cfg); err != nil { return nil, err } } if o.Bus == nil { + log.Debug("buildrepo.New: creating bus") o.Bus = event.NilBus } pro := o.Profiles.Owner() + log.Debug("buildrepo.New: profile %q, %q", pro.Peername, pro.ID) switch cfg.Repo.Type { case "fs": if o.Logbook == nil { @@ -78,7 +86,7 @@ func New(ctx context.Context, path string, cfg *config.Config, opts ...func(o *O return fsrepo.NewRepo(path, o.Filesystem, o.Logbook, o.Dscache, o.Profiles, o.Bus) case "mem": - return repo.NewMemRepo(ctx, o.Profiles, o.Filesystem, o.Bus) + return repo.NewMemRepo(ctx, o.Filesystem, o.Logbook, o.Dscache, o.Profiles, o.Bus) default: return nil, fmt.Errorf("unknown repo type: %s", cfg.Repo.Type) } diff --git a/repo/mem_repo.go b/repo/mem_repo.go index f4a326943..e5e63debe 100644 --- a/repo/mem_repo.go +++ b/repo/mem_repo.go @@ -45,24 +45,32 @@ func NewMemRepoWithProfile(ctx context.Context, owner *profile.Profile, fs *muxf if err != nil { return nil, err } - return NewMemRepo(ctx, pros, fs, bus) + return NewMemRepo(ctx, fs, nil, nil, pros, bus) } // NewMemRepo creates a new in-memory repository -func NewMemRepo(ctx context.Context, pros profile.Store, fs *muxfs.Mux, bus event.Bus) (*MemRepo, error) { +func NewMemRepo(ctx context.Context, fs *muxfs.Mux, book *logbook.Book, cache *dscache.Dscache, pros profile.Store, bus event.Bus) (*MemRepo, error) { + var err error if fs.Filesystem(qfs.MemFilestoreType) == nil { - fs.SetFilesystem(qfs.NewMemFS()) + err := fs.SetFilesystem(qfs.NewMemFS()) + if err != nil { + return nil, err + } } p := pros.Owner() - book, err := logbook.NewJournal(p.PrivKey, p.Peername, bus, fs, "/mem/logbook.qfb") - if err != nil { - return nil, err + if book == nil { + book, err = logbook.NewJournal(p.PrivKey, p.Peername, bus, fs, "/mem/logbook.qfb") + if err != nil { + return nil, err + } } - // NOTE: This dscache won't get change notifications from FSI, because it's not constructed - // with the hook for FSI. - cache := dscache.NewDscache(ctx, fs, bus, p.Peername, "") + if cache == nil { + // NOTE: This dscache won't get change notifications from FSI, because it's not constructed + // with the hook for FSI. + cache = dscache.NewDscache(ctx, fs, bus, p.Peername, "") + } mr := &MemRepo{ bus: bus,