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

MyBatis fails to find constructor for records #2803

Closed
tzie0062 opened this issue Feb 9, 2023 · 6 comments · Fixed by #2804
Closed

MyBatis fails to find constructor for records #2803

tzie0062 opened this issue Feb 9, 2023 · 6 comments · Fixed by #2804
Assignees
Labels
Milestone

Comments

@tzie0062
Copy link

tzie0062 commented Feb 9, 2023

Issue

While upgrading an application from SpringBoot2/MyBatisSpring2 to SpringBoot3/MyBatisSpring3 I noticed errors while running the tests:

Failed to find a constructor in 'com.example.demo.Foo' by arg names [id, name]. There might be more info in debug log.

Debugging org.apache.ibatis.mapping.ResultMap showed that the argNames were resolved correctly yet argTypesMatch() returned false, which in turn lead to the error above.

Workaround

  • use class instead of record
  • use javaType in the resultMap
  • explicit downgrade (via pom.xml to 3.5.9)

MyBatis version

3.5.11

Database vendor and version

PostgreSQL 14.4

Test case or example project

https://github.com/tzie0062/issue-mybatis-record

Steps to reproduce

Using a resultMap with records

Expected result

Mapper works

Actual result

Mapper fails with

Caused by: org.apache.ibatis.builder.BuilderException: Error in result map 'com.example.demo.FooRepository.foo_map'. Failed to find a constructor in 'com.example.demo.Foo' by arg names [id, name]. There might be more info in debug log.
	at org.apache.ibatis.mapping.ResultMap$Builder.build(ResultMap.java:130)
	at org.apache.ibatis.builder.MapperBuilderAssistant.addResultMap(MapperBuilderAssistant.java:208)
	at org.apache.ibatis.builder.ResultMapResolver.resolve(ResultMapResolver.java:47)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElement(XMLMapperBuilder.java:289)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElement(XMLMapperBuilder.java:254)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.resultMapElements(XMLMapperBuilder.java:246)
	at org.apache.ibatis.builder.xml.XMLMapperBuilder.configurationElement(XMLMapperBuilder.java:119)
	... 102 more
@harawata
Copy link
Member

Hello, @tzie0062 ,

Thank you for the report and the repro!
You can verify the fix in the latest 3.5.12-SNAPSHOT.
Please let me know if there is any issue.

grallcd added a commit to grallcd/mybatis-3 that referenced this issue Mar 6, 2023
* Update dependency org.javassist:javassist to v3.29.2-GA

* Update dependency org.slf4j:slf4j-api to v2.0.1

* Update dependency org.apache.logging.log4j:log4j-api to v2.19.0

* Update dependency org.apache.logging.log4j:log4j-core to v2.19.0

* Update dependency ch.qos.logback:logback-classic to v1.4.1

* [pom] Update project build output timestamp for reproducible builds

* [ci] Correct spelling mistake in proxy factory in better way to avoid naming conflict

Alternative attempt to mybatis#2692

* [maven-release-plugin] prepare release mybatis-3.5.11

* [maven-release-plugin] prepare for next development iteration

* Update dependency org.slf4j:slf4j-api to v2.0.2

* Update junit5 monorepo to v5.9.1

* Referencing collection parameter by name fails

... when the first argument is a special one (i.e. `RowBounds` or `ResultHandler`).

fixes mybatis#2693

* Update dependency org.slf4j:slf4j-api to v2.0.3

* Update dependency org.testcontainers:junit-jupiter to v1.17.4

* Update dependency org.testcontainers:mysql to v1.17.4

* Update dependency org.testcontainers:postgresql to v1.17.4

* Fix a race condition caused by other threads calling mapper methods while mappedStatements are being constructed

* Update dependency ch.qos.logback:logback-classic to v1.4.2

* Update dependency ch.qos.logback:logback-classic to v1.4.3

* Update dependency org.testcontainers:junit-jupiter to v1.17.5

* Update dependency org.testcontainers:mysql to v1.17.5

* Update dependency org.testcontainers:postgresql to v1.17.5

* Update dependency ch.qos.logback:logback-classic to v1.4.4

* Update dependency org.mockito:mockito-junit-jupiter to v4.8.1

* Update dependency mysql:mysql-connector-java to v8.0.31

* Update dependency org.mockito:mockito-core to v4.8.1

* Enable ability to provide custom configuration to XMLConfigBuilder

Supported via the InputStream/Reader constructors

Default constructors will keep using a new configuration

* A subclassed configuration class can now be passed to allow XMLConfigBuilder
to use a specific implementation when creating the configuration

* Adding mapper could fail under JPMS

Under JPMS, ClassLoader#getResources() throws `FileSystemException` it seems.
It might be better to catch IOException, but it could swallow some exception that should be thrown.

Should fix mybatis#2598

* Update mockito monorepo to v4.9.0

