OpenFeign/querydsl

Avoid depending on Spring Data and Spring Boot

Closed this issue · 15 comments

Querydsl is an upstream dependency of both Spring Data and Spring Boot. Therefore, introducing a dependency back on released versions of either of the two constitutes a cyclic dependency between the projects that is very likely to create versioning and compatibility problems for users going forward.

It would be nice if those dependencies would be removed and integration – if needed and feasible – was moved to the downstream projects.

velo commented

That ship have sailed 10 years ago with querydsl-sql-spring module.

It didn't caused any problems

velo commented

Seems the discussion was a bit more detailed on a PR

#141

It would be nice if those dependencies would be removed and integration – if needed and feasible – was moved to the downstream projects.

Bottom line is N downstream projects would represent a overhead for me that I can't afford.

I'll continue commenting here, as your tone seems more civilized here (EDIT: looks like I was mistaken 😔). Upon close inspection: the classic Querydsl Spring integration does not impose a problem. The existing dependency chain is between Spring projects is:

Boot -> Spring Data -> Spring Framework

Insert original Querydsl into the mix, you get

Boot -> Spring Data -> Querydsl -> Spring Framework

In other words: Querydsl depending on Spring Framework is not an issue at all. If, however, Querydsl starts depending on both Boot (for dependency versions) and Spring Data (for type references), you introduce a cycle between the projects and I don't think I'll have to recite architecture 1:1 why this is a problem. It is currently masked by the fact that Data depends on classic Querydsl, not your fork. If you intend your fork to be used as compile target by the community going forward, this would need to be resolved. Otherwise everyone building integration on top of Querydsl might find themselves broken, as soon as you get the idea to integrate with them yourself.

It would be nice if those dependencies would be removed and integration – if needed and feasible – was moved to the downstream projects.

Bottom line is N downstream projects would represent a overhead for me that I can't afford.

The downstream project I was referring to is Spring Data, whose Querydsl integration has existed for almost as log as Querydsl itself. In other words, you wouldn't accrue work, you'd hand it off.

Let me close by stating that I am happy that someone is taking over work and responsibility for Querydsl. It has been of tremendous use to the Spring Data community for more than a decade. I am just hoping that its development will continue in a way that recognizes its position in the ecosystem and not endanger that by short-sighted moves that make it harder for downstream projects to integrate with it.

I can only hope you consider the input, and don't immediately brush it off by deleting comments and “Ow boy, …”. What would certainly help is that you don't jump to conclusions about what you think I (we) allegedly ask you to do / consider (maintain more code / projects) but rather start a dialogue in case there's unclarity to be resolved.

velo commented

I still fail to see what is the real problem.

I totally get why dependency A -> B -> A.

What I don't understand the is the github project level issue.

Boot -> Spring Data -> Querydsl -> Spring Framework

Project level seems just an arbitrary granularity. Specially cause projects don't depend on each other.
We cool go for organization level, spring would be in a cyclic dependency with almost everything.

Now, looking at maven dependencies level:
querydsl-jpa-spring -> Boot ->Data -> querydsl-jpa -> querydsl-core -> Spring

There is no cyclic dependency there. That is exactly why I move it to new module.

So, why is that a problem?

Saying that endangers the ecosystem.... sounds a lot like drama and it doesn't explain anything.

I am surprised I have to explain this. Boot depends on the Querydsl BOM, which lists all dependencies on the used versions of every artifact. I just realized the module doesn't exist in this fork, which basically just adds another problem on top of all of this because it makes it impossible for Boot to switch to your fork. Even with that out of the picture, assume you have released some version that refers to Boot 3.2, we released Boot 3.3 pointing to your fork, now the dependency chain of a user project looks like this.

User Project ------ 3.3 -----> Boot -> Spring Data -> Querydsl
            \                   /
             - Querydsl - 3.2 -

The user project now, all of a sudden, contains references to Boot in two different versions. One (the one the user establishes) declared through a BOM, one declared through a transitive BOM reference. This is very likely to result in version mismatches in the reference libraries.

Project level seems just an arbitrary granularity. Specially cause projects don't depend on each other.

That makes the issue worse, as for a user it might appear in some case and not in others. For Maven users, it might even depend on the way they declare dependencies (via parent, via BOM) the order of declarations.

We cool go for organization level, spring would be in a cyclic dependency with almost everything.

Nope. Not at all. Making sure that projects and individual JAR dependencies build up a consistent tree is a number one in priority of the Spring team. That's what makes Spring (Boot) so usable, and that's why I chimed in here.

Now, looking at maven dependencies level:
querydsl-jpa-spring -> Boot ->Data -> querydsl-jpa -> querydsl-core -> Spring

There is no cyclic dependency there. That is exactly why I move it to new module.

That graph is incorrect as the user project is likely to depend on a particular, different Boot version directly via a parent or BOM import. In short: if someone were to use that new JAR, they'll immediately have to deal with an inconsistent dependency version problem. That's not an issue for you if you choose not to care. It will be one for the users of that particular JAR, if you care or not.

sounds a lot like drama and it doesn't explain anything.

I've opened a ticket asking to consider a problem that you create by moving this fork in particular direction. What adds drama is polemic language like “Ow boy…”.

I guess I am just puzzled why you'd start a fork of a library depended on by the largest Java application framework with good intentions but immediately start to alienate the community, you'd like your fork to be ultimately successful in. The lack of a BOM, the dependency on Boot, and the non-idiomatic repository extensions feel like a quite rude way to step into a room.

