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

Property type misdetection with Kotlin + default methods #314

Open
mvysny opened this issue Jan 21, 2019 · 3 comments
Open

Property type misdetection with Kotlin + default methods #314

mvysny opened this issue Jan 21, 2019 · 3 comments

Comments

@mvysny
Copy link

mvysny commented Jan 21, 2019

Thank you very much for your awesome work on Sql2o! I've built a small library on top of sql2o which also handles entity CRUD: the vok-orm.

However, there is a subtle bug in Sql2o which sometimes misdetects the property type. The problem is related to mvysny/vok-orm#10 . The problem is as follows.

In vok-orm there is an interface Entity<T> which all entities implement. The interface contains the T getId() method. When a class implements this interface, Kotlin compiler outputs two getId() methods: one that returns Object and the other which returns T (the same with setters, setId(Object) and setId(T).

The problem is in PojoMetadata:117 cycle: JVM lists those getters/setters in random order, and sometimes the T-typed one wins, and sometimes the Object-typed wins. That then causes Sql2o to not to pick up a proper converter for that field, and the setter fails.

I'm thinking of a fix of prioritising the T-typed getters/setters over Object-typed ones: if there is a setter with Object already in the propertyGetters/propertySetters map, overwrite it with a new one.

I can create a PR for this, I'm just wondering whether the project is alive and you're interested in PR and releasing 1.6.1 eventually :) Thanks!

@mvysny
Copy link
Author

mvysny commented Jan 21, 2019

Created a PR: #315

@stapel
Copy link

stapel commented Jan 27, 2019

Maybe it would be better to skip synthetic bridge-methods all together. Btw., this affects Java as well.

https://docs.oracle.com/javase/tutorial/java/generics/bridgeMethods.html#bridgeMethods

stapel added a commit to stapel/sql2o that referenced this issue Jan 27, 2019
Bridge-methods are generated by the compiler. In case of aaberg#314, getter and setter are created because the class inherited from a parametr. class and the getter/setter work on Object (as in the parent-class, after type erasure). That's why they need to be excluded from the list of methods to choose getters and setters from.
stapel added a commit to stapel/sql2o that referenced this issue Jan 27, 2019
Skip bridge-methods for POJO-getter/setter aaberg#314
@mvysny
Copy link
Author

mvysny commented Jan 28, 2019

Thank you, that's an excellent tip! Simply skipping over bridge methods simplifies the patch as well.

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

No branches or pull requests

2 participants