Add LocalDateTime conversion to the Convert class

[marcom1981]

With new MySql Drive 8.0.23:
https://dev.mysql.com/doc/relnotes/connector-j/8.0/en/news-8-0-23.html

We have issue with getDate Method related new mechanism to menage DateTime Type.
When invoke Model.getDate("columnName") we receive a "Cannot convert value to Date" error, because Model.getRaw("columnName) returns a LocalDateTime object.

We use 2.4-j8 and this JDBCUrl property:
"?autoReconnect=true&serverTimezone=CET"

[ipolevoy]

@marcom1981

Please, provide:

• Version of Java
• DDL for a table
• Version of MySQL

[gaivao]

@ipolevoy

We have this problem with

• Java Version: 8 (1.8.0_312 azul), 11 (11.0.11 adopt) e 16 (OpenJdk 16.0.2)

• Mysql Version: 5.7.31-34-57-log Percona XtraDB Cluster

• JDBC driver mysql:mysql-connector-java:8.0.23 (the problem not happens with version 8.0.22)

• this is a simple DDL that reproduce the problem
  CREATE TABLE test_table(idvarchar(100) NOT NULL,expires_on datetime NOT NULL, PRIMARY KEY (id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8

With this snippet of code

Base.open(driver, url, username, password);
TestTableModel data = TestTableModel.findFirst("id = ?", "1");
 Date passwordExpiration = data.getDate("expires_on");

we encounter this exception at runtime

Exception in thread "main" org.javalite.common.ConversionException: failed to convert: '2021-12-17T00:00' to java.sql.Date
	at org.javalite.common.Convert.toSqlDate(Convert.java:151)
	at org.javalite.activejdbc.Model.getDate(Model.java:320)
	at TestActiveJdbc.main(TestActiveJdbc.java:17)
Caused by: java.lang.IllegalArgumentException
	at java.sql/java.sql.Date.valueOf(Date.java:141)
	at org.javalite.common.Convert.toSqlDate(Convert.java:149)
	... 2 more
  

[ipolevoy]

@gaivao , can you please stop in the debugger and see the actual Java type of expires_on attribute before you get an exception? Please, report it here then.

[gaivao]

@ipolevoy as written by @marcom1981 previously, the actual Java type returned by Model.getRaw("expires_on") is java.time.LocalDateTime, so java.sql.Data.valueOf(rawObject) throw an IllegalArgumentException,because that data type is not accepted on valueOf function.

With version 8.0.22 of MySQL Driver, the java returned type was java.sql.Timestamp.

In both cases no converter was used to transform rawObject in java.sql.Date, so Convert.toSqlDate(value) was called to get converted object.

[ipolevoy]

@gaivao, ActiveJDBC is a Pass-Through framework: https://javalite.io/pass_through_framework#loading-attributes. In other words, it is not managing type conversions. It is a responsibility of an application developer to pick the right converter for the job. Makes sense?

[gaivao]

@ipolevoy yes makes sense, but there is a refutable point:

based on the javadoc of method org.javalite.common.Convert.toSqlDate(Object value) ActiveJDBC also manages directly and transparently a conversion of type and this is well done with version prior to 8.0.22 of mysql driver.

/**
     * Expects a <code>java.sql.Date</code>, <code>java.sql.Timestamp</code>, <code>java.sql.Time</code>, <code>java.util.Date</code>,
     * <code>Long</code> or any object whose toString method has this format: <code>yyyy-mm-dd</code>.
     *
     * @param value argument that is possible to convert to <code>java.sql.Date</code>.
     * @return <code>java.sql.Date</code> instance representing input value.
     */
    public static java.sql.Date toSqlDate(Object value){

In this case a raw object is typed converted to java.sql.Date only if its type is one of (java.sql.Date, java.sql.Timestamp, java.sql.Time, java.util.Date), but with the version after 8.0.23 of jdbc driver, this list of types is not sufficient to manage datetime datatype on the DB.

So in my opinion, if this method can manage conversion of type in version prior to 8.0.22, will can do the same with version after 8.0.22, simply adding a new type (java.time.LocalDateTime) to the list of convertible objected to java.sql.Date.

I belive this improvement will be usefull to a lot of developers that work with the new version of mysql driver, so they can change transparently the version of the driver used without have problem with datatype conversions.

Do you agree?

[ipolevoy]

@gaivao, I think I agree with you. An additional conversion will be good, why not? I see this more as an enhancement than a bug though.
There is a related task: #1119 that is in progress. There were a few commits there, but it is not complete yet.

In other words, the Convert class now has a new method public static LocalDateTime toLocalDateTime(Object value) {}, so it could easily be used inside the convenience Model.getLocalDateTime("attr").

The question is: do you really want to convert LocalDateTime to java.sql.Date ?

Are you open for contributions?

[ipolevoy]

@gaivao also, keep in mind that conversion of any date object with a time component to one without will lose the time part.

[gaivao]

@ipolevoy on the activejdbc point of view, i agree, this is an enhancement and not a bug.

Also on external software mainmaintenance point of view, have same behavior with different version of libraries, is a good requirement to avoid unexpected results.

So, in my point of view the expectation is have possibility to use transparently the toDate static function also with the new datetime management implemented by mysql driver.

If is necessary we are open to contribution for this enhancement.

For the last sentence, there is the possibility to maintain time part if you use a TimeZone or if you convert and then format a string value obtained with DateTimeFormatter.
So in case of datetime data type no timezone convertion is used on the server side, the value is stored as is. The only convertion, if needed, is used on the client side.

[ipolevoy]

@gaivao go for it and submit a pull request (with a corresponding test). If you have any questions on how to write test, there are plenty of examples, and you can ask here of course. I hope you are not going to convert from string to an object, if I understood correctly :)
I assigned this to you for now.

[gaivao]

@ipolevoy thanks.

Only for clarify, using valueOf(String s) is the default method used in org.javalite.common.Convert class when datatype is not managed, and only for luck (or unluck depends on point of view) in current version LocalDateTime cannot convert to SqlDate o Timestamp.

So using of convertion from string to an Object is not completely wrong

[gaivao]

@ipolevoy i've a local branch "issue_1090" with org.javalite.Convert class modified and related tests added to org.javalite.common.ConvertTest can you add me as collaborator to push the branch and make a pull request?

Thanks in advance

[ipolevoy]

@gaivao I added you as a collaborator, go ahead!

[gaivao]

I've resolved a SourceTree Issue, and now i've push my branch and create a pull request

I've modified org.javalite.Convert class and reorganize test methods belonging to modified parts

[ipolevoy]

@gaivao I changed the name of this issue to a more appropriate one

[ipolevoy]

@gaivao , the code looks good and thanks for the tests in the same style as the rest!
The build is successful, and a 3-0-SNAPSHOT was deployed to Sonatype. Closing this.