Skip to content

Add new method for propagating mapper info on LanguageDriver #1492

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

Closed

Conversation

kazuki43zoo
Copy link
Member

By this changes, scripting language plugin (mybatis-freemarker, mybatis-velocity and mybatis-thymelaf) can loaded a template file automatically by rule-based if SQL is not specify on XML or annotation.

for example:

package com.example.demo.mapper;

interface DemoMapper {
  @Insert("")
  @SelectKey(statement = "", ...)
  void insert(Demo demo);
}

The scripting language plugin (e.g. mybatis-thymeleaf) can be load a template files (e.g. com/example/demo/mapper/DemoMapper/insert.sql or com/example/demo/mapper/DemoMapper/insert_selectKey.sql) automatically.

WDYT?

By this changes, scripting language plugin (mybatis-freemarker, mybatis-velocity and mybatis-thymelaf) can loaded a template file automatically by rule-based if SQL is not specify on XML or annotation.
@kazuki43zoo
Copy link
Member Author

@harawata

WDYT? If you can approve this PR, I want to add feature that load a template file automatically at first release of mybatis-thymeleaf.

@harawata
Copy link
Member

harawata commented Mar 8, 2019

I think I understand what you want, but it does not seem right for LanguageDriver implementations to have a logic that depends on statement ID (i.e. LanguageDriver should not need statement ID).

FWIW, it would be possible to allow users to write some LanguageDriver-specific syntax.

interface DemoMapper {
  @Insert("file:example.sqlfiles.DemoMapper_Insert.sql")
  @SelectKey(statement = "file:example.sqlfiles.DemoMapper_SelectKey.sql", ...)
  void insert(Demo demo);
}

@kazuki43zoo
Copy link
Member Author

@harawata thanks for your review comment!

I think I understand what you want, but it does not seem right for LanguageDriver implementations to have a logic that depends on statement ID

I understand your opinion.
However, I think that propagating SQL definition source information(e.g. method location, XML location and element information) as meta data is not wrong solution. WDYT?
If there is no room for consideration, I will withdraw this PR.

FWIW, it would be possible to allow users to write some LanguageDriver-specific syntax.

yes! it support already as follow:

@Select("/NameMapper/findById.sql")
Name findById(@Param("id") Integer id);

@harawata
Copy link
Member

harawata commented Mar 9, 2019

Passing information about SQL source breaks separation of concerns, IMHO.
Those informations are used to load a SQL script from the source and that is not a LanguageDriver's task.
LanguageDriver's task is to process an already loaded SQL script and I think it's better to keep this separation.

To load SQL based on a mapper method, we already have SQL provider.
So, this might be related to #1391 .

@kazuki43zoo
Copy link
Member Author

@harawata Thanks for your comment.

I understand and agree with your opinion.

@kazuki43zoo kazuki43zoo closed this Mar 9, 2019
@kazuki43zoo kazuki43zoo deleted the add-method-on-languagedriver branch March 9, 2019 12:09
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.

2 participants