diff --git a/stbridge/saf.go b/stbridge/saf.go index 474363d..dbe79f2 100644 --- a/stbridge/saf.go +++ b/stbridge/saf.go @@ -36,6 +36,7 @@ var ( "removeChildLocked", "getInfo", "getChildren", + "getChild", "Resolve", "DirNames", "getOrCreateChild", @@ -521,19 +522,15 @@ func (sn *safNode) getInfo(opts *safOpts) (*safFileInfo, error) { } // Get the current node's list of children if it is a directory, refreshing the -// list if it expired. This returns a copy of the internal field. +// list if it expired. This returns the internal children map and must be called +// while the lock is held. // // If this node is not a directory and allowFiles is false, an error wrapping // [syscall.ENOTDIR] is returned. The cached metadata info is used to determine // if the node is a directory. [safNode.getInfoLocked] is never called. Because // SAF returns the metadata of all the children, the children will have // non-expired info fields, but expired (and empty/nil) children fields. -func (sn *safNode) getChildren(opts *safOpts, allowFiles bool) (map[string]*safNode, error) { - implLogf("getChildren", "sn=%q, allowFiles=%v", sn.uri, allowFiles) - - sn.lock.Lock() - defer sn.lock.Unlock() - +func (sn *safNode) getChildrenLocked(opts *safOpts, allowFiles bool) (map[string]*safNode, error) { if sn.children == nil { if allowFiles { return nil, nil @@ -544,7 +541,7 @@ func (sn *safNode) getChildren(opts *safOpts, allowFiles bool) (map[string]*safN now := time.Now() if !now.After(time.Unix(sn.childrenExpiry, 0)) { - return maps.Clone(sn.children), nil + return sn.children, nil } expiry := now.Add(opts.cacheDuration) @@ -617,9 +614,56 @@ func (sn *safNode) getChildren(opts *safOpts, allowFiles bool) (map[string]*safN sn.updateChildrenLocked(opts, children, expiry.Unix()) + return children, nil +} + +// Get the current node's list of children if it is a directory, refreshing the +// list if it expired. This returns a copy of the internal field. +// +// If this node is not a directory and allowFiles is false, an error wrapping +// [syscall.ENOTDIR] is returned. The cached metadata info is used to determine +// if the node is a directory. [safNode.getInfoLocked] is never called. Because +// SAF returns the metadata of all the children, the children will have +// non-expired info fields, but expired (and empty/nil) children fields. +func (sn *safNode) getChildren(opts *safOpts, allowFiles bool) (map[string]*safNode, error) { + implLogf("getChildren", "sn=%q, allowFiles=%v", sn.uri, allowFiles) + + sn.lock.Lock() + defer sn.lock.Unlock() + + children, err := sn.getChildrenLocked(opts, allowFiles) + if err != nil { + return nil, err + } + return maps.Clone(children), nil } +// Get the specified child from the current node's list of children if it is a +// directory, refreshing the list if it expired. This always returns either a +// non-nil child node or an error. +func (sn *safNode) getChild(opts *safOpts, name string) (*safNode, error) { + implLogf("getChild", "sn=%q, name=%q", sn.uri, name) + + sn.lock.Lock() + defer sn.lock.Unlock() + + children, err := sn.getChildrenLocked(opts, false) + if err != nil { + return nil, err + } + + child, ok := children[name] + if !ok { + return nil, fmt.Errorf( + "%q not a child of %q: %w", + name, sn.uri, os.ErrNotExist, + ) + } + + return child, err +} + // Get the child node referenced by the specified relative path. ".." components // are permitted in the path if they don't result in escaping the current node. // Evaluating ".." is done lexicographically without regard for whether the @@ -638,19 +682,11 @@ func (sn *safNode) Resolve(opts *safOpts, name string) (*safNode, error) { current := sn for _, component := range components { - children, err := current.getChildren(opts, false) + child, err := current.getChild(opts, component) if err != nil { return nil, err } - child, ok := children[component] - if !ok { - return nil, fmt.Errorf( - "%q not a child of %q: %w", - component, current.uri, os.ErrNotExist, - ) - } - current = child } diff --git a/stbridge/saf_test.go b/stbridge/saf_test.go index 5c1d576..a2209c9 100644 --- a/stbridge/saf_test.go +++ b/stbridge/saf_test.go @@ -408,6 +408,61 @@ func TestGetRoots(t *testing.T) { } } +func TestGetChild(t *testing.T) { + node := &safNode{ + uri: "uri://0", + name: "0", + size: 0, + mtime: 0, + infoExpiry: safExpired, + children: makeChildren(true), + childrenExpiry: safExpired, + watchManager: &safWatchManager{}, + } + childFileInfo := safFileInfo{ + Uri: "uri://1", + Name_: "1", + Size_: 1, + Mtime: 1, + IsDir_: false, + } + + calls := 0 + + client, opts := newTestClient() + client.queryChildDocumentsJson = func(documentUri string) (string, error) { + calls++ + return fileInfoListJson(&childFileInfo), nil + } + + child, err := node.getChild(opts, "2") + if child != nil || !errors.Is(err, os.ErrNotExist) { + t.Fatalf("did not fail with ENOENT: %+v: %+v", child, err) + } + if calls != 1 { + t.Errorf("queryChildDocumentsJson was not called when expired") + } + + child, err = node.getChild(opts, "1") + if err != nil { + t.Fatal(err) + } + if calls != 1 { + t.Errorf("queryChildDocumentsJson was called when not expired") + } + + actualInfo := child.cachedInfoLocked() + + if actualInfo != childFileInfo { + t.Errorf("child info does not match: %+v != %+v", actualInfo, childFileInfo) + } + + child, err = child.getChild(opts, "3") + if child != nil || !errors.Is(err, syscall.ENOTDIR) { + t.Errorf("did not fail with ENOTDIR: %+v: %+v", child, err) + } +} + func TestResolveSelf(t *testing.T) { node := &safNode{ uri: "0",