Decorators, begone!

Decorators were just a complicated way of constructing filters.
This commit is contained in:
Matthias Radestock
2017-11-21 20:16:30 +00:00
parent 093857f37f
commit 88e8b52d66
9 changed files with 65 additions and 122 deletions

View File

@@ -336,9 +336,8 @@ type APITopologyOptionGroup struct {
NoneLabel string `json:"noneLabel,omitempty"`
}
// Get the render filters to use for this option group as a Decorator, if any.
// If second arg is false, no decorator was needed.
func (g APITopologyOptionGroup) getFilterDecorator(value string) (render.Decorator, bool) {
// Get the render filters to use for this option group, if any, or nil otherwise.
func (g APITopologyOptionGroup) filter(value string) render.FilterFunc {
selectType := g.SelectType
if selectType == "" {
selectType = "one"
@@ -351,7 +350,7 @@ func (g APITopologyOptionGroup) getFilterDecorator(value string) (render.Decorat
values = strings.Split(value, ",")
default:
log.Errorf("Invalid select type %s for option group %s, ignoring option", selectType, g.ID)
return nil, false
return nil
}
filters := []render.FilterFunc{}
for _, opt := range g.Options {
@@ -374,11 +373,9 @@ func (g APITopologyOptionGroup) getFilterDecorator(value string) (render.Decorat
}
}
if len(filters) == 0 {
return nil, false
return nil
}
// Since we've encoded whether to ignore pseudo topologies into each subfilter,
// we want no special behaviour for pseudo topologies here, which corresponds to MakePseudo
return render.MakeFilterPseudoDecorator(render.AnyFilterFunc(filters...)), true
return render.AnyFilterFunc(filters...)
}
// APITopologyOption describes a &param=value to a given topology.
@@ -487,24 +484,24 @@ func (r *Registry) renderTopologies(rpt report.Report, req *http.Request) []APIT
topologies := []APITopologyDesc{}
req.ParseForm()
r.walk(func(desc APITopologyDesc) {
renderer, decorator, _ := r.RendererForTopology(desc.id, req.Form, rpt)
desc.Stats = decorateWithStats(rpt, renderer, decorator)
renderer, filter, _ := r.RendererForTopology(desc.id, req.Form, rpt)
desc.Stats = decorateWithStats(rpt, renderer, filter)
for i, sub := range desc.SubTopologies {
renderer, decorator, _ := r.RendererForTopology(sub.id, req.Form, rpt)
desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator)
renderer, filter, _ := r.RendererForTopology(sub.id, req.Form, rpt)
desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, filter)
}
topologies = append(topologies, desc)
})
return updateFilters(rpt, topologies)
}
func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator render.Decorator) topologyStats {
func decorateWithStats(rpt report.Report, renderer render.Renderer, filter render.FilterFunc) topologyStats {
var (
nodes int
realNodes int
edges int
)
r := render.Decorate(rpt, renderer, decorator)
r := render.Render(rpt, renderer, filter)
for _, n := range r.Nodes {
nodes++
if n.Topology != render.Pseudo {
@@ -521,7 +518,7 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re
}
// RendererForTopology ..
func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.Decorator, error) {
func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.FilterFunc, error) {
topology, ok := r.get(topologyID)
if !ok {
return nil, nil, fmt.Errorf("topology not found: %s", topologyID)
@@ -533,15 +530,15 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt
return topology.renderer, nil, nil
}
var decorators []render.Decorator
var filters []render.FilterFunc
for _, group := range topology.Options {
value := values.Get(group.ID)
if decorator, ok := group.getFilterDecorator(value); ok {
decorators = append(decorators, decorator)
if filter := group.filter(value); filter != nil {
filters = append(filters, filter)
}
}
if len(decorators) > 0 {
return topology.renderer, render.ComposeDecorators(decorators...), nil
if len(filters) > 0 {
return topology.renderer, render.ComposeFilterFuncs(filters...), nil
}
return topology.renderer, nil, nil
}
@@ -554,7 +551,7 @@ func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc {
}
}
type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.RenderContext, http.ResponseWriter, *http.Request)
type rendererHandler func(context.Context, render.Renderer, render.FilterFunc, report.RenderContext, http.ResponseWriter, *http.Request)
func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc {
return func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
@@ -572,11 +569,11 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu
return
}
req.ParseForm()
renderer, decorator, err := r.RendererForTopology(topologyID, req.Form, rpt)
renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rpt)
if err != nil {
respondWith(w, http.StatusInternalServerError, err)
return
}
f(ctx, renderer, decorator, RenderContextForReporter(rep, rpt), w, req)
f(ctx, renderer, filter, RenderContextForReporter(rep, rpt), w, req)
}
}

