Skip to content

Commit fee4754

Browse files
joelverhagenjoelverhagen
committed
Re-evaluate IsLatest properties when a package is delisted (#42)
Fix NuGet/NuGetGallery#3434
1 parent a66eaf4 commit fee4754

8 files changed

Lines changed: 117 additions & 72 deletions

File tree

‎src/NuGet.Server.Core/Infrastructure/IServerPackageCache.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ public interface IServerPackageCache
1818

1919
IEnumerable<ServerPackage> GetAll();
2020

21-
void Add(ServerPackage entity);
21+
void Add(ServerPackage entity, bool enableDelisting);
2222

23-
void AddRange(IEnumerable<ServerPackage> entities);
23+
void AddRange(IEnumerable<ServerPackage> entities, bool enableDelisting);
2424

2525
void Remove(string id, SemanticVersion version, bool enableDelisting);
2626

‎src/NuGet.Server.Core/Infrastructure/ServerPackageCache.cs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public void Remove(string id, SemanticVersion version, bool enableDelisting)
130130
_packages.RemoveWhere(p => IsMatch(p, id, version));
131131
}
132132

133-
UpdateLatestVersions(_packages.Where(p => IsMatch(p, id)));
133+
UpdateLatestVersions(_packages.Where(p => IsMatch(p, id)), enableDelisting);
134134

135135
_isDirty = true;
136136
}
@@ -140,15 +140,15 @@ public void Remove(string id, SemanticVersion version, bool enableDelisting)
140140
}
141141
}
142142

143-
public void Add(ServerPackage entity)
143+
public void Add(ServerPackage entity, bool enableDelisting)
144144
{
145145
_syncLock.EnterWriteLock();
146146
try
147147
{
148148
_packages.Remove(entity);
149149
_packages.Add(entity);
150150

151-
UpdateLatestVersions(_packages.Where(p => IsMatch(p, entity.Id)));
151+
UpdateLatestVersions(_packages.Where(p => IsMatch(p, entity.Id)), enableDelisting);
152152

153153
_isDirty = true;
154154
}
@@ -158,7 +158,7 @@ public void Add(ServerPackage entity)
158158
}
159159
}
160160

161-
public void AddRange(IEnumerable<ServerPackage> entities)
161+
public void AddRange(IEnumerable<ServerPackage> entities, bool enableDelisting)
162162
{
163163
_syncLock.EnterWriteLock();
164164
try
@@ -169,7 +169,7 @@ public void AddRange(IEnumerable<ServerPackage> entities)
169169
_packages.Add(entity);
170170
}
171171

172-
UpdateLatestVersions(_packages);
172+
UpdateLatestVersions(_packages, enableDelisting);
173173

174174
_isDirty = true;
175175
}
@@ -179,7 +179,7 @@ public void AddRange(IEnumerable<ServerPackage> entities)
179179
}
180180
}
181181

182-
private static void UpdateLatestVersions(IEnumerable<ServerPackage> packages)
182+
private static void UpdateLatestVersions(IEnumerable<ServerPackage> packages, bool enableDelisting)
183183
{
184184
var semVer1AbsoluteLatest = InitializePackageDictionary();
185185
var semVer1Latest = InitializePackageDictionary();
@@ -195,6 +195,12 @@ private static void UpdateLatestVersions(IEnumerable<ServerPackage> packages)
195195
package.SemVer2IsAbsoluteLatest = false;
196196
package.SemVer2IsLatest = false;
197197

198+
// Unlisted packages are never considered "latest".
199+
if (enableDelisting && !package.Listed)
200+
{
201+
return;
202+
}
203+
198204
// Update the SemVer1 views.
199205
if (!package.IsSemVer2)
200206
{

‎src/NuGet.Server.Core/Infrastructure/ServerPackageRepository.cs‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,12 @@ internal ServerPackageRepository(
7272
ISettingsProvider settingsProvider = null,
7373
Logging.ILogger logger = null)
7474
{
75-
if (fileSystem == null)
76-
{
77-
throw new ArgumentNullException(nameof(fileSystem));
78-
}
79-
8075
if (innerRepository == null)
8176
{
8277
throw new ArgumentNullException(nameof(innerRepository));
8378
}
8479

85-
_fileSystem = fileSystem;
80+
_fileSystem = fileSystem ?? throw new ArgumentNullException(nameof(fileSystem));
8681
_runBackgroundTasks = runBackgroundTasks;
8782
_settingsProvider = settingsProvider ?? new DefaultSettingsProvider();
8883
_logger = logger ?? new TraceLogger();
@@ -272,7 +267,7 @@ private void AddPackagesFromDropFolderWithoutLocking()
272267
}
273268

274269
// Add packages to metadata store in bulk
275-
_serverPackageCache.AddRange(serverPackages);
270+
_serverPackageCache.AddRange(serverPackages, EnableDelisting);
276271
_serverPackageCache.PersistIfDirty();
277272

278273
_logger.Log(LogLevel.Info, "Finished adding packages from drop folder.");
@@ -302,7 +297,7 @@ public async Task AddPackageAsync(IPackage package, CancellationToken token)
302297
EnableDelisting);
303298

304299
// Add the package to the metadata store.
305-
_serverPackageCache.Add(serverPackage);
300+
_serverPackageCache.Add(serverPackage, EnableDelisting);
306301

307302
_logger.Log(LogLevel.Info, "Finished adding package {0} {1}.", package.Id, package.Version);
308303
}
@@ -428,7 +423,7 @@ private async Task RebuildPackageStoreWithoutLockingAsync(CancellationToken toke
428423
// Build cache
429424
var packages = await ReadPackagesFromDiskWithoutLockingAsync(token);
430425
_serverPackageCache.Clear();
431-
_serverPackageCache.AddRange(packages);
426+
_serverPackageCache.AddRange(packages, EnableDelisting);
432427

433428
// Add packages from drop folder
434429
AddPackagesFromDropFolderWithoutLocking();
@@ -517,9 +512,11 @@ private void RegisterFileSystemWatcher()
517512
if (EnableFileSystemMonitoring && _runBackgroundTasks && _fileSystemWatcher == null && !string.IsNullOrEmpty(Source) && Directory.Exists(Source))
518513
{
519514
// ReSharper disable once UseObjectOrCollectionInitializer
520-
_fileSystemWatcher = new FileSystemWatcher(Source);
521-
_fileSystemWatcher.Filter = "*";
522-
_fileSystemWatcher.IncludeSubdirectories = true;
515+
_fileSystemWatcher = new FileSystemWatcher(Source)
516+
{
517+
Filter = "*",
518+
IncludeSubdirectories = true,
519+
};
523520

524521
_fileSystemWatcher.Changed += FileSystemChangedAsync;
525522
_fileSystemWatcher.Created += FileSystemChangedAsync;
@@ -643,12 +640,7 @@ private sealed class SuppressedFileSystemWatcher : IDisposable
643640

644641
public SuppressedFileSystemWatcher(ServerPackageRepository repository)
645642
{
646-
if (repository == null)
647-
{
648-
throw new ArgumentNullException(nameof(repository));
649-
}
650-
651-
_repository = repository;
643+
_repository = repository ?? throw new ArgumentNullException(nameof(repository));
652644
}
653645

654646
public bool LockTaken => _lockHandle.LockTaken;

‎src/NuGet.Server.Core/Infrastructure/ServerPackageStore.cs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ public void Remove(string id, SemanticVersion version, bool enableDelisting)
4545
{
4646
if (enableDelisting)
4747
{
48-
var physicalFileSystem = _fileSystem as PhysicalFileSystem;
49-
50-
if (physicalFileSystem != null)
48+
if (_fileSystem is PhysicalFileSystem physicalFileSystem)
5149
{
5250
var fileName = physicalFileSystem.GetFullPath(
5351
GetPackageFileName(id, version.ToNormalizedString()));
@@ -150,10 +148,7 @@ private PackageDerivedData GetPackageDerivedData(IPackage package, bool enableDe
150148
var normalizedVersion = package.Version.ToNormalizedString();
151149
var packageFileName = GetPackageFileName(package.Id, normalizedVersion);
152150
var hashFileName = GetHashFileName(package.Id, normalizedVersion);
153-
154-
// File system
155-
var physicalFileSystem = _fileSystem as PhysicalFileSystem;
156-
151+
157152
// Build package info
158153
var packageDerivedData = new PackageDerivedData();
159154

@@ -165,7 +160,7 @@ private PackageDerivedData GetPackageDerivedData(IPackage package, bool enableDe
165160

166161
// Read package info
167162
var localPackage = package as LocalPackage;
168-
if (physicalFileSystem != null)
163+
if (_fileSystem is PhysicalFileSystem physicalFileSystem)
169164
{
170165
// Read package info from file system
171166
var fullPath = _fileSystem.GetFullPath(packageFileName);

‎src/NuGet.Server.V2/Controllers/NuGetODataController.cs‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,7 @@ protected NuGetODataController(
4040
IServerPackageRepository repository,
4141
IPackageAuthenticationService authenticationService = null)
4242
{
43-
if (repository == null)
44-
{
45-
throw new ArgumentNullException(nameof(repository));
46-
}
47-
48-
_serverRepository = repository;
43+
_serverRepository = repository ?? throw new ArgumentNullException(nameof(repository));
4944
_authenticationService = authenticationService;
5045
}
5146

@@ -226,8 +221,7 @@ public virtual async Task<IHttpActionResult> GetUpdates(
226221
var packagesToUpdate = new List<IPackageMetadata>();
227222
for (var i = 0; i < idValues.Length; i++)
228223
{
229-
SemanticVersion semVersion;
230-
if(SemanticVersion.TryParse(versionValues[i],out semVersion))
224+
if(SemanticVersion.TryParse(versionValues[i], out var semVersion))
231225
{
232226
packagesToUpdate.Add(new PackageBuilder { Id = idValues[i], Version = semVersion });
233227
}
@@ -441,8 +435,7 @@ protected HttpResponseMessage CreateStringResponse(HttpStatusCode statusCode, st
441435
private string GetApiKeyFromHeader()
442436
{
443437
string apiKey = null;
444-
IEnumerable<string> values;
445-
if (Request.Headers.TryGetValues(ApiKeyHeader, out values))
438+
if (Request.Headers.TryGetValues(ApiKeyHeader, out var values))
446439
{
447440
apiKey = values.FirstOrDefault();
448441
}

‎test/NuGet.Server.Core.Tests/Infrastructure/TemporaryDirectory.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,14 @@ public void Dispose()
2323
{
2424
if (Directory.Exists(Path))
2525
{
26-
Directory.Delete(Path, true);
26+
try
27+
{
28+
Directory.Delete(Path, true);
29+
}
30+
catch
31+
{
32+
// Nothing to do here.
33+
}
2734
}
2835
}
2936

‎test/NuGet.Server.Core.Tests/ServerPackageCacheTest.cs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ public void Persist_RetainsSemVer2Version()
146146
{
147147
Id = PackageId,
148148
Version = SemVer2Version
149-
});
149+
}, enableDelisting: false);
150150

151151
// Act
152152
actual.Persist();
@@ -232,7 +232,11 @@ public void Exists_IsCaseInsensitive()
232232
.Setup(x => x.FileExists(CacheFileName))
233233
.Returns(false);
234234
var target = new ServerPackageCache(fileSystem.Object, CacheFileName);
235-
target.Add(new ServerPackage { Id = "NuGet.Versioning", Version = new SemanticVersion("3.5.0-beta2") });
235+
target.Add(new ServerPackage
236+
{
237+
Id = "NuGet.Versioning",
238+
Version = new SemanticVersion("3.5.0-beta2"),
239+
}, enableDelisting: false);
236240

237241
// Act
238242
var actual = target.Exists("nuget.versioning", new SemanticVersion("3.5.0-BETA2"));
@@ -250,7 +254,11 @@ public void Exists_ReturnsFalseWhenPackageDoesNotExist()
250254
.Setup(x => x.FileExists(CacheFileName))
251255
.Returns(false);
252256
var target = new ServerPackageCache(fileSystem.Object, CacheFileName);
253-
target.Add(new ServerPackage { Id = "NuGet.Versioning", Version = new SemanticVersion("3.5.0-beta2") });
257+
target.Add(new ServerPackage
258+
{
259+
Id = "NuGet.Versioning",
260+
Version = new SemanticVersion("3.5.0-beta2"),
261+
}, enableDelisting: false);
254262

255263
// Act
256264
var actual = target.Exists("NuGet.Frameworks", new SemanticVersion("3.5.0-beta2"));
@@ -268,7 +276,11 @@ public void Exists_ReturnsTrueWhenPackageExists()
268276
.Setup(x => x.FileExists(CacheFileName))
269277
.Returns(false);
270278
var target = new ServerPackageCache(fileSystem.Object, CacheFileName);
271-
target.Add(new ServerPackage { Id = "NuGet.Versioning", Version = new SemanticVersion("3.5.0-beta2") });
279+
target.Add(new ServerPackage
280+
{
281+
Id = "NuGet.Versioning",
282+
Version = new SemanticVersion("3.5.0-beta2"),
283+
}, enableDelisting: false);
272284

273285
// Act
274286
var actual = target.Exists("NuGet.Versioning", new SemanticVersion("3.5.0-beta2"));

0 commit comments

Comments
 (0)