commit 9773b2f1cc272108233815ba7170e37554d9d61f
parent f4e41a03d8546b95991ef1aa75ea5a5ec8fb95e3
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Thu, 6 May 2021 00:04:15 -0700
Make additional metadata for downloads more extensible and add display name (#931)
In response to a comment after the PR was merged, update the Delivery Optimization download code to be able to set the DisplayName property. We always set it to `Windows Package Manager`.
As part of this, I changed the way this metadata flows in to make it easier to add more to this optional data in the future.
Diffstat:
9 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt
@@ -100,6 +100,7 @@ docx
dotnet
downlevel
downloader
+downloaders
dword
DWORDLONG
elseif
diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp
@@ -62,7 +62,7 @@ namespace AppInstaller::CLI
void Command::OutputIntroHeader(Execution::Reporter& reporter) const
{
reporter.Info() <<
- "Windows Package Manager v"_liv << Runtime::GetClientVersion() << ' ' << Resource::String::PreviewVersion << std::endl <<
+ Resource::FixedString::ProductName << " v"_liv << Runtime::GetClientVersion() << ' ' << Resource::String::PreviewVersion << std::endl <<
Resource::String::MainCopyrightNotice << std::endl;
}
diff --git a/src/AppInstallerCLICore/Resources.cpp b/src/AppInstallerCLICore/Resources.cpp
@@ -4,6 +4,8 @@
#include "pch.h"
#include "Resources.h"
+using namespace AppInstaller::Utility::literals;
+
namespace AppInstaller::CLI::Resource
{
@@ -25,4 +27,14 @@ namespace AppInstaller::CLI::Resource
{
return Utility::ConvertToUTF8(m_wingetLoader.GetString(resKey));
}
-}-
\ No newline at end of file
+
+ Utility::LocIndView GetFixedString(FixedString fs)
+ {
+ switch (fs)
+ {
+ case FixedString::ProductName: return "Windows Package Manager"_liv;
+ }
+
+ THROW_HR(E_UNEXPECTED);
+ }
+}
diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h
@@ -287,9 +287,22 @@ namespace AppInstaller::CLI::Resource
Loader();
};
+
+ // Fixed strings are not localized, but we use a similar system to prevent duplication
+ enum class FixedString
+ {
+ ProductName,
+ };
+
+ Utility::LocIndView GetFixedString(FixedString fs);
}
inline std::ostream& operator<<(std::ostream& out, AppInstaller::CLI::Resource::StringId si)
{
return (out << AppInstaller::CLI::Resource::LocString{ si });
}
+
+inline std::ostream& operator<<(std::ostream& out, AppInstaller::CLI::Resource::FixedString fs)
+{
+ return (out << GetFixedString(fs));
+}
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp
@@ -105,8 +105,10 @@ namespace AppInstaller::CLI::Workflow
std::filesystem::path tempInstallerPath = Runtime::GetPathTo(Runtime::PathName::Temp);
tempInstallerPath /= Utility::ConvertToUTF16(manifest.Id + '.' + manifest.Version);
+ Utility::DownloadInfo downloadInfo{};
+ downloadInfo.DisplayName = Resource::GetFixedString(Resource::FixedString::ProductName);
// Use the SHA256 hash of the installer as the identifier for the download
- std::string hashString = SHA256::ConvertToString(installer.Sha256);
+ downloadInfo.ContentId = SHA256::ConvertToString(installer.Sha256);
AICLI_LOG(CLI, Info, << "Generated temp download path: " << tempInstallerPath);
@@ -126,7 +128,7 @@ namespace AppInstaller::CLI::Workflow
Utility::DownloadType::Installer,
std::placeholders::_1,
true,
- hashString));
+ downloadInfo));
success = true;
}
diff --git a/src/AppInstallerCommonCore/DODownloader.cpp b/src/AppInstallerCommonCore/DODownloader.cpp
@@ -340,7 +340,7 @@ namespace AppInstaller::Utility
const std::filesystem::path& dest,
IProgressCallback& progress,
bool computeHash,
- std::string_view downloadIdentifier)
+ std::optional<DownloadInfo> info)
{
AICLI_LOG(Core, Info, << "DeliveryOptimization downloading from url: " << url);
@@ -354,11 +354,23 @@ namespace AppInstaller::Utility
THROW_IF_FAILED(DeliveryOptimization::DODownloadStatusCallback::Create(progress, &callback));
download.Uri(url);
- download.ContentId(downloadIdentifier);
download.ForegroundPriority(true);
download.LocalPath(dest);
download.CallbackInterface(callback.get());
+ if (info)
+ {
+ if (!info->DisplayName.empty())
+ {
+ download.DisplayName(info->DisplayName);
+ }
+
+ if (!info->ContentId.empty())
+ {
+ download.ContentId(info->ContentId);
+ }
+ }
+
download.Start();
auto cancelLifetime = progress.SetCancellationFunction([&download, &callback]()
diff --git a/src/AppInstallerCommonCore/DODownloader.h b/src/AppInstallerCommonCore/DODownloader.h
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#pragma once
+#include <AppInstallerDownloader.h>
#include <AppInstallerProgress.h>
#include <optional>
@@ -19,5 +20,5 @@ namespace AppInstaller::Utility
const std::filesystem::path& dest,
IProgressCallback& progress,
bool computeHash,
- std::string_view downloadIdentifier);
+ std::optional<DownloadInfo> info);
}
diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp
@@ -134,7 +134,7 @@ namespace AppInstaller::Utility
DownloadType,
IProgressCallback& progress,
bool computeHash,
- std::string_view)
+ std::optional<DownloadInfo>)
{
THROW_HR_IF(E_INVALIDARG, url.empty());
return WinINetDownloadToStream(url, dest, progress, computeHash);
@@ -146,7 +146,7 @@ namespace AppInstaller::Utility
DownloadType type,
IProgressCallback& progress,
bool computeHash,
- std::string_view downloadIdentifier)
+ std::optional<DownloadInfo> info)
{
THROW_HR_IF(E_INVALIDARG, url.empty());
THROW_HR_IF(E_INVALIDARG, dest.empty());
@@ -172,7 +172,7 @@ namespace AppInstaller::Utility
if (setting == InstallerDownloader::DeliveryOptimization)
{
- return DODownload(url, dest, progress, computeHash, downloadIdentifier);
+ return DODownload(url, dest, progress, computeHash, info);
// While DO still requires an explicit opt-in, we will let failures through.
// When DO becomes the default, we may choose to catch exceptions and fall back to WinINet below.
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDownloader.h b/src/AppInstallerCommonCore/Public/AppInstallerDownloader.h
@@ -24,6 +24,13 @@ namespace AppInstaller::Utility
Installer,
};
+ // Extra metadata about a download for use by certain downloaders (Delivery Optimization for instance).
+ struct DownloadInfo
+ {
+ std::string DisplayName;
+ std::string ContentId;
+ };
+
// Downloads a file from the given URL and places it in the given location.
// url: The url to be downloaded from. http->https redirection is allowed.
// dest: The stream to be downloaded to.
@@ -35,7 +42,7 @@ namespace AppInstaller::Utility
DownloadType type,
IProgressCallback& progress,
bool computeHash = false,
- std::string_view downloadIdentifier = {});
+ std::optional<DownloadInfo> info = {});
// Downloads a file from the given URL and places it in the given location.
// url: The url to be downloaded from. http->https redirection is allowed.
@@ -48,7 +55,7 @@ namespace AppInstaller::Utility
DownloadType type,
IProgressCallback& progress,
bool computeHash = false,
- std::string_view downloadIdentifier = {});
+ std::optional<DownloadInfo> info = {});
// Determines if the given url is a remote location.
bool IsUrlRemote(std::string_view url);
