Skip to content

Commit

Permalink
Add the Delete action
Browse files Browse the repository at this point in the history
This commit also updates plugin.List's return type to plugin.EntryMap
and makes the appropriate modifications to reflect that change.

Signed-off-by: Enis Inan <[email protected]>
  • Loading branch information
ekinanp committed Oct 22, 2019
1 parent ebc4b72 commit a6d4dc5
Show file tree
Hide file tree
Showing 13 changed files with 192 additions and 63 deletions.
6 changes: 3 additions & 3 deletions api/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {

expectedChildren := make(map[string]plugin.Entry)
if children, err := plugin.List(reqCtx, parent); suite.Nil(err) {
suite.Equal(expectedChildren, children)
suite.Equal(expectedChildren, children.Map())
}

// Test clearing a different cache
Expand All @@ -107,7 +107,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
suite.Equal("[]\n", w.Body.String())

if children, err := plugin.List(context.Background(), parent); suite.Nil(err) {
suite.Equal(expectedChildren, children)
suite.Equal(expectedChildren, children.Map())
}

// Test clearing the cache
Expand All @@ -118,7 +118,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
suite.Equal(`["List::/dir"]`, strings.TrimSpace(w.Body.String()))

if children, err := plugin.List(context.Background(), parent); suite.Nil(err) {
suite.Equal(expectedChildren, children)
suite.Equal(expectedChildren, children.Map())
}

parent.AssertNumberOfCalls(suite.T(), "List", 2)
Expand Down
7 changes: 4 additions & 3 deletions api/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ var listHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
return erroredActionResponse(path, plugin.ListAction(), err.Error())
}

result := make([]apitypes.Entry, 0, len(entries))
for _, entry := range entries {
result := make([]apitypes.Entry, 0, entries.Len())
entries.Range(func(_ string, entry plugin.Entry) bool {
apiEntry := toAPIEntry(entry)
apiEntry.Path = path + "/" + apiEntry.CName
result = append(result, apiEntry)
}
return true
})
// Sort entries so they have a deterministic order.
sort.Slice(result, func(i, j int) bool { return result[i].Name < result[j].Name })
activity.Record(ctx, "API: List %v %+v", path, result)
Expand Down
22 changes: 12 additions & 10 deletions cmd/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,10 @@ func validateMain(cmd *cobra.Command, args []string) exitCode {

// We use a worker pool to limit work-in-progress. Put the plugin on the worker pool.
wp := cmdutil.NewPool(parallel)
for _, e := range entries {
entries.Range(func(_ string, e plugin.Entry) bool {
wp.Submit(func() { processEntry(ctx, pw, wp, e, all, errs) })
}
return true
})

// Wait for work to complete.
wp.Finish()
Expand Down Expand Up @@ -272,26 +273,25 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
return
}
cancelFunc()
entries := obj.(map[string]plugin.Entry)
entries := obj.(*plugin.EntryMap)

