IceBlizz6 / querydsl-ksp

KSP for QueryDSL
0 stars 0 forks source link

Hibernate UserType not managed #4

Open xlaussel opened 2 days ago

xlaussel commented 2 days ago

I have a user defined type:

class A

class AType: UserType<A> {
...
}

When I use it as entity field like:

@org.hibernate.annotations.Type(value = AType::class)
val a:A=A()

I have the following error: [ksp] java.lang.IllegalStateException: Error processing Test.a : 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.

Maybe you should just manage any unrecognized type as a SimplePath. So is not necessary to parse the org.hibernate.annotations.Type annotation.

IceBlizz6 commented 2 days ago

Maybe you should just manage any unrecognized type as a SimplePath. So is not necessary to parse the org.hibernate.annotations.Type annotation.

I hear you, but i want to hold off on that atleast for just a little bit longer. My main concern is that it will hide a lot of issues. The issues you are reporting now are very useful, and i suspect some of them would not be noticed if they were wrapped in SimplePath by default.

For now i tried to detect annotation for Type and Converter. please try this commit to see if it is working ksp("io.github.iceblizz6:querydsl-ksp:c0add3d449")

If we encounter more of these then i will likely change my mind and just go SimplePath as default.

xlaussel commented 2 days ago

Works ok but:

1/ I have now the following message when building: java.lang.IllegalStateException: Storage for [/home/xlaussel/srcs/newg/server/build/kspCaches/main/symbolLookups/id-to-file.tab] is already registered The first tries of your new version were OK so I think It should be an IDE problem but I don't know how to solve it.

2/ I have an issue with generics on a complicated case. The ksp parsing and code generation is ok but the QClasses not (not compiling). I tried to reproduce it with a basic schema (but not sure the bug is reproduced because the preceding error is blocking me) :

@Entity
@DiscriminatorColumn
class A {
    @Id
    @GeneratedValue(strategy = GenerationType.SEQUENCE)
    @Column(name = "id", nullable = false)
    var id: Long? = null
}

@MappedSuperclass
abstract class B<E:B<E>> : A()

@Entity
class C : B<C>()

3/ I have also an issue with Date property type (not managed). I changed it to LocalDateTime but maybe you should manage this and the other deprecated ones like Calendar, Time... I think it is not a big deal now. You can have a look here for an exhaustive list of JPA basic attributes and Hibernate additional ones: https://docs.jboss.org/hibernate/orm/6.6/introduction/html_single/Hibernate_Introduction.html#basic-attributes There are also the spatial extensions related types for last step !!

IceBlizz6 commented 2 days ago

1)

I have now the following message when building: java.lang.IllegalStateException: Storage for [/home/xlaussel/srcs/newg/server/build/kspCaches/main/symbolLookups/id-to-file.tab] is already registered

I also get this one occasionally, There is a pending issue for this in the google KSP github project here: https://github.com/google/ksp/issues/2072

The proposed solution to run jps | grep -E 'KotlinCompileDaemon' | awk '{print $1}' | xargs kill -9 || true

Works for me, but tbh i don't actually understand what it does, so use at your own risk :) As far as i can tell it seems a bit random and i had it on other projects running KSP aswell. If someone can point out a specific part of my code that could cause this issue then i'd love to look into it. But for now it's not something i could easily troubleshoot, and i don't even know if it's something i can do on this project that would stop this error.

2) I'm also getting errors in the output file, will look into this now. Seems like i didn't account properly for multiple layers of generics.

3) Thanks for the link, I noticed their message there: We’re begging you to use types from the java.time package instead of anything which inherits java.util.Date.

Your migration to java.time may be a good idea anyway. But i realize not everyone may have the possibility of migrating so i probably should look into supporting some legacy types.

Overall your effort to help me on this project is really appreciated. Are you able to keep going like this until we fixed all the issues?

IceBlizz6 commented 2 days ago
abstract class B<E:B<E>> : A()

This was a bit tricky to figure out how to process. I changed my approach a bit, now using * to avoid a lot of the constraint issues. It seems promising, please try this commit: ksp("io.github.iceblizz6:querydsl-ksp:de931fd976")

xlaussel commented 1 day ago

Are you able to keep going like this until we fixed all the issues?

Yes, I am interested in having it to work :) When it will be finalized, with all the bugs fixed and the types supported, maybe you should try again to integrate it in the official querydsl repos.

Maybe we should communicate by mail, it would be easier

xlaussel commented 1 day ago

Also another issue with derivation: The fields of the parent class (ie id) are not present in the child QClass:

@MappedSuperclass
class Super {
    @Id
    @GeneratedValue(strategy = GenerationType.SEQUENCE)
    @Column(name = "id", nullable = false)
    var id: Long? = null
}

@Entity
class Child : Super() {
  var name: String=""
}

All the super fields should be copied like this:

public final val id  =_super.id

//or
public final val id
  get() = _super.id

A very elegant Kotlin way avoiding fields copy (but less lazy) would be :

public interface QSuperInterface {
    val id: NumberPath<Long>
}

public class QSuper(type: Class<out Super>,metadata : PathMetadata) : EntityPathBase<Super>(type,metadata),  QSuperInterface {
    override val id: NumberPath<Long> = createNumber("id", Long::class.java)
    constructor(path: Path<out Super>) : this(path.type, path.metadata)
    //...
}

public interface QChildInterface : QSuperInterface {
    val name: StringPath
}

public class QChild private constructor(type: Class<out Child>,metadata : PathMetadata,val _super : QSuper=QSuper(type,metadata)) :
    EntityPathBase<Child>(type,metadata) , QChildInterface, QSuperInterface by _super {
    constructor(path: Path<out Child>) : this(path.type, path.metadata)
    constructor(metadata: PathMetadata) : this(Child::class.java,metadata)
    constructor(variable: String) : this(Child::class.java,com.querydsl.core.types.PathMetadataFactory.forVariable(variable))

    //Or else make the primary one public but it exposes creation fo the _super
    constructor(type: Class<out Child>,metadata : PathMetadata) : this(type,metadata,QSuper(type,metadata))

    override val name: StringPath = createString("name")
}

Note that it must be done for the whole hierarchy.