Skip to content

Add nullability support for server parameter delegation#5486

Open
nomisRev wants to merge 1 commit intomainfrom
nomisrev/nullable-parameter-delegation
Open

Add nullability support for server parameter delegation#5486
nomisRev wants to merge 1 commit intomainfrom
nomisrev/nullable-parameter-delegation

Conversation

@nomisRev
Copy link
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
Contributor Author

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

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: d828920f-49da-4f84-97ed-67412516833f

📥 Commits

Reviewing files that changed from the base of the PR and between 5710ef9 and 86d7c3f.

📒 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

📝 Walkthrough

Walkthrough

The changes generalize the Parameters.getValue operator and internal getOrFailImpl helper to support nullable type parameters, enabling delegated properties to resolve to null instead of throwing exceptions when the parameter is absent and the target type is nullable.

Changes

Cohort / File(s) Summary
API Signatures
ktor-server/ktor-server-core/api/ktor-server-core.klib.api
Updated type parameter bounds for getOrFailImpl and getValue from A: Any to A: Any?, removing the non-nullability constraint to support nullable delegated properties.
Core Implementation
ktor-server/ktor-server-core/common/src/io/ktor/server/util/Parameters.kt
Modified getValue operator to remove the Any bound on generic R and added logic to detect nullable types via typeInfo<R>(), returning null for absent parameters when R is nullable. Updated internal getOrFailImpl helper to match by removing the Any constraint. Updated KDoc to reflect nullable return behavior.
Test Coverage
ktor-server/ktor-server-core/common/test/io/ktor/server/http/ParametersTest.kt
Added three new test cases verifying nullable delegated property resolution (Int?, String? returning value or null), nullable type conversion exceptions, and missing non-null delegated value exceptions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding nullability support for parameter delegation in the Ktor server.
Description check ✅ Passed The description follows the template with all required sections (Subsystem, Motivation, Solution) properly filled with clear, concrete information about the problem and implementation.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nomisrev/nullable-parameter-delegation
📝 Coding Plan
  • Generate coding plan for human review comments

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
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
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

return getOrFail<R>(property.name)
public inline operator fun <reified R> Parameters.getValue(thisRef: Any?, property: KProperty<*>): R {
val typeInfo = typeInfo<R>()
return if (typeInfo.kotlinType?.isMarkedNullable == true && get(property.name) == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively we can use null is R if we want to avoid creating TypeInfo instance

Copy link
Member

Choose a reason for hiding this comment

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

This would probably allow us to keep getOrFailImpl signature untouched because of smart cast. But I'm not sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I forgot about null is R in reified generics. Seems like a much better solution here.

* [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.util.getValue)
*
* @throws MissingRequestParameterException if no values associated with name
* If [R] is nullable and no values are associated with the delegated property name, returns `null`.
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this above [Report a problem] link

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.

3 participants