Grails

Better support to programmatic transactions (withTransaction closure extension/changing)

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: 2.3-M2
  • Component/s: Persistence
  • Labels:
    None
  • Patch Submitted:
    Yes

Description

It's just a suggestion, but i really believe it would be a useful improvement:

Apparently, withTransaction supports only propagation "required". This could be enough for regular applications, but it "fails" when you need a fine-grained transaction control.

The suggested improvement is to offer another closure or to change withTransaction definition (HibernatePluginSupport.groovy) to support additional properties. These properties will be used to configure TransactionTemplate instance. I've written something similar in other project:

    def ctx = grailsApplication.mainContext
    grailsApplication.domainClasses.each {
      domainClass ->
	domainClass.metaClass.static.withNewTransaction = {
	  properties = [propagationBehavior: TransactionDefinition.PROPAGATION_REQUIRES_NEW],
	  Closure callable ->
	    def template = new TransactionTemplate(ctx.getBean('transactionManager'))
	    properties.each {
	      property ->
		template[property.key] = property.value
	    }
	    template.execute({status ->
	      callable.call(status)
			     } as TransactionCallback)
        }
    }

So we can easily start a new different transaction (even inside a Service class method/closure):

MyDomainClass.withNewTransaction = {
   // Your code here
}

or explicitly:

MyDomainClass.withNewTransaction(propagationBehavior: TransactionDefinition.PROPAGATION_REQUIRES_NEW) = {
   // Your code here
}

or even something more complex:

MyDomainClass.withNewTransaction(propagationBehavior: TransactionDefinition.PROPAGATION_NESTED, isolationLevelName: 'ISOLATION_READ_UNCOMMITTED') = {
   // Your code here
}

It makes more sense to me to change withTransaction implementation than adding another closure because not every propagation type will create a "new transaction".

For a new closure implementation, properties could be initialized as the sample (it's just a suggestion).
For a changing in withTransaction implementation, properties should be initialized as an empty Map (properties = [:]) or null map (with null check), for backward compatibility.

Reference:
http://grails.org/doc/latest/ref/Domain%20Classes/withTransaction.html
http://static.springsource.org/spring/docs/2.0.x/api/org/springframework/transaction/TransactionDefinition.html#PROPAGATION_REQUIRED
http://static.springsource.org/spring/docs/2.0.x/api/org/springframework/transaction/support/TransactionTemplate.html

Thanks!

Issue Links

duplicates

New Feature - A new feature of the product, which has yet to be developed. GRAILS-6630 Support detached transactions by DomainClass.withNewTransaction { ... }

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

http://grails.1312388.n4.nabble.com/Better-support-to-programmatic-transactions-withTransaction-improvement-td3166486.html

Show
Daniel Henrique Alves Lima added a comment - http://grails.1312388.n4.nabble.com/Better-support-to-programmatic-transactions-withTransaction-improvement-td3166486.html
Hide
Permalink
Jeff Brown added a comment -

I am removing 1.3.7 from the Fix Version field. I do not think that a change like this should be considered for that release. 1.4 and 2.0 are better candidates.

Show
Jeff Brown added a comment - I am removing 1.3.7 from the Fix Version field. I do not think that a change like this should be considered for that release. 1.4 and 2.0 are better candidates.
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

Based in some feedback provided by Luke and Peter, i'm suggesting a new version:

    def ctx = grailsApplication.mainContext

    def withTransactionImpl = {
      Map sharedVars, Map properties = [propagation: "required"], Closure callable ->
        
        def propsMapping = [isolation: "isolationLevelName", propagation: "propagationBehaviorName"]
        synchronized(sharedVars) {
          if (sharedVars.isEmpty()) {
            /* Lazy init of shared variables. */   
            propsMapping.keySet().each {
              propName ->
                sharedVars[propName + "Mapping"] = [:]
            }

            def modifier = Modifier.FINAL | Modifier.STATIC | Modifier.PUBLIC
            /* Build NAME -> CONSTANT NAME mappings. */
            def constants = (TransactionDefinition.class.declaredFields as List).findAll {
              field ->
                if ((field.modifiers & modifier) == modifier) {
                  def constant = field
                  propsMapping.keySet().each {
                    propName ->
                      if (constant.name.startsWith(TransactionTemplate["PREFIX_" + propName.toUpperCase()])) {
                        def key = constant.name.replace(TransactionTemplate["PREFIX_" + propName.toUpperCase()], "")
                        key = key.replace("_", "-").toLowerCase()
                        sharedVars[propName + "Mapping"][GrailsNameUtils.getPropertyNameForLowerCaseHyphenSeparatedName(key)] = constant.name
                      } 
                  }
                }
            }
            
          }
        }
        
        //println "sharedVars ${sharedVars}"
        def template = new TransactionTemplate(ctx.getBean('transactionManager'))
        properties.entrySet().each {
          property ->
            if (propsMapping.containsKey(property.key)) {
              def newKey = propsMapping[property.key]
              template[newKey] = sharedVars[property.key + "Mapping"][property.value] 
            } else {
              template[property.key] = property.value
            }
        }

        template.execute(
          {status ->
            callable.call(status)
          } as TransactionCallback)
    }

    withTransactionImpl = withTransactionImpl.curry([:])
    def withNewTransactionImpl = {
      Map properties = [:], Closure callable ->
        properties.propagation = "requiresNew"
        withTransactionImpl(properties, callable)
    }

    grailsApplication.domainClasses.each {
      domainClass ->
        domainClass.metaClass.static.withTransaction = withTransactionImpl
        domainClass.metaClass.static.withNewTransaction = withNewTransactionImpl
    }

To be able to write transactional statements like:

MyDomainClass.withTransaction = {
   // Your code here
}

MyDomainClass.withNewTransaction = {
   // Your code here
}


MyDomainClass.withTransaction(propagation: "supports", timeout: 30000) = {
   // Your code here
}

MyDomainClass.withNewTransaction(isolation: "readUncommitted", readOnly: true) = {
   // Your code here
}

Luke has suggested less verbose property names and values:
http://grails.1312388.n4.nabble.com/Better-support-to-programmatic-transactions-withTransaction-improvement-tp3166486p3166647.html

Peter has suggested withTransaction and withNewTransaction usage:
http://grails.1312388.n4.nabble.com/Gorm-improvements-tp3167704p3175662.html

Show
Daniel Henrique Alves Lima added a comment - Based in some feedback provided by Luke and Peter, i'm suggesting a new version: def ctx = grailsApplication.mainContext def withTransactionImpl = { Map sharedVars, Map properties = [propagation: "required" ], Closure callable -> def propsMapping = [isolation: "isolationLevelName" , propagation: "propagationBehaviorName" ] synchronized (sharedVars) { if (sharedVars.isEmpty()) { /* Lazy init of shared variables. */ propsMapping.keySet().each { propName -> sharedVars[propName + "Mapping" ] = [:] } def modifier = Modifier.FINAL | Modifier.STATIC | Modifier.PUBLIC /* Build NAME -> CONSTANT NAME mappings. */ def constants = (TransactionDefinition.class.declaredFields as List).findAll { field -> if ((field.modifiers & modifier) == modifier) { def constant = field propsMapping.keySet().each { propName -> if (constant.name.startsWith(TransactionTemplate[ "PREFIX_" + propName.toUpperCase()])) { def key = constant.name.replace(TransactionTemplate[ "PREFIX_" + propName.toUpperCase()], "") key = key.replace( "_" , "-" ).toLowerCase() sharedVars[propName + "Mapping" ][GrailsNameUtils.getPropertyNameForLowerCaseHyphenSeparatedName(key)] = constant.name } } } } } } //println "sharedVars ${sharedVars}" def template = new TransactionTemplate(ctx.getBean('transactionManager')) properties.entrySet().each { property -> if (propsMapping.containsKey(property.key)) { def newKey = propsMapping[property.key] template[newKey] = sharedVars[property.key + "Mapping" ][property.value] } else { template[property.key] = property.value } } template.execute( {status -> callable.call(status) } as TransactionCallback) } withTransactionImpl = withTransactionImpl.curry([:]) def withNewTransactionImpl = { Map properties = [:], Closure callable -> properties.propagation = "requiresNew" withTransactionImpl(properties, callable) } grailsApplication.domainClasses.each { domainClass -> domainClass.metaClass. static .withTransaction = withTransactionImpl domainClass.metaClass. static .withNewTransaction = withNewTransactionImpl } To be able to write transactional statements like: MyDomainClass.withTransaction = { // Your code here } MyDomainClass.withNewTransaction = { // Your code here } MyDomainClass.withTransaction(propagation: "supports" , timeout: 30000) = { // Your code here } MyDomainClass.withNewTransaction(isolation: "readUncommitted" , readOnly: true ) = { // Your code here } Luke has suggested less verbose property names and values: http://grails.1312388.n4.nabble.com/Better-support-to-programmatic-transactions-withTransaction-improvement-tp3166486p3166647.html Peter has suggested withTransaction and withNewTransaction usage: http://grails.1312388.n4.nabble.com/Gorm-improvements-tp3167704p3175662.html
Hide
Permalink
Daniel Henrique Alves Lima added a comment - - edited

Patch file generated using "diff -u base_file modified_file".
Grails 1.3.6 was used as base.

Show
Daniel Henrique Alves Lima added a comment - - edited Patch file generated using "diff -u base_file modified_file". Grails 1.3.6 was used as base.
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

Patch file generated using "diff -u base_file modified_file".
Grails 1.3.6 was used as base.

Show
Daniel Henrique Alves Lima added a comment - Patch file generated using "diff -u base_file modified_file". Grails 1.3.6 was used as base.
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

I believe configurable defaults for properties like 'timeout' and 'readonly' would be a nice addition.

The maps

def withTxDefaults = [propagation: 'required']
def withNewTxDefaults = [propagation: 'requiresNew']

can be populated with values loaded from Config.groovy.

Show
Daniel Henrique Alves Lima added a comment - I believe configurable defaults for properties like 'timeout' and 'readonly' would be a nice addition. The maps def withTxDefaults = [propagation: 'required'] def withNewTxDefaults = [propagation: 'requiresNew'] can be populated with values loaded from Config.groovy.
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

Hi.

Will this feature be incorporated to Grails 1.4 "as it is"? I'm considering to backport it as a separated plugin so people using Grails < 1.4 (including me) can use this feature.

Thanks in advance.

Show
Daniel Henrique Alves Lima added a comment - Hi. Will this feature be incorporated to Grails 1.4 "as it is"? I'm considering to backport it as a separated plugin so people using Grails < 1.4 (including me) can use this feature. Thanks in advance.
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

https://github.com/daniel-lima/grails-transaction-handling-plugin
http://grails.org/plugin/transaction-handling

Show
Daniel Henrique Alves Lima added a comment - https://github.com/daniel-lima/grails-transaction-handling-plugin http://grails.org/plugin/transaction-handling
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

I added configurable defaults:

https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/grails-app/conf/TransactionHandlingDefaultConfig.groovy

Show
Daniel Henrique Alves Lima added a comment - I added configurable defaults: https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/grails-app/conf/TransactionHandlingDefaultConfig.groovy
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

And now we have configurable defaults (including rollback rules) for the different forms of transaction management in Grails:

https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/grails-app/conf/TransactionHandlingDefaultConfig.groovy

Show
Daniel Henrique Alves Lima added a comment - And now we have configurable defaults (including rollback rules) for the different forms of transaction management in Grails: https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/grails-app/conf/TransactionHandlingDefaultConfig.groovy
Hide
Permalink
marko.m added a comment -

awesome piece of work Daniel, sure hope this makes it into 2.0.

in the mean time, thanks for the backlevel plugin!

Show
marko.m added a comment - awesome piece of work Daniel, sure hope this makes it into 2.0. in the mean time, thanks for the backlevel plugin!
Hide
Permalink
Jeff Brown added a comment -

Daniel,

Have you written any relevant test coverage?

Thanks for the help.

Show
Jeff Brown added a comment - Daniel, Have you written any relevant test coverage? Thanks for the help.
Hide
Permalink
Daniel Henrique Alves Lima added a comment -

@Marko: Thanks.

@Jeff: Hi Jeff.

== Up-to-date impl ==
https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/src/groovy/grails/plugin/transaction/handling/GroovyDynamicMethods.groovy
https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/src/groovy/grails/plugin/transaction/handling/TransactionPropertiesUtil.groovy

== Tests ==
https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/test/integration/grails/plugin/transaction/handling/DynamicMethodsTests.groovy
https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/test/unit/grails/plugin/transaction/handling/TransactionPropertiesUtilTests.groovy

Best regards,

Daniel.

Show
Daniel Henrique Alves Lima added a comment - @Marko: Thanks. @Jeff: Hi Jeff. == Up-to-date impl == https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/src/groovy/grails/plugin/transaction/handling/GroovyDynamicMethods.groovy https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/src/groovy/grails/plugin/transaction/handling/TransactionPropertiesUtil.groovy == Tests == https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/test/integration/grails/plugin/transaction/handling/DynamicMethodsTests.groovy https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/test/unit/grails/plugin/transaction/handling/TransactionPropertiesUtilTests.groovy Best regards, Daniel.
Hide
Permalink
François Terrier added a comment -

I believe this has made it into grails 2.0.x but is just not documented. Right?

Show
François Terrier added a comment - I believe this has made it into grails 2.0.x but is just not documented. Right?
Hide
Permalink
Jeff Brown added a comment -

Was this ever pulled into 2.0.x? I may have done that but I don't remember and I don't see the relevant commits.

Show
Jeff Brown added a comment - Was this ever pulled into 2.0.x? I may have done that but I don't remember and I don't see the relevant commits.
Hide
Permalink
Graeme Rocher added a comment -

I don't believe so, it is on your TODO list

Show
Graeme Rocher added a comment - I don't believe so, it is on your TODO list
Hide
Permalink
Aaron Long added a comment -

Some guy named Graeme added a withNewTransaction() method on 3/29/11 in revision a4717a1 of GormStaticApi

There is also a version that let's you pass a TransactionDefinition. However, I don't see anyway to override the timeout and indicate read-only at the transaction level so maybe some of this ticket is still outstanding.

Show
Aaron Long added a comment - Some guy named Graeme added a withNewTransaction() method on 3/29/11 in revision a4717a1 of GormStaticApi There is also a version that let's you pass a TransactionDefinition. However, I don't see anyway to override the timeout and indicate read-only at the transaction level so maybe some of this ticket is still outstanding.
Hide
Permalink
Mariano Lopez added a comment -

withNewTransaction() works, but isn't documented anywhere.

Show
Mariano Lopez added a comment - withNewTransaction() works, but isn't documented anywhere.

People

  • Assignee:
    Jeff Brown
    Reporter:
    Daniel Henrique Alves Lima
Vote (21)
Watch (18)

Dates

  • Created:
    Updated:
    Last Reviewed: