Add the @Options#timeoutString

[kazuki43zoo]

I propose to add a new attribute that specify the query timeout using configuration's variables such as ${timeout.query1}.
There are cases that a timeout value want to specify other value per runtime environment.

@Select("SELECT 1")
@Options(timeoutString = "${timeout.query1}")
int select();

timeout.query1=30

For example for using on MyBatis Spring Boot:

mybatis:
  configuration:
    variables:
      timeout:
        query1: 30

NOTE

Already XML mapper can use the configuration's variables as follow:

<select id="select" resultType="int" timeout="${timeout.query1}">
  SELECT 1
</select>

[coveralls]

coverage: 87.557% (+0.01%) from 87.545% when pulling 6dd9c12 on kazuki43zoo:support-timeoutString-on-Options into c060395 on mybatis:master.

[hazendaz]

@kazuki43zoo Just a question on this. Historically, it was suggested to check contains before getting it for some performance reason. Has that changed? I have seen recently less of doing it via containsKey.

[kazuki43zoo]

@hazendaz Thanks for your review comment!
Your comment right!!! I was tried getting Benchmark. As result, processing using with containsKey was high performance rather than without containsKey. Therefore, I reverted my changes via c8ff596.

[guoweixin]

学习了，看到了

[hazendaz]

@kazuki43zoo Is there any reason 'timeoutString' has to be a string instead of just an int? Other than obvious to match the XML, I'm not so certain that is necessary never mind that the XML already is 'timeout' so it more naturally would be 'timeout' to match. I realize under the hood 'timeout' is there but thinking it would be cleaner to just be 'timeout' as int.

[kazuki43zoo]

@kazuki43zoo Is there any reason 'timeoutString' has to be a string instead of just an int? Other than obvious to match the XML, I'm not so certain that is necessary never mind that the XML already is 'timeout' so it more naturally would be 'timeout' to match. I realize under the hood 'timeout' is there but thinking it would be cleaner to just be 'timeout' as int.

@hazendaz The 'timeout' as int already exists on @Options. Sometimes I want to configure a timeout using externalize configuration (e.g. k8s's ConfigMap, environmental variables, system properties, etc... ). For instance, to have a quick way to increase the timeout on a critical business operation without the code changes and keep business running. But if timeout define as int, we need build and release a new application for changing timeout. Therefore, I propose to add the 'timeoutString' option for improving it.

P.S.
I inspire with the @Transactional of Spring Framework.

[hazendaz]

Thanks for explanation! All good with this now. Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Kazuki Shimizu ***@***.***> Sent: Saturday, June 10, 2023 6:25:58 PM To: mybatis/mybatis-3 ***@***.***> Cc: Jeremy Landis ***@***.***>; Mention ***@***.***> Subject: Re: [mybatis/mybatis-3] Add the @options#timeoutString (PR #2886) @kazuki43zoo<https://github.com/kazuki43zoo> Is there any reason 'timeoutString' has to be a string instead of just an int? Other than obvious to match the XML, I'm not so certain that is necessary never mind that the XML already is 'timeout' so it more naturally would be 'timeout' to match. I realize under the hood 'timeout' is there but thinking it would be cleaner to just be 'timeout' as int. @hazendaz<https://github.com/hazendaz> The 'timeout' as int<https://github.com/mybatis/mybatis-3/blob/master/src/main/java/org/apache/ibatis/annotations/Options.java#L100> already exists on @options. Sometimes I want to configure a timeout using externalize configuration (e.g. k8s's ConfigMap, environmental variables, system properties, etc... ). For instance, to have a quick way to increase the timeout on a critical business operation without the code changes and keep business running. But if timeout define as int, we need build and release a new application for changing timeout. Therefore, I propose the 'timeoutString' option to improve it. P.S. I inspire the @transactional<https://github.com/spring-projects/spring-framework/blob/main/spring-tx/src/main/java/org/springframework/transaction/annotation/Transactional.java#L178-L199> of Spring Framework. — Reply to this email directly, view it on GitHub<#2886 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAHODI4HVPJAXYSA754PO33XKTX7NANCNFSM6AAAAAAZBQTFCE>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[kazuki43zoo]

@hazendaz Thanks for approving! @harawata If you agree with this changes, I will merge this.

[harawata]

Already XML mapper can use the configuration's variables as follow:

True, but I think this is not the designed behavior.
In other words, it works by accident and we should not promote this usage as a supported feature.

If we were to support this, we should add a new option (e.g. timeoutProperty) to both XML statement tags and @Options.
And the value of the option should be a key string i.e. timeoutProperty="timeout.select", not timeoutProperty="${timeout.select}".