Merge pull request #52 from gardener/removeemptynodes

Strip irrelevant nodes generated by resolved nodeSelector
gardener · Oct 15, 2020 · 07ac25d · 07ac25d
2 parents 36baf64 + b4c2c76
commit 07ac25d
Show file tree

Hide file tree

Showing 4 changed files with 178 additions and 61 deletions.
diff --git a/pkg/reactor/build_test.go b/pkg/reactor/build_test.go
@@ -0,0 +1,64 @@
+package reactor
+
+import (
+	"testing"
+
+	"github.com/gardener/docforge/pkg/api"
+	"github.com/gardener/docforge/pkg/resourcehandlers"
+)
+
+func Test_tasks(t *testing.T) {
+	newDoc := createNewDocumentation()
+	type args struct {
+		node  *api.Node
+		tasks []interface{}
+		lds   localityDomain
+	}
+	tests := []struct {
+		name          string
+		args          args
+		expectedTasks []*DocumentWorkTask
+	}{
+		{
+			name: "it creates tasks based on the provided doc",
+			args: args{
+				node:  newDoc.Root,
+				tasks: []interface{}{},
+			},
+			expectedTasks: []*DocumentWorkTask{
+				{
+					Node: newDoc.Root,
+				},
+				{
+					Node: archNode,
+				},
+				{
+					Node: apiRefNode,
+				},
+				{
+					Node: blogNode,
+				},
+				{
+					Node: tasksNode,
+				},
+			},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			rhs := resourcehandlers.NewRegistry(&FakeResourceHandler{})
+			tasks(tc.args.node, &tc.args.tasks)
+
+			if len(tc.args.tasks) != len(tc.expectedTasks) {
+				t.Errorf("expected number of tasks %d != %d", len(tc.expectedTasks), len(tc.args.tasks))
+			}
+
+			for i, task := range tc.args.tasks {
+				if task.(*DocumentWorkTask).Node.Name != tc.expectedTasks[i].Node.Name {
+					t.Errorf("expected task with Node name %s != %s", task.(*DocumentWorkTask).Node.Name, tc.expectedTasks[i].Node.Name)
+				}
+			}
+			rhs.Remove()
+		})
+	}
+}
diff --git a/pkg/reactor/reactor_test.go b/pkg/reactor/reactor_test.go
@@ -4,10 +4,7 @@ import (
 	"context"
 	// "fmt"
 
-	"testing"
-
 	"github.com/gardener/docforge/pkg/api"
-	"github.com/gardener/docforge/pkg/resourcehandlers"
 	"github.com/gardener/docforge/pkg/util/tests"
 )
 
@@ -44,62 +41,6 @@ var (
 	}
 )
 
-func Test_tasks(t *testing.T) {
-	newDoc := createNewDocumentation()
-	type args struct {
-		node  *api.Node
-		tasks []interface{}
-		lds   localityDomain
-	}
-	tests := []struct {
-		name          string
-		args          args
-		expectedTasks []*DocumentWorkTask
-	}{
-		{
-			name: "it creates tasks based on the provided doc",
-			args: args{
-				node:  newDoc.Root,
-				tasks: []interface{}{},
-			},
-			expectedTasks: []*DocumentWorkTask{
-				{
-					Node: newDoc.Root,
-				},
-				{
-					Node: archNode,
-				},
-				{
-					Node: apiRefNode,
-				},
-				{
-					Node: blogNode,
-				},
-				{
-					Node: tasksNode,
-				},
-			},
-		},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			rhs := resourcehandlers.NewRegistry(&FakeResourceHandler{})
-			tasks(tc.args.node, &tc.args.tasks)
-
-			if len(tc.args.tasks) != len(tc.expectedTasks) {
-				t.Errorf("expected number of tasks %d != %d", len(tc.expectedTasks), len(tc.args.tasks))
-			}
-
-			for i, task := range tc.args.tasks {
-				if task.(*DocumentWorkTask).Node.Name != tc.expectedTasks[i].Node.Name {
-					t.Errorf("expected task with Node name %s != %s", task.(*DocumentWorkTask).Node.Name, tc.expectedTasks[i].Node.Name)
-				}
-			}
-			rhs.Remove()
-		})
-	}
-}
-
 func createNewDocumentation() *api.Documentation {
 	return &api.Documentation{
 		Root: &api.Node{

diff --git a/pkg/resourcehandlers/github/github_resource_handler.go b/pkg/resourcehandlers/github/github_resource_handler.go
@@ -116,15 +116,38 @@ func buildNodes(node *api.Node, childResourceLocators []*ResourceLocator, cache
 	}
 }
 
+// - remove contentSources that reference tree objects. They are used
+//   internally to build the structure but are not a valid contentSource
+// - remove empty nodes that do not contain markdown. The build algorithm
+//   is blind for the content of a node and leaves nodes that are folders
+//   containing for example images only adn thus irrelevant to the
+//   documentation structure
 func cleanupNodeTree(node *api.Node) {
+	if len(node.ContentSelectors) > 0 {
+		source := node.ContentSelectors[0].Source
+		if rl, _ := parse(source); rl.Type == Tree {
+			node.ContentSelectors = nil
+		}
+	}
 	for _, n := range node.Nodes {
 		cleanupNodeTree(n)
 	}
+	childrenCopy := make([]*api.Node, len(node.Nodes))
 	if len(node.Nodes) > 0 {
-		node.ContentSelectors = nil
+		copy(childrenCopy, node.Nodes)
+	}
+	for i, n := range node.Nodes {
+		if n.ContentSelectors == nil && len(n.Nodes) == 0 {
+			childrenCopy = removeNode(childrenCopy, i)
+		}
+		node.Nodes = childrenCopy
 	}
 }
 
+func removeNode(n []*api.Node, i int) []*api.Node {
+	return append(n[:i], n[i+1:]...)
+}
+
 // Cache is indexes GitHub TreeEntries by website resource URLs as keys,
 // mapping ResourceLocator objects to them.
 // TODO: implement me efficiently and for parallel use

diff --git a/pkg/resourcehandlers/github/github_resource_handler_test.go b/pkg/resourcehandlers/github/github_resource_handler_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/gardener/docforge/pkg/api"
 	"github.com/gardener/docforge/pkg/util/tests"
 	"github.com/google/go-github/v32/github"
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -437,7 +438,6 @@ func TestGitHub_ResolveRelLink(t *testing.T) {
 }
 
 func TestGetLocalityDomainCandidate(t *testing.T) {
-
 	tests := []struct {
 		name        string
 		link        string
@@ -474,3 +474,92 @@ func TestGetLocalityDomainCandidate(t *testing.T) {
 		})
 	}
 }
+
+func TestCleanupNodeTree(t *testing.T) {
+	tests := []struct {
+		name     string
+		node     *api.Node
+		wantNode *api.Node
+	}{
+		{
+			name: "",
+			node: &api.Node{
+				Name: "00",
+				ContentSelectors: []api.ContentSelector{
+					api.ContentSelector{
+						Source: "https://github.com/gardener/gardener/tree/master/docs/00",
+					},
+				},
+				Nodes: []*api.Node{
+					&api.Node{
+						Name: "01",
+						ContentSelectors: []api.ContentSelector{
+							api.ContentSelector{
+								Source: "https://github.com/gardener/gardener/blob/master/docs/01.md",
+							},
+						},
+					},
+					&api.Node{
+						Name: "02",
+						ContentSelectors: []api.ContentSelector{
+							api.ContentSelector{
+								Source: "https://github.com/gardener/gardener/tree/master/docs/02",
+							},
+						},
+						Nodes: []*api.Node{
+							&api.Node{
+								Name: "021",
+								ContentSelectors: []api.ContentSelector{
+									api.ContentSelector{
+										Source: "https://github.com/gardener/gardener/blob/master/docs/021.md",
+									},
+								},
+							},
+						},
+					},
+					&api.Node{
+						Name: "03",
+						ContentSelectors: []api.ContentSelector{
+							api.ContentSelector{
+								Source: "https://github.com/gardener/gardener/tree/master/docs/03",
+							},
+						},
+						Nodes: []*api.Node{},
+					},
+				},
+			},
+			wantNode: &api.Node{
+				Name: "00",
+				Nodes: []*api.Node{
+					&api.Node{
+						Name: "01",
+						ContentSelectors: []api.ContentSelector{
+							api.ContentSelector{
+								Source: "https://github.com/gardener/gardener/blob/master/docs/01.md",
+							},
+						},
+					},
+					&api.Node{
+						Name: "02",
+						Nodes: []*api.Node{
+							&api.Node{
+								Name: "021",
+								ContentSelectors: []api.ContentSelector{
+									api.ContentSelector{
+										Source: "https://github.com/gardener/gardener/blob/master/docs/021.md",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cleanupNodeTree(tc.node)
+			assert.Equal(t, tc.wantNode, tc.node)
+		})
+	}
+}