View File

@@ -109,7 +109,7 @@ func TestRendererForTopologyWithFiltering(t *testing.T) {
urlvalues.Set(systemGroupID, customAPITopologyOptionFilterID)
urlvalues.Set("stopped", "running")
urlvalues.Set("pseudo", "hide")
renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report)
renderer, filter, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report)
if err != nil {
t.Fatalf("Topology Registry Report error: %s", err)
}
@@ -118,7 +118,7 @@ func TestRendererForTopologyWithFiltering(t *testing.T) {
input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{
docker.LabelPrefix + "works.weave.role": "system",
})
have := utils.Prune(render.Decorate(input, renderer, decorator).Nodes)
have := utils.Prune(render.Render(input, renderer, filter).Nodes)
want := utils.Prune(expected.RenderedContainers.Copy())
delete(want, fixture.ClientContainerNodeID)
delete(want, render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID))
@@ -140,7 +140,7 @@ func TestRendererForTopologyNoFiltering(t *testing.T) {
urlvalues.Set(systemGroupID, customAPITopologyOptionFilterID)
urlvalues.Set("stopped", "running")
urlvalues.Set("pseudo", "hide")
renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report)
renderer, filter, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report)
if err != nil {
t.Fatalf("Topology Registry Report error: %s", err)
}
@@ -149,7 +149,7 @@ func TestRendererForTopologyNoFiltering(t *testing.T) {
input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{
docker.LabelPrefix + "works.weave.role": "system",
})
have := utils.Prune(render.Decorate(input, renderer, decorator).Nodes)
have := utils.Prune(render.Render(input, renderer, filter).Nodes)
want := utils.Prune(expected.RenderedContainers.Copy())
delete(want, render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID))
delete(want, render.OutgoingInternetID)
@@ -178,12 +178,12 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det
urlvalues.Set(systemGroupID, customAPITopologyOptionFilterID)
urlvalues.Set("stopped", "running")
urlvalues.Set("pseudo", "hide")
renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report)
renderer, filter, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report)
if err != nil {
return nil, err
}
return detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.Decorate(fixture.Report, renderer, decorator).Nodes), nil
return detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.Render(fixture.Report, renderer, filter).Nodes), nil
}
func TestAPITopologyAddsKubernetes(t *testing.T) {

View File

@@ -29,23 +29,23 @@ type APINode struct {
}
// Full topology.
func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) {
func handleTopology(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) {
respondWith(w, http.StatusOK, APITopology{
Nodes: detailed.Summaries(rc, render.Decorate(rc.Report, renderer, decorator).Nodes),
Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, filter).Nodes),
})
}
// Individual nodes.
func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) {
func handleNode(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) {
var (
vars = mux.Vars(r)
topologyID = vars["topology"]
nodeID = vars["id"]
)
// We must not lose the node during decoration. We achieve that by
// We must not lose the node during filtering. We achieve that by
// (1) rendering the report with the base renderer, without
// decoration, which gives us the node (if it exists at all), then
// (2) performing a normal decorated render of the report. If the
// filtering, which gives us the node (if it exists at all), then
// (2) performing a normal filtered render of the report. If the
// node is lost in the second step, we simply put it back.
//
// To avoid repeating the work from step (1) in step (2), we
@@ -57,11 +57,11 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render.
http.NotFound(w, r)
return
}
if decorator != nil {
nodes = render.Decorate(rc.Report, render.ConstantRenderer{Nodes: nodes}, decorator)
if decoratedNode, ok := nodes.Nodes[nodeID]; ok {
node = decoratedNode
} else { // we've lost the node during decoration; put it back
if filter != nil {
nodes = render.Render(rc.Report, render.ConstantRenderer{Nodes: nodes}, filter)
if filteredNode, ok := nodes.Nodes[nodeID]; ok {
node = filteredNode
} else { // we've lost the node during filtering; put it back
nodes.Nodes[nodeID] = node
nodes.Filtered--
}
@@ -135,12 +135,12 @@ func handleWebsocket(
log.Errorf("Error generating report: %v", err)
return
}
renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re)
renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re)
if err != nil {
log.Errorf("Error generating report: %v", err)
return
}
newTopo := detailed.Summaries(RenderContextForReporter(rep, re), render.Decorate(re, renderer, decorator).Nodes)
newTopo := detailed.Summaries(RenderContextForReporter(rep, re), render.Render(re, renderer, filter).Nodes)
diff := detailed.TopoDiff(previousTopo, newTopo)
previousTopo = newTopo

View File

@@ -65,10 +65,10 @@ func BenchmarkTopologyContainers(b *testing.B) {
func benchmarkOneTopology(b *testing.B, topologyID string) {
benchmarkRender(b, func(report report.Report) {
renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, url.Values{}, report)
renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, url.Values{}, report)
if err != nil {
b.Fatal(err)
}
render.Decorate(report, renderer, decorator)
render.Render(report, renderer, filter)
})
}

View File

@@ -15,18 +15,10 @@ import (
"github.com/weaveworks/scope/test/utils"
)
// FilterApplication is a Renderer which filters out application nodes.
func FilterApplication(r render.Renderer) render.Renderer {
return render.MakeFilter(render.IsApplication, r)
}
// FilterSystem is a Renderer which filters out system nodes.
func FilterSystem(r render.Renderer) render.Renderer {
return render.MakeFilter(render.IsSystem, r)
}
// FilterNoop does nothing.
func FilterNoop(r render.Renderer) render.Renderer { return r }
var (
filterApplication = render.AnyFilterFunc(render.IsPseudoTopology, render.IsApplication)
filterSystem = render.AnyFilterFunc(render.IsPseudoTopology, render.IsSystem)
)
func TestMapProcess2Container(t *testing.T) {
for _, input := range []testcase{
@@ -70,12 +62,10 @@ func TestContainerFilterRenderer(t *testing.T) {
// tag on of the containers in the topology and ensure
// it is filtered out correctly.
input := fixture.Report.Copy()
input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{
docker.LabelPrefix + "works.weave.role": "system",
})
have := utils.Prune(render.Decorate(input, render.ContainerWithImageNameRenderer, FilterApplication).Nodes)
have := utils.Prune(render.Render(input, render.ContainerWithImageNameRenderer, filterApplication).Nodes)
want := utils.Prune(expected.RenderedContainers.Copy())
delete(want, fixture.ClientContainerNodeID)
if !reflect.DeepEqual(want, have) {
@@ -84,7 +74,7 @@ func TestContainerFilterRenderer(t *testing.T) {
}
func TestContainerHostnameRenderer(t *testing.T) {
have := utils.Prune(render.Decorate(fixture.Report, render.ContainerHostnameRenderer, FilterNoop).Nodes)
have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, nil).Nodes)
want := utils.Prune(expected.RenderedContainerHostnames)
if !reflect.DeepEqual(want, have) {
t.Error(test.Diff(want, have))
@@ -92,7 +82,7 @@ func TestContainerHostnameRenderer(t *testing.T) {
}
func TestContainerHostnameFilterRenderer(t *testing.T) {
have := utils.Prune(render.Decorate(fixture.Report, render.ContainerHostnameRenderer, FilterSystem).Nodes)
have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, filterSystem).Nodes)
want := utils.Prune(expected.RenderedContainerHostnames.Copy())
delete(want, fixture.ClientContainerHostname)
delete(want, fixture.ServerContainerHostname)
@@ -103,7 +93,7 @@ func TestContainerHostnameFilterRenderer(t *testing.T) {
}
func TestContainerImageRenderer(t *testing.T) {
have := utils.Prune(render.Decorate(fixture.Report, render.ContainerImageRenderer, FilterNoop).Nodes)
have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, nil).Nodes)
want := utils.Prune(expected.RenderedContainerImages)
if !reflect.DeepEqual(want, have) {
t.Error(test.Diff(want, have))
@@ -111,7 +101,7 @@ func TestContainerImageRenderer(t *testing.T) {
}
func TestContainerImageFilterRenderer(t *testing.T) {
have := utils.Prune(render.Decorate(fixture.Report, render.ContainerImageRenderer, FilterSystem).Nodes)
have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, filterSystem).Nodes)
want := utils.Prune(expected.RenderedContainerHostnames.Copy())
delete(want, fixture.ClientContainerHostname)
delete(want, fixture.ServerContainerHostname)

View File

@@ -106,22 +106,6 @@ func MakeFilterPseudo(f FilterFunc, r Renderer) Renderer {
}
}
// MakeFilterDecorator makes a decorator that filters out non-pseudo nodes
// which match the predicate.
func MakeFilterDecorator(f FilterFunc) Decorator {
return func(renderer Renderer) Renderer {
return MakeFilter(f, renderer)
}
}
// MakeFilterPseudoDecorator makes a decorator that filters out all nodes
// (including pseudo nodes) which match the predicate.
func MakeFilterPseudoDecorator(f FilterFunc) Decorator {
return func(renderer Renderer) Renderer {
return MakeFilterPseudo(f, renderer)
}
}
// Render implements Renderer
func (f Filter) Render(rpt report.Report) Nodes {
return f.render(rpt)

View File

@@ -9,13 +9,8 @@ import (
"github.com/weaveworks/scope/test/reflect"
)
func isNotBar(renderer render.Renderer) render.Renderer {
return &render.Filter{
FilterFunc: func(node report.Node) bool {
return node.ID != "bar"
},
Renderer: renderer,
}
func isNotBar(node report.Node) bool {
return node.ID != "bar"
}
func TestFilterRender(t *testing.T) {
@@ -25,7 +20,7 @@ func TestFilterRender(t *testing.T) {
"baz": report.MakeNode("baz"),
}}
have := report.MakeIDList()
for id := range render.Decorate(report.MakeReport(), renderer, render.FilterUnconnected).Nodes {
for id := range render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes {
have = have.Add(id)
}
want := report.MakeIDList("foo", "bar")
@@ -41,7 +36,7 @@ func TestFilterRender2(t *testing.T) {
"bar": report.MakeNode("bar").WithAdjacent("foo"),
"baz": report.MakeNode("baz"),
}}
have := render.Decorate(report.MakeReport(), renderer, isNotBar).Nodes
have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes
if have["foo"].Adjacency.Contains("bar") {
t.Error("adjacencies for removed nodes should have been removed")
}
@@ -57,16 +52,8 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) {
"baz": report.MakeNode("baz").WithTopology(render.Pseudo),
}
renderer := mockRenderer{Nodes: nodes}
filter := func(renderer render.Renderer) render.Renderer {
return &render.Filter{
FilterFunc: func(node report.Node) bool {
return true
},
Renderer: renderer,
}
}
want := nodes
have := render.Decorate(report.MakeReport(), renderer, filter).Nodes
have := render.Render(report.MakeReport(), renderer, nil).Nodes
if !reflect.DeepEqual(want, have) {
t.Error(test.Diff(want, have))
}
@@ -77,7 +64,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) {
"bar": report.MakeNode("bar").WithAdjacent("baz"),
"baz": report.MakeNode("baz").WithTopology(render.Pseudo),
}}
have := render.Decorate(report.MakeReport(), renderer, isNotBar).Nodes
have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes
if _, ok := have["baz"]; ok {
t.Error("expected the unconnected pseudonode baz to have been removed")
}
@@ -88,7 +75,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) {
"bar": report.MakeNode("bar").WithAdjacent("foo"),
"baz": report.MakeNode("baz").WithTopology(render.Pseudo).WithAdjacent("bar"),
}}
have := render.Decorate(report.MakeReport(), renderer, isNotBar).Nodes
have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes
if _, ok := have["baz"]; ok {
t.Error("expected the unconnected pseudonode baz to have been removed")
}
@@ -102,7 +89,7 @@ func TestFilterUnconnectedSelf(t *testing.T) {
"foo": report.MakeNode("foo").WithAdjacent("foo"),
}
renderer := mockRenderer{Nodes: nodes}
have := render.Decorate(report.MakeReport(), renderer, render.FilterUnconnected).Nodes
have := render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes
if len(have) > 0 {
t.Error("expected node only connected to self to be removed")
}

View File

@@ -20,9 +20,7 @@ func TestPodRenderer(t *testing.T) {
}
}
func filterNonKubeSystem(renderer render.Renderer) render.Renderer {
return render.MakeFilter(render.Complement(render.IsNamespace("kube-system")), renderer)
}
var filterNonKubeSystem = render.Complement(render.IsNamespace("kube-system"))
func TestPodFilterRenderer(t *testing.T) {
// tag on containers or pod namespace in the topology and ensure
@@ -32,7 +30,7 @@ func TestPodFilterRenderer(t *testing.T) {
kubernetes.Namespace: "kube-system",
})
have := utils.Prune(render.Decorate(input, render.PodRenderer, filterNonKubeSystem).Nodes)
have := utils.Prune(render.Render(input, render.PodRenderer, filterNonKubeSystem).Nodes)
want := utils.Prune(expected.RenderedPods.Copy())
delete(want, fixture.ClientPodNodeID)
if !reflect.DeepEqual(want, have) {
@@ -56,7 +54,7 @@ func TestPodServiceFilterRenderer(t *testing.T) {
kubernetes.Namespace: "kube-system",
})
have := utils.Prune(render.Decorate(input, render.PodServiceRenderer, filterNonKubeSystem).Nodes)
have := utils.Prune(render.Render(input, render.PodServiceRenderer, filterNonKubeSystem).Nodes)
want := utils.Prune(expected.RenderedPodServices.Copy())
delete(want, fixture.ServiceNodeID)
delete(want, render.IncomingInternetID)

View File

@@ -29,10 +29,10 @@ func (r Nodes) Merge(o Nodes) Nodes {
}
}
// Decorate renders the report with a decorated renderer
func Decorate(rpt report.Report, renderer Renderer, dct Decorator) Nodes {
if dct != nil {
renderer = dct(renderer)
// Render renders the report and then applies the filter
func Render(rpt report.Report, renderer Renderer, filter FilterFunc) Nodes {
if filter != nil {
renderer = MakeFilterPseudo(filter, renderer)
}
return renderer.Render(rpt)
}
@@ -119,19 +119,6 @@ func (m Map) Render(rpt report.Report) Nodes {
return Nodes{Nodes: output}
}
// Decorator transforms one renderer to another. e.g. Filters.
type Decorator func(Renderer) Renderer
// ComposeDecorators composes decorators into one.
func ComposeDecorators(decorators ...Decorator) Decorator {
return func(r Renderer) Renderer {
for _, decorator := range decorators {
r = decorator(r)
}
return r
}
}
func propagateLatest(key string, from, to report.Node) report.Node {
if value, timestamp, ok := from.Latest.LookupEntry(key); ok {
to.Latest = to.Latest.Set(key, timestamp, value)