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

Java 8 new Date Time API support in code generation #680

Closed
b0nyb0y opened this issue Mar 20, 2014 · 18 comments
Closed

Java 8 new Date Time API support in code generation #680

b0nyb0y opened this issue Mar 20, 2014 · 18 comments
Labels
Milestone

Comments

@b0nyb0y
Copy link

b0nyb0y commented Mar 20, 2014

Hi,
Now that Java 8 is officially released, I think it will be great if QueryDSL will also support the new Date Time API, which supersedes Joda Time.

http://download.java.net/jdk8/docs/api/java/time/package-summary.html
http://docs.oracle.com/javase/tutorial/datetime/

Currently JPA @Temporal can't be applied on these classes yet, but JPA users can definitely use JPA 2.1 Type Converter to do Java field type <-> SQL column type translation themselves.

@timowest
Copy link
Member

The following things are needed for this

  • recognition of Date Time API types in the code generation (all modules)
  • support for Date Time API types in JDBC interaction (SQL module)

@timowest
Copy link
Member

It seems like there is no official JSR 310 dependency that could be used from Java 7.

@b0nyb0y
Copy link
Author

b0nyb0y commented Mar 24, 2014

Currently QueryDSL is built targeting JDK 6 compiler isn't it? Does it mean that you will now need a different Maven build profile using JDK 8 as well? Putting it another way, since QueryDSL code base itself doesn't use JDK 8 features like lambda expression and such, would the produced binary be compatible with older JDKs?

@timowest
Copy link
Member

Moving to JDK8 binary compatibility at this point is not an option. For the general code generation the Date Time API can be a runtime dependency, but for Querydsl SQL it is referenced directly at build time.

@b0nyb0y
Copy link
Author

b0nyb0y commented Mar 27, 2014

From the look of things, I don't think the Date and Time API (which is now part of JDK) will ever be officially released/backported as a separate artifact that can be used as a Maven dependency, so does that mean we cannot go forward with either Code Generation module(s) and SQL module?

The only project I've seen that provides really good support for Date Time API is FasterXML Jackson lib. Since their core codebase also targets JDK 6, besides the new Date Time API, they also have a separate module for JDK 7 support. These modules work by targeting their respective JDK versions independent of the core modules.

https://github.com/FasterXML/jackson-datatype-jdk7/blob/master/pom.xml
https://github.com/FasterXML/jackson-datatype-jsr310/blob/master/pom.xml

With this approach, they still can test JDK 8 Date Time API well (since the jsr310 module POM actually uses JDK 8). The compiled JAR for the module is a JDK 8 binary (but users who use older JDKs don't have to include it in their projects).

I'm probably missing something, but I'm not sure how QueryDSL can make JDK 6 JAR that properly test new Date Time API from JDK 8.

@timowest
Copy link
Member

APT support can be done by mentioning the Date Time API classes in the proper enums of this class https://github.com/mysema/codegen/blob/master/src/main/java/com/mysema/codegen/model/TypeCategory.java

SQL codegen type registration can be done in a similar way like for joda time https://github.com/mysema/querydsl/blob/master/querydsl-sql/src/main/java/com/mysema/query/sql/JavaTypeMapping.java#L85

@b0nyb0y would you be willing to tackle one of these or even both as a pull request?

Requiring JDK8 for building is ok, if the binaries are still JDK6 compatible and the JDK8 classes are optional in runtime mode.

@timowest
Copy link
Member

timowest commented Apr 1, 2014

@b0nyb0y Are the suggested mappings correct and sufficient? querydsl/codegen#16

@b0nyb0y
Copy link
Author

b0nyb0y commented Apr 1, 2014

Yes, that would give us the same level of support as the it does with Joda. Sorry I didn't reply earlier, I was caught up with things for the past few days.

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@b0nyb0y Could you test with the latest SNAPSHOT of Querydsl?

@b0nyb0y
Copy link
Author

b0nyb0y commented Apr 2, 2014

Hi,
I tried to run my code using 3.3.3.BUILD-SNAPSHOT but for the build seems to fail for querydsl-jpa:

Downloaded: https://oss.sonatype.org/content/repositories/snapshots/com/mysema/querydsl/querydsl-core/3.3.3.BUILD-SNAPSHOT/maven-metadata.xml (2 KB at 2.5 KB/sec)
[WARNING] The POM for com.mysema.querydsl:querydsl-jpa:jar:3.3.3.BUILD-SNAPSHOT is missing, no dependency information available
...
[ERROR] Failed to execute goal on project common-domain: Could not resolve dependencies for project com.b0nyb0y:common-domain:jar:1.0-SNAPSHOT: Could not find artifact com.mysema.querydsl:querydsl-jpa:jar:3.3.3.BUILD-SNAPSHOT -> [Help 1]

I tried to run mvn clean install -U to force update several times but still get the same result. Taking a peek at my local repo, querydsl-apt, querydsl-core, querydsl-root seem to download SNAPSHOT versions into my local repo just fine though.

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@timowest
Copy link
Member

timowest commented Apr 2, 2014

@b0nyb0y Could you try again? Maybe this is a temporary issue.

@b0nyb0y
Copy link
Author

b0nyb0y commented Apr 2, 2014

Hmm, I removed mysema folder from my local repo to force re-download, and now even less of QueryDSL is retrieved.

[INFO] ------------------------------------------------------------------------
[INFO] Building common-domain 1.0-SNAPSHOT
[INFO] ------------------------------------------------------------------------
Downloading: http://repo.maven.apache.org/maven2/com/mysema/maven/apt-maven-plugin/1.1.1/apt-maven-plugin-1.1.1.pom
Downloaded: http://repo.maven.apache.org/maven2/com/mysema/maven/apt-maven-plugin/1.1.1/apt-maven-plugin-1.1.1.pom (4 KB at 3.1 KB/sec)
Downloading: http://repo.maven.apache.org/maven2/com/mysema/home/mysema-source/0.3.0/mysema-source-0.3.0.pom
Downloaded: http://repo.maven.apache.org/maven2/com/mysema/home/mysema-source/0.3.0/mysema-source-0.3.0.pom (3 KB at 6.1 KB/sec)
Downloading: http://repo.maven.apache.org/maven2/com/mysema/maven/apt-maven-plugin/1.1.1/apt-maven-plugin-1.1.1.jar
Downloaded: http://repo.maven.apache.org/maven2/com/mysema/maven/apt-maven-plugin/1.1.1/apt-maven-plugin-1.1.1.jar (13 KB at 25.3 KB/sec)
[WARNING] The POM for com.mysema.querydsl:querydsl-apt:jar:3.3.3.BUILD-SNAPSHOT is missing, no dependency information available
[WARNING] The POM for com.mysema.querydsl:querydsl-jpa:jar:3.3.3.BUILD-SNAPSHOT is missing, no dependency information available
...
[ERROR] Failed to execute goal on project common-domain: Could not resolve dependencies for project com.b0nyb0y:common-domain:jar:1.0-SNAPSHOT: The following artifacts could not be resolved: com.mysema.querydsl:querydsl-apt:jar:3.3.3.BUILD-SNAPSHOT, com.mysema.querydsl:querydsl-jpa:jar:3.3.3.BUILD-SNAPSHOT: Could not find artifact com.mysema.querydsl:queryds
l-apt:jar:3.3.3.BUILD-SNAPSHOT -> [Help 1]

I will try to create a brand new Maven project for testing then. Might just be some misconfiguration on my part.

@b0nyb0y
Copy link
Author

b0nyb0y commented Apr 2, 2014

Ah, my bad. I forgot to add Sonatype SNAPSHOT repo into my POM. Got it now.

@b0nyb0y
Copy link
Author

b0nyb0y commented Apr 2, 2014

Hi, on a quick test, I created a simple JPA entity class with all the Date/Time classes listed, I can confirm that the generated Q class is working with the new API now.

@timowest timowest changed the title Java 8 new Date Time API support Java 8 new Date Time API support in APT Apr 2, 2014
@timowest timowest changed the title Java 8 new Date Time API support in APT Java 8 new Date Time API support in code generation Apr 2, 2014
@timowest timowest added the fixed label Apr 2, 2014
@timowest
Copy link
Member

timowest commented Apr 2, 2014

I created a new issue for the SQL part #696

@timowest timowest modified the milestone: 3.3.3 Apr 13, 2014
@timowest timowest modified the milestone: 3.3.3 Apr 30, 2014
@timowest
Copy link
Member

timowest commented May 2, 2014

Released in 3.3.3

@timowest timowest closed this as completed May 2, 2014
@dmiorandi
Copy link

Cool!

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

No branches or pull requests

3 participants