Allow DownloadUrl on embedded payloads.

#5253

8 files changed, 110 insertions, 58 deletions

diff --git a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
index deab4d78..4a4f06f3 100644
--- a/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
+++ b/src/WixToolset.Core.Burn/Bind/BindBundleCommand.cs
@@ -300,7 +300,7 @@ namespace WixToolset.Core.Burn
 
                     if (PackagingType.Embedded == payload.Packaging && String.IsNullOrEmpty(payload.EmbeddedId))
                     {
-                        payload.EmbeddedId = String.Format(CultureInfo.InvariantCulture, BurnCommon.BurnAttachedContainerEmbeddedIdFormat, payloadIndex);
+                        payload.EmbeddedId = String.Format(CultureInfo.InvariantCulture, BurnCommon.BurnAuthoredContainerEmbeddedIdFormat, payloadIndex);
                         ++payloadIndex;
                     }
                 }
diff --git a/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs b/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs
index e41c1058..c678b114 100644
--- a/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs
+++ b/src/WixToolset.Core.Burn/Bind/ResolveDownloadUrlsCommand.cs
@@ -5,6 +5,7 @@ namespace WixToolset.Core.Burn.Bind
     using System;
     using System.Collections.Generic;
     using WixToolset.Data;
+    using WixToolset.Data.Burn;
     using WixToolset.Data.Symbols;
     using WixToolset.Extensibility;
     using WixToolset.Extensibility.Services;
@@ -60,7 +61,14 @@ namespace WixToolset.Core.Burn.Bind
         {
             foreach (var payload in this.PayloadsById.Values)
             {
-                if (payload.Packaging == PackagingType.External)
+                if (payload.Packaging == PackagingType.Embedded && payload.ContainerRef == BurnConstants.BurnUXContainerName)
+                {
+                    if (!String.IsNullOrEmpty(payload.DownloadUrl))
+                    {
+                        this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForBAPayloads(payload.SourceLineNumbers, payload.Id.Id));
+                    }
+                }
+                else
                 {
                     var packageId = payload.ParentPackagePayloadRef;
                     var parentUrl = payload.ParentPackagePayloadRef == null ? null : this.PayloadsById[payload.ParentPackagePayloadRef].DownloadUrl;
@@ -70,13 +78,6 @@ namespace WixToolset.Core.Burn.Bind
                         payload.DownloadUrl = resolvedUrl;
                     }
                 }
-                else if (payload.Packaging == PackagingType.Embedded)
-                {
-                    if (!String.IsNullOrEmpty(payload.DownloadUrl))
-                    {
-                        this.Messaging.Write(WarningMessages.DownloadUrlNotSupportedForEmbeddedPayloads(payload.SourceLineNumbers, payload.Id.Id));
-                    }
-                }
             }
         }
 
diff --git a/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs b/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs
index ab3b7896..1eb3563a 100644
--- a/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs
+++ b/src/WixToolset.Core.Burn/Bundles/BurnCommon.cs
@@ -19,8 +19,7 @@ namespace WixToolset.Core.Burn.Bundles
     {
         public const string BurnNamespace = "http://wixtoolset.org/schemas/v4/2008/Burn";
         public const string BurnUXContainerEmbeddedIdFormat = "u{0}";
-        public const string BurnUXContainerPayloadIdFormat = "p{0}";
-        public const string BurnAttachedContainerEmbeddedIdFormat = "a{0}";
+        public const string BurnAuthoredContainerEmbeddedIdFormat = "a{0}";
 
         public const string BADataFileName = "BootstrapperApplicationData.xml";
         public const string BADataNamespace = "http://wixtoolset.org/schemas/v4/BootstrapperApplicationData";
diff --git a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
index db09b540..5655d23d 100644
--- a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
+++ b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs
@@ -166,9 +166,7 @@ namespace WixToolset.Core.Burn.Bundles
                 // write the UX allPayloads...
                 foreach (var payload in this.UXContainerPayloads)
                 {
-                    writer.WriteStartElement("Payload");
-                    this.WriteBurnManifestPayloadAttributes(writer, payload, true, this.Payloads);
-                    writer.WriteEndElement();
+                    this.WriteBurnManifestUXPayload(writer, payload);
                 }
 
                 writer.WriteEndElement(); // </UX>
@@ -183,20 +181,9 @@ namespace WixToolset.Core.Burn.Bundles
                     }
                 }
 
