Merge pull request #241 from vmarkovtsev/master

Do not require Features() for upstream items

internal/core/registry.go

@@ -84,43 +84,59 @@ func (registry *PipelineItemRegistry) GetPlumbingItems() []PipelineItem {
 	return items
 }
 
-type orderedFeaturedItems []FeaturedPipelineItem
-
-func (ofi orderedFeaturedItems) Len() int {
-	return len([]FeaturedPipelineItem(ofi))
-}
-
-func (ofi orderedFeaturedItems) Less(i, j int) bool {
-	cofi := []FeaturedPipelineItem(ofi)
-	return cofi[i].Name() < cofi[j].Name()
-}
-
-func (ofi orderedFeaturedItems) Swap(i, j int) {
-	cofi := []FeaturedPipelineItem(ofi)
-	cofi[i], cofi[j] = cofi[j], cofi[i]
-}
-
 // GetFeaturedItems returns all FeaturedPipelineItem-s registered.
-func (registry *PipelineItemRegistry) GetFeaturedItems() map[string][]FeaturedPipelineItem {
-	features := map[string][]FeaturedPipelineItem{}
+func (registry *PipelineItemRegistry) GetFeaturedItems() map[string][]PipelineItem {
+	features := map[string][]PipelineItem{}
 	for _, t := range registry.registered {
-		if fiface, ok := reflect.New(t.Elem()).Interface().(FeaturedPipelineItem); ok {
-			for _, f := range fiface.Features() {
-				list := features[f]
-				if list == nil {
-					list = []FeaturedPipelineItem{}
+		item := reflect.New(t.Elem()).Interface().(PipelineItem)
+		deps := registry.CollectAllDependencies(item)
+		deps = append(deps, item)
+		depFeatures := map[string]bool{}
+		for _, dep := range deps {
+			if fiface, ok := dep.(FeaturedPipelineItem); ok {
+				for _, f := range fiface.Features() {
+					depFeatures[f] = true
 				}
-				list = append(list, fiface)
-				features[f] = list
 			}
 		}
+		for f := range depFeatures {
+			features[f] = append(features[f], item)
+		}
 	}
+
 	for _, vals := range features {
-		sort.Sort(orderedFeaturedItems(vals))
+		sort.Slice(vals, func(i, j int) bool {
+			return vals[i].Name() < vals[j].Name()
+		})
 	}
 	return features
 }
 
+// CollectAllDependencies recursively builds the list of all the items on which the specified item
+// depends.
+func (registry *PipelineItemRegistry) CollectAllDependencies(item PipelineItem) []PipelineItem {
+	deps := map[string]PipelineItem{}
+	for stack := []PipelineItem{item}; len(stack) > 0; {
+		head := stack[len(stack)-1]
+		stack = stack[:len(stack)-1]
+		for _, reqID := range head.Requires() {
+			req := registry.Summon(reqID)[0]
+			if _, exists := deps[reqID]; !exists {
+				deps[reqID] = req
+				stack = append(stack, req)
+			}
+		}
+	}
+	result := make([]PipelineItem, 0, len(deps))
+	for _, val := range deps {
+		result = append(result, val)
+	}
+	sort.Slice(result, func(i, j int) bool {
+		return result[i].Name() < result[j].Name()
+	})
+	return result
+}
+
 var pathFlagTypeMasquerade bool
 
 // EnablePathFlagTypeMasquerade changes the type of all "path" command line arguments from "string"

internal/core/registry_test.go

@@ -114,6 +114,82 @@ func (item *dummyPipelineItem2) Fork(n int) []PipelineItem {
 func (item *dummyPipelineItem2) Merge(branches []PipelineItem) {
 }
 
+type dummyPipelineItem3 struct{}
+
+func (item *dummyPipelineItem3) Name() string {
+	return "dummy3"
+}
+
+func (item *dummyPipelineItem3) Provides() []string {
+	arr := [...]string{"dummy3"}
+	return arr[:]
+}
+
+func (item *dummyPipelineItem3) Requires() []string {
+	return []string{"dummy"}
+}
+
+func (item *dummyPipelineItem3) Configure(facts map[string]interface{}) error {
+	return nil
+}
+
+func (item *dummyPipelineItem3) ListConfigurationOptions() []ConfigurationOption {
+	return nil
+}
+
+func (item *dummyPipelineItem3) Initialize(repository *git.Repository) error {
+	return nil
+}
+
+func (item *dummyPipelineItem3) Consume(deps map[string]interface{}) (map[string]interface{}, error) {
+	return map[string]interface{}{"dummy": nil}, nil
+}
+
+func (item *dummyPipelineItem3) Fork(n int) []PipelineItem {
+	return nil
+}
+
+func (item *dummyPipelineItem3) Merge(branches []PipelineItem) {
+}
+
+type dummyPipelineItem4 struct{}
+
+func (item *dummyPipelineItem4) Name() string {
+	return "dummy4"
+}
+
+func (item *dummyPipelineItem4) Provides() []string {
+	arr := [...]string{"dummy4"}
+	return arr[:]
+}
+
+func (item *dummyPipelineItem4) Requires() []string {
+	return []string{"dummy3"}
+}
+
+func (item *dummyPipelineItem4) Configure(facts map[string]interface{}) error {
+	return nil
+}
+
+func (item *dummyPipelineItem4) ListConfigurationOptions() []ConfigurationOption {
+	return nil
+}
+
+func (item *dummyPipelineItem4) Initialize(repository *git.Repository) error {
+	return nil
+}
+
+func (item *dummyPipelineItem4) Consume(deps map[string]interface{}) (map[string]interface{}, error) {
+	return map[string]interface{}{"dummy": nil}, nil
+}
+
+func (item *dummyPipelineItem4) Fork(n int) []PipelineItem {
+	return nil
+}
+
+func (item *dummyPipelineItem4) Merge(branches []PipelineItem) {
+}
+
 func TestRegistrySummon(t *testing.T) {
 	reg := getRegistry()
 	assert.Len(t, reg.Summon("whatever"), 0)
@@ -187,6 +263,18 @@ func TestRegistryFeatures(t *testing.T) {
 	assert.True(t, val)
 }
 
+func TestRegistryCollectAllDependencies(t *testing.T) {
+	reg := getRegistry()
+	reg.Register(&dummyPipelineItem{})
+	reg.Register(&dummyPipelineItem3{})
+	reg.Register(&dummyPipelineItem4{})
+	assert.Len(t, reg.CollectAllDependencies(&dummyPipelineItem{}), 0)
+	deps := reg.CollectAllDependencies(&dummyPipelineItem4{})
+	assert.Len(t, deps, 2)
+	assert.Equal(t, deps[0].Name(), (&dummyPipelineItem{}).Name())
+	assert.Equal(t, deps[1].Name(), (&dummyPipelineItem3{}).Name())
+}
+
 func TestRegistryLeaves(t *testing.T) {
 	reg := getRegistry()
 	reg.Register(&testPipelineItem{})
@@ -213,11 +301,17 @@ func TestRegistryFeaturedItems(t *testing.T) {
 	reg.Register(&testPipelineItem{})
 	reg.Register(&dependingTestPipelineItem{})
 	reg.Register(&dummyPipelineItem{})
+	reg.Register(&dummyPipelineItem3{})
+	reg.Register(&dummyPipelineItem4{})
 	featured := reg.GetFeaturedItems()
 	assert.Len(t, featured, 1)
-	assert.Len(t, featured["power"], 2)
-	assert.Equal(t, featured["power"][0].Name(), (&testPipelineItem{}).Name())
-	assert.Equal(t, featured["power"][1].Name(), (&dummyPipelineItem{}).Name())
+	power := featured["power"]
+	assert.Len(t, power, 5)
+	assert.Equal(t, power[0].Name(), (&testPipelineItem{}).Name())
+	assert.Equal(t, power[1].Name(), (&dependingTestPipelineItem{}).Name())
+	assert.Equal(t, power[2].Name(), (&dummyPipelineItem{}).Name())
+	assert.Equal(t, power[3].Name(), (&dummyPipelineItem3{}).Name())
+	assert.Equal(t, power[4].Name(), (&dummyPipelineItem4{}).Name())
 }
 
 func TestRegistryPathMasquerade(t *testing.T) {

internal/plumbing/uast/uast.go

@@ -16,7 +16,7 @@ import (
 
 	"github.com/Jeffail/tunny"
 	"github.com/gogo/protobuf/proto"
-	bblfsh "gopkg.in/bblfsh/client-go.v3"
+	"gopkg.in/bblfsh/client-go.v3"
 	"gopkg.in/bblfsh/sdk.v2/uast/nodes"
 	"gopkg.in/bblfsh/sdk.v2/uast/nodes/nodesproto"
 	"gopkg.in/src-d/go-git.v4"
@@ -334,12 +334,6 @@ func (uc *Changes) Requires() []string {
 	return arr[:]
 }
 
-// Features which must be enabled for this PipelineItem to be automatically inserted into the DAG.
-func (uc *Changes) Features() []string {
-	arr := [...]string{FeatureUast}
-	return arr[:]
-}
-
 // ListConfigurationOptions returns the list of changeable public properties of this PipelineItem.
 func (uc *Changes) ListConfigurationOptions() []core.ConfigurationOption {
 	return []core.ConfigurationOption{}
@@ -446,12 +440,6 @@ func (saver *ChangesSaver) Requires() []string {
 	return arr[:]
 }
 
-// Features which must be enabled for this PipelineItem to be automatically inserted into the DAG.
-func (saver *ChangesSaver) Features() []string {
-	arr := [...]string{FeatureUast}
-	return arr[:]
-}
-
 // ListConfigurationOptions returns the list of changeable public properties of this PipelineItem.
 func (saver *ChangesSaver) ListConfigurationOptions() []core.ConfigurationOption {
 	options := [...]core.ConfigurationOption{{

internal/plumbing/uast/uast_test.go

@@ -204,9 +204,6 @@ func TestUASTChangesMeta(t *testing.T) {
 	assert.Equal(t, ch.Requires()[1], items.DependencyTreeChanges)
 	opts := ch.ListConfigurationOptions()
 	assert.Len(t, opts, 0)
-	feats := ch.Features()
-	assert.Len(t, feats, 1)
-	assert.Equal(t, feats[0], FeatureUast)
 }
 
 func TestUASTChangesRegistration(t *testing.T) {
@@ -336,9 +333,6 @@ func TestUASTChangesSaverMeta(t *testing.T) {
 	opts := chs.ListConfigurationOptions()
 	assert.Len(t, opts, 1)
 	assert.Equal(t, opts[0].Name, ConfigUASTChangesSaverOutputPath)
-	feats := chs.Features()
-	assert.Len(t, feats, 1)
-	assert.Equal(t, feats[0], FeatureUast)
 	assert.Equal(t, chs.Flag(), "dump-uast-changes")
 }
 

leaves/comment_sentiment.go

@@ -84,12 +84,6 @@ func (sent *CommentSentimentAnalysis) Requires() []string {
 	return arr[:]
 }
 
-// Features which must be enabled for this PipelineItem to be automatically inserted into the DAG.
-func (sent *CommentSentimentAnalysis) Features() []string {
-	arr := [...]string{uast_items.FeatureUast}
-	return arr[:]
-}
-
 // ListConfigurationOptions returns the list of changeable public properties of this PipelineItem.
 func (sent *CommentSentimentAnalysis) ListConfigurationOptions() []core.ConfigurationOption {
 	options := [...]core.ConfigurationOption{{

leaves/comment_sentiment_test.go

@@ -55,8 +55,6 @@ func TestCommentSentimentMeta(t *testing.T) {
 	}
 	assert.Len(t, opts, matches)
 	assert.Equal(t, sent.Flag(), "sentiment")
-	assert.Len(t, sent.Features(), 1)
-	assert.Equal(t, sent.Features()[0], uast_items.FeatureUast)
 }
 
 func TestCommentSentimentConfigure(t *testing.T) {

leaves/shotness.go

@@ -95,12 +95,6 @@ func (shotness *ShotnessAnalysis) Requires() []string {
 	return arr[:]
 }
 
-// Features which must be enabled for this PipelineItem to be automatically inserted into the DAG.
-func (shotness *ShotnessAnalysis) Features() []string {
-	arr := [...]string{uast_items.FeatureUast}
-	return arr[:]
-}
-
 // ListConfigurationOptions returns the list of changeable public properties of this PipelineItem.
 func (shotness *ShotnessAnalysis) ListConfigurationOptions() []core.ConfigurationOption {
 	opts := [...]core.ConfigurationOption{{

leaves/shotness_test.go

@@ -51,9 +51,6 @@ func TestShotnessMeta(t *testing.T) {
 	assert.Nil(t, sh.Configure(facts))
 	assert.Equal(t, sh.XpathStruct, "xpath!")
 	assert.Equal(t, sh.XpathName, "another!")
-	features := sh.Features()
-	assert.Len(t, features, 1)
-	assert.Equal(t, features[0], uast_items.FeatureUast)
 }
 
 func TestShotnessRegistration(t *testing.T) {