Support for java.time

[graynk]

Hopefully closes #154

Adds support for Java 8 Time API (JSR-310), specifically for LocalDate, LocalTime, LocalDateTime, OffsetTime, OffsetDateTime and Instant.

Default persisters use JDBC 4.2 API and pass values without conversion.

For DBs that don't fully support 4.2 (H2 and PostgreSQL don't support TIME WITH TIME ZONE, but support TIMESTAMP WITH TIME ZONE) there's a secondary persister for OffsetTime that converts OffsetTime to OffsetDateTime with date part fixed at epoch. There's an overriden FieldConverter for those DBs in ormlite-jdbc, but these persisters can also be enabled by specifying DataType in annotation of the Dao class. Instant is not a part of 4.2 API, so it converts to/from OffsetDateTime. For LocalDate|Time classes there are also secondary persisters, that convert java.time values to java.sql values using methods, added in Java 8. These are, again, forced in overriden 'FieldConverter's for those DBs in ormlite-jdbc and can also be enabled by specifying DataType. Someone with a better knowledge of DBs than me should take a look at those, since finding anything concrete in the documentation for all those different DBs is a pain (I presume DB2 and Netezza don't support 4.2, but I'm still not sure. Same for Sqlite).

For every persister there's a check in getSingleton() method that returns null, if there's no corresponding java.time class in classpath to allow compiling for older JDKs. No actual testing was done for that though.

I am not entirely clear on purpose of isArgumentHolderRequired() and moveToNextValue() methods of BaseData, so I aped those off of DateType (aped the tests as well).

New tests will fail for H2 1.2.128, since that version doesn't support 4.2 yet. For the latest H2 (1.4.197) they pass. However, old tests begin to fail, due to some changes in API, though this does not affect this PR, and I will create a separate issue for that.

[graynk]

Thanks for this. How is backwards compatibility achieved? Does this force ORMLite to Java 8?

I got an email with this reply, but it no longer seems to be here. I'll reply anyway. The idea for backwards compatibility was returning null in getSingleton methods, when not being able to find java.time classes, so Java 8 is required to compile, but not to run ORMLite.

However, I just got around to actually testing it on Java 1.6 and found out, that I totally botched it, by initializing static singleton at declaration (duh). Lazily initializing it after checking if java.time class exists on classpath seems to work as intended, though that makes the singleton not final, and as such - a rather poor singleton. The constructor is still private and the field is not reassigned anywhere, so I hope it's okay. I will push a commit that fixes this a bit later, sorry.

[graynk]

1. I've added inheritance to reduce code duplication, but I wouldn't call *_SQL types wrappers. After all, I still have to override most of the methods.
2. I've added same precision to OffsetTime, but as for *_DateTimes - this is the same reason why old DateTypeTest fails on H2 1.4.197. For me, LocalDateTime.now() on Java 8 returns with precision of 3 on Linux machine (but with precision of 6 on Java 11, strangely enough). So DateTimeFormatter formats it with trailing zeroes. However, getString() in new version of H2 returns the string without trailing zeroes, and the formatter fails. I guess I could do .SSS[SSS], but I'm not sure how to handle this issue properly, to be honest. I had to truncate LocalTime to seconds as well, but OffsetTime works no problem. It's a mess.
3. Should be fixed now as well
4. I wanted to handle this the same way that DateType right now is used by default and DateStringType and others can be used if needed, but I couldn't find what made DateType special, so I used order of enums. I'd like to do that properly, if you point me in the right direction. UPD: ah, I see what you mean about omitting the class from constructor. Will do.

There's another point I'd like to discuss. What are the downsides of dropping Class.forName() approach, and instead using ternary operator, i.e. private final static LocalDateType singleton = Double.parseDouble(System.getProperty("java.specification.version")) < 1.8 ? null : new LocalDateType() The upsides are: singleton is back to being final singleton, it's evaluated once at the start, it works on 1.6, we don't need to check for null on every getSingleton() call and there's no need to throw 10 exceptions every time we launch. The downsides that I see:

1. Java radically changes version numbering in the future
2. Property gets removed or changes, so that it doesn't parse to Double anymore
3. It's ugly
4. Something else?

[Bo98]

1. I've added inheritance to reduce code duplication, but I wouldn't call *_SQL types wrappers. After all, I still have to override most of the methods.

Well you could do things like Date.valueOf((LocalDate)super.parseDefaultString(...)). But yeah, I understand there's only so much that can be done.

For me, LocalDateTime.now() on Java 8 returns with precision of 3 on Linux machine (but with precision of 6 on Java 11, strangely enough).

Interesting. It's documented to have nanosecond precision.

3. Should be fixed now as well

Did I miss these changes? The class Javadoc for InstantType still refers to LocalDate, for example.

What are the downsides of dropping Class.forName() approach, and instead using ternary operator, i.e. private final static LocalDateType singleton = Double.parseDouble(System.getProperty("java.specification.version")) < 1.8 ? null : new LocalDateType()

Personally, I prefer feature checking over version checking but others may disagree with me on that.

To answer your points specifically:

Java radically changes version numbering in the future

It has, but it didn't break your check.

Property gets removed or changes, so that it doesn't parse to Double anymore

I think removal is unlikely. It might change, but I think they'd be cautious about it.

It's ugly

Yep. Java feature & version checking in general is a bit ugly.

---

I've also replied to your *_SQL comment above.

[graynk]

Interesting. It's documented to have nanosecond precision.

Quoting the docs for Clock class,

The system factory methods provide clocks based on the best available system clock This may use System.currentTimeMillis(), or a higher resolution clock if one is available.

As this answer on SO put it, the classes are capable of carrying the nanoseconds, but not capturing it. This was changed in Java 9 to microseconds (but still not nano though).

Did I miss these changes? The class Javadoc for InstantType still refers to LocalDate, for example.

Oh, for the love of.. Yeah, I missed it, I changed just the text of the Exceptions, not the javadocs. I'll push it in the next commit, along with clearer explanations for *_SQL types.

Personally, I prefer feature checking over version checking but others may disagree with me on that.

I understand that, but this is not really an exceptional situation, it's a regular check (one that will always throw an exception for each class on older JDKs at that), so IMO it should be an actual if check. Problem is: I don't know a way to check for class existence without throwing an exception. This is the main thing I'm trying to avoid.

It has, but it didn't break your check.

True. But they could use something like 2018.3.1, for example (or add letters), I don't know.

I've also replied to your *_SQL comment above.

I mean, OFFSET_TIME_TIMESTAMP is sort of okay, but then there's LOCAL_DATE_DATE and LOCAL_TIME_TIME and that's just plain confusing. What about adding _TO_ in-between?

[Bo98]

I mean, OFFSET_TIME_TIMESTAMP is sort of okay, but then there's LOCAL_DATE_DATE and LOCAL_TIME_TIME and that's just plain confusing. What about adding TO in-between?

If you wanted to keep _SQL for the others I wouldn't have too much of an issue since the purpose of them is to use java.sql. The underlying SQL type itself is unchanged - LOCAL_DATE_DATE could apply to either class. If you wanted to change it, the only other things I would think would be suitable are _JAVA_SQL, _COMPAT, _LEGACY or similar.

It's just OFFSET_TIME_SQL that I didn't think fit its name since it doesn't use java.sql but rather its purpose is to change the underlying SQL type from TIME to TIMESTAMP.

If you think adding _TO_ would help then sure.

[graynk]

In the end I made the call to do a single static check for Java specification version. My reasoning: our target is 1.6 and java.time became available in Java 8. Judging by the fact that java.util.Date is still around, java.time packages won't be deprecated for the foreseeable future. So, we need to check only that specification string is not equal to 1.6 or 1.7, no need to convert version to Double and so possible change in version numbering won't affect this. I think this is a better approach than Class.forName(), for reasons I explained above (Effective Java, Item 57, basically), even if it's not ideal. Of course, it's up to maintainers to make a final call.

I've expanded a bit on javadocs for the new classes (and hopefully fixed all the wrong references), and renamed OffsetTime to OffsetTimeCompat. The problem with formatting time with 0/3/6 precision in *_Time tests is still in the air, but it concerns only conversion from Strings which shouldn't happen anyway, not sure why it's there.

Ideally this should also include new paragraphs to HTML tutorials, but this can be added later in a separate PR.

[Bo98]

j256/ormlite-core#165 for the additional H2 fixes.

[mpe85]

Are there any news on this?

[graynk]

Are there any news on this?

Well, I'm using a build with these changes in my project with H2 and it works for me, the tests are passing as well. There were some problematic DBs that don't follow JDBC standard and we didn't seem to really settle on a final approach for cleanly dealing with them, so whether this commit is ready for general use is not for me to decide.

[maximillian2]

Any chance of this to be merged? @j256

[j256]

Yeah I need to work on this. Portability is the worry. I'll look at it this week.

[BomBardyGamer]

Hey, know this is a bit old now, but any updates on this?

[j256]

Right now I've not forced the library to go to Java 8 which is the holdup. Yes I know Java 7 is ancient but I'm always reluctant to upgrade my libraries.

[noordawod]

Right now I've not forced the library to go to Java 8 which is the holdup. Yes I know Java 7 is ancient but I'm always reluctant to upgrade my libraries.

Well, Java 8 is about to be deprecated so I would say it's a good time to upgrade ;)

[BomBardyGamer]

Could we say get this merged into a different branch or something? Say, as a dev build, just so we can use it? Because I kinda really need this. Or you could just do what Exposed did and make this a separate module (e.g. ormlite-java-time) maybe?

Java 8 has already reached the end of LTS by the way, so Java 7 is even further out of support range.

[noordawod]

I like the idea of a separate module, what do you think @j256? Possible?

[j256]

I'd rather not do a whole module for this although maybe that would be easier than the reflection hack alternatives.

[perlun]

For the record, while waiting on this PR to get finalized, it's quite possible to add support for e.g. java.time.Instant on your own by using code from this branch. You can then register it using e.g. DataPersisterManager.registerDataPersisters(InstantType.getSingleton()).

I could extract the code I have to a GitHub gist/public repo in case anyone's interested. Note: it only supports Instant at the moment, which gets stored as a TIMESTAMP WITHOUT TIME ZONE in my case - using Postgres. Supporting more databases/different field types is probably harder without getting some of this merged, since that would presumably rely on the SqlType support which this PR adds. Here's how I did it in my local implementation for now:

    @Override
    public String getSqlOtherType() {
        return "TIMESTAMP WITHOUT TIME ZONE";
    }