-                foreach (var payload in this.Payloads.Values)
+                foreach (var payload in this.Payloads.Values.Where(p => p.ContainerRef != BurnConstants.BurnUXContainerName))
                 {
-                    if (PackagingType.Embedded == payload.Packaging && BurnConstants.BurnUXContainerName != payload.ContainerRef)
-                    {
-                        writer.WriteStartElement("Payload");
-                        this.WriteBurnManifestPayloadAttributes(writer, payload, true, this.Payloads);
-                        writer.WriteEndElement();
-                    }
-                    else if (PackagingType.External == payload.Packaging)
-                    {
-                        writer.WriteStartElement("Payload");
-                        this.WriteBurnManifestPayloadAttributes(writer, payload, false, this.Payloads);
-                        writer.WriteEndElement();
-                    }
+                    this.WriteBurnManifestPayload(writer, payload);
                 }
 
                 foreach (var rollbackBoundary in this.RollbackBoundaries)
@@ -654,9 +641,9 @@ namespace WixToolset.Core.Burn.Bundles
             }
         }
 
-        private void WriteBurnManifestPayloadAttributes(XmlTextWriter writer, WixBundlePayloadSymbol payload, bool embeddedOnly, Dictionary<string, WixBundlePayloadSymbol> allPayloads)
+        private void WriteBurnManifestPayload(XmlTextWriter writer, WixBundlePayloadSymbol payload)
         {
-            Debug.Assert(!embeddedOnly || PackagingType.Embedded == payload.Packaging);
+            writer.WriteStartElement("Payload");
 
             writer.WriteAttributeString("Id", payload.Id.Id);
             writer.WriteAttributeString("FilePath", payload.Name);
@@ -668,28 +655,46 @@ namespace WixToolset.Core.Burn.Bundles
                 writer.WriteAttributeString("LayoutOnly", "yes");
             }
 
+            if (!String.IsNullOrEmpty(payload.DownloadUrl))
+            {
+                writer.WriteAttributeString("DownloadUrl", payload.DownloadUrl);
+            }
+
             switch (payload.Packaging)
             {
                 case PackagingType.Embedded: // this means it's in a container.
+                    Debug.Assert(BurnConstants.BurnUXContainerName != payload.ContainerRef);
+
                     writer.WriteAttributeString("Packaging", "embedded");
                     writer.WriteAttributeString("SourcePath", payload.EmbeddedId);
-
-                    if (BurnConstants.BurnUXContainerName != payload.ContainerRef)
-                    {
-                        writer.WriteAttributeString("Container", payload.ContainerRef);
-                    }
+                    writer.WriteAttributeString("Container", payload.ContainerRef);
                     break;
 
                 case PackagingType.External:
-                    if (!String.IsNullOrEmpty(payload.DownloadUrl))
-                    {
-                        writer.WriteAttributeString("DownloadUrl", payload.DownloadUrl);
-                    }
-
                     writer.WriteAttributeString("Packaging", "external");
                     writer.WriteAttributeString("SourcePath", payload.Name);
                     break;
             }
+
+            writer.WriteEndElement();
+        }
+
+        private void WriteBurnManifestUXPayload(XmlTextWriter writer, WixBundlePayloadSymbol payload)
+        {
+            Debug.Assert(PackagingType.Embedded == payload.Packaging);
+            Debug.Assert(BurnConstants.BurnUXContainerName == payload.ContainerRef);
+
+            writer.WriteStartElement("Payload");
+
+            // TODO: The engine should be updated to not require FileSize, Hash, or Packaging for UX payloads since the values are never used.
+            writer.WriteAttributeString("Id", payload.Id.Id);
+            writer.WriteAttributeString("FilePath", payload.Name);
+            writer.WriteAttributeString("FileSize", payload.FileSize.Value.ToString(CultureInfo.InvariantCulture));
+            writer.WriteAttributeString("Hash", payload.Hash);
+            writer.WriteAttributeString("Packaging", "embedded");
+            writer.WriteAttributeString("SourcePath", payload.EmbeddedId);
+
+            writer.WriteEndElement();
         }
     }
 }
