Repository: puppetlabs/wash
Pull request #386 (2019-07-12 22:41:04)

Don't require type ID + continue supporting empty methods array
Contributions to this project require sign-off consistent with the [Developers Certificate of Origin](https://developercertificate.org). This can be as simple as using `git commit -s` on each commit.


Patch:
diff --git a/plugin/externalPluginEntry.go b/plugin/externalPluginEntry.go
index ba54e48..1d2b187 100644
--- a/plugin/externalPluginEntry.go
+++ b/plugin/externalPluginEntry.go
@@ -77,12 +77,9 @@ func (e decodedExternalPluginEntry) toExternalPluginEntry(schemaKnown bool, isRo
 	if len(e.Name) <= 0 {
 		return nil, fmt.Errorf("the entry name must be provided")
 	}
-	if len(e.Methods) <= 0 {
+	if e.Methods == nil {
 		return nil, fmt.Errorf("the entry's methods must be provided")
 	}
-	if len(e.TypeID) <= 0 {
-		return nil, fmt.Errorf("the entry's type ID must be provided")
-	}
 
 	methods, err := mungeToMethods(e.Methods)
 	if err != nil {
@@ -96,6 +93,9 @@ func (e decodedExternalPluginEntry) toExternalPluginEntry(schemaKnown bool, isRo
 	// if the root implements it. It is also reasonable for us to expect (and require)
 	// every entry to _not_ implement schema if the root does not implement it.
 	if result, ok := methods["schema"]; ok {
+		if len(e.TypeID) == 0 {
+			return nil, fmt.Errorf("entry %v implements schema, but no type ID was provided", e.Name)
+		}
 		if !schemaKnown && !isRoot {
 			return nil, fmt.Errorf("entry %v (%v) implements schema, but the plugin root doesn't", e.Name, e.TypeID)
 		}
@@ -290,7 +290,7 @@ func (e *externalPluginEntry) schema() (*EntrySchema, error) {
 	return s, nil
 }
 
-const listFormat = "[{\"name\":\"entry1\",\"methods\":[\"list\"],\"type_id\":\"type1\"},{\"name\":\"entry2\",\"methods\":[\"list\"],\"type_id\":\"type2\"}]"
+const listFormat = "[{\"name\":\"entry1\",\"methods\":[\"list\"]},{\"name\":\"entry2\",\"methods\":[\"list\"]}]"
 
 func (e *externalPluginEntry) List(ctx context.Context) ([]Entry, error) {
 	var decodedEntries []decodedExternalPluginEntry
@@ -562,7 +562,7 @@ func (e *externalPluginEntry) unmarshalSchemaGraph(stdout []byte) (*linkedhashma
 		if len(node.Label) <= 0 {
 			return fmt.Errorf("a label must be provided")
 		}
-		if len(node.Methods) <= 0 {
+		if node.Methods == nil {
 			return fmt.Errorf("the entry's methods must be provided")
 		}
 		isParent := false


Test patch:
diff --git a/plugin/externalPluginEntry_test.go b/plugin/externalPluginEntry_test.go
index 4d1cc20..7375b0f 100644
--- a/plugin/externalPluginEntry_test.go
+++ b/plugin/externalPluginEntry_test.go
@@ -75,23 +75,17 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryRequired
 	suite.Regexp("methods", err)
 	decodedEntry.Methods = []interface{}{"list"}
 
-	_, err = decodedEntry.toExternalPluginEntry(false, false)
-	suite.Regexp("type.*ID", err)
-	decodedEntry.TypeID = "foo"
-
 	entry, err := decodedEntry.toExternalPluginEntry(false, false)
 	if suite.NoError(err) {
 		suite.Equal(decodedEntry.Name, entry.name())
 		suite.Equal(1, len(entry.methods))
 		suite.Contains(entry.methods, "list")
 		suite.Nil(entry.methods["list"])
-		suite.Equal("foo", entry.typeID)
 	}
 }
 
 func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryExtraFields() {
 	decodedEntry := decodedExternalPluginEntry{
-		TypeID:  "foo",
 		Name:    "decodedEntry",
 		Methods: []interface{}{"list", "stream"},
 	}
@@ -112,10 +106,21 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryExtraFie
 	}
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntry_SupportsEmptyMethodsArray() {
+	decodedEntry := decodedExternalPluginEntry{
+		Name:    "decodedEntry",
+		Methods: []interface{}{},
+	}
+
+	entry, err := decodedEntry.toExternalPluginEntry(false, false)
+	if suite.NoError(err) {
+		suite.Equal(decodedEntry.Name, entry.name())
+	}
+}
+
 func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithMethodResults() {
 	childEntry := map[string]interface{}{"name": "foo", "methods": []string{"read"}}
 	decodedEntry := decodedExternalPluginEntry{
-		TypeID:  "foo",
 		Name:    "decodedEntry",
 		Methods: []interface{}{[]interface{}{"list", []interface{}{childEntry}}, "read"},
 	}
@@ -133,7 +138,6 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithMeth
 
 func newMockDecodedEntry(name string) decodedExternalPluginEntry {
 	return decodedExternalPluginEntry{
-		TypeID:  "foo",
 		Name:    name,
 		Methods: []interface{}{"list"},
 	}
@@ -189,7 +193,6 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSche
 	decodedEntry := decodedExternalPluginEntry{
 		Name:    "decodedEntry",
 		Methods: []interface{}{"list"},
-		TypeID:  "foo",
 	}
 	entry, err := decodedEntry.toExternalPluginEntry(false, false)
 	if suite.NoError(err) {
@@ -197,6 +200,15 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSche
 	}
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSchema_SchemaUnknown_ImplementsSchema_TypeIDNotIncluded() {
+	decodedEntry := decodedExternalPluginEntry{
+		Name:    "decodedEntry",
+		Methods: []interface{}{"list", "schema"},
+	}
+	_, err := decodedEntry.toExternalPluginEntry(false, false)
+	suite.Regexp("decodedEntry.*implements.*schema.*no.*type.*ID", err)
+}
+
 func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSchema_SchemaUnknown_ImplementsSchema() {
 	decodedEntry := decodedExternalPluginEntry{
 		Name:    "decodedEntry",
@@ -217,6 +229,15 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSche
 	suite.Regexp("decodedEntry.*foo.*must.*implement.*schema", err)
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSchema_SchemaKnown_ImplementsSchema_TypeIDNotIncluded() {
+	decodedEntry := decodedExternalPluginEntry{
+		Name:    "decodedEntry",
+		Methods: []interface{}{"list", "schema"},
+	}
+	_, err := decodedEntry.toExternalPluginEntry(true, false)
+	suite.Regexp("decodedEntry.*implements.*schema.*no.*type.*ID", err)
+}
+
 func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryWithSchema_SchemaKnown_PrefetchesSchema() {
 	decodedEntry := decodedExternalPluginEntry{
 		Name: "decodedEntry",
@@ -464,7 +485,7 @@ func (suite *ExternalPluginEntryTestSuite) TestList() {
 
 	// Test that List properly decodes the entries from stdout
 	stdout := "[" +
-		"{\"name\":\"foo\",\"methods\":[\"list\"],\"type_id\":\"foo\"}" +
+		"{\"name\":\"foo\",\"methods\":[\"list\"]}" +
 		"]"
 	mockInvokeAndWait([]byte(stdout), nil)
 	entries, err := entry.List(ctx)
@@ -475,7 +496,6 @@ func (suite *ExternalPluginEntryTestSuite) TestList() {
 				EntryBase:    entryBase,
 				methods:      map[string]interface{}{"list": nil},
 				script:       entry.script,
-				typeID:       "foo",
 				schemaGraphs: entry.schemaGraphs,
 			},
 		}
@@ -529,9 +549,9 @@ func (suite *ExternalPluginEntryTestSuite) TestListOpenWithMethodResults() {
 	// Test that List is invoked when
 	stdoutFn := func(content string) []byte {
 		return []byte(`
-[{"name": "foo", "type_id": "bar", "methods": [
+[{"name": "foo", "methods": [
 	["list", [
-		{"name": "bar", "type_id": "baz", "methods": [["read", "` + content + `"]]}
+		{"name": "bar", "methods": [["read", "` + content + `"]]}
 	]]
 ]}]`)
 	}
@@ -576,7 +596,7 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeWithErrors() {
 	}
 
 	// Test that List is invoked when
-	stdout := `[{"name": "foo", "type_id": "bar", "methods": [
+	stdout := `[{"name": "foo", "methods": [
 								["list", {"name": "bar"}],
 								["read", [1, 2]]
 							]}]`
@@ -590,7 +610,7 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeWithErrors() {
 
 		_, err = entries[0].(Parent).List(ctx)
 		suite.EqualError(err, `implementation of list must conform to `+
-			`[{"name":"entry1","methods":["list"],"type_id":"type1"},{"name":"entry2","methods":["list"],"type_id":"type2"}], not map[name:bar]`)
+			`[{"name":"entry1","methods":["list"]},{"name":"entry2","methods":["list"]}], not map[name:bar]`)
 
 		_, err = entries[0].(Readable).Open(ctx)
 		suite.EqualError(err, "Read method must provide a string, not [1 2]")
diff --git a/plugin/externalPluginRoot_test.go b/plugin/externalPluginRoot_test.go
index 5590afd..c794f36 100644
--- a/plugin/externalPluginRoot_test.go
+++ b/plugin/externalPluginRoot_test.go
@@ -45,7 +45,7 @@ func (suite *ExternalPluginRootTestSuite) TestInit() {
 	suite.Regexp(regexp.MustCompile("stdout"), err)
 
 	// Test that Init properly decodes the root from stdout
-	stdout := "{\"type_id\":\"root\"}"
+	stdout := "{}"
 	mockInvokeAndWait([]byte(stdout), nil)
 	err = root.Init(nil)
 	if suite.NoError(err) {
@@ -54,7 +54,6 @@ func (suite *ExternalPluginRootTestSuite) TestInit() {
 				EntryBase: NewEntry("foo"),
 				methods:   map[string]interface{}{"list": nil},
 				script:    root.script,
-				typeID:    "root",
 			},
 		}
 
@@ -74,7 +73,7 @@ func (suite *ExternalPluginRootTestSuite) TestInitWithConfig() {
 		"init",
 		nil,
 		`{"key":["value"]}`,
-	).Return(mockInvocation([]byte("{\"type_id\":\"root\"}")), nil).Once()
+	).Return(mockInvocation([]byte("{}")), nil).Once()
 
 	suite.NoError(root.Init(map[string]interface{}{"key": []string{"value"}}))
 }
diff --git a/plugin/testdata/externalPluginSchema.json b/plugin/testdata/externalPluginSchema.json
index 5e1101a..15b8f59 100644
--- a/plugin/testdata/externalPluginSchema.json
+++ b/plugin/testdata/externalPluginSchema.json
@@ -1160,9 +1160,7 @@
     "aws.ec2InstanceMetadataJSON": {
         "label": "metadata.json",
         "singleton": true,
-        "methods": [
-            "read"
-        ],
+        "methods": [],
         "meta_attribute_schema": {
             "$schema": "http://json-schema.org/draft-04/schema#",
             "additionalProperties": false,