Skip to content

Add typed attribute converters for session, request, flash and servletContext#15715

Open
codeconsole wants to merge 4 commits into
apache:8.0.xfrom
codeconsole:feat/typeconverting-servlet-attributes-internal
Open

Add typed attribute converters for session, request, flash and servletContext#15715
codeconsole wants to merge 4 commits into
apache:8.0.xfrom
codeconsole:feat/typeconverting-servlet-attributes-internal

Conversation

@codeconsole
Copy link
Copy Markdown
Contributor

Summary

Adds null-safe, typed conversion methods to the session, request, flash and servletContext objects, mirroring the existing params.int('id', 1) API. Also adds a missing string/getString converter to TypeConvertingMap itself.

Integer page    = request.int('page', 1)
String  tz      = session.string('userTimeZoneId', 'America/Los_Angeles')
Boolean active  = session.boolean('active', false)
String  notice  = flash.string('notice')
int     uploads = servletContext.int('maxUploads', 10)

The full converter set is available — string, byte, char, short, int, long, double, float, boolean, list, date — each with an optional default-value overload, exactly as on params.

Motivation

Under @CompileStatic / @GrailsCompileStatic, dynamic attribute access such as session.userTimeZoneId does not compile, and the explicit form requires a cast at every call site:

String tz = (session.getAttribute('userTimeZoneId') as String) ?: 'America/Los_Angeles'

These converters give the same null-safe, typed, default-aware ergonomics as params, and they compile under static compilation. They are equally useful in dynamic code for the parsing/defaulting behaviour (just like params.int(...)).

Design

  • Conversion logic is extracted into an internal helper, org.grails.util.TypeConverters, which converts a raw Object value to the target type (with an optional default). Both TypeConvertingMap's instance getX methods and the new attribute extensions delegate to it, so the logic lives in a single place and no new conversion methods are added to the public TypeConvertingMap API.
  • The extensions call the converters directly on the attribute value, avoiding any per-access allocation.
  • The instance getX(name, default) semantics are preserved, so params behaviour is unchanged (verified by GrailsParameterMapBindingSpec).
  • The one deliberate exception is the boolean default: "is the value present" differs by holder (map containsKey vs. attribute set), which cannot be expressed from a value alone, so it stays per-site and is documented inline.
  • The string converter returns the first element when the attribute is an array, mirroring the single-value semantics of the other converters; use list for all values.

Tests & docs

  • Unit tests for the internal converters (incl. default-value coercion) and for getString/string.
  • A Spock spec per holder (HttpSessionExtensionSpec, HttpServletRequestExtensionSpec, ServletContextExtensionSpec, FlashScopeExtensionSpec) covering conversion, null-safety, defaults, and a @CompileStatic static-resolution guard.
  • GrailsParameterMapBindingSpec passes unchanged (regression guard for the core refactor).
  • New "Type Conversion of Attributes" section in the controllers guide.

@matrei
Copy link
Copy Markdown
Contributor

matrei commented Jun 5, 2026

@codeconsole Can this target 8?
Decided in the weekly meeting May 27th, point 1, 7.x.x is not accepting new features.
Focus should now be on 8+.

@codeconsole codeconsole changed the base branch from 7.2.x to 8.0.x June 5, 2026 06:15
@codeconsole
Copy link
Copy Markdown
Contributor Author

@codeconsole Can this target 8? Decided in the weekly meeting May 27th, point 1, 7.x.x is not accepting new features. Focus should now be on 8+.

@matrei done, how about a review ;P

Comment thread grails-web-core/src/main/groovy/org/grails/web/servlet/FlashScopeExtension.groovy Outdated
@codeconsole codeconsole force-pushed the feat/typeconverting-servlet-attributes-internal branch from 4f61d56 to e5df31d Compare June 5, 2026 16:39
…, flash and servletContext

Extract the value-level conversion logic into an internal helper class
`org.grails.util.TypeConverters` (toByte/toInteger/toLong/toBoolean/
toStringValue/toDate/toList, etc., each with a default-value overload), have the
existing `TypeConvertingMap` instance `getX` methods delegate to it, and add a
`string`/`getString` converter.

Expose the full set of converters (`string`, `byte`, `char`, `short`, `int`,
`long`, `double`, `float`, `boolean`, `list`, `date`) directly on `HttpSession`,
`HttpServletRequest`, `ServletContext` and the `flash` scope (`FlashScope`) by
calling the internal converters on the attribute value, so attribute access is
statically compilable and type-safe under `@CompileStatic`/`@GrailsCompileStatic`,
e.g.:

    session.int('age', 21)
    session.string('tz', 'America/Los_Angeles')
    flash.string('notice')

Both the conversion logic and the default-value handling live in the single
internal `TypeConverters` class, shared by the map and the extensions, with no
new conversion methods added to the public `TypeConvertingMap` API. Calling the
internal converters directly avoids any per-access allocation. The only
exception is the boolean default, whose "is the value present" rule differs by
holder (map containsKey vs attribute set) and so stays per-site by design.
Includes tests covering conversion, null-safety, defaults and static resolution,
plus documentation.
@codeconsole codeconsole force-pushed the feat/typeconverting-servlet-attributes-internal branch from e5df31d to 0ecfc36 Compare June 5, 2026 16:44
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 63.47826% with 84 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.3575%. Comparing base (41f9fb9) to head (60980ef).

Files with missing lines Patch % Lines
.../grails/web/servlet/ServletContextExtension.groovy 16.6667% 20 Missing ⚠️
...ache/grails/core/internal/util/TypeConverters.java 84.8214% 7 Missing and 10 partials ⚠️
...ils/web/servlet/HttpServletRequestExtension.groovy 29.1667% 16 Missing and 1 partial ⚠️
.../org/grails/web/servlet/FlashScopeExtension.groovy 30.4348% 15 Missing and 1 partial ⚠️
...org/grails/web/servlet/HttpSessionExtension.groovy 41.6667% 13 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.0.x     #15715        +/-   ##
==================================================
+ Coverage     48.3483%   48.3575%   +0.0092%     
- Complexity      15104      15181        +77     
==================================================
  Files            1870       1872         +2     
  Lines           85459      85571       +112     
  Branches        14901      14899         -2     
==================================================
+ Hits            41318      41380        +62     
- Misses          37805      37851        +46     
- Partials         6336       6340         +4     
Files with missing lines Coverage Δ
.../groovy/grails/util/AbstractTypeConvertingMap.java 81.2500% <100.0000%> (+3.2839%) ⬆️
...c/main/groovy/grails/util/TypeConvertingMap.groovy 87.5000% <100.0000%> (+1.1364%) ⬆️
...org/grails/web/servlet/HttpSessionExtension.groovy 44.8276% <41.6667%> (-15.1724%) ⬇️
.../org/grails/web/servlet/FlashScopeExtension.groovy 30.4348% <30.4348%> (ø)
...ache/grails/core/internal/util/TypeConverters.java 84.8214% <84.8214%> (ø)
...ils/web/servlet/HttpServletRequestExtension.groovy 15.1163% <29.1667%> (+5.4389%) ⬆️
.../grails/web/servlet/ServletContextExtension.groovy 15.3846% <16.6667%> (+15.3846%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Guard SimpleDateFormat parsing in toDate against a null toString() (which
throws NullPointerException, not the caught ParseException) and name all
empty catch variables 'ignored'. Adds test coverage for the null toString()
edge case across every converter.
The class-level @SuppressWarnings({"rawtypes","unchecked"}) was copied from
AbstractTypeConvertingMap, where it covers raw Map usage. In TypeConverters the
only raw-type usage was toList; genericize it to return List<Object> using a
typed ArrayList copy so the suppression is no longer needed, and remove it.
Relocate the internal helper out of the legacy org.grails.util package into the
project's org.apache.grails namespace: .core for gradle-project uniqueness,
.internal to mark it as non-public (mirroring the old grails.* reservation),
and .util since it was extracted from util-package code. Update all imports in
AbstractTypeConvertingMap, the servlet extensions and the test.
@codeconsole
Copy link
Copy Markdown
Contributor Author

codeconsole commented Jun 5, 2026

@sbglasius @jdaugherty @matrei annoying that I got bonus changes from other peoples work when I switched the base to 8.0.x, but I replayed everything on top of a fresh 8.0.x then force pushed this PR. Should be clean now.

@testlens-app
Copy link
Copy Markdown

testlens-app Bot commented Jun 5, 2026

✅ All tests passed ✅

⚠️ TestLens detected flakiness ⚠️

Test Summary

Check Project/Task Test Runs
CI / Functional Tests (Java 21, indy=false) :grails-test-examples-scaffolding:integrationTest UserControllerSpec > User list ❌ ✅ 🚫 🚫

🏷️ Commit: 60980ef
▶️ Tests: 9385 executed
⚪️ Checks: 43/43 completed


Learn more about TestLens at testlens.app.

@codeconsole codeconsole requested a review from jdaugherty June 6, 2026 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants