Revert "Update Microsoft.OpenApi to version 2 (#1766)"#1785
Merged
Conversation
This reverts commit d0ed567.
HauklandJ
approved these changes
May 29, 2026
martinothamar
approved these changes
May 29, 2026
ivarne
added
the
feature
Contributor
There was a problem hiding this comment.
Pull request overview
Reverts the previous Microsoft.OpenApi v2 upgrade by downgrading OpenAPI/Swashbuckle dependencies and adjusting OpenAPI generation + tests to match the older Microsoft.OpenApi APIs and serialization behavior.
Changes:
- Downgrade
Microsoft.OpenApi(andSwashbuckle.AspNetCore) and addMicrosoft.OpenApi.Readerswhere needed. - Update OpenAPI document generation/serialization to use the v1.x reader/serializer APIs.
- Update OpenAPI snapshot tests and public API baselines to match the reverted dependency behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Packages.props | Downgrades Microsoft.OpenApi and Swashbuckle versions; adds Microsoft.OpenApi.Readers version. |
| src/Altinn.App.Api/Controllers/CustomOpenApiController.cs | Refactors OpenAPI document construction/serialization to work with Microsoft.OpenApi v1.x. |
| src/Altinn.App.Api/Extensions/SwaggerFilterExtensions.cs | Updates OpenAPI namespace import to Microsoft.OpenApi.Models. |
| test/Altinn.App.Api.Tests/Altinn.App.Api.Tests.csproj | Adds Microsoft.OpenApi.Readers to support parsing OpenAPI docs in tests. |
| test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.cs | Updates OpenAPI parsing/serialization to v1.x APIs and aligns output with controller settings. |
| test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveCustomOpenApiSpec.verified.json | Updates verified OpenAPI output snapshot for reverted libraries. |
| test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveJsonSwagger.verified.json | Updates verified OpenAPI output snapshot for reverted libraries. |
| test/Altinn.App.Api.Tests/Program.cs | Updates OpenAPI namespace import to Microsoft.OpenApi.Models. |
| test/Altinn.App.Api.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt | Updates public API baseline to reflect current build output. |
| test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/config/applicationmetadata.json | Adjusts test app metadata used by OpenAPI generation tests. |
| test/Altinn.App.Analyzers.Tests/testapp/App/Program.cs | Updates OpenAPI namespace import to Microsoft.OpenApi.Models. |
| test/Altinn.App.Integration.Tests/_testapps/basic/App/Program.cs | Updates OpenAPI namespace import to Microsoft.OpenApi.Models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
📝 WalkthroughWalkthroughThis PR downgrades ChangesOpenAPI Model Version Downgrade and API Adaptation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Altinn.App.Api/Controllers/CustomOpenApiController.cs (1)
283-289:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a valid
Encoding.ContentTypevalue (comma-separated MIME types).
CustomOpenApiControllerbuildsOpenApiEncoding.ContentTypewithstring.Join(' ', contentTypes), producing values like"image/png image/jpeg". OpenAPI 3.0 expects a single media type or a comma-separated list (e.g.,"image/png, image/jpeg"), so multipart encoding becomes malformed when multiple content types are allowed.Suggested fix
- ? string.Join(' ', contentTypes) + ? string.Join(", ", contentTypes)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Altinn.App.Api/Controllers/CustomOpenApiController.cs` around lines 283 - 289, CustomOpenApiController currently builds OpenApiEncoding.ContentType using string.Join(' ', contentTypes) which yields space-separated MIME types like "image/png image/jpeg" and is invalid; change the construction in the multipartMediaType.Encoding.Add call (referencing dataType.Id, OpenApiEncoding.ContentType and dataType.AllowedContentTypes) to join allowed content types with a comma separator (e.g., string.Join(", ", contentTypes)) and still fall back to "application/octet-stream" when AllowedContentTypes is empty so ContentType is a valid comma-separated list or a single media type.
🧹 Nitpick comments (1)
test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.cs (1)
43-50: 💤 Low valueConsider enabling diagnostic error assertion or removing the commented line.
The commented-out
Assert.Empty(diagnostic.Errors)line could hide parsing issues during test runs. If parsing errors are expected due to library differences, document why; otherwise, consider uncommenting to catch regressions.Suggested change
var reader = new OpenApiStreamReader(); OpenApiDocument document = reader.Read(stream, out OpenApiDiagnostic diagnostic); - // Assert.Empty(diagnostic.Errors); + Assert.Empty(diagnostic.Errors); document.Info.Version = "";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.cs` around lines 43 - 50, Uncomment or remove the commented Assert.Empty(diagnostic.Errors) in the OpenApi parsing test so parsing issues are not silently ignored: in the OpenApiSpecChangeDetection test, after using OpenApiStreamReader and obtaining OpenApiDocument and the diagnostic object, either restore Assert.Empty(diagnostic.Errors) to fail on unexpected parse errors or replace it with a clear assertion/guard that documents expected errors (e.g., Assert.Equal(expectedCount, diagnostic.Errors.Count) or a comment explaining why errors are allowed) before proceeding to document.Info.Version = "" and calling VerifyJson(document.Serialize(CustomOpenApiController.SpecVersion, CustomOpenApiController.SpecFormat), _verifySettings).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Altinn.App.Api/Controllers/CustomOpenApiController.cs`:
- Line 1284: The types Snippets and SchemaPostVisitor are declared public but
are internal implementation helpers; change their accessibility from public to
internal to avoid expanding the assembly API surface. Locate the type
declarations named Snippets (the static helper class) and SchemaPostVisitor (the
visitor class) and replace the public modifier with internal for each, ensuring
any usages within the same file/assembly still compile and no external code
relies on them.
- Around line 1201-1206: The OpenAPI schema for the application/octet-stream
media type currently sets only Format = "binary" which must be paired with Type
= "string"; locate the OpenApiMediaType initialization where Schema = new
OpenApiSchema() { Format = "binary" } in CustomOpenApiController (the
application/octet-stream block) and add Type = "string" to that OpenApiSchema
instance so the schema becomes explicitly type string with format binary.
- Around line 328-333: Remove the bogus content entry that adds ["empty"] = new
OpenApiMediaType(...) in CustomOpenApiController (the OpenAPI request body
builder code), since RequestBody.Required = false already marks the body
optional and "empty" is not a valid media type and schema.type = "null" is
unsupported; delete that dictionary entry (or omit adding any content when there
are no real media types) so only valid media type keys and schemas are emitted
for the RequestBody.
In
`@test/Altinn.App.Api.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt`:
- Around line 597-601: The SchemaPostVisitor type is declared public and
unsealed; change its declaration to internal sealed to follow project
guidelines. Update the class modifier on SchemaPostVisitor (and keep the
existing parameterless constructor and Visit override intact) so the type is
internal sealed SchemaPostVisitor, ensuring no external inheritance or public
exposure.
- Around line 623-637: The Snippets helper type is declared public but should be
internal per project guidelines; change the declaration of the Snippets type
from public static class Snippets to internal static class Snippets and update
any internal references (e.g., AltinnTokenSecurityScheme,
ProblemDetailsResponseReference, CommonParameters, DataGuidParameterReference,
InstanceGuidParameterReference, InstanceOwnerPartyIdParameterReference,
InstanceWriteSchema, LanguageParameterReference, PatchSchema,
ProblemDetailsResponseSchema, AddCommonErrorResponses) so compilation and tests
still pass; ensure no external assemblies rely on Snippets before making it
internal.
---
Outside diff comments:
In `@src/Altinn.App.Api/Controllers/CustomOpenApiController.cs`:
- Around line 283-289: CustomOpenApiController currently builds
OpenApiEncoding.ContentType using string.Join(' ', contentTypes) which yields
space-separated MIME types like "image/png image/jpeg" and is invalid; change
the construction in the multipartMediaType.Encoding.Add call (referencing
dataType.Id, OpenApiEncoding.ContentType and dataType.AllowedContentTypes) to
join allowed content types with a comma separator (e.g., string.Join(", ",
contentTypes)) and still fall back to "application/octet-stream" when
AllowedContentTypes is empty so ContentType is a valid comma-separated list or a
single media type.
---
Nitpick comments:
In `@test/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.cs`:
- Around line 43-50: Uncomment or remove the commented
Assert.Empty(diagnostic.Errors) in the OpenApi parsing test so parsing issues
are not silently ignored: in the OpenApiSpecChangeDetection test, after using
OpenApiStreamReader and obtaining OpenApiDocument and the diagnostic object,
either restore Assert.Empty(diagnostic.Errors) to fail on unexpected parse
errors or replace it with a clear assertion/guard that documents expected errors
(e.g., Assert.Equal(expectedCount, diagnostic.Errors.Count) or a comment
explaining why errors are allowed) before proceeding to document.Info.Version =
"" and calling
VerifyJson(document.Serialize(CustomOpenApiController.SpecVersion,
CustomOpenApiController.SpecFormat), _verifySettings).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0734924-2493-437b-83ee-1e901d0d85fc
📒 Files selected for processing (12)
Directory.Packages.propssrc/Altinn.App.Api/Controllers/CustomOpenApiController.cssrc/Altinn.App.Api/Extensions/SwaggerFilterExtensions.cstest/Altinn.App.Analyzers.Tests/testapp/App/Program.cstest/Altinn.App.Api.Tests/Altinn.App.Api.Tests.csprojtest/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/config/applicationmetadata.jsontest/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveCustomOpenApiSpec.verified.jsontest/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.SaveJsonSwagger.verified.jsontest/Altinn.App.Api.Tests/OpenApi/OpenApiSpecChangeDetection.cstest/Altinn.App.Api.Tests/Program.cstest/Altinn.App.Api.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txttest/Altinn.App.Integration.Tests/_testapps/basic/App/Program.cs
💤 Files with no reviewable changes (1)
- test/Altinn.App.Api.Tests/Data/apps/tdd/contributer-restriction/config/applicationmetadata.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit d0ed567.
It requires changes to
Program.cswith makes updating apps harderSummary by CodeRabbit
New Features
Bug Fixes
Chores