From 0d49cffb44e9035a2b15d4d830a0cee0b96d7fb2 Mon Sep 17 00:00:00 2001 From: Attila Tajti Date: Wed, 6 Jan 2016 17:44:08 +0100 Subject: [PATCH] pkg/client: set camliType, use correct blobref - make sure camliType is set to "file" in UploadFile - fileMapFromDuplicate uses the blob ref calculated from the file schema blob in the call to ReceiveBlob - add test for above changes - TestTransportSetup updated after field name change - hasComponent fixed for case fullpath[:len(component)] == component - hasComponent returns true if the first match is missing either separator, but there is another match down the path - add special handling for Windows in hasCompontent and TestIgnoreFns Change-Id: I96325b060e9421709bd9f684bcc9eceed7135f7b --- pkg/client/config.go | 36 ++++---- pkg/client/ignored_test.go | 113 ++++++++++++++++++++++++- pkg/client/transport_test.go | 8 +- pkg/client/upload.go | 6 +- pkg/client/upload_test.go | 155 +++++++++++++++++++++++++++++++++++ 5 files changed, 295 insertions(+), 23 deletions(-) create mode 100644 pkg/client/upload_test.go diff --git a/pkg/client/config.go b/pkg/client/config.go index 0bf84e581..a6d2c9e48 100644 --- a/pkg/client/config.go +++ b/pkg/client/config.go @@ -561,21 +561,27 @@ func hasDirPrefix(dirPrefix, fullpath string) bool { return false } -// hasComponent returns whether the pathComponent is a path component of fullpath. i.e it is a part of fullpath that fits exactly between two path separators. +// hasComponent returns whether the pathComponent is a path component of +// fullpath. i.e it is a part of fullpath that fits exactly between two path +// separators. func hasComponent(component, fullpath string) bool { - idx := strings.Index(fullpath, component) - if idx == -1 { - return false + // trim Windows volume name + fullpath = strings.TrimPrefix(fullpath, filepath.VolumeName(fullpath)) + for { + i := strings.Index(fullpath, component) + if i == -1 { + return false + } + if i != 0 && fullpath[i-1] == filepath.Separator { + componentEnd := i + len(component) + if componentEnd == len(fullpath) { + return true + } + if fullpath[componentEnd] == filepath.Separator { + return true + } + } + fullpath = fullpath[i+1:] } - if fullpath[idx-1] != filepath.Separator { - return false - } - componentEnd := idx + len(component) - if componentEnd == len(fullpath) { - return true - } - if fullpath[componentEnd] == filepath.Separator { - return true - } - return false + panic("unreachable") } diff --git a/pkg/client/ignored_test.go b/pkg/client/ignored_test.go index 8b412e38e..2add5bbf8 100644 --- a/pkg/client/ignored_test.go +++ b/pkg/client/ignored_test.go @@ -17,7 +17,9 @@ limitations under the License. package client import ( + "path" "path/filepath" + "runtime" "testing" ) @@ -42,8 +44,14 @@ func TestIgnoreMultiPattern(t *testing.T) { func TestIsIgnoredFile(t *testing.T) { old := osutilHomeDir defer func() { osutilHomeDir = old }() - osutilHomeDir = func() string { - return "/Fake/Home/Camli" + if runtime.GOOS == "windows" { + osutilHomeDir = func() string { + return `C:\Fake\Users\Camli` + } + } else { + osutilHomeDir = func() string { + return "/Fake/Home/Camli" + } } home := osutilHomeDir() @@ -86,6 +94,7 @@ type patternTest struct { } func TestIgnoreFns(t *testing.T) { + // POSIX tests tests := []patternTest{ { name: "isShellPatternMatch", @@ -143,6 +152,106 @@ func TestIgnoreFns(t *testing.T) { fullpath: "/home/pony/rainbow.jpg", want: false, }, + { + name: "hasComponent", + fn: hasComponent, + pattern: "/home/pony", + fullpath: "/home/pony/rainbow.jpg", + want: false, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: "pony", + fullpath: "/home/ponytail/pony/rainbow.jpg", + want: true, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: "pony", + fullpath: "/home/pony/ponytail/rainbow.jpg", + want: true, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: "rainbow.jpg", + fullpath: "/home/ponytail/pony/rainbow.jpg", + want: true, + }, + } + if runtime.GOOS == "windows" { + // A path starting with a single slash is relative + // path on Windows. Prepend a drive letter to it to + // make it absolute. + // Also clean paths so that the test work on Windows. + const driveSpec = "C:" + for i := range tests { + v := &tests[i] + // Check path.IsAbs, not filepath.IsAbs to see + // if v.pattern should be absolute. + if path.IsAbs(v.pattern) { + v.pattern = driveSpec + filepath.Clean(v.pattern) + } else { + v.pattern = filepath.Clean(v.pattern) + } + v.fullpath = driveSpec + filepath.Clean(v.fullpath) + } + + // On Windows, a volume name such as a drive letter or UNC volume: + // `C:` + // `\\server\sharename` + // is considered a single path component. Therefore neither of: + // `server` + // `sharename` + // `server\sharename` + // should be accepted for fullpath == `\\server\sharename\...` + windowsTests := []patternTest{ + { + name: "hasComponent", + fn: hasComponent, + pattern: `pony`, + fullpath: `C:\pony\rainbow.jpg`, + want: true, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: `pony`, + fullpath: `\\server\sharename\pony\rainbow.jpg`, + want: true, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: `C:`, + fullpath: `C:\windows\system32`, + want: false, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: `server`, + fullpath: `\\server\sharename\rainbow.jpg`, + want: false, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: `sharename`, + fullpath: `\\server\sharename\rainbow.jpg`, + want: false, + }, + { + name: "hasComponent", + fn: hasComponent, + pattern: `server\sharename`, + fullpath: `\\server\sharename\rainbow.jpg`, + want: false, + }, + } + tests = append(tests, windowsTests...) } for _, v := range tests { if v.fn(v.pattern, v.fullpath) != v.want { diff --git a/pkg/client/transport_test.go b/pkg/client/transport_test.go index b41ceb588..5d34ad106 100644 --- a/pkg/client/transport_test.go +++ b/pkg/client/transport_test.go @@ -150,10 +150,10 @@ func TestTransportSetup(t *testing.T) { } for tti, tt := range transportTests { cl := &Client{ - paramsOnly: true, - server: tt.server, - trustedCerts: tt.trustedCerts, - InsecureTLS: tt.insecureTLS, + paramsOnly: true, + server: tt.server, + trustedCerts: tt.trustedCerts, + insecureAnyTLSCert: tt.insecureTLS, } android.OnAndroidHook = func() bool { return tt.onAndroid diff --git a/pkg/client/upload.go b/pkg/client/upload.go index cbe317658..106613d1d 100644 --- a/pkg/client/upload.go +++ b/pkg/client/upload.go @@ -547,6 +547,7 @@ func (cl *Client) UploadFile(filename string, contents io.Reader, opts *FileUplo fileMap.SetModTime(modTime) } } + fileMap.SetType("file") var wholeRef blob.Ref if opts != nil && opts.WholeRef.Valid() { @@ -616,11 +617,12 @@ func (cl *Client) fileMapFromDuplicate(fileMap *schema.Builder, wholeRef blob.Re if err != nil { return blob.Ref{}, fmt.Errorf("could not write file map for wholeRef %q: %v", wholeRef, err) } - if blob.SHA1FromString(json) == dupFileRef { + bref := blob.SHA1FromString(json) + if bref == dupFileRef { // Unchanged (same filename, modtime, JSON serialization, etc) return dupFileRef, nil } - sbr, err := cl.ReceiveBlob(dupFileRef, strings.NewReader(json)) + sbr, err := cl.ReceiveBlob(bref, strings.NewReader(json)) if err != nil { return blob.Ref{}, err } diff --git a/pkg/client/upload_test.go b/pkg/client/upload_test.go new file mode 100644 index 000000000..a71a4cacc --- /dev/null +++ b/pkg/client/upload_test.go @@ -0,0 +1,155 @@ +/* +Copyright 2016 The Camlistore Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "camlistore.org/pkg/osutil" + "camlistore.org/pkg/schema" + "camlistore.org/pkg/serverinit" + "camlistore.org/pkg/types/serverconfig" + + // For registering all the handler constructors needed in newTestServer + _ "camlistore.org/pkg/blobserver/cond" + _ "camlistore.org/pkg/blobserver/replica" + _ "camlistore.org/pkg/importer/allimporters" + _ "camlistore.org/pkg/search" + _ "camlistore.org/pkg/server" +) + +type fakeFile struct { + name string + size int64 + modTime time.Time + + content string +} + +func newFakeFile(name, content string, modTime time.Time) *fakeFile { + return &fakeFile{name, int64(len(content)), modTime, content} +} + +func (f *fakeFile) Name() string { return f.name } +func (f *fakeFile) Size() int64 { return f.size } +func (f *fakeFile) ModTime() time.Time { return f.modTime } +func (f *fakeFile) Mode() os.FileMode { return 0666 } +func (f *fakeFile) IsDir() bool { return false } +func (f *fakeFile) Sys() interface{} { return nil } + +// TestUploadFile checks if uploading a file with the same content +// but different metadata works, and whether camliType is set to "file". +func TestUploadFile(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + + ts := newTestServer(t) + defer ts.Close() + + c := New(ts.URL) + + f := newFakeFile("foo.txt", "bar", time.Date(2011, 1, 28, 2, 3, 4, 0, time.Local)) + + testUploadFile(t, c, f, false) + testUploadFile(t, c, f, true) + + f.modTime.Add(time.Hour) + + testUploadFile(t, c, f, true) + + f.name = "baz.txt" + + testUploadFile(t, c, f, true) +} + +// testUploadFile uploads a file and checks if it can be retrieved. +func testUploadFile(t *testing.T, c *Client, f *fakeFile, withFileOpts bool) *schema.Blob { + var opts *FileUploadOptions + if withFileOpts { + opts = &FileUploadOptions{FileInfo: f} + } + bref, err := c.UploadFile(f.Name(), strings.NewReader(f.content), opts) + if err != nil { + t.Fatal(err) + } + sb, err := c.FetchSchemaBlob(bref) + if err != nil { + t.Fatal(err) + } + if sb.Type() != "file" { + t.Fatal(`schema blob from UploadFile must have "file" type`) + } + return sb +} + +// newTestServer creates a new test server with in memory storage for use in upload tests +func newTestServer(t *testing.T) *httptest.Server { + camroot, err := osutil.GoPackagePath("camlistore.org") + if err != nil { + t.Fatalf("failed to find camlistore.org GOPATH root: %v", err) + } + + conf := serverconfig.Config{ + Listen: ":3179", + HTTPS: false, + Auth: "localhost", + Identity: "26F5ABDA", + IdentitySecretRing: filepath.Join(camroot, filepath.FromSlash("pkg/jsonsign/testdata/test-secring.gpg")), + MemoryStorage: true, + MemoryIndex: true, + } + + confData, err := json.MarshalIndent(conf, "", " ") + if err != nil { + t.Fatalf("Could not json encode config: %v", err) + } + + // Setting CAMLI_CONFIG_DIR to avoid triggering failInTests in osutil.CamliConfigDir + defer os.Setenv("CAMLI_CONFIG_DIR", os.Getenv("CAMLI_CONFIG_DIR")) // restore after test + os.Setenv("CAMLI_CONFIG_DIR", "whatever") + lowConf, err := serverinit.Load(confData) + if err != nil { + t.Fatal(err) + } + // because these two are normally consumed in camlistored.go + // TODO(mpl): serverinit.Load should consume these 2 as well. Once + // consumed, we should keep all the answers as private fields, and then we + // put accessors on serverinit.Config. Maybe we even stop embedding + // jsonconfig.Obj in serverinit.Config too, so none of those methods are + // accessible. + lowConf.OptionalBool("https", true) + lowConf.OptionalString("listen", "") + + reindex := false + var context *http.Request // only used by App Engine. See handlerLoader in serverinit.go + hi := http.NewServeMux() + address := "http://" + conf.Listen + _, err = lowConf.InstallHandlers(hi, address, reindex, context) + if err != nil { + t.Fatal(err) + } + + return httptest.NewServer(hi) +}