From 7bbded9e84bc440a9c908cea7e3185e090354272 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 22 Nov 2017 18:44:49 +0000 Subject: [PATCH] refactor(ish): introduce Tranformers so we can generalise the filter step in render.Render et al. That will allow us to apply whole-topology filters in that step. --- app/api_topologies.go | 12 ++++++------ app/api_topologies_test.go | 8 ++++---- app/api_topology.go | 20 +++++++++----------- render/container_test.go | 4 ++-- render/filters.go | 6 +++--- render/filters_test.go | 12 ++++++------ render/render.go | 23 ++++++++++++++++++----- 7 files changed, 48 insertions(+), 37 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 53405196a..a7f3d9328 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -496,13 +496,13 @@ func (r *Registry) renderTopologies(rpt report.Report, req *http.Request) []APIT return updateFilters(rpt, topologies) } -func computeStats(rpt report.Report, renderer render.Renderer, filter render.FilterFunc) topologyStats { +func computeStats(rpt report.Report, renderer render.Renderer, transformer render.Transformer) topologyStats { var ( nodes int realNodes int edges int ) - r := render.Render(rpt, renderer, filter) + r := render.Render(rpt, renderer, transformer) for _, n := range r.Nodes { nodes++ if n.Topology != render.Pseudo { @@ -519,7 +519,7 @@ func computeStats(rpt report.Report, renderer render.Renderer, filter render.Fil } // RendererForTopology .. -func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.FilterFunc, error) { +func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.Transformer, error) { topology, ok := r.get(topologyID) if !ok { return nil, nil, fmt.Errorf("topology not found: %s", topologyID) @@ -528,7 +528,7 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt if len(values) == 0 { // Do not apply filtering if no options where provided - return topology.renderer, nil, nil + return topology.renderer, render.Transformers(nil), nil } var filters []render.FilterFunc @@ -541,7 +541,7 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt if len(filters) > 0 { return topology.renderer, render.ComposeFilterFuncs(filters...), nil } - return topology.renderer, nil, nil + return topology.renderer, render.Transformers(nil), nil } type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) @@ -552,7 +552,7 @@ func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { } } -type rendererHandler func(context.Context, render.Renderer, render.FilterFunc, report.RenderContext, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.Transformer, 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) { diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index a238adcf4..2dbfeb6a5 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -164,14 +164,14 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det var ( topologyRegistry = app.MakeRegistry() - filter render.FilterFunc + filterFunc render.FilterFunc ) if exclude == true { - filter = render.DoesNotHaveLabel(fixture.TestLabelKey2, fixture.ApplicationLabelValue2) + filterFunc = render.DoesNotHaveLabel(fixture.TestLabelKey2, fixture.ApplicationLabelValue2) } else { - filter = render.HasLabel(fixture.TestLabelKey1, fixture.ApplicationLabelValue1) + filterFunc = render.HasLabel(fixture.TestLabelKey1, fixture.ApplicationLabelValue1) } - option := app.MakeAPITopologyOption(customAPITopologyOptionFilterID, "title", filter, false) + option := app.MakeAPITopologyOption(customAPITopologyOptionFilterID, "title", filterFunc, false) topologyRegistry.AddContainerFilters(option) urlvalues := url.Values{} diff --git a/app/api_topology.go b/app/api_topology.go index e0a3ded16..145b5aa85 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,14 +29,14 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, filter).Nodes), + Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, transformer).Nodes), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] @@ -53,14 +53,12 @@ func handleNode(ctx context.Context, renderer render.Renderer, filter render.Fil http.NotFound(w, r) return } - if filter != nil { - nodes = filter.Apply(nodes) - 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-- - } + nodes = transformer.Transform(nodes) + 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-- } respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, nodes.Nodes, node)}) } diff --git a/render/container_test.go b/render/container_test.go index 9ec27887c..cc0dd4148 100644 --- a/render/container_test.go +++ b/render/container_test.go @@ -74,7 +74,7 @@ func TestContainerFilterRenderer(t *testing.T) { } func TestContainerHostnameRenderer(t *testing.T) { - have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, nil).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, render.Transformers(nil)).Nodes) want := utils.Prune(expected.RenderedContainerHostnames) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -93,7 +93,7 @@ func TestContainerHostnameFilterRenderer(t *testing.T) { } func TestContainerImageRenderer(t *testing.T) { - have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, nil).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, render.Transformers(nil)).Nodes) want := utils.Prune(expected.RenderedContainerImages) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) diff --git a/render/filters.go b/render/filters.go index 16b11971f..7093c84bf 100644 --- a/render/filters.go +++ b/render/filters.go @@ -60,8 +60,8 @@ func Complement(f FilterFunc) FilterFunc { return func(node report.Node) bool { return !f(node) } } -// Apply applies the filter to all nodes -func (f FilterFunc) Apply(nodes Nodes) Nodes { +// Transform applies the filter to all nodes +func (f FilterFunc) Transform(nodes Nodes) Nodes { output := report.Nodes{} inDegrees := map[string]int{} filtered := nodes.Filtered @@ -128,7 +128,7 @@ func MakeFilterPseudo(f FilterFunc, r Renderer) Renderer { // Render implements Renderer func (f Filter) Render(rpt report.Report) Nodes { - return f.FilterFunc.Apply(f.Renderer.Render(rpt)) + return f.FilterFunc.Transform(f.Renderer.Render(rpt)) } // IsConnectedMark is the key added to Node.Metadata by diff --git a/render/filters_test.go b/render/filters_test.go index 5854641fd..8def19720 100644 --- a/render/filters_test.go +++ b/render/filters_test.go @@ -20,7 +20,7 @@ func TestFilterRender(t *testing.T) { "baz": report.MakeNode("baz"), }} have := report.MakeIDList() - for id := range render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes { + for id := range render.Render(report.MakeReport(), render.ColorConnected(renderer), render.FilterFunc(render.IsConnected)).Nodes { have = have.Add(id) } want := report.MakeIDList("foo", "bar") @@ -36,7 +36,7 @@ func TestFilterRender2(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("foo"), "baz": report.MakeNode("baz"), }} - have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, render.FilterFunc(isNotBar)).Nodes if have["foo"].Adjacency.Contains("bar") { t.Error("adjacencies for removed nodes should have been removed") } @@ -53,7 +53,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { } renderer := mockRenderer{Nodes: nodes} want := nodes - have := render.Render(report.MakeReport(), renderer, nil).Nodes + have := render.Render(report.MakeReport(), renderer, render.Transformers(nil)).Nodes if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } @@ -64,7 +64,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("baz"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo), }} - have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, render.FilterFunc(isNotBar)).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -75,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.Render(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, render.FilterFunc(isNotBar)).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -89,7 +89,7 @@ func TestFilterUnconnectedSelf(t *testing.T) { "foo": report.MakeNode("foo").WithAdjacent("foo"), } renderer := mockRenderer{Nodes: nodes} - have := render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes + have := render.Render(report.MakeReport(), render.ColorConnected(renderer), render.FilterFunc(render.IsConnected)).Nodes if len(have) > 0 { t.Error("expected node only connected to self to be removed") } diff --git a/render/render.go b/render/render.go index bcf3e2495..05901aa52 100644 --- a/render/render.go +++ b/render/render.go @@ -29,15 +29,28 @@ func (r Nodes) Merge(o Nodes) Nodes { } } -// Render renders the report and then applies the filter -func Render(rpt report.Report, renderer Renderer, filter FilterFunc) Nodes { - nodes := renderer.Render(rpt) - if filter != nil { - nodes = filter.Apply(nodes) +// Transformer is something that transforms one set of Nodes to +// another set of Nodes. +type Transformer interface { + Transform(nodes Nodes) Nodes +} + +// Transformers is a composition of Transformers +type Transformers []Transformer + +// Transform implements Transformer +func (ts Transformers) Transform(nodes Nodes) Nodes { + for _, t := range ts { + nodes = t.Transform(nodes) } return nodes } +// Render renders the report and then transforms it +func Render(rpt report.Report, renderer Renderer, transformer Transformer) Nodes { + return transformer.Transform(renderer.Render(rpt)) +} + // Reduce renderer is a Renderer which merges together the output of several // other renderers. type Reduce []Renderer