Skip to content

Commit

Permalink
Use package version as potential last update timestamp (#3759)
Browse files Browse the repository at this point in the history
This change moves the background update determination to be under the control of the source reference, and then uses the package version (which is based on the creation time for us) as a potential later "last update time".  This enables an update to the package from outside of our control to still prevent any update check from occurring.
  • Loading branch information
JohnMcPMS authored Oct 13, 2023
1 parent 54d6e5b commit 546b509
Show file tree
Hide file tree
Showing 18 changed files with 358 additions and 99 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ minexample
minidump
minschema
missingdependency
mkgmtime
MMmmbbbb
mof
monicka
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@
<ClCompile Include="CompositeSource.cpp" />
<ClCompile Include="Correlation.cpp" />
<ClCompile Include="CustomHeader.cpp" />
<ClCompile Include="DateTime.cpp" />
<ClCompile Include="Dependencies.cpp" />
<ClCompile Include="Downloader.cpp" />
<ClCompile Include="DownloadFlow.cpp" />
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@
<ClCompile Include="CheckpointDatabase.cpp">
<Filter>Source Files\Repository</Filter>
</ClCompile>
<ClCompile Include="DateTime.cpp">
<Filter>Source Files\Common</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down
84 changes: 84 additions & 0 deletions src/AppInstallerCLITests/DateTime.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#include "TestCommon.h"
#include <AppInstallerDateTime.h>

using namespace AppInstaller::Utility;
using namespace TestCommon;
using namespace std::chrono;

namespace Catch
{
template<>
struct StringMaker<std::chrono::system_clock::time_point>
{
static std::string convert(const std::chrono::system_clock::time_point& value)
{
std::ostringstream stream;
OutputTimePoint(stream, value);
return std::move(stream).str();
}
};
}

void VerifyGetTimePointFromVersion(std::string_view version, int year, int month, int day, int hour, int minute)
{
system_clock::time_point result = GetTimePointFromVersion(UInt64Version{ std::string{ version } });

tm time{};
auto tt = system_clock::to_time_t(result);
_gmtime64_s(&time, &tt);

REQUIRE(year == time.tm_year + 1900);
REQUIRE(month == time.tm_mon + 1);
REQUIRE(day == time.tm_mday);
REQUIRE(hour == time.tm_hour);
REQUIRE(minute == time.tm_min);
}

std::string StringFromTimePoint(system_clock::time_point input)
{
tm time{};
auto tt = system_clock::to_time_t(input);
_gmtime64_s(&time, &tt);

std::ostringstream stream;
stream << time.tm_year + 1900 << '.' << ((time.tm_mon + 1) * 100) + time.tm_mday << '.' << ((time.tm_hour + 1) * 100) + time.tm_min;
return std::move(stream).str();
}

TEST_CASE("GetTimePointFromVersion", "[datetime]")
{
// Years out of range
REQUIRE(GetTimePointFromVersion(UInt64Version{ "1969.1231.2459.0" }) == system_clock::time_point::min());
REQUIRE(GetTimePointFromVersion(UInt64Version{ "3001.101.100.0" }) == system_clock::time_point::min());

// Months out of range
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.1.100.0" }) == system_clock::time_point::min());
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.1301.100.0" }) == system_clock::time_point::min());

// Days out of range
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.100.100.0" }) == system_clock::time_point::min());
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.132.100.0" }) == system_clock::time_point::min());

// Hours out of range
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.101.0.0" }) == system_clock::time_point::min());
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.101.2500.0" }) == system_clock::time_point::min());

// Minutes out of range
REQUIRE(GetTimePointFromVersion(UInt64Version{ "2023.101.160.0" }) == system_clock::time_point::min());

// In range baseline
VerifyGetTimePointFromVersion("2023.101.100.0", 2023, 1, 1, 0, 0);

// Time for presents!
VerifyGetTimePointFromVersion("2023.1225.814.0", 2023, 12, 25, 7, 14);

// Epoch time
REQUIRE(GetTimePointFromVersion(UInt64Version{ "1970.101.100.0" }) == system_clock::time_point{});

// Round trip now
system_clock::time_point now = system_clock::now();
REQUIRE(GetTimePointFromVersion(UInt64Version{ StringFromTimePoint(now) }) == time_point_cast<minutes>(now));
}
12 changes: 8 additions & 4 deletions src/AppInstallerCLITests/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -644,17 +644,21 @@ TEST_CASE("InstallFlow_Portable_SymlinkCreationFail", "[InstallFlow][workflow]")
OverridePortableInstaller(installContext);
TestHook::SetCreateSymlinkResult_Override createSymlinkResultOverride(false);
const auto& targetDirectory = tempDirectory.GetPath();
const auto& portableTargetPath = targetDirectory / "AppInstallerTestExeInstaller.exe";
installContext.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Portable.yaml").GetPath().u8string());
installContext.Args.AddArg(Execution::Args::Type::InstallLocation, targetDirectory.u8string());
installContext.Args.AddArg(Execution::Args::Type::InstallScope, "user"sv);

InstallCommand install({});
install.Execute(installContext);
INFO(installOutput.str());

const auto& portableTargetPath = targetDirectory / "AppInstallerTestExeInstaller.exe";
REQUIRE(std::filesystem::exists(portableTargetPath));
REQUIRE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
{
INFO(installOutput.str());

// Use CHECK to allow the uninstall to still occur
CHECK(std::filesystem::exists(portableTargetPath));
CHECK(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
}

// Perform uninstall
std::ostringstream uninstallOutput;
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/Sources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]")
bool updateCalledOnFactory = false;
TestSourceFactory factory{ SourcesTestSource::Create };
factory.OnUpdate = [&](const SourceDetails&) { updateCalledOnFactory = true; };
factory.ShouldUpdateBeforeOpenResult = true;
TestHook_SetSourceFactoryOverride(type, factory);

SetSetting(Stream::UserSources, s_SingleSource);
Expand Down
10 changes: 8 additions & 2 deletions src/AppInstallerCLITests/TestSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,20 @@ namespace TestCommon

std::shared_ptr<ISourceReference> TestSourceFactory::Create(const SourceDetails& details)
{
std::shared_ptr<TestSourceReference> result;

if (OnOpenWithCustomHeader)
{
return std::make_shared<TestSourceReference>(details, OnOpenWithCustomHeader);
result = std::make_shared<TestSourceReference>(details, OnOpenWithCustomHeader);
}
else
{
return std::make_shared<TestSourceReference>(details, OnOpen);
result = std::make_shared<TestSourceReference>(details, OnOpen);
}

result->ShouldUpdateBeforeOpenResult = ShouldUpdateBeforeOpenResult;

return result;
}

bool TestSourceFactory::Add(SourceDetails& details, IProgressCallback&)
Expand Down
4 changes: 4 additions & 0 deletions src/AppInstallerCLITests/TestSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ namespace TestCommon

bool SetCustomHeader(std::optional<std::string> header) override { m_header = header; return true; }

bool ShouldUpdateBeforeOpenResult = false;
bool ShouldUpdateBeforeOpen(const std::optional<AppInstaller::Repository::TimeSpan>&) override { return ShouldUpdateBeforeOpenResult; }

std::shared_ptr<AppInstaller::Repository::ISource> Open(AppInstaller::IProgressCallback&) override
{
if (m_onOpenWithCustomHeader)
Expand Down Expand Up @@ -156,6 +159,7 @@ namespace TestCommon
// Make copies of self when requested.
operator std::function<std::unique_ptr<AppInstaller::Repository::ISourceFactory>()>();

bool ShouldUpdateBeforeOpenResult = false;
OpenFunctor OnOpen;
OpenFunctorWithCustomHeader OnOpenWithCustomHeader;
AddFunctor OnAdd;
Expand Down
1 change: 0 additions & 1 deletion src/AppInstallerCommonCore/Public/winget/MsixManifest.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#pragma once

#include "AppInstallerStrings.h"
#include "AppInstallerVersions.h"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@
<ClInclude Include="SourceFactory.h" />
<ClInclude Include="SourceList.h" />
<ClInclude Include="SourcePolicy.h" />
<ClInclude Include="SourceUpdateChecks.h" />
<ClInclude Include="SQLiteStatementBuilder.h" />
<ClInclude Include="SQLiteTempTable.h" />
<ClInclude Include="SQLiteWrapper.h" />
Expand Down Expand Up @@ -541,6 +542,7 @@
<ClCompile Include="Rest\Schema\SearchResponseParser.cpp" />
<ClCompile Include="SourceList.cpp" />
<ClCompile Include="SourcePolicy.cpp" />
<ClCompile Include="SourceUpdateChecks.cpp" />
<ClCompile Include="SQLiteStatementBuilder.cpp" />
<ClCompile Include="SQLiteTempTable.cpp" />
<ClCompile Include="SQLiteWrapper.cpp" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,9 @@
<ClInclude Include="Public\winget\Checkpoint.h">
<Filter>Public\winget</Filter>
</ClInclude>
<ClInclude Include="SourceUpdateChecks.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -653,9 +656,12 @@
<ClCompile Include="Microsoft\Schema\Checkpoint_1_0\CheckpointDatabaseInterface_1_0.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClInclude Include="Microsoft\Schema\Checkpoint_1_0\CheckpointTable.cpp">
<ClCompile Include="Microsoft\Schema\Checkpoint_1_0\CheckpointTable.cpp">
<Filter>Microsoft\Schema\Checkpoint_1_0</Filter>
</ClInclude>
</ClCompile>
<ClCompile Include="SourceUpdateChecks.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerRepositoryCore/ISource.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ namespace AppInstaller::Repository
// Set caller.
virtual void SetCaller(std::string) {}

// Determine if the source needs to be updated before being opened.
virtual bool ShouldUpdateBeforeOpen(const std::optional<TimeSpan>&) { return false; }

// Opens the source. This function should throw upon open failure rather than returning an empty pointer.
virtual std::shared_ptr<ISource> Open(IProgressCallback& progress) = 0;
};
Expand Down
Loading

0 comments on commit 546b509

Please sign in to comment.