IceBlizz6 / querydsl-ksp

KSP for QueryDSL
5 stars 1 forks source link

UShort type not recognized #2

Open xlaussel opened 2 months ago

xlaussel commented 2 months ago

UShort type (and others unsigned maybe) not recognized:

@Entity
class A(
   @Column(nullable = false)
   val parts: UShort)

Leads to the following error: [ksp] java.lang.IllegalStateException: Error processing test.A.parts: Type was not recognised, This may be an entity that has not been annotated with @Entity, or maybe you are using javax instead of jakarta.

IceBlizz6 commented 2 months ago

Support for unsigned types has been added and should be working now.

Are you able to test this version to see if it works for you? ksp("io.github.iceblizz6:querydsl-ksp:fd2f530a6e")

xlaussel commented 2 months ago

I have the following errors:

[ksp] java.lang.IllegalStateException: Declaration was not a KSClassDeclaration: D? at com.squareup.kotlinpoet.ksp.KsTypesKt.toClassName(KsTypes.kt:49) at iceblizz6.querydsl.ksp.TypeExtractor.simpleType(TypeExtractor.kt:21) at iceblizz6.querydsl.ksp.TypeExtractor.extract(TypeExtractor.kt:14) ...

I think it is related to the following generic class (maybe a lack of generics support?):

@MappedSuperclass
@Embeddable
class HistorizedDocument<D:AbstractDocument> : DomainEntity() {

    @OneToOne(cascade = [CascadeType.ALL], optional = true, orphanRemoval = true)
    @JoinColumn(nullable = true)
    var current : D?=null
        set(new) {if (field!=null && field!==new) olds.add(field!!);field=new}

    @ManyToMany(cascade = [CascadeType.ALL])
    val olds= TreeSet<D> {a,b->a.created.compareTo(b.created)}
}

I also have the following error: [ksp] java.lang.IllegalStateException: Error processing test.AbstractDocument.pathInit: Type was not recognised, This may be an entity that has not been annotated with @Entity, or maybe you are using javax instead of jakarta.

where pathInit is a transient lambda field :

@MappedSuperClass
abstract class AbstractDocument {

abstract val pathInit : AbstractDocument.() -> String

}

I have the error also if I put the @Transient annotation:

@get:Transient
abstract val pathInit : AbstractDocument.() -> String
IceBlizz6 commented 2 months ago

I'm working through this now, thanks for sticking with me. I think my changes above fixed the unsigned integer types (?) And that revealed the next set of issues.

I added support for @get:Transient now, it only supported @Transient before.

abstract val pathInit : AbstractDocument.() -> String This one is interesting, i can see that kapt will turn this into SimplePath<Function1<AbstractDocument, String>>

Do you know if it is actually possible to query anything using this field? If not then would it make sense to use Transient to exclude fields like these?

My goal was to replicate functionality of kapt, but i'm not entirely sure if it's good to include unusable fields into the Q-classes.

I'm open for suggestions on this.

IceBlizz6 commented 2 months ago

I'm actually having trouble trying to make this code work in kapt.

This is the code which i assume is somewhat close to what you have

@Entity
class MyDocument : AbstractDocument() {
    override val pathInit: AbstractDocument.() -> String
        get() = TODO("Not yet implemented")

    override val created: LocalDate
        get() = TODO("Not yet implemented")
}

@MappedSuperclass
@Embeddable
class HistorizedDocument<D : AbstractDocument> {

    @OneToOne(cascade = [CascadeType.ALL], optional = true, orphanRemoval = true)
    @JoinColumn(nullable = true)
    var current : D? = null
        set(new) { if (field!=null && field!==new) olds.add(field!!);field=new }

    @ManyToMany(cascade = [CascadeType.ALL])
    val olds= TreeSet<D> { a, b -> a.created.compareTo(b.created) }
}

@MappedSuperclass
abstract class AbstractDocument {
    abstract val pathInit : AbstractDocument.() -> String

    abstract val created: LocalDate
}

It gives me this output

@Generated("com.querydsl.kotlin.codegen.KotlinEmbeddableSerializer")
public class QHistorizedDocument : BeanPath<HistorizedDocument<AbstractDocument>> {
  public val current: QAbstractDocument by lazy {
    QAbstractDocument(forProperty("current"))
  }

  public val olds: SetPath<AbstractDocument, QAbstractDocument> = this.createSet<AbstractDocument,
      QAbstractDocument>("olds", AbstractDocument::class.java, QAbstractDocument::class.java, null)

  public constructor(variable: String) : super(HistorizedDocument::class.java,
      forVariable(variable))

  public constructor(path: Path<HistorizedDocument<AbstractDocument>>) : super(path.type,
      path.metadata)

  public constructor(metadata: PathMetadata) : super(HistorizedDocument::class.java, metadata)

  public constructor(type: Class<out HistorizedDocument<AbstractDocument>>, metadata: PathMetadata)
      : super(type, metadata)

  public companion object {
    private const val serialVersionUID: Long = 2005377780

    @JvmField
    public val historizedDocument: QHistorizedDocument = QHistorizedDocument("historizedDocument")
  }
}

I get error on these lines since they are referencing HistorizedDocument without using type parameters.

public constructor(variable: String) : super(HistorizedDocument::class.java, forVariable(variable))
public constructor(metadata: PathMetadata) : super(HistorizedDocument::class.java, metadata)
xlaussel commented 2 months ago

PathInit is not a database field of course and JPA doesn't persist it because it is not a registered persisted type I guess. So it doesn't need @Transient and you should simply discard it because it is a Lambda, I just tried to add the @get:Transient (@Transient alone didn't work because it is abstract if I remember) in order to make your querydsl-ksp work.

xlaussel commented 2 months ago

If It can help you here is an abstract of the (working) java code generated by kapt for these classes:

public class QAbstractDocument extends EntityPathBase<AbstractDocument> {

    public static final QAbstractDocument abstractDocument = new QAbstractDocument("abstractDocument");

    public final deegixl.framework.jpa.QDomainEntity _super = new deegixl.framework.jpa.QDomainEntity(this);

    ....

    public QAbstractDocument(String variable) {
        super(AbstractDocument.class, forVariable(variable));
    }

    public QAbstractDocument(Path<? extends AbstractDocument> path) {
        super(path.getType(), path.getMetadata());
    }

    public QAbstractDocument(PathMetadata metadata) {
        super(AbstractDocument.class, metadata);
    }

}

public class QHistorizedDocument extends BeanPath<HistorizedDocument<? extends AbstractDocument>> {

    public static final QHistorizedDocument historizedDocument = new QHistorizedDocument("historizedDocument");

    public final deegixl.framework.jpa.QDomainEntity _super = new deegixl.framework.jpa.QDomainEntity(this);

    public final QAbstractDocument current;

    //inherited
    public final NumberPath<Long> id = _super.id;

    public final SetPath<AbstractDocument, QAbstractDocument> olds = this.<AbstractDocument, QAbstractDocument>createSet("olds", AbstractDocument.class, QAbstractDocument.class, PathInits.DIRECT2);

    @SuppressWarnings({"all", "rawtypes", "unchecked"})
    public QHistorizedDocument(String variable) {
        this((Class) HistorizedDocument.class, forVariable(variable), INITS);
    }

    @SuppressWarnings({"all", "rawtypes", "unchecked"})
    public QHistorizedDocument(Path<? extends HistorizedDocument> path) {
        this((Class) path.getType(), path.getMetadata(), PathInits.getFor(path.getMetadata(), INITS));
    }

    public QHistorizedDocument(PathMetadata metadata) {
        this(metadata, PathInits.getFor(metadata, INITS));
    }

    @SuppressWarnings({"all", "rawtypes", "unchecked"})
    public QHistorizedDocument(PathMetadata metadata, PathInits inits) {
        this((Class) HistorizedDocument.class, metadata, inits);
    }

    public QHistorizedDocument(Class<? extends HistorizedDocument<? extends AbstractDocument>> type, PathMetadata metadata, PathInits inits) {
        super(type, metadata, inits);
        this.current = inits.isInitialized("current") ? new QAbstractDocument(forProperty("current")) : null;
    }

}
IceBlizz6 commented 2 months ago

PathInit is not a database field of course and JPA doesn't persist it because it is not a registered persisted type I guess. So it doesn't need @transient and you should simply discard it because it is a Lambda

This makes sense, so i could solve this by having a blacklist of types that will be excluded when generating the Q-classes. We can try that. Maybe there are other types that could be blacklisted aswell.

If It can help you here is an abstract of the (working) java code generated by kapt for these classes:

Ah i see, i'm using kapt with

implementation("com.querydsl:querydsl-kotlin:5.1.0")
implementation("com.querydsl:querydsl-kotlin-codegen:5.1.0")

So i'm getting Kotlin classes from kapt. This project has been trying to replicate the output from that. And those do not work with the code you posted, but i see that if i skip these dependencies then i get the java Q classes and those do work.

So i will look into generics support now.

xlaussel commented 2 months ago

So i'm getting Kotlin classes from kapt. This project has been trying to replicate the output from that. And those do not work with the code you posted, but i see that if i skip these dependencies then i get the java Q classes and those do work.

Ah ok, I didn't manage to make it work with querydsl-kotlin-codegen so I use java classes. It wouls be very kind to give me an extract of your querydsl related gradle section!

This makes sense, so i could solve this by having a blacklist of types that will be excluded when generating the Q-classes. We can try that. Maybe there are other types that could be blacklisted aswell.

I suggest you to have a look at the kapt code

IceBlizz6 commented 2 months ago

This makes sense, so i could solve this by having a blacklist of types that will be excluded when generating the Q-classes. We can try that. Maybe there are other types that could be blacklisted aswell.

I suggest you to have a look at the kapt code

I don't think QueryDSL has a concept like that, it just adds any unknown type as SimplePath. I see that it ends up like this in the java Q-class.

public final SimplePath<kotlin.jvm.functions.Function1<AbstractDocument, String>> pathInit = createSimple("pathInit", kotlin.jvm.functions.Function1.class);

I could probably do this too.

There are atleast 3 options here:

  1. Blindly add any unknown types as SimplePath
  2. Blacklist specific types and give errors on the rest
  3. Make users annotate the fields that should be excluded.

Ah ok, I didn't manage to make it work with querydsl-kotlin-codegen so I use java classes.

I can understand why this wasn't in the querydsl-kotlin kapt module. Looks like the Kotlin solution for this is a bit ugly. I landed on this now

@Suppress("UNCHECKED_CAST")
public constructor(metadata: PathMetadata) : super(HistorizedDocument::class.java as Class<HistorizedDocument<AbstractDocument>>, metadata)

Essentially just casting it and suppressing the warning. So the code that led to error in Kotlin was acceptable in java. Probably because Kotlin has more type awareness than java.

It wouls be very kind to give me an extract of your querydsl related gradle section!

I didn't entirely understand this, are you asking for gradle configuration file? you can find that here build.gradle.kts it's available from the source code at the front page.

I also have a sample project where i'm testing querydsl kapt. The gradle from that project looks like this

plugins {
    kotlin("jvm") version "2.0.20"
    kotlin("kapt") version "2.0.20"
    kotlin("plugin.jpa") version "2.0.20"
    kotlin("plugin.serialization") version "2.0.20"
}

dependencies {
    implementation("jakarta.persistence:jakarta.persistence-api:3.2.0")
    implementation("com.querydsl:querydsl-core:5.1.0")
    implementation("com.querydsl:querydsl-kotlin:5.1.0")
    implementation("com.querydsl:querydsl-kotlin-codegen:5.1.0")
    kapt("com.querydsl:querydsl-apt:5.1.0:jakarta")
}
xlaussel commented 2 months ago

I didn't entirely understand this, are you asking for gradle configuration file?`

For the gradle configuration file for the generation of kotlin QClasses with kapt. If did copy this one but I have always the following error when generating the QClasses when implementation("com.querydsl:querydsl-kotlin-codegen:5.1.0") is present in the gradle file:

Execution failed for task ':kaptKotlin'.
A failure occurred while executing org.jetbrains.kotlin.gradle.internal.KaptWithoutKotlincTask$KaptExecutionWorkAction
java.lang.reflect.InvocationTargetException (no error message)

Maybe something is wrong in my entities.

xlaussel commented 2 months ago

I could probably do this too.

There are atleast 3 options here:

  1. Blindly add any unknown types as SimplePath
  2. Blacklist specific types and give errors on the rest
  3. Make users annotate the fields that should be excluded.

I think option 1 is the best, anyway if the type is not managed by JPA it will crash at startup. If the type is a user defined type it must be added. @Transient is the annotation for excluded fields.

But in my case the val is abstract and so it is not persisted and it should be excluded automatically (like does query-dsl in java classes generation for me). It is the same for val or var without backing field: They are just getter/setter functions in fact and they have nothing to do with persistence.

IceBlizz6 commented 2 months ago

We are making good progress, please try this one: ksp("io.github.iceblizz6:querydsl-ksp:a53be3d1f9") If it solves this problem above but reveals other problems then i must kindly ask you to create a new issue. (we could also just move this into discussions instead)

I tried to add generics support and it should also be excluding properties without backing field now. The code above goes through the processor without errors for me now.

When we have worked through all the issues i will release 0.0.2