spring-projects-experimental / spring-fu

Configuration DSLs for Spring Boot
Apache License 2.0
1.67k stars 138 forks source link

feat(kofu.jooq): add primitive support for JOOQ api in kofu #325

Closed davinkevin closed 3 years ago

davinkevin commented 4 years ago

This commit integrate a pretty simple JOOQ dsl and a kofu example with servlet for "blocking" version.

@sdeleuze | @aneveu : I think the Datasource should be externalized from JdbcDsl. Thanks to that, we should be able to use something like this:


val app = webApplication {
    datasource {
        schema = listOf("classpath:db/h2/schema.sql")
        data = listOf("classpath:db/h2/data.sql")
        generateUniqueName = false
        name = "foo"
    }
}

And if someone want to enable jdbc support, we can do this:

val app = webApplication {
    datasource {
        …
    }
    jdbc { … }
}

or with jooq:

And if someone want to enable jdbc support, we can do this:

val app = webApplication {
    datasource {
        …
    }
    jooq { … }
}

This solution could propose to use both system in same time of course (why not? Could be good during migration between both systems):

val app = webApplication {
    datasource {
        …
    }
    jooq { … }
    jdbc { … }
}

This is out of the scope of this PR but is part of some foundation for all Datasource connection for kofu/jafu.

PS: Due to this important question, I choose to not implement tests because all can be changed depending of your answer.

Thanks

fteychene commented 3 years ago

The modelisation around Datasource is in discussion with also #332 I think we could merge this after 4.3 is released since we want to work on the Datasource design for next version. This will integrate Jooq and keep our work to work for both jooq and jdbc. What do you think @sdeleuze ?

sdeleuze commented 3 years ago

Yeah good idea let's do that 👍🏼

davinkevin commented 3 years ago

Let me know if you want something special in this merge request. I'll be happy to participate on this part if possible.

fteychene commented 3 years ago

To be more idiomatic with #332 datasource should be in jooq dsl

davinkevin commented 3 years ago

Ok, this is what I've made in the current merge request, by doing an important duplication between jdbc and jooq at the datasource level. I will "backport" the evolution made in #332 in this MR and implement required tests if this is ok for all of you.

Thx 👍

davinkevin commented 3 years ago

All evolutions from #332 is now in this PR. I've added unit test for PG and H2.

If you want some evolution about this JOOQ support, let me know @fteychene | @aneveu

Sorry for the delay, I was quite busy but I finally "done" it 🎉

aneveu commented 3 years ago

@davinkevin no problem, many thanks for contributing it! It looks all good to me, I just noticed a detail: would you mind replacing your class headers by @author Kevin Davin? Just to be aligned with the convention in place :)

davinkevin commented 3 years ago

Done 👍.

Merry Christmas btw 😉🎄.

aneveu commented 3 years ago

Merry Christmas to you too and many thanks again for your contribution!