Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow omit the 'method' attribute when provider method name is same with mapper method #1279

Closed
picc-lu opened this Issue May 15, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@picc-lu
Copy link
Contributor

picc-lu commented May 15, 2018

Example

@SelectProvider(type = SqlProvider.class, method = "listAirSensorsByDuidAndTimeRange")
List<AirSensor> listAirSensorsByDuidAndTimeRange();

class SqlProvider {
public static String listAirSensorsByDuidAndTimeRange() {
// return SQL;
}
}

When method name of mapper's and sqlprovider's is the same (in bold), method attribute can be ignored for easy development.
Typing that every time is pretty annoying.
#1283
Thank you!

@kazuki43zoo

This comment has been minimized.

Copy link
Member

kazuki43zoo commented May 17, 2018

I think this is useful. @harawata WDYT?

@h3adache

This comment has been minimized.

Copy link
Member

h3adache commented May 18, 2018

@kazuki43zoo @harawata this is a good request but I think we can make it even better.
Similar to what we did for constructor injection, we can just pick whatever method matches the provider signature. That would be really a really nice feature cause you can name it as you like without having it string typed in the annotation.

@harawata

This comment has been minimized.

Copy link
Member

harawata commented May 18, 2018

Can this be achieved with the enhancement #1055 ?
i.e. instead of omitting method , you can specify some 'universalMethod' on all methods.

@SelectProvider(type = SqlProvider.class, method = "universalSelect")
List<AirSensor> listAirSensorsByDuidAndTimeRange();

Then in SqlProvider#universalSelect(), you can search the actual method using the method name or signature.

class SqlProvider {
  String universalSelect(ProviderContext context) {
     // look for a method with the same name, etc.
  }
  String listAirSensorsByDuidAndTimeRange() {
     return "select * from ...";
  }
}

I'm not strongly opposed to the change, but there seems to be some overlap in these features.

@picc-lu

This comment has been minimized.

Copy link
Contributor Author

picc-lu commented May 23, 2018

Thank you for your reply! @harawata
I've tried my best to implement universalSelect method as you suggested, but it only works fine with methods without parameters, the methods with parameters has to be dealt with the universalSelect with an extra java.util.Map instance.

Mapper's methods with and without params

@SelectProvider(type = AirSensorSqlProvider.class, method = "universalSelect")
List<AirSensor> listAirSensorsByDuidAndTimeRange(@Param("duid") Long duid,
        @Param("timeStart") Date timeStart, @Param("timeEnd") Date timeEnd);

@SelectProvider(type = AirSensorSqlProvider.class, method = "universalSelectNoParam")
List<AirSensor> listAllAirSensors();

Methods in Sqlprovider

public String universalSelectNoParam(ProviderContext providerContext) {
    String sql = "";
    try {
        Method mapperMethod = providerContext.getMapperMethod();
        Method providerMethod = this.getClass().getMethod(mapperMethod.getName());
        sql = (String) providerMethod.invoke(this);
    } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
        e.printStackTrace();
    }
    return sql;
}

public String universalSelect(ProviderContext providerContext, Map<String, Object> map) {
    String sql = "";
    String keyPrefix = "param";
    Method mapperMethod = providerContext.getMapperMethod();
    Object[] params = new Object[mapperMethod.getParameterCount()];
    for (Map.Entry entry : map.entrySet()) {
        String key = (String) entry.getKey();
        // detect "param1", "param2" and so on, place them in right order
        if (key.startsWith(keyPrefix)) {
            params[Integer.parseInt(key.substring(keyPrefix.length())) - 1] = entry.getValue();
        }
    }
    try {
        Method providerMethod = this.getClass().getMethod(mapperMethod.getName(), mapperMethod.getParameterTypes());
        // invoke method with parameters.
        sql = (String) providerMethod.invoke(this, params);
    } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
        e.printStackTrace();
    }
    return sql;
}

Those two universalSelect and universalSelectNoParam are running well, but I think it is somehow complex like parameters placed in invoke must be in order.
I hope I do all things right.

@harawata

This comment has been minimized.

Copy link
Member

harawata commented May 23, 2018

Thank you, @picc-lu . You made a good point!

Then, how about introducing a new interface like this?

public interface SqlProviderMethodResolver {
  Method resolve(ProviderContext context);
}

If the class specified in type implements this interface, MyBatis uses the method returned from resolve() as the SQL provider.

This way, you may have to write the name matching rule by yourself, but it allows other users to write their own rules.

@h3adache @kazuki43zoo
Please let me know if you guys think it's over-engineered. :D

@kazuki43zoo

This comment has been minimized.

Copy link
Member

kazuki43zoo commented May 23, 2018

@harawata I think it's good 👍
And as option, we may consider that providing a default implementation of rule base on default method.

public interface SqlProviderMethodResolver {
  default Method resolve(ProviderContext context) {
    // providing default implementation?
  }
}

WDYT?

@h3adache

This comment has been minimized.

Copy link
Member

h3adache commented May 23, 2018

I think it's a potentially good change but what I meant was similar to #1055 but having a simple "provide" (or "provideSql"?) as the default method name instead of doing any lookup for name based on classname or anything more complicated. Too simple? Am I missing anything?

@harawata

This comment has been minimized.

Copy link
Member

harawata commented May 25, 2018

@h3adache ,
I'm sorry, I couldn't follow. 😅
Could you post some code snippets to help me understand the idea?

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this issue Mar 21, 2019

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this issue Mar 21, 2019

@kazuki43zoo

This comment has been minimized.

Copy link
Member

kazuki43zoo commented Mar 21, 2019

@harawata @h3adache @picc-lu

I've tried to fix this. Please check #1499! Thanks.

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this issue Mar 21, 2019

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this issue Mar 24, 2019

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this issue Mar 24, 2019

kazuki43zoo added a commit that referenced this issue Mar 24, 2019

Merge pull request #1499 from kazuki43zoo/gh-1279
Allow omit a 'method' attribute on SqlProvider annotation

@kazuki43zoo kazuki43zoo self-assigned this Mar 24, 2019

@kazuki43zoo kazuki43zoo added this to the 3.5.1 milestone Mar 24, 2019

@kazuki43zoo kazuki43zoo changed the title [Add] apply method name of mapper's when it is the same as sqlprovider's when using @XXXProvider Allow omit the 'method' attribute when provider method name is same with mapper method Mar 25, 2019

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this issue Apr 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.