Skip to content

Commit b0d3c49

Browse files
jeffkljeffkl
committed
Feedback
1 parent 79664c2 commit b0d3c49

4 files changed

Lines changed: 41 additions & 34 deletions

File tree

‎src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/PackageSourceTelemetry.cs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,19 +204,25 @@ internal static void AddNupkgCopiedData(ProtocolDiagnosticNupkgCopiedEvent ncEve
204204
{
205205
data.NupkgCount++;
206206
data.NupkgSize += ncEvent.FileSize;
207-
data.IdContainsNonAsciiCharacter = data.IdContainsNonAsciiCharacter || (ncEvent.PackageId != null && HasNonASCIICharacters(ncEvent.PackageId));
207+
data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter || (ncEvent.PackageId != null && HasNonAlphanumericDotDashOrUnderscoreCharacters(ncEvent.PackageId));
208208
}
209209

210-
bool HasNonASCIICharacters(string packageId)
210+
bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId)
211211
{
212212
foreach (char c in packageId.AsSpan())
213213
{
214-
if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-'))
214+
if (!IsCharacterAlphanumericDotDashOrUnderscore(c))
215215
{
216216
return true;
217217
}
218218
}
219+
219220
return false;
221+
222+
bool IsCharacterAlphanumericDotDashOrUnderscore(char c)
223+
{
224+
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_';
225+
}
220226
}
221227
}
222228

@@ -275,7 +281,7 @@ internal static async Task<TelemetryEvent> ToTelemetryAsync(Data data, SourceRep
275281
telemetry[PropertyNames.Duration.Total] = data.Resources.Values.Sum(r => r.duration.TotalMilliseconds);
276282
telemetry[PropertyNames.Nupkgs.Copied] = data.NupkgCount;
277283
telemetry[PropertyNames.Nupkgs.Bytes] = data.NupkgSize;
278-
telemetry[PropertyNames.Nupkgs.IdContainsNonAsciiCharacter] = data.IdContainsNonAsciiCharacter;
284+
telemetry[PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter] = data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter;
279285
AddResourceProperties(telemetry, data.Resources);
280286

281287
if (data.Http.Requests > 0)
@@ -441,7 +447,7 @@ internal class Data
441447
internal HttpData Http { get; }
442448
internal int NupkgCount { get; set; }
443449
internal long NupkgSize { get; set; }
444-
internal bool IdContainsNonAsciiCharacter { get; set; }
450+
internal bool IdContainsNonAlphanumericDotDashOrUnderscoreCharacter { get; set; }
445451

446452
internal Data()
447453
{
@@ -491,7 +497,7 @@ internal static class Nupkgs
491497
{
492498
internal const string Copied = "nupkgs.copied";
493499
internal const string Bytes = "nupkgs.bytes";
494-
internal const string IdContainsNonAsciiCharacter = "nupkgs.idcontainsnonasciicharacter";
500+
internal const string IdContainsNonAlphanumericDotDashOrUnderscoreCharacter = "nupkgs.idcontainsNonAlphanumericDotDashOrUnderscorecharacter";
495501
}
496502

497503
internal static class Resources

‎src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ private readonly Dictionary<RestoreTargetGraph, Dictionary<string, LibraryInclud
5454
private const string IsCentralVersionManagementEnabled = nameof(IsCentralVersionManagementEnabled);
5555
private const string TotalUniquePackagesCount = nameof(TotalUniquePackagesCount);
5656
private const string NewPackagesInstalledCount = nameof(NewPackagesInstalledCount);
57-
private const string AnyPackageIdContainsNonASCIICharacters = nameof(AnyPackageIdContainsNonASCIICharacters);
57+
private const string AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters = nameof(AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters);
5858
private const string SourcesCount = nameof(SourcesCount);
5959
private const string HttpSourcesCount = nameof(HttpSourcesCount);
6060
private const string LocalSourcesCount = nameof(LocalSourcesCount);
@@ -742,7 +742,7 @@ private async Task<EvaluateLockFileResult>
742742
}
743743

744744
telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count();
745-
telemetry.TelemetryEvent[AnyPackageIdContainsNonASCIICharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonASCIICharacters(i.Key.Name));
745+
telemetry.TelemetryEvent[AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Flattened).Any(i => HasNonAlphanumericDotDashOrUnderscoreCharacters(i.Key.Name));
746746
telemetry.TelemetryEvent[RestoreSuccess] = success;
747747
}
748748

@@ -756,21 +756,22 @@ private async Task<EvaluateLockFileResult>
756756
packagesLockFilePath,
757757
cacheFile);
758758

759-
bool HasNonASCIICharacters(string packageId)
759+
bool HasNonAlphanumericDotDashOrUnderscoreCharacters(string packageId)
760760
{
761-
if (string.IsNullOrWhiteSpace(packageId))
762-
{
763-
return false;
764-
}
765-
766761
foreach (char c in packageId.AsSpan())
767762
{
768-
if (!((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-'))
763+
if (!IsCharacterAlphanumericDotDashOrUnderscore(c))
769764
{
770765
return true;
771766
}
772767
}
768+
773769
return false;
770+
771+
bool IsCharacterAlphanumericDotDashOrUnderscore(char c)
772+
{
773+
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '-' || c == '_';
774+
}
774775
}
775776
}
776777

‎test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/Telemetry/PackageSourceTelemetryTests.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public void AddNupkgCopiedData_MultipleEvents_AccumulatesCorrectly()
145145
[InlineData("alllower")]
146146
[InlineData("123Numeric")]
147147
[InlineData("a")]
148-
public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFalse(string packageId)
148+
public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse(string packageId)
149149
{
150150
// Arrange
151151
var data = CreateDataDictionary(SampleSource);
@@ -156,7 +156,7 @@ public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFa
156156

157157
// Assert
158158
var result = Assert.Single(data).Value;
159-
Assert.False(result.IdContainsNonAsciiCharacter);
159+
Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
160160
}
161161

162162
[Theory]
@@ -165,7 +165,7 @@ public void AddNupkgCopiedData_StandardPackageId_IdContainsNonAsciiCharacterIsFa
165165
[InlineData("Ünïcödé")]
166166
[InlineData("Package Name")]
167167
[InlineData("package+extra")]
168-
public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAsciiCharacterIsTrue(string packageId)
168+
public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue(string packageId)
169169
{
170170
// Arrange
171171
var data = CreateDataDictionary(SampleSource);
@@ -176,11 +176,11 @@ public void AddNupkgCopiedData_NonstandardPackageId_IdContainsNonAsciiCharacterI
176176

177177
// Assert
178178
var result = Assert.Single(data).Value;
179-
Assert.True(result.IdContainsNonAsciiCharacter);
179+
Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
180180
}
181181

182182
[Fact]
183-
public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAsciiCharacterIsTrue()
183+
public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsTrue()
184184
{
185185
// Arrange
186186
var data = CreateDataDictionary(SampleSource);
@@ -192,11 +192,11 @@ public void AddNupkgCopiedData_MultiplePackagesOneNonstandard_IdContainsNonAscii
192192

193193
// Assert
194194
var result = Assert.Single(data).Value;
195-
Assert.True(result.IdContainsNonAsciiCharacter);
195+
Assert.True(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
196196
}
197197

198198
[Fact]
199-
public void AddNupkgCopiedData_NullPackageId_IdContainsNonAsciiCharacterIsFalse()
199+
public void AddNupkgCopiedData_NullPackageId_IdContainsNonAlphanumericDotDashOrUnderscoreCharacterIsFalse()
200200
{
201201
// Arrange
202202
var data = CreateDataDictionary(SampleSource);
@@ -207,7 +207,7 @@ public void AddNupkgCopiedData_NullPackageId_IdContainsNonAsciiCharacterIsFalse(
207207

208208
// Assert
209209
var result = Assert.Single(data).Value;
210-
Assert.False(result.IdContainsNonAsciiCharacter);
210+
Assert.False(result.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter);
211211
}
212212

213213
[Fact]
@@ -381,7 +381,7 @@ public async Task ToTelemetry_WithData_CreatesTelemetryProperties(string package
381381

382382
Assert.Equal(data.NupkgCount, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Copied]);
383383
Assert.Equal(data.NupkgSize, result[PackageSourceTelemetry.PropertyNames.Nupkgs.Bytes]);
384-
Assert.Equal(data.IdContainsNonAsciiCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAsciiCharacter]);
384+
Assert.Equal(data.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter, result[PackageSourceTelemetry.PropertyNames.Nupkgs.IdContainsNonAlphanumericDotDashOrUnderscoreCharacter]);
385385

386386
Assert.Equal(data.Resources.Sum(r => r.Value.count), result[PackageSourceTelemetry.PropertyNames.Resources.Calls]);
387387
foreach (var resource in data.Resources)

‎test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests/RestoreCommandTests.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2957,7 +2957,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
29572957
["NoOpDuration"] = value => value.Should().NotBeNull(),
29582958
["TotalUniquePackagesCount"] = value => value.Should().Be(1),
29592959
["NewPackagesInstalledCount"] = value => value.Should().Be(1),
2960-
["AnyPackageIdContainsNonASCIICharacters"] = value => value.Should().Be(false),
2960+
["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"] = value => value.Should().Be(false),
29612961
["EvaluateLockFileDuration"] = value => value.Should().NotBeNull(),
29622962
["CreateRestoreTargetGraphDuration"] = value => value.Should().NotBeNull(),
29632963
["GenerateRestoreGraphDuration"] = value => value.Should().NotBeNull(),
@@ -3646,7 +3646,7 @@ private static PackageSpec CreatePackageSpec(List<TargetFrameworkInformation> tf
36463646
}
36473647

36483648
[Fact]
3649-
public async Task ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsFalse()
3649+
public async Task ExecuteAsync_WithASCIIPackageId_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsFalse()
36503650
{
36513651
// Arrange
36523652
using var pathContext = new SimpleTestPathContext();
@@ -3682,7 +3682,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
36823682
// Assert
36833683
result.Success.Should().BeTrue(because: logger.ShowMessages());
36843684
var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation"));
3685-
projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(false);
3685+
projectInformationEvent["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"].Should().Be(false);
36863686
}
36873687

36883688
[Theory]
@@ -3691,7 +3691,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
36913691
[InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1)
36923692
[InlineData("Package\u00E9")] // Latin small letter e with acute (U+00E9)
36933693
[InlineData("\u0410.Package")] // Cyrillic capital A (U+0410)
3694-
public async Task ExecuteAsync_WithNonASCIIPackageId_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId)
3694+
public async Task ExecuteAsync_WithNonAlphanumericDotDashOrUnderscorePackageId_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsTrue(string packageId)
36953695
{
36963696
// Arrange
36973697
using var pathContext = new SimpleTestPathContext();
@@ -3727,28 +3727,28 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
37273727
// Assert
37283728
result.Success.Should().BeTrue(because: logger.ShowMessages());
37293729
var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation"));
3730-
projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true);
3730+
projectInformationEvent["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"].Should().Be(true);
37313731
}
37323732

37333733
[Theory]
37343734
[InlineData("My_Package")] // underscore
37353735
[InlineData("Pac\u212Bage")] // Kelvin sign K (U+212A)
37363736
[InlineData("Package\u03B1")] // Greek lowercase alpha (U+03B1)
3737-
public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonASCIICharactersIsTrue(string packageId)
3737+
public async Task ExecuteAsync_WithMixedPackageIds_AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharactersIsTrue(string packageId)
37383738
{
37393739
// Arrange
37403740
using var pathContext = new SimpleTestPathContext();
37413741
var projectName = "TestProject";
37423742
var projectPath = Path.Combine(pathContext.SolutionRoot, projectName);
37433743
var asciiOnlyPkg = new SimpleTestPackageContext("Some.Package", "1.0.0");
3744-
var nonASCIIPkg = new SimpleTestPackageContext(packageId, "1.0.0");
3745-
asciiOnlyPkg.Dependencies.Add(nonASCIIPkg);
3744+
var NonAlphanumericDotDashOrUnderscorePkg = new SimpleTestPackageContext(packageId, "1.0.0");
3745+
asciiOnlyPkg.Dependencies.Add(NonAlphanumericDotDashOrUnderscorePkg);
37463746

37473747
await SimpleTestPackageUtility.CreateFolderFeedV3Async(
37483748
pathContext.PackageSource,
37493749
PackageSaveMode.Defaultv3,
37503750
asciiOnlyPkg,
3751-
nonASCIIPkg);
3751+
NonAlphanumericDotDashOrUnderscorePkg);
37523752

37533753
PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpec(projectName, pathContext.SolutionRoot, "net472", "Some.Package");
37543754
var logger = new TestLogger();
@@ -3775,7 +3775,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async(
37753775
// Assert
37763776
result.Success.Should().BeTrue(because: logger.ShowMessages());
37773777
var projectInformationEvent = telemetryEvents.Single(e => e.Name.Equals("ProjectRestoreInformation"));
3778-
projectInformationEvent["AnyPackageIdContainsNonASCIICharacters"].Should().Be(true);
3778+
projectInformationEvent["AnyPackageIdContainsNonAlphanumericDotDashOrUnderscoreCharacters"].Should().Be(true);
37793779
}
37803780

37813781
private Task<GraphNode<RemoteResolveResult>> DoWalkAsync(RemoteDependencyWalker walker, string name, NuGetFramework framework)

0 commit comments

Comments
 (0)