Grails Controller Web Binding does not work with Serializable or Generic Binding Objects

[codeconsole]

The following (understandably) does not work, but a mechanism should be provided to facilitate binding

class MyController<T>  {

    def get(Serializable id) { // given a request to /my/1234
         assert(id, null) // true
         assert(params.id, != null) // true 
         assert(params.long('id'), != null) // true
    }

    def save(T instance)  {
        assert(instance, null)
    } 
}

ControllerActionTransformer.java

accepts a parameter of type [java.io.Serializable]. Interface types and abstract class types are not supported as command objects. This parameter will be ignored.

grails-core/grails-plugin-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java

Lines 844 to 856 in 8871703

	protected void initializeCommandObjectParameter(final BlockStatement wrapper,
	final ClassNode commandObjectNode, final String paramName, SourceUnit source) {
	final ArgumentListExpression initializeCommandObjectArguments = new ArgumentListExpression();
	initializeCommandObjectArguments.addExpression(new ClassExpression(commandObjectNode));
	initializeCommandObjectArguments.addExpression(new ConstantExpression(paramName));
	final MethodCallExpression initializeCommandObjectMethodCall = new MethodCallExpression(new VariableExpression("this"), "initializeCommandObject", initializeCommandObjectArguments);
	applyDefaultMethodTarget(initializeCommandObjectMethodCall, commandObjectNode);
	final Expression assignCommandObjectToParameter = new BinaryExpression(new VariableExpression(paramName), Token.newSymbol(Types.EQUALS, 0, 0), initializeCommandObjectMethodCall);
	wrapper.addStatement(new ExpressionStatement(assignCommandObjectToParameter));
	}

grails-core/grails-plugin-controllers/src/main/groovy/grails/artefact/Controller.groovy

Lines 363 to 460 in 9ea9c60

	def initializeCommandObject(final Class type, final String commandObjectParameterName) throws Exception {
	final HttpServletRequest request = getRequest()
	def commandObjectInstance = null
	try {
	final DataBindingSource dataBindingSource = DataBindingUtils
	.createDataBindingSource(
	getGrailsApplication(), type,
	request)
	final DataBindingSource commandObjectBindingSource = getCommandObjectBindingSourceForPrefix(
	commandObjectParameterName, dataBindingSource)
	def entityIdentifierValue = null
	final boolean isDomainClass
	if(GroovyObject.isAssignableFrom(type)) {
	isDomainClass = DomainClass.isAssignableFrom(type)
	} else {
	isDomainClass = DomainClassArtefactHandler
	.isDomainClass(type)
	}
	if (isDomainClass) {
	entityIdentifierValue = commandObjectBindingSource
	.getIdentifierValue()
	if (entityIdentifierValue == null) {
	final GrailsWebRequest webRequest = GrailsWebRequest
	.lookup(request)
	entityIdentifierValue = webRequest?.getParams().getIdentifier()
	}
	}
	if (entityIdentifierValue instanceof String) {
	entityIdentifierValue = ((String) entityIdentifierValue).trim()
	if ("".equals(entityIdentifierValue)
	|| "null".equals(entityIdentifierValue)) {
	entityIdentifierValue = null
	}
	}
	final HttpMethod requestMethod = HttpMethod.valueOf(request.getMethod())
	if (entityIdentifierValue != null) {
	try {
	commandObjectInstance = InvokerHelper.invokeStaticMethod(type, "get", entityIdentifierValue)
	} catch (Exception e) {
	final Errors errors = getErrors()
	if (errors != null) {
	errors.reject(getClass().getName()
	+ ".commandObject."
	+ commandObjectParameterName + ".error",
	e.getMessage())
	}
	}
	} else if (requestMethod == HttpMethod.POST || !isDomainClass) {
	commandObjectInstance = type.newInstance()
	}
	if (commandObjectInstance != null
	&& commandObjectBindingSource != null) {
	boolean shouldDoDataBinding
	if (entityIdentifierValue != null) {
	switch (requestMethod) {
	case HttpMethod.PATCH:
	case HttpMethod.POST:
	case HttpMethod.PUT:
	shouldDoDataBinding = true
	break
	default:
	shouldDoDataBinding = false
	}
	} else {
	shouldDoDataBinding = true
	}
	if (shouldDoDataBinding) {
	bindData(commandObjectInstance, commandObjectBindingSource, Collections.EMPTY_MAP, null)
	}
	}
	} catch (Exception e) {
	final exceptionHandlerMethodFor = getExceptionHandlerMethodFor(e.getClass())
	if(exceptionHandlerMethodFor != null) {
	throw e
	}
	commandObjectInstance = type.newInstance()
	final o = GrailsMetaClassUtils.invokeMethodIfExists(commandObjectInstance, "getErrors")
	if(o instanceof BindingResult) {
	final BindingResult errors = (BindingResult)o
	String msg = "Error occurred initializing command object [" + commandObjectParameterName + "]. " + e.getMessage()
	ObjectError error = new ObjectError(commandObjectParameterName, msg)
	errors.addError(error)
	}
	}
	if(commandObjectInstance != null) {
	final ApplicationContext applicationContext = getApplicationContext()
	final AutowireCapableBeanFactory autowireCapableBeanFactory = applicationContext.getAutowireCapableBeanFactory()
	autowireCapableBeanFactory.autowireBeanProperties(commandObjectInstance, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, false)
	}
	commandObjectInstance
	}

