Skip to content

Add nullability support for server parameter delegation#5486

Merged
bjhham merged 4 commits into
mainfrom
nomisrev/nullable-parameter-delegation
May 12, 2026
Merged

Add nullability support for server parameter delegation#5486
bjhham merged 4 commits into
mainfrom
nomisrev/nullable-parameter-delegation

Conversation

@nomisRev
Copy link
Copy Markdown
Contributor

Subsystem
Server, core

Motivation
Currently it is not possible to use nullable values with parameter delegation.

get("/nullable") {
    val nameOrNull: String? by call.queryParameters
    val ageOrNull: Int? by parameters
}

Solution

This PR add support by removing the : Any constraint and relying on typeInfo.kotlinType?.isMarkedNullable to check if null is allowed or not.

@nomisRev nomisRev requested review from bjhham, e5l and osipxd March 20, 2026 09:37
@nomisRev
Copy link
Copy Markdown
Contributor Author

I'm a bit unsure how reliable typeInfo.kotlinType?.isMarkedNullable is across platforms?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 20, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4e462fcb-6d0d-4ccf-bdf8-891352dfeb5f

📥 Commits

Reviewing files that changed from the base of the PR and between 2973653 and 65a6d88.

📒 Files selected for processing (1)
  • ktor-server/ktor-server-core/api/ktor-server-core.klib.api
🚧 Files skipped from review as they are similar to previous changes (1)
  • ktor-server/ktor-server-core/api/ktor-server-core.klib.api

📝 Walkthrough

Walkthrough

Generic type constraints for Parameters delegated accessors were relaxed to allow nullable result types; implementation now returns null for absent parameters when the target type is nullable and preserves existing exceptions for non-nullable types and conversion failures.

Changes

Parameters nullable handling

Layer / File(s) Summary
Core implementation change
ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt
KDoc and operator getValue changed to reified R (no : Any bound); getOrFail and internal getOrFailImpl generalized similarly and now return null as R when R is nullable and the parameter is absent; otherwise previous exception behavior remains.
Exported API signatures
ktor-server/ktor-server-core/api/ktor-server-core.klib.api
Exported signatures updated: getOrFailImpl and getValue bounds relaxed from A: Any to A: Any? to reflect nullable-capable generics.
Tests
ktor-server/ktor-server-core/common/test/io/ktor/server/http/ParametersTest.kt
Added tests for nullable delegated parameters resolving to value or null, conversion failure for nullable target, and missing non-null delegated value throwing MissingRequestParameterException.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • marychatte
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding nullability support for server parameter delegation, which is the primary objective of this PR.
Description check ✅ Passed The description includes all required sections: Subsystem, Motivation, and Solution. It clearly explains the problem and the approach taken.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nomisrev/nullable-parameter-delegation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@bjhham bjhham left a comment

Choose a reason for hiding this comment

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

Looks like an easy win 👍

I'm pretty sure isMarkedNullable works cross-platform on reified KTypes.

I don't know why we didn't just implement it that way in the first place...

@bjhham
Copy link
Copy Markdown
Contributor

bjhham commented Mar 20, 2026

FYI logged an issue to handle property delegation in the compiler plugin:
KTOR-9422 OpenAPI code inference misses property delegation

Comment thread ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt Outdated
Comment thread ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt Outdated
Copy link
Copy Markdown
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

lgtm

@nomisRev
Copy link
Copy Markdown
Contributor Author

nomisRev commented Apr 1, 2026

@osipxd I updated the PR to move the nullability check in the actual implementation. This way we now also support nullable types for getOrFail.

TypeInfo is always being created because it's used by the underlying conversions mechanism. Perhaps the conversions mechanism can be optimised to avoid TypeInfo where possible but I left that for outside of this scope since it seems unrelated to my changes.

Could you review again? Thank you for the careful review and pointing it out! 🙏 We're still getting a better outcome thanks to it!

Comment thread ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt Outdated
@bjhham bjhham force-pushed the nomisrev/nullable-parameter-delegation branch from 1056631 to 2973653 Compare May 12, 2026 11:48
@bjhham bjhham requested a review from osipxd May 12, 2026 11:48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt`:
- Around line 53-54: Update the KDoc for getOrFail<R> to reflect its new
nullable-missing behavior: state that when R is non-nullable a missing parameter
throws MissingRequestParameterException, but when R is a nullable type the
function returns null for missing values; also keep the existing note that
ParameterConversionException is thrown when conversion from String to R fails.
Modify the documentation on the getOrFail<R> declaration to mention the
conditional behavior based on R's nullability and include both exception and
null-return cases for clarity.
🪄 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: bb869399-a278-451d-b6c4-7876eda331cb

📥 Commits

Reviewing files that changed from the base of the PR and between 1056631 and 2973653.

📒 Files selected for processing (3)
  • ktor-server/ktor-server-core/api/ktor-server-core.klib.api
  • ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt
  • ktor-server/ktor-server-core/common/test/io/ktor/server/http/ParametersTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • ktor-server/ktor-server-core/common/test/io/ktor/server/http/ParametersTest.kt
  • ktor-server/ktor-server-core/api/ktor-server-core.klib.api

Comment on lines 53 to 54
* @throws MissingRequestParameterException if no values associated with this [name]
* @throws ParameterConversionException when conversion from String to [R] fails
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update generic getOrFail KDoc to include nullable-missing behavior.

getOrFail<R> now returns null for missing values when R is nullable, but this KDoc still says missing always throws. Please align docs with the new contract.

As per coding guidelines: "Keep KDoc documentation correct when behavior/signatures change".

🤖 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 `@ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt`
around lines 53 - 54, Update the KDoc for getOrFail<R> to reflect its new
nullable-missing behavior: state that when R is non-nullable a missing parameter
throws MissingRequestParameterException, but when R is a nullable type the
function returns null for missing values; also keep the existing note that
ParameterConversionException is thrown when conversion from String to R fails.
Modify the documentation on the getOrFail<R> declaration to mention the
conditional behavior based on R's nullability and include both exception and
null-return cases for clarity.

@bjhham bjhham enabled auto-merge (squash) May 12, 2026 17:31
@bjhham bjhham merged commit e45fc3d into main May 12, 2026
17 checks passed
@bjhham bjhham deleted the nomisrev/nullable-parameter-delegation branch May 12, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants