Advice on Plugin architecture


(thirsch81) #1

Hello, I have these three classes, which I want to use as a plugin. Would you approve the way I made use of the “unnamed extension” to add utility methods to the project? I intend this to mainly serve as a utility plugin, where, in my oppinion, an explicitly named extension would not really make sense Please share any advice you might have.

See Gist here.

Cheers,

Thomas


(Peter Niederwieser) #2

The recommended best practice is that plugins don’t make use of extra properties. Extra properties aren’t statically known, so no documentation can be generated for them, and IDEs won’t be able to offer code assistance for them. Also there’s a risk of name collisions.


(thirsch81) #3

So would you refactor it like this?

class PropertiesPlugin implements Plugin {
    @Override
    public void apply(Object project) {
        project.extensions.create("configUtils", ConfigUtilsExtension, project)
        project.ext {
     ConfigureProperties = my.company.gradle.plugin.tasks.ConfigurePropertiesTask
        }
    }
}

And then use the extension from the custom task like this?

@TaskAction
    void configurePropertiesFile() {
        ConfigUtilsExtension utils = project.extensions.findByType(ConfigUtilsExtension)
        ConfigObject config = utils.slurpConfig source, sourcePrefix
        // do stuff
    }

Again, the basic idea is to provide some utility methods to be reused in ad-hoc tasks of any type, as well as premade CustomTask Types, and also from inside other plugins, that would depend on this plugin.

On a sidenote what do you think about this method:

def getParam(name, defaultValue = null) {
        (project.hasProperty(name) && project.getProperty(name) != "") ? project.getProperty(name) : defaultValue
    }

Is there a general misconception when I have to resort to stuff like that, as in relying too much on properties passed via the command line?

Thanks for helping!


(Peter Niederwieser) #4

Extensions are certainly a better choice than extra properties (for a plugin). But as stated previously, I’d first question if the utility methods really need to be exposed to build scripts, and I’d also question the use of ‘ConfigSlurper’ (I’d rather encourage use of Gradle’s own configuration language than embedding another configuration language). Same goes for making everything configurable from the command line.


(Robert Fischer) #5

I’ve written that “getParam” method before once or twice or more, especially since (IIRC), “getProperty” on a nonexistent property explodes unhelpfully.

One Groovyism: the empty string is false (http://groovyconsole.appspot.com/script/1101002), so there’s no need to explicitly check it. Also, you can use the Elvis operator for short-circuiting:

def getParam(name, defaultValue=null) {
  if(!project.hasProperty(name)) return defaultValue
  return project."$name" ?: defaultValue
}

That’s more code style than anything else, but I thought you’d like to know.

(For the truly pedantic, it is true that there is one slight implementation nuance difference – to see it, assume the property is set to the boolean value ‘false’ but the defaultValue passed in is the boolean value ‘true’.)


(Robert Fischer) #6

More to the point of the original post – your refactored code is better, because you’re reducing the pollution of the variable namespace. You could hook onto the project as “project.configUtils” or something like that, just to make life easier for the caller, but I wouldn’t shove too much to the top level.