Grails
  1. Grails
  2. GRAILS-7093

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

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 2.4
    • 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

          Activity

          Show
          Daniel Henrique Alves Lima added a comment - http://grails.1312388.n4.nabble.com/Better-support-to-programmatic-transactions-withTransaction-improvement-td3166486.html
          Hide
          Jeff Scott 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 Scott 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
          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
          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
          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
          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
          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.
          Show
          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 - I added configurable defaults: https://github.com/daniel-lima/grails-transaction-handling-plugin/blob/master/grails-app/conf/TransactionHandlingDefaultConfig.groovy
          Hide
          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
          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
          Jeff Scott Brown added a comment -

          Daniel,

          Have you written any relevant test coverage?

          Thanks for the help.

          Show
          Jeff Scott Brown added a comment - Daniel, Have you written any relevant test coverage? Thanks for the help.
          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
          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
          Jeff Scott 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 Scott 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
          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
          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
          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.
          Hide
          Tamer Shahin added a comment -

          will this be fixed sooner or later?

          Show
          Tamer Shahin added a comment - will this be fixed sooner or later?
          Hide
          Graeme Rocher added a comment -

          Something else to sneak into 2.4 I think

          Show
          Graeme Rocher added a comment - Something else to sneak into 2.4 I think
          Hide
          Tamer Shahin added a comment -

          thank you!
          well, considering that it will be available on 2.4, for the moment we preferred not to use the plugin by Daniel Lima but we created a thinner helper with methods like this:

          static DefaultTransactionDefinition txDefinition()

          { DefaultTransactionDefinition definition = new DefaultTransactionDefinition() definition.setIsolationLevel(TransactionDefinition.ISOLATION_READ_COMMITTED) definition.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED) return definition }

          and, only where is needed we import statically the method above and using it like this:

          Book.withTransaction txDefinition(),

          { .. }
          Show
          Tamer Shahin added a comment - thank you! well, considering that it will be available on 2.4, for the moment we preferred not to use the plugin by Daniel Lima but we created a thinner helper with methods like this: static DefaultTransactionDefinition txDefinition() { DefaultTransactionDefinition definition = new DefaultTransactionDefinition() definition.setIsolationLevel(TransactionDefinition.ISOLATION_READ_COMMITTED) definition.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRED) return definition } and, only where is needed we import statically the method above and using it like this: Book.withTransaction txDefinition(), { .. }

            People

            • Assignee:
              Jeff Scott Brown
              Reporter:
              Daniel Henrique Alves Lima
            • Votes:
              24 Vote for this issue
              Watchers:
              19 Start watching this issue

              Dates

              • Created:
                Updated:
                Last Reviewed:

                Development