diff --git a/cmd/noms/noms_commit_test.go b/cmd/noms/noms_commit_test.go index 03262f931e..d4cf3b1711 100644 --- a/cmd/noms/noms_commit_test.go +++ b/cmd/noms/noms_commit_test.go @@ -58,11 +58,11 @@ func (s *nomsCommitTestSuite) TestNomsCommitReadPathFromStdin() { stdinWriter.Write([]byte("#" + ref.TargetHash().String() + "\n")) stdinWriter.Close() }() - stdoutString, stderrString := s.MustRun(main, []string{"commit", sp.Spec}) + stdoutString, stderrString := s.MustRun(main, []string{"commit", sp.String()}) s.Empty(stderrString) s.Contains(stdoutString, "New head #") - sp, _ = spec.ForDataset(sp.Spec) + sp, _ = spec.ForDataset(sp.String()) defer sp.Close() commit, ok := sp.GetDataset().MaybeHead() @@ -81,11 +81,11 @@ func (s *nomsCommitTestSuite) TestNomsCommitToDatasetWithoutHead() { _, ok := sp.GetDataset().MaybeHead() s.False(ok, "should not have a commit") - stdoutString, stderrString := s.MustRun(main, []string{"commit", "#" + ref.TargetHash().String(), sp.Spec}) + stdoutString, stderrString := s.MustRun(main, []string{"commit", "#" + ref.TargetHash().String(), sp.String()}) s.Empty(stderrString) s.Contains(stdoutString, "New head #") - sp, _ = spec.ForDataset(sp.Spec) + sp, _ = spec.ForDataset(sp.String()) defer sp.Close() commit, ok := sp.GetDataset().MaybeHead() @@ -115,7 +115,7 @@ func (s *nomsCommitTestSuite) runDuplicateTest(allowDuplicate bool) { if allowDuplicate { cliOptions = append(cliOptions, "--allow-dupe=1") } - cliOptions = append(cliOptions, dsName+".value", sp.Spec) + cliOptions = append(cliOptions, dsName+".value", sp.String()) stdoutString, stderrString, err := s.Run(main, cliOptions) s.Nil(err) @@ -127,7 +127,7 @@ func (s *nomsCommitTestSuite) runDuplicateTest(allowDuplicate bool) { s.Contains(stdoutString, "Commit aborted") } - sp, _ = spec.ForDataset(sp.Spec) + sp, _ = spec.ForDataset(sp.String()) defer sp.Close() value, ok := sp.GetDataset().MaybeHeadValue() @@ -147,12 +147,12 @@ func (s *nomsCommitTestSuite) TestNomsCommitMetadata() { metaOld := sp.GetDataset().Head().Get(datas.MetaField).(types.Struct) - stdoutString, stderrString, err := s.Run(main, []string{"commit", "--allow-dupe=1", "--message=foo", dsName + ".value", sp.Spec}) + stdoutString, stderrString, err := s.Run(main, []string{"commit", "--allow-dupe=1", "--message=foo", dsName + ".value", sp.String()}) s.Nil(err) s.Empty(stderrString) s.Contains(stdoutString, "New head #") - sp, _ = spec.ForDataset(sp.Spec) + sp, _ = spec.ForDataset(sp.String()) defer sp.Close() metaNew := sp.GetDataset().Head().Get(datas.MetaField).(types.Struct) @@ -164,11 +164,11 @@ func (s *nomsCommitTestSuite) TestNomsCommitMetadata() { metaOld = metaNew - stdoutString, stderrString = s.MustRun(main, []string{"commit", "--allow-dupe=1", "--meta=message=bar", "--date=" + spec.CommitMetaDateFormat, dsName + ".value", sp.Spec}) + stdoutString, stderrString = s.MustRun(main, []string{"commit", "--allow-dupe=1", "--meta=message=bar", "--date=" + spec.CommitMetaDateFormat, dsName + ".value", sp.String()}) s.Empty(stderrString) s.Contains(stdoutString, "New head #") - sp, _ = spec.ForDataset(sp.Spec) + sp, _ = spec.ForDataset(sp.String()) defer sp.Close() metaNew = sp.GetDataset().Head().Get(datas.MetaField).(types.Struct) @@ -184,7 +184,7 @@ func (s *nomsCommitTestSuite) TestNomsCommitHashNotFound() { defer sp.Close() s.Panics(func() { - s.MustRun(main, []string{"commit", "#9ei6fbrs0ujo51vifd3f2eebufo4lgdu", sp.Spec}) + s.MustRun(main, []string{"commit", "#9ei6fbrs0ujo51vifd3f2eebufo4lgdu", sp.String()}) }) } @@ -193,7 +193,7 @@ func (s *nomsCommitTestSuite) TestNomsCommitMetadataBadDateFormat() { defer sp.Close() s.Panics(func() { - s.MustRun(main, []string{"commit", "--allow-dupe=1", "--date=a", "#" + ref.TargetHash().String(), sp.Spec}) + s.MustRun(main, []string{"commit", "--allow-dupe=1", "--date=a", "#" + ref.TargetHash().String(), sp.String()}) }) } @@ -202,7 +202,7 @@ func (s *nomsCommitTestSuite) TestNomsCommitInvalidMetadataPaths() { defer sp.Close() s.Panics(func() { - s.MustRun(main, []string{"commit", "--allow-dupe=1", "--meta-p=#beef", "#" + ref.TargetHash().String(), sp.Spec}) + s.MustRun(main, []string{"commit", "--allow-dupe=1", "--meta-p=#beef", "#" + ref.TargetHash().String(), sp.String()}) }) } @@ -211,6 +211,6 @@ func (s *nomsCommitTestSuite) TestNomsCommitInvalidMetadataFieldName() { defer sp.Close() s.Panics(func() { - s.MustRun(main, []string{"commit", "--allow-dupe=1", "--meta=_foo=bar", "#" + ref.TargetHash().String(), sp.Spec}) + s.MustRun(main, []string{"commit", "--allow-dupe=1", "--meta=_foo=bar", "#" + ref.TargetHash().String(), sp.String()}) }) } diff --git a/cmd/noms/noms_log_test.go b/cmd/noms/noms_log_test.go index 3041000c60..97c999a4ad 100644 --- a/cmd/noms/noms_log_test.go +++ b/cmd/noms/noms_log_test.go @@ -43,10 +43,10 @@ func (s *nomsLogTestSuite) TestNomsLog() { defer sp.Close() sp.GetDatabase() // create the database - s.Panics(func() { s.MustRun(main, []string{"log", sp.Spec}) }) + s.Panics(func() { s.MustRun(main, []string{"log", sp.String()}) }) - testCommitInResults(s, sp.Spec, 1) - testCommitInResults(s, sp.Spec, 2) + testCommitInResults(s, sp.String(), 1) + testCommitInResults(s, sp.String(), 2) } func addCommit(ds datas.Dataset, v string) (datas.Dataset, error) { diff --git a/cmd/noms/noms_merge_test.go b/cmd/noms/noms_merge_test.go index d1490dd951..f0f2f88abf 100644 --- a/cmd/noms/noms_merge_test.go +++ b/cmd/noms/noms_merge_test.go @@ -165,10 +165,10 @@ func (s *nomsMergeTestSuite) TestBadInput() { {[]string{"foo", "bar"}, "error: Incorrect number of arguments\n"}, {[]string{"foo", "bar", "baz"}, "error: Incorrect number of arguments\n"}, {[]string{"foo", "bar", "baz", "quux", "five"}, "error: Incorrect number of arguments\n"}, - {[]string{sp.Spec, l + "!!", r, o}, "error: Invalid dataset " + l + "!!, must match [a-zA-Z0-9\\-_/]+\n"}, - {[]string{sp.Spec, l + "2", r, o}, "error: Dataset " + l + "2 has no data\n"}, - {[]string{sp.Spec, l, r + "2", o}, "error: Dataset " + r + "2 has no data\n"}, - {[]string{sp.Spec, l, r, "!invalid"}, "error: Invalid dataset !invalid, must match [a-zA-Z0-9\\-_/]+\n"}, + {[]string{sp.String(), l + "!!", r, o}, "error: Invalid dataset " + l + "!!, must match [a-zA-Z0-9\\-_/]+\n"}, + {[]string{sp.String(), l + "2", r, o}, "error: Dataset " + l + "2 has no data\n"}, + {[]string{sp.String(), l, r + "2", o}, "error: Dataset " + r + "2 has no data\n"}, + {[]string{sp.String(), l, r, "!invalid"}, "error: Invalid dataset !invalid, must match [a-zA-Z0-9\\-_/]+\n"}, } db := sp.GetDatabase() diff --git a/go/config/resolver.go b/go/config/resolver.go index b4bd93c711..b46a13cdbf 100644 --- a/go/config/resolver.go +++ b/go/config/resolver.go @@ -112,7 +112,7 @@ func (r *Resolver) GetRootTracker(str string) (chunks.RootTracker, error) { } var rt chunks.RootTracker = sp.NewChunkStore() if rt == nil { - rt = datas.NewHTTPBatchStore(sp.Spec, "") + rt = datas.NewHTTPBatchStore(sp.String(), "") } return rt, nil } diff --git a/go/spec/absolute_path.go b/go/spec/absolute_path.go index 8da69b80ef..bf0c4d0fa9 100644 --- a/go/spec/absolute_path.go +++ b/go/spec/absolute_path.go @@ -16,15 +16,24 @@ import ( var datasetCapturePrefixRe = regexp.MustCompile("^(" + datas.DatasetRe.String() + ")") -// AbsolutePath represents a path originating at a dataset or a well-formed -// hash (i.e. '#' + 32 chars) representing a Noms Value that is independently -// addressable. Either the Dataset of Hash field will indicate the beginning of -// the AbsolutePath and the other one will be nil. The Path field holds the -// remainder of the path. +// AbsolutePath describes the location of a Value within a Noms database. +// +// To locate a value relative to some other value, see Path. To locate a value +// globally, see Spec. +// +// For more on paths, absolute paths, and specs, see: +// https://github.com/attic-labs/noms/blob/master/doc/spelling.md. type AbsolutePath struct { + // Dataset is the dataset this AbsolutePath is rooted at. Only one of + // Dataset and Hash should be set. Dataset string - Hash hash.Hash - Path types.Path + // Hash is the hash this AbsolutePath is rooted at. Only one of Dataset and + // Hash should be set. + Hash hash.Hash + // Path is the relative path from Dataset or Hash. This can be empty. In + // that case, the AbsolutePath describes the value at either Dataset or + // Hash. + Path types.Path } // NewAbsolutePath attempts to parse 'str' and return an AbsolutePath. @@ -98,6 +107,10 @@ func (p AbsolutePath) IsEmpty() bool { } func (p AbsolutePath) String() (str string) { + if p.IsEmpty() { + return "" + } + if len(p.Dataset) > 0 { str = p.Dataset } else if !p.Hash.IsEmpty() { diff --git a/go/spec/spec.go b/go/spec/spec.go index 89297a57d1..809fd61628 100644 --- a/go/spec/spec.go +++ b/go/spec/spec.go @@ -7,13 +7,13 @@ package spec import ( + "errors" "fmt" "net/url" "regexp" "strings" "github.com/attic-labs/noms/go/chunks" - "github.com/attic-labs/noms/go/d" "github.com/attic-labs/noms/go/datas" "github.com/attic-labs/noms/go/nbs" "github.com/attic-labs/noms/go/types" @@ -32,13 +32,8 @@ type SpecOptions struct { Authorization string } -// Spec describes a Database, Dataset, or a path to a Value. They should be -// constructed by parsing strings, either through ForDatabase, ForDataset, or -// ForPath (or their Opts variations). +// Spec locates a Noms database, dataset, or value globally. type Spec struct { - // Spec is the spec string this was parsed into. - Spec string - // Protocol is one of "mem", "ldb", "http", or "https". Protocol string @@ -49,9 +44,6 @@ type Spec struct { // Options are the SpecOptions that the Spec was constructed with. Options SpecOptions - // DatasetName is empty unless the spec was created with ForDataset. - DatasetName string - // Path is nil unless the spec was created with ForPath. Path AbsolutePath @@ -59,14 +51,13 @@ type Spec struct { db *datas.Database } -func newSpec(spec string, dbSpec string, opts SpecOptions) (Spec, error) { +func newSpec(dbSpec string, opts SpecOptions) (Spec, error) { protocol, dbName, err := parseDatabaseSpec(dbSpec) if err != nil { return Spec{}, err } return Spec{ - Spec: spec, Protocol: protocol, DatabaseName: dbName, Options: opts, @@ -81,7 +72,7 @@ func ForDatabase(spec string) (Spec, error) { // ForDatabaseOpts parses a spec for a Database. func ForDatabaseOpts(spec string, opts SpecOptions) (Spec, error) { - return newSpec(spec, spec, opts) + return newSpec(spec, opts) } // ForDataset parses a spec for a Dataset. @@ -91,21 +82,27 @@ func ForDataset(spec string) (Spec, error) { // ForDatasetOpts parses a spec for a Dataset. func ForDatasetOpts(spec string, opts SpecOptions) (Spec, error) { - dbSpec, dsName, err := splitDatabaseSpec(spec) + dbSpec, pathStr, err := splitDatabaseSpec(spec) + + sp, err := newSpec(dbSpec, opts) if err != nil { return Spec{}, err } - if !datasetRe.MatchString(dsName) { - return Spec{}, fmt.Errorf("Dataset %s must match %s", dsName, datasetRe.String()) - } - - sp, err := newSpec(spec, dbSpec, opts) + path, err := NewAbsolutePath(pathStr) if err != nil { return Spec{}, err } - sp.DatasetName = dsName + if path.Dataset == "" { + return Spec{}, errors.New("dataset name required for dataset spec") + } + + if !path.Path.IsEmpty() { + return Spec{}, errors.New("path is not allowed for dataset spec") + } + + sp.Path = path return sp, nil } @@ -121,12 +118,15 @@ func ForPathOpts(spec string, opts SpecOptions) (Spec, error) { return Spec{}, err } - path, err := NewAbsolutePath(pathStr) - if err != nil { - return Spec{}, err + var path AbsolutePath + if pathStr != "" { + path, err = NewAbsolutePath(pathStr) + if err != nil { + return Spec{}, err + } } - sp, err := newSpec(spec, dbSpec, opts) + sp, err := newSpec(dbSpec, opts) if err != nil { return Spec{}, err } @@ -135,6 +135,18 @@ func ForPathOpts(spec string, opts SpecOptions) (Spec, error) { return sp, nil } +func (sp Spec) String() string { + s := sp.Protocol + if s != "mem" { + s += ":" + sp.DatabaseName + } + p := sp.Path.String() + if p != "" { + s += Separator + p + } + return s +} + // GetDatabase returns the Database instance that this Spec's DatabaseName // describes. The same Database instance is returned every time, unless Close // is called. If the Spec is closed, it is re-opened with a new Database. @@ -178,8 +190,8 @@ func parseAWSSpec(awsURL string) chunks.ChunkStore { // new up-to-date Dataset will returned on the next call to GetDataset. If // this is not a Dataset spec, returns nil. func (sp Spec) GetDataset() (ds datas.Dataset) { - if sp.DatasetName != "" { - ds = sp.GetDatabase().GetDataset(sp.DatasetName) + if sp.Path.Dataset != "" { + ds = sp.GetDatabase().GetDataset(sp.Path.Dataset) } return } @@ -229,16 +241,11 @@ func (sp Spec) Pin() (Spec, bool) { return Spec{}, false } - spec := sp.Protocol + sp.DatabaseName + Separator + "#" + commit.Hash().String() - if sp.Path.Path != nil { - spec += sp.Path.Path.String() - } + r := sp + r.Path.Hash = commit.Hash() + r.Path.Dataset = "" - pinned, err := ForPathOpts(spec, sp.Options) - d.PanicIfError(err) - *pinned.db = *sp.db - - return pinned, true + return r, true } func (sp Spec) Close() error { diff --git a/go/spec/spec_test.go b/go/spec/spec_test.go index b00fa25026..8b5af4374d 100644 --- a/go/spec/spec_test.go +++ b/go/spec/spec_test.go @@ -26,7 +26,6 @@ func TestMemDatabaseSpec(t *testing.T) { assert.Equal("mem", spec.Protocol) assert.Equal("", spec.DatabaseName) - assert.Equal("", spec.DatasetName) assert.True(spec.Path.IsEmpty()) s := types.String("hello") @@ -44,8 +43,8 @@ func TestMemDatasetSpec(t *testing.T) { assert.Equal("mem", spec.Protocol) assert.Equal("", spec.DatabaseName) - assert.Equal("test", spec.DatasetName) - assert.True(spec.Path.IsEmpty()) + assert.Equal("test", spec.Path.Dataset) + assert.True(spec.Path.Path.IsEmpty()) ds := spec.GetDataset() _, ok := spec.GetDataset().MaybeHeadValue() @@ -68,7 +67,6 @@ func TestMemHashPathSpec(t *testing.T) { assert.Equal("mem", spec.Protocol) assert.Equal("", spec.DatabaseName) - assert.Equal("", spec.DatasetName) assert.False(spec.Path.IsEmpty()) // This would be a reasonable check, and the equivalent JS test does it, but @@ -89,7 +87,6 @@ func TestMemDatasetPathSpec(t *testing.T) { assert.Equal("mem", spec.Protocol) assert.Equal("", spec.DatabaseName) - assert.Equal("", spec.DatasetName) assert.False(spec.Path.IsEmpty()) assert.Nil(spec.GetValue()) @@ -222,25 +219,25 @@ func TestForDatabase(t *testing.T) { defer os.RemoveAll(tmpDir) testCases := []struct { - spec, protocol, databaseName string + spec, protocol, databaseName, canonicalSpecIfAny string }{ - {"http://localhost:8000", "http", "//localhost:8000"}, - {"http://localhost:8000/fff", "http", "//localhost:8000/fff"}, - {"https://local.attic.io/john/doe", "https", "//local.attic.io/john/doe"}, - {"mem", "mem", ""}, - {tmpDir, "nbs", tmpDir}, - {"nbs:" + tmpDir, "nbs", tmpDir}, - {"http://server.com/john/doe?access_token=jane", "http", "//server.com/john/doe?access_token=jane"}, - {"https://server.com/john/doe/?arg=2&qp1=true&access_token=jane", "https", "//server.com/john/doe/?arg=2&qp1=true&access_token=jane"}, - {"http://some/::/one", "http", "//some/::/one"}, - {"http://::1", "http", "//::1"}, - {"http://192.30.252.154", "http", "//192.30.252.154"}, - {"http://::192.30.252.154", "http", "//::192.30.252.154"}, - {"http://0:0:0:0:0:ffff:c01e:fc9a", "http", "//0:0:0:0:0:ffff:c01e:fc9a"}, - {"http://::ffff:c01e:fc9a", "http", "//::ffff:c01e:fc9a"}, - {"http://::ffff::1e::9a", "http", "//::ffff::1e::9a"}, - {"aws://table:bucket/db", "aws", "//table:bucket/db"}, - {"aws://table/db", "aws", "//table/db"}, + {"http://localhost:8000", "http", "//localhost:8000", ""}, + {"http://localhost:8000/fff", "http", "//localhost:8000/fff", ""}, + {"https://local.attic.io/john/doe", "https", "//local.attic.io/john/doe", ""}, + {"mem", "mem", "", ""}, + {tmpDir, "nbs", tmpDir, "nbs:" + tmpDir}, + {"nbs:" + tmpDir, "nbs", tmpDir, ""}, + {"http://server.com/john/doe?access_token=jane", "http", "//server.com/john/doe?access_token=jane", ""}, + {"https://server.com/john/doe/?arg=2&qp1=true&access_token=jane", "https", "//server.com/john/doe/?arg=2&qp1=true&access_token=jane", ""}, + {"http://some/::/one", "http", "//some/::/one", ""}, + {"http://::1", "http", "//::1", ""}, + {"http://192.30.252.154", "http", "//192.30.252.154", ""}, + {"http://::192.30.252.154", "http", "//::192.30.252.154", ""}, + {"http://0:0:0:0:0:ffff:c01e:fc9a", "http", "//0:0:0:0:0:ffff:c01e:fc9a", ""}, + {"http://::ffff:c01e:fc9a", "http", "//::ffff:c01e:fc9a", ""}, + {"http://::ffff::1e::9a", "http", "//::ffff::1e::9a", ""}, + {"aws://table:bucket/db", "aws", "//table:bucket/db", ""}, + {"aws://table/db", "aws", "//table/db", ""}, } for _, tc := range testCases { @@ -248,11 +245,15 @@ func TestForDatabase(t *testing.T) { assert.NoError(err, tc.spec) defer spec.Close() - assert.Equal(tc.spec, spec.Spec) assert.Equal(tc.protocol, spec.Protocol) assert.Equal(tc.databaseName, spec.DatabaseName) - assert.Equal("", spec.DatasetName) assert.True(spec.Path.IsEmpty()) + + if tc.canonicalSpecIfAny == "" { + assert.Equal(tc.spec, spec.String()) + } else { + assert.Equal(tc.canonicalSpecIfAny, spec.String()) + } } } @@ -274,6 +275,7 @@ func TestForDataset(t *testing.T) { "nbs:", "nbs:hello", "aws://t:b/db", + "mem::foo.value", } for _, spec := range badSpecs { @@ -298,24 +300,24 @@ func TestForDataset(t *testing.T) { defer os.RemoveAll(tmpDir) testCases := []struct { - spec, protocol, databaseName, datasetName string + spec, protocol, databaseName, datasetName, canonicalSpecIfAny string }{ - {"http://localhost:8000::ds1", "http", "//localhost:8000", "ds1"}, - {"http://localhost:8000/john/doe/::ds2", "http", "//localhost:8000/john/doe/", "ds2"}, - {"https://local.attic.io/john/doe::ds3", "https", "//local.attic.io/john/doe", "ds3"}, - {"http://local.attic.io/john/doe::ds1", "http", "//local.attic.io/john/doe", "ds1"}, - {"nbs:" + tmpDir + "::ds/one", "nbs", tmpDir, "ds/one"}, - {tmpDir + "::ds/one", "nbs", tmpDir, "ds/one"}, - {"http://localhost:8000/john/doe?access_token=abc::ds/one", "http", "//localhost:8000/john/doe?access_token=abc", "ds/one"}, - {"https://localhost:8000?qp1=x&access_token=abc&qp2=y::ds/one", "https", "//localhost:8000?qp1=x&access_token=abc&qp2=y", "ds/one"}, - {"http://192.30.252.154::foo", "http", "//192.30.252.154", "foo"}, - {"http://::1::foo", "http", "//::1", "foo"}, - {"http://::192.30.252.154::foo", "http", "//::192.30.252.154", "foo"}, - {"http://0:0:0:0:0:ffff:c01e:fc9a::foo", "http", "//0:0:0:0:0:ffff:c01e:fc9a", "foo"}, - {"http://::ffff:c01e:fc9a::foo", "http", "//::ffff:c01e:fc9a", "foo"}, - {"http://::ffff::1e::9a::foo", "http", "//::ffff::1e::9a", "foo"}, - {"aws://table:bucket/db::ds", "aws", "//table:bucket/db", "ds"}, - {"aws://table/db::ds", "aws", "//table/db", "ds"}, + {"http://localhost:8000::ds1", "http", "//localhost:8000", "ds1", ""}, + {"http://localhost:8000/john/doe/::ds2", "http", "//localhost:8000/john/doe/", "ds2", ""}, + {"https://local.attic.io/john/doe::ds3", "https", "//local.attic.io/john/doe", "ds3", ""}, + {"http://local.attic.io/john/doe::ds1", "http", "//local.attic.io/john/doe", "ds1", ""}, + {"nbs:" + tmpDir + "::ds/one", "nbs", tmpDir, "ds/one", ""}, + {tmpDir + "::ds/one", "nbs", tmpDir, "ds/one", "nbs:" + tmpDir + "::ds/one"}, + {"http://localhost:8000/john/doe?access_token=abc::ds/one", "http", "//localhost:8000/john/doe?access_token=abc", "ds/one", ""}, + {"https://localhost:8000?qp1=x&access_token=abc&qp2=y::ds/one", "https", "//localhost:8000?qp1=x&access_token=abc&qp2=y", "ds/one", ""}, + {"http://192.30.252.154::foo", "http", "//192.30.252.154", "foo", ""}, + {"http://::1::foo", "http", "//::1", "foo", ""}, + {"http://::192.30.252.154::foo", "http", "//::192.30.252.154", "foo", ""}, + {"http://0:0:0:0:0:ffff:c01e:fc9a::foo", "http", "//0:0:0:0:0:ffff:c01e:fc9a", "foo", ""}, + {"http://::ffff:c01e:fc9a::foo", "http", "//::ffff:c01e:fc9a", "foo", ""}, + {"http://::ffff::1e::9a::foo", "http", "//::ffff::1e::9a", "foo", ""}, + {"aws://table:bucket/db::ds", "aws", "//table:bucket/db", "ds", ""}, + {"aws://table/db::ds", "aws", "//table/db", "ds", ""}, } for _, tc := range testCases { @@ -323,11 +325,15 @@ func TestForDataset(t *testing.T) { assert.NoError(err, tc.spec) defer spec.Close() - assert.Equal(tc.spec, spec.Spec) assert.Equal(tc.protocol, spec.Protocol) assert.Equal(tc.databaseName, spec.DatabaseName) - assert.Equal(tc.datasetName, spec.DatasetName) - assert.True(spec.Path.IsEmpty()) + assert.Equal(tc.datasetName, spec.Path.Dataset) + + if tc.canonicalSpecIfAny == "" { + assert.Equal(tc.spec, spec.String()) + } else { + assert.Equal(tc.canonicalSpecIfAny, spec.String()) + } } } @@ -351,22 +357,22 @@ func TestForPath(t *testing.T) { defer os.RemoveAll(tmpDir) testCases := []struct { - spec, protocol, databaseName, pathString string + spec, protocol, databaseName, pathString, canonicalSpecIfAny string }{ - {"http://local.attic.io/john/doe::#0123456789abcdefghijklmnopqrstuv", "http", "//local.attic.io/john/doe", "#0123456789abcdefghijklmnopqrstuv"}, - {tmpDir + "::#0123456789abcdefghijklmnopqrstuv", "nbs", tmpDir, "#0123456789abcdefghijklmnopqrstuv"}, - {"nbs:" + tmpDir + "::#0123456789abcdefghijklmnopqrstuv", "nbs", tmpDir, "#0123456789abcdefghijklmnopqrstuv"}, - {"mem::#0123456789abcdefghijklmnopqrstuv", "mem", "", "#0123456789abcdefghijklmnopqrstuv"}, - {"http://local.attic.io/john/doe::#0123456789abcdefghijklmnopqrstuv", "http", "//local.attic.io/john/doe", "#0123456789abcdefghijklmnopqrstuv"}, - {"http://localhost:8000/john/doe/::ds1", "http", "//localhost:8000/john/doe/", "ds1"}, - {"http://192.30.252.154::foo.bar", "http", "//192.30.252.154", "foo.bar"}, - {"http://::1::foo.bar.baz", "http", "//::1", "foo.bar.baz"}, - {"http://::192.30.252.154::baz[42]", "http", "//::192.30.252.154", "baz[42]"}, - {"http://0:0:0:0:0:ffff:c01e:fc9a::foo[42].bar", "http", "//0:0:0:0:0:ffff:c01e:fc9a", "foo[42].bar"}, - {"http://::ffff:c01e:fc9a::foo.foo", "http", "//::ffff:c01e:fc9a", "foo.foo"}, - {"http://::ffff::1e::9a::hello[\"world\"]", "http", "//::ffff::1e::9a", "hello[\"world\"]"}, - {"aws://table:bucket/db::foo.foo", "aws", "//table:bucket/db", "foo.foo"}, - {"aws://table/db::foo.foo", "aws", "//table/db", "foo.foo"}, + {"http://local.attic.io/john/doe::#0123456789abcdefghijklmnopqrstuv", "http", "//local.attic.io/john/doe", "#0123456789abcdefghijklmnopqrstuv", ""}, + {tmpDir + "::#0123456789abcdefghijklmnopqrstuv", "nbs", tmpDir, "#0123456789abcdefghijklmnopqrstuv", "nbs:" + tmpDir + "::#0123456789abcdefghijklmnopqrstuv"}, + {"nbs:" + tmpDir + "::#0123456789abcdefghijklmnopqrstuv", "nbs", tmpDir, "#0123456789abcdefghijklmnopqrstuv", ""}, + {"mem::#0123456789abcdefghijklmnopqrstuv", "mem", "", "#0123456789abcdefghijklmnopqrstuv", ""}, + {"http://local.attic.io/john/doe::#0123456789abcdefghijklmnopqrstuv", "http", "//local.attic.io/john/doe", "#0123456789abcdefghijklmnopqrstuv", ""}, + {"http://localhost:8000/john/doe/::ds1", "http", "//localhost:8000/john/doe/", "ds1", ""}, + {"http://192.30.252.154::foo.bar", "http", "//192.30.252.154", "foo.bar", ""}, + {"http://::1::foo.bar.baz", "http", "//::1", "foo.bar.baz", ""}, + {"http://::192.30.252.154::baz[42]", "http", "//::192.30.252.154", "baz[42]", ""}, + {"http://0:0:0:0:0:ffff:c01e:fc9a::foo[42].bar", "http", "//0:0:0:0:0:ffff:c01e:fc9a", "foo[42].bar", ""}, + {"http://::ffff:c01e:fc9a::foo.foo", "http", "//::ffff:c01e:fc9a", "foo.foo", ""}, + {"http://::ffff::1e::9a::hello[\"world\"]", "http", "//::ffff::1e::9a", "hello[\"world\"]", ""}, + {"aws://table:bucket/db::foo.foo", "aws", "//table:bucket/db", "foo.foo", ""}, + {"aws://table/db::foo.foo", "aws", "//table/db", "foo.foo", ""}, } for _, tc := range testCases { @@ -374,11 +380,15 @@ func TestForPath(t *testing.T) { assert.NoError(err) defer spec.Close() - assert.Equal(tc.spec, spec.Spec) assert.Equal(tc.protocol, spec.Protocol) assert.Equal(tc.databaseName, spec.DatabaseName) - assert.Equal("", spec.DatasetName) assert.Equal(tc.pathString, spec.Path.String()) + + if tc.canonicalSpecIfAny == "" { + assert.Equal(tc.spec, spec.String()) + } else { + assert.Equal(tc.canonicalSpecIfAny, spec.String()) + } } } @@ -399,7 +409,7 @@ func TestPinPathSpec(t *testing.T) { head := db.GetDataset("foo").Head() assert.Equal(head.Hash(), pinned.Path.Hash) - assert.Equal(fmt.Sprintf("mem::#%s.value", head.Hash().String()), pinned.Spec) + assert.Equal(fmt.Sprintf("mem::#%s.value", head.Hash().String()), pinned.String()) assert.Equal(types.Number(42), pinned.GetValue()) assert.Equal(types.Number(42), unpinned.GetValue()) @@ -429,7 +439,7 @@ func TestPinDatasetSpec(t *testing.T) { } assert.Equal(head.Hash(), pinned.Path.Hash) - assert.Equal(fmt.Sprintf("mem::#%s", head.Hash().String()), pinned.Spec) + assert.Equal(fmt.Sprintf("mem::#%s", head.Hash().String()), pinned.String()) assert.Equal(types.Number(42), commitValue(pinned.GetValue())) assert.Equal(types.Number(42), unpinned.GetDataset().HeadValue()) diff --git a/go/spec/util.go b/go/spec/util.go index 323b82d8a2..16ab079680 100644 --- a/go/spec/util.go +++ b/go/spec/util.go @@ -5,19 +5,20 @@ package spec import ( - "fmt" - + "github.com/attic-labs/noms/go/d" "github.com/attic-labs/noms/go/hash" ) -func CreateDatabaseSpecString(protocol, path string) string { - return fmt.Sprintf("%s:%s", protocol, path) +func CreateDatabaseSpecString(protocol, db string) string { + return Spec{Protocol: protocol, DatabaseName: db}.String() } -func CreateValueSpecString(protocol, path, value string) string { - return fmt.Sprintf("%s:%s%s%s", protocol, path, Separator, value) +func CreateValueSpecString(protocol, db, path string) string { + p, err := NewAbsolutePath(path) + d.Chk.NoError(err) + return Spec{Protocol: protocol, DatabaseName: db, Path: p}.String() } -func CreateHashSpecString(protocol, path string, h hash.Hash) string { - return fmt.Sprintf("%s:%s%s#%s", protocol, path, Separator, h.String()) +func CreateHashSpecString(protocol, db string, h hash.Hash) string { + return Spec{Protocol: protocol, DatabaseName: db, Path: AbsolutePath{Hash: h}}.String() } diff --git a/go/types/path.go b/go/types/path.go index d9ef7b0329..6bd4f1f84f 100644 --- a/go/types/path.go +++ b/go/types/path.go @@ -22,9 +22,12 @@ import ( // Note, @at() is valid under this regexp, code should deal with the error. var annotationRe = regexp.MustCompile(`^([a-z]+)(\(([\w\-"']*)\))?`) -// A Path is an address to a Noms value - and unlike hashes (i.e. #abcd...) they -// can address inlined values. -// See https://github.com/attic-labs/noms/blob/master/doc/spelling.md. +// A Path locates a value in Noms relative to some other value. For locating +// values absolutely within a database, see AbsolutePath. To locate values +// globally, see Spec. +// +// For more details, see: +// https://github.com/attic-labs/noms/blob/master/doc/spelling.md. type Path []PathPart type PathPart interface { @@ -176,6 +179,10 @@ func (p Path) String() string { return strings.Join(strs, "") } +func (p Path) IsEmpty() bool { + return len(p) == 0 +} + // Gets Struct field values by name. type FieldPath struct { // The name of the field, e.g. `.Name`. diff --git a/samples/go/poke/main_test.go b/samples/go/poke/main_test.go index 9ad7884156..9a37ae5756 100644 --- a/samples/go/poke/main_test.go +++ b/samples/go/poke/main_test.go @@ -103,13 +103,13 @@ func (s *testSuite) TestLose() { {[]string{"foo"}, "Incorrect number of arguments\n"}, {[]string{"foo", "bar"}, "Incorrect number of arguments\n"}, {[]string{"foo", "bar", "baz", "quux"}, "Incorrect number of arguments\n"}, - {[]string{sp.Spec + "!!", ".foo", `"bar"`}, "Invalid input dataset '" + sp.Spec + "!!': Dataset test!! must match ^[a-zA-Z0-9\\-_/]+$\n"}, - {[]string{sp.Spec + "2", ".foo", `"bar"`}, "Input dataset '" + sp.Spec + "2' does not exist\n"}, - {[]string{sp.Spec, "[invalid", `"bar"`}, "Invalid path '[invalid': Invalid index: invalid\n"}, - {[]string{sp.Spec, ".nothinghere", `"bar"`}, "No value at path '.nothinghere' - cannot update\n"}, - {[]string{sp.Spec, ".foo", "bar"}, "Invalid new value: 'bar': Invalid index: bar\n"}, - {[]string{"--out-ds-name", "!invalid", sp.Spec, ".foo", `"bar"`}, "Invalid output dataset name: !invalid\n"}, - {[]string{sp.Spec, `.bar[#00000000000000000000000000000000]`, "42"}, "Invalid path '.bar[#00000000000000000000000000000000]': Invalid hash: 00000000000000000000000000000000\n"}, + {[]string{sp.String() + "!!", ".foo", `"bar"`}, "Invalid input dataset '" + sp.String() + "!!': Invalid operator: !\n"}, + {[]string{sp.String() + "2", ".foo", `"bar"`}, "Input dataset '" + sp.String() + "2' does not exist\n"}, + {[]string{sp.String(), "[invalid", `"bar"`}, "Invalid path '[invalid': Invalid index: invalid\n"}, + {[]string{sp.String(), ".nothinghere", `"bar"`}, "No value at path '.nothinghere' - cannot update\n"}, + {[]string{sp.String(), ".foo", "bar"}, "Invalid new value: 'bar': Invalid index: bar\n"}, + {[]string{"--out-ds-name", "!invalid", sp.String(), ".foo", `"bar"`}, "Invalid output dataset name: !invalid\n"}, + {[]string{sp.String(), `.bar[#00000000000000000000000000000000]`, "42"}, "Invalid path '.bar[#00000000000000000000000000000000]': Invalid hash: 00000000000000000000000000000000\n"}, } sp.GetDatabase().CommitValue(sp.GetDataset(), types.NewStruct("", map[string]types.Value{