arnaudroger / SimpleFlatMapper

Fast and Easy mapping from database and csv to POJO. A java micro ORM, lightweight alternative to iBatis and Hibernate. Fast Csv Parser and Csv Mapper
http://simpleflatmapper.org
MIT License
440 stars 76 forks source link

SqlParameterSource hasValue evalutes to true for first non existing param #713

Open maciej-pajak opened 4 years ago

maciej-pajak commented 4 years ago

SqlParameterSource hasValue returns true when called for first non existing param, and getValue returns the object passed as param to newSqlParameterSource

Reproduction:

Object sourceObject = new Object();
SqlParameterSource paramSource = JdbcTemplateMapperFactory
        .newInstance()
        .ignorePropertyNotFound()
        .newSqlParameterSourceFactory(Object.class)
        .newSqlParameterSource(sourceObject);

paramSource.hasValue("nonExistingParam1"); // evaluates to true
paramSource.hasValue("nonExistingParam2"); // evaluates to false
paramSource.getValue("nonExistingParam1"); // returns sourceObject 

Tested on sfm-springjdbc version 8.2.1

maciej-pajak commented 4 years ago

I looked at the issue, but I am not sure how this should be fixed. I think of adding org.simpleflatmapper.reflect.meta.DisallowSelfReference property to org.simpleflatmapper.map.property.FieldMapperColumnDefinition in org.simpleflatmapper.jdbc.spring.SqlParameterSourceBuilder#add(java.lang.String) - this fixes the problem, but I don't know the project well enough to fully assess effects of this change. What do you think @arnaudroger ?

arnaudroger commented 4 years ago

I’ll try to have a look at it tomorrow

arnaudroger commented 4 years ago

so looking at it yes, it would behave like that. nonExistingParam1 will resolve to the actual object. and following property will not resolve to anything. the DisallowSelfReference will make disable allowing that. but I'm assuming that there are actual column that would match and that's the problem here?

arnaudroger commented 4 years ago

the problem is that it can't know all the column and the design of the sql parameter source, make it hard to solve - hasValue does not know of the future columns.

though newSqlParameterSourceFactory can take take the sql query so if you do

        SqlParameterSource paramSource = JdbcTemplateMapperFactory
                .newInstance()
                .ignorePropertyNotFound()
                .newSqlParameterSourceFactory(Object.class, "select id from table")
                .newSqlParameterSource(sourceObject);

        assertFalse(paramSource.hasValue("nonExistingParam1")); // evaluates to true
        assertFalse(paramSource.hasValue("nonExistingParam2")); // evaluates to false

then it work has expected

maciej-pajak commented 4 years ago

Thanks for taking a look at this. I am not sure if I can use org.simpleflatmapper.jdbc.spring.JdbcTemplateMapperFactory#newSqlParameterSourceFactory(java.lang.Class<T>, java.lang.String), beacause I have multiple sqls using the same SqlParameterSourceFactory. My use case is that, I have custom delegate SqlParameterSource which takes multiple sources and looks for param in all of them. But looking through the code I found that I can add DisallowSelfReference property to all columns via: .addColumnProperty(ConstantPredicate.truePredicate(), new DisallowSelfReference() {}) and I think that solves the issue for me.

If you decided you would like to implement any solution in the project I could give you a hand with PR - if you let me know which way you would like to go with that.

Cheers