Skip to content

Commit

Permalink
Replace /users/me with /me in middleware. (#285)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* Update src/Microsoft.Graph.Cli.Core.Tests/Http/UriReplacement/MeUriReplacementTests.cs

Co-authored-by: Peter Ombwa <[email protected]>

* Update MeUriReplacementOption

* Update MeUriReplacement to use kiota http handler

* Update sonarcloud

* Update .github/workflows/sonarcloud.yml

Co-authored-by: Peter Ombwa <[email protected]>

* 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 <[email protected]>
  • Loading branch information
calebkiage and peombwa authored Nov 6, 2023
1 parent d8c1014 commit 6fffc18
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 33 deletions.
78 changes: 78 additions & 0 deletions .github/workflows/sonarcloud.yml
Original file line number Diff line number Diff line change
@@ -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 }}"
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="coverlet.collector" Version="6.0.0">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="coverlet.msbuild" Version="6.0.0" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Microsoft.Graph.Cli.Core\Microsoft.Graph.Cli.Core.csproj" />
</ItemGroup>

</Project>
</Project>
21 changes: 3 additions & 18 deletions src/Microsoft.Graph.Cli.Core/Http/LoggingHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,6 @@ protected override async Task<HttpResponseMessage> SendAsync(
return response;
}

/// <inheritdoc/>
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;
Expand All @@ -61,7 +55,7 @@ static string selector(KeyValuePair<string, IEnumerable<string>> h)
{
value = "[PROTECTED]";
}
return string.Format("{0}: {1}\n", h.Key, value);
return $"{h.Key}: {value}\n";
};

static string aggregator(string a, string b)
Expand Down Expand Up @@ -95,17 +89,8 @@ private static async Task<string> ContentToStringAsync(HttpContent? content, Can
{
return await content.ReadAsStringAsync(cancellationToken);
}
else
{
if (content.Headers.ContentLength > 0)
{
return $"[...<{content.Headers.ContentLength} byte data stream>...]";
}
else
{
return "[...<data stream>...]";
}
}

return content.Headers.ContentLength > 0 ? $"[...<{content.Headers.ContentLength} byte data stream>...]" : "[...<data stream>...]";
}

[LoggerMessage(EventId = 1, Level = LogLevel.Debug, Message = "\nRequest:\n\n{RequestMethod} {RequestUri} HTTP/{HttpVersion}\n{Headers}\n{RequestContent}\n")]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
using System;
using Microsoft.Kiota.Http.HttpClientLibrary.Middleware.Options;

namespace Microsoft.Graph.Cli.Core.Http.UriReplacement;

/// <summary>
/// Specialized replacement for /[version]/users/me with /[version]/me
/// </summary>
public class MeUriReplacementOption : IUriReplacementHandlerOption
{
private readonly bool isEnabled;

/// <summary>
/// Create new MeUriReplacementOption
/// </summary>
/// <param name="isEnabled">Whether the uri replacement is enabled.</param>
public MeUriReplacementOption(bool isEnabled = true)
{
this.isEnabled = isEnabled;
}

/// <inheritdoc/>
public bool IsEnabled()
{
return isEnabled;
}

/// <summary>
/// If a URI path starts with /[version]/users/me, replace it with /[version]/me
/// </summary>
/// <param name="original">The original URI</param>
/// <returns>A URI with /[version]/users/me replaced with /[version]/me</returns>
/// <remarks>This method assumes that the first segment after the root is a version segment to match Microsoft Graph API's format.</remarks>
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<char> 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;
}
}
22 changes: 13 additions & 9 deletions src/Microsoft.Graph.Cli.Core/IO/GraphCliClientFactory.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -38,29 +40,31 @@ public static HttpClient GetDefaultClient(GraphClientOptions? options = null, st

m.AddRange(middlewares);

// Add replacement handler for /users/me to /me
m.Add(new UriReplacementHandler<MeUriReplacementOption>(new MeUriReplacementOption()));

// Add logging handler.
if (loggingHandler is LoggingHandler lh)
if (loggingHandler is { } lh)
{
m.Add(lh);
}

// 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<AssemblyOriginatorKeyFile>../35MSSharedLib1024.snk</AssemblyOriginatorKeyFile>
<SignAssembly>false</SignAssembly>
<DelaySign>false</DelaySign>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageReleaseNotes>
</PackageReleaseNotes>
Expand Down
24 changes: 24 additions & 0 deletions src/sample/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -42,6 +43,24 @@ class Program

static async Task<int> 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)
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 6fffc18

Please sign in to comment.