Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve progress handling for group processor #4121

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Improve progress handling for group processor #4121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e54f05d
Read in existing passthru results on progress
JohnMcPMS Jan 30, 2024
9a51ef0
Make progress explicit in group handler
JohnMcPMS Jan 31, 2024
d6d8595
Better tests
JohnMcPMS Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/AppInstallerSharedLib/Public/winget/AsyncTokens.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,29 @@ namespace AppInstaller::WinRT
private:
Token m_token;
};

// Type containing winrt EventHandler.
template <typename ResultT, typename ProgressT>
struct AsyncProgressEventHandlerT : public AsyncProgressTypeErasure<ResultT, ProgressT>
{
using Token = winrt::Windows::Foundation::EventHandler<ProgressT>;

AsyncProgressEventHandlerT(Token&& token) : m_token(std::move(token)) {}

void Progress(ProgressT const& progress) const override
{
m_token(m_result, progress);
}

void Result(ResultT const& result) const override
{
m_result = result;
}

private:
mutable ResultT m_result = nullptr;
Token m_token;
};
}

// May hold a progress token and provide the ability to send progress updates.
Expand All @@ -136,6 +159,14 @@ namespace AppInstaller::WinRT
m_token = std::make_unique<details::AsyncProgressT<Promise, ResultT, ProgressT>>(std::move(progress));
}

// Create a progress object from an EventHandler.
template <typename Promise>
AsyncProgress(winrt::Windows::Foundation::EventHandler<ProgressT>&& progress, winrt::impl::cancellation_token<Promise>&& cancellation) :
AsyncCancellation(std::move(cancellation))
{
m_token = std::make_unique<details::AsyncProgressEventHandlerT<ResultT, ProgressT>>(std::move(progress));
}

