Skip to content

Commit

Permalink
Add duplicate id check for existing package identifiers (#54)
Browse files Browse the repository at this point in the history
* Add logic for checking duplicate package ID

* clean up

* Update variable name to ProgramHeader

* remove whitespace

* Remove case sensitivity for update command

* Prompt package id separately
  • Loading branch information
ryfu-msft authored Jun 4, 2021
1 parent a5a01e5 commit 8e6ec72
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 26 deletions.
60 changes: 45 additions & 15 deletions src/WingetCreateCLI/Commands/NewCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ namespace Microsoft.WingetCreateCLI.Commands
using Microsoft.WingetCreateCLI.Properties;
using Microsoft.WingetCreateCLI.Telemetry;
using Microsoft.WingetCreateCLI.Telemetry.Events;
using Microsoft.WingetCreateCore;
using Microsoft.WingetCreateCore;
using Microsoft.WingetCreateCore.Common;
using Microsoft.WingetCreateCore.Models;
using Microsoft.WingetCreateCore.Models.DefaultLocale;
using Microsoft.WingetCreateCore.Models.Installer;
Expand Down Expand Up @@ -114,7 +115,7 @@ public override async Task<bool> Execute()
{
Logger.ErrorLocalized(nameof(Resources.PackageParsing_Error));
return false;
}
}

Console.WriteLine(Resources.NewCommand_Header);
Console.WriteLine();
Expand All @@ -126,7 +127,14 @@ public override async Task<bool> Execute()
Logger.DebugLocalized(nameof(Resources.EnterFollowingFields_Message));

do
{
{
if (!await this.PromptPackageIdentifierAndCheckDuplicates(manifests))
{
Console.WriteLine();
Logger.ErrorLocalized(nameof(Resources.PackageIdAlreadyExists_Error));
return false;
}

PromptRequiredProperties(manifests.VersionManifest);
PromptRequiredProperties(manifests.InstallerManifest, manifests.VersionManifest);
PromptRequiredProperties(manifests.DefaultLocaleManifest, manifests.VersionManifest);
Expand All @@ -140,8 +148,8 @@ public override async Task<bool> Execute()
this.OutputDir = Directory.GetCurrentDirectory();
}

string manifestDirectoryPath = SaveManifestDirToLocalPath(manifests, this.OutputDir);

string manifestDirectoryPath = SaveManifestDirToLocalPath(manifests, this.OutputDir);

if (!ValidateManifest(manifestDirectoryPath) ||
!Prompt.Confirm(Resources.ConfirmGitHubSubmitManifest_Message) ||
!await this.SetAndCheckGitHubToken())
Expand All @@ -159,7 +167,7 @@ public override async Task<bool> Execute()
{
TelemetryManager.Log.WriteEvent(commandEvent);
}
}
}

private static void PromptRequiredProperties<T>(T manifest, VersionManifest versionManifest = null)
{
Expand All @@ -180,17 +188,13 @@ private static void PromptRequiredProperties<T>(T manifest, VersionManifest vers
}
else if (property.PropertyType.IsValueType || property.PropertyType == typeof(string))
{
if (property.Name == nameof(VersionManifest.ManifestType) || property.Name == nameof(VersionManifest.ManifestVersion))
if (property.Name == nameof(VersionManifest.PackageIdentifier) ||
property.Name == nameof(VersionManifest.ManifestType) ||
property.Name == nameof(VersionManifest.ManifestVersion))
{
continue;
}

if ((property.Name == nameof(VersionManifest.PackageIdentifier)) && versionManifest != null)
{
property.SetValue(manifest, versionManifest.PackageIdentifier);
continue;
}

if (property.Name == nameof(VersionManifest.PackageVersion) && versionManifest != null)
{
property.SetValue(manifest, versionManifest.PackageVersion);
Expand All @@ -209,8 +213,8 @@ private static void PromptRequiredProperties<T>(T manifest, VersionManifest vers
Logger.Trace($"Property [{property.Name}] set to the value [{result}]");
}
}
}

}

