commit d94b94bfaff4b2262780a7c80a837fefe8d55870
parent 532c4417389c43deb5b426083117c1ef1c9cc2f1
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Wed, 18 Mar 2020 13:05:58 -0700
Fix version only sorting in the manifest selection (#61)
Diffstat:
13 files changed, 176 insertions(+), 90 deletions(-)
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp
@@ -3,7 +3,6 @@
#include "pch.h"
#include "InstallFlow.h"
-#include "ManifestComparator.h"
#include "ShellExecuteInstallerHandler.h"
#include "MsixInstallerHandler.h"
@@ -18,46 +17,30 @@ namespace AppInstaller::Workflow
if (m_argsRef.Contains(ExecutionArgs::Type::Manifest))
{
m_manifest = Manifest::Manifest::CreateFromPath(*(m_argsRef.GetArg(ExecutionArgs::Type::Manifest)));
- InstallInternal();
}
else
{
- if (WorkflowBase::IndexSearch() && WorkflowBase::EnsureOneMatchFromSearchResult())
+ if (!IndexSearch() || !EnsureOneMatchFromSearchResult() || !GetManifest())
{
- GetManifest();
- InstallInternal();
+ return;
}
+ m_reporterRef.ShowMsg("Found app: " + m_searchResult.Matches[0].Application->GetName());
}
+
+ SelectInstaller();
+ InstallInternal();
}
void InstallFlow::InstallInternal()
{
Logging::Telemetry().LogManifestFields(m_manifest.Name, m_manifest.Version);
- // Select Installer
- ManifestComparator manifestComparator(m_manifest, m_reporterRef);
- m_selectedInstaller = manifestComparator.GetPreferredInstaller(m_argsRef);
-
auto installerHandler = GetInstallerHandler();
installerHandler->Download();
installerHandler->Install();
}
- void InstallFlow::GetManifest()
- {
- auto app = m_searchResult.Matches.at(0).Application.get();
-
- Logging::Telemetry().LogManifestFields(app->GetName(), app->GetId());
- m_reporterRef.ShowMsg("Found app: " + app->GetName());
-
- // Todo: handle failure if necessary after real search is in place
- m_manifest = app->GetManifest(
- m_argsRef.Contains(ExecutionArgs::Type::Version) ? *m_argsRef.GetArg(ExecutionArgs::Type::Version) : "",
- m_argsRef.Contains(ExecutionArgs::Type::Channel) ? *m_argsRef.GetArg(ExecutionArgs::Type::Channel) : ""
- );
- }
-
std::unique_ptr<InstallerHandlerBase> InstallFlow::GetInstallerHandler()
{
switch (m_selectedInstaller.InstallerType)
diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h
@@ -9,20 +9,16 @@
namespace AppInstaller::Workflow
{
- class InstallFlow : public WorkflowBase
+ class InstallFlow : public SingleManifestWorkflow
{
public:
- InstallFlow(AppInstaller::CLI::ExecutionContext& context) : WorkflowBase(context) {}
+ InstallFlow(AppInstaller::CLI::ExecutionContext& context) : SingleManifestWorkflow(context) {}
// Execute will perform a query against index and do app install if a target app is found.
// If a manifest is given with /manifest, use the manifest and no index search is performed.
void Execute();
protected:
- AppInstaller::Manifest::Manifest m_manifest;
- AppInstaller::Manifest::ManifestInstaller m_selectedInstaller;
-
- void GetManifest();
void InstallInternal();
// Creates corresponding InstallerHandler according to InstallerType
diff --git a/src/AppInstallerCLICore/Workflows/ShowFlow.cpp b/src/AppInstallerCLICore/Workflows/ShowFlow.cpp
@@ -12,7 +12,7 @@ namespace AppInstaller::Workflow
{
void ShowFlow::Execute()
{
- if (WorkflowBase::IndexSearch() && WorkflowBase::EnsureOneMatchFromSearchResult())
+ if (IndexSearch() && EnsureOneMatchFromSearchResult())
{
if (m_argsRef.Contains(ExecutionArgs::Type::ListVersions))
{
@@ -27,31 +27,27 @@ namespace AppInstaller::Workflow
void ShowFlow::ShowAppInfo()
{
- auto app = m_searchResult.Matches.at(0).Application.get();
-
- auto manifest = app->GetManifest(
- m_argsRef.Contains(ExecutionArgs::Type::Version) ? *m_argsRef.GetArg(ExecutionArgs::Type::Version) : "",
- m_argsRef.Contains(ExecutionArgs::Type::Channel) ? *m_argsRef.GetArg(ExecutionArgs::Type::Channel) : ""
- );
-
- ManifestComparator manifestComparator(manifest, m_reporterRef);
- auto selectedLocalization = manifestComparator.GetPreferredLocalization(m_argsRef);
- auto selectedInstaller = manifestComparator.GetPreferredInstaller(m_argsRef);
+ if (GetManifest())
+ {
+ SelectInstaller();
+ ManifestComparator manifestComparator(m_manifest, m_reporterRef);
+ auto selectedLocalization = manifestComparator.GetPreferredLocalization(m_argsRef);
- m_reporterRef.ShowMsg("Id: " + manifest.Id);
- m_reporterRef.ShowMsg("Name: " + manifest.Name);
- m_reporterRef.ShowMsg("Version: " + manifest.Version);
- m_reporterRef.ShowMsg("Author: " + manifest.Author);
- m_reporterRef.ShowMsg("AppMoniker: " + manifest.AppMoniker);
- m_reporterRef.ShowMsg("Description: " + selectedLocalization.Description);
- m_reporterRef.ShowMsg("Homepage: " + selectedLocalization.Homepage);
- m_reporterRef.ShowMsg("License: " + selectedLocalization.LicenseUrl);
+ m_reporterRef.ShowMsg("Id: " + m_manifest.Id);
+ m_reporterRef.ShowMsg("Name: " + m_manifest.Name);
+ m_reporterRef.ShowMsg("Version: " + m_manifest.Version);
+ m_reporterRef.ShowMsg("Author: " + m_manifest.Author);
+ m_reporterRef.ShowMsg("AppMoniker: " + m_manifest.AppMoniker);
+ m_reporterRef.ShowMsg("Description: " + selectedLocalization.Description);
+ m_reporterRef.ShowMsg("Homepage: " + selectedLocalization.Homepage);
+ m_reporterRef.ShowMsg("License: " + selectedLocalization.LicenseUrl);
- m_reporterRef.ShowMsg("Installer info:" + manifest.Id);
- m_reporterRef.ShowMsg("--Installer Language: " + selectedInstaller.Language);
- m_reporterRef.ShowMsg("--Installer SHA256: " + Utility::SHA256::ConvertToString(selectedInstaller.Sha256));
- m_reporterRef.ShowMsg("--Installer Download Url: " + selectedInstaller.Url);
- m_reporterRef.ShowMsg("--Installer Type: " + Manifest::ManifestInstaller::InstallerTypeToString(selectedInstaller.InstallerType));
+ m_reporterRef.ShowMsg("Installer info:" + m_manifest.Id);
+ m_reporterRef.ShowMsg("--Installer Language: " + m_selectedInstaller.Language);
+ m_reporterRef.ShowMsg("--Installer SHA256: " + Utility::SHA256::ConvertToString(m_selectedInstaller.Sha256));
+ m_reporterRef.ShowMsg("--Installer Download Url: " + m_selectedInstaller.Url);
+ m_reporterRef.ShowMsg("--Installer Type: " + Manifest::ManifestInstaller::InstallerTypeToString(m_selectedInstaller.InstallerType));
+ }
}
void ShowFlow::ShowAppVersion()
diff --git a/src/AppInstallerCLICore/Workflows/ShowFlow.h b/src/AppInstallerCLICore/Workflows/ShowFlow.h
@@ -7,12 +7,12 @@
namespace AppInstaller::Workflow
{
- class ShowFlow : public WorkflowBase
+ class ShowFlow : public SingleManifestWorkflow
{
public:
- ShowFlow(AppInstaller::CLI::ExecutionContext& context) : WorkflowBase(context) {}
+ ShowFlow(AppInstaller::CLI::ExecutionContext& context) : SingleManifestWorkflow(context) {}
- void Execute();;
+ void Execute();
protected:
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
@@ -1,10 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
-
#include "pch.h"
#include "WorkflowBase.h"
-#include "Public/AppInstallerRepositorySearch.h"
-#include "Public/AppInstallerRepositorySource.h"
+#include "ManifestComparator.h"
using namespace AppInstaller::CLI;
using namespace AppInstaller::Repository;
@@ -104,7 +102,30 @@ namespace AppInstaller::Workflow
return true;
}
- bool WorkflowBase::EnsureOneMatchFromSearchResult()
+ void WorkflowBase::ReportSearchResult()
+ {
+ for (auto& match : m_searchResult.Matches)
+ {
+ auto app = match.Application.get();
+ auto allVersions = app->GetVersions();
+
+ // Todo: Assume versions are sorted when returned so we'll use the first one as the latest version
+ // Need to call sort if the above is not the case.
+ std::string msg = app->GetId() + ", " + app->GetName() + ", " + allVersions.at(0).GetVersion().ToString();
+
+ if (match.MatchCriteria.Field != ApplicationMatchField::Id && match.MatchCriteria.Field != ApplicationMatchField::Name)
+ {
+ msg += ", [";
+ msg += ApplicationMatchFieldToString(match.MatchCriteria.Field);
+ msg += ": " + match.MatchCriteria.Value + "]";
+ }
+
+ Logging::Telemetry().LogSearchResultCount(m_searchResult.Matches.size());
+ m_reporterRef.ShowMsg(msg);
+ }
+ }
+
+ bool SingleManifestWorkflow::EnsureOneMatchFromSearchResult()
{
if (m_searchResult.Matches.size() == 0)
{
@@ -124,26 +145,43 @@ namespace AppInstaller::Workflow
return true;
}
- void WorkflowBase::ReportSearchResult()
+ bool SingleManifestWorkflow::GetManifest()
{
- for (auto& match : m_searchResult.Matches)
- {
- auto app = match.Application.get();
- auto allVersions = app->GetVersions();
+ auto app = m_searchResult.Matches.at(0).Application.get();
- // Todo: Assume versions are sorted when returned so we'll use the first one as the latest version
- // Need to call sort if the above is not the case.
- std::string msg = app->GetId() + ", " + app->GetName() + ", " + allVersions.at(0).GetVersion().ToString();
+ Logging::Telemetry().LogManifestFields(app->GetName(), app->GetId());
- if (match.MatchCriteria.Field != ApplicationMatchField::Id && match.MatchCriteria.Field != ApplicationMatchField::Name)
+ std::string_view version = (m_argsRef.Contains(ExecutionArgs::Type::Version) ? *m_argsRef.GetArg(ExecutionArgs::Type::Version) : std::string_view{});
+ std::string_view channel = (m_argsRef.Contains(ExecutionArgs::Type::Channel) ? *m_argsRef.GetArg(ExecutionArgs::Type::Channel) : std::string_view{});
+
+ std::optional<Manifest::Manifest> manifest = app->GetManifest(version, channel);
+
+ if (!manifest)
+ {
+ std::string message = "No version found matching ";
+ if (!version.empty())
{
- msg += ", [";
- msg += ApplicationMatchFieldToString(match.MatchCriteria.Field);
- msg += ": " + match.MatchCriteria.Value + "]";
+ message += version;
+ }
+ if (!channel.empty())
+ {
+ message += '[';
+ message += channel;
+ message += ']';
}
- Logging::Telemetry().LogSearchResultCount(m_searchResult.Matches.size());
- m_reporterRef.ShowMsg(msg);
+ m_reporterRef.ShowMsg(message, ExecutionReporter::Level::Warning);
+ return false;
}
+
+ m_manifest = std::move(manifest.value());
+
+ return true;
+ }
+
+ void SingleManifestWorkflow::SelectInstaller()
+ {
+ ManifestComparator manifestComparator(m_manifest, m_reporterRef);
+ m_selectedInstaller = manifestComparator.GetPreferredInstaller(m_argsRef);
}
}
\ No newline at end of file
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.h b/src/AppInstallerCLICore/Workflows/WorkflowBase.h
@@ -22,11 +22,23 @@ namespace AppInstaller::Workflow
bool IndexSearch();
- bool EnsureOneMatchFromSearchResult();
-
void ReportSearchResult();
std::shared_ptr<AppInstaller::Repository::ISource> m_source;
AppInstaller::Repository::SearchResult m_searchResult;
};
+
+ // A workflow that requires a single manifest to operate properly.
+ class SingleManifestWorkflow : public WorkflowBase
+ {
+ protected:
+ using WorkflowBase::WorkflowBase;
+
+ bool EnsureOneMatchFromSearchResult();
+ bool GetManifest();
+ void SelectInstaller();
+
+ AppInstaller::Manifest::Manifest m_manifest;
+ AppInstaller::Manifest::ManifestInstaller m_selectedInstaller;
+ };
}
\ No newline at end of file
diff --git a/src/AppInstallerCLICore/pch.h b/src/AppInstallerCLICore/pch.h
@@ -31,6 +31,7 @@
#include <AppInstallerErrors.h>
#include <AppInstallerLogging.h>
#include <AppInstallerMsixInfo.h>
+#include <AppInstallerRepositorySearch.h>
#include <AppInstallerRepositorySource.h>
#include <AppInstallerRuntime.h>
#include <AppInstallerSHA256.h>
diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp
@@ -721,6 +721,56 @@ TEST_CASE("SQLiteIndex_Search_VersionSorting", "[sqliteindex]")
}
}
+TEST_CASE("SQLiteIndex_PathString_VersionSorting", "[sqliteindex]")
+{
+ TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
+ INFO("Using temporary file named: " << tempFile.GetPath());
+
+ std::vector<VersionAndChannel> sortedList =
+ {
+ { Version("15.0.0"), Channel("") },
+ { Version("14.0.0"), Channel("") },
+ { Version("13.2.0-bugfix"), Channel("") },
+ { Version("13.2.0"), Channel("") },
+ { Version("13.0.0"), Channel("") },
+ { Version("16.0.0"), Channel("alpha") },
+ { Version("15.8.0"), Channel("alpha") },
+ { Version("15.1.0"), Channel("beta") },
+ };
+
+ SQLiteIndex index = SearchTestSetup(tempFile, {
+ { "Id", "Name", "Moniker", "14.0.0", "", { "foot" }, { "com34" }, "Path1" },
+ { "Id", "Name", "Moniker", "16.0.0", "alpha", { "floor" }, { "com3" }, "Path2" },
+ { "Id", "Name", "Moniker", "15.0.0", "", {}, { "Command" }, "Path3" },
+ { "Id", "Name", "Moniker", "13.2.0", "", {}, { "Command" }, "Path4" },
+ { "Id", "Name", "Moniker", "15.1.0", "beta", { "foo" }, { "com3" }, "Path5" },
+ { "Id", "Name", "Moniker", "15.8.0", "alpha", { "foo" }, { "com3" }, "Path6" },
+ { "Id", "Name", "Moniker", "13.2.0-bugfix", "", { "foo" }, { "com3" }, "Path7" },
+ { "Id", "Name", "Moniker", "13.0.0", "", { "foo" }, { "com3" }, "Path8" },
+ });
+
+ SearchRequest request;
+ request.Filters.emplace_back(ApplicationMatchField::Id, MatchType::Exact, "Id");
+
+ auto results = index.Search(request);
+ REQUIRE(results.size() == 1);
+
+ auto result = index.GetPathStringByKey(results[0].first, "", "");
+ REQUIRE(result.has_value());
+ REQUIRE(result.value() == "Path3");
+
+ result = index.GetPathStringByKey(results[0].first, "", "alpha");
+ REQUIRE(result.has_value());
+ REQUIRE(result.value() == "Path2");
+
+ result = index.GetPathStringByKey(results[0].first, "", "beta");
+ REQUIRE(result.has_value());
+ REQUIRE(result.value() == "Path5");
+
+ result = index.GetPathStringByKey(results[0].first, "", "gamma");
+ REQUIRE(!result.has_value());
+}
+
TEST_CASE("SQLiteIndex_SearchResultsTableSearches", "[sqliteindex][V1_0]")
{
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
diff --git a/src/AppInstallerCLITests/SQLiteIndexSource.cpp b/src/AppInstallerCLITests/SQLiteIndexSource.cpp
@@ -154,14 +154,19 @@ TEST_CASE("SQLiteIndexSource_GetManifest", "[sqliteindexsource]")
IApplication* app = results.Matches[0].Application.get();
auto specificResult = app->GetManifest(manifest.Version, manifest.Channel);
- REQUIRE(specificResult.Id == manifest.Id);
- REQUIRE(specificResult.Name == manifest.Name);
- REQUIRE(specificResult.Version == manifest.Version);
- REQUIRE(specificResult.Channel == manifest.Channel);
+ REQUIRE(specificResult.has_value());
+ REQUIRE(specificResult->Id == manifest.Id);
+ REQUIRE(specificResult->Name == manifest.Name);
+ REQUIRE(specificResult->Version == manifest.Version);
+ REQUIRE(specificResult->Channel == manifest.Channel);
auto latestResult = app->GetManifest("", manifest.Channel);
- REQUIRE(latestResult.Id == manifest.Id);
- REQUIRE(latestResult.Name == manifest.Name);
- REQUIRE(latestResult.Version == manifest.Version);
- REQUIRE(latestResult.Channel == manifest.Channel);
+ REQUIRE(latestResult.has_value());
+ REQUIRE(latestResult->Id == manifest.Id);
+ REQUIRE(latestResult->Name == manifest.Name);
+ REQUIRE(latestResult->Version == manifest.Version);
+ REQUIRE(latestResult->Channel == manifest.Channel);
+
+ auto noResult = app->GetManifest("blargle", "flargle");
+ REQUIRE(!noResult.has_value());
}
diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp
@@ -70,7 +70,7 @@ struct TestSource : public ISource
{
TestApplication(const Manifest manifest) : m_manifest(manifest) {}
- Manifest GetManifest(const NormalizedString&, const NormalizedString&) override
+ std::optional<Manifest> GetManifest(const NormalizedString&, const NormalizedString&) override
{
return m_manifest;
}
diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp
@@ -26,10 +26,15 @@ namespace AppInstaller::Repository::Microsoft
return GetSource()->GetIndex().GetNameStringById(m_id).value();
}
- Manifest::Manifest GetManifest(const Utility::NormalizedString& version, const Utility::NormalizedString& channel) override
+ std::optional<Manifest::Manifest> GetManifest(const Utility::NormalizedString& version, const Utility::NormalizedString& channel) override
{
std::shared_ptr<SQLiteIndexSource> source = GetSource();
- std::string relativePath = source->GetIndex().GetPathStringByKey(m_id, version, channel).value();
+ std::optional<std::string> relativePathOpt = source->GetIndex().GetPathStringByKey(m_id, version, channel);
+ if (!relativePathOpt)
+ {
+ return {};
+ }
+ std::string relativePath = relativePathOpt.value();
std::string fullPath = source->GetDetails().Arg;
if (fullPath.back() != '/')
diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.cpp
@@ -98,8 +98,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0
std::sort(versions.begin(), versions.end());
- // Get the first version in the list and its rowid
- const std::string& latestVersion = versions[0].ToString();
+ // Get the last version in the list (the highest version) and its rowid
+ const std::string& latestVersion = versions.back().ToString();
versionIdOpt = VersionTable::SelectIdByValue(connection, latestVersion);
}
else
diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h
@@ -81,7 +81,7 @@ namespace AppInstaller::Repository
// Gets a manifest for this application.
// An empty version implies 'latest'.
// An empty channel is the 'general audience'.
- virtual Manifest::Manifest GetManifest(const Utility::NormalizedString& version, const Utility::NormalizedString& channel) = 0;
+ virtual std::optional<Manifest::Manifest> GetManifest(const Utility::NormalizedString& version, const Utility::NormalizedString& channel) = 0;
// Gets all versions of this application.
// The versions will be returned in sorted, descending order.
