Merge pull request #739 from ekinanp/727

Include core plugin entry sgraph in external plugin entry sgraph

Author: MikaelSmith

plugin/entrySchema.go

@@ -18,12 +18,31 @@ func TypeID(e Entry) string {
 	pluginName := pluginName(e)
 	rawTypeID := rawTypeID(e)
 	if pluginName == "" {
-		// e is the plugin registry
+		// e is the plugin registry or a core entry used by an external plugin
 		return rawTypeID
 	}
 	return namespace(pluginName, rawTypeID)
 }
 
+// SchemaGraph returns e's schema graph. This is effectively a
+// map[string]plugin.EntrySchema object that preserves insertion
+// order (where the string is each entry's type ID).
+func SchemaGraph(e Entry) (*linkedhashmap.Map, error) {
+	s, err := schema(e)
+	if err != nil {
+		return nil, err
+	}
+	if s == nil {
+		return nil, nil
+	}
+	if s.graph == nil {
+		// e is a core plugin entry so fill its graph
+		s.graph = linkedhashmap.New()
+		s.fill(s.graph)
+	}
+	return s.graph, nil
+}
+
 func schema(e Entry) (*EntrySchema, error) {
 	switch t := e.(type) {
 	case externalPlugin:
@@ -49,7 +68,10 @@ func schema(e Entry) (*EntrySchema, error) {
 			IsSingleton().
 			SetDescription(registryDescription)
 		schema.graph = linkedhashmap.New()
-		schema.graph.Put(TypeID(t), &schema.entrySchema)
+		typeID := TypeID(t)
+		// We start by putting in a stub value for s so that we preserve the insertion
+		// order. We'll then update this value once the "Children" array's been calculated.
+		schema.graph.Put(typeID, EntrySchema{})
 		for _, root := range t.pluginRoots {
 			childSchema, err := Schema(root)
 			if err != nil {
@@ -73,6 +95,8 @@ func schema(e Entry) (*EntrySchema, error) {
 				schema.graph.Put(key, value)
 			})
 		}
+		// Update the graph
+		schema.graph.Put(typeID, schema.clone())
 		return schema, nil
 	default:
 		// e is a core-plugin
@@ -137,9 +161,10 @@ func NewEntrySchema(e Entry, label string) *EntrySchema {
 // how plugin.EntrySchema objects are meant to be used.
 func (s EntrySchema) MarshalJSON() ([]byte, error) {
 	if s.entry == nil {
-		// Nodes in the external plugin graph don't use NewEntrySchema, they directly set the
-		// undocumented fields of EntrySchema. Since graph and entry won't be set - and this is
-		// part of a graph already - directly serialize entrySchema instead of using the graph.
+		// This corresponds to an "EntrySchema" value in another entry's schema graph.
+		// These values directly set the undocumented fields of EntrySchema. Since graph
+		// and entry won't be set - and this is part of a graph already - directly serialize
+		// entrySchema instead of using the graph.
 		return json.Marshal(s.entrySchema)
 	}
 
@@ -253,12 +278,18 @@ func (s *EntrySchema) fill(graph *linkedhashmap.Map) {
 			s.fillPanicf("bad value passed into SetMetadataSchema: %v", err)
 		}
 	}
-	graph.Put(TypeID(s.entry), &s.entrySchema)
+	typeID := TypeID(s.entry)
 
-	// Fill-in the children
 	if !ListAction().IsSupportedOn(s.entry) {
+		graph.Put(typeID, s.clone())
 		return
 	}
+
+	// Fill-in the children. We start by putting in a stub value for s
+	// so that we preserve the insertion order and so that we can mark
+	// the node as visited. We'll then update this value once the "Children"
+	// array's been calculated.
+	graph.Put(typeID, EntrySchema{})
 	// "sParent" is read as "s.parent"
 	sParent := s.entry.(Parent)
 	children := sParent.ChildSchemas()
@@ -280,6 +311,16 @@ func (s *EntrySchema) fill(graph *linkedhashmap.Map) {
 		passAlongWrappedTypes(sParent, child.entry)
 		child.fill(graph)
 	}
+	// Update the graph
+	graph.Put(typeID, s.clone())
+}
+
+// clone returns a copy of s.entrySchema's fields as an EntrySchema
+// object
+func (s *EntrySchema) clone() EntrySchema {
+	var copy EntrySchema
+	copy.entrySchema = s.entrySchema
+	return copy
 }
 
 // This helper's used by CachedList + EntrySchema#fill(). The reason for
@@ -339,16 +380,9 @@ func pluginName(e Entry) string {
 		switch e.(type) {
 		case Root:
 			return CName(e)
-		case *Registry:
-			return ""
 		default:
-			// e has no ID. This is possible if e's from the apifs package. For now,
-			// it is enough to return "__apifs__" here because this is an unlikely
-			// edge case.
-			//
-			// TODO: Panic here once https://github.com/puppetlabs/wash/issues/438
-			// is resolved.
-			return "__apifs__"
+			// e is an apifs entry or a core plugin entry used by an external plugin
+			return ""
 		}
 	}
 	segments := strings.SplitN(trimmedID, "/", 2)

plugin/external/coreEntries.go

@@ -10,7 +10,7 @@ import (
 
 type coreEntry interface {
 	createInstance(parent *pluginEntry, decodedEntry decodedExternalPluginEntry) (plugin.Entry, error)
-	schema() *plugin.EntrySchema
+	template() plugin.Entry
 }
 
 var coreEntries = map[string]coreEntry{
@@ -31,6 +31,6 @@ func (volumeFS) createInstance(parent *pluginEntry, e decodedExternalPluginEntry
 	return volume.NewFS(e.Name, parent, int(opts.Maxdepth)), nil
 }
 
-func (volumeFS) schema() *plugin.EntrySchema {
-	return (&volume.FS{}).Schema()
+func (volumeFS) template() plugin.Entry {
+	return &volume.FS{}
 }

plugin/external/pluginEntry.go

@@ -13,7 +13,6 @@ import (
 	"time"
 
 	"github.com/getlantern/deepcopy"
-	"github.com/jinzhu/copier"
 
 	"github.com/emirpasic/gods/maps/linkedhashmap"
 	"github.com/puppetlabs/wash/activity"
@@ -711,17 +710,24 @@ func unmarshalSchemaGraph(pluginName, rawTypeID string, stdout []byte) (*linkedh
 	graph := linkedhashmap.New()
 	putNode := func(rawTypeID string, rawSchema interface{}) error {
 		if coreEnt, ok := coreEntries[rawTypeID]; ok {
-			pluginSchema := coreEnt.schema()
-
-			// Copy only the public fields so we serialize it as just data. Uses copier because it uses
-			// reflect to copy public fields, rather than Marshal/UnmarshalJSON which we've overridden.
-			var schema plugin.EntrySchema
-			err := copier.Copy(&schema, pluginSchema)
-			if err != nil {
-				panic(fmt.Sprintf("should always be able to copy from EntrySchema to EntrySchema: %v", err))
+			template := coreEnt.template()
+			rawTypeID = plugin.TypeID(template)
+			if populatedTypeIDs[rawTypeID] {
+				// We've already included this entry's schema-graph
+				return nil
 			}
+			coreEntSchemaGraph, _ := plugin.SchemaGraph(template)
+			coreEntSchemaGraph.Each(func(rawTypeIDV interface{}, schemaV interface{}) {
+				// Munge the schema's children's type IDs
+				schema := schemaV.(plugin.EntrySchema)
+				children := []string{}
+				for _, child := range schema.Children {
+					children = append(children, namespace(pluginName, child))
+				}
+				schema.Children = children
+				graph.Put(namespace(pluginName, rawTypeIDV.(string)), schema)
+			})
 			populatedTypeIDs[rawTypeID] = true
-			graph.Put(namespace(pluginName, rawTypeID), schema)
 			return nil
 		}
 
@@ -764,6 +770,9 @@ func unmarshalSchemaGraph(pluginName, rawTypeID string, stdout []byte) (*linkedh
 			}
 			var namespacedChildren []string
 			for _, child := range node.Children {
+				if coreEnt, ok := coreEntries[child]; ok {
+					child = plugin.TypeID(coreEnt.template())
+				}
 				requiredTypeIDs[child] = true
 				namespacedChildren = append(namespacedChildren, namespace(pluginName, child))
 			}

plugin/external/pluginEntry_test.go

@@ -1385,6 +1385,61 @@ func (suite *ExternalPluginEntryTestSuite) TestUnmarshalSchemaGraph_ValidInput()
 	}
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestUnmarshalSchemaGraph_CoreEntries() {
+	entry := &pluginEntry{
+		rawTypeID: "foo",
+	}
+	entry.SetTestID("fooPlugin")
+
+	stdout := []byte(`
+{
+	"foo":{
+		"label": "fooLabel",
+		"methods": ["list"],
+		"children": ["__volume::fs__"]
+	},
+	"__volume::fs__": {}
+}
+`)
+
+	graph, err := unmarshalSchemaGraph(pluginName(entry), rawTypeID(entry), stdout)
+	if suite.NoError(err) {
+		assertSchema := func(typeID string, assertFunc func(plugin.EntrySchema)) {
+			schema, found := graph.Get(typeID)
+			if !found {
+				suite.FailNow("expected %v to be present in schema graph", typeID)
+			}
+			assertFunc(schema.(plugin.EntrySchema))
+		}
+
+		// Ensure that only four nodes exist in schema graph -- "foo", volume::fs,
+		// volume::dir, and volume::file
+		suite.Equal(int(4), graph.Size())
+
+		// Now ensure that the right nodes are set in the graph
+		volumeFSTemplate := (&volumeFS{}).template()
+		assertSchema("fooPlugin::foo", func(s plugin.EntrySchema) {
+			expectedVolumeFSTypeID := fmt.Sprintf("fooPlugin::%v", plugin.TypeID(volumeFSTemplate))
+			suite.Equal([]string{expectedVolumeFSTypeID}, s.Children)
+		})
+		// Ensure that volume::fs' schema-graph's been merged with our schema graph
+		// and that all the type IDs are properly namespaced
+		volumeFSGraph, _ := plugin.SchemaGraph(volumeFSTemplate)
+		volumeFSGraph.Each(func(typeIDV interface{}, schemaV interface{}) {
+			typeID := typeIDV.(string)
+			schema := schemaV.(plugin.EntrySchema)
+			expectedChildren := []string{}
+			for _, child := range schema.Children {
+				expectedChildren = append(expectedChildren, "fooPlugin::"+child)
+			}
+			assertSchema("fooPlugin::"+typeID, func(s plugin.EntrySchema) {
+				suite.Equal(schema.Label, s.Label)
+				suite.Equal(expectedChildren, s.Children)
+			})
+		})
+	}
+}
+
 func TestExternalPluginEntry(t *testing.T) {
 	suite.Run(t, new(ExternalPluginEntryTestSuite))
 }