Resolve feedback (#2)

* Rename package AzureSamples.Security.KeyVault.Proxy Basically, replaced "AzureSample" with "AzureSamples", which sounds better. * Resolve feedback * Use same test frameworks as azure-sdk-for-net * Fix TargetFrameworks for non-Windows * Use dotnet-version 2.1.20 * Use SDK version (not runtime) * Install 2.1 runtime to fix build Also separates build and test to more obviously show which one failed. * Work around actions/setup-dotnet#25 * Use SDK version instead of runtime version * More build fixes CIs really need some way of testing locally. 😔 * Fix xcopy command on Windows
heaths · Aug 10, 2020 · 797fa72 · 797fa72
1 parent fd7697d
commit 797fa72
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 55 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -29,17 +29,45 @@ jobs:
 
     steps:
     - uses: actions/checkout@v2
+
+    - uses: actions/setup-dotnet@v1
+      with:
+        dotnet-version: '2.1.808'
+
     - uses: actions/setup-dotnet@v1
       with:
-        dotnet-version: '3.1.201'
+        dotnet-version: '3.1.x'
+
+    - name: Copy .NET Core runtime SxS
+      shell: pwsh
+      run: |
+        # Work around https://github.com/actions/setup-dotnet/issues/25
+        $DOTNET_21_ROOT = $env:DOTNET_ROOT -replace '3\.1\.\d+', '2.1.808'
+        if ($env:OS -eq 'Windows_NT') {
+          xcopy "$DOTNET_21_ROOT\shared" "${env:DOTNET_ROOT}\shared" /b /e /h /q /y
+        } else {
+          rsync -a $DOTNET_21_ROOT/shared/* ${env:DOTNET_ROOT}/shared
+        }
+
+    - name: Build
+      run: >
+        dotnet build KeyVaultProxy.sln
+        -c Release -o bin/Release
+      env:
+        DOTNET_CLI_TELEMETRY_OPTOUT: 1
+        DOTNET_NOLOGO: 1
+        DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1
 
-    - id: test
+    - id: Test
+      name: test
       run: >
         dotnet test KeyVaultProxy.sln
         -c Release -o bin/Release
+        --no-build
         -l trx -r bin/Release/TestResults/${{ matrix.platform }}
       env:
         DOTNET_CLI_TELEMETRY_OPTOUT: 1
+        DOTNET_NOLOGO: 1
         DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1
 
     - uses: actions/upload-artifact@v2
@@ -55,23 +83,26 @@ jobs:
     - uses: actions/checkout@v2
     - uses: actions/setup-dotnet@v1
       with:
-        dotnet-version: '3.1.201'
+        dotnet-version: '3.1.x'
         source-url: https://nuget.pkg.github.com/heaths/index.json
       env:
         NUGET_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
-    - run: >
+    - name: Pack
+      run: >
         dotnet pack KeyVaultProxy.sln
         -c Release -o bin/Release
         --version-suffix 'preview.${{ github.run_number }}'
       env:
         DOTNET_CLI_TELEMETRY_OPTOUT: 1
+        DOTNET_NOLOGO: 1
         DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1
 
     - uses: actions/upload-artifact@v2
       with:
         name: artifacts
         path: bin/Release/*.nupkg
 
-    - run: dotnet nuget push bin/Release/*.nupkg
+    - name: Publish
+      run: dotnet nuget push bin/Release/*.nupkg
       if: github.ref == 'refs/heads/master' && github.event_name == 'push'
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -1,6 +1,6 @@
 <Project>
   <PropertyGroup>
-    <RootNamespace>AzureSample.Security.KeyVault.Proxy</RootNamespace>
+    <RootNamespace>AzureSamples.Security.KeyVault.Proxy</RootNamespace>
     <LangVersion>latest</LangVersion>
     <GenerateDocumentation>true</GenerateDocumentation>
   </PropertyGroup>

diff --git a/KeyVaultProxy.sln b/KeyVaultProxy.sln
@@ -3,9 +3,9 @@ Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio Version 16
 VisualStudioVersion = 16.0.30011.22
 MinimumVisualStudioVersion = 15.0.26124.0
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AzureSample.Security.KeyVault.Proxy", "src\AzureSample.Security.KeyVault.Proxy.csproj", "{FA43C911-4EAB-4D84-AA0D-128A8242E8CD}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AzureSamples.Security.KeyVault.Proxy", "src\AzureSamples.Security.KeyVault.Proxy.csproj", "{FA43C911-4EAB-4D84-AA0D-128A8242E8CD}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AzureSample.Security.KeyVault.Proxy.Tests", "tests\AzureSample.Security.KeyVault.Proxy.Tests.csproj", "{B1DBCC52-CD7E-44B4-9CE2-CC04379AB9B0}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AzureSamples.Security.KeyVault.Proxy.Tests", "tests\AzureSamples.Security.KeyVault.Proxy.Tests.csproj", "{B1DBCC52-CD7E-44B4-9CE2-CC04379AB9B0}"
 EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution

diff --git a/...zureSample.Security.KeyVault.Proxy.csproj → ...ureSamples.Security.KeyVault.Proxy.csproj b/...zureSample.Security.KeyVault.Proxy.csproj → ...ureSamples.Security.KeyVault.Proxy.csproj
@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
+    <Version>1.0.0</Version>
     <TargetFramework>netstandard2.0</TargetFramework>
     <Nullable>enable</Nullable>
     <Authors>heaths</Authors>

diff --git a/src/Cache.cs b/src/Cache.cs
@@ -7,7 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     /// <summary>
     /// Maintains a cache of <see cref="CachedResponse"/> items.
@@ -26,12 +26,6 @@ internal class Cache
         /// <returns>A new <see cref="Response"/>.</returns>
         internal async ValueTask<Response> GetOrAddAsync(bool isAsync, string uri, TimeSpan ttl, Func<ValueTask<Response>> action)
         {
-            // Try to get a valid response outside of the lock.
-            if (_cache.TryGetValue(uri, out CachedResponse cachedResponse) && cachedResponse.IsValid)
-            {
-                return await cachedResponse.CloneAsync(isAsync);
-            }
-
             if (isAsync)
             {
                 await _lock.WaitAsync().ConfigureAwait(false);
@@ -43,16 +37,16 @@ internal async ValueTask<Response> GetOrAddAsync(bool isAsync, string uri, TimeS
 
             try
             {
-                // Try again to get a valid cached response inside the lock before fetching.
-                if (_cache.TryGetValue(uri, out cachedResponse) && cachedResponse.IsValid)
+                // Try to get a valid cached response inside the lock before fetching.
+                if (_cache.TryGetValue(uri, out CachedResponse cachedResponse) && cachedResponse.IsValid)
                 {
-                    return await cachedResponse.CloneAsync(isAsync);
+                    return await cachedResponse.CloneAsync(isAsync).ConfigureAwait(false);
                 }
 
                 Response response = await action().ConfigureAwait(false);
                 if (response.Status == 200 && response.ContentStream is { })
                 {
-                    cachedResponse = await CachedResponse.CreateAsync(isAsync, response, ttl);
+                    cachedResponse = await CachedResponse.CreateAsync(isAsync, response, ttl).ConfigureAwait(false);
                     _cache[uri] = cachedResponse;
                 }
 

diff --git a/src/CachedResponse.cs b/src/CachedResponse.cs
@@ -11,25 +11,22 @@
 using Azure;
 using Azure.Core;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     /// <summary>
     /// A cached <see cref="Response"/> that is cloned and returned for subsequent requests.
     /// </summary>
     internal class CachedResponse : Response
     {
-        private readonly Dictionary<string, IList<string>> _headers = new Dictionary<string, IList<string>>(StringComparer.OrdinalIgnoreCase);
+        private readonly ResponseHeaders _headers;
         private DateTimeOffset _expires;
 
         private CachedResponse(int status, string reasonPhrase, ResponseHeaders headers)
         {
             Status = status;
             ReasonPhrase = reasonPhrase;
 
-            foreach (HttpHeader header in headers)
-            {
-                _headers[header.Name] = header.Value?.Split(',');
-            }
+            _headers = headers;
         }
 
         /// <inheritdoc/>
@@ -76,33 +73,16 @@ internal async ValueTask<Response> CloneAsync(bool isAsync) =>
             await CloneAsync(isAsync, this).ConfigureAwait(false);
 
         /// <inheritdoc/>
-        protected override bool ContainsHeader(string name) => _headers.ContainsKey(name);
+        protected override bool ContainsHeader(string name) => _headers.Contains(name);
 
         /// <inheritdoc/>
-        protected override IEnumerable<HttpHeader> EnumerateHeaders() =>
-            _headers.Select(kvp => new HttpHeader(kvp.Key, string.Join(",", kvp.Value)));
+        protected override IEnumerable<HttpHeader> EnumerateHeaders() => _headers;
 
         /// <inheritdoc/>
-        protected override bool TryGetHeader(string name, out string value)
-        {
-            if (_headers.TryGetValue(name, out IList<string> headerValues))
-            {
-                value = string.Join(",", headerValues);
-                return true;
-            }
-
-            value = null;
-            return false;
-        }
+        protected override bool TryGetHeader(string name, out string value) => _headers.TryGetValue(name, out value);
 
         /// <inheritdoc/>
-        protected override bool TryGetHeaderValues(string name, out IEnumerable<string> values)
-        {
-            bool result = _headers.TryGetValue(name, out IList<string> headerValues);
-            values = headerValues;
-
-            return result;
-        }
+        protected override bool TryGetHeaderValues(string name, out IEnumerable<string> values) => _headers.TryGetValues(name, out values);
 
         private static async ValueTask<CachedResponse> CloneAsync(bool isAsync, Response response)
         {

diff --git a/src/KeyVaultProxy.cs b/src/KeyVaultProxy.cs
@@ -6,7 +6,7 @@
 using Azure.Core;
 using Azure.Core.Pipeline;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     /// <summary>
     /// Cache <c>GET</c> requests for secrets, keys, or certificates for Azure Key Vault clients.

diff --git a/src/Properties/AssemblyInfo.cs b/src/Properties/AssemblyInfo.cs
@@ -3,4 +3,4 @@
 
 using System.Runtime.CompilerServices;
 
-[assembly: InternalsVisibleTo("AzureSample.Security.KeyVault.Proxy.Tests")]
+[assembly: InternalsVisibleTo("AzureSamples.Security.KeyVault.Proxy.Tests")]
diff --git a/...mple.Security.KeyVault.Proxy.Tests.csproj → ...ples.Security.KeyVault.Proxy.Tests.csproj b/...mple.Security.KeyVault.Proxy.Tests.csproj → ...ples.Security.KeyVault.Proxy.Tests.csproj
@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>netcoreapp3.1</TargetFramework>
+    <TargetFrameworks>netcoreapp2.1</TargetFrameworks>
+    <TargetFrameworks Condition="'$(OS)' == 'Windows_NT'">$(TargetFrameworks);net461</TargetFrameworks>
     <IsPackable>false</IsPackable>
   </PropertyGroup>
 
@@ -16,7 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <ProjectReference Include="..\src\AzureSample.Security.KeyVault.Proxy.csproj" />
+    <ProjectReference Include="..\src\AzureSamples.Security.KeyVault.Proxy.csproj" />
   </ItemGroup>
 
 </Project>
diff --git a/tests/KeyVaultProxyTests.Live.cs b/tests/KeyVaultProxyTests.Live.cs
@@ -11,7 +11,7 @@
 using Azure.Security.KeyVault.Secrets;
 using Xunit;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     public partial class KeyVaultProxyTests : IClassFixture<SecretsFixture>, IDisposable
     {

diff --git a/tests/KeyVaultProxyTests.cs b/tests/KeyVaultProxyTests.cs
@@ -4,7 +4,7 @@
 using System;
 using Xunit;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     public partial class KeyVaultProxyTests
     {

diff --git a/tests/LiveFactAttribute.cs b/tests/LiveFactAttribute.cs
@@ -4,7 +4,7 @@
 using Xunit;
 using Xunit.Sdk;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     /// <summary>
     /// Test attribute to run tests synchronously or asynchronously in conjunction with a <see cref="SecretsFixture"/>.

diff --git a/tests/LiveFactDiscoverer.cs b/tests/LiveFactDiscoverer.cs
@@ -8,7 +8,7 @@
 using Xunit.Abstractions;
 using Xunit.Sdk;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     public class LiveFactDiscoverer : IXunitTestCaseDiscoverer
     {

diff --git a/tests/SecretsFixture.cs b/tests/SecretsFixture.cs
@@ -11,7 +11,7 @@
 using Azure.Core.TestFramework;
 using Azure.Security.KeyVault.Secrets;
 
-namespace AzureSample.Security.KeyVault.Proxy
+namespace AzureSamples.Security.KeyVault.Proxy
 {
     /// <summary>
     /// A common test fixture to request secrets through the <see cref="KeyVaultProxy"/>.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3,4 +3,4 @@

		using System.Runtime.CompilerServices;

		[assembly: InternalsVisibleTo("AzureSample.Security.KeyVault.Proxy.Tests")]
		[assembly: InternalsVisibleTo("AzureSamples.Security.KeyVault.Proxy.Tests")]