From 0e8ea6363aaae19d780e60ac43491f155a420bda Mon Sep 17 00:00:00 2001 From: jolo Date: Sat, 14 Jan 2017 02:11:59 +0100 Subject: [PATCH 1/7] Support vertical graph layouts --- api/graph.go | 10 ++++++++-- cmd/graph.go | 6 +++++- deb/graph.go | 35 +++++++++++++++++++++++++---------- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/api/graph.go b/api/graph.go index 66ed4444..847f26a6 100644 --- a/api/graph.go +++ b/api/graph.go @@ -11,15 +11,21 @@ import ( "os/exec" ) -// GET /api/graph.:ext +// GET /api/graph.:ext/:vertical func apiGraph(c *gin.Context) { var ( err error output []byte + vertical bool = false ) ext := c.Params.ByName("ext") + // TODO: api is untested! + if c.Params.ByName("vertical") == "vertical" { + vertical = true + } + factory := context.CollectionFactory() factory.RemoteRepoCollection().RLock() @@ -31,7 +37,7 @@ func apiGraph(c *gin.Context) { factory.PublishedRepoCollection().RLock() defer factory.PublishedRepoCollection().RUnlock() - graph, err := deb.BuildGraph(factory) + graph, err := deb.BuildGraph(factory, vertical) if err != nil { c.JSON(500, err) return diff --git a/cmd/graph.go b/cmd/graph.go index eb35559f..825aa82c 100644 --- a/cmd/graph.go +++ b/cmd/graph.go @@ -21,8 +21,11 @@ func aptlyGraph(cmd *commander.Command, args []string) error { return commander.ErrCommandError } + vertical := context.Flags().Lookup("vertical").Value.Get().(bool) + fmt.Printf("Generating graph...\n") - graph, err := deb.BuildGraph(context.CollectionFactory()) + graph, err := deb.BuildGraph(context.CollectionFactory(), vertical) + if err != nil { return err } @@ -108,6 +111,7 @@ Example: cmd.Flag.String("format", "png", "render graph to specified format (png, svg, pdf, etc.)") cmd.Flag.String("output", "", "specify output filename, default is to open result in viewer") + cmd.Flag.Bool("vertical", false, "try to achieve a more vertical graph layout") return cmd } diff --git a/deb/graph.go b/deb/graph.go index 37abd692..6b0579eb 100644 --- a/deb/graph.go +++ b/deb/graph.go @@ -7,13 +7,26 @@ import ( ) // BuildGraph generates graph contents from aptly object database -func BuildGraph(collectionFactory *CollectionFactory) (gographviz.Interface, error) { +func BuildGraph(collectionFactory *CollectionFactory, vertical bool) (gographviz.Interface, error) { var err error graph := gographviz.NewEscape() graph.SetDir(true) graph.SetName("aptly") + var labelStart string + var labelEnd string + + if vertical { + graph.AddAttr("aptly", "rankdir", "LR") + labelStart = "" + labelEnd = "" + } else { + graph.AddAttr("aptly", "rankdir", "TB") + labelStart = "{" + labelEnd = "}" + } + existingNodes := map[string]bool{} err = collectionFactory.RemoteRepoCollection().ForEach(func(repo *RemoteRepo) error { @@ -26,9 +39,9 @@ func BuildGraph(collectionFactory *CollectionFactory) (gographviz.Interface, err "shape": "Mrecord", "style": "filled", "fillcolor": "darkgoldenrod1", - "label": fmt.Sprintf("{Mirror %s|url: %s|dist: %s|comp: %s|arch: %s|pkgs: %d}", - repo.Name, repo.ArchiveRoot, repo.Distribution, strings.Join(repo.Components, ", "), - strings.Join(repo.Architectures, ", "), repo.NumPackages()), + "label": fmt.Sprintf("%sMirror %s|url: %s|dist: %s|comp: %s|arch: %s|pkgs: %d%s", labelStart, repo.Name, repo.ArchiveRoot, + repo.Distribution, strings.Join(repo.Components, ", "), + strings.Join(repo.Architectures, ", "), repo.NumPackages(), labelEnd), }) existingNodes[repo.UUID] = true return nil @@ -48,8 +61,8 @@ func BuildGraph(collectionFactory *CollectionFactory) (gographviz.Interface, err "shape": "Mrecord", "style": "filled", "fillcolor": "mediumseagreen", - "label": fmt.Sprintf("{Repo %s|comment: %s|pkgs: %d}", - repo.Name, repo.Comment, repo.NumPackages()), + "label": fmt.Sprintf("%sRepo %s|comment: %s|pkgs: %d%s", labelStart, + repo.Name, repo.Comment, repo.NumPackages(), labelEnd), }) existingNodes[repo.UUID] = true return nil @@ -72,14 +85,15 @@ func BuildGraph(collectionFactory *CollectionFactory) (gographviz.Interface, err description := snapshot.Description if snapshot.SourceKind == "repo" { - description = "Snapshot from repo" + description = "Snapshot from repo\n" } graph.AddNode("aptly", snapshot.UUID, map[string]string{ "shape": "Mrecord", "style": "filled", "fillcolor": "cadetblue1", - "label": fmt.Sprintf("{Snapshot %s|%s|pkgs: %d}", snapshot.Name, description, snapshot.NumPackages()), + "label": fmt.Sprintf("%sSnapshot %s|%s|pkgs: %d%s", labelStart, + snapshot.Name, description, snapshot.NumPackages(), labelEnd), }) if snapshot.SourceKind == "repo" || snapshot.SourceKind == "local" || snapshot.SourceKind == "snapshot" { @@ -102,8 +116,9 @@ func BuildGraph(collectionFactory *CollectionFactory) (gographviz.Interface, err "shape": "Mrecord", "style": "filled", "fillcolor": "darkolivegreen1", - "label": fmt.Sprintf("{Published %s/%s|comp: %s|arch: %s}", repo.Prefix, repo.Distribution, - strings.Join(repo.Components(), " "), strings.Join(repo.Architectures, ", ")), + "label": fmt.Sprintf("%sPublished %s/%s|comp: %s|arch: %s%s", labelStart, + repo.Prefix, repo.Distribution, strings.Join(repo.Components(), " "), + strings.Join(repo.Architectures, ", "), labelEnd), }) for _, uuid := range repo.Sources { From 91561b40f65f46e46d277f0dc68328a1912fe74b Mon Sep 17 00:00:00 2001 From: jolo Date: Mon, 16 Jan 2017 22:13:01 +0100 Subject: [PATCH 2/7] Change 'vertical' argument to a more generic 'layout', fix api --- api/graph.go | 12 ++++-------- cmd/graph.go | 6 +++--- deb/graph.go | 20 +++++++++++--------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/api/graph.go b/api/graph.go index 847f26a6..8babcd2d 100644 --- a/api/graph.go +++ b/api/graph.go @@ -11,20 +11,16 @@ import ( "os/exec" ) -// GET /api/graph.:ext/:vertical +// GET /api/graph.:ext?layout=[vertical|horizontal(default)] func apiGraph(c *gin.Context) { var ( err error output []byte - vertical bool = false ) ext := c.Params.ByName("ext") - - // TODO: api is untested! - if c.Params.ByName("vertical") == "vertical" { - vertical = true - } + layout := c.Request.URL.Query().Get("layout") + fmt.Printf("Layout is: "+layout) factory := context.CollectionFactory() @@ -37,7 +33,7 @@ func apiGraph(c *gin.Context) { factory.PublishedRepoCollection().RLock() defer factory.PublishedRepoCollection().RUnlock() - graph, err := deb.BuildGraph(factory, vertical) + graph, err := deb.BuildGraph(factory, layout) if err != nil { c.JSON(500, err) return diff --git a/cmd/graph.go b/cmd/graph.go index 825aa82c..45d836b0 100644 --- a/cmd/graph.go +++ b/cmd/graph.go @@ -21,10 +21,10 @@ func aptlyGraph(cmd *commander.Command, args []string) error { return commander.ErrCommandError } - vertical := context.Flags().Lookup("vertical").Value.Get().(bool) + layout := context.Flags().Lookup("layout").Value.String() fmt.Printf("Generating graph...\n") - graph, err := deb.BuildGraph(context.CollectionFactory(), vertical) + graph, err := deb.BuildGraph(context.CollectionFactory(), layout) if err != nil { return err @@ -111,7 +111,7 @@ Example: cmd.Flag.String("format", "png", "render graph to specified format (png, svg, pdf, etc.)") cmd.Flag.String("output", "", "specify output filename, default is to open result in viewer") - cmd.Flag.Bool("vertical", false, "try to achieve a more vertical graph layout") + cmd.Flag.String("layout", "horizontal", "create a more 'vertical' or a more 'horizontal' graph layout") return cmd } diff --git a/deb/graph.go b/deb/graph.go index 6b0579eb..bc8623e3 100644 --- a/deb/graph.go +++ b/deb/graph.go @@ -7,7 +7,7 @@ import ( ) // BuildGraph generates graph contents from aptly object database -func BuildGraph(collectionFactory *CollectionFactory, vertical bool) (gographviz.Interface, error) { +func BuildGraph(collectionFactory *CollectionFactory, layout string) (gographviz.Interface, error) { var err error graph := gographviz.NewEscape() @@ -17,14 +17,16 @@ func BuildGraph(collectionFactory *CollectionFactory, vertical bool) (gographviz var labelStart string var labelEnd string - if vertical { - graph.AddAttr("aptly", "rankdir", "LR") - labelStart = "" - labelEnd = "" - } else { - graph.AddAttr("aptly", "rankdir", "TB") - labelStart = "{" - labelEnd = "}" + switch layout { + case "vertical": + graph.AddAttr("aptly", "rankdir", "LR") + labelStart = "" + labelEnd = "" + case "horizontal": + fallthrough + default: + labelStart = "{" + labelEnd = "}" } existingNodes := map[string]bool{} From 43e64987132a1d5057966b55df71f7823d97239f Mon Sep 17 00:00:00 2001 From: jolo Date: Mon, 16 Jan 2017 22:39:47 +0100 Subject: [PATCH 3/7] Add me to authors --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index dacec23e..f4db9e5f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -22,3 +22,4 @@ List of contributors, in chronological order: * Phil Frost (https://github.com/bitglue) * Benoit Foucher (https://github.com/bentoi) * Geoffrey Thomas (https://github.com/geofft) +* Johannes Layher (https://github.com/jola5) From 96948d6f18fe9ecc6c59424acffdb78765b83ac5 Mon Sep 17 00:00:00 2001 From: jolo Date: Mon, 16 Jan 2017 23:31:45 +0100 Subject: [PATCH 4/7] Basic test of graph layout --- system/lib.py | 8 ++++++++ system/t12_api/graph.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/system/lib.py b/system/lib.py index d0240f6a..928109f2 100644 --- a/system/lib.py +++ b/system/lib.py @@ -270,6 +270,14 @@ class BaseTest(object): if a != b: self.verify_match(a, b, match_prepare=pprint.pformat) + def check_ge(self, a, b): + if not a >= b: + raise Exception("%s is not greater or equal to %s" % (a, b)) + + def check_gt(self, a, b): + if not a > b: + raise Exception("%s is not greater to %s" % (a, b)) + def check_in(self, item, l): if not item in l: raise Exception("item %r not in %r", item, l) diff --git a/system/t12_api/graph.py b/system/t12_api/graph.py index 2f2a9f10..0bd1e3a4 100644 --- a/system/t12_api/graph.py +++ b/system/t12_api/graph.py @@ -1,4 +1,5 @@ from api_lib import APITest +import xml.etree.ElementTree as ET class GraphAPITest(APITest): @@ -15,3 +16,20 @@ class GraphAPITest(APITest): resp = self.get("/api/graph.svg") self.check_equal(resp.headers["Content-Type"], "image/svg+xml") self.check_equal(resp.content[:4], ' Date: Tue, 17 Jan 2017 01:02:40 +0100 Subject: [PATCH 5/7] Disable tests failing due to inappropriate test data --- system/t12_api/graph.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/t12_api/graph.py b/system/t12_api/graph.py index 0bd1e3a4..7efeafd5 100644 --- a/system/t12_api/graph.py +++ b/system/t12_api/graph.py @@ -31,5 +31,5 @@ class GraphAPITest(APITest): svgVWidth = ET.fromstring(vertical).get('width').replace("pt","") svgVHeight = ET.fromstring(vertical).get('height').replace("pt","") - self.check_gt(svgHWidth, svgVWidth) - self.check_gt(svgVHeight, svgHHeight) + #self.check_gt(svgHWidth, svgVWidth) + #self.check_gt(svgVHeight, svgHHeight) From b0ab39e07f4332b0a25fbed9340589c1130902d5 Mon Sep 17 00:00:00 2001 From: jolo Date: Fri, 20 Jan 2017 01:40:02 +0100 Subject: [PATCH 6/7] Manually undo unintended changes --- api/graph.go | 1 - deb/graph.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/graph.go b/api/graph.go index 24f09668..5c3b8dc6 100644 --- a/api/graph.go +++ b/api/graph.go @@ -20,7 +20,6 @@ func apiGraph(c *gin.Context) { ext := c.Params.ByName("ext") layout := c.Request.URL.Query().Get("layout") - fmt.Printf("Layout is: "+layout) factory := context.CollectionFactory() diff --git a/deb/graph.go b/deb/graph.go index bc8623e3..088f9953 100644 --- a/deb/graph.go +++ b/deb/graph.go @@ -87,7 +87,7 @@ func BuildGraph(collectionFactory *CollectionFactory, layout string) (gographviz description := snapshot.Description if snapshot.SourceKind == "repo" { - description = "Snapshot from repo\n" + description = "Snapshot from repo" } graph.AddNode("aptly", snapshot.UUID, map[string]string{ From 92116072c2be72f0008d66356e74876b941e5e26 Mon Sep 17 00:00:00 2001 From: jolo Date: Fri, 20 Jan 2017 01:26:11 +0100 Subject: [PATCH 7/7] Fix and enable broken graph layout tests --- system/t12_api/graph.py | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/system/t12_api/graph.py b/system/t12_api/graph.py index 70eec6de..bf84ec4c 100644 --- a/system/t12_api/graph.py +++ b/system/t12_api/graph.py @@ -21,19 +21,27 @@ class GraphAPITest(APITest): self.check_equal(resp.headers["Content-Type"], "text/plain; charset=utf-8") self.check_equal(resp.content[:13], 'digraph aptly') - # make sure our default layout is horizontal - default = self.get("/api/graph.svg").content - horizontal = self.get("/api/graph.svg?layout=horizontal").content - self.check_equal(default, horizontal) - # basic test of layout: - # horizontal should be wider than vertical - # vertical should be higher than horizontal - vertical = self.get("/api/graph.svg?layout=vertical").content - svgHWidth = ET.fromstring(horizontal).get('width').replace("pt","") - svgHHeight = ET.fromstring(horizontal).get('height').replace("pt","") - svgVWidth = ET.fromstring(vertical).get('width').replace("pt","") - svgVHeight = ET.fromstring(vertical).get('height').replace("pt","") + # horizontal should be wider than vertical + # vertical should be higher than horizontal + # for this to work we need at couple of repos + tempRepos = [self.random_name() for r in range(3)] + for repo in tempRepos: + self.check_equal(self.post("/api/repos", json={"Name": repo, "Comment": "graph test repo"}).status_code, 201) - #self.check_gt(svgHWidth, svgVWidth) - #self.check_gt(svgVHeight, svgHHeight) \ No newline at end of file + horizontal = self.get("/api/graph.svg?layout=horizontal").content + vertical = self.get("/api/graph.svg?layout=vertical").content + horizontalWidth = int(ET.fromstring(horizontal).get('width').replace("pt","")) + horizontalHeight = int(ET.fromstring(horizontal).get('height').replace("pt","")) + verticalWidth = int(ET.fromstring(vertical).get('width').replace("pt","")) + verticalHeight = int(ET.fromstring(vertical).get('height').replace("pt","")) + + self.check_gt(horizontalWidth, verticalWidth) + self.check_gt(verticalHeight, horizontalHeight) + + # make sure our default layout is horizontal + self.check_equal(horizontal, self.get("/api/graph.svg").content) + + # remove the repos again + for repo in tempRepos: + self.check_equal(self.delete("/api/repos/" + repo, params={"force": "1"}).status_code, 200)