Repository: puppetlabs/wash
Pull request #641 (2019-12-05 20:22:57)

Support block-readable external plugin entries
Supports https://github.com/puppetlabs/wash/issues/619

Signed-off-by: Enis Inan <enis.inan@puppet.com>


Patch:
diff --git a/plugin/cache.go b/plugin/cache.go
index de33bc6..4985053 100644
--- a/plugin/cache.go
+++ b/plugin/cache.go
@@ -222,14 +222,12 @@ func cachedRead(ctx context.Context, e Entry) (entryContent, error) {
 			}
 			return newEntryContent(rawContent), nil
 		case blockReadableSignature:
-			// Go doesn't allow overloaded functions, so the external plugin entry type
-			// cannot implement both BlockReadable#Read and Readable#Read. Thus, external
-			// plugins implement the BlockReadable interface via a separate blockRead
-			// method.
 			var readFunc blockReadFunc
 			switch t := e.(type) {
 			case externalPlugin:
-				// TODO: external plugin implementation here!
+				readFunc = func(ctx context.Context, size int64, offset int64) ([]byte, error) {
+					return t.blockRead(ctx, size, offset)
+				}
 			case BlockReadable:
 				readFunc = func(ctx context.Context, size int64, offset int64) ([]byte, error) {
 					return t.Read(ctx, size, offset)
diff --git a/plugin/externalPluginEntry.go b/plugin/externalPluginEntry.go
index a72b118..0f967dd 100644
--- a/plugin/externalPluginEntry.go
+++ b/plugin/externalPluginEntry.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"io/ioutil"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -31,6 +32,11 @@ type externalPlugin interface {
 	// interface.
 	schema() (*EntrySchema, error)
 	RawTypeID() string
+	// Go doesn't allow overloaded functions, so the external plugin entry type
+	// cannot implement both BlockReadable#Read and Readable#Read. Thus, external
+	// plugins implement the BlockReadable interface via a separate blockRead
+	// method.
+	blockRead(ctx context.Context, size int64, offset int64) ([]byte, error)
 }
 
 type decodedCacheTTLs struct {
@@ -51,7 +57,7 @@ type decodedExternalPluginEntry struct {
 	State              string           `json:"state"`
 }
 
-const entryMethodTypeError = "each method must be a string or tuple [<method>, <result>], not %v"
+const entryMethodTypeError = "each method must be a string or tuple [<method>, <result_or_signature>], not %v"
 
 func mungeToMethods(input []interface{}) (map[string]methodInfo, error) {
 	methods := make(map[string]methodInfo)
@@ -69,10 +75,24 @@ func mungeToMethods(input []interface{}) (map[string]methodInfo, error) {
 			if !ok {
 				return nil, fmt.Errorf(entryMethodTypeError, data)
 			}
-			methods[name] = methodInfo{
+			info := methodInfo{
 				signature: defaultSignature,
-				result:    data[1],
 			}
+			switch name {
+			default:
+				info.result = data[1]
+			case "read":
+				// Check if we have ["read", <block_readable?>] or ["read", <result>].
+				// The latter implies <block_readable> == false.
+				if block_readable, ok := data[1].(bool); ok {
+					if block_readable {
+						info.signature = blockReadableSignature
+					}
+				} else {
+					info.result = data[1]
+				}
+			}
+			methods[name] = info
 		default:
 			return nil, fmt.Errorf(entryMethodTypeError, data)
 		}
@@ -362,7 +382,13 @@ func (e *externalPluginEntry) Read(ctx context.Context) ([]byte, error) {
 	return inv.stdout.Bytes(), nil
 }
 
-// TODO: Implement blockRead
+func (e *externalPluginEntry) blockRead(ctx context.Context, size int64, offset int64) ([]byte, error) {
+	inv, err := e.script.InvokeAndWait(ctx, "read", e, strconv.FormatInt(size, 10), strconv.FormatInt(offset, 10))
+	if err != nil {
+		return nil, err
+	}
+	return inv.stdout.Bytes(), nil
+}
 
 func (e *externalPluginEntry) Metadata(ctx context.Context) (JSONObject, error) {
 	if !e.implements("metadata") {


Test patch:
diff --git a/plugin/cache_test.go b/plugin/cache_test.go
index a9e0066..55ce92e 100644
--- a/plugin/cache_test.go
+++ b/plugin/cache_test.go
@@ -460,7 +460,36 @@ func (suite *CacheTestSuite) TestCachedRead_BlockReadableCorePluginEntry() {
 	}
 }
 
-// TODO: Add block readable external plugin entry test case
+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,
+			},
+		},
+	}
+	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())
+		}
+	}
+}
 
 func (suite *CacheTestSuite) TestCachedMetadata() {
 	mockJSONObject := JSONObject{"foo": "bar"}
diff --git a/plugin/externalPluginEntry_test.go b/plugin/externalPluginEntry_test.go
index 035c503..40a19ca 100644
--- a/plugin/externalPluginEntry_test.go
+++ b/plugin/externalPluginEntry_test.go
@@ -94,8 +94,10 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryExtraFie
 	if suite.NoError(err) {
 		suite.Equal(decodedEntry.Name, entry.name())
 		suite.Contains(entry.methods, "list")
+		suite.Equal(defaultSignature, entry.methods["list"].signature)
 		suite.Nil(entry.methods["list"].result)
 		suite.Contains(entry.methods, "stream")
+		suite.Equal(defaultSignature, entry.methods["stream"].signature)
 		suite.Nil(entry.methods["stream"].result)
 		suite.False(entry.isPrefetched())
 
@@ -129,6 +131,7 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithMeth
 	if suite.NoError(err) {
 		suite.Equal(decodedEntry.Name, entry.name())
 		suite.Contains(entry.methods, "list")
+		suite.Equal(defaultSignature, entry.methods["list"].signature)
 		suite.NotNil(entry.methods["list"].result)
 		suite.Contains(entry.methods, "read")
 		suite.Nil(entry.methods["read"].result)
@@ -136,6 +139,33 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithMeth
 	}
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryMethodTuple_Read() {
+	decodedEntry := decodedExternalPluginEntry{
+		Name:    "decodedEntry",
+		Methods: []interface{}{[]interface{}{"read", true}},
+	}
+
+	type testCase struct {
+		data              interface{}
+		expectedSignature methodSignature
+		expectedResult    interface{}
+	}
+	testCases := []testCase{
+		testCase{true, blockReadableSignature, nil},
+		testCase{false, defaultSignature, nil},
+		testCase{"foo", defaultSignature, "foo"},
+	}
+	for _, testCase := range testCases {
+		decodedEntry.Methods = []interface{}{[]interface{}{"read", testCase.data}}
+		entry, err := decodedEntry.toExternalPluginEntry(context.Background(), false, false)
+		if suite.NoError(err) {
+			suite.NotNil(entry.methods["read"])
+			suite.Equal(testCase.expectedSignature, entry.methods["read"].signature)
+			suite.Equal(testCase.expectedResult, entry.methods["read"].result)
+		}
+	}
+}
+
 func newMockDecodedEntry(name string) decodedExternalPluginEntry {
 	return decodedExternalPluginEntry{
 		Name:    name,
@@ -540,13 +570,13 @@ func (suite *ExternalPluginEntryTestSuite) TestRead() {
 		mockScript.OnInvokeAndWait(ctx, "read", entry).Return(mockInvocation(stdout), err).Once()
 	}
 
-	// Test that if InvokeAndWait errors, then Open returns its error
+	// Test that if InvokeAndWait errors, then Read returns its error
 	mockErr := fmt.Errorf("execution error")
 	mockInvokeAndWait([]byte{}, mockErr)
 	_, err := entry.Read(ctx)
 	suite.EqualError(mockErr, err.Error())
 
-	// Test that Open wraps all of stdout into a SizedReader
+	// Test that Read returns the invocation's stdout
 	stdout := "foo"
 	mockInvokeAndWait([]byte(stdout), nil)
 	content, err := entry.Read(ctx)
@@ -556,6 +586,35 @@ func (suite *ExternalPluginEntryTestSuite) TestRead() {
 	}
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestBlockRead() {
+	mockScript := &mockExternalPluginScript{path: "plugin_script"}
+	entry := &externalPluginEntry{
+		EntryBase: NewEntry("foo"),
+		script:    mockScript,
+	}
+	entry.SetTestID("/foo")
+
+	ctx := context.Background()
+	mockInvokeAndWait := func(stdout []byte, err error) {
+		mockScript.OnInvokeAndWait(ctx, "read", entry, "10", "0").Return(mockInvocation(stdout), err).Once()
+	}
+
+	// Test that if InvokeAndWait errors, then blockRead returns its error
+	mockErr := fmt.Errorf("execution error")
+	mockInvokeAndWait([]byte{}, mockErr)
+	_, err := entry.blockRead(ctx, 10, 0)
+	suite.EqualError(mockErr, err.Error())
+
+	// Test that blockRead returns the invocation's stdout
+	stdout := "foo"
+	mockInvokeAndWait([]byte(stdout), nil)
+	content, err := entry.blockRead(ctx, 10, 0)
+	if suite.NoError(err) {
+		expectedContent := []byte(stdout)
+		suite.Equal(expectedContent, content)
+	}
+}
+
 func (suite *ExternalPluginEntryTestSuite) TestListReadWithMethodResults() {
 	mockScript := &mockExternalPluginScript{path: "plugin_script"}
 	entry := &externalPluginEntry{