From c7f4064653d372eed0bc3d05ffc20c0801b0362e Mon Sep 17 00:00:00 2001 From: James Fredley Date: Thu, 28 May 2026 23:07:22 -0400 Subject: [PATCH 1/2] Fix ignored failOnError argument in DeleteEntityDataFetcher GORM's delete(Map) only honors the flush parameter, so the failOnError: true argument passed in deleteInstance() was silently ignored. Switch to delete(flush: true) so the delete executes immediately and any failure surfaces through the data fetcher's response handler. Flagged during review of #15599. Assisted-by: claude-code:claude-4.8-opus --- .../fetcher/impl/DeleteEntityDataFetcher.groovy | 2 +- .../impl/DeleteEntityDataFetcherSpec.groovy | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy b/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy index 65a80fac38f..56559e4d396 100644 --- a/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy +++ b/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy @@ -48,7 +48,7 @@ class DeleteEntityDataFetcher extends DefaultGormDataFetcher implements De } protected void deleteInstance(GormEntity instance) { - instance.delete(failOnError: true) + instance.delete(flush: true) } @Override diff --git a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy index 12d686ce2bb..99e2e4b8956 100644 --- a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy +++ b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy @@ -21,6 +21,7 @@ package org.grails.gorm.graphql.fetcher.impl import grails.gorm.transactions.Transactional import graphql.schema.DataFetchingEnvironment +import org.grails.datastore.gorm.GormEntity import org.grails.gorm.graphql.HibernateSpec import org.grails.gorm.graphql.domain.general.toone.One import org.grails.gorm.graphql.fetcher.GraphQLDataFetcherType @@ -76,6 +77,19 @@ class DeleteEntityDataFetcherSpec extends HibernateSpec { 1 * responseHandler.createResponse(env, false, _ as NullPointerException) } + void "test deleteInstance flushes the delete and does not pass the ignored failOnError argument"() { + given: + DeleteEntityDataFetcher fetcher = new DeleteEntityDataFetcher<>(mappingContext.getPersistentEntity(One.name)) + GormEntity instance = Mock(GormEntity) + + when: + fetcher.deleteInstance(instance) + + then: 'the delete is flushed so database failures surface, and failOnError (ignored by delete) is not used' + 1 * instance.delete([flush: true]) + 0 * instance.delete([failOnError: true]) + } + void "test supports"() { when: DeleteEntityDataFetcher fetcher = new DeleteEntityDataFetcher<>(mappingContext.getPersistentEntity(One.name)) From cb9afdd16ac30c9cabcc2df90d43439a337919cd Mon Sep 17 00:00:00 2001 From: James Fredley Date: Wed, 3 Jun 2026 15:54:35 -0400 Subject: [PATCH 2/2] Remove no-op failOnError argument from DeleteEntityDataFetcher GORM's delete(Map) only honors the flush key, so the failOnError: true argument passed in deleteInstance() was silently ignored - the call behaved as a plain non-flushing delete(). Switching to delete(flush: true) would force an immediate flush and therefore change runtime behavior. Instead call instance.delete() with no arguments, preserving the original behavior exactly while removing the misleading failOnError argument that did nothing. Flagged during review of #15599. Assisted-by: claude-code:claude-4.8-opus --- .../graphql/fetcher/impl/DeleteEntityDataFetcher.groovy | 2 +- .../fetcher/impl/DeleteEntityDataFetcherSpec.groovy | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy b/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy index 56559e4d396..6f04f74560b 100644 --- a/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy +++ b/grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy @@ -48,7 +48,7 @@ class DeleteEntityDataFetcher extends DefaultGormDataFetcher implements De } protected void deleteInstance(GormEntity instance) { - instance.delete(flush: true) + instance.delete() } @Override diff --git a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy index 99e2e4b8956..76f9d629485 100644 --- a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy +++ b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcherSpec.groovy @@ -77,7 +77,7 @@ class DeleteEntityDataFetcherSpec extends HibernateSpec { 1 * responseHandler.createResponse(env, false, _ as NullPointerException) } - void "test deleteInstance flushes the delete and does not pass the ignored failOnError argument"() { + void "test deleteInstance calls delete without the ignored failOnError argument"() { given: DeleteEntityDataFetcher fetcher = new DeleteEntityDataFetcher<>(mappingContext.getPersistentEntity(One.name)) GormEntity instance = Mock(GormEntity) @@ -85,9 +85,9 @@ class DeleteEntityDataFetcherSpec extends HibernateSpec { when: fetcher.deleteInstance(instance) - then: 'the delete is flushed so database failures surface, and failOnError (ignored by delete) is not used' - 1 * instance.delete([flush: true]) - 0 * instance.delete([failOnError: true]) + then: 'delete is called with no arguments; failOnError was silently ignored by GORM, so removing it preserves the original behavior' + 1 * instance.delete() + 0 * instance.delete(_) } void "test supports"() {