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

MapperFactoryBean should return runtime type of mapper, if possible. #23

Closed
wants to merge 1 commit into from

Conversation

FrantaM
Copy link

@FrantaM FrantaM commented Nov 22, 2013

So, reason: We use custom MapperProxyFactory and add several interfaces to created proxies. But we can't query spring to return such instances, because it checks only agains the type returned by FactoryBean#getObjectType.

@emacarron
Copy link
Member

Looks a bit like a hack Franta. During the spring startup that method is called 4 times. With your change it returns 2 different things depending on the phase of the startup that is calling it.

But, why searching for proxies? There can be other proxies that are not mappers. Wouldn't it be better to use a marker interface of an annotation?

@FrantaM
Copy link
Author

FrantaM commented Nov 22, 2013

It may return different types, but the type is always the type of the returned object.
It's definitely not a hack. Just a couple of examples from spring itself:

The test is meant to only demonstrate the issue.
Our use case is following (I did write it in short above): We have custom MapperProxyFactory (more precisely MapperRegistry, but that doesn't matter) and are creating javassist proxies with superclasses (remember http://code.google.com/p/mybatis/issues/detail?id=645? :-) ). These superclasses can implement additional interfaces (say IFooService). But we can't do springContext.getBean(IFooService.class) (it does not return anything), because the type of the bean is IFooMapper - even though the runtime type is FooServiceImpl implements IFooMapper, IFooService.

@emacarron
Copy link
Member

Hi again Franta. Thanks for the explanation, I wasn't able to understand the motivation behind the change and now I am.

I honestly think that this cannot be used by anyone but someone who overwrote the MapperRegistry and made mybatis return something that is not a JDK proxy. I would bet there is not many people doing that. ;)

When using standard MyBatis that mappers are just interfaces and proxies, nothing more. With the change (and the standard core) we just enable searching for proxies and that does not make any sense.

I would say that this is a change you should add to your mybatis customized version but should not go in the standard distribution.

@FrantaM
Copy link
Author

FrantaM commented Nov 22, 2013

Well I still think that returning the runtime type is quite common and reasonable regardless of motivation :)

@FrantaM
Copy link
Author

FrantaM commented Nov 22, 2013

btw this is one of many tiny things that make mybatis hard to extend for our needs. Yes, I do agree we have very specific needs but we are making a flight booking system after all :-). I don't think anyone will notice this change but someone like us may appreciate it in the future.

@hazendaz
Copy link
Member

@FrantaM Should this request be closed?

@FrantaM
Copy link
Author

FrantaM commented May 31, 2015

Closing in favor or already merged pr #40 (#70)

@FrantaM FrantaM closed this May 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants