Conversation
* update to .net10 (#1764) * update to .net10 * updated dockerfile and actions to use .net10, revert automapper to 15.1 * remove dotnet version 3.0 from github actions * exclude schemafilters from code coverage, fix null setting --------- Co-authored-by: Dhanalakshmi Gopalswamy <acn-dgopa@ai-dev.no> * forwarded continuation token to the repository sql command (#1782) * fix party uuid requirement for e2e tests for AM requests (#1790) --------- Co-authored-by: Dhanalakshmi Gopalswamy <34273718+acn-dgopa@users.noreply.github.com> Co-authored-by: Dhanalakshmi Gopalswamy <acn-dgopa@ai-dev.no> Co-authored-by: Vegard Wright <vegard.nyeng@gmail.com>
📝 WalkthroughWalkthroughThis PR migrates the project to .NET 10.0 across all CI/CD workflows, project files, and NuGet dependencies. Additionally, it updates OpenAPI schema filter implementations to use the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/Altinn.Platform.Authentication.SystemIntegrationTests/Tests/SystemUserTests.cs (1)
498-507: Potential null reference inApproveSystemUserRequest.The method accepts
Testuser? testuserbut accessestestuser.AltinnPartyIdon line 501 and passestestusertoApproveRequestwhich now requires a non-nullableTestuser. This could cause aNullReferenceExceptionif called with null.Suggested fix
- private async Task ApproveSystemUserRequest(Testuser? testuser, string requestId, HttpStatusCode expectedStatusCode = HttpStatusCode.OK) + private async Task ApproveSystemUserRequest(Testuser testuser, string requestId, HttpStatusCode expectedStatusCode = HttpStatusCode.OK) { var approveUrl = Endpoints.ApproveSystemUserRequest.Url() .Replace("{party}", testuser.AltinnPartyId) .Replace("{requestId}", requestId); var approveResponse = await ApproveRequest(approveUrl, testuser); Assert.True(approveResponse.StatusCode == expectedStatusCode, $"Approval failed with status code: {approveResponse.StatusCode}"); }src/Persistance/RepositoryImplementations/RequestRepository.cs (1)
592-629: MissingORDER BYclause breaks pagination.
GetAllAgentRequestsBySystemlacksORDER BY r.id ASC(compare withGetAllRequestsBySystemat line 529). Without deterministic ordering, pagination may return duplicates or skip records.Fix: Add ORDER BY clause
FROM business_application.request r WHERE r.system_id = `@system_id` and r.id > `@continue_from` and r.is_deleted = false - and systemuser_type = `@systemuser_type`;"; + and systemuser_type = `@systemuser_type` + ORDER BY r.id ASC;";
🤖 Fix all issues with AI agents
In @.github/workflows/codeql-analysis.yml:
- Around line 33-37: The workflow is setting actions/setup-dotnet with
dotnet-version: 10.0.x which CodeQL's C#/.NET extractor doesn't support; fix by
either changing the dotnet-version value used by the actions/setup-dotnet step
to a supported SDK like 9.0.x or 8.0.x, or remove/disable the autobuild step and
add explicit build commands (dotnet restore and dotnet build) between the CodeQL
init and analyze steps so the analyzer sees a proper build; update the
actions/setup-dotnet usage or the autobuild/init/analyze sequence accordingly to
ensure CodeQL can build the C# code.
🧹 Nitpick comments (7)
src/Core/Models/SystemUsers/RequestSystemResponseInternal.cs (1)
95-99: Consider addingJsonPropertyNameattribute for consistency.The
Escalatedproperty lacks a[JsonPropertyName("escalated")]attribute, while other properties in this class (e.g.,Created,ConfirmUrl) and the correspondingRequestSystemResponsemodel have JSON serialization attributes.Suggested change
/// <summary> /// Tracks if the original user creating the Request have escalated the Approval of this Request /// to somebody else in the organisation with AccessManager (Package:Tilgangsstyring) /// </summary> + [JsonPropertyName("escalated")] public bool Escalated { get; set; }src/Authentication/Services/RequestSystemUserService.cs (1)
798-804: Duplicate pagination logic could be extracted.This pagination token handling is identical to lines 766-773 in
GetAllRequestsForVendor. Consider extracting a helper method to reduce duplication.Example helper
private static Guid ExtractContinuationToken(Page<Guid>.Request continueRequest) { if (continueRequest is not null && continueRequest.ContinuationToken != Guid.Empty) { return continueRequest.ContinuationToken; } return Guid.Empty; }src/Core/RepositoryInterfaces/IRequestRepository.cs (2)
47-53: Documentation incomplete for new parameter.The XML documentation is missing the
<param name="continueFrom">description for the new pagination parameter.Suggested documentation
/// <summary> /// Retrieves a list of Status-Response-model for all Requests that the Vendor has /// </summary> /// <param name="systemId">The chosen system</param> + /// <param name="continueFrom">The continuation token (Guid) for pagination; use Guid.Empty for the first page</param> /// <param name="cancellationToken">The cancellationToken</param> /// <returns></returns> Task<List<RequestSystemResponse>> GetAllRequestsBySystem(string systemId, Guid continueFrom, CancellationToken cancellationToken);
55-61: Documentation incomplete for new parameter.Same as above - missing
<param name="continueFrom">description.Suggested documentation
/// <summary> /// Retrieves a list of Status-Response-model for all agent Requests that the Vendor has /// </summary> /// <param name="systemId">The chosen system</param> + /// <param name="continueFrom">The continuation token (Guid) for pagination; use Guid.Empty for the first page</param> /// <param name="cancellationToken">The cancellationToken</param> /// <returns>list of agent requests for a system the vendor has</returns> Task<List<AgentRequestSystemResponse>> GetAllAgentRequestsBySystem(string systemId, Guid continueFrom, CancellationToken cancellationToken);src/Authentication/Model/ClientInfoPaginated.cs (1)
3-3: Unused import detected.The
using Microsoft.OpenApi;directive does not appear to be used in this file. No types from the OpenAPI namespace are referenced in the visible code.♻️ Suggested fix
`#nullable` enable using System.Collections.Generic; -using Microsoft.OpenApi; using Swashbuckle.AspNetCore.Annotations; using Swashbuckle.AspNetCore.SwaggerGen;src/Authentication/Model/ItemStream.cs (1)
89-94: Inconsistent schema filter implementation pattern.This implementation directly accesses
schema.Requiredwithout type-checking, whilePaginated.cs(lines 52-61) andOpaque.cs(lines 218-225) use defensiveif (schema is OpenApiSchema openApiSchema)checks before accessing schema properties.Consider aligning with the defensive pattern used elsewhere for consistency and to guard against potential interface changes:
♻️ Suggested defensive pattern
public void Apply(IOpenApiSchema schema, SchemaFilterContext context) { - schema.Required.Add("pageStart"); - schema.Required.Add("pageEnd"); - schema.Required.Add("sequenceMax"); + if (schema is OpenApiSchema openApiSchema) + { + openApiSchema.Required.Add("pageStart"); + openApiSchema.Required.Add("pageEnd"); + openApiSchema.Required.Add("sequenceMax"); + } }src/Authentication/Model/ListObject.cs (1)
33-39: Inconsistent schema filter implementation pattern.Same concern as
ItemStream.cs— this implementation directly accessesschema.Propertiesandschema.Requiredwithout the defensive type-checking pattern used inPaginated.csandOpaque.cs.♻️ Suggested defensive pattern
public void Apply(IOpenApiSchema schema, SchemaFilterContext context) { - foreach (var prop in schema.Properties) + if (schema is OpenApiSchema openApiSchema) { - schema.Required.Add(prop.Key); + foreach (var prop in openApiSchema.Properties) + { + openApiSchema.Required.Add(prop.Key); + } } }Note: You'll need to add
using Microsoft.OpenApi.Models;forOpenApiSchematype access.
| - name: Setup .NET 10.0.* SDK | ||
| uses: actions/setup-dotnet@2016bd2012dba4e32de620c46fe006a3ac9f0602 # v5 | ||
| with: | ||
| dotnet-version: | | ||
| 9.0.x | ||
| 3.1.x | ||
| 10.0.x |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for global.json files in the repository
fd -a '^global\.json$' -t f -x cat {}Repository: Altinn/altinn-authentication
Length of output: 54
🏁 Script executed:
# Also check the CodeQL workflow file to understand the full context
cat -n .github/workflows/codeql-analysis.yml | head -50Repository: Altinn/altinn-authentication
Length of output: 1909
🌐 Web query:
CodeQL autobuild .NET 10 support compatibility
💡 Result:
CodeQL’s C#/.NET extractor (and therefore autobuild for C#) is not officially listed as supporting .NET 10 yet. The current “Supported languages and frameworks” table lists .NET 5 through .NET 9 (plus .NET Framework / .NET Core versions), but does not include .NET 10. [1]
In practice, this shows up as autobuild/finalize failures for some .NET 10 setups (for example, a reported failure with .NET 10 RC2 on macOS/Linux where CodeQL reports it “didn’t build any” C# code). [2]
What to do:
- If you need reliability today, build with a supported SDK (e.g., .NET 9 or .NET 8) for the CodeQL job (often by pinning via
global.json). [1] - Or switch to manual build in the CodeQL workflow and run explicit
dotnet restore/build/teststeps betweeninitandanalyze(recommended whenautobuilddoesn’t correctly detect/build your project). [3]
Sources: [1] [2] [3]
CodeQL does not support .NET 10—downgrade SDK or use manual build.
.NET 10 is not officially supported by CodeQL's C#/.NET extractor (supported versions: .NET 5–9). Using dotnet-version: 10.0.x will cause the autobuild step to fail with "didn't build any" C# code errors.
Either:
- Change line 37 to
10.0.x→9.0.x(or8.0.x) to use a supported SDK version, or - Replace the autobuild step with explicit
dotnet restoreanddotnet buildcommands betweeninitandanalyze
There is no global.json in the repository to override the workflow setting.
🤖 Prompt for AI Agents
In @.github/workflows/codeql-analysis.yml around lines 33 - 37, The workflow is
setting actions/setup-dotnet with dotnet-version: 10.0.x which CodeQL's C#/.NET
extractor doesn't support; fix by either changing the dotnet-version value used
by the actions/setup-dotnet step to a supported SDK like 9.0.x or 8.0.x, or
remove/disable the autobuild step and add explicit build commands (dotnet
restore and dotnet build) between the CodeQL init and analyze steps so the
analyzer sees a proper build; update the actions/setup-dotnet usage or the
autobuild/init/analyze sequence accordingly to ensure CodeQL can build the C#
code.
|
1 participant
Description
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.