Skip to content

Uninstalling if cost of repair is larger than cost of content #55

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

Merged
merged 2 commits into from
Jun 12, 2018
Merged

Uninstalling if cost of repair is larger than cost of content #55

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc6453f
Uninstalling if cost of repair is larger than cost of content
Dzejkop Jun 7, 2018
dbb60ea
Calculating valid repair cost in pre update
Dzejkop Jun 7, 2018
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
40 changes: 22 additions & 18 deletions Assets/PatchKit Patcher/Scripts/AppUpdater/AppUpdater.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Linq;
using System.Linq.Expressions;
using System.Threading;
using System.Collections.Generic;
using PatchKit.Unity.Patcher.Cancellation;
using PatchKit.Unity.Patcher.Debug;
using PatchKit.Unity.Patcher.AppUpdater.Commands;
Expand Down Expand Up @@ -71,43 +72,46 @@ AppContentSummary latestVersionContentSummary
checkIntegrity.Prepare(_status);
checkIntegrity.Execute(cancellationToken);

int missingFilesCount = checkIntegrity.Results.Files
.Select(f => f.Status == FileIntegrityStatus.MissingData)
.Count();
var missingFiles = checkIntegrity.Results.Files
.Where(f => f.Status == FileIntegrityStatus.MissingData);

int missingFilesCount = missingFiles.Count();

int invalidSizeFilesCount = checkIntegrity.Results.Files
.Select(f => f.Status == FileIntegrityStatus.InvalidSize)
.Count();
var invalidSizeFiles = checkIntegrity.Results.Files
.Where(f => f.Status == FileIntegrityStatus.InvalidSize);

int invalidSizeFilesCount = invalidSizeFiles.Count();

if (missingFilesCount + invalidSizeFilesCount == 0)
{
DebugLogger.Log("No missing or invalid size files.");
return;
}

var repairStrategy = new AppUpdaterRepairAndDiffStrategy(Context, _status, performDiff: false);

double repairCost = (missingFilesCount + invalidSizeFilesCount) * 2;
if (isNewVersionAvailable)
{
repairCost *= latestVersionContentSummary.Chunks.Size;
}
else
{
repairCost *= installedVersionContentSummary.Chunks.Size;
}
double repairCost = CalculateRepairCost(installedVersionContentSummary, missingFiles.Concat(invalidSizeFiles));

if (repairCost < contentSize)
Copy link
Contributor

@genail genail Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repairCost seems to be a number of missing and broken files (x2). Is contentSize the content size in bytes?
This does not seem right.

Copy link
Contributor Author

@Dzejkop Dzejkop Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below that there is:

if (isNewVersionAvailable)
{
    repairCost *= latestVersionContentSummary.Chunks.Size;
}
else
{
    repairCost *= installedVersionContentSummary.Chunks.Size;
}

repairCost is estimated to be equal to the sum of missing and broken files, multiplied by two and then multiplied by chunk size in the respective version.

Copy link
Contributor

@genail genail Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't notice that.
I can't imagine it though. For instance if there's a large file broken, would the cost be the same as for small file?

Copy link
Contributor Author

@Dzejkop Dzejkop Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it now. The cost calculation will now use actual file sizes.

{
DebugLogger.Log(string.Format("Repair cost {0} is smaller than content cost {1}, repairing...", repairCost, contentSize));
IAppUpdaterStrategy repairStrategy = _strategyResolver.Create(StrategyType.RepairAndDiff, Context);
repairStrategy.Update(cancellationToken);
}
else
{
DebugLogger.Log("Content cost is smaller than repair.");
DebugLogger.Log("Content cost is smaller than repair. Uninstalling to prepare for content strategy.");
IUninstallCommand uninstall = commandFactory.CreateUninstallCommand(Context);
uninstall.Prepare(_status);
uninstall.Execute(cancellationToken);
}
}

private long CalculateRepairCost(AppContentSummary contentSummary, IEnumerable<FileIntegrity> filesToRepair)
{
return filesToRepair
.Select(f => contentSummary.Files.FirstOrDefault(e => e.Path == f.FileName))
.Sum(f => f.Size);
Copy link
Contributor

@genail genail Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it work? Select() transforms enumerable to a new form (https://msdn.microsoft.com/en-us/library/bb548891(v=vs.110).aspx). When a file is not found, a null is returned. Sum() is dereferencing the value so it looks like NRE for me.

Copy link
Contributor

@witcher112 witcher112 Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a file is not found, default value of struct AppContentSummaryFile is returned. There won't be any NRE here.

}

public void Update(CancellationToken cancellationToken)
{
Assert.MethodCalledOnlyOnce(ref _updateHasBeenCalled, "Update");
Expand Down