Revert "Revert "- Revert: fix for GRAILS-6749 "subflow state redirects to wrong url"""

This reverts commit ed9d0f8.

Author: graemerocher

grails-plugin-gsp/src/main/groovy/org/codehaus/groovy/grails/plugins/web/taglib/ApplicationTagLib.groovy

@@ -29,8 +29,9 @@ import org.springframework.beans.factory.InitializingBean
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.context.ApplicationContext
 import org.springframework.context.ApplicationContextAware
+import org.codehaus.groovy.grails.web.servlet.mvc.GrailsWebRequest
 
- /**
+/**
  * The base application tag library for Grails many of which take inspiration from Rails helpers (thanks guys! :)
  * This tag library tends to get extended by others as tags within here can be re-used in said libraries
  *
@@ -348,6 +349,9 @@ class ApplicationTagLib implements ApplicationContextAware, InitializingBean, Gr
         if (request['flowExecutionKey']) {
             params."execution" = request['flowExecutionKey']
             urlAttrs.params = params
+            if (attrs.controller == null && attrs.action == null && attrs.url == null && attrs.uri == null) {
+                urlAttrs[LinkGenerator.ATTRIBUTE_ACTION] = GrailsWebRequest.lookup().actionName
+            }
         }
         if (urlAttrs.event) {
             params."_eventId" = urlAttrs.remove('event')

grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/webflow/SubflowExecutionTests.groovy

@@ -7,8 +7,15 @@ import org.codehaus.groovy.grails.webflow.support.AbstractGrailsTagAwareFlowExec
  * @since 1.2
  */
 class SubflowExecutionTests extends AbstractGrailsTagAwareFlowExecutionTests {
+    def subberFlow = {
+        subber {
+            on("proceed").to("subberEnd")
+        }
+        subberEnd()
+    }
 
     void testSubFlowExecution() {
+        registerFlow('subflowExecutionTests/subber', subberFlow)
         startFlow()
         assertCurrentStateEquals "start"
 
@@ -20,13 +27,6 @@ class SubflowExecutionTests extends AbstractGrailsTagAwareFlowExecutionTests {
     }
 
     Closure getFlowClosure() {
-        def subberFlow = {
-            subber {
-                on("proceed").to("subberEnd")
-            }
-            subberEnd()
-        }
-
         return {
             start {
                 on("next").to "subber"

grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/webflow/SubflowExecutionWithExternalSubflowTests.groovy

@@ -7,6 +7,7 @@ import org.codehaus.groovy.grails.webflow.support.AbstractGrailsTagAwareFlowExec
 class SubflowExecutionWithExternalSubflowTests extends AbstractGrailsTagAwareFlowExecutionTests {
 
     void testSubFlowExecution() {
+        registerFlow('otherSubflow/subber', new OtherSubflowController().subberFlow)
         startFlow()
         assertCurrentStateEquals "start"
 

grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/webflow/engine/builder/FlowBuilderSubFlowExecutionTests.groovy

@@ -8,10 +8,6 @@ import org.codehaus.groovy.grails.webflow.support.AbstractGrailsTagAwareFlowExec
 class FlowBuilderSubFlowExecutionTests extends AbstractGrailsTagAwareFlowExecutionTests{
 
     def searchMoreAction = { [moreResults:["one", "two", "three"]] }
-
-    Closure getFlowClosure() {
-        def searchService = [executeSearch:{["foo", "bar"]}]
-        def params = [q:"foo"]
         def extendedSearchFlow = {
             startExtendedSearch {
                 on("findMore").to "searchMore"
@@ -30,6 +26,10 @@ class FlowBuilderSubFlowExecutionTests extends AbstractGrailsTagAwareFlowExecuti
             noResults()
         }
 
+    Closure getFlowClosure() {
+        def searchService = [executeSearch:{["foo", "bar"]}]
+        def params = [q:"foo"]
+
         return {
             displaySearchForm {
                 on("submit").to "executeSearch"
@@ -68,6 +68,7 @@ class FlowBuilderSubFlowExecutionTests extends AbstractGrailsTagAwareFlowExecuti
     }
 
     void testSubFlowDefinition() {
+        registerFlow('flowBuilderSubFlowExecutionTests/extendedSearch', extendedSearchFlow)
         grails.util.GrailsWebUtil.bindMockWebRequest()
         def theFlow = getFlowDefinition()
 
@@ -85,6 +86,7 @@ class FlowBuilderSubFlowExecutionTests extends AbstractGrailsTagAwareFlowExecuti
     }
 
     void testSubFlowExecution() {
+        registerFlow('flowBuilderSubFlowExecutionTests/extendedSearch', extendedSearchFlow)
         grails.util.GrailsWebUtil.bindMockWebRequest()
         startFlow()
         assertCurrentStateEquals "displaySearchForm"
@@ -103,6 +105,7 @@ class FlowBuilderSubFlowExecutionTests extends AbstractGrailsTagAwareFlowExecuti
 
     void testSubFlowExecution2() {
         searchMoreAction = { error() }
+        registerFlow('flowBuilderSubFlowExecutionTests/extendedSearch', extendedSearchFlow)
         startFlow()
         assertCurrentStateEquals "displaySearchForm"
 

grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/webflow/support/AbstractGrailsTagAwareFlowExecutionTests.groovy

@@ -48,8 +48,7 @@ abstract class AbstractGrailsTagAwareFlowExecutionTests extends AbstractFlowExec
     ServletContext servletContext
     GrailsWebRequest webRequest
     FlowBuilderServices flowBuilderServices
-    FlowDefinitionLocator definitionLocator = new FlowDefinitionRegistryImpl()
-
+    FlowDefinitionRegistry flowDefinitionRegistry = new FlowDefinitionRegistryImpl()
     MockHttpServletRequest request
     MockHttpServletResponse response
     def ctx
@@ -156,11 +155,17 @@ abstract class AbstractGrailsTagAwareFlowExecutionTests extends AbstractFlowExec
         return context
     }
 
+    FlowDefinition registerFlow(String flowId, Closure flowClosure) {
+        FlowBuilder builder = new FlowBuilder(flowId, flowClosure, flowBuilderServices, getFlowDefinitionRegistry())
+        builder.viewPath = "/"
+        builder.applicationContext = appCtx
+        FlowAssembler assembler = new FlowAssembler(builder, builder.getFlowBuilderContext())
+        getFlowDefinitionRegistry().registerFlowDefinition(new DefaultFlowHolder(assembler))
+        return getFlowDefinitionRegistry().getFlowDefinition(flowId)
+    }
 
     FlowDefinition getFlowDefinition() {
-        FlowBuilder builder = new FlowBuilder(getFlowId(), getFlowBuilderServices(), getDefinitionLocator())
-        builder.applicationContext = appCtx
-        builder.flow(getFlowClosure())
+        return registerFlow(getFlowId(), getFlowClosure())
     }
 
     protected void onInit() {}

grails-webflow/src/main/groovy/grails/test/WebFlowTestCase.groovy

@@ -48,7 +48,11 @@ abstract class WebFlowTestCase extends AbstractFlowExecutionTests {
     protected MockHttpServletRequest mockRequest
     protected MockHttpServletResponse mockResponse
     protected MockServletContext mockServletContext
+    protected FlowDefinitionRegistry flowDefinitionRegistry
     protected ApplicationContext applicationContext
+    protected FlowBuilderServices flowBuilderServices
+    protected MvcViewFactoryCreator viewCreator
+
     /**
      * Subclasses should return the flow closure that within the controller. For example:
      * <code>return new TestController().myFlow</code>.
@@ -77,7 +81,18 @@ abstract class WebFlowTestCase extends AbstractFlowExecutionTests {
             mockServletContext = new MockServletContext()
             RequestContextHolder.setRequestAttributes(new GrailsWebRequest(mockRequest,mockResponse,mockServletContext,applicationContext))
         }
-
+        if (!applicationContext) applicationContext = new GrailsWebApplicationContext()
+        flowDefinitionRegistry = new FlowDefinitionRegistryImpl()
+        flowBuilderServices = new FlowBuilderServices()
+        viewCreator = new MvcViewFactoryCreator()
+        viewCreator.applicationContext = applicationContext
+        def viewResolvers = applicationContext?.getBeansOfType(ViewResolver)
+        if (viewResolvers) {
+            viewCreator.viewResolvers = viewResolvers.values().toList()
+    }
+        flowBuilderServices.viewFactoryCreator = viewCreator
+        flowBuilderServices.conversionService = new DefaultConversionService()
+        flowBuilderServices.expressionParser = DefaultExpressionParserFactory.getExpressionParser()
     }
 
     protected void tearDown() {
@@ -88,28 +103,24 @@ abstract class WebFlowTestCase extends AbstractFlowExecutionTests {
         RequestContextHolder.setRequestAttributes(null)
     }
 
+    FlowDefinition registerFlow(String flowId, Closure flowClosure) {
+        FlowBuilder builder = new FlowBuilder(flowId, flowClosure, flowBuilderServices, flowDefinitionRegistry)
+        builder.viewPath = "/"
+        builder.applicationContext = applicationContext
+        FlowAssembler assembler = new FlowAssembler(builder, builder.getFlowBuilderContext())
+        flowDefinitionRegistry.registerFlowDefinition(new DefaultFlowHolder(assembler))
+        return flowDefinitionRegistry.getFlowDefinition(flowId)
+    }
 
     FlowDefinition getFlowDefinition() {
-        def flowBuilderServices = new FlowBuilderServices()
-
-        MvcViewFactoryCreator viewCreator = new MvcViewFactoryCreator()
-        viewCreator.applicationContext = applicationContext
-        def viewResolvers = applicationContext?.getBeansOfType(ViewResolver)
-        if (viewResolvers) {
-            viewCreator.viewResolvers = viewResolvers.values().toList()
-        }
-        flowBuilderServices.viewFactoryCreator = viewCreator
-        flowBuilderServices.conversionService = new DefaultConversionService()
-        flowBuilderServices.expressionParser = DefaultExpressionParserFactory.getExpressionParser()
-
-        FlowBuilder builder = new FlowBuilder(getFlowId(), flowBuilderServices, new FlowDefinitionRegistryImpl())
         def flow = getFlow()
 
         if (flow instanceof Closure) {
             // delegate will be controller
             GrailsWebRequest.lookup()?.request?.setAttribute(GrailsApplicationAttributes.CONTROLLER, flow.delegate)
-            builder.flow(flow)
         }
+
+        return registerFlow(getFlowId(), flow)
     }
 
     /**

grails-webflow/src/main/groovy/org/codehaus/groovy/grails/webflow/engine/builder/FlowBuilder.groovy

@@ -90,11 +90,13 @@ class FlowBuilder extends AbstractFlowBuilder implements GroovyObject, Applicati
     String viewPath = "/"
 
     protected FlowBuilderServices flowBuilderServices
+    protected FlowDefinitionLocator definitionLocator
 
     FlowBuilder(String flowId, FlowBuilderServices flowBuilderServices, FlowDefinitionLocator definitionLocator) {
         this.flowId = flowId
         Assert.notNull flowBuilderServices, "Argument [flowBuilderServices] is required!"
         this.flowBuilderServices = flowBuilderServices
+        this.definitionLocator = definitionLocator
         this.metaClass = GroovySystem.getMetaClassRegistry().getMetaClass(FlowBuilder.class)
 
         def context = new FlowBuilderContextImpl(flowId,null, definitionLocator, flowBuilderServices)
@@ -180,19 +182,8 @@ class FlowBuilder extends AbstractFlowBuilder implements GroovyObject, Applicati
                         // add action state
                         state = createActionState(name, action, trans, flowFactory, flowInfo.entryAction, flowInfo.exitAction)
                     }
-                    else if (flowInfo.subflow) {
-                        def i = flowId.indexOf('/')
-                        def subflowClass = flowInfo.subflow.getThisObject().getClass()
-                        def controllerName = GrailsNameUtils.getLogicalPropertyName(subflowClass.name, "Controller")
-                        def subflowId = "${controllerName}/$name"
-                        FlowBuilder subFlowBuilder = new FlowBuilder(subflowId, flowBuilderServices,
-                                                                        getContext().getFlowDefinitionLocator())
-
-                        subFlowBuilder.viewPath = this.viewPath
-
-                        Flow subflow = subFlowBuilder.flow(flowInfo.subflow)
-
-                        state = createSubFlow(flowInfo, name, subflow, trans, flowFactory)
+                    else if (flowInfo.subflow || flowInfo.subflowAction) {
+                        state = createSubFlow(flowInfo, flowFactory, name)
                     }
                     else {
                         String view = createViewPath(flowInfo, name)
@@ -296,8 +287,31 @@ class FlowBuilder extends AbstractFlowBuilder implements GroovyObject, Applicati
 
     }
 
-    private State createSubFlow(flowInfo, String stateId, Flow subflow, Transition[] trans, FlowArtifactFactory flowFactory) {
-        return flowFactory.createSubflowState(stateId, getFlow(), null, new StaticExpression(subflow),new GrailsSubflowAttributeMapper(flowInfo.subflowInput),trans, null, null, null)
+    private State createSubFlow(flowInfo, FlowArtifactFactory flowFactory, stateId) {
+        def subflowId
+        if (flowInfo.subflow) {
+            //backwards compatibility: only subflow closure is supplied and the containing state must have the same name as the called subflow
+            def controllerClass = flowInfo.subflow.getThisObject().getClass()
+            def controllerName = GrailsNameUtils.getLogicalPropertyName(controllerClass.name, "Controller")
+            subflowId = "${controllerName}/$stateId"
+    }
+        else {
+            //preferred way: subflow action and optional controller name are supplied
+            def controllerName
+            if (flowInfo.subflowController) {
+                controllerName = flowInfo.subflowController
+            }
+            else {
+                //assume that the subflow is defined in the same controller as the calling flow
+                Class controllerClass = flowClosure.getThisObject().getClass()
+                controllerName = GrailsNameUtils.getLogicalPropertyName(controllerClass.name, "Controller")
+            }
+            subflowId = "${controllerName}/$flowInfo.subflowAction"
+        }
+        Flow subflow = definitionLocator.getFlowDefinition(subflowId)
+
+        return flowFactory.createSubflowState(stateId, getFlow(), null, new StaticExpression(subflow),
+                new GrailsSubflowAttributeMapper(flowInfo.subflowInput), flowInfo.transitions, null, null, null)
     }
 
     private State createActionState(String stateId, Closure action, Transition[] transitions,
@@ -395,6 +409,8 @@ class FlowInfoCapturer {
     private Closure entryAction
     private Closure exitAction
     private Closure subflow
+    private String subflowController
+    private String subflowAction
     private Map subflowInput = [:]
     private String viewName
     private applicationContext
@@ -417,6 +433,8 @@ class FlowInfoCapturer {
     Closure getEntryAction() { this.entryAction}
     Closure getExitAction() { this.exitAction}
     Closure getSubflow() { this.subflow }
+    String getSubflowController() { this.subflowController }
+    String getSubflowAction() { this.subflowAction }
     Map getSubflowInput() { this.subflowInput }
     Mapper getOutputMapper() { this.outputMapper }
 
@@ -459,8 +477,16 @@ class FlowInfoCapturer {
         this.subflow = callable
     }
 
-    void subflow(Map args, Closure callable) {
-        this.subflow = callable
+    void subflow(Map args, Closure callable = null) {
+        if (callable != null) {
+            this.subflow = callable
+        } else {
+            if (!args.action) {
+                throw new FlowDefinitionException("subflow action is mandatory")
+            }
+            this.subflowController = args.controller
+            this.subflowAction = args.action
+        }
         args.input?.each {key, value ->
             if (value instanceof Closure) {
                 this.subflowInput[key] = new ClosureExpression(value)