refactor(grails-data-graphql): clean up the types package by borinquenkid · Pull Request #16204 · apache/grails-core

borinquenkid · 2026-08-23T16:05:37Z

Summary

Stacked on #16201. Fixes a series of IntelliJ-flagged issues in org.grails.gorm.graphql.types.*:

DateCoercion: fixed an NPE when no date formats are configured.
AbstractObjectTypeBuilder: fixed a control-flow issue where build()'s declared-then-reassigned objectType local confused Groovy's static type checker on objectTypeCache.put(...); fixed an unrelated latent bug where an ArrayList capacity was computed from a not-yet-declared properties variable (which actually resolved to the unrelated GroovyObject#getProperties() bean map); removed an unused parameter from buildInterfaceType; and migrated the deprecated GraphQLInterfaceType.Builder#typeResolver call to registration via GraphQLCodeRegistry.Builder. Added the first direct coverage of the interface-type branch (a root entity with child entities), which no other spec reached.
All Coercing implementations in types.scalars.coercing: migrated off the deprecated 1-arg serialize/parseValue/parseLiteral overrides to the context/locale-aware overloads, fixing a ClassCastException-in-waiting in ByteArrayCoercion#parseLiteral, a deprecated Character constructor usage in CharacterArrayCoercion, and an unassigned-variable branch in InstantCoercion#parseLiteral along the way.

Test plan

./gradlew :grails-data-graphql-core:test :grails-data-graphql:test :grails-data-graphql-core:codeStyle :grails-data-graphql:codeStyle passes, including SchemaSpec which exercises these coercions through real GraphQL query execution

🤖 Generated with Claude Code

parseDate() returned a bare null instead of Optional.empty() when formats was empty, which caused an NPE from parseLiteral()'s .orElse(null) call on the null return value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tTypeBuilder - build(entity) declared a blank GraphQLOutputType reassigned across if/else branches then passed to objectTypeCache.put(...) - Groovy's static type checker narrowed its inferred type to an intersection of GraphQLInterfaceType/GraphQLObjectType's common interfaces rather than the declared GraphQLOutputType, so put() no longer type-checked. Rewrote to return the cached value directly and assign objectType via a ternary instead of branch reassignment, the same treatment given to DefaultGormDataFetcher.resolveDatastore() earlier in this cleanup. - Fixed an unrelated latent bug found while touching build(): fields' ArrayList capacity was computed from `properties.size()` one line before `properties` was declared, which under CompileStatic actually resolved to the GroovyObject-inherited `getProperties()` bean-property map (an unrelated count), not the domain property list. Moved the properties declaration first. - buildInterfaceType(entity, name, description, fields) never used its entity parameter; dropped it (no overrides exist, undocumented). - GraphQLInterfaceType.Builder#typeResolver(TypeResolver) is deprecated since 2018-12-03 in favor of registering the resolver on GraphQLCodeRegistry.Builder by interface name, same migration already applied to CustomOperation/Schema's dataFetcher registration earlier in this cleanup. Added AbstractObjectTypeBuilderSpec, the first coverage of build()'s interface-type branch (a root entity with child entities) - the other object type builder specs construct their builder with a null GraphQLTypeManager and can't reach build() at all. Backs it with a minimal Animal/Dog GORM inheritance fixture and verifies the interface type, the child's implements-interface relationship, and that the migrated codeRegistry type resolver is actually registered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

graphql-java deprecated Coercing's 1-arg serialize(Object)/parseValue(Object)/ parseLiteral(Object) since 2022-08-22 in favor of context/locale-aware overloads: serialize(Object, GraphQLContext, Locale), parseValue(Object, GraphQLContext, Locale), and parseLiteral(Value<?>, CoercedVariables, GraphQLContext, Locale). Every Coercing implementation in this package overrode the old methods. Migrated all of them - ByteArrayCoercion, CharacterArrayCoercion, CurrencyCoercion, DateCoercion, SqlDateCoercion, TimeCoercion, TimeZoneCoercion, TimestampCoercion, URICoercion, URLCoercion, UUIDCoercion, Jsr310Coercion (the base class LocalDate/LocalDateTime/ LocalTime/OffsetDateTime/OffsetTime/ZonedDateTime all extend, so they needed no direct changes), and InstantCoercion - to the new signatures. Fixed three real bugs surfaced while touching these classes: - ByteArrayCoercion#parseLiteral cast a List<Byte>#toArray() result (always an Object[] at runtime) directly to Byte[], which throws ClassCastException whenever called. Used toArray(new Byte[0]) instead. - CharacterArrayCoercion used the deprecated new Character(char) constructor in three places; switched to Character.valueOf(char). - InstantCoercion#parseLiteral declared a blank Object value reassigned across an if/else-if with no else branch, so an unrecognized Value subtype fell through with value never assigned. Rewrote each branch to return directly instead, the same treatment given elsewhere in this cleanup, and dropped the now-redundant outer `instanceof Value` check since IntValue/StringValue already narrow to Value subtypes. Updated every coercion spec (18 files) to call the new signatures. Removed InstantCoercionSpec's "non Value type returns null" test since that scenario is no longer reachable - the method's static parameter type is now Value<?>, and the still-present "unrecognized Value type" test (BooleanValue) covers the equivalent behavior. Verified against SchemaSpec, which exercises these coercions through real GraphQL query execution, not just direct unit calls. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

testlens-app · 2026-08-23T16:22:23Z

🚨 TestLens detected 1 failed test 🚨

Here is what you can do:

Inspect the test failures carefully.
If you are convinced that some of the tests are flaky, you can mute them below.
Finally, trigger a rerun by checking the rerun checkbox.

Test Summary

CI / Spring Security Tests (Java 25, shard 1) > :grails-test-examples-spring-security-core-misc-functional-test-app-group:integrationTest

Test	Runs	Flakiness
SecuredControllerSpec > test login as watson, watson belongs to detective groups. All detectives have the role ADMIN	❌	1% 🟡

🏷️ Commit: b0f3b14
▶️ Tests: 47665 executed
⚪️ Checks: 70/70 completed

Test Failures

SecuredControllerSpec > test login as watson, watson belongs to detective groups. All detectives have the role ADMIN (:grails-test-examples-spring-security-core-misc-functional-test-app-group:integrationTest in CI / Spring Security Tests (Java 25, shard 1))

Condition not satisfied:

pageSource.contains('you have ROLE_ADMIN')
|          |
|          false
<html lang="en" class="no-js"><head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <title>
        Login
    </title>
    <meta name="viewport" content="width=device-width, initial-scale=1">
    
    <meta name="layout" content="main">
    
 
</head>
<body>
 
<div class="container py-5">
    <div class="row justify-content-center">
        <div class="col-11 col-sm-9 col-md-7 col-lg-5 col-xl-4">
            <div class="card shadow-sm">
                <div class="card-body p-4 p-sm-5">
                    <h1 class="h4 card-title text-center mb-4">Please Login</h1>
 
                    
 
                    <form action="/login/authenticate" method="POST" id="loginForm" autocomplete="off">
                        <div class="mb-3">
                            <label for="username" class="form-label">Username</label>
                            <input type="text" class="form-control" name="username" id="username" autocapitalize="none" autocomplete="username">
                        </div>
 
                        <div class="mb-3">
                            <label for="password" class="form-label">Password</label>
                            <div class="input-group">
                                <input type="password" class="form-control" name="password" id="password" autocomplete="current-password">
                                <button class="btn btn-outline-secondary" type="button" id="passwordToggler" onclick="passwordDisplayToggle()" aria-pressed="false" aria-label="Show or hide password">
                                    <i id="passwordTogglerIcon" class="bi bi-eye" aria-hidden="true"></i>
                                </button>
                            </div>
                        </div>
 
                        <div class="mb-3 form-check">
                            <input type="checkbox" class="form-check-input" name="remember-me" id="remember_me">
                            <label class="form-check-label" for="remember_me">Remember me</label>
                        </div>
 
                        <button type="submit" id="submit" class="btn btn-primary w-100">Login</button>
                    </form>
                </div>
            </div>
        </div>
    </div>
</div>
<script type="text/javascript">
    document.addEventListener("DOMContentLoaded", function () {
        var form = document.forms['loginForm'];
        if (form) {
            form.elements['username'].focus();
        }
    });
 
    function passwordDisplayToggle() {
        var button = document.getElementById("passwordToggler");
        var icon = document.getElementById("passwordTogglerIcon");
        var field = document.getElementById("password");
        var hiding = field.type === "text";
        field.type = hiding ? "password" : "text";
        icon.classList.toggle("bi-eye", hiding);
        icon.classList.toggle("bi-eye-slash", !hiding);
        button.setAttribute("aria-pressed", String(!hiding));
    }
</script>
 
 
 
</body></html>

	at demo.SecuredControllerSpec.$tt__$spock_feature_1_1(SecuredControllerSpec.groovy:70)
	at demo.SecuredControllerSpec.test login as watson, watson belongs to detective groups. All detectives have the role ADMIN_closure3(SecuredControllerSpec.groovy)
	at grails.gorm.transactions.GrailsTransactionTemplate$1.doInTransaction(GrailsTransactionTemplate.groovy:72)
	at org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:137)
	at grails.gorm.transactions.GrailsTransactionTemplate.executeAndRollback(GrailsTransactionTemplate.groovy:69)
	at demo.SecuredControllerSpec.test login as watson, watson belongs to detective groups. All detectives have the role ADMIN(SecuredControllerSpec.groovy)

Rerun Controls

Select tests to mute in this pull request:

SecuredControllerSpec > test login as watson, watson belongs to detective groups. All detectives have the role ADMIN

Reuse successful test results:

♻️ Only rerun the tests that failed or were muted before

Click the checkbox to trigger a rerun:

Rerun jobs

Learn more about TestLens at testlens.app/docs.

codecov · 2026-08-23T16:58:49Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.4199%. Comparing base (36b7716) to head (b0f3b14).

Additional details and impacted files

@@                             Coverage Diff                             @@
##             test/grails-data-graphql-coverage     #16204        +/-   ##
===========================================================================
+ Coverage                              53.4149%   53.4199%   +0.0050%     
- Complexity                               19459      19460         +1     
===========================================================================
  Files                                     2081       2081                
  Lines                                    98993      98993                
  Branches                                 17361      17361                
===========================================================================
+ Hits                                     52877      52882         +5     
+ Misses                                   38566      38562         -4     
+ Partials                                  7550       7549         -1

see 2 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.

borinquenkid and others added 3 commits August 23, 2026 10:22

Fix DateCoercion NPE when no date formats are configured

0dac4ae

parseDate() returned a bare null instead of Optional.empty() when formats was empty, which caused an NPE from parseLiteral()'s .orElse(null) call on the null return value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings August 23, 2026 16:05

github-project-automation Bot added this to Apache Grails Aug 23, 2026

Copilot AI reviewed Aug 23, 2026

borinquenkid mentioned this pull request Aug 23, 2026

test(grails-data-graphql): add missing test coverage across the module #16201

Open

1 task

borinquenkid requested a review from jamesfredley August 23, 2026 16:18

borinquenkid moved this to In Progress in Apache Grails Aug 23, 2026

borinquenkid added this to the grails:8.1.0-M1 milestone Aug 23, 2026

Navigation Menu

Uh oh!

Uh oh!

refactor(grails-data-graphql): clean up the types package - #16204

refactor(grails-data-graphql): clean up the types package#16204
borinquenkid wants to merge 3 commits into
test/grails-data-graphql-coveragefrom
refactor/graphql-types

borinquenkid commented Aug 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

testlens-app Bot commented Aug 23, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

borinquenkid commented Aug 23, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

testlens-app Bot commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚨 TestLens detected 1 failed test 🚨

Test Summary

CI / Spring Security Tests (Java 25, shard 1) > :grails-test-examples-spring-security-core-misc-functional-test-app-group:integrationTest

Test Failures

Rerun Controls

Uh oh!

codecov Bot commented Aug 23, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

testlens-app Bot commented Aug 23, 2026 •

edited

Loading