Skip to content

Enable nullable#254

Merged
damianh merged 17 commits intomainfrom
dh/intro-2-nullable
Sep 22, 2025
Merged

Enable nullable#254
damianh merged 17 commits intomainfrom
dh/intro-2-nullable

Conversation

@damianh
Copy link
Member

@damianh damianh commented Sep 18, 2025

Enable nullable in OAuth2Introspection.

@damianh damianh requested a review from bhazen September 18, 2025 16:34
@damianh damianh self-assigned this Sep 18, 2025
Copilot AI review requested due to automatic review settings September 18, 2025 16:34
@damianh damianh marked this pull request as draft September 18, 2025 16:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables nullable reference types for the OAuth2 introspection library by removing the <Nullable>disable</Nullable> setting and updating code to handle nullable references correctly.

Key changes include:

  • Removing nullable disable directives from project files
  • Adding null-forgiving operators and nullable annotations throughout the codebase
  • Replacing custom JSON serialization with System.Text.Json
  • Removing validation logic that checked for null TokenRetriever

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
SimpleJson.cs Completely removed custom JSON serialization library
PipelineFactory.cs Added nullable annotations and switched to System.Text.Json
MockHttpRequest.cs Added null-forgiving operators and nullable property types
IntrospectionEndpointHandler.cs Added nullable annotations and improved null handling
Introspection.cs Added null-forgiving operators for test assertions
Configuration.cs Removed test for null TokenRetriever validation and added nullable annotation
AssemlyInfo.cs Added new assembly info file with console capture attribute
OAuth2IntrospectionOptions.cs Made ClientId and ClientSecret nullable, removed TokenRetriever validation
Project files Removed nullable disable directives

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@damianh damianh force-pushed the dh/intro-2-nullable branch 4 times, most recently from 7ebed11 to 45456d1 Compare September 21, 2025 09:07
@damianh damianh added the impact/breaking The fix or change will be a breaking one label Sep 21, 2025
@damianh damianh requested review from a team and Copilot and removed request for bhazen September 21, 2025 09:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

introspection/test/AspNetCore.Authentication.OAuth2Introspection.Tests/Util/IntrospectionEndpointHandler.cs:1

  • The clientAssertion variable is created before checking if it's needed. This creates an unnecessary object when options.ClientSecret is not null. Consider moving the creation inside the if block or after the time check.
// Copyright (c) Duende Software. All rights reserved.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@damianh damianh marked this pull request as ready for review September 21, 2025 09:11
@damianh damianh enabled auto-merge September 21, 2025 09:11
@damianh damianh force-pushed the dh/intro-2-nullable branch from fccf6e8 to cbbc499 Compare September 21, 2025 13:29
@damianh damianh merged commit d557351 into main Sep 22, 2025
5 checks passed
@damianh damianh deleted the dh/intro-2-nullable branch September 22, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/foss/introspection impact/breaking The fix or change will be a breaking one

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants