add properties checkMapperMethodRelatedMappedStatementExist to find bug as soon as possible, false as default value ,when setting true ,this will check mapper method related mappedStatement whether exist when application startup ,exception will be thrown when related mappedStatement not exist.
Comment From: harawata
Hi @yangjianzhou ,
I have added two commits.
The first commits shows some false positive cases. BTW, false-positive means that an error is reported even though there is no error. False positive decreases the usefulness of validation. (and please do not try to fix this. I still would not accept it because of the next point)
In the second commit, I added a simple test case that calls updateByName method and it fails (obviously).
My point is that this mistake (i.e. forget to define SQL for a method) is quite easy to detect and it is not worth complicate the internals for it.
Hope you understand my explanation.
p.s. There are many unrelated lines in the config file, you should always keep every file minimum. Unrelated lines waste reviewers' time.
Comment From: yangjianzhou
Hi @harawata , thank you for your commits. I have fix the bug of false positive. and I must to explain clearly ：my purpose of this pr is to find bug when program startup , this is import for engineer when depoly the program to online . when they notice that program throw exception at startup ,they will realize that this program has problem and stop depolying, instead of throw exception when invoke the method ,and at most time , mybatis is integrated with spring ,instead of running mybatis alone. your second commit just close the check switch and run error when invoke mapper method. so I think my pr is useful.
Comment From: yangjianzhou
Hi @harawata :
I deleted your test case , because your test case test fail .
@Test
public void testUpdateByName() throws Exception {
String resource = "org/apache/ibatis/builder/xml/UserMapperConfig.xml";
InputStream inputStream = Resources.getResourceAsStream(resource);
SqlSessionFactory sqlSessionFactory = new SqlSessionFactoryBuilder().build(inputStream);
try (SqlSession sqlSession = sqlSessionFactory.openSession()) {
UserMapper userMapper = sqlSession.getMapper(UserMapper.class);
int result = userMapper.updateByName("any param", 0);
assertEquals(result, 1);
}
}
and this run fail has nothing to do with check ,just only error code.
Comment From: harawata
@yangjianzhou ,
Frankly, this approach is not very clean (there are more tricky cases and the method doCheckMapperRelatedMappedStatementExist will keep getting uglier), so I would like to try the approach I mentioned in #1506 .
I can't give high priority to this feature, but please let me look into it.
Note that even if we add detection of this simple mistake, the error is reported when Configuration#buildAllStatements() is called just like other mapper errors.
Comment From: yangjianzhou
@harawata thank you for replying. when mapper method related mappedStatement not exist ,it will throw exception and detail message likes 'Error parsing SQL Mapper Configuration. Cause: org.apache.ibatis.builder.IncompleteElementException: mappedStatementId (org.apache.ibatis.builder.xml.mapper.UserMapper.updateByName) related mappedStatement not found '. could you give some cases this detection can not detect?
Comment From: harawata
Hi @yangjianzhou , I have submitted a new PR #1645 which, I believe, is a cleaner solution. See if it meets your requirements when you have time, please. Thank you!
Comment From: yangjianzhou
OK,I will see it
Comment From: yangjianzhou
your solution is simple and effective . but I think the validation can be triggered automatic instead of by invoking .
your code :
ssFactory.getConfiguration().hasStatement("org.apache.ibatis.submitted.unboundmethod.Mapper.getUser")
I think all mappers validation can be triggered at
private void parseConfiguration(XNode root) {
try {
propertiesElement(root.evalNode("properties"));
********
mapperElement(root.evalNode("mappers"));
** validation can be place here, or at the end of this method **
} catch (Exception e) {
throw new BuilderException("Error parsing SQL Mapper Configuration. Cause: " + e, e);
}
}
this will be more friendly
thank you for your reply!
Comment From: harawata
First of all, XMLConfigBuilder is used only when there is XML config file which is not always the case.
Also, it is possible for users to add mappers after XMLConfigBuilder#parseConfiguration() is called (e.g. via Configuration#addMapper()).
So, no, it is not possible for MyBatis to validate these mapper errors until Configuration#buildAllStatements() is called, trust me. :)
Comment From: harawata
No response. Let me know if my explanation is unclear.