Repository: puppetlabs/wash
Pull request #551 (2019-10-18 05:58:00)

Create method wrappers for each entry method
Resolves https://github.com/puppetlabs/wash/issues/550


Patch:
diff --git a/api/exec.go b/api/exec.go
index 25f78b6..77df6c9 100644
--- a/api/exec.go
+++ b/api/exec.go
@@ -90,7 +90,7 @@ var execHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 	if body.Opts.Input != "" {
 		opts.Stdin = strings.NewReader(body.Opts.Input)
 	}
-	cmd, err := plugin.Exec(ctx, entry.(plugin.Execable), body.Cmd, body.Args, opts)
+	cmd, err := plugin.ExecWithAnalytics(ctx, entry.(plugin.Execable), body.Cmd, body.Args, opts)
 	if err != nil {
 		return erroredActionResponse(path, plugin.ExecAction(), err.Error())
 	}
diff --git a/api/list.go b/api/list.go
index 67e016e..1d3bf9b 100644
--- a/api/list.go
+++ b/api/list.go
@@ -46,7 +46,7 @@ var listHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 	}
 
 	parent := entry.(plugin.Parent)
-	entries, err := plugin.List(ctx, parent)
+	entries, err := plugin.ListWithAnalytics(ctx, parent)
 	if err != nil {
 		if cnameErr, ok := err.(plugin.DuplicateCNameErr); ok {
 			return duplicateCNameResponse(cnameErr)
diff --git a/api/metadata.go b/api/metadata.go
index 8ab45d5..ae7e401 100644
--- a/api/metadata.go
+++ b/api/metadata.go
@@ -37,7 +37,7 @@ var metadataHandler handler = func(w http.ResponseWriter, r *http.Request) *erro
 		return errResp
 	}
 
-	metadata, err := plugin.CachedMetadata(ctx, entry)
+	metadata, err := plugin.Metadata(ctx, entry)
 
 	if err != nil {
 		return unknownErrorResponse(err)
diff --git a/api/read.go b/api/read.go
index 944ce6a..4d92110 100644
--- a/api/read.go
+++ b/api/read.go
@@ -35,7 +35,7 @@ var readHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 		return unsupportedActionResponse(path, plugin.ReadAction())
 	}
 
-	content, err := plugin.Open(ctx, entry.(plugin.Readable))
+	content, err := plugin.OpenWithAnalytics(ctx, entry.(plugin.Readable))
 
 	if err != nil {
 		return erroredActionResponse(path, plugin.ReadAction(), err.Error())
diff --git a/api/stream.go b/api/stream.go
index dfb96a5..e62f55b 100644
--- a/api/stream.go
+++ b/api/stream.go
@@ -41,7 +41,7 @@ var streamHandler handler = func(w http.ResponseWriter, r *http.Request) *errorR
 	}
 
 	ctx := r.Context()
-	rdr, err := plugin.Stream(ctx, entry.(plugin.Streamable))
+	rdr, err := plugin.StreamWithAnalytics(ctx, entry.(plugin.Streamable))
 
 	if err != nil {
 		return erroredActionResponse(path, plugin.StreamAction(), err.Error())
diff --git a/cmd/validate.go b/cmd/validate.go
index 23698d5..f8aa4b4 100644
--- a/cmd/validate.go
+++ b/cmd/validate.go
@@ -138,10 +138,10 @@ func validateMain(cmd *cobra.Command, args []string) exitCode {
 		wg.Done()
 	}()
 
-	// Use CachedList on the registry to ensure cache IDs are generated.
-	entries, err := plugin.CachedList(ctx, registry)
+	// Use List on the registry to ensure cache IDs are generated.
+	entries, err := plugin.List(ctx, registry)
 	if err != nil {
-		panic("CachedList on registry should not fail")
+		panic("List on registry should not fail")
 	}
 
 	// We use a worker pool to limit work-in-progress. Put the plugin on the worker pool.
@@ -265,7 +265,7 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 
 	if plugin.ListAction().IsSupportedOn(e) {
 		obj, cancelFunc, err := withTimeout(ctx, "list", name, func(ctx context.Context) (interface{}, error) {
-			return plugin.CachedList(ctx, e.(plugin.Parent))
+			return plugin.List(ctx, e.(plugin.Parent))
 		})
 		if err != nil {
 			errs <- err
@@ -312,7 +312,7 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 
 	if plugin.ReadAction().IsSupportedOn(e) {
 		_, cancelFunc, err := withTimeout(ctx, "read", name, func(ctx context.Context) (interface{}, error) {
-			return plugin.CachedOpen(ctx, e.(plugin.Readable))
+			return plugin.Open(ctx, e.(plugin.Readable))
 		})
 		if err != nil {
 			errs <- err
@@ -324,7 +324,7 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 
 	if plugin.StreamAction().IsSupportedOn(e) {
 		obj, cancelFunc, err := withTimeout(ctx, "stream", name, func(ctx context.Context) (interface{}, error) {
-			return e.(plugin.Streamable).Stream(ctx)
+			return plugin.Stream(ctx, e.(plugin.Streamable))
 		})
 		if err != nil {
 			errs <- err
@@ -338,7 +338,7 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 	if plugin.ExecAction().IsSupportedOn(e) {
 		const testMessage = "hello"
 		obj, cancelFunc, err := withTimeout(ctx, "exec", name, func(ctx context.Context) (interface{}, error) {
-			return e.(plugin.Execable).Exec(ctx, "echo", []string{testMessage}, plugin.ExecOptions{})
+			return plugin.Exec(ctx, e.(plugin.Execable), "echo", []string{testMessage}, plugin.ExecOptions{})
 		})
 		if err != nil {
 			errs <- err
diff --git a/fuse/dir.go b/fuse/dir.go
index 8def2a8..72f2d72 100644
--- a/fuse/dir.go
+++ b/fuse/dir.go
@@ -34,7 +34,7 @@ func (d *dir) children(ctx context.Context) (map[string]plugin.Entry, error) {
 
 	// Cache List requests. FUSE often lists the contents then immediately calls find on individual entries.
 	if plugin.ListAction().IsSupportedOn(updatedEntry) {
-		return plugin.List(ctx, updatedEntry.(plugin.Parent))
+		return plugin.ListWithAnalytics(ctx, updatedEntry.(plugin.Parent))
 	}
 
 	return nil, fuse.ENOENT
diff --git a/fuse/file.go b/fuse/file.go
index 13e12de..13ec9e5 100644
--- a/fuse/file.go
+++ b/fuse/file.go
@@ -36,7 +36,7 @@ func (f *file) Open(ctx context.Context, req *fuse.OpenRequest, resp *fuse.OpenR
 
 	// Initiate content request and return a channel providing the results.
 	if plugin.ReadAction().IsSupportedOn(updatedEntry) {
-		content, err := plugin.Open(ctx, updatedEntry.(plugin.Readable))
+		content, err := plugin.OpenWithAnalytics(ctx, updatedEntry.(plugin.Readable))
 		if err != nil {
 			activity.Warnf(ctx, "FUSE: Open %v errored: %v", f, err)
 			return nil, err
diff --git a/plugin/analyticsWrappers.go b/plugin/analyticsWrappers.go
index 0fc0d56..1e01f5b 100644
--- a/plugin/analyticsWrappers.go
+++ b/plugin/analyticsWrappers.go
@@ -7,32 +7,32 @@ import (
 	"github.com/puppetlabs/wash/activity"
 )
 
-// List is a wrapper to plugin.CachedList. Use it when you need to report
-// a 'List' invocation to analytics. Otherwise, use plugin.CachedList
-func List(ctx context.Context, p Parent) (map[string]Entry, error) {
+// 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) {
 	submitMethodInvocation(ctx, p, "List")
-	return CachedList(ctx, p)
+	return List(ctx, p)
 }
 
-// Open is a wrapper to plugin.CachedOpen. Use it when you need to report
-// a 'Read' invocation to analytics. Otherwise, use plugin.CachedOpen
-func Open(ctx context.Context, r Readable) (SizedReader, error) {
+// OpenWithAnalytics is a wrapper to plugin.Open. Use it when you need to report
+// a 'Read' invocation to analytics. Otherwise, use plugin.Open
+func OpenWithAnalytics(ctx context.Context, r Readable) (SizedReader, error) {
 	submitMethodInvocation(ctx, r, "Read")
-	return CachedOpen(ctx, r)
+	return Open(ctx, r)
 }
 
-// Stream is a wrapper to s#Stream. Use it when you need to report a 'Stream'
+// StreamWithAnalytics is a wrapper to s#Stream. Use it when you need to report a 'Stream'
 // invocation to analytics. Otherwise, use s#Stream
-func Stream(ctx context.Context, s Streamable) (io.ReadCloser, error) {
+func StreamWithAnalytics(ctx context.Context, s Streamable) (io.ReadCloser, error) {
 	submitMethodInvocation(ctx, s, "Stream")
-	return s.Stream(ctx)
+	return Stream(ctx, s)
 }
 
-// Exec is a wrapper to e#Exec. Use it when you need to report an 'Exec'
+// ExecWithAnalytics is a wrapper to e#Exec. Use it when you need to report an 'Exec'
 // invocation to analytics. Otherwise, use e#Exec.
-func Exec(ctx context.Context, e Execable, cmd string, args []string, opts ExecOptions) (ExecCommand, error) {
+func ExecWithAnalytics(ctx context.Context, e Execable, cmd string, args []string, opts ExecOptions) (ExecCommand, error) {
 	submitMethodInvocation(ctx, e, "Exec")
-	return e.Exec(ctx, cmd, args, opts)
+	return Exec(ctx, e, cmd, args, opts)
 }
 
 func submitMethodInvocation(ctx context.Context, e Entry, method string) {
diff --git a/plugin/cache.go b/plugin/cache.go
index d8b0501..f554be6 100644
--- a/plugin/cache.go
+++ b/plugin/cache.go
@@ -149,7 +149,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) (map[string]Entry, 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
@@ -196,7 +196,7 @@ func CachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
 // When using the reader returned by this method, use idempotent read operations
 // such as ReadAt or wrap it in a SectionReader. Using Read operations on the cached
 // reader will change it and make subsequent uses of the cached reader invalid.
-func CachedOpen(ctx context.Context, r Readable) (SizedReader, error) {
+func cachedOpen(ctx context.Context, r Readable) (SizedReader, error) {
 	cachedContent, err := cachedDefaultOp(ctx, OpenOp, r, func() (interface{}, error) {
 		return r.Open(ctx)
 	})
@@ -208,8 +208,8 @@ func CachedOpen(ctx context.Context, r Readable) (SizedReader, error) {
 	return cachedContent.(SizedReader), nil
 }
 
-// CachedMetadata caches an entry's Metadata method
-func CachedMetadata(ctx context.Context, e Entry) (JSONObject, error) {
+// cachedMetadata caches an entry's Metadata method
+func cachedMetadata(ctx context.Context, e Entry) (JSONObject, error) {
 	cachedMetadata, err := cachedDefaultOp(ctx, MetadataOp, e, func() (interface{}, error) {
 		return e.Metadata(ctx)
 	})
diff --git a/plugin/entrySchema.go b/plugin/entrySchema.go
index 4627ba8..e1d77be 100644
--- a/plugin/entrySchema.go
+++ b/plugin/entrySchema.go
@@ -23,9 +23,7 @@ func TypeID(e Entry) string {
 	return namespace(pluginName, rawTypeID)
 }
 
-// Schema returns the entry's schema. It is needed by the API,
-// so plugin authors should ignore this.
-func Schema(e Entry) (*EntrySchema, error) {
+func schema(e Entry) (*EntrySchema, error) {
 	switch t := e.(type) {
 	case externalPlugin:
 		return t.schema()
diff --git a/plugin/findEntry.go b/plugin/findEntry.go
index 6ceeabe..0fe0488 100644
--- a/plugin/findEntry.go
+++ b/plugin/findEntry.go
@@ -13,7 +13,7 @@ func FindEntry(ctx context.Context, start Entry, segments []string) (Entry, erro
 		switch curParent := start.(type) {
 		case Parent:
 			// Get the entries via. List()
-			entries, err := CachedList(ctx, curParent)
+			entries, err := List(ctx, curParent)
 			if err != nil {
 				return nil, err
 			}
diff --git a/plugin/helpers.go b/plugin/helpers.go
index 7a31afd..2f09c98 100644
--- a/plugin/helpers.go
+++ b/plugin/helpers.go
@@ -1,103 +1,14 @@
 package plugin
 
 import (
-	"fmt"
-	"strings"
 	"time"
 
 	log "github.com/sirupsen/logrus"
 )
 
-// DefaultTimeout is the default timeout for prefetching
-var DefaultTimeout = 10 * time.Second
-
-/*
-Name returns the entry's name as it was passed into
-plugin.NewEntry. It is meant to be called by other
-Wash packages. Plugin authors should use EntryBase#Name
-when writing their plugins.
-*/
-func Name(e Entry) string {
-	// The reason we don't expose EntryBase#Name in the Entry
-	// interface is so plugin authors don't override it. It ensures
-	// that whatever name they pass into plugin.NewEntry is the
-	// name received by Wash.
-	return e.name()
-}
-
-/*
-CName returns the entry's canonical name, which is what Wash uses to
-construct the entry's path. The entry's cname is plugin.Name(e), but with
-all '/' characters replaced by a '#' character. CNames are necessary
-because it is possible for entry names to have '/'es in them, which is
-illegal in bourne shells and UNIX-y filesystems.
-
-CNames are unique. CName uniqueness is checked in plugin.CachedList.
-
-NOTE: The '#' character was chosen because it is unlikely to appear in
-a meaningful entry's name. If, however, there's a good chance that an
-entry's name can contain the '#' character, and that two entries can
-have the same cname (e.g. 'foo/bar', 'foo#bar'), then you can use
-e.SetSlashReplacer(<char>) to change the default slash replacer from
-a '#' to <char>.
-*/
-func CName(e Entry) string {
-	if len(e.name()) == 0 {
-		panic("plugin.CName: e.name() is empty")
-	}
-	// We make the CName a separate function instead of embedding it
-	// in the Entry interface because doing so prevents plugin authors
-	// from overriding it.
-	return strings.Replace(
-		e.name(),
-		"/",
-		string(e.slashReplacer()),
-		-1,
-	)
-}
-
-// ID returns the entry's ID, which is just its path rooted at Wash's mountpoint.
-// An entry's ID is described as
-//     /<plugin_name>/<parent1_cname>/<parent2_cname>/.../<entry_cname>
-//
-// NOTE: <plugin_name> is really <plugin_cname>. However since <plugin_name>
-// can never contain a '/', <plugin_cname> reduces to <plugin_name>.
-func ID(e Entry) string {
-	if e.id() == "" {
-		msg := fmt.Sprintf("plugin.ID: entry %v (cname %v) has no ID", e.name(), CName(e))
-		panic(msg)
-	}
-	return e.id()
-}
-
 // TrackTime helper is useful for timing functions.
 // Use with `defer plugin.TrackTime(time.Now(), "funcname")`.
 func TrackTime(start time.Time, name string) {
 	elapsed := time.Since(start)
 	log.Infof("%s took %s", name, elapsed)
 }
-
-// Attributes returns the entry's attributes. If size is unknown, it will check whether the entry
-// has locally cached content and if so set that for the size.
-func Attributes(e Entry) EntryAttributes {
-	// Sometimes an entry doesn't know its size unless it's already downloaded some content. Having
-	// to download content makes list slow, and is a burden for external plugin developers. Check if
-	// we already know the size. If not, FUSE will use a reasonable default so tools don't ignore it.
-	attr := e.attributes()
-	if !attr.HasSize() && cache != nil {
-		// We have no way to preserve this on the entry, and it likely wouldn't help because we often
-		// recreate the entry to ensure we have an accurate representation. So when the cache expires
-		// we revert to stating the size is unknown until the next read operation.
-		if val, _ := cache.Get(defaultOpCodeToNameMap[OpenOp], e.id()); val != nil {
-			rdr := val.(SizedReader)
-			attr.SetSize(uint64(rdr.Size()))
-		}
-	}
-	return attr
-}
-
-// IsPrefetched returns whether an entry has data that was added during creation that it would
-// like to have updated.
-func IsPrefetched(e Entry) bool {
-	return e.isPrefetched()
-}
diff --git a/plugin/metadataJSON.go b/plugin/metadataJSON.go
index e7bdb80..a058ec6 100644
--- a/plugin/metadataJSON.go
+++ b/plugin/metadataJSON.go
@@ -42,7 +42,7 @@ func (m *MetadataJSONFile) Schema() *EntrySchema {
 
 // Open returns the metadata of the `other` entry as its content.
 func (m *MetadataJSONFile) Open(ctx context.Context) (SizedReader, error) {
-	meta, err := CachedMetadata(ctx, m.other)
+	meta, err := Metadata(ctx, m.other)
 	if err != nil {
 		return nil, err
 	}
diff --git a/plugin/methodWrappers.go b/plugin/methodWrappers.go
new file mode 100644
index 0000000..2c9cce1
--- /dev/null
+++ b/plugin/methodWrappers.go
@@ -0,0 +1,139 @@
+package plugin
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"strings"
+	"time"
+)
+
+// This file contains all of the plugin.<Method> wrappers. You should
+// invoke plugin.<Method> instead of e.<Method> because plugin.<Method>
+// could contain additional, plugin-agnostic code required to get e.<Method>
+// working correctly (like e.g. caching and validation).
+
+// DefaultTimeout is the default timeout for prefetching
+var DefaultTimeout = 10 * time.Second
+
+/*
+Name returns the entry's name as it was passed into
+plugin.NewEntry. It is meant to be called by other
+Wash packages. Plugin authors should use EntryBase#Name
+when writing their plugins.
+*/
+func Name(e Entry) string {
+	// The reason we don't expose EntryBase#Name in the Entry
+	// interface is so plugin authors don't override it. It ensures
+	// that whatever name they pass into plugin.NewEntry is the
+	// name received by Wash.
+	return e.name()
+}
+
+/*
+CName returns the entry's canonical name, which is what Wash uses to
+construct the entry's path. The entry's cname is plugin.Name(e), but with
+all '/' characters replaced by a '#' character. CNames are necessary
+because it is possible for entry names to have '/'es in them, which is
+illegal in bourne shells and UNIX-y filesystems.
+
+CNames are unique. CName uniqueness is checked in plugin.CachedList.
+
+NOTE: The '#' character was chosen because it is unlikely to appear in
+a meaningful entry's name. If, however, there's a good chance that an
+entry's name can contain the '#' character, and that two entries can
+have the same cname (e.g. 'foo/bar', 'foo#bar'), then you can use
+e.SetSlashReplacer(<char>) to change the default slash replacer from
+a '#' to <char>.
+*/
+func CName(e Entry) string {
+	if len(e.name()) == 0 {
+		panic("plugin.CName: e.name() is empty")
+	}
+	// We make the CName a separate function instead of embedding it
+	// in the Entry interface because doing so prevents plugin authors
+	// from overriding it.
+	return strings.Replace(
+		e.name(),
+		"/",
+		string(e.slashReplacer()),
+		-1,
+	)
+}
+
+// ID returns the entry's ID, which is just its path rooted at Wash's mountpoint.
+// An entry's ID is described as
+//     /<plugin_name>/<parent1_cname>/<parent2_cname>/.../<entry_cname>
+//
+// NOTE: <plugin_name> is really <plugin_cname>. However since <plugin_name>
+// can never contain a '/', <plugin_cname> reduces to <plugin_name>.
+func ID(e Entry) string {
+	if e.id() == "" {
+		msg := fmt.Sprintf("plugin.ID: entry %v (cname %v) has no ID", e.name(), CName(e))
+		panic(msg)
+	}
+	return e.id()
+}
+
+// Attributes returns the entry's attributes. If size is unknown, it will check whether the entry
+// has locally cached content and if so set that for the size.
+func Attributes(e Entry) EntryAttributes {
+	// Sometimes an entry doesn't know its size unless it's already downloaded some content. Having
+	// to download content makes list slow, and is a burden for external plugin developers. Check if
+	// we already know the size. If not, FUSE will use a reasonable default so tools don't ignore it.
+	attr := e.attributes()
+	if !attr.HasSize() && cache != nil {
+		// We have no way to preserve this on the entry, and it likely wouldn't help because we often
+		// recreate the entry to ensure we have an accurate representation. So when the cache expires
+		// we revert to stating the size is unknown until the next read operation.
+		if val, _ := cache.Get(defaultOpCodeToNameMap[OpenOp], e.id()); val != nil {
+			rdr := val.(SizedReader)
+			attr.SetSize(uint64(rdr.Size()))
+		}
+	}
+	return attr
+}
+
+// IsPrefetched returns whether an entry has data that was added during creation that it would
+// like to have updated.
+func IsPrefetched(e Entry) bool {
+	return e.isPrefetched()
+}
+
+// Schema returns the entry's schema.
+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.
+//
+// Note that List's results could be cached.
+func List(ctx context.Context, p Parent) (map[string]Entry, error) {
+	return cachedList(ctx, p)
+}
+
+// Open reads the entry's content. Note that Open's results could be cached. Thus, when
+// using the reader returned by this method, use idempotent read operations such as ReadAt
+// or wrap it in a SectionReader. Using Read operations on the cached reader will change it
+// and make subsequent uses of the cached reader invalid.
+//
+// TODO: Could we change this to Read? E.g. plugin.Read.
+func Open(ctx context.Context, r Readable) (SizedReader, error) {
+	return cachedOpen(ctx, r)
+}
+
+// Metadata returns the entry's metadata. Note that Metadata's results could be cached.
+func Metadata(ctx context.Context, e Entry) (JSONObject, error) {
+	return cachedMetadata(ctx, e)
+}
+
+// Exec execs the command on the given entry.
+func Exec(ctx context.Context, e Execable, cmd string, args []string, opts ExecOptions) (ExecCommand, error) {
+	return e.Exec(ctx, cmd, args, opts)
+}
+
+// Stream streams the entry's content for updates.
+func Stream(ctx context.Context, s Streamable) (io.ReadCloser, error) {
+	return s.Stream(ctx)
+}
diff --git a/volume/fs.go b/volume/fs.go
index e561a1e..bfd9cd7 100644
--- a/volume/fs.go
+++ b/volume/fs.go
@@ -64,7 +64,7 @@ func exec(ctx context.Context, executor plugin.Execable, cmdline []string) (*byt
 	// it interactively, see character device vs named pipe on /dev/stderr as an example. This also
 	// ensures we cleanup correctly when the context is cancelled.
 	opts := plugin.ExecOptions{Elevate: true, Tty: plugin.IsInteractive()}
-	cmd, err := executor.Exec(ctx, cmdline[0], cmdline[1:], opts)
+	cmd, err := plugin.Exec(ctx, executor, cmdline[0], cmdline[1:], opts)
 	if err != nil {
 		return nil, err
 	}
@@ -128,7 +128,7 @@ func (d *FS) VolumeOpen(ctx context.Context, path string) (plugin.SizedReader, e
 func (d *FS) VolumeStream(ctx context.Context, path string) (io.ReadCloser, error) {
 	activity.Record(ctx, "Streaming %v on %v", path, plugin.ID(d.executor))
 	execOpts := plugin.ExecOptions{Elevate: true, Tty: true}
-	cmd, err := d.executor.Exec(ctx, "tail", []string{"-f", path}, execOpts)
+	cmd, err := plugin.Exec(ctx, d.executor, "tail", []string{"-f", path}, execOpts)
 	if err != nil {
 		activity.Record(ctx, "Exec error in VolumeRead: %v", err)
 		return nil, err


Test patch:
diff --git a/api/cache_test.go b/api/cache_test.go
index befe74b..658a4fd 100644
--- a/api/cache_test.go
+++ b/api/cache_test.go
@@ -87,7 +87,7 @@ func (suite *CacheHandlerTestSuite) TestRejectsGet() {
 }
 
 func (suite *CacheHandlerTestSuite) TestClearCache() {
-	// Populate the cache with a mocked resource and plugin.Cached*
+	// Populate the cache with a mocked resource and plugin.cached*
 	parent := newMockedParent()
 	parent.SetTestID("/dir")
 	parent.On("List", mock.Anything).Return([]plugin.Entry{}, nil)
@@ -95,7 +95,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
 	reqCtx := context.WithValue(context.Background(), mountpointKey, "/")
 
 	expectedChildren := make(map[string]plugin.Entry)
-	if children, err := plugin.CachedList(reqCtx, parent); suite.Nil(err) {
+	if children, err := plugin.List(reqCtx, parent); suite.Nil(err) {
 		suite.Equal(expectedChildren, children)
 	}
 
@@ -106,7 +106,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
 	suite.Equal(http.StatusOK, w.Code)
 	suite.Equal("[]\n", w.Body.String())
 
-	if children, err := plugin.CachedList(context.Background(), parent); suite.Nil(err) {
+	if children, err := plugin.List(context.Background(), parent); suite.Nil(err) {
 		suite.Equal(expectedChildren, children)
 	}
 
@@ -117,7 +117,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
 	suite.Equal(http.StatusOK, w.Code)
 	suite.Equal(`["List::/dir"]`, strings.TrimSpace(w.Body.String()))
 
-	if children, err := plugin.CachedList(context.Background(), parent); suite.Nil(err) {
+	if children, err := plugin.List(context.Background(), parent); suite.Nil(err) {
 		suite.Equal(expectedChildren, children)
 	}
 
diff --git a/plugin/cache_test.go b/plugin/cache_test.go
index 8e01d42..7c805cd 100644
--- a/plugin/cache_test.go
+++ b/plugin/cache_test.go
@@ -276,7 +276,7 @@ func (suite *CacheTestSuite) TestCachedListDefaultOp() {
 	mockChildren := []Entry{newCacheTestsMockEntry("mockChild")}
 	mungedOpValue := toMap(mockChildren)
 	suite.testCachedDefaultOp(ListOp, "List", mockChildren, mungedOpValue, func(ctx context.Context, e Entry) (interface{}, error) {
-		return CachedList(ctx, e.(Parent))
+		return cachedList(ctx, e.(Parent))
 	})
 }
 
@@ -293,7 +293,7 @@ func (suite *CacheTestSuite) TestCachedListCNameErrors() {
 	child3 := newCacheTestsMockEntry("baz")
 	mockChildren := []Entry{child1, child2, child3}
 	entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
-	_, err := CachedList(ctx, entry)
+	_, err := cachedList(ctx, entry)
 	if suite.Error(err) {
 		expectedErr := DuplicateCNameErr{
 			ParentID:                 "/my_plugin/foo",
@@ -321,7 +321,7 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
 	entry.SetTestID("/")
 	entry.DisableDefaultCaching()
 	entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
-	children, err := CachedList(ctx, entry)
+	children, err := cachedList(ctx, entry)
 	if suite.NoError(err) {
 		if suite.Equal(toMap(mockChildren), children) {
 			suite.Equal("/foo#child1", children["foo#child1"].id())
@@ -334,7 +334,7 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
 	entry.SetTestID("/parent")
 	entry.DisableDefaultCaching()
 	entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
-	children, err = CachedList(ctx, entry)
+	children, err = cachedList(ctx, entry)
 	if suite.NoError(err) {
 		if suite.Equal(toMap(mockChildren), children) {
 			suite.Equal("/parent/foo#child1", children["foo#child1"].id())
@@ -346,14 +346,14 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
 func (suite *CacheTestSuite) TestCachedOpen() {
 	mockReader := strings.NewReader("foo")
 	suite.testCachedDefaultOp(OpenOp, "Open", mockReader, mockReader, func(ctx context.Context, e Entry) (interface{}, error) {
-		return CachedOpen(ctx, e.(Readable))
+		return cachedOpen(ctx, e.(Readable))
 	})
 }
 
 func (suite *CacheTestSuite) TestCachedMetadata() {
 	mockJSONObject := JSONObject{"foo": "bar"}
 	suite.testCachedDefaultOp(MetadataOp, "Metadata", mockJSONObject, mockJSONObject, func(ctx context.Context, e Entry) (interface{}, error) {
-		return CachedMetadata(ctx, e)
+		return cachedMetadata(ctx, e)
 	})
 }
 
diff --git a/plugin/helpers_test.go b/plugin/helpers_test.go
deleted file mode 100644
index 9b39fb4..0000000
--- a/plugin/helpers_test.go
+++ /dev/null
@@ -1,85 +0,0 @@
-package plugin
-
-import (
-	"testing"
-	"time"
-
-	"github.com/puppetlabs/wash/datastore"
-	"github.com/stretchr/testify/mock"
-	"github.com/stretchr/testify/suite"
-)
-
-type HelpersTestSuite struct {
-	suite.Suite
-}
-
-func (suite *HelpersTestSuite) SetupSuite() {
-	SetTestCache(datastore.NewMemCache())
-}
-
-func (suite *HelpersTestSuite) TearDownSuite() {
-	UnsetTestCache()
-}
-
-func (suite *HelpersTestSuite) TestName() {
-	e := newHelpersTestsMockEntry("foo")
-	suite.Equal(Name(e), "foo")
-}
-
-func (suite *HelpersTestSuite) TestCName() {
-	e := newHelpersTestsMockEntry("foo/bar/baz")
-	suite.Equal("foo#bar#baz", CName(e))
-
-	e.SetSlashReplacer(':')
-	suite.Equal("foo:bar:baz", CName(e))
-}
-
-func (suite *HelpersTestSuite) TestID() {
-	e := newHelpersTestsMockEntry("foo/bar")
-
-	e.SetTestID("")
-	suite.Panics(
-		func() { ID(e) },
-		"plugin.ID: entry foo (cname foo#bar) has no ID",
-	)
-
-	e.SetTestID("/foo/bar")
-	suite.Equal(ID(e), "/foo/bar")
-}
-
-type helpersTestsMockEntry struct {
-	EntryBase
-	mock.Mock
-}
-
-func (e *helpersTestsMockEntry) Schema() *EntrySchema {
-	return nil
-}
-
-func newHelpersTestsMockEntry(name string) *helpersTestsMockEntry {
-	e := &helpersTestsMockEntry{
-		EntryBase: NewEntry(name),
-	}
-	e.SetTestID("id")
-	e.DisableDefaultCaching()
-
-	return e
-}
-
-func (suite *HelpersTestSuite) TestAttributes() {
-	e := newHelpersTestsMockEntry("mockEntry")
-	e.attr = EntryAttributes{}
-	e.attr.SetCtime(time.Now())
-	suite.Equal(e.attr, Attributes(e))
-}
-
-func (suite *HelpersTestSuite) TestPrefetched() {
-	e := newHelpersTestsMockEntry("mockEntry")
-	suite.False(IsPrefetched(e))
-	e.Prefetched()
-	suite.True(IsPrefetched(e))
-}
-
-func TestHelpers(t *testing.T) {
-	suite.Run(t, new(HelpersTestSuite))
-}
diff --git a/plugin/methodWrappers_test.go b/plugin/methodWrappers_test.go
new file mode 100644
index 0000000..a94a325
--- /dev/null
+++ b/plugin/methodWrappers_test.go
@@ -0,0 +1,85 @@
+package plugin
+
+import (
+	"testing"
+	"time"
+
+	"github.com/puppetlabs/wash/datastore"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/suite"
+)
+
+type MethodWrappersTestSuite struct {
+	suite.Suite
+}
+
+func (suite *MethodWrappersTestSuite) SetupSuite() {
+	SetTestCache(datastore.NewMemCache())
+}
+
+func (suite *MethodWrappersTestSuite) TearDownSuite() {
+	UnsetTestCache()
+}
+
+func (suite *MethodWrappersTestSuite) TestName() {
+	e := newmethodWrappersTestsMockEntry("foo")
+	suite.Equal(Name(e), "foo")
+}
+
+func (suite *MethodWrappersTestSuite) TestCName() {
+	e := newmethodWrappersTestsMockEntry("foo/bar/baz")
+	suite.Equal("foo#bar#baz", CName(e))
+
+	e.SetSlashReplacer(':')
+	suite.Equal("foo:bar:baz", CName(e))
+}
+
+func (suite *MethodWrappersTestSuite) TestID() {
+	e := newmethodWrappersTestsMockEntry("foo/bar")
+
+	e.SetTestID("")
+	suite.Panics(
+		func() { ID(e) },
+		"plugin.ID: entry foo (cname foo#bar) has no ID",
+	)
+
+	e.SetTestID("/foo/bar")
+	suite.Equal(ID(e), "/foo/bar")
+}
+
+type methodWrappersTestsMockEntry struct {
+	EntryBase
+	mock.Mock
+}
+
+func (e *methodWrappersTestsMockEntry) Schema() *EntrySchema {
+	return nil
+}
+
+func newmethodWrappersTestsMockEntry(name string) *methodWrappersTestsMockEntry {
+	e := &methodWrappersTestsMockEntry{
+		EntryBase: NewEntry(name),
+	}
+	e.SetTestID("id")
+	e.DisableDefaultCaching()
+
+	return e
+}
+
+func (suite *MethodWrappersTestSuite) TestAttributes() {
+	e := newmethodWrappersTestsMockEntry("mockEntry")
+	e.attr = EntryAttributes{}
+	e.attr.SetCtime(time.Now())
+	suite.Equal(e.attr, Attributes(e))
+}
+
+func (suite *MethodWrappersTestSuite) TestPrefetched() {
+	e := newmethodWrappersTestsMockEntry("mockEntry")
+	suite.False(IsPrefetched(e))
+	e.Prefetched()
+	suite.True(IsPrefetched(e))
+}
+
+func TestMethodWrappers(t *testing.T) {
+	suite.Run(t, new(MethodWrappersTestSuite))
+}
diff --git a/volume/file_test.go b/volume/file_test.go
index 55b43cb..bfa03b6 100644
--- a/volume/file_test.go
+++ b/volume/file_test.go
@@ -64,7 +64,7 @@ func TestVolumeFile(t *testing.T) {
 		assert.Equal(t, "hello", string(buf))
 	}
 
-	rdr2, err := vf.Stream(context.Background())
+	rdr2, err := plugin.Stream(context.Background(), vf)
 	assert.Nil(t, err)
 	if assert.NotNil(t, rdr2) {
 		buf, err := ioutil.ReadAll(rdr2)
@@ -82,7 +82,7 @@ func TestVolumeFileErr(t *testing.T) {
 	assert.Nil(t, rdr)
 	assert.Equal(t, errors.New("fail"), err)
 
-	rdr2, err := vf.Stream(context.Background())
+	rdr2, err := plugin.Stream(context.Background(), vf)
 	assert.Nil(t, rdr2)
 	assert.Equal(t, errors.New("fail"), err)
 }
diff --git a/volume/fs_test.go b/volume/fs_test.go
index 69cbd0a..9f8426e 100644
--- a/volume/fs_test.go
+++ b/volume/fs_test.go
@@ -75,7 +75,7 @@ func (suite *fsTestSuite) find(parent plugin.Parent, path string) plugin.Entry {
 	names := strings.Split(path, "/")
 	entry := plugin.Entry(parent)
 	for _, name := range names {
-		entries, err := plugin.CachedList(suite.ctx, entry.(plugin.Parent))
+		entries, err := plugin.List(suite.ctx, entry.(plugin.Parent))
 		if !suite.NoError(err) {
 			suite.FailNow("Listing entries failed")
 		}
@@ -154,18 +154,18 @@ func (suite *fsTestSuite) TestFSListTwice() {
 	fs.SetTestID("/instance/fs")
 
 	entry := suite.find(fs, "var/log").(plugin.Parent)
-	entries, err := plugin.CachedList(suite.ctx, entry)
+	entries, err := plugin.List(suite.ctx, entry)
 	if !suite.NoError(err) {
 		suite.FailNow("Listing entries failed")
 	}
 	suite.Equal(1, len(entries))
 	suite.Contains(entries, "path")
 
-	entries1, err := plugin.CachedList(suite.ctx, entries["path"].(plugin.Parent))
+	entries1, err := plugin.List(suite.ctx, entries["path"].(plugin.Parent))
 	if suite.NoError(err) {
 		suite.Equal(1, len(entries1))
 		suite.Contains(entries1, "has")
-		entries2, err := plugin.CachedList(suite.ctx, entries1["has"].(plugin.Parent))
+		entries2, err := plugin.List(suite.ctx, entries1["has"].(plugin.Parent))
 		if suite.NoError(err) {
 			suite.Equal(1, len(entries2))
 			suite.Contains(entries2, "got")
@@ -190,7 +190,7 @@ func (suite *fsTestSuite) TestFSListExpiredCache() {
 	fs.SetTestID("/instance/fs")
 
 	entry := suite.find(fs, "var/log").(plugin.Parent)
-	entries, err := plugin.CachedList(suite.ctx, entry)
+	entries, err := plugin.List(suite.ctx, entry)
 	if !suite.NoError(err) {
 		suite.FailNow("Listing entries failed")
 	}
@@ -199,7 +199,7 @@ func (suite *fsTestSuite) TestFSListExpiredCache() {
 
 	_, err = plugin.ClearCacheFor("/instance/fs")
 	suite.NoError(err)
-	entries, err = plugin.CachedList(suite.ctx, entry)
+	entries, err = plugin.List(suite.ctx, entry)
 	if suite.NoError(err) {
 		suite.Equal(1, len(entries))
 	}