fix: docfx download command behaviors (#9721)

filzrev · web-flow · commit f11958f09b81 · 2024-02-23T13:59:16.000+08:00
* fix: docfx download command behaviors

* chore: add logics to set baseurl if not exists. and resolve relative href to absolute
diff --git a/src/Docfx.Build/XRefMaps/XRefArchiveBuilder.cs b/src/Docfx.Build/XRefMaps/XRefArchiveBuilder.cs
@@ -24,7 +24,7 @@ public async Task<bool> DownloadAsync(Uri uri, string outputFile)
             try
             {
                 using var xa = XRefArchive.Open(outputFile, XRefArchiveMode.Create);
-                await DownloadCoreAsync(uri, xa, true);
+                await DownloadCoreAsync(uri, xa);
             }
             catch (Exception ex)
             {
@@ -36,88 +36,42 @@ public async Task<bool> DownloadAsync(Uri uri, string outputFile)
         }
     }
 
-    private async Task<string> DownloadCoreAsync(Uri uri, XRefArchive xa, bool isMajor)
+    private async Task<string> DownloadCoreAsync(Uri uri, XRefArchive xa)
     {
         IXRefContainer container;
         container = await _downloader.DownloadAsync(uri);
         if (container is not XRefMap map)
         {
-            // not support download an xref archive, or reference to an xref archive
+            // XRefArchive is not supported by `docfx download`.
+            Logger.LogWarning($"Download an xref archive, or reference to an xref archive is not supported. URI: {uri}");
             return null;
         }
-        if (map.Redirections?.Count > 0)
-        {
-            await RewriteRedirections(uri, xa, map);
-        }
-        if (map.References?.Count > 0 && map.HrefUpdated != true)
-        {
-            if (string.IsNullOrEmpty(map.BaseUrl))
-            {
-                XRefMapDownloader.UpdateHref(map, uri);
-            }
-        }
-        lock (_syncRoot)
-        {
-            if (isMajor)
-            {
-                return xa.CreateMajor(map);
-            }
-            else
-            {
-                return xa.CreateMinor(map, GetNames(uri, map));
-            }
-        }
-    }
 
-    private static IEnumerable<string> GetNames(Uri uri, XRefMap map)
-    {
-        var name = uri.Segments.LastOrDefault();
-        yield return name;
-        if (map.References?.Count > 0)
+        // If BaseUrl is not set. Use xrefmap file download url as basePath.
+        if (string.IsNullOrEmpty(map.BaseUrl))
         {
-            yield return map.References[0].Uid;
+            var baseUrl = uri.GetLeftPart(UriPartial.Path);
+            baseUrl = baseUrl.Substring(0, baseUrl.LastIndexOf('/') + 1);
+            map.BaseUrl = baseUrl;
+            map.UpdateHref(new Uri(baseUrl)); // Update hrefs from relative to absolute url.
+            map.HrefUpdated = null; // Don't save this flag for downloaded XRefMap.
         }
-    }
-
-    #region Rewrite redirections
 
-    private async Task<List<XRefMapRedirection>> RewriteRedirections(Uri uri, XRefArchive xa, XRefMap map) =>
-        (from list in
-            await Task.WhenAll(
-                from r in map.Redirections
-                where !string.IsNullOrEmpty(r.Href)
-                group r by r.Href into g
-                let href = GetHrefUri(uri, g.Key)
-                where href != null
-                select RewriteRedirectionsCore(g.ToList(), href, xa))
-         from r in list
-         orderby (r.UidPrefix ?? string.Empty).Length descending, (r.UidPrefix ?? string.Empty)
-         select r).ToList();
-
-    private async Task<List<XRefMapRedirection>> RewriteRedirectionsCore(List<XRefMapRedirection> redirections, Uri uri, XRefArchive xa)
-    {
-        var fileRef = await DownloadCoreAsync(uri, xa, false);
-        if (fileRef == null)
+        // Enforce XRefMap's references are sorted by uid.
+        // Note:
+        //   Sort is not needed if `map.Sorted == true`.
+        //   But there are some xrefmap files that is not propery sorted by using InvariantCulture.
+        //   (e.g. Unity xrefmap that maintained by community)
+        if (map.References != null && map.References.Count > 0)
         {
-            return new List<XRefMapRedirection>();
+            map.References.Sort(XRefSpecUidComparer.Instance);
+            map.Sorted = true;
         }
-        return (from r in redirections
-                select new XRefMapRedirection { UidPrefix = r.UidPrefix, Href = fileRef }).ToList();
-    }
 
