From a7d9268e4de36ebb925d578a713631f1773885cf Mon Sep 17 00:00:00 2001 From: Audrius Butkevicius Date: Mon, 1 Feb 2021 08:27:34 +0000 Subject: [PATCH] lib/model: Make /browse endpoint return sane objects (#7306) --- lib/api/api.go | 9 +- lib/api/mocked_model_test.go | 4 +- lib/model/model.go | 69 ++++-- lib/model/model_test.go | 416 ++++++++++++--------------------- lib/protocol/bep_extensions.go | 18 ++ 5 files changed, 219 insertions(+), 297 deletions(-) diff --git a/lib/api/api.go b/lib/api/api.go index 83e5eb1b6..5cbb4427c 100644 --- a/lib/api/api.go +++ b/lib/api/api.go @@ -710,14 +710,19 @@ func (s *service) getDBBrowse(w http.ResponseWriter, r *http.Request) { qs := r.URL.Query() folder := qs.Get("folder") prefix := qs.Get("prefix") - dirsonly := qs.Get("dirsonly") != "" + dirsOnly := qs.Get("dirsonly") != "" levels, err := strconv.Atoi(qs.Get("levels")) if err != nil { levels = -1 } + result, err := s.model.GlobalDirectoryTree(folder, prefix, levels, dirsOnly) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } - sendJSON(w, s.model.GlobalDirectoryTree(folder, prefix, levels, dirsonly)) + sendJSON(w, result) } func (s *service) getDBCompletion(w http.ResponseWriter, r *http.Request) { diff --git a/lib/api/mocked_model_test.go b/lib/api/mocked_model_test.go index 58b605f3d..f38f16f76 100644 --- a/lib/api/mocked_model_test.go +++ b/lib/api/mocked_model_test.go @@ -21,8 +21,8 @@ import ( type mockedModel struct{} -func (m *mockedModel) GlobalDirectoryTree(folder, prefix string, levels int, dirsonly bool) map[string]interface{} { - return nil +func (m *mockedModel) GlobalDirectoryTree(folder, prefix string, levels int, dirsOnly bool) ([]*model.TreeEntry, error) { + return nil, nil } func (m *mockedModel) Completion(device protocol.DeviceID, folder string) model.FolderCompletion { diff --git a/lib/model/model.go b/lib/model/model.go index 11d06742b..c7b05ffb8 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -110,7 +110,7 @@ type Model interface { PendingFolders(device protocol.DeviceID) (map[string]db.PendingFolder, error) StartDeadlockDetector(timeout time.Duration) - GlobalDirectoryTree(folder, prefix string, levels int, dirsonly bool) map[string]interface{} + GlobalDirectoryTree(folder, prefix string, levels int, dirsOnly bool) ([]*TreeEntry, error) } type model struct { @@ -2510,15 +2510,34 @@ func (m *model) Revert(folder string) { runner.Revert() } -func (m *model) GlobalDirectoryTree(folder, prefix string, levels int, dirsonly bool) map[string]interface{} { +type TreeEntry struct { + Name string `json:"name"` + ModTime time.Time `json:"modTime"` + Size int64 `json:"size"` + Type protocol.FileInfoType `json:"type"` + Children []*TreeEntry `json:"children,omitempty"` +} + +func findByName(slice []*TreeEntry, name string) *TreeEntry { + for _, child := range slice { + if child.Name == name { + return child + } + } + return nil +} + +func (m *model) GlobalDirectoryTree(folder, prefix string, levels int, dirsOnly bool) ([]*TreeEntry, error) { m.fmut.RLock() files, ok := m.folderFiles[folder] m.fmut.RUnlock() if !ok { - return nil + return nil, errFolderMissing } - output := make(map[string]interface{}) + root := &TreeEntry{ + Children: make([]*TreeEntry, 0), + } sep := string(filepath.Separator) prefix = osutil.NativeFilename(prefix) @@ -2528,6 +2547,7 @@ func (m *model) GlobalDirectoryTree(folder, prefix string, levels int, dirsonly snap := files.Snapshot() defer snap.Release() + var err error snap.WithPrefixedGlobalTruncated(prefix, func(fi protocol.FileIntf) bool { f := fi.(db.FileInfoTruncated) @@ -2538,42 +2558,43 @@ func (m *model) GlobalDirectoryTree(folder, prefix string, levels int, dirsonly f.Name = strings.Replace(f.Name, prefix, "", 1) - var dir, base string - if f.IsDirectory() && !f.IsSymlink() { - dir = f.Name - } else { - dir = filepath.Dir(f.Name) - base = filepath.Base(f.Name) - } + dir := filepath.Dir(f.Name) + base := filepath.Base(f.Name) if levels > -1 && strings.Count(f.Name, sep) > levels { return true } - last := output + parent := root if dir != "." { for _, path := range strings.Split(dir, sep) { - directory, ok := last[path] - if !ok { - newdir := make(map[string]interface{}) - last[path] = newdir - last = newdir - } else { - last = directory.(map[string]interface{}) + child := findByName(parent.Children, path) + if child == nil { + err = fmt.Errorf("could not find child '%s' for path '%s' in parent '%s'", path, f.Name, parent.Name) + return false } + parent = child } } - if !dirsonly && base != "" { - last[base] = []interface{}{ - f.ModTime(), f.FileSize(), - } + if dirsOnly && !f.IsDirectory() { + return true } + parent.Children = append(parent.Children, &TreeEntry{ + Name: base, + Type: f.Type, + ModTime: f.ModTime(), + Size: f.FileSize(), + }) + return true }) + if err != nil { + return nil, err + } - return output + return root.Children, nil } func (m *model) GetFolderVersions(folder string) (map[string][]versioner.FileVersion, error) { diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 26969449a..f88320769 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -1713,8 +1713,23 @@ func TestGlobalDirectoryTree(t *testing.T) { Size: 0xa, } } - - filedata := []interface{}{time.Unix(0x666, 0), 0xa} + f := func(name string) *TreeEntry { + return &TreeEntry{ + Name: name, + ModTime: time.Unix(0x666, 0), + Size: 0xa, + Type: protocol.FileInfoTypeFile, + } + } + d := func(name string, entries ...*TreeEntry) *TreeEntry { + return &TreeEntry{ + Name: name, + ModTime: time.Unix(0x666, 0), + Size: 128, + Type: protocol.FileInfoTypeDirectory, + Children: entries, + } + } testdata := []protocol.FileInfo{ b(false, "another"), @@ -1739,43 +1754,43 @@ func TestGlobalDirectoryTree(t *testing.T) { b(false, "some", "directory", "with", "a"), b(true, "some", "directory", "with", "a", "file"), - b(true, "rootfile"), + b(true, "zzrootfile"), } - expectedResult := map[string]interface{}{ - "another": map[string]interface{}{ - "directory": map[string]interface{}{ - "afile": filedata, - "with": map[string]interface{}{ - "a": map[string]interface{}{ - "file": filedata, - }, - "file": filedata, - }, - }, - "file": filedata, - }, - "other": map[string]interface{}{ - "rand": map[string]interface{}{}, - "random": map[string]interface{}{ - "dir": map[string]interface{}{}, - "dirx": map[string]interface{}{}, - }, - "randomx": map[string]interface{}{}, - }, - "some": map[string]interface{}{ - "directory": map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{ - "file": filedata, - }, - }, - }, - }, - "rootfile": filedata, + expectedResult := []*TreeEntry{ + d("another", + d("directory", + f("afile"), + d("with", + d("a", + f("file"), + ), + f("file"), + ), + ), + f("file"), + ), + d("other", + d("rand"), + d("random", + d("dir"), + d("dirx"), + ), + d("randomx"), + ), + d("some", + d("directory", + d("with", + d("a", + f("file"), + ), + ), + ), + ), + f("zzrootfile"), } mm := func(data interface{}) string { - bytes, err := json.Marshal(data) + bytes, err := json.MarshalIndent(data, "", " ") if err != nil { panic(err) } @@ -1784,150 +1799,150 @@ func TestGlobalDirectoryTree(t *testing.T) { m.Index(device1, "default", testdata) - result := m.GlobalDirectoryTree("default", "", -1, false) + result, _ := m.GlobalDirectoryTree("default", "", -1, false) if mm(result) != mm(expectedResult) { - t.Errorf("Does not match:\n%#v\n%#v", result, expectedResult) + t.Errorf("Does not match:\n%s\n============\n%s", mm(result), mm(expectedResult)) } - result = m.GlobalDirectoryTree("default", "another", -1, false) + result, _ = m.GlobalDirectoryTree("default", "another", -1, false) - if mm(result) != mm(expectedResult["another"]) { - t.Errorf("Does not match:\n%s\n%s", mm(result), mm(expectedResult["another"])) + if mm(result) != mm(findByName(expectedResult, "another").Children) { + t.Errorf("Does not match:\n%s\n============\n%s", mm(result), mm(findByName(expectedResult, "another").Children)) } - result = m.GlobalDirectoryTree("default", "", 0, false) - currentResult := map[string]interface{}{ - "another": map[string]interface{}{}, - "other": map[string]interface{}{}, - "some": map[string]interface{}{}, - "rootfile": filedata, + result, _ = m.GlobalDirectoryTree("default", "", 0, false) + currentResult := []*TreeEntry{ + d("another"), + d("other"), + d("some"), + f("zzrootfile"), + } + + if mm(result) != mm(currentResult) { + t.Errorf("Does not match:\n%s\n============\n%s", mm(result), mm(currentResult)) + } + + result, _ = m.GlobalDirectoryTree("default", "", 1, false) + currentResult = []*TreeEntry{ + d("another", + d("directory"), + f("file"), + ), + d("other", + d("rand"), + d("random"), + d("randomx"), + ), + d("some", + d("directory"), + ), + f("zzrootfile"), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "", 1, false) - currentResult = map[string]interface{}{ - "another": map[string]interface{}{ - "directory": map[string]interface{}{}, - "file": filedata, - }, - "other": map[string]interface{}{ - "rand": map[string]interface{}{}, - "random": map[string]interface{}{}, - "randomx": map[string]interface{}{}, - }, - "some": map[string]interface{}{ - "directory": map[string]interface{}{}, - }, - "rootfile": filedata, + result, _ = m.GlobalDirectoryTree("default", "", -1, true) + currentResult = []*TreeEntry{ + d("another", + d("directory", + d("with", + d("a"), + ), + ), + ), + d("other", + d("rand"), + d("random", + d("dir"), + d("dirx"), + ), + d("randomx"), + ), + d("some", + d("directory", + d("with", + d("a"), + ), + ), + ), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "", -1, true) - currentResult = map[string]interface{}{ - "another": map[string]interface{}{ - "directory": map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{}, - }, - }, - }, - "other": map[string]interface{}{ - "rand": map[string]interface{}{}, - "random": map[string]interface{}{ - "dir": map[string]interface{}{}, - "dirx": map[string]interface{}{}, - }, - "randomx": map[string]interface{}{}, - }, - "some": map[string]interface{}{ - "directory": map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{}, - }, - }, - }, + result, _ = m.GlobalDirectoryTree("default", "", 1, true) + currentResult = []*TreeEntry{ + d("another", + d("directory"), + ), + d("other", + d("rand"), + d("random"), + d("randomx"), + ), + d("some", + d("directory"), + ), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "", 1, true) - currentResult = map[string]interface{}{ - "another": map[string]interface{}{ - "directory": map[string]interface{}{}, - }, - "other": map[string]interface{}{ - "rand": map[string]interface{}{}, - "random": map[string]interface{}{}, - "randomx": map[string]interface{}{}, - }, - "some": map[string]interface{}{ - "directory": map[string]interface{}{}, - }, + result, _ = m.GlobalDirectoryTree("default", "another", 0, false) + currentResult = []*TreeEntry{ + d("directory"), + f("file"), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "another", 0, false) - currentResult = map[string]interface{}{ - "directory": map[string]interface{}{}, - "file": filedata, + result, _ = m.GlobalDirectoryTree("default", "some/directory", 0, false) + currentResult = []*TreeEntry{ + d("with"), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "some/directory", 0, false) - currentResult = map[string]interface{}{ - "with": map[string]interface{}{}, + result, _ = m.GlobalDirectoryTree("default", "some/directory", 1, false) + currentResult = []*TreeEntry{ + d("with", + d("a"), + ), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "some/directory", 1, false) - currentResult = map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{}, - }, + result, _ = m.GlobalDirectoryTree("default", "some/directory", 2, false) + currentResult = []*TreeEntry{ + d("with", + d("a", + f("file"), + ), + ), } if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) } - result = m.GlobalDirectoryTree("default", "some/directory", 2, false) - currentResult = map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{ - "file": filedata, - }, - }, - } - - if mm(result) != mm(currentResult) { - t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) - } - - result = m.GlobalDirectoryTree("default", "another", -1, true) - currentResult = map[string]interface{}{ - "directory": map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{}, - }, - }, + result, _ = m.GlobalDirectoryTree("default", "another", -1, true) + currentResult = []*TreeEntry{ + d("directory", + d("with", + d("a"), + ), + ), } if mm(result) != mm(currentResult) { @@ -1935,145 +1950,8 @@ func TestGlobalDirectoryTree(t *testing.T) { } // No prefix matching! - result = m.GlobalDirectoryTree("default", "som", -1, false) - currentResult = map[string]interface{}{} - - if mm(result) != mm(currentResult) { - t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) - } -} - -func TestGlobalDirectorySelfFixing(t *testing.T) { - w, fcfg, wCancel := tmpDefaultWrapper() - defer wCancel() - m := setupModel(t, w) - defer cleanupModelAndRemoveDir(m, fcfg.Filesystem().URI()) - - b := func(isfile bool, path ...string) protocol.FileInfo { - typ := protocol.FileInfoTypeDirectory - blocks := []protocol.BlockInfo{} - if isfile { - typ = protocol.FileInfoTypeFile - blocks = []protocol.BlockInfo{{Offset: 0x0, Size: 0xa, Hash: []uint8{0x2f, 0x72, 0xcc, 0x11, 0xa6, 0xfc, 0xd0, 0x27, 0x1e, 0xce, 0xf8, 0xc6, 0x10, 0x56, 0xee, 0x1e, 0xb1, 0x24, 0x3b, 0xe3, 0x80, 0x5b, 0xf9, 0xa9, 0xdf, 0x98, 0xf9, 0x2f, 0x76, 0x36, 0xb0, 0x5c}}} - } - return protocol.FileInfo{ - Name: filepath.Join(path...), - Type: typ, - ModifiedS: 0x666, - Blocks: blocks, - Size: 0xa, - } - } - - filedata := []interface{}{time.Unix(0x666, 0).Format(time.RFC3339), 0xa} - - testdata := []protocol.FileInfo{ - b(true, "another", "directory", "afile"), - b(true, "another", "directory", "with", "a", "file"), - b(true, "another", "directory", "with", "file"), - - b(false, "other", "random", "dirx"), - b(false, "other", "randomx"), - - b(false, "some", "directory", "with", "x"), - b(true, "some", "directory", "with", "a", "file"), - - b(false, "this", "is", "a", "deep", "invalid", "directory"), - - b(true, "xthis", "is", "a", "deep", "invalid", "file"), - } - expectedResult := map[string]interface{}{ - "another": map[string]interface{}{ - "directory": map[string]interface{}{ - "afile": filedata, - "with": map[string]interface{}{ - "a": map[string]interface{}{ - "file": filedata, - }, - "file": filedata, - }, - }, - }, - "other": map[string]interface{}{ - "random": map[string]interface{}{ - "dirx": map[string]interface{}{}, - }, - "randomx": map[string]interface{}{}, - }, - "some": map[string]interface{}{ - "directory": map[string]interface{}{ - "with": map[string]interface{}{ - "a": map[string]interface{}{ - "file": filedata, - }, - "x": map[string]interface{}{}, - }, - }, - }, - "this": map[string]interface{}{ - "is": map[string]interface{}{ - "a": map[string]interface{}{ - "deep": map[string]interface{}{ - "invalid": map[string]interface{}{ - "directory": map[string]interface{}{}, - }, - }, - }, - }, - }, - "xthis": map[string]interface{}{ - "is": map[string]interface{}{ - "a": map[string]interface{}{ - "deep": map[string]interface{}{ - "invalid": map[string]interface{}{ - "file": filedata, - }, - }, - }, - }, - }, - } - - mm := func(data interface{}) string { - bytes, err := json.Marshal(data) - if err != nil { - panic(err) - } - return string(bytes) - } - - m.Index(device1, "default", testdata) - - result := m.GlobalDirectoryTree("default", "", -1, false) - - if mm(result) != mm(expectedResult) { - t.Errorf("Does not match:\n%s\n%s", mm(result), mm(expectedResult)) - } - - result = m.GlobalDirectoryTree("default", "xthis/is/a/deep", -1, false) - currentResult := map[string]interface{}{ - "invalid": map[string]interface{}{ - "file": filedata, - }, - } - - if mm(result) != mm(currentResult) { - t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) - } - - result = m.GlobalDirectoryTree("default", "xthis/is/a/deep", -1, true) - currentResult = map[string]interface{}{ - "invalid": map[string]interface{}{}, - } - - if mm(result) != mm(currentResult) { - t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) - } - - // !!! This is actually BAD, because we don't have enough level allowance - // to accept this file, hence the tree is left unbuilt !!! - result = m.GlobalDirectoryTree("default", "xthis", 1, false) - currentResult = map[string]interface{}{} + result, _ = m.GlobalDirectoryTree("default", "som", -1, false) + currentResult = []*TreeEntry{} if mm(result) != mm(currentResult) { t.Errorf("Does not match:\n%s\n%s", mm(result), mm(currentResult)) diff --git a/lib/protocol/bep_extensions.go b/lib/protocol/bep_extensions.go index 88b7f29c5..a17111acc 100644 --- a/lib/protocol/bep_extensions.go +++ b/lib/protocol/bep_extensions.go @@ -5,6 +5,7 @@ package protocol import ( "bytes" "encoding/binary" + "encoding/json" "errors" "fmt" "runtime" @@ -394,3 +395,20 @@ func VectorHash(v Vector) []byte { } return h.Sum(nil) } + +func (x *FileInfoType) MarshalJSON() ([]byte, error) { + return json.Marshal(x.String()) +} + +func (x *FileInfoType) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return err + } + n, ok := FileInfoType_value[s] + if !ok { + return errors.New("invalid value: " + s) + } + *x = FileInfoType(n) + return nil +}