Produce a DGML graph from the ApiPort data #695

AlexGhiondea · 2018-07-30T22:35:39Z

Create a mechanism to create a DGLM graph from the ApiPort data that will show the dependencies of the input assemblies.

This is how the DGML document looks like. Each node has 2 indexes (one for itself in isolation and one that describes the portability of the references).

/cc @twsouthwick @conniey

AlexGhiondea · 2018-08-21T17:53:52Z

Ping?

conniey · 2018-09-08T20:37:14Z

src/lib/Microsoft.Fx.Portability.MetadataReader/DependencyFinderEngineHelper.cs

@@ -38,6 +40,61 @@ public DependencyFinderEngineHelper(IDependencyFilter assemblyFilter, MetadataRe
            _currentAssemblyName = _reader.GetString(assemblyDefinition.Name);
        }

+        private IList<AssemblyReferenceInformation> ComputeAssemblyReferences(MetadataReader metadataReader)
+        {
+            List<AssemblyReferenceInformation> refs = new List<AssemblyReferenceInformation>();


Maybe it's a VS code fix, but we usually use var when the types are obvious.
Same for the other variables below.

I am not a fan of style, but I will change to match the project style!

conniey · 2018-09-08T20:43:32Z

src/lib/Microsoft.Fx.Portability.MetadataReader/DependencyFinderEngineHelper.cs

+            return refs;
+        }
+
+        private static string FormatPublicKeyToken(MetadataReader metadataReader, BlobHandle handle)


We have extension methods that provide this: MetadataReaderExtensions.FormatAssemblyInfo

Can you remove this?

conniey · 2018-09-08T20:44:38Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceGraph.cs

+
+namespace Microsoft.Fx.Portability.Reports.DGML
+{
+    class ReferenceGraph


@twsouthwick I thought we had stylecop analysers that forced us to specify the access modifier as public, internal, etc?

I'll go and modify the ruleset. :)

conniey · 2018-09-08T20:45:22Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/DGMLOutputWriter.cs

+            FileExtension = ".dgml"
+        };
+
+        private DGMLManager dgml = new DGMLManager();


nit: private readonly

conniey · 2018-09-08T20:46:01Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/DGMLManager.cs

+    {
+        private readonly Dictionary<string, Guid> _nodesDictionary = new Dictionary<string, Guid>();
+
+        private XElement nodes;


nit: private readonly

conniey · 2018-09-08T20:50:20Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceGraph.cs

+                    var refNode = rg.GetOrAddNodeForAssembly(new ReferenceNode(reference.ToString()));
+
+                    // if the reference is missing, flag it as such.
+                    if (unresolvedAssemblies.Contains(reference.ToString()))


refNode.IsMissing = unresolvedAssemblies.Contains(reference.ToString());

conniey · 2018-09-08T20:52:46Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceGraph.cs

+{
+    class ReferenceGraph
+    {
+        public Dictionary<ReferenceNode, ReferenceNode> Nodes { get; set; }


Is this Nodes member set else where? If not, we can make this:
public Dictionary<ReferenceNode, ReferenceNode> Nodes { get; }

conniey · 2018-09-08T20:54:13Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceNode.cs

+
+        public List<TargetUsageInfo> UsageData { get; set; }
+
+        public string Assembly { get; set; }


public string Assembly { get; }
and same with any other members below that are only set in the constructor

conniey · 2018-09-08T20:55:42Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceNode.cs

+        public ReferenceNode(string AssemblyName, bool unresolved = false)
+        {
+            Assembly = AssemblyName;
+            this.Unresolved = unresolved;


Unnecessary this. and naming of AssemblyName to assemblyName

conniey · 2018-09-08T20:56:55Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceNode.cs

+        {
+            Assembly = AssemblyName;
+            this.Unresolved = unresolved;
+            Nodes = new HashSet<ReferenceNode>();


Do we also need to pass in your ReferenceNodeComparer for the HashSet? Might be better to have a ReferenceNodeComparer.Instance so we don't have to create multiple instances of this comparer.

conniey · 2018-09-08T20:59:03Z

I apologise for the delayed response. I don't get emails from github for some reason. Thanks for this! Excited to have another ReportFormatter

AlexGhiondea · 2018-09-11T04:11:23Z

Thanks for the review @conniey! I will try and address the comments this week!

twsouthwick

A couple comments, but I need to understand why the breaking change on the IReportWriter. This will cause the change to have some ripple effects on the service

src/lib/Microsoft.Fx.Portability.MetadataReader/DependencyFinderEngineHelper.cs

twsouthwick · 2018-09-20T00:02:12Z

src/lib/Microsoft.Fx.Portability.MetadataReader/DependencyFinderEngineHelper.cs

+            return refs;
+        }
+
+        private static string FormatPublicKeyToken(MetadataReader metadataReader, BlobHandle handle)


Can you remove this?

twsouthwick · 2018-09-20T00:03:51Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/DGMLManager.cs

@@ -0,0 +1,131 @@
+// Copyright (c) Microsoft. All rights reserved.


It might be easier with the public class below. Construct a request and a response and check if outputs the expectedc structure. Usually easiest to create a simple project, run the tool, and dump those values

twsouthwick · 2018-09-20T00:05:40Z

src/lib/Microsoft.Fx.Portability.Reports.Excel/ExcelReportWriter.cs

@@ -26,7 +26,7 @@ public ExcelReportWriter(ITargetMapper targetMapper)

        public ResultFormatInformation Format { get { return _formatInformation; } }

-        public void WriteStream(Stream stream, AnalyzeResponse response)
+        public void WriteStream(Stream stream, AnalyzeResponse response, AnalyzeRequest request)


What information do you need from the AnalyzeRequest? This may make it difficult to incorporate into the service

I use that information to create the assembly reference graph. When we first analyze the assemblies (before we send them back) we capture enough information about their dependencies to allow us to generate the DGML graph.

That information can be computed before we send the request as well -- and we can probably stash it somewhere in order for the information to survive the round-trip.

Keep in mind that the writers are inserted as output formatters in the MVC pipeline, so it just gets the output. If you can find a way to have it go through the AnalyzeResponse, that would be ideal, as I'm not sure how we could easily get the request without some jumping through hoops (maybe accessing the HttpContext or something, but I'm not as familiar with ASP.NET as ASP.NET Core). You can annotate it with [JsonIgnore] so it won't be serialized down if application/json is requested

In order to pass the required information to the report generator we are going to pass the AnalyzeRequest to the report generator.

…embly

This is used to display the data as-is.

…ed (available and not) This is the information required to add information about the references into the data.

twsouthwick · 2018-09-20T22:41:34Z

src/lib/Microsoft.Fx.Portability.MetadataReader/DependencyFinderEngineHelper.cs

@@ -6,6 +6,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection.Metadata;
+using System.Security.Cryptography;


Do you still need this?

I will double check and fix this and the other comment from Connie! Thanks a lot!!!

conniey · 2018-09-20T22:55:19Z

src/lib/Microsoft.Fx.Portability.Reports.DGML/ReferenceGraph.cs

+
+        public ReferenceGraph()
+        {
+            Nodes = new Dictionary<ReferenceNode, ReferenceNode>(new ReferenceNodeComparer());


ReferenceNodeComparer.Instance

…e unchanged.

AlexGhiondea · 2018-09-20T23:26:28Z

I have resolved the last issues you mentioned. Thanks for the review!

I have rebased some of the changes away to keep the history in a better spot (ie. remove all the 'address feedback' commits).

twsouthwick · 2018-09-25T17:43:47Z

Thanks @AlexGhiondea!

AlexGhiondea requested review from twsouthwick and conniey July 31, 2018 03:57

AlexGhiondea force-pushed the PrepareForPR branch from c707ddf to 72a18d5 Compare August 1, 2018 03:21

conniey changed the base branch from master to dev September 8, 2018 20:33

conniey suggested changes Sep 8, 2018

View reviewed changes

twsouthwick suggested changes Sep 20, 2018

View reviewed changes

AlexGhiondea and others added 5 commits September 20, 2018 11:16

Add project for generating a DGML file from the results.

ccd6264

In order to pass the required information to the report generator we are going to pass the AnalyzeRequest to the report generator.

Capture information about the referenced assemblies for each user ass…

0840c58

…embly

Initial prototype for producing the DGML report

a36e9be

Introduce a graph representation of the data

5fda4cf

This is used to display the data as-is.

Add methods to provide access to the raw number of APIs that are call…

0d55189

…ed (available and not) This is the information required to add information about the references into the data.

AlexGhiondea force-pushed the PrepareForPR branch from 1171982 to 78453bc Compare September 20, 2018 18:20

twsouthwick approved these changes Sep 20, 2018

View reviewed changes

conniey approved these changes Sep 20, 2018

View reviewed changes

conniey reviewed Sep 20, 2018

View reviewed changes

AlexGhiondea added 2 commits September 20, 2018 16:25

Display the portability of the references for each assembly

07e87ad

Move the request object into the AnalyzeResponse to keep the API shap…

fdff46d

…e unchanged.

AlexGhiondea force-pushed the PrepareForPR branch from 297b19d to fdff46d Compare September 20, 2018 23:25

twsouthwick merged commit a90b32b into microsoft:dev Sep 25, 2018

mjrousos mentioned this pull request Apr 16, 2019

Update service reference to support DGML #762

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Produce a DGML graph from the ApiPort data #695

Produce a DGML graph from the ApiPort data #695

AlexGhiondea commented Jul 30, 2018

AlexGhiondea commented Aug 21, 2018

conniey Sep 8, 2018

AlexGhiondea Sep 11, 2018

conniey Sep 8, 2018

AlexGhiondea Sep 11, 2018

twsouthwick Sep 20, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey Sep 8, 2018

conniey commented Sep 8, 2018

AlexGhiondea commented Sep 11, 2018

twsouthwick left a comment

twsouthwick Sep 20, 2018

twsouthwick Sep 20, 2018

twsouthwick Sep 20, 2018

AlexGhiondea Sep 20, 2018

twsouthwick Sep 20, 2018

twsouthwick Sep 20, 2018

AlexGhiondea Sep 20, 2018

conniey Sep 20, 2018

AlexGhiondea commented Sep 20, 2018

twsouthwick commented Sep 25, 2018


		public List<TargetUsageInfo> UsageData { get; set; }

		public string Assembly { get; set; }

		@@ -0,0 +1,131 @@
		// Copyright (c) Microsoft. All rights reserved.

Produce a DGML graph from the ApiPort data #695

Produce a DGML graph from the ApiPort data #695

Conversation

AlexGhiondea commented Jul 30, 2018

AlexGhiondea commented Aug 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conniey commented Sep 8, 2018

AlexGhiondea commented Sep 11, 2018

twsouthwick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexGhiondea commented Sep 20, 2018

twsouthwick commented Sep 25, 2018