infobip/infobip-spring-data-querydsl

[bug] JDBC Configured NamingStrategy produces errors.

Closed this issue · 9 comments

bilak commented

Hello,
when using PascalCaseNamingStrategy all existing queries are broken due to fact that the field names are generated inorrectly.
When I configure the naming strategy to use spring's default then old queries are ok but if I create new query using repository.queryOne(...) then it's not working because of bad field names.

here is test (note: branch is feature/querydsl-infobip).

Generated query is

select UserEntity.Id, UserEntity.FirstName, UserEntity.LastName
from users UserEntity
where UserEntity.FirstName = ?

but it should be

select UserEntity.Id, UserEntity.first_name, UserEntity.last_name
from users UserEntity
where UserEntity.first_name = ?

Current annotation processor is set to support pascal naming only (https://github.com/infobip/infobip-spring-data-querydsl/blob/master/infobip-spring-data-jdbc-annotation-processor/src/main/java/com/infobip/spring/data/jdbc/annotation/processor/SpringDataJdbcQuerydslNamingStrategy.java).
To support use case from this issue SpringDataJdbcQuerydslNamingStrategy would have to be overridable/configurable:

  • user would need to define his own com.querydsl.sql.codegen.NamingStrategy
  • this custom naming strategy would need to be present on annotation processor classpath during compilation
  • user would need to be able to tell annotation processor to use it.

All those bullets are relatively hard to achieve in Maven:

  • user would have to override maven compiler plugin
    • add the annotation processor explicitly
    • set the option on annotation processor
    • add dependency to the jar that contains the custom NamingStrategy inside maven compiler plugin definition.

User would then have to do this on every project that wants to use this NamingStrategy.
This doesn't scale well when you want to use same strategy across multiple projects.

To avoid forcing this onto users I believe another approach would be better:

  • make annotation processor easily modifiable by creating a new module (e.g. infobip-spring-data-jdbc-annotation-processor-common) that would contain everything needed to easily create an annotation processor with custom NamingStrategy
  • user would then define a new maven project for their own annotation processor with the custom NamingStrategy
  • in projects that would use this annotation processor user would just have to exclude default annotation processor and add dependency to their own annotation processor

This I believe scales better and would most likely provide a clear path in future for easier extensions.

What do you think?

bilak commented

Maybe just silly question as I don't see to the core of spring-data, but how spring guys are doing this?
I would imagine that they introspect all classes being used in repository definition (entities/projections) and then based on naming strategy generate some metadata (name of field vs name of database field) that are used then while constructing queries. So if it's possible to do it like this maybe I would choose this path. So you would have option to create bean of naming strategy and based on that generate metadata. But as I said I don't know the internals.

Spring Data doesn't support static model of queries in any form (like JOOQ or Querydsl does) so they don't have this problem.
Querydsl JPA generates static model (Q classes) based on entity classes and Querydsl SQL needs to connect to database to generate it. Annotation processor in this project imitates what Querydsl JPA annotation processor does but it's significantly less feature rich.

bilak commented

Ah that's true, I forgot this is querydsl not spring data.
Well maybe if there is a place in between querydsl and jdbc connection to take metadata of table it could be possible to get uniformed metadata (I used this project in past for something similar) and based on that metadata reuse the NamingStrategy to create query. Then maybe you can have the default naming strategy of spring nad it could behave very similar. But that's just the idea, again I'm not in detail and don't know very well how to connect all those metadata together.

This was a conscious decision, the section I mentioned links to how it can be done with maven alone and without annotation processor by connecting to database (it's how Querydsl SQL module or JOOQ work). The thing is: I don't want to force users to connect to database during build phase.
Anyway, I'll add support for custom annotation processors hopefully next week.

This should be solvable in 5.3.0 (just released) with https://github.com/infobip/infobip-spring-data-querydsl#AnnotationProcessor. Can you give it a shot once it's available on Maven Central?

@bilak any update?

@bilak try

@ProjectColumnCaseFormat(CaseFormat.LOWER_UNDERSCORE)

with 5.4.0. It works and generates the query as you specified, but test still fails for some reason.