From 8e6ec72f74252ba652bfba032776c86cc1a7e2c5 Mon Sep 17 00:00:00 2001 From: Ryan Fu <69221034+ryfu-msft@users.noreply.github.com> Date: Fri, 4 Jun 2021 14:07:44 -0700 Subject: [PATCH] Add duplicate id check for existing package identifiers (#54) * 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 --- src/WingetCreateCLI/Commands/NewCommand.cs | 60 ++++++++++++++----- src/WingetCreateCLI/Commands/UpdateCommand.cs | 24 +++++--- src/WingetCreateCLI/Program.cs | 2 +- .../Properties/Resources.Designer.cs | 9 +++ src/WingetCreateCLI/Properties/Resources.resx | 3 + src/WingetCreateCore/Common/GitHub.cs | 39 +++++++++++- .../UnitTests/GitHubTests.cs | 12 ++++ 7 files changed, 123 insertions(+), 26 deletions(-) diff --git a/src/WingetCreateCLI/Commands/NewCommand.cs b/src/WingetCreateCLI/Commands/NewCommand.cs index 1b1ea3d7..187f8990 100644 --- a/src/WingetCreateCLI/Commands/NewCommand.cs +++ b/src/WingetCreateCLI/Commands/NewCommand.cs @@ -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; @@ -114,7 +115,7 @@ public override async Task Execute() { Logger.ErrorLocalized(nameof(Resources.PackageParsing_Error)); return false; - } + } Console.WriteLine(Resources.NewCommand_Header); Console.WriteLine(); @@ -126,7 +127,14 @@ public override async Task 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); @@ -140,8 +148,8 @@ public override async Task 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()) @@ -159,7 +167,7 @@ public override async Task Execute() { TelemetryManager.Log.WriteEvent(commandEvent); } - } + } private static void PromptRequiredProperties(T manifest, VersionManifest versionManifest = null) { @@ -180,17 +188,13 @@ private static void PromptRequiredProperties(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); @@ -209,8 +213,8 @@ private static void PromptRequiredProperties(T manifest, VersionManifest vers Logger.Trace($"Property [{property.Name}] set to the value [{result}]"); } } - } - + } + private static void PromptInstallerProperties(T manifest, PropertyInfo property) { List installers = new List((ICollection)property.GetValue(manifest)); @@ -290,6 +294,32 @@ private static T PromptProperty(object model, T property, string memberName, var promptResult = Prompt.Input(message, property, new[] { FieldValidation.ValidateProperty(model, memberName, property) }); return promptResult is string str ? (T)(object)str.Trim() : promptResult; } + } + + /// + /// 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. + /// + /// Manifests object model. + /// Boolean value indicating whether the package identifier is valid. + private async Task 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; + } } } } diff --git a/src/WingetCreateCLI/Commands/UpdateCommand.cs b/src/WingetCreateCLI/Commands/UpdateCommand.cs index c1ea864a..f199b911 100644 --- a/src/WingetCreateCLI/Commands/UpdateCommand.cs +++ b/src/WingetCreateCLI/Commands/UpdateCommand.cs @@ -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; + /// /// Command for updating the elements of an existing or local manifest. /// @@ -45,7 +45,7 @@ public static IEnumerable Examples /// /// Gets or sets the id used for looking up an existing manifest in the Windows Package Manager repository. - /// + /// [Value(0, MetaName = "PackageIdentifier", Required = true, HelpText = "PackageIdentifier_HelpText", ResourceType = typeof(Resources))] public string Id { get; set; } @@ -90,17 +90,25 @@ public override async Task 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 latestManifestContent; try - { - GitHub client = new GitHub(null, this.WingetRepoOwner, this.WingetRepo); + { latestManifestContent = await client.GetLatestManifestContentAsync(this.Id); } catch (Octokit.NotFoundException e) @@ -201,7 +209,7 @@ private static void UpdatePropertyForLocaleManifests(string propertyName, string { manifest.GetType().GetProperty(propertyName).SetValue(manifest, value); } - } + } private async Task UpdateManifest(Manifests manifests) { diff --git a/src/WingetCreateCLI/Program.cs b/src/WingetCreateCLI/Program.cs index 9c7386a5..6dd5419a 100644 --- a/src/WingetCreateCLI/Program.cs +++ b/src/WingetCreateCLI/Program.cs @@ -43,7 +43,7 @@ private static async Task 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) diff --git a/src/WingetCreateCLI/Properties/Resources.Designer.cs b/src/WingetCreateCLI/Properties/Resources.Designer.cs index 995f066f..bf53dca9 100644 --- a/src/WingetCreateCLI/Properties/Resources.Designer.cs +++ b/src/WingetCreateCLI/Properties/Resources.Designer.cs @@ -861,6 +861,15 @@ public static string PackageFamilyName_KeywordDescription { } } + /// + /// Looks up a localized string similar to We have detected that this package identifier already exists. If you intend to update an existing package, please use the update command.. + /// + public static string PackageIdAlreadyExists_Error { + get { + return ResourceManager.GetString("PackageIdAlreadyExists_Error", resourceCulture); + } + } + /// /// Looks up a localized string similar to Package identifier used to lookup the existing manifest on the Windows Package Manager repo. Id is case-sensitive.. /// diff --git a/src/WingetCreateCLI/Properties/Resources.resx b/src/WingetCreateCLI/Properties/Resources.resx index 7663e0d6..1d8a52f7 100644 --- a/src/WingetCreateCLI/Properties/Resources.resx +++ b/src/WingetCreateCLI/Properties/Resources.resx @@ -553,4 +553,7 @@ Unable to successfully parse the package and determine the installer type. + + We have detected that this package identifier already exists. If you intend to update an existing package, please use the update command. + \ No newline at end of file diff --git a/src/WingetCreateCore/Common/GitHub.cs b/src/WingetCreateCore/Common/GitHub.cs index 787cf605..82372257 100644 --- a/src/WingetCreateCore/Common/GitHub.cs +++ b/src/WingetCreateCore/Common/GitHub.cs @@ -84,7 +84,7 @@ public async Task> GetAppVersions() return new PublisherAppVersion(publisher, app, version, $"{publisher}.{app}", i.Path); }) .ToList(); - } + } /// /// Obtains the latest manifest using the specified packageId. @@ -199,6 +199,17 @@ public async Task GetFileContentsAsync(string path) public async Task CheckAccess() { await this.github.Repository.Get(this.wingetRepoOwner, this.wingetRepo); + } + + /// + /// Recursively searches the repository for the provided package identifer to determine if it already exists. + /// + /// The package identifier. + /// The exact matching package identifier or null if no match was found. + public async Task FindPackageId(string packageId) + { + string path = Constants.WingetManifestRoot + '/' + $"{char.ToLowerInvariant(packageId[0])}"; + return await this.FindPackageIdRecursive(packageId.Split('.'), path, string.Empty, 0); } /// @@ -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 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 SubmitPRAsync(string packageId, string version, Dictionary contents, bool submitToFork) diff --git a/src/WingetCreateTests/WingetCreateTests/UnitTests/GitHubTests.cs b/src/WingetCreateTests/WingetCreateTests/UnitTests/GitHubTests.cs index 1d5b5a5a..b7be886a 100644 --- a/src/WingetCreateTests/WingetCreateTests/UnitTests/GitHubTests.cs +++ b/src/WingetCreateTests/WingetCreateTests/UnitTests/GitHubTests.cs @@ -47,6 +47,18 @@ public void InvalidPackageIdentifier() Assert.ThrowsAsync(async () => await this.gitHub.GetLatestManifestContentAsync(TestConstants.TestInvalidPackageIdentifier), "Octokit.NotFoundException should be thrown"); } + /// + /// Verifies the ability to identify matching package identifiers. + /// + /// A representing the asynchronous unit test. + [Test] + public async Task FindMatchingPackageIdentifierAsync() + { + string exactMatch = await this.gitHub.FindPackageId(TestConstants.TestPackageIdentifier); + StringAssert.AreEqualIgnoringCase(TestConstants.TestPackageIdentifier, exactMatch, "Failed to find existing package identifier"); + } + + /// /// Verifies that the GitHub client is able to submit a PR by verifying that the generated PR url matches the correct pattern. ///