Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class GrailsExtension {
this.project = project
this.pluginDefiner = new PluginDefiner(project)
this.indy = project.objects.property(Boolean).convention(false)
this.preserveParameterNames = project.objects.property(Boolean).convention(true)
}

/**
Expand Down Expand Up @@ -106,6 +107,15 @@ class GrailsExtension {
this.indy.set(enabled)
}

/**
* Keep class file parameter names so autowire by name can be supported without additional annotations such as '@qualifier'.
*/
final Property<Boolean> preserveParameterNames

void setPreserveParameterNames(boolean enabled) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been removed and replaced with simple field variable, which defaults to true.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still seeing this exist in the latest code base. The groovy dsl in gradle supports transparently calling the .set() on the preserveParameterNames property - that's why this isn't needed. For any java code / non-dsl code, we'll have to explicitly call .set()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdaugherty — You were right that the setter is unnecessary. The root cause was how the extension was being registered. The extension was registered using extensions.add('grails', new GrailsExtension(project)), passing a ready made extension skipping gradles handling, I have switched to extensions.create('grails', GrailsExtension, project) this would let ObjectFactory take care of initialization. I have removed the setter as well.

this.preserveParameterNames.set(enabled)
}

DependencyHandler getPlugins() {
if (pluginDefiner == null) {
pluginDefiner = new PluginDefiner(project)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,17 @@ class GrailsGradlePlugin implements Plugin<Project> {

// Configure indy and log status after evaluation so user's grails { } block has been applied
project.afterEvaluate {
boolean indyEnabled = grailsExtension?.indy?.getOrElse(false) ?: false
boolean indyEnabled = grailsExtension.indy?.getOrElse(false) ?: false
boolean preserveParameterNames = grailsExtension.preserveParameterNames?.getOrElse(true)

project.tasks.withType(GroovyCompile).configureEach { GroovyCompile c ->
c.groovyOptions.optimizationOptions.indy = indyEnabled

if (preserveParameterNames) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't think this is right. There should be 3 cases:
null => do not set anything
false => set parameters to false
true => set parameters to true

Default should be "true"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdaugherty - This is done, please check

c.groovyOptions.parameters = preserveParameterNames
}
}

if (!indyEnabled) {
project.logger.info('Grails: Groovy invokedynamic (indy) is disabled to improve performance (see issue #15293).')
project.logger.info(' To enable invokedynamic: grails { indy = true } in build.gradle')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.grails.gradle.plugin.core

class GrailsGradlePreserveParametersSpec extends GradleSpecification {

def "Grails extension is created with default preserveParameterNames = true"() {
given:
setupTestResourceProject('preserve-params-default')

when:
def result = executeTask('inspectPreserveParam')

then:
result.output.contains("HAS_PRESERVE_PARAM_ENABLED=true")
}

def "preserveParameterNames can be configured to false via grails block"() {
given:
setupTestResourceProject('preserve-params-disabled')

when:
def result = executeTask('inspectPreserveParam')

then:
result.output.contains("HAS_PRESERVE_PARAM_ENABLED=false")
}

def "GroovyCompile tasks get parameters = true when preserveParameterNames is enabled"() {
given:
setupTestResourceProject('preserve-params-enabled')

when:
def result = executeTask('inspectPreserveParam')

then:
result.output.contains("HAS_PRESERVE_PARAM_ENABLED=true")
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
plugins {
id 'org.apache.grails.gradle.grails-app'
}

tasks.register('inspectPreserveParam') {
doLast {
def compileTasks = tasks.withType(GroovyCompile)
def paramsEnabled = compileTasks.every { it.groovyOptions.parameters }
println "HAS_PRESERVE_PARAM_ENABLED=${paramsEnabled}"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
grailsVersion=__PROJECT_VERSION__
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rootProject.name = 'test-preserve-params-default'
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
plugins {
id 'org.apache.grails.gradle.grails-app'
}

grails {
preserveParameterNames = false
}

tasks.register('inspectPreserveParam') {
doLast {
def compileTasks = tasks.withType(GroovyCompile)
def paramsEnabled = compileTasks.every { it.groovyOptions.parameters }
println "HAS_PRESERVE_PARAM_ENABLED=${paramsEnabled}"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
grailsVersion=__PROJECT_VERSION__
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rootProject.name = 'test-preserve-params-disabled'
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
plugins {
id 'org.apache.grails.gradle.grails-app'
}

grails {
preserveParameterNames = true
}

tasks.register('inspectPreserveParam') {
doLast {
def compileTasks = tasks.withType(GroovyCompile)
def paramsEnabled = compileTasks.every { it.groovyOptions.parameters }
println "HAS_PRESERVE_PARAM_ENABLED=${paramsEnabled}"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
grailsVersion=__PROJECT_VERSION__
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rootProject.name = 'test-preserve-params-enabled'