Skip to content
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.

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

Configurable cache file name #49

Merged
merged 11 commits into from
Feb 3, 2018
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using NuGet.Server.Core.Infrastructure;

namespace NuGet.Server.V2.Samples.OwinHost
{
public class DictionarySettingsProvider : ISettingsProvider
{
readonly Dictionary<string, bool> _settings;
readonly Dictionary<string, object> _settings;

public DictionarySettingsProvider(Dictionary<string, bool> settings)
public DictionarySettingsProvider(Dictionary<string, object> settings)
{
_settings = settings;
}
Expand All @@ -18,8 +19,13 @@ public DictionarySettingsProvider(Dictionary<string, bool> settings)
public bool GetBoolSetting(string key, bool defaultValue)
{
System.Diagnostics.Debug.WriteLine("getSetting: " + key);
return _settings.ContainsKey(key) ? _settings[key] : defaultValue;
return _settings.ContainsKey(key) ? Convert.ToBoolean(_settings[key]) : defaultValue;

}

public string GetStringSetting(string key, string defaultValue)
{
return _settings.ContainsKey(key) ? Convert.ToString(_settings[key]) : defaultValue;
}
}
}
2 changes: 1 addition & 1 deletion samples/NuGet.Server.V2.Samples.OwinHost/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ static void Main(string[] args)

// Set up a common settingsProvider to be used by all repositories.
// If a setting is not present in dictionary default value will be used.
var settings = new Dictionary<string, bool>();
var settings = new Dictionary<string, object>();
settings.Add("enableDelisting", false); //default=false
settings.Add("enableFrameworkFiltering", false); //default=false
settings.Add("ignoreSymbolsPackages", true); //default=false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,10 @@ public bool GetBoolSetting(string key, bool defaultValue)
{
return defaultValue;
}

public string GetStringSetting(string key, string defaultValue)
{
return defaultValue;
}
}
}
1 change: 1 addition & 0 deletions src/NuGet.Server.Core/Infrastructure/ISettingsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ namespace NuGet.Server.Core.Infrastructure
public interface ISettingsProvider
{
bool GetBoolSetting(string key, bool defaultValue);
string GetStringSetting(string key, string defaultValue);
}
}
21 changes: 20 additions & 1 deletion src/NuGet.Server.Core/Infrastructure/ServerPackageRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,28 @@ internal ServerPackageRepository(
private bool EnableFileSystemMonitoring =>
_settingsProvider.GetBoolSetting("enableFileSystemMonitoring", true);

private string CacheFileName => _settingsProvider.GetStringSetting("cacheFileName", null);

private ServerPackageCache InitializeServerPackageStore()
{
return new ServerPackageCache(_fileSystem, Environment.MachineName.ToLowerInvariant() + ".cache.bin");
return new ServerPackageCache(_fileSystem, ResolveCacheFileName());
}

private string ResolveCacheFileName()
{
var fileName = CacheFileName;
const string suffix = ".cache.bin";

if (String.IsNullOrWhiteSpace(fileName))
{
// Default file name
return Environment.MachineName.ToLowerInvariant() + suffix;
}

if (fileName.EndsWith(suffix, StringComparison.OrdinalIgnoreCase))
return fileName;
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: curly { } around all if blocks, even if one line.


return fileName + suffix;
}

/// <summary>
Expand Down
6 changes: 6 additions & 0 deletions src/NuGet.Server/Infrastructure/WebConfigSettingsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,11 @@ public bool GetBoolSetting(string key, bool defaultValue)
bool value;
return !bool.TryParse(settings[key], out value) ? defaultValue : value;
}

public string GetStringSetting(string key, string defaultValue)
{
var settings = _getSettings();
return settings[key] ?? defaultValue;
}
}
}
5 changes: 5 additions & 0 deletions src/NuGet.Server/Web.config
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
-->
<add key="packagesPath" value="" />

<!--
Change the name of the internal cache file. Default is machine name (System.Environment.MachineName).
Copy link
Member

Choose a reason for hiding this comment

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

It's unclear whether putting a path here works (or even should work).

I would recommend updated the XML comment to state that this must be a file name and not a relative or absolute path. Additionally, I would validate at runtime that this is just a file name. In the future we could extend this to support relative paths but that seems unnecessary now and can be expanded without being a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I updated the XML comment and added validation of a valid file name.

-->
<add key="cacheFileName" value="" />

<!--
Set allowOverrideExistingPackageOnPush to false to mimic NuGet.org's behaviour (do not allow overwriting packages with same id + version).
-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ namespace NuGet.Server.Core.Tests.Infrastructure
{
class FuncSettingsProvider : ISettingsProvider
{
readonly Func<string, bool, bool> _getSetting;
internal FuncSettingsProvider(Func<string,bool,bool> getSetting)
readonly Func<string, object, object> _getSetting;
internal FuncSettingsProvider(Func<string,object,object> getSetting)
Copy link
Member

Choose a reason for hiding this comment

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

nitpic: spaces between type parameters after the comma ,

{
if (getSetting == null)
{
Expand All @@ -20,7 +20,12 @@ internal FuncSettingsProvider(Func<string,bool,bool> getSetting)

public bool GetBoolSetting(string key, bool defaultValue)
{
return _getSetting(key, defaultValue);
return Convert.ToBoolean(_getSetting(key, defaultValue));
}

public string GetStringSetting(string key, string defaultValue)
{
return Convert.ToString(_getSetting(key, defaultValue));
}
}
}
12 changes: 6 additions & 6 deletions test/NuGet.Server.Core.Tests/ServerPackageRepositoryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class ServerPackageRepositoryTest
public static async Task<ServerPackageRepository> CreateServerPackageRepositoryAsync(
string path,
Action<ExpandedPackageRepository> setupRepository = null,
Func<string, bool, bool> getSetting = null)
Func<string, object, object> getSetting = null)
{
var fileSystem = new PhysicalFileSystem(path);
var expandedPackageRepository = new ExpandedPackageRepository(fileSystem);
Expand Down Expand Up @@ -514,7 +514,7 @@ public async Task ServerPackageRepositorySemVer1IsAbsoluteLatest(bool enableDeli
using (var temporaryDirectory = new TemporaryDirectory())
{
// Arrange
var getSetting = enableDelisting ? EnableDelisting : (Func<string, bool, bool>)null;
var getSetting = enableDelisting ? EnableDelisting : (Func<string, object, object>)null;
var serverRepository = await CreateServerPackageRepositoryAsync(temporaryDirectory.Path, repository =>
{
repository.AddPackage(CreatePackage("test", "2.0-alpha"));
Expand Down Expand Up @@ -548,7 +548,7 @@ public async Task ServerPackageRepositorySemVer2IsAbsoluteLatest(bool enableDeli
using (var temporaryDirectory = new TemporaryDirectory())
{
// Arrange
var getSetting = enableDelisting ? EnableDelisting : (Func<string, bool, bool>)null;
var getSetting = enableDelisting ? EnableDelisting : (Func<string, object, object>)null;
var serverRepository = await CreateServerPackageRepositoryAsync(temporaryDirectory.Path, repository =>
{
repository.AddPackage(CreatePackage("test", "2.0-alpha"));
Expand Down Expand Up @@ -603,7 +603,7 @@ public async Task ServerPackageRepositorySemVer1IsLatest(bool enableDelisting)
using (var temporaryDirectory = new TemporaryDirectory())
{
// Arrange
var getSetting = enableDelisting ? EnableDelisting : (Func<string, bool, bool>)null;
var getSetting = enableDelisting ? EnableDelisting : (Func<string, object, object>)null;
var serverRepository = await CreateServerPackageRepositoryAsync(temporaryDirectory.Path, repository =>
{
repository.AddPackage(CreatePackage("test1", "1.0.0"));
Expand Down Expand Up @@ -634,7 +634,7 @@ public async Task ServerPackageRepositorySemVer2IsLatest(bool enableDelisting)
using (var temporaryDirectory = new TemporaryDirectory())
{
// Arrange
var getSetting = enableDelisting ? EnableDelisting : (Func<string, bool, bool>)null;
var getSetting = enableDelisting ? EnableDelisting : (Func<string, object, object>)null;
var serverRepository = await CreateServerPackageRepositoryAsync(temporaryDirectory.Path, repository =>
{
repository.AddPackage(CreatePackage("test", "1.11"));
Expand Down Expand Up @@ -879,7 +879,7 @@ private IPackage CreatePackage(
return outputPackage;
}

private static bool EnableDelisting(string key, bool defaultValue)
private static object EnableDelisting(string key, object defaultValue)
{
if (key == "enableDelisting")
{
Expand Down