Solution Proposal - @Bind annotation

@Bind(Sample) 
class MyController extends GenericController<Sample> {
    SampleController() {
        super(Sample)
    }
}

class GenericController<T>  {
    GenericController(Class<T> domainClass) {
        this.domainClass = domainClass
    }

    def get(Serializable id) { // given a request to /my/1234
         assert(id, !null) // true
         respond domainClass.get(id)
    }

    def save(T instance)  {
        assert(instance.class, domainClass) // true
        domainClass.save instance, flush: true
        respond instance
    } 

    def update(T instance, @Bind(skip=true) Map model) {
         // model is null but this is useful for inheritance where if
         // this class is extended and super.update(instance, [message:'Hello world']) is called
         // this method could do the update and return the model.
         domainClass.save instance, flush: true
         respond instance, model: (model?:[message:'Instance updated successfully'])
     }
}

additionally the following will work the same and allow generic and serializable binding

@Scaffold(value = GenericController, domain = User)
class UserController {
     def update(User user) {
         super.update(user, [message: 'User updated'])
     }
}

@Bind should

1. skip=true allow suppression of controller warning messages.
2. enable binding of Serializable id to the same type of the id of the domain object.
3. enable binding of generic types to the specified object.
4. allow annotating of other annotations where it uses the domain attribute or generic attribute of that attribute.
   For instance annotation @Scaffold annotation definition with @Bind will allow @Scaffold(domain=Sample) to also function as @Bind and only require 1 attribute.

[matrei]

There is already a grails.databinding.BindUsing annotation. Would it be possible/beneficial to add this functionality to that annotation instead of creating a new one, for this 'edgy' case?

For example:

@BindUsing(domain = Sample)

[codeconsole]

@matrei @BindUsing serves a completely different purpose. The annotation defined here is for instructing an @AstTransformer specifically on Controllers at compile time. I know this sounds edgy, but it actually has a lot of potential to dramatically promote code reuse.

[matrei]

The annotation defined here is for instructing an @AstTransformer specifically on Controllers at compile time.

Aha, I see.

I know this sounds edgy, but it actually has a lot of potential to dramatically promote code reuse.

I don't doubt it.
I might be missing something from the bigger picture, but it is primarily to be used for scaffolding, right?
And not something the average developer would use in his own application code.

@Bind is kind of generic. Maybe @BindDomain or @BindType to not hog the @Bind annotation name.

[codeconsole]

@matrei actually this could be used for any sort of controller inheritance and is not specific to scaffolding. It also provides a mechanism to squash warnings from ControllerActionTransformer.java. The main point behind this is to further eliminate the need to repeat common code functionality across controllers. This tag would only be used by ControllerActionTransformer which adds code to each controller to allow it to bind request attributes to method parameters. Without this, you have to bind manually in situations where you want to share controller logic.

This is what RestfulContoller does.

[matrei]

actually this could be used for any sort of controller inheritance and is not specific to scaffolding

Ok, good info!

The skip parameter name is a bit hard to decipher.
Is it this warning from AST processing that will be skipped:

grails-core/grails-plugin-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java

Lines 760 to 766 in 8871703

	if(commandObjectNode.isInterface() || Modifier.isAbstract(commandObjectNode.getModifiers())) {
	final String warningMessage = "The [" + actionName + "] action in [" +
	controllerNode.getName() + "] accepts a parameter of type [" +
	commandObjectNode.getName() +
	"]. Interface types and abstract class types are not supported as command objects. This parameter will be ignored.";
	GrailsASTUtils.warning(source, actionNode, warningMessage);
	} else {

[codeconsole]

That is 1 case. It will forcefully ignore binding with the annotation and also would suppress warning messages like you see there.