From 6fffc187024bd87da46105bfe7bc5255b21bfdbe Mon Sep 17 00:00:00 2001 From: Caleb Kiage <747955+calebkiage@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:36:13 +0300 Subject: [PATCH] Replace `/users/me` with `/me` in middleware. (#285) * Replace `/users/me` with `/me` in middleware. * Update args replacement * Refactor `UriReplacementHandler` to use generics, thus eliminating `MeUriReplacement` boxing allocations. Cleanup code. * Use from end indexing shorthand * Use explicit help detection * Move UriReplacement to kiota cli commons * Remove redundant pipeline * Revert: Remove redundant pipeline * Simplify stackalloc initialization * Update src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementTests.cs Co-authored-by: Peter Ombwa * Update src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementTests.cs Co-authored-by: Peter Ombwa * Update MeUriReplacementOption * Update MeUriReplacement to use kiota http handler * Update sonarcloud * Update .github/workflows/sonarcloud.yml Co-authored-by: Peter Ombwa * Update sonar org. * Update sonarcloud org name * Fix code smell * Add coverlet msbuild for coverage reporting * Exclude sample project from sonarcloud exclusions. * Add a me alias to users to show in help --------- Co-authored-by: Peter Ombwa --- .github/workflows/sonarcloud.yml | 78 +++++++++++++++++ .../MeUriReplacementOptionTests.cs | 42 +++++++++ .../Microsoft.Graph.Cli.Core.Tests.csproj | 7 +- .../Http/LoggingHandler.cs | 21 +---- .../UriReplacement/MeUriReplacementOption.cs | 86 +++++++++++++++++++ .../IO/GraphCliClientFactory.cs | 22 +++-- .../Microsoft.Graph.Cli.Core.csproj | 2 +- src/sample/Program.cs | 24 ++++++ 8 files changed, 249 insertions(+), 33 deletions(-) create mode 100644 .github/workflows/sonarcloud.yml create mode 100644 src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementOptionTests.cs create mode 100644 src/Microsoft.Graph.Cli.Core/Http/UriReplacement/MeUriReplacementOption.cs diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml new file mode 100644 index 00000000..f76ff9bd --- /dev/null +++ b/.github/workflows/sonarcloud.yml @@ -0,0 +1,78 @@ +name: Sonarcloud +on: + workflow_dispatch: + push: + branches: + - main + paths-ignore: ['**.md', '.vscode/**', '**.svg'] + pull_request: + types: [opened, synchronize, reopened] + paths-ignore: ['**.md', '.vscode/**', '**.svg'] + +env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + +jobs: + checksecret: + name: check if SONAR_TOKEN is set in github secrets + runs-on: ubuntu-latest + outputs: + is_SONAR_TOKEN_set: ${{ steps.checksecret_job.outputs.is_SONAR_TOKEN_set }} + steps: + - name: Check whether unity activation requests should be done + id: checksecret_job + run: | + echo "is_SONAR_TOKEN_set=${{ env.SONAR_TOKEN != '' }}" >> $GITHUB_OUTPUT + build: + needs: [checksecret] + if: needs.checksecret.outputs.is_SONAR_TOKEN_set == 'true' + name: Build + runs-on: ubuntu-latest + steps: + - name: Set up JDK 17 + uses: actions/setup-java@v3 + with: + distribution: 'adopt' + java-version: 17 + - name: Setup .NET 5 # At the moment the scanner requires dotnet 5 https://www.nuget.org/packages/dotnet-sonarscanner + uses: actions/setup-dotnet@v3 + with: + dotnet-version: 5.0.x + - name: Setup .NET + uses: actions/setup-dotnet@v3 + with: + dotnet-version: 7.0.x + - uses: actions/checkout@v3 + with: + fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis + - name: Cache SonarCloud packages + uses: actions/cache@v3 + with: + path: ~/.sonar/cache + key: ${{ runner.os }}-sonar + restore-keys: ${{ runner.os }}-sonar + - name: Cache SonarCloud scanner + id: cache-sonar-scanner + uses: actions/cache@v3 + with: + path: ./.sonar/scanner + key: ${{ runner.os }}-sonar-scanner + restore-keys: ${{ runner.os }}-sonar-scanner + - name: Install SonarCloud scanner + if: steps.cache-sonar-scanner.outputs.cache-hit != 'true' + shell: pwsh + run: | + New-Item -Path ./.sonar/scanner -ItemType Directory + dotnet tool update dotnet-sonarscanner --tool-path ./.sonar/scanner + - name: Build and analyze + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any + CollectCoverage: true + CoverletOutputFormat: 'opencover' # https://github.com/microsoft/vstest/issues/4014#issuecomment-1307913682 + shell: pwsh + run: | + ./.sonar/scanner/dotnet-sonarscanner begin /k:"microsoftgraph_msgraph-cli-core" /o:"microsoftgraph2" /d:sonar.login="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="**/*.Tests/**/coverage.opencover.xml" /d:sonar.coverage.exclusions="src/sample/**" + dotnet workload restore + dotnet build + dotnet test msgraph-cli-core.sln --no-build --verbosity normal /p:CollectCoverage=true /p:CoverletOutputFormat=opencover + ./.sonar/scanner/dotnet-sonarscanner end /d:sonar.login="${{ secrets.SONAR_TOKEN }}" diff --git a/src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementOptionTests.cs b/src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementOptionTests.cs new file mode 100644 index 00000000..4e8f92df --- /dev/null +++ b/src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementOptionTests.cs @@ -0,0 +1,42 @@ +using System; +using Microsoft.Graph.Cli.Core.Http.UriReplacement; +using Xunit; + +namespace Microsoft.Graph.Cli.Core.Tests.Http.UriReplacement; + +public class MeUriReplacementOptionTests +{ + [Fact] + public void Returns_Null_When_Given_A_Null_Url() + { + var replacement = new MeUriReplacementOption(); + + Assert.Null(replacement.Replace(null)); + } + + [Theory] + [InlineData("http://example.com/test")] + [InlineData("http://example.com/users/messages")] + [InlineData("http://example.com/v1.0/users/messages")] + [InlineData("http://example.com/users/test/me")] + [InlineData("http://example.com/a/b/users/test/me")] + public void Returns_Original_Uri_When_No_Match_Is_Found(string inputUri) + { + var uri = new Uri(inputUri); + var replacement = new MeUriReplacementOption(); + + Assert.Equal(uri, replacement.Replace(uri)); + } + + [Theory] + [InlineData("http://example.com/v1.0/users/me/messages", "http://example.com/v1.0/me/messages")] + [InlineData("http://example.com/v1.0/users/me", "http://example.com/v1.0/me")] + [InlineData("http://example.com/v1.0/users/me?a=b", "http://example.com/v1.0/me?a=b")] + public void Returns_A_New_Url_When_A_Match_Is_Found(string inputUri, string expectedUri) + { + var replacement = new MeUriReplacementOption(); + + var uri = new Uri(inputUri); + Assert.Equal(expectedUri, replacement.Replace(uri)!.ToString()); + } +} diff --git a/src/Microsoft.Graph.Cli.Core.Tests/Microsoft.Graph.Cli.Core.Tests.csproj b/src/Microsoft.Graph.Cli.Core.Tests/Microsoft.Graph.Cli.Core.Tests.csproj index f0f2f0dd..4a01a512 100644 --- a/src/Microsoft.Graph.Cli.Core.Tests/Microsoft.Graph.Cli.Core.Tests.csproj +++ b/src/Microsoft.Graph.Cli.Core.Tests/Microsoft.Graph.Cli.Core.Tests.csproj @@ -19,14 +19,11 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all - - runtime; build; native; contentfiles; analyzers; buildtransitive - all - + - \ No newline at end of file + diff --git a/src/Microsoft.Graph.Cli.Core/Http/LoggingHandler.cs b/src/Microsoft.Graph.Cli.Core/Http/LoggingHandler.cs index 66900383..60b79797 100644 --- a/src/Microsoft.Graph.Cli.Core/Http/LoggingHandler.cs +++ b/src/Microsoft.Graph.Cli.Core/Http/LoggingHandler.cs @@ -45,12 +45,6 @@ protected override async Task SendAsync( return response; } - /// - protected override void Dispose(bool disposing) - { - base.Dispose(disposing); - } - private static string HeadersToString(in HttpHeaders headers, in HttpContentHeaders? contentHeaders) { if (!headers.Any() && contentHeaders?.Any() == false) return string.Empty; @@ -61,7 +55,7 @@ static string selector(KeyValuePair> h) { value = "[PROTECTED]"; } - return string.Format("{0}: {1}\n", h.Key, value); + return $"{h.Key}: {value}\n"; }; static string aggregator(string a, string b) @@ -95,17 +89,8 @@ private static async Task ContentToStringAsync(HttpContent? content, Can { return await content.ReadAsStringAsync(cancellationToken); } - else - { - if (content.Headers.ContentLength > 0) - { - return $"[...<{content.Headers.ContentLength} byte data stream>...]"; - } - else - { - return "[......]"; - } - } + + return content.Headers.ContentLength > 0 ? $"[...<{content.Headers.ContentLength} byte data stream>...]" : "[......]"; } [LoggerMessage(EventId = 1, Level = LogLevel.Debug, Message = "\nRequest:\n\n{RequestMethod} {RequestUri} HTTP/{HttpVersion}\n{Headers}\n{RequestContent}\n")] diff --git a/src/Microsoft.Graph.Cli.Core/Http/UriReplacement/MeUriReplacementOption.cs b/src/Microsoft.Graph.Cli.Core/Http/UriReplacement/MeUriReplacementOption.cs new file mode 100644 index 00000000..eb69ed3d --- /dev/null +++ b/src/Microsoft.Graph.Cli.Core/Http/UriReplacement/MeUriReplacementOption.cs @@ -0,0 +1,86 @@ +using System; +using Microsoft.Kiota.Http.HttpClientLibrary.Middleware.Options; + +namespace Microsoft.Graph.Cli.Core.Http.UriReplacement; + +/// +/// Specialized replacement for /[version]/users/me with /[version]/me +/// +public class MeUriReplacementOption : IUriReplacementHandlerOption +{ + private readonly bool isEnabled; + + /// + /// Create new MeUriReplacementOption + /// + /// Whether the uri replacement is enabled. + public MeUriReplacementOption(bool isEnabled = true) + { + this.isEnabled = isEnabled; + } + + /// + public bool IsEnabled() + { + return isEnabled; + } + + /// + /// If a URI path starts with /[version]/users/me, replace it with /[version]/me + /// + /// The original URI + /// A URI with /[version]/users/me replaced with /[version]/me + /// This method assumes that the first segment after the root is a version segment to match Microsoft Graph API's format. + public Uri? Replace(Uri? original) + { + if (original is null) + { + return null; + } + + if (!isEnabled || original.Segments.Length < 4) + { + // Must have at least segments "/", "[version]/", "users/", "me" + return original; + } + + Span toMatch = stackalloc char[] { '/', 'u', 's', 'e', 'r', 's', '/', 'm', 'e' }; + var separator = toMatch[..1]; + var matchUsers = toMatch[1..6]; + var matchMe = toMatch[7..]; + + var maybeUsersSegment = original.Segments[2].AsSpan(); + if (!maybeUsersSegment[..^1].SequenceEqual(matchUsers)) + { + return original; + } + + var maybeMeSegment = original.Segments[3].AsSpan(); + if (!maybeMeSegment[..(maybeMeSegment.EndsWith(separator) ? ^1 : ^0)].SequenceEqual(matchMe)) + { + return original; + } + + var newUrl = new UriBuilder(original); + var versionSegment = original.Segments[1].AsSpan(); + const int usersMeLength = 9; + var length = versionSegment.Length + usersMeLength; + if (newUrl.Path.Length == length) + { + // Matched /[version]/users/me + newUrl.Path = string.Concat(separator, versionSegment, matchMe); + } + else + { + // Maybe matched /[version]/users/me... + // Logic to make sure we don't match paths like /users/messages + var span = newUrl.Path.AsSpan(length); + if (span[0] == '/') + { + newUrl.Path = string.Concat(separator, versionSegment, matchMe, span); + } + } + + return newUrl.Uri; + } +} diff --git a/src/Microsoft.Graph.Cli.Core/IO/GraphCliClientFactory.cs b/src/Microsoft.Graph.Cli.Core/IO/GraphCliClientFactory.cs index 67ae7c05..2a1d3cff 100644 --- a/src/Microsoft.Graph.Cli.Core/IO/GraphCliClientFactory.cs +++ b/src/Microsoft.Graph.Cli.Core/IO/GraphCliClientFactory.cs @@ -1,6 +1,8 @@ using System.Collections.Generic; using System.Net.Http; using Microsoft.Graph.Cli.Core.Http; +using Microsoft.Graph.Cli.Core.Http.UriReplacement; +using Microsoft.Kiota.Http.HttpClientLibrary.Middleware; namespace Microsoft.Graph.Cli.Core.IO; @@ -38,8 +40,11 @@ public static HttpClient GetDefaultClient(GraphClientOptions? options = null, st m.AddRange(middlewares); + // Add replacement handler for /users/me to /me + m.Add(new UriReplacementHandler(new MeUriReplacementOption())); + // Add logging handler. - if (loggingHandler is LoggingHandler lh) + if (loggingHandler is { } lh) { m.Add(lh); } @@ -47,20 +52,19 @@ public static HttpClient GetDefaultClient(GraphClientOptions? options = null, st // Set compression handler to be last (Allows logging handler to log request body) m.Sort((a, b) => { - var a_match = a is Kiota.Http.HttpClientLibrary.Middleware.CompressionHandler; - var b_match = b is Kiota.Http.HttpClientLibrary.Middleware.CompressionHandler; - if (a_match && !b_match) + var aMatch = a is Kiota.Http.HttpClientLibrary.Middleware.CompressionHandler; + var bMatch = b is Kiota.Http.HttpClientLibrary.Middleware.CompressionHandler; + if (aMatch && !bMatch) { return 1; } - else if (b_match && !a_match) + + if (bMatch && !aMatch) { return -1; } - else - { - return 0; - } + + return 0; }); return GraphClientFactory.Create(version: version, nationalCloud: nationalCloud, finalHandler: finalHandler, handlers: m); diff --git a/src/Microsoft.Graph.Cli.Core/Microsoft.Graph.Cli.Core.csproj b/src/Microsoft.Graph.Cli.Core/Microsoft.Graph.Cli.Core.csproj index 7dc12f56..1c302bf9 100644 --- a/src/Microsoft.Graph.Cli.Core/Microsoft.Graph.Cli.Core.csproj +++ b/src/Microsoft.Graph.Cli.Core/Microsoft.Graph.Cli.Core.csproj @@ -25,7 +25,7 @@ ../35MSSharedLib1024.snk false false - true + false true diff --git a/src/sample/Program.cs b/src/sample/Program.cs index 48a1805f..412b4f56 100644 --- a/src/sample/Program.cs +++ b/src/sample/Program.cs @@ -6,6 +6,7 @@ using System.CommandLine.Parsing; using System.Diagnostics.Tracing; using System.IO; +using System.Linq; using System.Net.Http; using System.Reflection; using System.Threading.Tasks; @@ -42,6 +43,24 @@ class Program static async Task Main(string[] args) { + // Replace `me ...` with `users ... --user-id me` + if (args[0] == "me") + { + var hasHelp = Array.Exists(args, static x => x == "--help" || x == "-h" || x == "/?"); + var newArgs = hasHelp ? args : new string[args.Length + 2]; + newArgs[0] = "users"; + for (var i = 1; i < args.Length; i++) + { + newArgs[i] = args[i]; + } + if (newArgs.Length > args.Length) + { + newArgs[args.Length] = "--user-id"; + newArgs[args.Length + 1] = "me"; + args = newArgs; + } + } + var builder = BuildCommandLine() .UseDefaults() .UseHost(CreateHostBuilder) @@ -127,6 +146,11 @@ static CommandLineBuilder BuildCommandLine() rootCommand.Add(new LoginCommand(builder)); rootCommand.AddGlobalOption(debugOption); + if (rootCommand.Subcommands.FirstOrDefault(static c => c.Name == "users") is {} usersCmd) + { + usersCmd.AddAlias("me"); + } + return builder; }