Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with jOOQ 3.15 and later #4727

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

shakuzen
Copy link
Member

@shakuzen shakuzen commented Feb 7, 2024

Implementing jOOQ's DSLContext interface was causing revlock and subsequently frequent breaking. Now that jOOQ OSS supported versions no longer work with Java 8, we cannot compile against them to update our implementation MetricsDSLContext. This switches to extending DefaultDSLContext instead which eliminates a lot of boilerplate implementation and should be more compatible. A sample has been added that runs the tests against the latest version of jOOQ to ensure MetricsDSLContext works with it while compiling against the newest OSS version of jOOQ that supports Java 8.

Resolves gh-3828
Obviates gh-4508 for now

This does break binary compatibility, as reported by japi. I think that's going to be unavoidable given the circumstances and it is limited to the jOOQ integration. Users will need to recompile their code that uses MetricsDSLContext. I am not aware of any libraries that configure the jOOQ integration, so it should mostly be used in application code, making the recompile requirement less of an issue. Typical practice is to recompile application code when changing dependency versions.

@shakuzen shakuzen force-pushed the jooq-unlock branch 2 times, most recently from 36916c3 to 1b4143d Compare February 7, 2024 12:19
@shakuzen shakuzen marked this pull request as ready for review February 7, 2024 12:20
@shakuzen shakuzen force-pushed the jooq-unlock branch 2 times, most recently from b3d9da5 to 32c8869 Compare February 7, 2024 12:26
Implementing jOOQ's `DSLContext` interface was causing revlock and subsequently frequent breaking. Now that jOOQ OSS supported versions no longer work with Java 8, we cannot compile against them to update our implementation `MetricsDSLContext`. This switches to extending `DefaultDSLContext` instead which eliminates a lot of boilerplate implementation and should be more compatible. A sample has been added that runs the tests against the latest version of jOOQ to ensure MetricsDSLContext works with it while compiling against the newest OSS version of jOOQ that supports Java 8.
@shakuzen
Copy link
Member Author

shakuzen commented Feb 8, 2024

It looks like since jOOQ 3.17, open source versions only support Java 17+. I've updated the sample to only run with 17+.

@shakuzen shakuzen mentioned this pull request Feb 8, 2024
* @author Jon Schneider
* @author Johnny Lim
*/
class MetricsDSLContextTest {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is copied from micrometer-core with only import/JavaDoc changes.

@shakuzen shakuzen merged commit e4b625f into micrometer-metrics:main Feb 8, 2024
6 checks passed
@shakuzen shakuzen deleted the jooq-unlock branch February 8, 2024 08:11
@@ -136,4972 +116,678 @@ public DSLContext tags(Iterable<Tag> tags) {
}

@Override
public Schema map(Schema schema) {
return context.map(schema);
public <R extends Record> SelectWhereStep<R> selectFrom(Table<R> table) {
Copy link

@ferdinand-swoboda ferdinand-swoboda May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason this is not

Suggested change
public <R extends Record> SelectWhereStep<R> selectFrom(Table<R> table) {
public <R extends Record> SelectWhereStep<R> selectFrom(TableLike<R> table) {

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we compile against jooq 3.14 and TableLike does not exist in that version.

This pull request was closed.
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.

Support jOOQ 3.15 and later
2 participants