Skip to content

fix: resolve trimming warnings in project #903

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

Merged
merged 2 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ configuration:
# Required status checks to pass before merging. Values can be any string, but if the value does not correspond to any existing status check, the status check will be stuck on pending for status since nothing exists to push an actual status
requiredStatusChecks:
- Build and Test # Contains CodeQL
- Validate Project for Trimming
- license/cla
# Require branches to be up to date before merging. boolean
requiresStrictStatusChecks: true
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/sonarcloud.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ jobs:
CoverletOutputFormat: "opencover" # https://github.com/microsoft/vstest/issues/4014#issuecomment-1307913682
shell: pwsh
run: |
./.sonar/scanner/dotnet-sonarscanner begin /k:"microsoftgraph_msgraph-sdk-dotnet-core" /o:"microsoftgraph2" /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="tests/Microsoft.Graph.DotnetCore.Core.Test/coverage.opencover.xml"
./.sonar/scanner/dotnet-sonarscanner begin /k:"microsoftgraph_msgraph-sdk-dotnet-core" /o:"microsoftgraph2" /d:sonar.scanner.scanAll=false /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.cs.opencover.reportsPaths="tests/Microsoft.Graph.DotnetCore.Core.Test/coverage.opencover.xml"
dotnet workload restore
dotnet build
dotnet test Microsoft.Graph.Core.sln --no-build --verbosity normal /p:CollectCoverage=true /p:CoverletOutputFormat=opencover --framework net6.0
Expand Down
19 changes: 16 additions & 3 deletions .github/workflows/validatePullRequest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ jobs:

- name: Install needed dotnet workloads
run: dotnet workload install android macos ios maccatalyst

- name: Restore nuget dependencies
run: dotnet restore ${{ env.solutionName }}

- name: Lint the code
run: dotnet format --verify-no-changes

- name: Restore nuget dependencies
run: dotnet restore ${{ env.solutionName }}

- name: Build
run: dotnet build ${{ env.solutionName }} -c Debug /p:UseSharedCompilation=false,IncludeMauiTargets=true
Expand All @@ -56,4 +56,17 @@ jobs:
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v3

validate-trimming:
name: Validate Project for Trimming
runs-on: windows-latest
steps:
- uses: actions/checkout@v4.1.7

- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: 8.x

- name: Validate Trimming warnings
run: dotnet publish -c Release -r win-x64 /p:TreatWarningsAsErrors=true /warnaserror -f net8.0
working-directory: ./tests/Microsoft.Graph.DotnetCore.Core.Trimming
10 changes: 5 additions & 5 deletions src/Microsoft.Graph.Core/Microsoft.Graph.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,16 @@
</None>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.0.1" />
<PackageReference Include="Microsoft.IdentityModel.Validators" Version="8.0.1" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.0.2" />
<PackageReference Include="Microsoft.IdentityModel.Validators" Version="8.0.2" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" />
<PackageReference Include="Microsoft.Kiota.Abstractions" Version="1.12.3" />
<PackageReference Include="Microsoft.Kiota.Authentication.Azure" Version="1.12.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Json" Version="1.12.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Text" Version="1.11.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Form" Version="1.11.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Text" Version="1.12.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Form" Version="1.12.3" />
<PackageReference Include="Microsoft.Kiota.Http.HttpClientLibrary" Version="1.12.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Multipart" Version="1.11.3" />
<PackageReference Include="Microsoft.Kiota.Serialization.Multipart" Version="1.12.3" />
<PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.11.20">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
Expand Down
60 changes: 41 additions & 19 deletions src/Microsoft.Graph.Core/Tasks/LargeFileUploadTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@
{
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.IO;
using System.Net.Http;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Graph.Core.Models;
using Microsoft.Kiota.Abstractions;
using Microsoft.Kiota.Abstractions.Authentication;
using Microsoft.Kiota.Abstractions.Serialization;
using Microsoft.Kiota.Serialization.Json;

/// <summary>
/// Task to help with resumable large file uploads.
Expand All @@ -40,7 +43,23 @@
/// <param name="maxSliceSize">Max size(in bytes) of each slice to be uploaded. Defaults to 5MB. When uploading to OneDrive or SharePoint, this value needs to be a multiple of 320 KiB (327,680 bytes).
/// If less than 0, default value of 5 MiB is used.</param>
/// <param name="requestAdapter"><see cref="IRequestAdapter"/> to use for making upload requests. The client should not set Auth headers as upload urls do not need them.</param>
public LargeFileUploadTask(IParsable uploadSession, Stream uploadStream, int maxSliceSize = -1, IRequestAdapter requestAdapter = null)
[Obsolete("Use the overload that takes in IUploadSession instead.")]

Check warning on line 46 in src/Microsoft.Graph.Core/Tasks/LargeFileUploadTask.cs

View workflow job for this annotation

GitHub Actions / Build

Do not forget to remove this deprecated code someday.

Check warning on line 46 in src/Microsoft.Graph.Core/Tasks/LargeFileUploadTask.cs

View workflow job for this annotation

GitHub Actions / Build

Do not forget to remove this deprecated code someday.

Check warning on line 46 in src/Microsoft.Graph.Core/Tasks/LargeFileUploadTask.cs

View workflow job for this annotation

GitHub Actions / Build

Do not forget to remove this deprecated code someday.
[EditorBrowsable(EditorBrowsableState.Never)]
public LargeFileUploadTask(IParsable uploadSession, Stream uploadStream, int maxSliceSize = -1,
IRequestAdapter requestAdapter = null) : this(ExtractSessionFromParsable(uploadSession), uploadStream, maxSliceSize, requestAdapter)
{
}

/// <summary>
/// Task to help with resumable large file uploads. Generates slices based on <paramref name="uploadSession"/>
/// information, and can control uploading of requests.
/// </summary>
/// <param name="uploadSession">Session information of type <see cref="IUploadSession"/>></param>
/// <param name="uploadStream">Readable, seekable stream to be uploaded. Length of session is determined via uploadStream.Length</param>
/// <param name="maxSliceSize">Max size(in bytes) of each slice to be uploaded. Defaults to 5MB. When uploading to OneDrive or SharePoint, this value needs to be a multiple of 320 KiB (327,680 bytes).
/// If less than 0, default value of 5 MiB is used.</param>
/// <param name="requestAdapter"><see cref="IRequestAdapter"/> to use for making upload requests. The client should not set Auth headers as upload urls do not need them.</param>
public LargeFileUploadTask(IUploadSession uploadSession, Stream uploadStream, int maxSliceSize = -1, IRequestAdapter requestAdapter = null)
{
if (!uploadStream.CanRead || !uploadStream.CanSeek)
{
Expand All @@ -50,8 +69,8 @@
{
throw new ArgumentException("Must a stream that is not empty");
}
this.Session = ExtractSessionFromParsable(uploadSession);
this._requestAdapter = requestAdapter ?? this.InitializeAdapter(Session.UploadUrl);
this.Session = uploadSession;
this._requestAdapter = requestAdapter ?? InitializeAdapter(Session.UploadUrl);
this._uploadStream = uploadStream;
this._rangesRemaining = this.GetRangesRemaining(Session);
this._maxSliceSize = maxSliceSize < 0 ? DefaultMaxSliceSize : maxSliceSize;
Expand All @@ -63,31 +82,34 @@
/// <param name="uploadSession"><see cref="IParsable"/> to initialize an <see cref="IUploadSession"/> from</param>
/// <returns>A <see cref="IUploadSession"/> instance</returns>
/// <exception cref="NotImplementedException"></exception>
private IUploadSession ExtractSessionFromParsable(IParsable uploadSession)
internal static IUploadSession ExtractSessionFromParsable(IParsable uploadSession)
{
if (!uploadSession.GetFieldDeserializers().ContainsKey("expirationDateTime"))
if (uploadSession is IUploadSession uploadSessionCast)
{
return uploadSessionCast;
}

// this scenario (unlikely) will occur in the event the model in the service libraries hasn't been updated to implement the IUploadSession interface from core.
var fieldDeserializers = uploadSession.GetFieldDeserializers();
if (!fieldDeserializers.ContainsKey("expirationDateTime"))
throw new ArgumentException("The Parsable does not contain the 'expirationDateTime' property");
if (!uploadSession.GetFieldDeserializers().ContainsKey("nextExpectedRanges"))
if (!fieldDeserializers.ContainsKey("nextExpectedRanges"))
throw new ArgumentException("The Parsable does not contain the 'nextExpectedRanges' property");
if (!uploadSession.GetFieldDeserializers().ContainsKey("uploadUrl"))
if (!fieldDeserializers.ContainsKey("uploadUrl"))
throw new ArgumentException("The Parsable does not contain the 'uploadUrl' property");

var uploadSessionType = uploadSession.GetType();

return new UploadSession()
{
ExpirationDateTime = uploadSessionType.GetProperty("ExpirationDateTime").GetValue(uploadSession, null) as DateTimeOffset?,
NextExpectedRanges = uploadSessionType.GetProperty("NextExpectedRanges").GetValue(uploadSession, null) as List<string>,
UploadUrl = uploadSessionType.GetProperty("UploadUrl").GetValue(uploadSession, null) as string
};
// convert to local type as we don't have the type info for the upload session just that it implements IParsable
using var uploadSessionStream = KiotaJsonSerializer.SerializeAsStream(uploadSession, false);// just in case there's a backing store
var uploadSessionJsonNode = new JsonParseNode(JsonDocument.Parse(uploadSessionStream).RootElement);
return uploadSessionJsonNode.GetObjectValue(UploadSession.CreateFromDiscriminatorValue);
}

/// <summary>
/// Initialize a baseClient to use for the upload that does not have Auth enabled as the upload URLs explicitly do not need authentication.
/// </summary>
/// <param name="uploadUrl">Url to perform the upload to from the session</param>
/// <returns></returns>
private IRequestAdapter InitializeAdapter(string uploadUrl)
private static BaseGraphRequestAdapter InitializeAdapter(string uploadUrl)
{
HttpClient httpClient = GraphClientFactory.Create(); //no auth
httpClient.SetFeatureFlag(FeatureFlag.FileUploadTask);
Expand Down Expand Up @@ -196,7 +218,7 @@
return uploadResult;
}

ThrowIfUploadCancelled(cancellationToken, trackedExceptions);
ThrowIfUploadCancelled(trackedExceptions, cancellationToken);
}

await this.UpdateSessionStatusAsync(cancellationToken).ConfigureAwait(false);
Expand All @@ -207,7 +229,7 @@
await Task.Delay(2000 * uploadTries * uploadTries, cancellationToken).ConfigureAwait(false);
}

ThrowIfUploadCancelled(cancellationToken, trackedExceptions);
ThrowIfUploadCancelled(trackedExceptions, cancellationToken);
}

throw new TaskCanceledException("Upload failed too many times. See InnerException for list of exceptions that occured.", new AggregateException(trackedExceptions.ToArray()));
Expand Down Expand Up @@ -292,7 +314,7 @@
: sizeBasedOnRange;
}

private void ThrowIfUploadCancelled(CancellationToken cancellationToken, ICollection<Exception> trackedExceptions)
private static void ThrowIfUploadCancelled(ICollection<Exception> trackedExceptions, CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested)
throw new OperationCanceledException("File upload cancelled. See InnerException for list of exceptions that occured.", new AggregateException(trackedExceptions));
Expand Down
11 changes: 9 additions & 2 deletions src/Microsoft.Graph.Core/Tasks/PageIterator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
using System.Threading.Tasks;
using Microsoft.Kiota.Abstractions;
using Microsoft.Kiota.Abstractions.Serialization;
#if NET5_0_OR_GREATER
using System.Diagnostics.CodeAnalysis;
#endif

/*
Spec https://github.com/microsoftgraph/msgraph-sdk-design/blob/main/tasks/PageIteratorTask.md
Expand All @@ -21,7 +24,11 @@
/// </summary>
/// <typeparam name="TEntity">The Microsoft Graph entity type returned in the result set.</typeparam>
/// <typeparam name="TCollectionPage">The Microsoft Graph collection response type returned in the collection response.</typeparam>
#if NET5_0_OR_GREATER
public class PageIterator<TEntity, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)]TCollectionPage> where TCollectionPage : IParsable, IAdditionalDataHolder, new()
#else
public class PageIterator<TEntity, TCollectionPage> where TCollectionPage : IParsable, IAdditionalDataHolder, new()
#endif
{
private IRequestAdapter _requestAdapter;
private TCollectionPage _currentPage;
Expand Down Expand Up @@ -89,7 +96,7 @@
if (requestAdapter == null)
throw new ArgumentNullException(nameof(requestAdapter));

if (page == null)

Check warning on line 99 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.

Check warning on line 99 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.

Check warning on line 99 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.

Check warning on line 99 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.
throw new ArgumentNullException(nameof(page));

if (callback == null)
Expand Down Expand Up @@ -146,7 +153,7 @@
if (requestAdapter == null)
throw new ArgumentNullException(nameof(requestAdapter));

if (page == null)

Check warning on line 156 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.

Check warning on line 156 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.

Check warning on line 156 in src/Microsoft.Graph.Core/Tasks/PageIterator.cs

View workflow job for this annotation

GitHub Actions / Build

Use a comparison to 'default(TCollectionPage)' instead or add a constraint to 'TCollectionPage' so that it can't be a value type.
throw new ArgumentNullException(nameof(page));

if (asyncCallback == null)
Expand Down Expand Up @@ -364,7 +371,7 @@
/// <exception cref="ArgumentException">Thrown when the object doesn't contain a collection inside it</exception>
private static List<TEntity> ExtractEntityListFromParsable(TCollectionPage parsableCollection)
{
return parsableCollection.GetType().GetProperty("Value")?.GetValue(parsableCollection, null) as List<TEntity> ?? throw new ArgumentException("The Parsable does not contain a collection property");
return typeof(TCollectionPage).GetProperty("Value")?.GetValue(parsableCollection, null) as List<TEntity> ?? throw new ArgumentException("The Parsable does not contain a collection property");
}

/// <summary>
Expand All @@ -375,7 +382,7 @@
/// <returns></returns>
private static string ExtractNextLinkFromParsable(TCollectionPage parsableCollection, string nextLinkPropertyName = "OdataNextLink")
{
var nextLinkProperty = parsableCollection.GetType().GetProperty(nextLinkPropertyName);
var nextLinkProperty = typeof(TCollectionPage).GetProperty(nextLinkPropertyName);
if (nextLinkProperty != null &&
nextLinkProperty.GetValue(parsableCollection, null) is string nextLinkString
&& !string.IsNullOrEmpty(nextLinkString))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
namespace Microsoft.Graph.DotnetCore.Core.Test.Mocks
{
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Kiota.Abstractions.Serialization;

/// <summary>
/// Concrete implementation of the IUploadSession interface
/// </summary>
internal class MockUploadSessionWithoutUploadSessionInterface : IParsable, IAdditionalDataHolder
{
/// <summary>
/// Expiration date of the upload session
/// </summary>
public DateTimeOffset? ExpirationDateTime
{
get; set;
}

/// <summary>
/// The ranges yet to be uploaded to the server
/// </summary>
public List<string> NextExpectedRanges
{
get; set;
}

/// <summary>
/// The URL for upload
/// </summary>
public string UploadUrl
{
get; set;
}

/// <summary>
/// Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well.
/// </summary>
public IDictionary<string, object> AdditionalData { get; set; } = new Dictionary<string, object>();

/// <summary>
/// The deserialization information for the current model
/// </summary>
public IDictionary<string, Action<IParseNode>> GetFieldDeserializers()
{
return new Dictionary<string, Action<IParseNode>>(StringComparer.OrdinalIgnoreCase)
{
{"expirationDateTime", (n) => { ExpirationDateTime = n.GetDateTimeOffsetValue(); } },
{"nextExpectedRanges", (n) => { NextExpectedRanges = n.GetCollectionOfPrimitiveValues<string>().ToList(); } },
{"uploadUrl", (n) => { UploadUrl = n.GetStringValue(); } },
};
}

/// <summary>
/// Serializes information the current object
/// <param name="writer">Serialization writer to use to serialize this model</param>
/// </summary>
public void Serialize(ISerializationWriter writer)
{
_ = writer ?? throw new ArgumentNullException(nameof(writer));
writer.WriteDateTimeOffsetValue("expirationDateTime", ExpirationDateTime);
writer.WriteCollectionOfPrimitiveValues<string>("nextExpectedRanges", NextExpectedRanges);
writer.WriteStringValue("uploadUrl", UploadUrl);
writer.WriteAdditionalData(AdditionalData);
}

/// <summary>
/// Creates a new instance of the appropriate class based on discriminator value
/// <param name="parseNode">The parse node to use to read the discriminator value and create the object</param>
/// </summary>
public static MockUploadSessionWithoutUploadSessionInterface CreateFromDiscriminatorValue(IParseNode parseNode)
{
_ = parseNode ?? throw new ArgumentNullException(nameof(parseNode));
return new MockUploadSessionWithoutUploadSessionInterface();
}
}
}
Loading