-    private static Uri GetHrefUri(Uri uri, string href)
-    {
-        if (!Uri.TryCreate(href, UriKind.RelativeOrAbsolute, out Uri hrefUri))
-        {
-            Logger.LogWarning($"Invalid redirection href: {href}.");
-            return null;
-        }
-        if (!hrefUri.IsAbsoluteUri)
+        // Write XRefMap content to `xrefmap.yml`.
+        lock (_syncRoot)
         {
-            hrefUri = new Uri(uri, hrefUri);
+            return xa.CreateMajor(map);
         }
-        return hrefUri;
     }
-
-    #endregion
 }
diff --git a/src/Docfx.Build/XRefMaps/XRefMap.cs b/src/Docfx.Build/XRefMaps/XRefMap.cs
@@ -10,28 +10,49 @@
 
 namespace Docfx.Build.Engine;
 
+/// <summary>
+/// Xrefmap.
+/// </summary>
 public class XRefMap : IXRefContainer
 {
+    /// <summary>
+    /// Indicate <see cref="References"/> are sorted by <see cref="XRefSpec.Uid"/> with or not.
+    /// </summary>
     [YamlMember(Alias = "sorted")]
     [JsonProperty("sorted")]
     [JsonPropertyName("sorted")]
     public bool? Sorted { get; set; }
 
+    /// <summary>
+    /// Indicate href links are updated or not.
+    /// </summary>
     [YamlMember(Alias = "hrefUpdated")]
     [JsonProperty("hrefUpdated")]
     [JsonPropertyName("hrefUpdated")]
     public bool? HrefUpdated { get; set; }
 
+    /// <summary>
+    /// Base url. It's used when href is specified as relative url.
+    /// </summary>
     [YamlMember(Alias = "baseUrl")]
     [JsonProperty("baseUrl")]
     [JsonPropertyName("baseUrl")]
     public string BaseUrl { get; set; }
 
+    /// <summary>
+    /// List of <see href="XRefMapRedirection"/> settings.
+    /// </summary>
     [YamlMember(Alias = "redirections")]
     [JsonProperty("redirections")]
     [JsonPropertyName("redirections")]
     public List<XRefMapRedirection> Redirections { get; set; }
 
+    /// <summary>
+    /// List of <see href="XRefSpec"/>.
+    /// </summary>
+    /// <remarks>
+    /// If <see cref="Sorted"/> is true. XRefSpec items must be sorted by <see cref="XRefSpec.Uid"/> with InvariantCulture.
+    /// </remarks>
     [YamlMember(Alias = "references")]
     [JsonProperty("references")]
     [JsonPropertyName("references")]
diff --git a/src/Docfx.Build/XRefMaps/XRefMapDownloader.cs b/src/Docfx.Build/XRefMaps/XRefMapDownloader.cs
@@ -81,7 +81,7 @@ private IXRefContainer ReadLocalFileWithFallback(Uri uri)
     /// </remarks>
     protected virtual async Task<IXRefContainer> DownloadBySchemeAsync(Uri uri)
     {
-        IXRefContainer result = null;
+        IXRefContainer result;
         if (uri.IsFile)
         {
             result = DownloadFromLocal(uri);
@@ -123,8 +123,6 @@ private static IXRefContainer ReadLocalFile(string filePath)
     protected static async Task<XRefMap> DownloadFromWebAsync(Uri uri)
     {
         Logger.LogVerbose($"Reading from web: {uri.OriginalString}");
-        var baseUrl = uri.GetLeftPart(UriPartial.Path);
-        baseUrl = baseUrl.Substring(0, baseUrl.LastIndexOf('/') + 1);
 
         using var httpClient = new HttpClient(new HttpClientHandler()
         {
@@ -138,8 +136,7 @@ protected static async Task<XRefMap> DownloadFromWebAsync(Uri uri)
         using var stream = await httpClient.GetStreamAsync(uri);
         using var sr = new StreamReader(stream, bufferSize: 81920); // Default :1024 byte
         var map = YamlUtility.Deserialize<XRefMap>(sr);
-        map.BaseUrl = baseUrl;
-        UpdateHref(map, null);
+
         return map;
     }
 
diff --git a/src/docfx/Properties/launchSettings.json b/src/docfx/Properties/launchSettings.json
@@ -42,6 +42,14 @@
       "environmentVariables": {
       }
     },
+    // Run `docfx download` command.
+    "docfx download": {
+      "commandName": "Project",
+      "commandLineArgs": "download xrefmap.zip --xref https://github.com/dotnet/docfx/raw/main/.xrefmap.json",
+      "workingDirectory": ".",
+      "environmentVariables": {
+      }
+    },
     // Run `docfx` command.
     "docfx": {
       "commandName": "Project",
diff --git a/test/Docfx.Build.Tests/XRefArchiveBuilderTest.cs b/test/Docfx.Build.Tests/XRefArchiveBuilderTest.cs
@@ -14,12 +14,18 @@ public async Task TestDownload()
         const string ZipFile = "test.zip";
         var builder = new XRefArchiveBuilder();
 
+        // Download following xrefmap.yml content.
+        // ```
+        // ### YamlMime:XRefMap
+        // sorted: true
+        // references: []
+        // ```
         Assert.True(await builder.DownloadAsync(new Uri("http://dotnet.github.io/docfx/xrefmap.yml"), ZipFile));
 
         using (var xar = XRefArchive.Open(ZipFile, XRefArchiveMode.Read))
         {
             var map = xar.GetMajor();
-            Assert.True(map.HrefUpdated);
+            Assert.Null(map.HrefUpdated);
             Assert.True(map.Sorted);
             Assert.NotNull(map.References);
             Assert.Null(map.Redirections);

Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ private IXRefContainer ReadLocalFileWithFallback(Uri uri)`
`81`	`81`	`/// </remarks>`
`82`	`82`	`protected virtual async Task<IXRefContainer> DownloadBySchemeAsync(Uri uri)`
`83`	`83`	`{`
`84`		`- IXRefContainer result = null;`
	`84`	`+ IXRefContainer result;`
`85`	`85`	`if (uri.IsFile)`
`86`	`86`	`{`
`87`	`87`	`result = DownloadFromLocal(uri);`
`@@ -123,8 +123,6 @@ private static IXRefContainer ReadLocalFile(string filePath)`
`123`	`123`	`protected static async Task<XRefMap> DownloadFromWebAsync(Uri uri)`
`124`	`124`	`{`
`125`	`125`	`Logger.LogVerbose($"Reading from web: {uri.OriginalString}");`
`126`		`- var baseUrl = uri.GetLeftPart(UriPartial.Path);`
`127`		`- baseUrl = baseUrl.Substring(0, baseUrl.LastIndexOf('/') + 1);`
`128`	`126`
`129`	`127`	`using var httpClient = new HttpClient(new HttpClientHandler()`
`130`	`128`	`{`
`@@ -138,8 +136,7 @@ protected static async Task<XRefMap> DownloadFromWebAsync(Uri uri)`
`138`	`136`	`using var stream = await httpClient.GetStreamAsync(uri);`
`139`	`137`	`using var sr = new StreamReader(stream, bufferSize: 81920); // Default :1024 byte`
`140`	`138`	`var map = YamlUtility.Deserialize<XRefMap>(sr);`
`141`		`- map.BaseUrl = baseUrl;`
`142`		`- UpdateHref(map, null);`
	`139`	`+`
`143`	`140`	`return map;`
`144`	`141`	`}`
`145`	`142`