diff --git a/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs b/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs
index 8f361626..87a63cc3 100644
--- a/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs
+++ b/src/WixToolset.Core.Burn/Bundles/CreateContainerCommand.cs
@@ -44,13 +44,6 @@ namespace WixToolset.Core.Burn.Bundles
 
         public void Execute()
         {
-            var payloadCount = this.Payloads.Count(); // The number of embedded payloads
-
-            if (!String.IsNullOrEmpty(this.ManifestFile))
-            {
-                ++payloadCount;
-            }
-
             var cabinetPath = Path.GetFullPath(this.OutputPath);
 
             var files = new List<CabinetCompressFile>();
diff --git a/src/WixToolset.Core/Compile/CompilerPayload.cs b/src/WixToolset.Core/Compile/CompilerPayload.cs
index 4c7e843d..3f423034 100644
--- a/src/WixToolset.Core/Compile/CompilerPayload.cs
+++ b/src/WixToolset.Core/Compile/CompilerPayload.cs
@@ -15,8 +15,6 @@ namespace WixToolset.Core
 
         public string Description { get; set; }
 
-        public string DisplayName { get; set; }
-
         public string DownloadUrl { get; set; }
 
         public string Hash { get; set; }
@@ -158,7 +156,7 @@ namespace WixToolset.Core
 
                 if (!String.IsNullOrEmpty(this.DownloadUrl))
                 {
-                    this.Core.Write(WarningMessages.DownloadUrlNotSupportedForEmbeddedPayloads(this.SourceLineNumbers, this.Id.Id));
+                    this.Core.Write(WarningMessages.DownloadUrlNotSupportedForBAPayloads(this.SourceLineNumbers, this.Id.Id));
                 }
 
                 this.Compressed = YesNoDefaultType.Yes;
@@ -174,7 +172,7 @@ namespace WixToolset.Core
                     DownloadUrl = this.DownloadUrl,
                     Compressed = (this.Compressed == YesNoDefaultType.Yes) ? true : (this.Compressed == YesNoDefaultType.No) ? (bool?)false : null,
                     UnresolvedSourceFile = this.SourceFile, // duplicate of sourceFile but in a string column so it won't get resolved to a full path during binding.
-                    DisplayName = this.DisplayName ?? this.ProductName,
+                    DisplayName = this.ProductName,
                     Description = this.Description,
                     Hash = this.Hash,
                     FileSize = this.Size,
@@ -245,11 +243,6 @@ namespace WixToolset.Core
             this.Description = this.Core.GetAttributeValue(this.SourceLineNumbers, attrib);
         }
 
