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

Support java.util.Optional as return type of mapper method #799

Merged
merged 7 commits into from Mar 16, 2018

Conversation

Projects
None yet
4 participants
@kazuki43zoo
Copy link
Member

kazuki43zoo commented Oct 8, 2016

I know this fix is ad hoc. However, i want to provide early this feature for Java 8 users. I will improve this feature at 3.5 or later version.

This fix allow following method definition:

public interface UserMapper {

  @Select("select * from users where id = #{id}")
  Optional<User> findOne(Integer id);

}

Please review.

@kazuki43zoo kazuki43zoo force-pushed the kazuki43zoo:support-optional-mapper-method branch from 4eca279 to d4099b7 Oct 8, 2016

@kazuki43zoo kazuki43zoo changed the title Support java.util.Optionl as return type of mapper method Support java.util.Optional as return type of mapper method Oct 8, 2016

@kazuki43zoo kazuki43zoo force-pushed the kazuki43zoo:support-optional-mapper-method branch from d4099b7 to 452ba6b Oct 8, 2016

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Oct 10, 2016

Hi,

I wrote a patch to support Optional, a while ago.
Here's the branch...
https://github.com/harawata/mybatis-3/commits/optional
... and test cases.
https://github.com/harawata/mybatis-optional-test

I consider it is still under the committers' review (@eddumelendez seemed to like it), but I would appreciate if you could check it out and let me know what you think.

Thank you,
Iwao

@kazuki43zoo

This comment has been minimized.

Copy link
Member Author

kazuki43zoo commented Oct 10, 2016

Hi @harawata, it's great !! I will confirm your patch at later and leave comments if needed.

Thanks !

Merge branch 'master' into pr/kazuki43zoo-gh-799
# Conflicts:
#	src/main/java/org/apache/ibatis/binding/MapperMethod.java
@harawata

This comment has been minimized.

Copy link
Member

harawata commented Jan 11, 2018

Hi @kazuki43zoo ,

Do you mind if I add some commits to this PR?

@harawata harawata reopened this Jan 11, 2018

harawata added some commits Jan 15, 2018

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Jan 15, 2018

@kazuki43zoo , I have added a few commits (hope it was OK!).

@jeffgbutler @christianpoitras
It is very simple, but satisfies the minimum requirement we discussed on #228 , I think.
Please take a look when you have time.

To reduce the noise, it's better to add w=1 when reviewing the diff.
https://github.com/mybatis/mybatis-3/pull/799/files?w=1

Revert "Use clearer condition."
This reverts commit 9ce0fcd.
@kazuki43zoo

This comment has been minimized.

Copy link
Member Author

kazuki43zoo commented Jan 16, 2018

@harawata Thanks for your improvements. I feel your changes are good!

One more minor simplification. getActualTypeArguments() always return…
…s an array with one element here, I think.
@alexhilman

This comment has been minimized.

Copy link

alexhilman commented Mar 1, 2018

Hello, I'm curious: is there a timeline for the acceptance and release of this pull request?

@christianpoitras

This comment has been minimized.

Copy link
Member

christianpoitras commented Mar 7, 2018

All changes seem fine. @harawata, you can merge the pull request.

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Mar 7, 2018

Thanks @christianpoitras !
I will merge it as soon as we release 3.4.6 .

@alexhilman

This comment has been minimized.

Copy link

alexhilman commented Mar 7, 2018

Thanks @christianpoitras and @harawata - I am looking forward to using it.

@harawata harawata added this to the 3.5.0 milestone Mar 16, 2018

@harawata harawata merged commit c92b65a into mybatis:master Mar 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

kazuki43zoo added a commit that referenced this pull request Mar 16, 2018

@kazuki43zoo kazuki43zoo deleted the kazuki43zoo:support-optional-mapper-method branch Mar 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment