From 2af11ac8dfedb59d90e3ef7b975aa113484b3ffa Mon Sep 17 00:00:00 2001 From: James Fredley Date: Fri, 17 Jul 2026 14:23:48 -0400 Subject: [PATCH 1/2] Add artefact name contract characterization spec Assisted-by: opencode:gpt-5.6-sol --- .../ArtefactNamePrecomputationSpec.groovy | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy diff --git a/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy b/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy new file mode 100644 index 00000000000..4c383dea6df --- /dev/null +++ b/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy @@ -0,0 +1,213 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.core + +import grails.core.GrailsClass +import spock.lang.Specification +import spock.lang.Unroll + +/** + * Tests for deterministic GrailsClass naming metadata. + */ +class ArtefactNamePrecomputationSpec extends Specification { + + private static final String BASE_PACKAGE = 'org.grails.core' + + @Unroll + void "naming contract for #entry.label is precomputed"() { + given: + GrailsClass grailsClass = grailsClassFor(entry.artifactType, entry.wrapperClass) + + expect: + grailsClass.name == entry.name + grailsClass.shortName == entry.shortName + grailsClass.fullName == entry.fullName + grailsClass.packageName == entry.packageName + grailsClass.propertyName == entry.propertyName + grailsClass.logicalPropertyName == entry.logicalPropertyName + grailsClass.naturalName == entry.naturalName + + where: + entry << [ + [ + label: 'Acronym controller', + artifactType: 'controller', + wrapperClass: HTMLController, + name: 'HTML', + shortName: 'HTMLController', + fullName: "${BASE_PACKAGE}.HTMLController", + packageName: BASE_PACKAGE, + propertyName: 'HTMLController', + logicalPropertyName: 'HTML', + naturalName: 'HTMLC ontroller' + ], + [ + label: 'Camel mixed controller', + artifactType: 'controller', + wrapperClass: PayRollController, + name: 'PayRoll', + shortName: 'PayRollController', + fullName: "${BASE_PACKAGE}.PayRollController", + packageName: BASE_PACKAGE, + propertyName: 'payRollController', + logicalPropertyName: 'payRoll', + naturalName: 'Pay Roll Controller' + ], + [ + label: 'Acronym service', + artifactType: 'service', + wrapperClass: JSONAPIService, + name: 'JSONAPI', + shortName: 'JSONAPIService', + fullName: "${BASE_PACKAGE}.JSONAPIService", + packageName: BASE_PACKAGE, + propertyName: 'JSONAPIService', + logicalPropertyName: 'JSONAPI', + naturalName: 'JSONAPIS ervice' + ], + [ + label: 'Acronym domain', + artifactType: 'domain', + wrapperClass: URLDomain, + name: 'URLDomain', + shortName: 'URLDomain', + fullName: "${BASE_PACKAGE}.URLDomain", + packageName: BASE_PACKAGE, + propertyName: 'URLDomain', + logicalPropertyName: 'URLDomain', + naturalName: 'URLD omain' + ], + [ + label: 'Url mappings artefact', + artifactType: 'urlMappings', + wrapperClass: HomeUrlMappings, + name: 'Home', + shortName: 'HomeUrlMappings', + fullName: "${BASE_PACKAGE}.HomeUrlMappings", + packageName: BASE_PACKAGE, + propertyName: 'homeUrlMappings', + logicalPropertyName: 'home', + naturalName: 'Home Url Mappings' + ] + ] + } + + @Unroll + void "naming accessors are stable on repeated read for #entry.label"() { + given: + GrailsClass grailsClass = grailsClassFor(entry.artifactType, entry.wrapperClass) + + expect: + 10.times { + assert grailsClass.name == entry.name + assert grailsClass.shortName == entry.shortName + assert grailsClass.fullName == entry.fullName + assert grailsClass.packageName == entry.packageName + assert grailsClass.propertyName == entry.propertyName + assert grailsClass.logicalPropertyName == entry.logicalPropertyName + assert grailsClass.naturalName == entry.naturalName + } + + where: + entry << [ + [ + label: 'Acronym controller', + artifactType: 'controller', + wrapperClass: HTMLController, + name: 'HTML', + shortName: 'HTMLController', + fullName: "${BASE_PACKAGE}.HTMLController", + packageName: BASE_PACKAGE, + propertyName: 'HTMLController', + logicalPropertyName: 'HTML', + naturalName: 'HTMLC ontroller' + ], + [ + label: 'Acronym service', + artifactType: 'service', + wrapperClass: JSONAPIService, + name: 'JSONAPI', + shortName: 'JSONAPIService', + fullName: "${BASE_PACKAGE}.JSONAPIService", + packageName: BASE_PACKAGE, + propertyName: 'JSONAPIService', + logicalPropertyName: 'JSONAPI', + naturalName: 'JSONAPIS ervice' + ] + ] + } + + @Unroll + void "naming is stable across wrapper construction for #entry.label"() { + expect: + GrailsClass first = grailsClassFor(entry.artifactType, entry.wrapperClass) + GrailsClass second = grailsClassFor(entry.artifactType, entry.wrapperClass) + first.name == second.name + first.shortName == second.shortName + first.fullName == second.fullName + first.packageName == second.packageName + first.propertyName == second.propertyName + first.logicalPropertyName == second.logicalPropertyName + first.naturalName == second.naturalName + + where: + entry << [ + [ + label: 'Camel mixed controller', + artifactType: 'controller', + wrapperClass: PayRollController + ], + [ + label: 'Url mappings artefact', + artifactType: 'urlMappings', + wrapperClass: HomeUrlMappings + ], + [ + label: 'Acronym domain', + artifactType: 'domain', + wrapperClass: URLDomain + ] + ] + } + + private GrailsClass grailsClassFor(String artifactType, Class wrapperClass) { + switch (artifactType) { + case 'controller': + return new DefaultGrailsControllerClass(wrapperClass) + case 'service': + return new DefaultGrailsServiceClass(wrapperClass) + case 'domain': + return new DefaultGrailsDomainClass(wrapperClass) + case 'urlMappings': + return new DefaultGrailsUrlMappingsClass(wrapperClass) + default: + throw new IllegalArgumentException("Unknown artifact type [${artifactType}]") + } + } +} + +class HTMLController {} + +class PayRollController {} + +class JSONAPIService {} + +class URLDomain {} + +class HomeUrlMappings {} From bfe84593dd2ab8a1808372781fdbae39363ccaf0 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Mon, 27 Jul 2026 12:33:16 -0500 Subject: [PATCH 2/2] Address jdaugherty review feedback on artefact naming characterization spec Copilot's 5 inline comments claiming the acronym naturalName expectations were wrong (e.g. 'HTMLC ontroller') are false positives - verified by running the spec: GrailsNameUtils.getNaturalName genuinely produces those values, and jdaugherty's review already confirmed this. No change needed there. jdaugherty's own feedback was substantive and is addressed here: - Rename ArtefactNamePrecomputationSpec -> ArtefactNamingContractSpec: "precomputed" was aspirational, since nothing in the spec exercises actual precomputation, only naming stability. - Add a comment above the acronym-heavy naturalName assertions explaining the quirky-but-intentional GrailsNameUtils splitting behavior they pin, so a future reader doesn't "fix" the expectations or the algorithm without realizing this spec exists to catch exactly that change. - Add three cases exercising the artefact detection contract (ArtefactHandler#isArtefactClass), which the original spec bypassed entirely by constructing GrailsClass wrappers directly: an abstract controller is rejected (ControllerArtefactHandler's allowAbstract is false), a suffix-matching concrete controller is accepted, and a domain-named class with no @Entity/@Artefact annotation is rejected by DomainClassArtefactHandler - this is the part a naming precomputation refactor is most likely to disturb, and the prior spec gave it no coverage at all. Co-Authored-By: Claude Sonnet 5 --- ...oovy => ArtefactNamingContractSpec.groovy} | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) rename grails-core/src/test/groovy/org/grails/core/{ArtefactNamePrecomputationSpec.groovy => ArtefactNamingContractSpec.groovy} (83%) diff --git a/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy b/grails-core/src/test/groovy/org/grails/core/ArtefactNamingContractSpec.groovy similarity index 83% rename from grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy rename to grails-core/src/test/groovy/org/grails/core/ArtefactNamingContractSpec.groovy index 4c383dea6df..f04c7bae604 100644 --- a/grails-core/src/test/groovy/org/grails/core/ArtefactNamePrecomputationSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/core/ArtefactNamingContractSpec.groovy @@ -19,21 +19,31 @@ package org.grails.core import grails.core.GrailsClass +import org.grails.core.artefact.ControllerArtefactHandler +import org.grails.core.artefact.DomainClassArtefactHandler import spock.lang.Specification import spock.lang.Unroll /** - * Tests for deterministic GrailsClass naming metadata. + * Tests for deterministic GrailsClass naming metadata, and for the artefact detection + * contract (ArtefactHandler#isArtefactClass) that a naming precomputation refactor is + * most likely to disturb. */ -class ArtefactNamePrecomputationSpec extends Specification { +class ArtefactNamingContractSpec extends Specification { private static final String BASE_PACKAGE = 'org.grails.core' @Unroll - void "naming contract for #entry.label is precomputed"() { + void "#entry.label naming metadata matches the observed contract"() { given: GrailsClass grailsClass = grailsClassFor(entry.artifactType, entry.wrapperClass) + // GrailsNameUtils.getNaturalName splits into words at each lower-to-upper case + // transition, but an acronym run (HTML, JSONAPI, URL) absorbs the first letter + // of the following word before that transition triggers - hence 'HTMLC ontroller' + // (not 'HTML Controller') and 'JSONAPIS ervice' (not 'JSONAPI Service') below. + // These are pinned intentionally: don't "fix" these expectations, or the + // algorithm, without knowing this spec exists to catch exactly that change. expect: grailsClass.name == entry.name grailsClass.shortName == entry.shortName @@ -186,6 +196,21 @@ class ArtefactNamePrecomputationSpec extends Specification { ] } + void "ControllerArtefactHandler accepts a concrete class whose name ends with the controller suffix"() { + expect: + new ControllerArtefactHandler().isArtefactClass(HTMLController) + } + + void "ControllerArtefactHandler rejects an abstract class even when the name matches the controller suffix"() { + expect: + !new ControllerArtefactHandler().isArtefactClass(AbstractFooController) + } + + void "DomainClassArtefactHandler rejects a class named like a domain class but carrying no domain annotation"() { + expect: + !new DomainClassArtefactHandler().isArtefactClass(URLDomain) + } + private GrailsClass grailsClassFor(String artifactType, Class wrapperClass) { switch (artifactType) { case 'controller': @@ -204,6 +229,8 @@ class ArtefactNamePrecomputationSpec extends Specification { class HTMLController {} +abstract class AbstractFooController {} + class PayRollController {} class JSONAPIService {}