That said, feel free to totally ignore this. All I am saying is that those moves might not be particularly helpful, considering a large part of the Querydsl user base are Spring (Boot) users that are used to libraries nicely fitting into the otherwise well-organized ecosystem.

velo commented

I am surprised I have to explain this. Boot depends on the Querydsl BOM, which lists all dependencies on the used versions of every artifact. I just realized the module doesn't exist in this fork, which basically just adds another problem on top of all of this because it makes it impossible for Boot to switch to your fork. Even with that out of the picture, assume you have released some version that refers to Boot 3.2, we released Boot 3.3 pointing to your fork, now the dependency chain of a user project looks like this.

User Project ------ 3.3 -----> Boot -> Spring Data -> Querydsl
            \                   /
             - Querydsl - 3.2 -

The user project now, all of a sudden, contains references to Boot in two different versions. One (the one the user establishes) declared through a BOM, one declared through a transitive BOM reference. This is very likely to result in version mismatches in the reference libraries.

Project level seems just an arbitrary granularity. Specially cause projects don't depend on each other.

That makes the issue worse, as for a user it might appear in some case and not in others. For Maven users, it might even depend on the way they declare dependencies (via parent, via BOM) the order of declarations.

We cool go for organization level, spring would be in a cyclic dependency with almost everything.

Nope. Not at all. Making sure that projects and individual JAR dependencies build up a consistent tree is a number one in priority of the Spring team. That's what makes Spring (Boot) so usable, and that's why I chimed in here.

Now, looking at maven dependencies level:
querydsl-jpa-spring -> Boot ->Data -> querydsl-jpa -> querydsl-core -> Spring
There is no cyclic dependency there. That is exactly why I move it to new module.

That graph is incorrect as the user project is likely to depend on a particular, different Boot version directly via a parent or BOM import. In short: if someone were to use that new JAR, they'll immediately have to deal with an inconsistent dependency version problem. That's not an issue for you if you choose not to care. It will be one for the users of that particular JAR, if you care or not.

sounds a lot like drama and it doesn't explain anything.

I've opened a ticket asking to consider a problem that you create by moving this fork in particular direction. What adds drama is polemic language like “Ow boy…”.

I guess I am just puzzled why you'd start a fork of a library depended on by the largest Java application framework with good intentions but immediately start to alienate the community, you'd like your fork to be ultimately successful in. The lack of a BOM, the dependency on Boot, and the non-idiomatic repository extensions feel like a quite rude way to step into a room.

That said, feel free to totally ignore this. All I am saying is that those moves might not be particularly helpful, considering a large part of the Querydsl user base are Spring (Boot) users that are used to libraries nicely fitting into the otherwise well-organized ecosystem.

Ow, the bom.... Hrmmm, okay, need to read everything else and think about it.

Ok, haven't considered the boom file

See, explaining the problem actually helps.

@odrotbohm Wouldn't the problem be solved by setting the scope of the spring-data-jpa dependency to provided? That would avoid the version issues since it will always use the version available on the classpath during runtime.

velo commented

FWIW, that won't change the bom file at all, as the bom doesn't include any of the spring dependencies

Sure, but if someone uses querydsl-jpa-spring together with spring-data-jpa they might get version conflicts. That's the only way I can think of this being problematic. Or am I missing something?

There might also be an issue with having querydsl-jpa-spring in one version and then spring pulling querydsl in a different version.

If the dependecy tree would look like

User project -> querydsl-jpa-spring -> spring-data-jpa -> querydsl

without a direct dependency from querydsl-jpa-spring to querydsl but resolving it transitively through spring. The module would then be more of a spring extension, rather then a querydsl one.

I guess this is a what came first the chicken or the egg issue.

velo commented

Sure, but if someone uses querydsl-jpa-spring together with spring-data-jpa they might get version conflicts. That's the only way I can think of this being problematic. Or am I missing something?

But that is not a cyclic dependency and you might get into the same scenario with JPA, with spring recommending one version and we a diferent one.

There might also be an issue with having querydsl-jpa-spring in one version and then spring pulling querydsl in a different version.

If the dependecy tree would look like

User project -> querydsl-jpa-spring -> spring-data-jpa -> querydsl

without a direct dependency from querydsl-jpa-spring to querydsl but resolving it transitively through spring. The module would then be more of a spring extension, rather then a querydsl one.

I guess this is a what came first the chicken or the egg issue.

Note querydsl bom doesn't include information about transitive dependencies. It only lock version for querydsl-*

Users can pick their spring, JPA, drivers versions.

I think you can dance around the issue and apply all sorts of band-aid, but you're not going to solve the problem you introduced with those tricks.

A real solution would be moving the code where it needs to live to satisfy the dependency arrangement. That, theoretically, would be Spring Data, but as I've outlined that we think the feature proposed here breaks with the abstractions and ideas of how repositories fit into a software architecture in general. So it's unlikely that a proposal to add it there will be accepted.

velo commented

we think the feature proposed here breaks with the abstractions ....

we who?

We who?

The Spring Data team.

velo commented

Oh, ok, so team is having a conversation about this. That probably explain why I don't see to be getting enough information (like what is the problem)

Can I take a look on the discussion so I might understand what is the problem my change causes?

Just want to see what is broken by this change.

velo commented

The PR they could show the issue was closed, so reopen if when you can show me the issue