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

generate QClasses with KSP #638

Closed
JYC11 opened this issue Oct 17, 2024 · 23 comments
Closed

generate QClasses with KSP #638

JYC11 opened this issue Oct 17, 2024 · 23 comments

Comments

@JYC11
Copy link

JYC11 commented Oct 17, 2024

Would it be possible for KSP to work with this fork to generate QClasses? Kapt is in maintenance mode and I'd like to move on from it.

@IceBlizz6
Copy link
Contributor

Hello @velo

I am the author of https://github.com/IceBlizz6/querydsl-ksp
A project that aims to provide KSP support for QueryDSL.

Since starting this project multiple people has reached out to me (including OP) to ask about the possibility to
merge my project into yours.

I wanted to start a discussion to hear your thoughts on this and how you would like to proceed,
But it seems like you have disabled discussions tab in this github project, your readme is referencing discussions tab in the original querydsl project instead.
Feel free to enable it here and we could move this discussion.

I think this may be the simplest and quickest path for you seeing as you have limited time and i have already fleshed out a lot of the code.

@velo
Copy link
Member

velo commented Oct 18, 2024

Hello @IceBlizz6 , nice to meet you =)

I have no objections for you to submit a PR to include ksp support to this queryDSL fork AND for you to get maintenance access level for this repository.

Ideally, I would say to use a git merge --allow-unrelated-histories to preserve history, as much as possible.

Just two things you will need to be aware and stick to:

  • mavenize your build, not sure if you need anything gradle specific, but I don't want to change build from maven.
  • not sure if you ever published to maven central, if you did, would need a final release with relocation instructions
  • include tests, I couldn't find any, at the bare minimal we need test that covers happy path, like does it run and generated what its expected to be generated?!
  • add or update example https://github.com/OpenFeign/querydsl/tree/master/querydsl-examples/querydsl-example-kotlin-codegen

I know it would be a shore, but I'm sure eventually I will need to touch this, so need at least to make consistent with the rest of the project.

Please let me know if you want to proceed.

@velo
Copy link
Member

velo commented Oct 18, 2024

Ow, and if kapt is indeed going away, might be a good thing to deprecate it left and right.

@velo
Copy link
Member

velo commented Oct 18, 2024

I will close this to clear the board. But feel free to reopen it when you are happy to get this started. Thanks

@velo velo closed this as completed Oct 18, 2024
@IceBlizz6
Copy link
Contributor

mavenize your build, not sure if you need anything gradle specific, but I don't want to change build from maven.

I will look into this, i don't think i'm using any specific gradle functionality.

not sure if you ever published to maven central, if you did, would need a final release with relocation instructions

I never published to maven central, i spent some time trying to figure this out but it seemed like they were in the middle of a big migration and most guides were outdated.

include tests, I couldn't find any, at the bare minimal we need test that covers happy path, like does it run and generated what its expected to be generated?!

There are tests actually, but only on my local machine.
I'm using com.github.tschuchortdev:kotlin-compile-testing-ksp, and the problem i encountered was that it did not seem compatible with Kotlin 2.0
So i'm downgrading version locally in order to run the tests (which is obviously cumbersome).

The other problem with this package is that the tests are actually running Kotlin code through the compiler so the tests feels heavy.

Testing for this project can be split up into 2 parts

  1. Ensuring that source code for entity classes is correctly translated into intermediate models.
  2. Ensuring that intermediate models produce correct Q classes.

I'm not sure if it's possible to do 1. without com.github.tschuchortdev:kotlin-compile-testing-ksp and i have yet to find another library with similar functionality.
2. should be pretty straightforward to make tests for.

In conclusion i think i may only be able to provide tests covering 2. for now.
And i'm less sure if we should even try to solve 1. seeing as i don't think it can be done without introducing more computationally expensive tests.

add or update example https://github.com/OpenFeign/querydsl/tree/master/querydsl-examples/querydsl-example-kotlin-codegen

Did you intend to reference a readme file?

Also do you want to continue discussion here in this issue?

@velo velo reopened this Oct 18, 2024
@velo
Copy link
Member

velo commented Oct 18, 2024

I'm happy to continue the discussion here.

Maybe you can duplicate the existing example that uses kapt to use ksp and that would be enough testing for now.

How does that sound?

@velo
Copy link
Member

velo commented Oct 18, 2024

Also, you should be able to have a test module that overrides kotlin versions;

@IceBlizz6
Copy link
Contributor

Maybe you can duplicate the existing example that uses kapt to use ksp and that would be enough testing for now.

Existing tests in querydsl-kotlin-codegen only tests for 2. ([Intermediate models] -> [Q-classes])
Which makes sense because other modules in the QueryDSL project has already read through Kotlin as java code.
And those models are then fed into querydsl-kotlin-codegen.

KSP needs to read it as Kotlin code, so cannot go through the exact same process.
But i could try to adapt the tests to my project so that they work in a similar way.

My project has its own set of intermediate model representations now, i'm not sure if i should keep those as is or try to use the representation that QueryDSL is using for java classes (EntityType).

Also, you should be able to have a test module that overrides kotlin versions;

I will look into this.

@velo
Copy link
Member

velo commented Oct 18, 2024

Ow, when I say examples I mean https://github.com/OpenFeign/querydsl/tree/master/querydsl-examples/querydsl-example-kotlin-codegen

So you can have a querydsl-example-kotlin-ksp example that will generate the code and then run unit tests over it to have at least end to end testing.

@IceBlizz6
Copy link
Contributor

Do you have a guide on compiling your project?

I got through a few of the errors already.
I created toolchains.xml and that got me further.

Now i'm on this error

[ERROR] Failed to parse plugin descriptor for io.github.openfeign.querydsl:querydsl-maven-plugin:6.9-SNAPSHOT (/home/iceblizz6/source/querydsl/querydsl-tooling/querydsl-maven-plugin/target/classes): No plugin descriptor found at META-INF/maven/plugin.xml -> [Help 1]

Also maven cannot locate this plugin, But that could be resolved by solving the error above.
<plugin> <groupId>org.asciidoctor</groupId> <artifactId>asciidoctor-maven-plugin</artifactId> </plugin>

@velo
Copy link
Member

velo commented Oct 18, 2024

Do you have a guide on compiling your project?

These is one of many things needed.

I created toolchains.xml and that got me further.

Need to create and include java 11, 17 and 21, something like this:

<?xml version="1.0" encoding="UTF8"?>
<toolchains>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>11</version>
    </provides>
    <configuration>
      <jdkHome>/Users/marvin/.sdkman/candidates/java/11.0.2-open</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>17</version>
    </provides>
    <configuration>
      <jdkHome>/Users/marvin/.sdkman/candidates/java/17.0.12-oracle</jdkHome>
    </configuration>
  </toolchain>
  <toolchain>
    <type>jdk</type>
    <provides>
      <version>21</version>
    </provides>
    <configuration>
      <jdkHome>/Users/marvin/.sdkman/candidates/java/21.0.2-open</jdkHome>
    </configuration>
  </toolchain>
</toolchains>

I have no idea how asciidoctor got there, removed. Please pull.

Now i'm on this error

make sure to use mvnw ...

./mvnw clean instal

@IceBlizz6
Copy link
Contributor

IceBlizz6 commented Oct 18, 2024

Think i finished the module now.

  • It has a working pom.xml and nothing gradle related.
  • I included a couple of tests, not sure how extensive they should be. They only test intermediate model to Q classes.

I named it querydsl-ksp-codegen and put it under querydsl-tooling.

Still lots of issues with building the project, i got through mvnw install command after deleting 600+ test files.
But IntelliJ is still complaining about errors.

I'm not sure if i will be able to create a maven example, there doesn't seem to be any good KSP support for maven projects.
Closest one i can find is this https://github.com/Dyescape/kotlin-maven-symbol-processing
I could not get it to work, it has not been touched in years and there is a single issue of someone asking for Kotlin 2.0 support, and that has not received any replies.

It worked well in my gradle example project, but you mentioned that you didn't want gradle to be included.

@velo
Copy link
Member

velo commented Oct 19, 2024

Run ./mvnw clean install -Pdev, it still run tests, but not all

@velo
Copy link
Member

velo commented Oct 19, 2024

If gradle completely unavoidable, we could use one of the maven plugins that run gradle behind the scenes.

My concern is how well they would integrate during release process

@IceBlizz6
Copy link
Contributor

To be clear gradle was not necessary for making the KSP module.
But it may be necessary for the example project.

We could skip the example project or we could try the maven plugin for gradle.
How do you want to proceed?

@velo
Copy link
Member

velo commented Oct 19, 2024 via email

@velo
Copy link
Member

velo commented Oct 31, 2024

How is the example coming along?

Just waiting on it for a new release

@IceBlizz6
Copy link
Contributor

Hey, i apologize for the delay, things are happening at work.
I found some time yesterday to set up a simple example in gradle.

So we agreed that it was acceptable for this example to be in gradle, right?

I tested a bit and it seems like this works, it takes the 6.9-SNAPSHOT version that was installed into maven local when i built the project. So i'm able to import that into gradle.

repositories {
	mavenCentral()
	mavenLocal()
}

dependencies {
	implementation("io.github.openfeign.querydsl:querydsl-core:6.9-SNAPSHOT")
	ksp("io.github.openfeign.querydsl:querydsl-ksp-codegen:6.9-SNAPSHOT")
}

The example now runs as expected, the main method does nothing, it has a few entity and mappedsuperclass classes.
I'm trying to determine if i should include unit tests and maybe put some code into main to make it query data through the generated Q classes.

What do you think?
What is your expectations to this example project?

@velo
Copy link
Member

velo commented Nov 1, 2024

My expectations for the example is something similar to existing examples.

A project that you build, generate querydsl artifacts, compile and then run a few test that do access an in-memory database.

They operate almost as end 2 end testing or a happy path.

@velo
Copy link
Member

velo commented Nov 22, 2024

@IceBlizz6 are we good to release?

@IceBlizz6
Copy link
Contributor

I am not aware of any issues ,so yes.
I believe we can release.

@velo
Copy link
Member

velo commented Nov 22, 2024

@velo velo closed this as completed Nov 22, 2024
@CorayThan
Copy link
Contributor

@IceBlizz6 Hey, I just created a PR with some fixes for the KSP functionality. Would you have some time to take a look? #718

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

4 participants