Allow omit a 'method' attribute on SqlProvider annotation

[kazuki43zoo]

Fixes gh-1279

I've tried to fix gh-1279. In this change, a developer can omit the method attribute on SqlProvider annotation (such as @SelectProvider, etc...).

If the method attribute was omitted, the MyBatis try to resolve a target method as follow:

• If specified type is implements ProviderMethodResolver added by this PR, the MyBatis use a method that returned by it

• If cannot resolve by ProviderMethodResolver(= not implement or it was returned null), the MyBatis will search a provideSql method(= reserved fallback method) from specified type.

If cannot resolve a target method by above processing, the MyBatis throw a BuilderException.

NOTE:
For this change, I add the ProviderMethodResolver interface and default implementation. In default implementation, it return a method that method name is matched with mapper method. If not found or found multiple methods, it throw a BuilderException.

WDYT?

Related Issues or PRs

• Enhance SqlProvider behaviour #1283

[h3adache]

LGTM. I would like to see if @abel533 and @harawata thinks as this seems to make generic crud operations very easy to support

[abel533]

This PR will make it easier to use ProviderSqlSource normally. I use a special way to make generic crud operations . In my case, method will point to a fixed number of methods. These methods will generate special SQL based on the mapperMethod name. One of them is as follows

@SelectProvider(type = JPAQueryMethodProvider.class, method = "queryMethod")
User findByLastnameAndFirstname(@Param("lastname") String lastname, @Param("firstname")String firstname);

@SelectProvider(type = JPAQueryMethodProvider.class, method = "queryMethod")
User findByFirstnameLike(@Param("firstname")String firstname);

[abel533]

The common basic methods are as follows:

public interface SelectAllMapper<T> {

    @SelectProvider(type = BaseSelectProvider.class, method = "dynamicSQL")
    List<T> selectAll();
}

public interface SelectMapper<T> {

    @SelectProvider(type = BaseSelectProvider.class, method = "dynamicSQL")
    List<T> select(T record);

}

public UserMapper extends SelectAllMapper<User>, SelectMapper<User> {

}

[harawata]

Thank you, @kazuki43zoo !

I didn't give much thought when I proposed the resolver interface, but it should be possible to define the resolver method as 'static'.
Wouldn't you agree?

Then we might have to consider searching the method by its signature instead.

• Name : resolveMethod
• Parameter : ProviderContext
• Return : Method
• static (should this be optional?)

Or, is there any cleaner idea?

[harawata]

By the way, I thought about the original request after a while and it looked a little bit unusual.
It assumes that there are 1) many mapper methods and 2) each of them is mapped to a different provider method.

With the common/generic mapper strategy, as @abel533 explained, the base mapper contains the provider annotations, so users don't have to write method names manually in most cases.

So, it's useful only when defining SQL providers manually.
And I'm not sure if there is a good reason to write such many SQL providers because writing SQL directly in annotation or XML is easier in most cases.

[kazuki43zoo]

So, it's useful only when defining SQL providers manually.

I think so, too.

And I'm not sure if there is a good reason to write such many SQL providers because writing SQL directly in annotation or XML is easier in most cases.

I think that there are developers that prefer to write a dynamic SQL using provider method instead of XML because the Java code can be debug or measure coverage easily rather than XML.

[kazuki43zoo]

Hi @harawata , Thank you for you comments.

but it should be possible to define the resolver method as 'static'.

I think it is clearer to use an interface for indicating an API specification, what is the reason (advantage) to not use an interface?
In my opinion, supporting 'static' can be contribute to reducing the number of instances generated, but I'm not sure that it will be an advantage because it created at start-up time only.

[harawata]

Added a few comments.

[harawata]

It might be better to check the return type later to throw clearer error message e.g. 'the return type of the method must be CharSequence or its subclass'.

[kazuki43zoo]

Fix via 877284a

[harawata]

resolveSql -> provideSql ?

[kazuki43zoo]

Fix via b1cfb84

[kazuki43zoo]

@harawata

Thanks for your review!
I've fixed your review comments.

[harawata]

Thanks for the update!
It looks good to me. :)

(the below is the comment I was going to post before submitted the review)

I think that there are developers that prefer to write a dynamic SQL using provider method instead of XML because the Java code can be debug or measure coverage easily rather than XML.

I guess you are right. It should be the developer's choice.

I think it is clearer to use an interface for indicating an API specification, what is the reason (advantage) to not use an interface?
In my opinion, supporting 'static' can be contribute to reducing the number of instances generated, but I'm not sure that it will be an advantage because it created at start-up time only.

Okay. Let's go with the interface.
If it causes a memory problem, it would be possible to cache the instances.