Move the external plugin implementation to its own package

Separates the external plugin implementation into a separate package from `plugin` so that it can make use of other packages in Wash that themselves depend on `plugin` (like `volume` and `transport`). The separation is possible by making `MethodSignature` a public type, and adding a hidden `externalPlugin` interface for "dynamic" plugin functionality ( where an entry's features may not be known at compile-time). Signed-off-by: Michael Smith <[email protected]>
MikaelSmith · Jan 16, 2020 · a19ce1b · a19ce1b
1 parent 18e95f8
commit a19ce1b
Show file tree

Hide file tree

Showing 24 changed files with 457 additions and 369 deletions.
diff --git a/cmd/server.go b/cmd/server.go
@@ -15,6 +15,7 @@ import (
 	"github.com/puppetlabs/wash/cmd/internal/server"
 	cmdutil "github.com/puppetlabs/wash/cmd/util"
 	"github.com/puppetlabs/wash/plugin"
+	"github.com/puppetlabs/wash/plugin/external"
 	"gopkg.in/yaml.v2"
 
 	log "github.com/sirupsen/logrus"
@@ -147,7 +148,7 @@ func serverOptsFor(cmd *cobra.Command) (map[string]plugin.Root, server.Opts, err
 
 	// Check the external plugins. First unmarshal their spec, ensure that
 	// they're valid scripts, then convert them to plugin.Root types.
-	var externalPlugins []plugin.ExternalPluginSpec
+	var externalPlugins []external.PluginSpec
 	if err := viper.UnmarshalKey("external-plugins", &externalPlugins); err != nil {
 		return nil, server.Opts{}, fmt.Errorf("failed to unmarshal the external-plugins key: %v", err)
 	}

diff --git a/cmd/validate.go b/cmd/validate.go
@@ -15,6 +15,7 @@ import (
 	"github.com/jedib0t/go-pretty/progress"
 	cmdutil "github.com/puppetlabs/wash/cmd/util"
 	"github.com/puppetlabs/wash/plugin"
+	"github.com/puppetlabs/wash/plugin/external"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -77,7 +78,7 @@ func validateMain(cmd *cobra.Command, args []string) exitCode {
 	root, ok := plugins[plug]
 	if !ok {
 		// See if it's a script we can run as an external plugin instead
-		spec := plugin.ExternalPluginSpec{Script: plug}
+		spec := external.PluginSpec{Script: plug}
 		root, err = spec.Load()
 		if err != nil {
 			pluginNames := make([]string, 0, len(plugins))

diff --git a/plugin/action.go b/plugin/action.go
@@ -1,23 +1,25 @@
 package plugin
 
-type methodSignature int
+// MethodSignature defines what method signature is supported for a method.
+type MethodSignature int
 
+// Defines different method signatures.
 const (
-	unsupportedSignature methodSignature = iota
-	defaultSignature
-	blockReadableSignature
+	UnsupportedSignature MethodSignature = iota
+	DefaultSignature
+	BlockReadableSignature
 )
 
 // Action represents a Wash action.
 type Action struct {
 	Name                         string `json:"name"`
 	Protocol                     string `json:"protocol"`
-	corePluginEntrySignatureFunc func(e Entry) methodSignature
+	corePluginEntrySignatureFunc func(e Entry) MethodSignature
 }
 
 var actions = make(map[string]Action)
 
-func newAction(name string, protocol string, corePluginEntrySignatureFunc func(e Entry) methodSignature) Action {
+func newAction(name string, protocol string, corePluginEntrySignatureFunc func(e Entry) MethodSignature) Action {
 	a := Action{
 		Name:                         name,
 		Protocol:                     protocol,
@@ -30,68 +32,68 @@ func newAction(name string, protocol string, corePluginEntrySignatureFunc func(e
 // IsSupportedOn returns true if the action's supported
 // on the specified entry, false otherwise.
 func (a Action) IsSupportedOn(entry Entry) bool {
-	return a.signature(entry) != unsupportedSignature
+	return a.signature(entry) != UnsupportedSignature
 }
 
-func (a Action) signature(entry Entry) methodSignature {
+func (a Action) signature(entry Entry) MethodSignature {
 	switch t := entry.(type) {
 	case externalPlugin:
-		return t.supportedMethods()[a.Name].signature
+		return t.MethodSignature(a.Name)
 	default:
 		return a.corePluginEntrySignatureFunc(entry)
 	}
 }
 
-var listAction = newAction("list", "Parent", func(e Entry) methodSignature {
+var listAction = newAction("list", "Parent", func(e Entry) MethodSignature {
 	if _, ok := e.(Parent); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
-var readAction = newAction("read", "Readable", func(e Entry) methodSignature {
+var readAction = newAction("read", "Readable", func(e Entry) MethodSignature {
 	if _, ok := e.(Readable); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
 	if _, ok := e.(BlockReadable); ok {
-		return blockReadableSignature
+		return BlockReadableSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
-var streamAction = newAction("stream", "Streamable", func(e Entry) methodSignature {
+var streamAction = newAction("stream", "Streamable", func(e Entry) MethodSignature {
 	if _, ok := e.(Streamable); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
-var writeAction = newAction("write", "Writable", func(e Entry) methodSignature {
+var writeAction = newAction("write", "Writable", func(e Entry) MethodSignature {
 	if _, ok := e.(Writable); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
-var execAction = newAction("exec", "Execable", func(e Entry) methodSignature {
+var execAction = newAction("exec", "Execable", func(e Entry) MethodSignature {
 	if _, ok := e.(Execable); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
-var deleteAction = newAction("delete", "Deletable", func(e Entry) methodSignature {
+var deleteAction = newAction("delete", "Deletable", func(e Entry) MethodSignature {
 	if _, ok := e.(Deletable); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
-var signalAction = newAction("signal", "Signalable", func(e Entry) methodSignature {
+var signalAction = newAction("signal", "Signalable", func(e Entry) MethodSignature {
 	if _, ok := e.(Signalable); ok {
-		return defaultSignature
+		return DefaultSignature
 	}
-	return unsupportedSignature
+	return UnsupportedSignature
 })
 
 // ListAction represents the list action
@@ -153,15 +155,15 @@ func SupportedActionsOf(entry Entry) []string {
 	switch t := entry.(type) {
 	case externalPlugin:
 		for _, action := range actions {
-			signature := t.supportedMethods()[action.Name].signature
-			if signature != unsupportedSignature {
+			signature := t.MethodSignature(action.Name)
+			if signature != UnsupportedSignature {
 				supportedActions = append(supportedActions, action.Name)
 			}
 		}
 	default:
 		for _, action := range actions {
 			signature := action.corePluginEntrySignatureFunc(entry)
-			if signature != unsupportedSignature {
+			if signature != UnsupportedSignature {
 				supportedActions = append(supportedActions, action.Name)
 			}
 		}

diff --git a/plugin/cache.go b/plugin/cache.go
@@ -229,7 +229,7 @@ func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {
 func cachedRead(ctx context.Context, e Entry) (entryContent, error) {
 	cachedContent, err := cachedDefaultOp(ctx, ReadOp, e, func() (interface{}, error) {
 		switch signature := ReadAction().signature(e); signature {
-		case defaultSignature:
+		case DefaultSignature:
 			// Both external and core plugin entries that have the default Read signature
 			// implement the Readable interface, so we can go ahead and cast directly.
 			r := e.(Readable)
@@ -238,12 +238,12 @@ func cachedRead(ctx context.Context, e Entry) (entryContent, error) {
 				return nil, err
 			}
 			return newEntryContent(rawContent), nil
-		case blockReadableSignature:
+		case BlockReadableSignature:
 			var readFunc blockReadFunc
 			switch t := e.(type) {
 			case externalPlugin:
 				readFunc = func(ctx context.Context, size int64, offset int64) ([]byte, error) {
-					return t.blockRead(ctx, size, offset)
+					return t.BlockRead(ctx, size, offset)
 				}
 			case BlockReadable:
 				readFunc = func(ctx context.Context, size int64, offset int64) ([]byte, error) {

diff --git a/plugin/cache_test.go b/plugin/cache_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/emirpasic/gods/maps/linkedhashmap"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
 )
@@ -401,35 +402,63 @@ func (suite *CacheTestSuite) TestCachedRead_ReadableCorePluginEntry_ErroredRead(
 	suite.Equal(expectedErr, err)
 }
 
+type mockExternalPlugin struct {
+	EntryBase
+	mock.Mock
+}
+
+func (e *mockExternalPlugin) Schema() *EntrySchema {
+	return e.Called().Get(0).(*EntrySchema)
+}
+
+func (e *mockExternalPlugin) Read(ctx context.Context) ([]byte, error) {
+	args := e.Called(ctx)
+	return args.Get(0).([]byte), args.Error(1)
+}
+
+func (e *mockExternalPlugin) MethodSignature(name string) MethodSignature {
+	return e.Called(name).Get(0).(MethodSignature)
+}
+
+func (e *mockExternalPlugin) SchemaGraph() (*linkedhashmap.Map, error) {
+	args := e.Called()
+	return args.Get(0).(*linkedhashmap.Map), args.Error(1)
+}
+
+func (e *mockExternalPlugin) RawTypeID() string {
+	return e.Called().String(0)
+}
+
+func (e *mockExternalPlugin) BlockRead(ctx context.Context, size int64, offset int64) ([]byte, error) {
+	args := e.Called(ctx, size, offset)
+	return args.Get(0).([]byte), args.Error(1)
+}
+
 func (suite *CacheTestSuite) TestCachedRead_ReadableExternalPluginEntry_SuccessfulRead() {
-	entry := &externalPluginEntry{
-		EntryBase: NewEntry("foo"),
-		methods: map[string]methodInfo{
-			"read": methodInfo{signature: defaultSignature, result: "some raw content"},
-		},
-	}
+	entry := &mockExternalPlugin{EntryBase: NewEntry("foo")}
+	ctx := context.Background()
+	entry.On("MethodSignature", "read").Return(DefaultSignature)
+	entry.On("Read", ctx).Return([]byte("some raw content"), nil)
 	entry.DisableDefaultCaching()
 	entry.SetTestID("/foo")
 
 	expectedContent := newEntryContent([]byte("some raw content"))
 
-	actualContent, err := cachedRead(context.Background(), entry)
+	actualContent, err := cachedRead(ctx, entry)
 	if suite.NoError(err) {
 		suite.Equal(expectedContent, actualContent)
 	}
 }
 
 func (suite *CacheTestSuite) TestCachedRead_ReadableExternalPluginEntry_ErroredRead() {
-	entry := &externalPluginEntry{
-		EntryBase: NewEntry("foo"),
-		methods: map[string]methodInfo{
-			"read": methodInfo{signature: defaultSignature, result: []byte("invalid content")},
-		},
-	}
+	entry := &mockExternalPlugin{EntryBase: NewEntry("foo")}
+	ctx := context.Background()
+	entry.On("MethodSignature", "read").Return(DefaultSignature)
+	entry.On("Read", ctx).Return([]byte{}, fmt.Errorf("Read failed, not a string"))
 	entry.DisableDefaultCaching()
 	entry.SetTestID("/foo")
 
-	_, err := cachedRead(context.Background(), entry)
+	_, err := cachedRead(ctx, entry)
 	suite.Regexp("Read.*string", err.Error())
 }
 
@@ -468,31 +497,20 @@ func (suite *CacheTestSuite) TestCachedRead_BlockReadableCorePluginEntry() {
 
 func (suite *CacheTestSuite) TestCachedRead_BlockReadableExternalPluginEntry() {
 	expectedRawContent := []byte("some raw content")
-
-	mockScript := &mockExternalPluginScript{path: "plugin_script"}
-	entry := &externalPluginEntry{
-		EntryBase: NewEntry("foo"),
-		script:    mockScript,
-		methods: map[string]methodInfo{
-			"read": methodInfo{
-				signature: blockReadableSignature,
-			},
-		},
-	}
+	ctx := context.Background()
+	entry := &mockExternalPlugin{EntryBase: NewEntry("foo")}
+	entry.On("MethodSignature", "read").Return(BlockReadableSignature)
+	entry.On("BlockRead", ctx, int64(10), int64(0)).Return(expectedRawContent, nil)
 	entry.DisableDefaultCaching()
 	entry.SetTestID("/foo")
 	entry.Attributes().SetSize(uint64(len(expectedRawContent)))
 
-	ctx := context.Background()
-	mockScript.OnInvokeAndWait(ctx, "read", entry, "10", "0").Return(mockInvocation(expectedRawContent), nil).Once()
-
 	content, err := cachedRead(ctx, entry)
 	suite.Equal(entry.Attributes().Size(), content.size())
 	if suite.NoError(err) {
 		actualRawContent, err := content.read(ctx, 10, 0)
 		if suite.NoError(err) {
 			suite.Equal(expectedRawContent, actualRawContent)
-			mockScript.AssertExpectations(suite.T())
 		}
 	}
 }

diff --git a/plugin/entryBase.go b/plugin/entryBase.go
@@ -99,6 +99,12 @@ func (e *EntryBase) Name() string {
 	return e.name
 }
 
+// ID returns the entry's ID. It won't panic on an empty string. See ID() for more detail.
+// This exists primarily to support the `external` package.
+func (e *EntryBase) ID() string {
+	return e.id
+}
+
 // String returns a unique identifier for the entry suitable for logging and error messages.
 func (e *EntryBase) String() string {
 	return e.id
@@ -140,6 +146,11 @@ func (e *EntryBase) MarkInaccessible(ctx context.Context, err error) {
 	e.isInaccessible = true
 }
 
+// IsInaccessible returns whether the entry is inaccessible.
+func (e *EntryBase) IsInaccessible() bool {
+	return e.isInaccessible
+}
+
 // Prefetched marks the entry as a prefetched entry. A prefetched entry
 // is an entry that was fetched as part of a batch operation that
 // fetched multiple levels of hierarchy at once. Volume directories and
@@ -170,6 +181,11 @@ func (e *EntryBase) SetTTLOf(op defaultOpCode, ttl time.Duration) *EntryBase {
 	return e
 }
 
+// TTLOf returns the TTL set for the specified op
+func (e *EntryBase) TTLOf(op defaultOpCode) time.Duration {
+	return e.ttl[op]
+}
+
 // DisableCachingFor disables caching for the specified op
 func (e *EntryBase) DisableCachingFor(op defaultOpCode) *EntryBase {
 	e.SetTTLOf(op, -1)