* Update dependency org.testcontainers:junit-jupiter to v1.17.6

* Update dependency org.testcontainers:mysql to v1.17.6

* Update dependency org.testcontainers:postgresql to v1.17.6

* Update dependency org.slf4j:slf4j-api to v2.0.4

* Update dependency ch.qos.logback:logback-classic to v1.4.5

* Minor correction: boolean can never be null

* Added failing tests

Some DBs support INSERT, UPDATE or DELETE statement that returns result set.
PostgreSQL has RETURNING
MS SQL Server has OUTPUT

MyBatis can return results by using @select, @SelectProvider or <select />, however, rollback does not work as expected because SELECT does not mark the session 'dirty'.

* Added 'affectData' attribute to SELECT statements

To indicate the SELECT affects DB data.
e.g. PostgreSQL's RETURNING, MS SQL Server's OUTPUT

* Update dependency ch.qos.reload4j:reload4j to v1.2.23

* Bump postgresql from 42.5.0 to 42.5.1

Bumps [postgresql](https://github.com/pgjdbc/pgjdbc) from 42.5.0 to 42.5.1.
- [Release notes](https://github.com/pgjdbc/pgjdbc/releases)
- [Changelog](https://github.com/pgjdbc/pgjdbc/blob/master/CHANGELOG.md)
- [Commits](pgjdbc/pgjdbc@REL42.5.0...REL42.5.1)

---
updated-dependencies:
- dependency-name: org.postgresql:postgresql
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update dependency org.slf4j:slf4j-api to v2.0.5

* Documentation

* selectCursor() should respect affectData as well

* Update dependency ch.qos.reload4j:reload4j to v1.2.24

* Missing entry in es and ko doc [ci skip]

* Awful typo [ci skip]

* [pom] Update mysql-connector-j to new GAV relocation

* [ci] Use charset with filewriter

* [pom] Add filter to exclude MANIFEST.MF from uber jar from other jars (using ours)

clears warning and behaviour is the same.

* Update dependency org.slf4j:slf4j-api to v2.0.6

* Update mockito monorepo to v4.10.0

* Avoid false-negative

* Update mockito monorepo to v4.11.0

* Update dependency org.mybatis:mybatis-parent to v37

* Resolve resultType by namespace and id when not provide resultType and resultMap

* [actions] Drop jdk 18

* Minor refactoring, more tests

* Update dependency org.assertj:assertj-core to v3.24.1

* Update junit5 monorepo to v5.9.2

* Update mockito monorepo to v5

* Update dependency org.assertj:assertj-core to v3.24.2

* Update dependency com.mysql:mysql-connector-j to v8.0.32

* Update dependency maven to v3.8.7

* Include JDK 21 (EA at this point) in CI build

* [pom] Add mockito subclass support

some of our tests need the legacy style support that mockito made available.  We should look at those in longer term to get on fact that inline is now the default as per mockito we will likely have other issues.

* Update mockito monorepo to v5.1.0

* Update mockito monorepo to v5.1.1

* Update dependency org.postgresql:postgresql to v42.5.2

* [ci] Type the array lists

* Update dependency org.postgresql:postgresql to v42.5.3

* [ci] Sort imports

* [ci] Run partial formatting

* [ci] Update copyright dates

* [ci] Remove extra /p as not required in the javadoc

* [ci] formatting

* [ci] Update copyright dates

* [ci] formatting

* [ci] Update copyright date

* [ci] formatting

* [ci] Formatting

* [ci] formatting

* [ci] formatting

* [ci] formatting

* [ci] formatting

* [ci] Add note about formatting to readme

* [ci] Try adjusting formatting on readme

* [pom] For pre jdk 16 skip impsort

* [ci] Fix readme for formatter tags

* [mvn] Bump to maven 3.9.0 and change maven.config to 3.9/4.x style

* Search readable property when resolving constructor arg type

Should fix mybatis#2803

* Update test code to clarify the consequence

* Update dependency org.postgresql:postgresql to v42.5.4

* Add Unit Test for PooledDataSource

* Using cursor causes OOM mybatis#2812

* [ci] Minor formatting adjustments

* [ci] Import sort new test class

* [ci] Fix end of line markers at end of file per git standards (ie empty last line)

* [ci] Apply open rewrite partially on java cleanup

* [ci] Add private constructors to classes that cannot be instantiated (all static)

* [test] Remove dumb test - not valid now as class is static and should not have been instantiated

* [test] Remove invalid extension of class for testing

* [ci] Tabs to spaces

* [ci] Add final where appropriate per open rewrite

relaxed some of its suggestions but generally took most.

* [pom] Move surefire configuration to parent and make properties more readable

* [pom] Update surefire property usage to non deprecated values

* [ci] Adjust argument

* [ci] Show active profiles being used

* [actions] Drop usage of ci only tests group as unnecessary / directly set excludedGroups

profile is not needed.  All we are doing is setting a property.

* [actions] Run all tests for ubuntu jdk 11 using arglines

* [pom] Add comment on how to disable excluded groups runs

* [pom] Add another comment around excluded groups

* [ci] Add readme that describes how our testing works

* [ci] Do not use star imports (across test classes)

* [ci] Use diamond operators (test classes)

squash with diamond

* [ci] Add missing seial version uid at 1L (throughout tests)

* [ci] Use mockito argument matchers instead of deprecated logic

* [ci] Use 'L' instead of 'l' for long

* [ci] Remove new of primative as known

* [ci] Remove super as no super to call

* [ci] More direct returns to save on GC

* [ci] Various small items cleaning up assignments

* [ci] Better parsing wrapper classes

* [ci] Remove public from methods in interfaces

* [ci] Formatting / update copyright dates

* [ci] Add missing overrides

* [ci] Various small cleanups

* [ci] Use objects class for hash, equals usage

* [ci] Cleanup if statement breaks / return logic

* [ci] Remove unnecessary super calls

* [ci] Make static classes 'static'

* [ci] Remove unnecessary parentheses and allow quicker exit

* [ci] Add missing serial version uid as 1L

* [ci] Use diamond operator

* [ci] Remove named wrapper of list as type given already

* [ci] Remove unused variable from internal method

* [ci] Smarter list creation

* [ci] Update copyright dates

* [ci] Add back type as not until jdk 9 can this be determined

* Add resultOrdered="true" and change expected cursor's nestedResultObjects size to be less than 3

* Update dependency org.apache.logging.log4j:log4j-api to v2.20.0

* Update dependency org.apache.logging.log4j:log4j-core to v2.20.0

* Use CountDownLatch to coordinate the execution order

* [pom] Add some comments in the pom for clarity

* [pom] Remove compiler test compiler argument parameters as not needed now

* [maven-release-plugin] prepare release mybatis-3.5.12

* [maven-release-plugin] prepare for next development iteration

* [ci] Add release backup to gitignore

* [fix] Change formatter around javadoc to retain L&F and correct javadoc error

* [maven-release-plugin] prepare release mybatis-3.5.12

* [maven-release-plugin] prepare for next development iteration

* fix Chinese translation error for `parameterType`

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Jeremy Landis <jeremylandis@hotmail.com>
Co-authored-by: Iwao AVE! <harawata@gmail.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: tianshuang <tianshuang.me@gmail.com>
Co-authored-by: Willie Scholtz <williescholtz@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: FlyInWind <2518509078@qq.com>
Co-authored-by: hogimn <hogimn@gmail.com>
Co-authored-by: gallyamb <gallyamb@gmail.com>
Co-authored-by: puppylpg <shininglhb@163.com>
@harawata
Copy link
Member

harawata commented Mar 7, 2023

Hello again, @tzie0062 .

After the discussion in #2804 , I now realize that it was a mistake to fix this issue and the fix will be reverted in 3.5.13.

javaType in constructor mapping is optional only when there is a 'writable property' in the target class.
As Foo is declared as a record, there is no writable properties.
So, you must specify javaTyps. The valid result map would be...

<resultMap id="foo_map" type="com.example.demo.Foo">
  <constructor>
     <idArg column="id" javaType="java.lang.Long" />
     <arg column="name" javaType="java.lang.String" /> 
  </constructor>
</resultMap>

Note that you don't have to specify name because the mapping order matches the actual constructor arguments order (see #721 if you are interested).

I apologize for the confusion.

@tzie0062
Copy link
Author

tzie0062 commented Mar 7, 2023

Hi @harawata !

I don't mind adding the javaType, however I still think something was broken between 3.5.9 and 3.5.11. Please have a look at the repo I mentioned above - I just updated it to have a BarRepository which returns an immutable object (no record). The Bar class does not have 'writeable properties' and the mapping works without javaType in the resultMap.

If you set the mybatis version to 3.5.9 in the pom, you will see that both record and immutable object work fine (without javaType), but the record fails in 3.5.11.

@harawata
Copy link
Member

harawata commented Mar 7, 2023

Hi @tzie0062 ,

I haven't checked your updated demo, but it probably is the same issue as #2840 .
Please see my comment there.

To avoid further confusion, we plan to release 3.5.13 as soon as possible.

@tzie0062
Copy link
Author

tzie0062 commented Mar 8, 2023

Thanks for the update @harawata !

I'll update all my mapper-files accordingly. It's a bit unfortunate that this change was introduced in a minor version, but I only see it as a minor inconvenience. Thanks for all the work/time you put in!

@harawata
Copy link
Member

harawata commented Mar 8, 2023

Hi @tzie0062 ,

Thank you again for your patience and for taking the time to participate!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants