Skip to content

Enable @QueryProjection annotation support for KSP.#997

Merged
IceBlizz6 merged 9 commits intoOpenFeign:masterfrom
a3magic3pocket:queryprojection
Mar 20, 2025
Merged

Enable @QueryProjection annotation support for KSP.#997
IceBlizz6 merged 9 commits intoOpenFeign:masterfrom
a3magic3pocket:queryprojection

Conversation

@a3magic3pocket
Copy link
Contributor

fixes #996

Could you please check this feature at @IceBlizz6 if possible?
Thank you.

a3magic3pocket and others added 4 commits March 17, 2025 14:49
Bumps [org.junit:junit-bom](https://github.com/junit-team/junit5) from 5.12.0 to 5.12.1.
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit-framework@r5.12.0...r5.12.1)

---
updated-dependencies:
- dependency-name: org.junit:junit-bom
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps `hibernate.version` from 6.6.10.Final to 6.6.11.Final.

Updates `org.hibernate.orm:hibernate-core` from 6.6.10.Final to 6.6.11.Final
- [Release notes](https://github.com/hibernate/hibernate-orm/releases)
- [Changelog](https://github.com/hibernate/hibernate-orm/blob/6.6.11/changelog.txt)
- [Commits](hibernate/hibernate-orm@6.6.10...6.6.11)

Updates `org.hibernate.orm:hibernate-envers` from 6.6.10.Final to 6.6.11.Final
- [Release notes](https://github.com/hibernate/hibernate-orm/releases)
- [Changelog](https://github.com/hibernate/hibernate-orm/blob/6.6.11/changelog.txt)
- [Commits](hibernate/hibernate-orm@6.6.10...6.6.11)

Updates `org.hibernate.orm:hibernate-c3p0` from 6.6.10.Final to 6.6.11.Final
- [Release notes](https://github.com/hibernate/hibernate-orm/releases)
- [Changelog](https://github.com/hibernate/hibernate-orm/blob/6.6.11/changelog.txt)
- [Commits](hibernate/hibernate-orm@6.6.10...6.6.11)

---
updated-dependencies:
- dependency-name: org.hibernate.orm:hibernate-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.hibernate.orm:hibernate-envers
  dependency-type: direct:development
  update-type: version-update:semver-patch
- dependency-name: org.hibernate.orm:hibernate-c3p0
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
@IceBlizz6
Copy link
Collaborator

Hey,
This looks like great, thank you!

I have just a few comments on this before we merge.

In QueryDslProcessor you are using mapNotNull and you are returning null for invalid usages of these annotations.
So it seems like invalid usages are now ignored rather than alerting the user of misconfiguration.
I tend to prefer more explicitly alerting the user in these cases, i can't think of a case where use would want to use these annotations and not let them be picked up by this module.

I'm open for discussion on this one if you have thoughts on this, but my initial idea is to change mapNotNull into map and use throw/error instead of returning null.

is KSFunctionDeclaration -> {
    if (declaration.isConstructor()) {
        declaration.parent as? KSClassDeclaration
    } else {
        null
    }
}

Is this part only valid for QueryProjection?
Thinking maybe it should check if the annotation was QueryProjection, but looking over the code i must admit that i don't see an easy solution to that without making many further code changes so we may accept it as it is.

So annotating constructors instead of the class is now possible as i understand it (?)
Would you be able to add some code into the example project that annotates the constructor?
This is both helpful to show others how it could be used, but also to ensure that it works as expected.

@a3magic3pocket
Copy link
Contributor Author

Hello @IceBlizz6,
Thank you for your kind comments.

Based on your feedback, I have modified the existing source code.
Please review it again.

  • 1, 2
    In the QueryDslProcessor, I have implemented logic to execute only when the @QueryProjection is used.
    I also added a check to ensure it only runs when the @QueryProjection annotation is present on the constructor function and class.
    In all other cases, I have made it throw an error to alert the user to the issue.

  • 3
    I have added an example showing how to apply the @QueryProjection annotation to a constructor function.

If my modifications are different from your intention or if you think there are any other areas that need adjustment, please feel free to let me know.

Thank you.

@IceBlizz6
Copy link
Collaborator

Thank you, the code changes looks good!
But i seem to be encountering a compile-time error when i try to run the tests in querydsl-example-ksp-codegen.

querydsl-example-ksp-codegen/src/test/kotlin/Tests.kt:131:25 Unresolved reference 'QPersonClassConstructorDTO'.
querydsl-example-ksp-codegen/src/test/kotlin/Tests.kt:149:25 Unresolved reference 'QPersonClassDTO'.

On my computer i need to add these lines to make it work.

import com.querydsl.example.ksp.QPersonClassConstructorDTO
import com.querydsl.example.ksp.QPersonClassDTO

It doesn't seem like github workflows has been set up to build this example project, so it won't be able to confirm whether this is an actual issue in the code or if this is a configuration issue on my local machine.

I just wanted to confirm with you that you are not encountering this issue?

@a3magic3pocket
Copy link
Contributor Author

Hello @IceBlizz6,

Thank you for your feedback.

After checking the issue you pointed out, I realized that it was my mistake.

I found that some import statements were missing when copying from the local Gradle querydsl-example environment to querydsl/querydsl-examples/querydsl-example-ksp-codegen.

I have added the missing import statements and pushed the changes.

Thank you!

@IceBlizz6 IceBlizz6 enabled auto-merge March 20, 2025 21:13
@IceBlizz6 IceBlizz6 merged commit be16069 into OpenFeign:master Mar 20, 2025
4 checks passed
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

Successfully merging this pull request may close these issues.

Enable @QueryProjection annotation support for KSP.

2 participants