commit 21f3c96a23e6f2c1c9593237f8fe971549d74a6f
parent 965c05d0730770646fa2348b38375b2a93b50f25
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Mon, 23 Mar 2020 12:43:58 -0700
Shorten auto-update time, retry on update failure, don't call deployment if no change (#65)
Diffstat:
4 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp
@@ -328,6 +328,43 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]")
REQUIRE((now - sources[0].LastUpdateTime) < 1s);
}
+TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]")
+{
+ using namespace std::chrono_literals;
+
+ RemoveSetting(s_RepositorySettings_UserSources);
+ TestHook_ClearSourceFactoryOverrides();
+
+ std::string name = "thisIsTheName";
+ std::string type = "thisIsTheType";
+ std::string arg = "thisIsTheArg";
+ std::string data = "thisIsTheData";
+
+ TestSourceFactory factory;
+ TestHook_SetSourceFactoryOverride(type, factory);
+
+ ProgressCallback progress;
+ AddSource(name, type, arg, progress);
+
+ // Reset for a call to update
+ bool updateShouldThrow = false;
+ bool updateCalledOnFactoryAgain = false;
+ factory.m_Update = [&](SourceDetails& sd)
+ {
+ if (updateShouldThrow)
+ {
+ updateShouldThrow = false;
+ THROW_HR(E_ACCESSDENIED);
+ }
+ updateCalledOnFactoryAgain = true;
+ sd.Data = data;
+ };
+
+ UpdateSource(name, progress);
+
+ REQUIRE(updateCalledOnFactoryAgain);
+}
+
TEST_CASE("RepoSources_RemoveSource", "[sources]")
{
RemoveSetting(s_RepositorySettings_UserSources);
diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp
@@ -160,8 +160,38 @@ namespace AppInstaller::Repository::Microsoft
return std::make_shared<SQLiteIndexSource>(details, std::move(index), std::move(lock));
}
- void UpdateInternal(std::string packageLocation, SourceDetails&, IProgressCallback& progress) override
+ void UpdateInternal(std::string packageLocation, SourceDetails& details, IProgressCallback& progress) override
{
+ // Check if the package is newer before calling into deployment.
+ // This can save us a lot of time over letting deployment detect same version.
+ auto optionalPackage = GetPackageFromDetails(details);
+ if (optionalPackage)
+ {
+ Msix::MsixInfo packageInfo(packageLocation);
+ THROW_HR_IF(APPINSTALLER_CLI_ERROR_PACKAGE_IS_BUNDLE, packageInfo.GetIsBundle());
+
+ if (progress.IsCancelled())
+ {
+ AICLI_LOG(Repo, Info, << "Cancelling update upon request");
+ return;
+ }
+
+ std::filesystem::path packagePath = optionalPackage.InstalledLocation().Path().c_str();
+ std::filesystem::path manifestPath = packagePath / s_PreIndexedPackageSourceFactory_AppxManifestFileName;
+
+ if (!packageInfo.IsNewerThan(manifestPath))
+ {
+ AICLI_LOG(Repo, Info, << "Remote source data was not newer than existing, no update needed");
+ return;
+ }
+ }
+
+ if (progress.IsCancelled())
+ {
+ AICLI_LOG(Repo, Info, << "Cancelling update upon request");
+ return;
+ }
+
winrt::Windows::Foundation::Uri uri(Utility::ConvertToUTF16(packageLocation));
Deployment::RequestAddPackageAsync(uri, winrt::Windows::Management::Deployment::DeploymentOptions::None, progress);
}
@@ -200,6 +230,12 @@ namespace AppInstaller::Repository::Microsoft
std::filesystem::path packageLocation = GetStatePathFromDetails(details);
packageLocation /= s_PreIndexedPackageSourceFactory_IndexFileName;
+ if (!std::filesystem::exists(packageLocation))
+ {
+ AICLI_LOG(Repo, Info, << "Data not found by family name " << details.Data);
+ THROW_HR(APPINSTALLER_CLI_ERROR_SOURCE_DATA_MISSING);
+ }
+
SQLiteIndex index = SQLiteIndex::Open(packageLocation.u8string(), SQLiteIndex::OpenDisposition::Read);
return std::make_shared<SQLiteIndexSource>(details, std::move(index), std::move(lock));
diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp
@@ -194,7 +194,21 @@ namespace AppInstaller::Repository
void UpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress)
{
- GetFactoryForType(details.Type)->Update(details, progress);
+ auto factory = GetFactoryForType(details.Type);
+
+ // Attempt to update; if it fails, wait a short time and retry.
+ try
+ {
+ factory->Update(details, progress);
+ return;
+ }
+ CATCH_LOG();
+
+ AICLI_LOG(Repo, Info, << "Source update failed, waiting a bit and retrying: " << details.Name);
+ std::this_thread::sleep_for(2s);
+
+ // If this one fails, maybe the problem is persistent.
+ factory->Update(details, progress);
}
void RemoveSourceFromDetails(const SourceDetails& details, IProgressCallback& progress)
@@ -221,14 +235,14 @@ namespace AppInstaller::Repository
}
// TODO: Enable some amount of user control over this.
- constexpr static auto s_DefaultAutoUpdateTime = 12h;
+ constexpr static auto s_DefaultAutoUpdateTime = 5min;
auto timeSinceLastUpdate = std::chrono::system_clock::now() - details.LastUpdateTime;
if (timeSinceLastUpdate > s_DefaultAutoUpdateTime)
{
AICLI_LOG(Repo, Info, << "Source past auto update time [" <<
- std::chrono::duration_cast<std::chrono::hours>(s_DefaultAutoUpdateTime).count() << " hours]; it has been at least " <<
- std::chrono::duration_cast<std::chrono::hours>(timeSinceLastUpdate).count() << " hours");
+ std::chrono::duration_cast<std::chrono::minutes>(s_DefaultAutoUpdateTime).count() << " mins]; it has been at least " <<
+ std::chrono::duration_cast<std::chrono::minutes>(timeSinceLastUpdate).count() << " mins");
return true;
}
diff --git a/src/AppInstallerRepositoryCore/pch.h b/src/AppInstallerRepositoryCore/pch.h
@@ -37,6 +37,7 @@
#include <string_view>
#include <sstream>
#include <system_error>
+#include <thread>
#include <tuple>
#include <type_traits>
#include <utility>
