Skip to content

Commit cbd38b6

Browse files
authored
Merge pull request #15195 from codeconsole/7.0.x-gormservice-dev-racecondition
7.0.x Bug - Don't call a resource constructor until GORM has been initialized by Spring
2 parents b4dcd8a + 2c12a6d commit cbd38b6

File tree

3 files changed

+67
-21
lines changed

3 files changed

+67
-21
lines changed

grails-doc/src/en/guide/upgrading/upgrading60x.adoc

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,3 +860,49 @@ The `+` marker is opt-in and fully backward compatible:
860860
- Existing URL mappings without the `+` marker continue to work as before (splitting at the first dot)
861861
- No changes are required to existing applications
862862
- The feature can be adopted incrementally on a per-mapping basis
863+
864+
===== 12.28 GormService API Changes
865+
866+
The `grails.plugin.scaffolding.GormService` class has been updated to fix a thread-safety issue and improve API clarity.
867+
868+
====== Changes
869+
870+
1. The `resource` field type changed from `GormAllOperations<T>` to `Class<T>`
871+
2. A new `gormStaticApi` field of type `GormAllOperations<T>` was added with thread-safe lazy initialization using `@Lazy`
872+
3. The constructor no longer instantiates the resource class - it now stores the Class reference directly
873+
874+
====== Migration Impact
875+
876+
If your code extends `GormService` or accesses its fields:
877+
878+
**Accessing GORM operations**: Previously the `resource` field provided GORM operations. Now use the `gormStaticApi` field instead:
879+
880+
[source,groovy]
881+
----
882+
// Before (7.1.x and earlier)
883+
class MyService extends GormService<MyDomain> {
884+
void myMethod() {
885+
def result = resource.list() // resource was GormAllOperations<T>
886+
// ...
887+
}
888+
}
889+
890+
// After (7.1.x with fix)
891+
class MyService extends GormService<MyDomain> {
892+
void myMethod() {
893+
def result = gormStaticApi.list() // use gormStaticApi instead
894+
// ...
895+
}
896+
}
897+
----
898+
899+
**Accessing the domain class**: If you need the Class reference, the `resource` field is now properly typed as `Class<T>`:
900+
901+
[source,groovy]
902+
----
903+
// Before
904+
Class<MyDomain> clazz = resource.getClass() // awkward, resource was an instance
905+
906+
// After
907+
Class<MyDomain> clazz = resource // resource is now the Class itself
908+
----

grails-scaffolding/src/main/groovy/grails/plugin/scaffolding/DomainServiceLocator.java

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
/**
3434
* Resolves the appropriate service bean for a given domain class by:
3535
* - Scanning only beans of type GormService
36-
* - Matching via: ((GormEntity<?>) service.getResource()).instanceOf(domainClass)
36+
* - Matching via: domainClass.isAssignableFrom(service.getResource())
3737
*
3838
* Keeps a single shared cache for the whole app.
3939
*/
@@ -72,28 +72,25 @@ private static <T extends GormEntity<T>> GormService<T> findService(Class<T> dom
7272

7373
for (String name : names) {
7474
GormService<?> gs = (GormService<?>) ctx.getBean(name);
75-
Object resource = gs.getResource();
76-
if (resource instanceof GormEntity) {
77-
GormEntity<?> ge = (GormEntity<?>) resource;
78-
if (ge.instanceOf(domainClass)) {
79-
matchingBeanNames.add(name);
80-
if (match != null) {
81-
throw new IllegalStateException(
82-
"Multiple GormService beans match domain " + domainClass.getName() +
83-
": " + matchingBeanNames
84-
);
85-
}
86-
@SuppressWarnings("unchecked")
87-
GormService<T> svc = (GormService<T>) gs;
88-
match = svc;
75+
Class<?> resourceClass = gs.getResource();
76+
if (resourceClass != null && domainClass.isAssignableFrom(resourceClass)) {
77+
matchingBeanNames.add(name);
78+
if (match != null) {
79+
throw new IllegalStateException(
80+
"Multiple GormService beans match domain " + domainClass.getName() +
81+
": " + matchingBeanNames
82+
);
8983
}
84+
@SuppressWarnings("unchecked")
85+
GormService<T> svc = (GormService<T>) gs;
86+
match = svc;
9087
}
9188
}
9289

9390
if (match == null) {
9491
throw new IllegalStateException(
9592
"No GormService bean found for domain " + domainClass.getName() +
96-
" using resource.instanceOf(..). Scanned " + names.length + " GormService beans."
93+
". Scanned " + names.length + " GormService beans."
9794
);
9895
}
9996

grails-scaffolding/src/main/groovy/grails/plugin/scaffolding/GormService.groovy

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import grails.gorm.api.GormAllOperations
2626
import grails.gorm.transactions.ReadOnly
2727
import grails.gorm.transactions.Transactional
2828
import grails.util.GrailsNameUtils
29+
import org.grails.datastore.gorm.GormEnhancer
2930
import org.grails.datastore.gorm.GormEntity
3031
import org.grails.datastore.gorm.GormEntityApi
3132

@@ -34,28 +35,30 @@ import org.grails.datastore.gorm.GormEntityApi
3435
@CompileStatic
3536
class GormService<T extends GormEntity<T>> {
3637

37-
GormAllOperations<T> resource
38+
@Lazy
39+
GormAllOperations<T> gormStaticApi = GormEnhancer.findStaticApi(resource) as GormAllOperations<T>
40+
Class<T> resource
3841
String resourceName
3942
String resourceClassName
4043
boolean readOnly
4144

4245
GormService(Class<T> resource, boolean readOnly) {
43-
this.resource = resource.getDeclaredConstructor().newInstance() as GormAllOperations<T>
46+
this.resource = resource
4447
this.readOnly = readOnly
4548
resourceClassName = resource.simpleName
4649
resourceName = GrailsNameUtils.getPropertyName(resource)
4750
}
4851

4952
T get(Serializable id) {
50-
resource.get(id)
53+
gormStaticApi.get(id)
5154
}
5255

5356
List<T> list(Map args) {
54-
resource.list(args)
57+
gormStaticApi.list(args)
5558
}
5659

5760
Long count(Map args) {
58-
resource.count()
61+
gormStaticApi.count()
5962
}
6063

6164
@Transactional

0 commit comments

Comments
 (0)