CR feedback

This commit is contained in:
damianedwards 2015-09-25 14:58:07 -07:00
parent 3c2408db68
commit a7b65efa75
3 changed files with 25 additions and 18 deletions

View File

@ -59,7 +59,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
// string used here. // string used here.
// The null-coalesce here is to protect against returning null after Dispose() is called, at which // The null-coalesce here is to protect against returning null after Dispose() is called, at which
// point _dateValue will be null forever after. // point _dateValue will be null forever after.
return _dateValue ?? _systemClock.UtcNow.ToString("r"); return _dateValue ?? _systemClock.UtcNow.ToString(Constants.RFC1123DateFormat);
} }
/// <summary> /// <summary>
@ -69,11 +69,8 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
{ {
lock (_timerLocker) lock (_timerLocker)
{ {
if (_dateValueTimer != null) DisposeTimer();
{
DisposeTimer();
}
_isDisposed = true; _isDisposed = true;
} }
} }
@ -94,7 +91,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
// Immediately assign the date value and start the timer again. We assign the value immediately // Immediately assign the date value and start the timer again. We assign the value immediately
// here as the timer won't fire until the timer interval has passed and we want a value assigned // here as the timer won't fire until the timer interval has passed and we want a value assigned
// inline now to serve requests that occur in the meantime. // inline now to serve requests that occur in the meantime.
_dateValue = _systemClock.UtcNow.ToString("r"); _dateValue = _systemClock.UtcNow.ToString(Constants.RFC1123DateFormat);
_dateValueTimer = new Timer(UpdateDateValue, state: null, dueTime: _timerInterval, period: _timerInterval); _dateValueTimer = new Timer(UpdateDateValue, state: null, dueTime: _timerInterval, period: _timerInterval);
} }
} }
@ -107,7 +104,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
var now = _systemClock.UtcNow; var now = _systemClock.UtcNow;
// See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.18 for required format of Date header // See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.18 for required format of Date header
_dateValue = now.ToString("r"); _dateValue = now.ToString(Constants.RFC1123DateFormat);
if (_hadRequestsSinceLastTimerTick) if (_hadRequestsSinceLastTimerTick)
{ {
@ -137,9 +134,12 @@ namespace Microsoft.AspNet.Server.Kestrel.Http
private void DisposeTimer() private void DisposeTimer()
{ {
_dateValueTimer.Dispose(); if (_dateValueTimer != null)
_dateValueTimer = null; {
_dateValue = null; _dateValueTimer.Dispose();
_dateValueTimer = null;
_dateValue = null;
}
} }
} }
} }

View File

@ -14,5 +14,11 @@ namespace Microsoft.AspNet.Server.Kestrel.Infrastructure
/// Prefix of host name used to specify Unix sockets in the configuration. /// Prefix of host name used to specify Unix sockets in the configuration.
/// </summary> /// </summary>
public const string UnixPipeHostPrefix = "unix:/"; public const string UnixPipeHostPrefix = "unix:/";
/// <summary>
/// DateTime format string for RFC1123. See https://msdn.microsoft.com/en-us/library/az4se3k1(v=vs.110).aspx#RFC1123
/// for info on the format.
/// </summary>
public const string RFC1123DateFormat = "r";
} }
} }

View File

@ -4,6 +4,7 @@
using System; using System;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.AspNet.Server.Kestrel.Http; using Microsoft.AspNet.Server.Kestrel.Http;
using Microsoft.AspNet.Server.Kestrel.Infrastructure;
using Microsoft.AspNet.Server.KestrelTests.TestHelpers; using Microsoft.AspNet.Server.KestrelTests.TestHelpers;
using Xunit; using Xunit;
@ -33,7 +34,7 @@ namespace Microsoft.AspNet.Server.KestrelTests
dateHeaderValueManager.Dispose(); dateHeaderValueManager.Dispose();
} }
Assert.Equal(now.ToString("r"), result); Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result);
} }
[Fact] [Fact]
@ -62,8 +63,8 @@ namespace Microsoft.AspNet.Server.KestrelTests
dateHeaderValueManager.Dispose(); dateHeaderValueManager.Dispose();
} }
Assert.Equal(now.ToString("r"), result1); Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result1);
Assert.Equal(now.ToString("r"), result2); Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result2);
Assert.Equal(1, systemClock.UtcNowCalled); Assert.Equal(1, systemClock.UtcNowCalled);
} }
@ -95,8 +96,8 @@ namespace Microsoft.AspNet.Server.KestrelTests
dateHeaderValueManager.Dispose(); dateHeaderValueManager.Dispose();
} }
Assert.Equal(now.ToString("r"), result1); Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result1);
Assert.Equal(future.ToString("r"), result2); Assert.Equal(future.ToString(Constants.RFC1123DateFormat), result2);
Assert.True(systemClock.UtcNowCalled >= 2); Assert.True(systemClock.UtcNowCalled >= 2);
} }
@ -118,8 +119,8 @@ namespace Microsoft.AspNet.Server.KestrelTests
systemClock.UtcNow = future; systemClock.UtcNow = future;
var result2 = dateHeaderValueManager.GetDateHeaderValue(); var result2 = dateHeaderValueManager.GetDateHeaderValue();
Assert.Equal(now.ToString("r"), result1); Assert.Equal(now.ToString(Constants.RFC1123DateFormat), result1);
Assert.Equal(future.ToString("r"), result2); Assert.Equal(future.ToString(Constants.RFC1123DateFormat), result2);
} }
} }
} }