private static void PromptInstallerProperties<T>(T manifest, PropertyInfo property)
{
List<Installer> installers = new List<Installer>((ICollection<Installer>)property.GetValue(manifest));
Expand Down Expand Up @@ -290,6 +294,32 @@ private static T PromptProperty<T>(object model, T property, string memberName,
var promptResult = Prompt.Input<T>(message, property, new[] { FieldValidation.ValidateProperty(model, memberName, property) });
return promptResult is string str ? (T)(object)str.Trim() : promptResult;
}
}

/// <summary>
/// Prompts for the package identifier and checks if the package identifier already exists.
/// If the package identifier is valid, the value is applied to the other manifests.
/// </summary>
/// <param name="manifests">Manifests object model.</param>
/// <returns>Boolean value indicating whether the package identifier is valid.</returns>
private async Task<bool> PromptPackageIdentifierAndCheckDuplicates(Manifests manifests)
{
VersionManifest versionManifest = manifests.VersionManifest;
versionManifest.PackageIdentifier = PromptProperty(versionManifest, versionManifest.PackageIdentifier, nameof(versionManifest.PackageIdentifier));

GitHub client = new GitHub(null, this.WingetRepoOwner, this.WingetRepo);

string exactMatch = await client.FindPackageId(versionManifest.PackageIdentifier);
if (!string.IsNullOrEmpty(exactMatch))
{
return false;
}
else
{
manifests.InstallerManifest.PackageIdentifier = versionManifest.PackageIdentifier;
manifests.DefaultLocaleManifest.PackageIdentifier = versionManifest.PackageIdentifier;
return true;
}
}
}
}
24 changes: 16 additions & 8 deletions src/WingetCreateCLI/Commands/UpdateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ namespace Microsoft.WingetCreateCLI.Commands
using Microsoft.WingetCreateCLI.Properties;
using Microsoft.WingetCreateCLI.Telemetry;
using Microsoft.WingetCreateCLI.Telemetry.Events;
using Microsoft.WingetCreateCore;
using Microsoft.WingetCreateCore;
using Microsoft.WingetCreateCore.Common;
using Microsoft.WingetCreateCore.Models;
using Microsoft.WingetCreateCore.Models.DefaultLocale;
using Microsoft.WingetCreateCore.Models.Installer;
using Microsoft.WingetCreateCore.Models.Locale;
using Microsoft.WingetCreateCore.Models.Singleton;
using Microsoft.WingetCreateCore.Models.Version;

using Microsoft.WingetCreateCore.Models.Version;

/// <summary>
/// Command for updating the elements of an existing or local manifest.
/// </summary>
Expand All @@ -45,7 +45,7 @@ public static IEnumerable<Example> Examples

/// <summary>
/// Gets or sets the id used for looking up an existing manifest in the Windows Package Manager repository.
/// </summary>
/// </summary>
[Value(0, MetaName = "PackageIdentifier", Required = true, HelpText = "PackageIdentifier_HelpText", ResourceType = typeof(Resources))]
public string Id { get; set; }

Expand Down Expand Up @@ -90,17 +90,25 @@ public override async Task<bool> Execute()
InstallerUrl = this.InstallerUrl,
Id = this.Id,
Version = this.Version,
HasGitHubToken = !string.IsNullOrEmpty(this.GitHubToken),
HasGitHubToken = !string.IsNullOrEmpty(this.GitHubToken),
};