// Sends progress if this object is not empty.
void Progress(ProgressT const& progress) const
{
Expand Down
14 changes: 8 additions & 6 deletions ...icrosoft.Management.Configuration.UnitTests/Helpers/TestConfigurationSetGroupProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ public object? Group
/// <summary>
/// Apply settings for the group.
/// </summary>
/// <param name="progressHandler">The progress handler.</param>
/// <returns>The operation to apply settings.</returns>
public IAsyncOperationWithProgress<IApplyGroupSettingsResult, IApplyGroupMemberSettingsResult> ApplyGroupSettingsAsync()
public IAsyncOperation<IApplyGroupSettingsResult> ApplyGroupSettingsAsync(EventHandler<IApplyGroupMemberSettingsResult> progressHandler)
{
return AsyncInfo.Run((CancellationToken cancellationToken, IProgress<IApplyGroupMemberSettingsResult> progress) => Task.Run<IApplyGroupSettingsResult>(() =>
return AsyncInfo.Run((CancellationToken cancellationToken) => Task.Run<IApplyGroupSettingsResult>(() =>
{
this.WaitOnAsyncEvent(cancellationToken);

Expand All @@ -62,7 +63,7 @@ public IAsyncOperationWithProgress<IApplyGroupSettingsResult, IApplyGroupMemberS

if (this.Set != null)
{
TestConfigurationUnitGroupProcessor.ApplyGroupSettings(this.Set.Units, progress, result);
TestConfigurationUnitGroupProcessor.ApplyGroupSettings(this.Set.Units, progressHandler, result);
}

return result;
Expand All @@ -72,10 +73,11 @@ public IAsyncOperationWithProgress<IApplyGroupSettingsResult, IApplyGroupMemberS
/// <summary>
/// Test settings for the group.
/// </summary>
/// <param name="progressHandler">The progress handler.</param>
/// <returns>The operation to test settings.</returns>
public IAsyncOperationWithProgress<ITestGroupSettingsResult, ITestSettingsResult> TestGroupSettingsAsync()
public IAsyncOperation<ITestGroupSettingsResult> TestGroupSettingsAsync(EventHandler<ITestSettingsResult> progressHandler)
{
return AsyncInfo.Run((CancellationToken cancellationToken, IProgress<ITestSettingsResult> progress) => Task.Run<ITestGroupSettingsResult>(() =>
return AsyncInfo.Run((CancellationToken cancellationToken) => Task.Run<ITestGroupSettingsResult>(() =>
{
this.WaitOnAsyncEvent(cancellationToken);

Expand All @@ -85,7 +87,7 @@ public IAsyncOperationWithProgress<ITestGroupSettingsResult, ITestSettingsResult
if (this.Set != null)
{
result.TestResult = TestConfigurationUnitGroupProcessor.GetTestResult(this.Set.Metadata);
TestConfigurationUnitGroupProcessor.TestGroupSettings(this.Set.Units, progress, result);
TestConfigurationUnitGroupProcessor.TestGroupSettings(this.Set.Units, progressHandler, result);
}

return result;
Expand Down
28 changes: 14 additions & 14 deletions ...crosoft.Management.Configuration.UnitTests/Helpers/TestConfigurationUnitGroupProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,18 @@ public object Group
/// <summary>
/// Apply settings for the group.
/// </summary>
/// <param name="progressHandler">The progress handler.</param>
/// <returns>The operation to apply settings.</returns>
public IAsyncOperationWithProgress<IApplyGroupSettingsResult, IApplyGroupMemberSettingsResult> ApplyGroupSettingsAsync()
public IAsyncOperation<IApplyGroupSettingsResult> ApplyGroupSettingsAsync(EventHandler<IApplyGroupMemberSettingsResult> progressHandler)
{
return AsyncInfo.Run((CancellationToken cancellationToken, IProgress<IApplyGroupMemberSettingsResult> progress) => Task.Run<IApplyGroupSettingsResult>(() =>
return AsyncInfo.Run((CancellationToken cancellationToken) => Task.Run<IApplyGroupSettingsResult>(() =>
{
this.WaitOnAsyncEvent(cancellationToken);

ApplyGroupSettingsResultInstance result = new (this.Group);
result.UnitResults = new List<IApplyGroupMemberSettingsResult>();

ApplyGroupSettings(this.Unit.Units, progress, result);
ApplyGroupSettings(this.Unit.Units, progressHandler, result);

return result;
}));
Expand All @@ -74,18 +75,19 @@ public IAsyncOperationWithProgress<IApplyGroupSettingsResult, IApplyGroupMemberS
/// <summary>
/// Test settings for the group.
/// </summary>
/// <param name="progressHandler">The progress handler.</param>
/// <returns>The operation to test settings.</returns>
public IAsyncOperationWithProgress<ITestGroupSettingsResult, ITestSettingsResult> TestGroupSettingsAsync()
public IAsyncOperation<ITestGroupSettingsResult> TestGroupSettingsAsync(EventHandler<ITestSettingsResult> progressHandler)
{
return AsyncInfo.Run((CancellationToken cancellationToken, IProgress<ITestSettingsResult> progress) => Task.Run<ITestGroupSettingsResult>(() =>
return AsyncInfo.Run((CancellationToken cancellationToken) => Task.Run<ITestGroupSettingsResult>(() =>
{
this.WaitOnAsyncEvent(cancellationToken);

TestGroupSettingsResultInstance result = new (this.Group);
result.UnitResults = new List<ITestSettingsResult>();

result.TestResult = GetTestResult(this.Unit.Metadata);
TestGroupSettings(this.Unit.Units, progress, result);
TestGroupSettings(this.Unit.Units, progressHandler, result);

return result;
}));
Expand Down Expand Up @@ -126,16 +128,17 @@ internal static ConfigurationTestResult GetTestResult(ValueSet values)
/// <param name="groupMembers">The group members.</param>
/// <param name="progress">The progress reporting object.</param>
/// <param name="result">The result object.</param>
internal static void ApplyGroupSettings(IList<ConfigurationUnit>? groupMembers, IProgress<IApplyGroupMemberSettingsResult> progress, ApplyGroupSettingsResultInstance result)
internal static void ApplyGroupSettings(IList<ConfigurationUnit>? groupMembers, EventHandler<IApplyGroupMemberSettingsResult> progress, ApplyGroupSettingsResultInstance result)
{
if (groupMembers != null)
{
foreach (ConfigurationUnit unit in groupMembers)
{
ApplyGroupMemberSettingsResultInstance unitResult = new (unit);
result.UnitResults!.Add(unitResult);

unitResult.State = ConfigurationUnitState.InProgress;
progress.Report(unitResult);
progress.Invoke(null, unitResult);

unitResult.PreviouslyInDesiredState = GetTestResult(unit) == ConfigurationTestResult.Positive;

Expand All @@ -145,9 +148,7 @@ internal static void ApplyGroupSettings(IList<ConfigurationUnit>? groupMembers,
}

unitResult.State = ConfigurationUnitState.Completed;
progress.Report(unitResult);

result.UnitResults!.Add(unitResult);
progress.Invoke(null, unitResult);
}
}
}
Expand All @@ -158,7 +159,7 @@ internal static void ApplyGroupSettings(IList<ConfigurationUnit>? groupMembers,
/// <param name="groupMembers">The group members.</param>
/// <param name="progress">The progress reporting object.</param>
/// <param name="result">The result object.</param>
internal static void TestGroupSettings(IList<ConfigurationUnit>? groupMembers, IProgress<ITestSettingsResult> progress, TestGroupSettingsResultInstance result)
internal static void TestGroupSettings(IList<ConfigurationUnit>? groupMembers, EventHandler<ITestSettingsResult> progress, TestGroupSettingsResultInstance result)
{
if (groupMembers != null)
{
Expand All @@ -172,9 +173,8 @@ internal static void TestGroupSettings(IList<ConfigurationUnit>? groupMembers, I
}

unitResult.TestResult = GetTestResult(unit);
progress.Report(unitResult);

result.UnitResults!.Add(unitResult);
progress.Invoke(null, unitResult);
}
}
}
Expand Down
37 changes: 22 additions & 15 deletions src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationProcessorGroupTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace Microsoft.Management.Configuration.UnitTests.Tests
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using Microsoft.Management.Configuration.UnitTests.Fixtures;
using Microsoft.Management.Configuration.UnitTests.Helpers;
using Xunit;
Expand Down Expand Up @@ -416,24 +417,27 @@ public void ApplySet_SetGroupProcessor()
configurationSet.Units = new ConfigurationUnit[] { configurationUnitNegative, configurationUnitPositive };

TestConfigurationProcessorFactory factory = new TestConfigurationProcessorFactory();
TestConfigurationSetGroupProcessor setProcessor = factory.CreateTestGroupProcessor(configurationSet);
setProcessor.ShouldWaitOnAsyncEvent = true;

ConfigurationProcessor processor = this.CreateConfigurationProcessorWithDiagnostics(factory);
List<ConfigurationSetChangeData> progressValues = new List<ConfigurationSetChangeData>();
ManualResetEvent startProcessing = new ManualResetEvent(false);
factory.CreateSetProcessorDelegate = (f, c) =>
{
startProcessing.WaitOne();
return f.CreateTestGroupProcessor(c);
};

var operation = processor.ApplySetAsync(configurationSet, ApplyConfigurationSetFlags.None);
operation.Progress = (Windows.Foundation.IAsyncOperationWithProgress<ApplyConfigurationSetResult, ConfigurationSetChangeData> op, ConfigurationSetChangeData unitResult) => { progressValues.Add(unitResult); };
setProcessor.SignalAsyncEvent();

List<ConfigurationSetChangeData> progressValues = new List<ConfigurationSetChangeData>();
operation.Progress = (Windows.Foundation.IAsyncOperationWithProgress<ApplyConfigurationSetResult, ConfigurationSetChangeData> op, ConfigurationSetChangeData unitResult) => { progressValues.Add(unitResult); };
startProcessing.Set();
operation.AsTask().Wait();
ApplyConfigurationSetResult result = operation.GetResults();

Assert.NotNull(result);
Assert.Null(result.ResultCode);
Assert.NotNull(result.UnitResults);
Assert.Equal(configurationSet.Units.Count, result.UnitResults.Count);
Assert.True((configurationSet.Units.Count * 2) + 2 >= progressValues.Count);
Assert.Equal((configurationSet.Units.Count * 2) + 2, progressValues.Count);

ApplyConfigurationUnitResult negativeResult = result.UnitResults.First(x => x.Unit == configurationUnitNegative);

Expand Down Expand Up @@ -505,24 +509,27 @@ public void ApplySet_SetGroupProcessor_WithGroupUnit()
configurationUnitGroup.Units = new ConfigurationUnit[] { configurationUnitGroupMember };

TestConfigurationProcessorFactory factory = new TestConfigurationProcessorFactory();
TestConfigurationSetGroupProcessor setProcessor = factory.CreateTestGroupProcessor(configurationSet);
setProcessor.ShouldWaitOnAsyncEvent = true;

ConfigurationProcessor processor = this.CreateConfigurationProcessorWithDiagnostics(factory);
List<ConfigurationSetChangeData> progressValues = new List<ConfigurationSetChangeData>();
ManualResetEvent startProcessing = new ManualResetEvent(false);
factory.CreateSetProcessorDelegate = (f, c) =>
{
startProcessing.WaitOne();
return f.CreateTestGroupProcessor(c);
};

var operation = processor.ApplySetAsync(configurationSet, ApplyConfigurationSetFlags.None);
operation.Progress = (Windows.Foundation.IAsyncOperationWithProgress<ApplyConfigurationSetResult, ConfigurationSetChangeData> op, ConfigurationSetChangeData unitResult) => { progressValues.Add(unitResult); };
setProcessor.SignalAsyncEvent();

List<ConfigurationSetChangeData> progressValues = new List<ConfigurationSetChangeData>();
operation.Progress = (Windows.Foundation.IAsyncOperationWithProgress<ApplyConfigurationSetResult, ConfigurationSetChangeData> op, ConfigurationSetChangeData unitResult) => { progressValues.Add(unitResult); };
startProcessing.Set();
operation.AsTask().Wait();
ApplyConfigurationSetResult result = operation.GetResults();

Assert.NotNull(result);
Assert.Null(result.ResultCode);
Assert.NotNull(result.UnitResults);
Assert.Equal(3, result.UnitResults.Count);
Assert.True(progressValues.Count <= 2 + (3 * 2));
Assert.Equal(2 + (3 * 2), progressValues.Count);

foreach (ConfigurationUnit unit in new ConfigurationUnit[] { configurationUnit, configurationUnitGroup, configurationUnitGroupMember })
{
Expand All @@ -536,7 +543,7 @@ public void ApplySet_SetGroupProcessor_WithGroupUnit()
Assert.True(unitResult.PreviouslyInDesiredState);

IEnumerable<ConfigurationSetChangeData> unitProgress = progressValues.Where(x => x.Unit == unit);
Assert.True(unitProgress.Count() <= 2);
Assert.Equal(2, unitProgress.Count());

foreach (ConfigurationSetChangeData change in unitProgress)
{
Expand Down
15 changes: 5 additions & 10 deletions src/Microsoft.Management.Configuration/ConfigurationProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,9 @@ namespace winrt::Microsoft::Management::Configuration::implementation
}
CATCH_LOG();

auto applyOperation = groupProcessor.ApplyGroupSettingsAsync();

// Forward unit result progress to caller
applyOperation.Progress([&](const auto&, const IApplyGroupMemberSettingsResult& unitResult)
auto applyOperation = groupProcessor.ApplyGroupSettingsAsync([&](const auto&, const IApplyGroupMemberSettingsResult& unitResult)
{
// Update overall result
auto itr = unitResultMap.find(unitResult.Unit().InstanceIdentifier());
if (itr != unitResultMap.end())
{
Expand Down Expand Up @@ -597,13 +594,13 @@ namespace winrt::Microsoft::Management::Configuration::implementation

try
{
auto testOperation = groupProcessor.TestGroupSettingsAsync();

// Forward unit result progress to caller
testOperation.Progress([&](const auto&, const ITestSettingsResult& unitResult)
auto testOperation = groupProcessor.TestGroupSettingsAsync([&](const auto&, const ITestSettingsResult& unitResult)
{
auto testResult = make_self<wil::details::module_count_wrapper<implementation::TestConfigurationUnitResult>>();
testResult->Initialize(unitResult);

result->AppendUnitResult(*testResult);
progress.Progress(*testResult);
});

Expand All @@ -612,7 +609,7 @@ namespace winrt::Microsoft::Management::Configuration::implementation

ITestGroupSettingsResult testResult = testOperation.get();

// Place all results from the processor into our result
// Send telemetry for all results
for (const ITestSettingsResult& unitResult : testResult.UnitResults())
{
auto testUnitResult = make_self<wil::details::module_count_wrapper<implementation::TestConfigurationUnitResult>>();
Expand All @@ -624,8 +621,6 @@ namespace winrt::Microsoft::Management::Configuration::implementation
ConfigurationUnitIntent::Assert,
TelemetryTraceLogger::TestAction,
testUnitResult->ResultInformation());

result->AppendUnitResult(*testUnitResult);
}

m_threadGlobals.GetTelemetryLogger().LogConfigProcessingSummaryForTest(*winrt::get_self<implementation::ConfigurationSet>(configurationSet), *result);
Expand Down
4 changes: 1 addition & 3 deletions src/Microsoft.Management.Configuration/ConfigurationSetApplyProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,7 @@ namespace winrt::Microsoft::Management::Configuration::implementation

if (groupProcessor)
{
auto applyOperation = groupProcessor.ApplyGroupSettingsAsync();

applyOperation.Progress([&](const auto&, const IApplyGroupMemberSettingsResult& unitResult)
auto applyOperation = groupProcessor.ApplyGroupSettingsAsync([&](const auto&, const IApplyGroupMemberSettingsResult& unitResult)
{
m_progress.Progress(unitResult);
});
Expand Down
Loading
Loading