-        public void ParseDisplayName(XAttribute attrib)
-        {
-            this.DisplayName = this.Core.GetAttributeValue(this.SourceLineNumbers, attrib);
-        }
-
         public void ParseDownloadUrl(XAttribute attrib)
         {
             this.DownloadUrl = this.Core.GetAttributeValue(this.SourceLineNumbers, attrib);
diff --git a/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs b/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs
index ff48ee05..dd381dfe 100644
--- a/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/ContainerFixture.cs
@@ -15,6 +15,50 @@ namespace WixToolsetTest.CoreIntegration
 
     public class ContainerFixture
     {
+        [Fact(Skip = "Test demonstrates failure")]
+        public void CanBuildWithCustomAttachedContainer()
+        {
+            var folder = TestData.Get(@"TestData");
+
+            using (var fs = new DisposableFileSystem())
+            {
+                var baseFolder = fs.GetFolder();
+                var intermediateFolder = Path.Combine(baseFolder, "obj");
+                var binFolder = Path.Combine(baseFolder, "bin");
+                var bundlePath = Path.Combine(binFolder, "test.exe");
+                var baFolderPath = Path.Combine(baseFolder, "ba");
+                var extractFolderPath = Path.Combine(baseFolder, "extract");
+
+                this.BuildMsis(folder, intermediateFolder, binFolder, buildToSubfolder: true);
+
+                var result = WixRunner.Execute(new[]
+                {
+                    "build",
+                    Path.Combine(folder, "Container", "HarvestIntoAttachedContainer.wxs"),
+                    Path.Combine(folder, "BundleWithPackageGroupRef", "Bundle.wxs"),
+                    "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
+                    "-bindpath", binFolder,
+                    "-intermediateFolder", intermediateFolder,
+                    "-o", bundlePath
+                });
+
+                result.AssertSuccess();
+
+                Assert.True(File.Exists(bundlePath));
+
+                var extractResult = BundleExtractor.ExtractBAContainer(null, bundlePath, baFolderPath, extractFolderPath);
+                extractResult.AssertSuccess();
+
+                var payloads = extractResult.SelectManifestNodes("/burn:BurnManifest/burn:Payload");
+                Assert.Equal(4, payloads.Count);
+                var ignoreAttributes = new Dictionary<string, List<string>> { { "Payload", new List<string> { "FileSize", "Hash" } } };
+                Assert.Equal(@"<Payload Id='FirstX64' FilePath='FirstX64\FirstX64.msi' FileSize='*' Hash='*' DownloadUrl='http://example.com//FirstX64/FirstX64/FirstX64.msi' Packaging='embedded' SourcePath='a0' Container='BundlePackages' />", payloads[0].GetTestXml(ignoreAttributes));
+                Assert.Equal(@"<Payload Id='FirstX86.msi' FilePath='FirstX86\FirstX86.msi' FileSize='*' Hash='*' DownloadUrl='http://example.com//FirstX86.msi/FirstX86/FirstX86.msi' Packaging='embedded' SourcePath='a1' Container='BundlePackages' />", payloads[1].GetTestXml(ignoreAttributes));
+                Assert.Equal(@"<Payload Id='fk1m38Cf9RZ2Bx_ipinRY6BftelU' FilePath='FirstX86\PFiles\MsiPackage\test.txt' FileSize='*' Hash='*' DownloadUrl='http://example.com/FirstX86/fk1m38Cf9RZ2Bx_ipinRY6BftelU/FirstX86/PFiles/MsiPackage/test.txt' Packaging='embedded' SourcePath='a2' Container='BundlePackages' />", payloads[2].GetTestXml(ignoreAttributes));
+                Assert.Equal(@"<Payload Id='ff2L_N_DLQ.nSUi.l8LxG14gd2V4' FilePath='FirstX64\PFiles\MsiPackage\test.txt' FileSize='*' Hash='*' DownloadUrl='http://example.com/FirstX64/ff2L_N_DLQ.nSUi.l8LxG14gd2V4/FirstX64/PFiles/MsiPackage/test.txt' Packaging='embedded' SourcePath='a3' Container='BundlePackages' />", payloads[3].GetTestXml(ignoreAttributes));
+            }
+        }
+
         [Fact]
         public void HarvestedPayloadsArePutInCorrectContainer()
         {
@@ -309,7 +353,7 @@ namespace WixToolsetTest.CoreIntegration
             }
         }
 
-        private void BuildMsis(string folder, string intermediateFolder, string binFolder)
+        private void BuildMsis(string folder, string intermediateFolder, string binFolder, bool buildToSubfolder = false)
         {
             var result = WixRunner.Execute(new[]
             {
@@ -319,7 +363,7 @@ namespace WixToolsetTest.CoreIntegration
                 Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
                 "-bindpath", Path.Combine(folder, "SingleFile", "data"),
                 "-intermediateFolder", intermediateFolder,
-                "-o", Path.Combine(binFolder, "FirstX86.msi"),
+                "-o", Path.Combine(binFolder, buildToSubfolder ? "FirstX86" : ".", "FirstX86.msi"),
             });
 
             result.AssertSuccess();
@@ -332,7 +376,7 @@ namespace WixToolsetTest.CoreIntegration
                 Path.Combine(folder, "ProductWithComponentGroupRef", "Product.wxs"),
                 "-bindpath", Path.Combine(folder, "SingleFile", "data"),
                 "-intermediateFolder", intermediateFolder,
-                "-o", Path.Combine(binFolder, "FirstX64.msi"),
+                "-o", Path.Combine(binFolder, buildToSubfolder ? "FirstX64" : ".", "FirstX64.msi"),
             });
 
             result.AssertSuccess();
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs
new file mode 100644
index 00000000..ec757c5d
--- /dev/null
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/Container/HarvestIntoAttachedContainer.wxs
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
+    <Fragment>
+        <PackageGroup Id="BundlePackages">
+            <MsiPackage Id="FirstX86">
+                <PayloadGroupRef Id="FirstX86Payloads" />
+            </MsiPackage>
+            <MsiPackage Id="FirstX64" Name="FirstX64\FirstX64.msi" SourceFile="FirstX64\" DownloadUrl="http://example.com/{0}/{1}/{2}" />
+        </PackageGroup>
+        <Container Id="BundlePackages" Type="attached">
+            <PackageGroupRef Id="BundlePackages" />
+        </Container>
+        <PayloadGroup Id="FirstX86Payloads">
+            <MsiPackagePayload Name="FirstX86\FirstX86.msi" SourceFile="FirstX86\" DownloadUrl="http://example.com/{0}/{1}/{2}" />
+        </PayloadGroup>
+    </Fragment>
+</Wix>