if all {
for _, entry := range entries {
// Make a local copy for the lambda to capture.
entry := entry
entries.Range(func(_ string, entry plugin.Entry) bool {
wp.Submit(func() { processEntry(ctx, pw, wp, entry, all, errs) })
}
return true
})
} else {
// Group children by ones that look "similar", and select one from each group to test.
groups := make(map[criteria][]plugin.Entry)
for _, entry := range entries {
entries.Range(func(_ string, entry plugin.Entry) bool {
// Skip files that we shouldn't read. This includes character devices, devices, and
// those we don't have permission to read.
attr := plugin.Attributes(entry)
if mode := attr.Mode(); attr.HasMode() &&
(mode&os.ModeCharDevice == os.ModeCharDevice ||
mode&os.ModeNamedPipe == os.ModeNamedPipe ||
mode&os.ModeDevice == os.ModeDevice || mode&0400 == 0) {
continue
return true
}

ccrit := newCriteria(entry)
Expand All @@ -300,7 +300,9 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
if ccrit.typeID == "" || ccrit.typeID != crit.typeID {
groups[ccrit] = append(groups[ccrit], entry)
}
}

return true
})

for _, items := range groups {
entry := items[rand.Intn(len(items))]
Expand Down
11 changes: 6 additions & 5 deletions fuse/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func newDir(p *dir, e plugin.Parent) *dir {
return &dir{newFuseNode("d", p, e)}
}

func (d *dir) children(ctx context.Context) (map[string]plugin.Entry, error) {
func (d *dir) children(ctx context.Context) (*plugin.EntryMap, error) {
// Check for an updated entry in case it has static state.
updatedEntry, err := d.refind(ctx)
if err != nil {
Expand Down Expand Up @@ -53,7 +53,7 @@ func (d *dir) Lookup(ctx context.Context, req *fuse.LookupRequest, resp *fuse.Lo
}

cname := req.Name
entry, ok := entries[cname]
entry, ok := entries.Load(cname)
if !ok {
log.Debugf("FUSE: %v not found in %v", req.Name, d)
return nil, fuse.ENOENT
Expand All @@ -79,15 +79,16 @@ func (d *dir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) {
return nil, err
}

res := make([]fuse.Dirent, 0, len(entries))
for cname, entry := range entries {
res := make([]fuse.Dirent, 0, entries.Len())
entries.Range(func(cname string, entry plugin.Entry) bool {
var de fuse.Dirent
de.Name = cname
if plugin.ListAction().IsSupportedOn(entry) {
de.Type = fuse.DT_Dir
}
res = append(res, de)
}
return true
})
activity.Record(ctx, "FUSE: Listed in %v: %+v", d, res)
return res, nil
}
9 changes: 9 additions & 0 deletions plugin/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var listAction = newAction("list", "Parent")
var readAction = newAction("read", "Readable")
var streamAction = newAction("stream", "Streamable")
var execAction = newAction("exec", "Execable")
var deleteAction = newAction("delete", "Deletable")

// ListAction represents the list action
func ListAction() Action {
Expand All @@ -54,6 +55,11 @@ func ExecAction() Action {
return execAction
}

// DeleteAction represents the delete action
func DeleteAction() Action {
return deleteAction
}

// Actions returns all of the available Wash actions as a map
// of <action_name> => <action_object>.
func Actions() map[string]Action {
Expand Down Expand Up @@ -98,6 +104,9 @@ func SupportedActionsOf(entry Entry) []string {
if _, ok := entry.(Execable); ok {
actions = append(actions, ExecAction().Name)
}
if _, ok := entry.(Deletable); ok {
actions = append(actions, DeleteAction().Name)
}

return actions
}
Expand Down
9 changes: 8 additions & 1 deletion plugin/analyticsWrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

// ListWithAnalytics is a wrapper to plugin.List. Use it when you need to report
// a 'List' invocation to analytics. Otherwise, use plugin.List
func ListWithAnalytics(ctx context.Context, p Parent) (map[string]Entry, error) {
func ListWithAnalytics(ctx context.Context, p Parent) (*EntryMap, error) {
submitMethodInvocation(ctx, p, "List")
return List(ctx, p)
}
Expand All @@ -35,6 +35,13 @@ func ExecWithAnalytics(ctx context.Context, e Execable, cmd string, args []strin
return Exec(ctx, e, cmd, args, opts)
}

// DeleteWithAnalytics is a wrapper to plugin.Delete. Use it when you need to report a
// 'Delete' invocation to analytics. Otherwise, use plugin.Delete.
func DeleteWithAnalytics(ctx context.Context, d Deletable) error {
submitMethodInvocation(ctx, d, "Delete")
return Delete(ctx, d)
}

func submitMethodInvocation(ctx context.Context, e Entry, method string) {
isCorePluginEntry := e.Schema() != nil
if !isCorePluginEntry {
Expand Down
11 changes: 6 additions & 5 deletions plugin/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (c DuplicateCNameErr) Error() string {
//
// CachedList returns a map of <entry_cname> => <entry_object> to optimize
// querying a specific entry.
func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {
cachedEntries, err := cachedDefaultOp(ctx, ListOp, p, func() (interface{}, error) {
// Including the entry's ID allows plugin authors to use any Cached* methods defined on the
// children after their creation. This is necessary when the child's Cached* methods are used
Expand All @@ -156,11 +156,11 @@ func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
return nil, err
}

searchedEntries := make(map[string]Entry)
searchedEntries := newEntryMap()
for _, entry := range entries {
cname := CName(entry)

if duplicateEntry, ok := searchedEntries[cname]; ok {
if duplicateEntry, ok := searchedEntries.mp[cname]; ok {
return nil, DuplicateCNameErr{
ParentID: p.id(),
FirstChildName: duplicateEntry.name(),
Expand All @@ -175,7 +175,8 @@ func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
// Skip entries that are expected to be inaccessible.
continue
}
searchedEntries[cname] = entry

searchedEntries.mp[cname] = entry

// Ensure ID is set on all entries so that we can use it for caching later in places
// where the context doesn't include the parent's ID.
Expand All @@ -191,7 +192,7 @@ func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
return nil, err
}

return cachedEntries.(map[string]Entry), nil
return cachedEntries.(*EntryMap), nil
}

// CachedOpen caches a Readable object's Open method.
Expand Down
15 changes: 8 additions & 7 deletions plugin/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ func toMap(children []Entry) map[string]Entry {

func (suite *CacheTestSuite) TestCachedListDefaultOp() {
mockChildren := []Entry{newCacheTestsMockEntry("mockChild")}
mungedOpValue := toMap(mockChildren)
mungedOpValue := newEntryMap()
mungedOpValue.mp = toMap(mockChildren)
suite.testCachedDefaultOp(ListOp, "List", mockChildren, mungedOpValue, func(ctx context.Context, e Entry) (interface{}, error) {
return cachedList(ctx, e.(Parent))
})
Expand Down Expand Up @@ -315,9 +316,9 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
children, err := cachedList(ctx, entry)
if suite.NoError(err) {
if suite.Equal(toMap(mockChildren), children) {
suite.Equal("/foo#child1", children["foo#child1"].id())
suite.Equal("/child2", children["child2"].id())
if suite.Equal(toMap(mockChildren), children.mp) {
suite.Equal("/foo#child1", children.mp["foo#child1"].id())
suite.Equal("/child2", children.mp["child2"].id())
}
}

Expand All @@ -328,9 +329,9 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
children, err = cachedList(ctx, entry)
if suite.NoError(err) {
if suite.Equal(toMap(mockChildren), children) {
suite.Equal("/parent/foo#child1", children["foo#child1"].id())
suite.Equal("/parent/child2", children["child2"].id())
if suite.Equal(toMap(mockChildren), children.mp) {
suite.Equal("/parent/foo#child1", children.mp["foo#child1"].id())
suite.Equal("/parent/child2", children.mp["child2"].id())
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/findEntry.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func FindEntry(ctx context.Context, start Entry, segments []string) (Entry, erro
}

// Search for the specific entry
entry, ok := entries[segment]
entry, ok := entries.Load(segment)
if !ok {
reason := fmt.Sprintf("The %v entry does not exist", segment)
if len(visitedSegments) != 0 {
Expand Down
23 changes: 20 additions & 3 deletions plugin/methodWrappers.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ func Schema(e Entry) (*EntrySchema, error) {
return schema(e)
}

// List lists the parent's children. It returns a map of <entry_cname> => <entry_object>
// to optimize querying a specific entry.
// List lists the parent's children. It returns an EntryMap to optimize querying a specific
// entry.
//
// Note that List's results could be cached.
func List(ctx context.Context, p Parent) (map[string]Entry, error) {
func List(ctx context.Context, p Parent) (*EntryMap, error) {
return cachedList(ctx, p)
}

Expand Down Expand Up @@ -137,3 +137,20 @@ func Exec(ctx context.Context, e Execable, cmd string, args []string, opts ExecO
func Stream(ctx context.Context, s Streamable) (io.ReadCloser, error) {
return s.Stream(ctx)
}

// Delete deletes the given entry.
func Delete(ctx context.Context, d Deletable) error {
if err := d.Delete(ctx); err != nil {
return err
}
ClearCacheFor(ID(d))
// Delete this entry from the parent's cached list result
segments := strings.Split(d.id(), "/")
parentID := strings.Join(segments[:len(segments)-1], "/")
entries, _ := cache.Get(defaultOpCodeToNameMap[ListOp], parentID)
if entries != nil {
cname := segments[len(segments)-1]
entries.(*EntryMap).Delete(cname)
}
return nil
}
Loading

0 comments on commit a6d4dc5

Please sign in to comment.