From 5d93b5a94dd761a01b01e0d86f1ae2d289b0902b Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Fri, 8 Aug 2025 10:30:53 +0500 Subject: [PATCH 1/8] fix: add dirty check to acl identity --- .../plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy b/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy index ec5d12751..bb717fb51 100644 --- a/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy +++ b/plugin-acl/plugin/src/main/groovy/grails/plugin/springsecurity/acl/AbstractAclObjectIdentity.groovy @@ -18,6 +18,7 @@ */ package grails.plugin.springsecurity.acl +import grails.gorm.dirty.checking.DirtyCheck import groovy.transform.EqualsAndHashCode import groovy.transform.ToString @@ -29,6 +30,7 @@ import groovy.transform.ToString */ @EqualsAndHashCode(includes=['aclClass', 'parent', 'owner', 'entriesInheriting']) @ToString(includeNames=true) +@DirtyCheck abstract class AbstractAclObjectIdentity implements Serializable { AclClass aclClass From b9dd59a75a2d024015f4ae5289be4511442433dc Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Fri, 8 Aug 2025 10:31:16 +0500 Subject: [PATCH 2/8] fix: do not fetch all records from db --- .../springsecurity/acl/AclObjectIdentityGormService.groovy | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy index c72de8c7a..dcfe4d83d 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy @@ -45,11 +45,7 @@ class AclObjectIdentityGormService { @CompileDynamic @ReadOnly List findAllByParentObjectIdAndParentAclClassName(Long objectId, String aclClassName) { - //findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() - List aclObjectIdentityList = findAll() - aclObjectIdentityList.findAll { AclObjectIdentity oid -> - (oid?.parent?.aclClass?.className == aclClassName) && ( oid?.parent?.objectId == objectId) - } + return findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() } @ReadOnly From 43596552d8531d41a4f3b6ce9e7c366059ce4c87 Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Fri, 8 Aug 2025 10:32:28 +0500 Subject: [PATCH 3/8] fix: sid matching, maxOrder, common save --- .../plugin/springsecurity/acl/AclSid.groovy | 7 +++ .../springsecurity/acl/AclService.groovy | 58 ++++++++++++------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy b/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy index 83eb6ec76..29f8fddc9 100644 --- a/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy +++ b/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy @@ -20,6 +20,9 @@ package grails.plugin.springsecurity.acl import groovy.transform.EqualsAndHashCode import groovy.transform.ToString +import org.springframework.security.acls.domain.GrantedAuthoritySid +import org.springframework.security.acls.domain.PrincipalSid +import org.springframework.security.acls.model.Sid /** * @author Burt Beckwith @@ -41,4 +44,8 @@ class AclSid implements Serializable { principal unique: 'sid' sid blank: false, size: 1..255 } + + Sid toSid() { + return principal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid) + } } diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy index 91961a4c3..1bea043bd 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy @@ -23,6 +23,7 @@ import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import groovy.util.logging.Slf4j import org.springframework.context.MessageSource +import org.springframework.context.i18n.LocaleContextHolder import org.springframework.security.acls.domain.AccessControlEntryImpl import org.springframework.security.acls.domain.GrantedAuthoritySid import org.springframework.security.acls.domain.ObjectIdentityImpl @@ -97,9 +98,7 @@ class AclService implements MutableAclService, WarnErros { objectId: object.identifier as Long, owner: ownerSid, entriesInheriting: true) - if ( !aclObjectIdentity.save() ) { - log.error '{}', errorsBeanBeingSaved(messageSource, aclObjectIdentity) - } + save(aclObjectIdentity) aclObjectIdentity } @@ -124,9 +123,7 @@ class AclService implements MutableAclService, WarnErros { AclSid aclSid = aclSidGormService.findBySidAndPrincipal(sidName, principal) if (!aclSid && allowCreate) { aclSid = new AclSid(sid: sidName, principal: principal) - if ( !aclSid.save() ) { - log.error '{}', errorsBeanBeingSaved(messageSource, aclSid) - } + save(aclSid) } aclSid } @@ -136,9 +133,7 @@ class AclService implements MutableAclService, WarnErros { AclClass aclClass = aclClassGormService.findByClassName(className) if (!aclClass && allowCreate) { aclClass = new AclClass(className: className) - if ( !aclClass.save() ) { - log.error '{}', errorsBeanBeingSaved(messageSource, aclClass) - } + save(aclClass) } aclClass } @@ -209,7 +204,9 @@ class AclService implements MutableAclService, WarnErros { log.trace 'Checking ace for delete: {}', ace !acl.entries.find { AccessControlEntry entry -> log.trace 'Checking entry for delete: {}', entry - entry.permission.mask == ace.mask && entry.sid == ace.sid.sid + entry.permission.mask == ace.mask && + entry.sid == ace.sid.toSid() && + entry.granting == ace.granting } } @@ -217,15 +214,19 @@ class AclService implements MutableAclService, WarnErros { log.trace 'Checking entry for create: {}', entry !existingAces.find { AclEntry ace -> log.trace 'Checking ace for create: {}', ace - entry.permission.mask == ace.mask && entry.sid == ace.sid.sid + entry.permission.mask == ace.mask && + entry.sid == ace.sid.toSid() && + entry.granting == ace.granting } } + Integer maxAceOrder = existingAces.max { it.aceOrder }?.aceOrder + // Delete this ACL's ACEs in the acl_entry table deleteEntries toDelete // Create this ACL's ACEs in the acl_entry table - createEntries(acl, toCreate as List) + createEntries(acl, maxAceOrder, toCreate as List) // Change the mutable columns in acl_object_identity updateObjectIdentity acl @@ -237,10 +238,9 @@ class AclService implements MutableAclService, WarnErros { } @Transactional - protected void createEntries(MutableAcl acl, List entries = null) { - List entryList = entries ?: acl.entries as List - int i = 0 - for (AuditableAccessControlEntry entry in entryList) { + protected void createEntries(MutableAcl acl, Integer maxOrder, List entries) { + int i = maxOrder != null ? maxOrder + 1 : 0 + for (AuditableAccessControlEntry entry in entries) { Assert.isInstanceOf AccessControlEntryImpl, entry, 'Unknown ACE class' AclEntry aclEntryInstance = new AclEntry( aclObjectIdentity: AclObjectIdentity.load(acl.id), @@ -250,9 +250,7 @@ class AclService implements MutableAclService, WarnErros { granting: entry.isGranting(), auditSuccess: entry.isAuditSuccess(), auditFailure: entry.isAuditFailure()) - if ( !aclEntryInstance.save() ) { - log.error "{}", errorsBeanBeingSaved(messageSource, aclEntryInstance) - } + save(aclEntryInstance) } } @@ -272,9 +270,7 @@ class AclService implements MutableAclService, WarnErros { aclObjectIdentity.parent = parent aclObjectIdentity.owner = createOrRetrieveSid(acl.owner, true) aclObjectIdentity.entriesInheriting = acl.isEntriesInheriting() - if ( !aclObjectIdentity.save() ) { - log.error "{}", errorsBeanBeingSaved(messageSource, aclObjectIdentity) - } + save(aclObjectIdentity) AclObjectIdentity.withSession { it.flush() } } @@ -335,4 +331,22 @@ class AclService implements MutableAclService, WarnErros { } return result } + + @CompileDynamic + protected save(bean) { + if (!bean.save(flush: true)) { + if (log.warnEnabled) { + def message = new StringBuilder("problem creating ${bean.getClass().simpleName}: $bean") + def locale = LocaleContextHolder.getLocale() + for (fieldErrors in bean.errors) { + for (error in fieldErrors.allErrors) { + message << '\n\t' << messageSource.getMessage(error, locale) + } + } + log.warn message.toString() + } + } + + bean + } } From bd9da61795fb46a9b60bdfa96bdd626c3bc6a878 Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Tue, 19 Aug 2025 13:39:57 +0500 Subject: [PATCH 4/8] fix: review comments --- .../plugin/springsecurity/acl/AclSid.groovy | 2 +- .../acl/AclObjectIdentityGormService.groovy | 2 -- .../springsecurity/acl/AclService.groovy | 23 ++++++------------- .../acl/AclObjectIdentity.groovy | 2 ++ 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy b/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy index 29f8fddc9..049f1d249 100644 --- a/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy +++ b/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy @@ -46,6 +46,6 @@ class AclSid implements Serializable { } Sid toSid() { - return principal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid) + principal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid) } } diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy index dcfe4d83d..f6b565623 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy @@ -21,7 +21,6 @@ package grails.plugin.springsecurity.acl import grails.gorm.DetachedCriteria import grails.gorm.transactions.ReadOnly -import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import org.springframework.security.acls.model.ObjectIdentity @@ -42,7 +41,6 @@ class AclObjectIdentityGormService { AclObjectIdentity.get(id) } - @CompileDynamic @ReadOnly List findAllByParentObjectIdAndParentAclClassName(Long objectId, String aclClassName) { return findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy index 1bea043bd..11a677fef 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy @@ -22,8 +22,8 @@ import grails.gorm.transactions.ReadOnly import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import groovy.util.logging.Slf4j +import org.grails.datastore.gorm.GormEntity import org.springframework.context.MessageSource -import org.springframework.context.i18n.LocaleContextHolder import org.springframework.security.acls.domain.AccessControlEntryImpl import org.springframework.security.acls.domain.GrantedAuthoritySid import org.springframework.security.acls.domain.ObjectIdentityImpl @@ -99,7 +99,6 @@ class AclService implements MutableAclService, WarnErros { owner: ownerSid, entriesInheriting: true) save(aclObjectIdentity) - aclObjectIdentity } @Transactional @@ -332,21 +331,13 @@ class AclService implements MutableAclService, WarnErros { return result } - @CompileDynamic - protected save(bean) { - if (!bean.save(flush: true)) { - if (log.warnEnabled) { - def message = new StringBuilder("problem creating ${bean.getClass().simpleName}: $bean") - def locale = LocaleContextHolder.getLocale() - for (fieldErrors in bean.errors) { - for (error in fieldErrors.allErrors) { - message << '\n\t' << messageSource.getMessage(error, locale) - } - } - log.warn message.toString() - } + @Transactional + protected > T save(T bean) { + if (!bean.save()) { + if (log.warnEnabled) { + log.warn errorsBeanBeingSaved(messageSource, bean) + } } - bean } } diff --git a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy index 306f43a1d..061bc98c7 100644 --- a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy +++ b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclObjectIdentity.groovy @@ -18,12 +18,14 @@ */ package grails.plugin.springsecurity.acl +import grails.gorm.dirty.checking.DirtyCheck import groovy.transform.ToString /** * @author Burt Beckwith */ @ToString(excludes='version', includeNames=true) +@DirtyCheck class AclObjectIdentity { private static final long serialVersionUID = 1 From a85ad4a1ff5d64d029730244505a2b3195be1661 Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Tue, 19 Aug 2025 13:40:08 +0500 Subject: [PATCH 5/8] fix: test cases --- .../domain/grails/plugin/springsecurity/acl/AclSid.groovy | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy index 063bc9dd8..8d4474c70 100644 --- a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy +++ b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy @@ -19,6 +19,9 @@ package grails.plugin.springsecurity.acl import groovy.transform.ToString +import org.springframework.security.acls.domain.GrantedAuthoritySid +import org.springframework.security.acls.domain.PrincipalSid +import org.springframework.security.acls.model.Sid /** * @author Burt Beckwith @@ -39,4 +42,8 @@ class AclSid implements Serializable { principal unique: 'sid' sid size: 1..255 } + + Sid toSid() { + principal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid) + } } From 38e3fc49287be1005344089c69895a1730186da2 Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Tue, 19 Aug 2025 13:48:04 +0500 Subject: [PATCH 6/8] fix: remove return --- .../springsecurity/acl/AclObjectIdentityGormService.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy index f6b565623..a8a423907 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclObjectIdentityGormService.groovy @@ -43,7 +43,7 @@ class AclObjectIdentityGormService { @ReadOnly List findAllByParentObjectIdAndParentAclClassName(Long objectId, String aclClassName) { - return findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() + findQueryByParentObjectIdAndParentAclClassName(objectId, aclClassName).list() } @ReadOnly From a79222ddc49f99a29389f9457bc34a11eb1c4495 Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Wed, 20 Aug 2025 11:08:03 +0500 Subject: [PATCH 7/8] fix: move method to AclService & test cases --- .../grails/plugin/springsecurity/acl/AclSid.groovy | 7 ------- .../plugin/springsecurity/acl/AclService.groovy | 13 +++++++------ .../grails/plugin/springsecurity/acl/AclSid.groovy | 7 ------- .../groovy/spec/AclEntrySpec.groovy | 8 ++++---- 4 files changed, 11 insertions(+), 24 deletions(-) diff --git a/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy b/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy index 049f1d249..83eb6ec76 100644 --- a/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy +++ b/plugin-acl/plugin/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy @@ -20,9 +20,6 @@ package grails.plugin.springsecurity.acl import groovy.transform.EqualsAndHashCode import groovy.transform.ToString -import org.springframework.security.acls.domain.GrantedAuthoritySid -import org.springframework.security.acls.domain.PrincipalSid -import org.springframework.security.acls.model.Sid /** * @author Burt Beckwith @@ -44,8 +41,4 @@ class AclSid implements Serializable { principal unique: 'sid' sid blank: false, size: 1..255 } - - Sid toSid() { - principal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid) - } } diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy index 11a677fef..c8f3122c2 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy @@ -203,9 +203,7 @@ class AclService implements MutableAclService, WarnErros { log.trace 'Checking ace for delete: {}', ace !acl.entries.find { AccessControlEntry entry -> log.trace 'Checking entry for delete: {}', entry - entry.permission.mask == ace.mask && - entry.sid == ace.sid.toSid() && - entry.granting == ace.granting + entryEqual(ace, entry) } } @@ -213,9 +211,7 @@ class AclService implements MutableAclService, WarnErros { log.trace 'Checking entry for create: {}', entry !existingAces.find { AclEntry ace -> log.trace 'Checking ace for create: {}', ace - entry.permission.mask == ace.mask && - entry.sid == ace.sid.toSid() && - entry.granting == ace.granting + entryEqual(ace, entry) } } @@ -340,4 +336,9 @@ class AclService implements MutableAclService, WarnErros { } bean } + + private static boolean entryEqual(AclEntry ace, AccessControlEntry entry) { + Sid sid = ace.sid.principal ? new PrincipalSid(ace.sid.sid) : new GrantedAuthoritySid(ace.sid.sid) + return entry.permission.mask == ace.mask && entry.sid == sid && entry.granting == ace.granting + } } diff --git a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy index 8d4474c70..063bc9dd8 100644 --- a/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy +++ b/plugin-ui/examples/extended/grails-app/domain/grails/plugin/springsecurity/acl/AclSid.groovy @@ -19,9 +19,6 @@ package grails.plugin.springsecurity.acl import groovy.transform.ToString -import org.springframework.security.acls.domain.GrantedAuthoritySid -import org.springframework.security.acls.domain.PrincipalSid -import org.springframework.security.acls.model.Sid /** * @author Burt Beckwith @@ -42,8 +39,4 @@ class AclSid implements Serializable { principal unique: 'sid' sid size: 1..255 } - - Sid toSid() { - principal ? new PrincipalSid(sid) : new GrantedAuthoritySid(sid) - } } diff --git a/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy b/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy index 630874ac0..c01313dba 100644 --- a/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy +++ b/plugin-ui/examples/extended/src/integration-test/groovy/spec/AclEntrySpec.groovy @@ -54,9 +54,9 @@ class AclEntrySpec extends AbstractSecuritySpec { aclEntrySearchPage.assertResults(1, 3, 3) assertContentContains('60') - assertContentContains('398') - assertContentContains('399') - assertContentContains('400') + assertContentContains('62') + assertContentContains('194') + assertContentContains('195') assertContentContains('user1') assertContentContains('admin') assertContentDoesNotContain('>user2') @@ -74,7 +74,7 @@ class AclEntrySpec extends AbstractSecuritySpec { then: browser.at(AclEntrySearchPage) aclEntrySearchPage.assertResults(1, 10, 67) - ['104', '111', '119', '126', '131', '136', '141', '146', '152', '159'].each { + ['75', '76', '78', '80', '82', '87', '89', '91', '93', '95'].each { assertContentContains it } } From d5da1901561a8927dcb99a272c336df30e789e84 Mon Sep 17 00:00:00 2001 From: Eugene Kamenev Date: Wed, 20 Aug 2025 18:13:40 +0500 Subject: [PATCH 8/8] fix: remove redundant condition --- .../grails/plugin/springsecurity/acl/AclService.groovy | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy index c8f3122c2..602fe74b6 100644 --- a/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy +++ b/plugin-acl/plugin/grails-app/services/grails/plugin/springsecurity/acl/AclService.groovy @@ -330,9 +330,7 @@ class AclService implements MutableAclService, WarnErros { @Transactional protected > T save(T bean) { if (!bean.save()) { - if (log.warnEnabled) { - log.warn errorsBeanBeingSaved(messageSource, bean) - } + log.warn errorsBeanBeingSaved(messageSource, bean) } bean }