try
{
Logger.DebugLocalized(nameof(Resources.RetrievingManifest_Message), this.Id);

GitHub client = new GitHub(null, this.WingetRepoOwner, this.WingetRepo);
string exactId = await client.FindPackageId(this.Id);

if (!string.IsNullOrEmpty(exactId))
{
this.Id = exactId;
}

List<string> latestManifestContent;

try
{
GitHub client = new GitHub(null, this.WingetRepoOwner, this.WingetRepo);
{
latestManifestContent = await client.GetLatestManifestContentAsync(this.Id);
}
catch (Octokit.NotFoundException e)
Expand Down Expand Up @@ -201,7 +209,7 @@ private static void UpdatePropertyForLocaleManifests(string propertyName, string
{
manifest.GetType().GetProperty(propertyName).SetValue(manifest, value);
}
}
}

private async Task<bool> UpdateManifest(Manifests manifests)
{
Expand Down
2 changes: 1 addition & 1 deletion src/WingetCreateCLI/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private static async Task<int> Main(string[] args)

try
{
WingetCreateCore.Serialization.ProducedBy = ProgramName;
WingetCreateCore.Serialization.ProducedBy = string.Join(" ", ProgramName, Utils.GetEntryAssemblyVersion());
return await command.Execute() ? 0 : 1;
}
catch (Exception ex)
Expand Down
9 changes: 9 additions & 0 deletions src/WingetCreateCLI/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/WingetCreateCLI/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -553,4 +553,7 @@
<data name="PackageParsing_Error" xml:space="preserve">
<value>Unable to successfully parse the package and determine the installer type.</value>
</data>
<data name="PackageIdAlreadyExists_Error" xml:space="preserve">
<value>We have detected that this package identifier already exists. If you intend to update an existing package, please use the update command.</value>
</data>
</root>
39 changes: 37 additions & 2 deletions src/WingetCreateCore/Common/GitHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public async Task<IList<PublisherAppVersion>> GetAppVersions()
return new PublisherAppVersion(publisher, app, version, $"{publisher}.{app}", i.Path);
})
.ToList();
}
}

/// <summary>
/// Obtains the latest manifest using the specified packageId.
Expand Down Expand Up @@ -199,6 +199,17 @@ public async Task<string> GetFileContentsAsync(string path)
public async Task CheckAccess()
{
await this.github.Repository.Get(this.wingetRepoOwner, this.wingetRepo);
}

/// <summary>
/// Recursively searches the repository for the provided package identifer to determine if it already exists.
/// </summary>
/// <param name="packageId">The package identifier.</param>
/// <returns>The exact matching package identifier or null if no match was found.</returns>
public async Task<string> FindPackageId(string packageId)
{
string path = Constants.WingetManifestRoot + '/' + $"{char.ToLowerInvariant(packageId[0])}";
return await this.FindPackageIdRecursive(packageId.Split('.'), path, string.Empty, 0);
}

/// <summary>
Expand All @@ -224,7 +235,31 @@ private static string GetJwtToken(string gitHubAppPrivateKeyPem, int gitHubAppId
iss = gitHubAppId,
};

return Jose.JWT.Encode(payload, rsa, JwsAlgorithm.RS256);
return JWT.Encode(payload, rsa, JwsAlgorithm.RS256);
}

private async Task<string> FindPackageIdRecursive(string[] packageId, string path, string exactPackageId, int index)
{
if (index == packageId.Length)
{
return exactPackageId.Trim('.');
}

var contents = await this.github.Repository.Content.GetAllContents(this.wingetRepoOwner, this.wingetRepo, path);
string packageIdToken = packageId[index].ToLowerInvariant();

foreach (RepositoryContent content in contents)
{
if (string.Equals(packageIdToken, content.Name.ToLowerInvariant()))
{
path = path + '/' + content.Name;
exactPackageId = string.Join(".", exactPackageId, content.Name);
index++;
return await this.FindPackageIdRecursive(packageId, path, exactPackageId, index);
}
}

return null;
}

private async Task<PullRequest> SubmitPRAsync(string packageId, string version, Dictionary<string, string> contents, bool submitToFork)
Expand Down
12 changes: 12 additions & 0 deletions src/WingetCreateTests/WingetCreateTests/UnitTests/GitHubTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ public void InvalidPackageIdentifier()
Assert.ThrowsAsync<NotFoundException>(async () => await this.gitHub.GetLatestManifestContentAsync(TestConstants.TestInvalidPackageIdentifier), "Octokit.NotFoundException should be thrown");
}

/// <summary>
/// Verifies the ability to identify matching package identifiers.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Test]
public async Task FindMatchingPackageIdentifierAsync()
{
string exactMatch = await this.gitHub.FindPackageId(TestConstants.TestPackageIdentifier);
StringAssert.AreEqualIgnoringCase(TestConstants.TestPackageIdentifier, exactMatch, "Failed to find existing package identifier");
}


/// <summary>
/// Verifies that the GitHub client is able to submit a PR by verifying that the generated PR url matches the correct pattern.
/// </summary>
Expand Down

0 comments on commit 8e6ec72

Please sign in to comment.