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

Dependency update #36

Merged
merged 5 commits into from
Jun 17, 2024
Merged

Dependency update #36

merged 5 commits into from
Jun 17, 2024

Conversation

Huulivoide
Copy link
Contributor

@Huulivoide Huulivoide commented Jun 14, 2024

Update Maven plugins

Bump Java version to 17

Update library versions

  • Retrofit: Kept at 2.9.0, pending Spring Boot 3
  • oauth2-oidc-sdk: 9.7 → 11.12
  • wiremock
    • Version update: 2.27.2 → 3.6.0
    • Switched to standalone version, to prevent Netty version conflict with Spring updates.
  • Mockito
    • Version update: 3.11.1 → 5.12.0
    • Switched to core version, as the inline-mocker has been the default mocker implementation since 5.0.0.

Migrate to Spring Boot 3

  • Spring boot updated to version 3.3.0
  • OpenAPI Generator upated to version 7.6.0
    • Swicthed generator to use spring-boot as lib
    • Configured Spring Boot V3 to be in use
  • Updated retrofit to version 2.11.0
  • Migrated from Springfox to Springdoc v2.5.0
  • Support for property 'pring.session.store-type' has been removed from Spring. So it has been replaced with a custom 'session.enabled' property. (Used to disable sesisons during test runs).
  • Removed superfluous @ExtendWith(SpringExtension::class) annotations.
  • Replaced javax imports with jakarta ones.

This change is Reviewable

Copy link
Contributor

@jarkkoka jarkkoka left a comment

Choose a reason for hiding this comment

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

Reviewed 21 of 22 files at r1, all commit messages.
Reviewable status: 21 of 22 files reviewed, 3 unresolved discussions (waiting on @Huulivoide)


-- commits line 28 at r1:
typo: pringspring


src/main/kotlin/fi/hsl/jore4/auth/Jore4AuthApplication.kt line 23 at r1 (raw file):

    @ConditionalOnProperty(prefix = "session", name = ["enabled"])
    open fun getDataSource(): DataSource {
        databaseProperties.assertAllGood()

To me this call seems to be ineffective. Even though a db property is empty or completely missing, the expected error messages (Assert.hasText(...) do not appear in startup at least when I tested locally.)


src/main/kotlin/fi/hsl/jore4/auth/common/DatabaseProperties.kt line 21 at r1 (raw file):

    fun assertAllGood() {
        Assert.hasText(hostname, "Expected app property db.hostname to be defined!")

IMO more idiomatic way of doing this would be:

require(hostname.isNotEmpty(), "...")

Copy link
Contributor

@jarkkoka jarkkoka left a comment

Choose a reason for hiding this comment

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

Reviewable status: 21 of 22 files reviewed, 3 unresolved discussions (waiting on @Huulivoide)


src/main/kotlin/fi/hsl/jore4/auth/common/DatabaseProperties.kt line 21 at r1 (raw file):

Previously, jarkkoka (Jarkko Kaura) wrote…

IMO more idiomatic way of doing this would be:

require(hostname.isNotEmpty(), "...")

Sorry, there is an error in the previous comment. Actually, the following would be a correct suggestion:

require(!hostname.isNullOrBlank(), "...")

...but unfortunately this correction is not so easy to read.

Copy link
Contributor

@jarkkoka jarkkoka left a comment

Choose a reason for hiding this comment

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

Reviewable status: 21 of 22 files reviewed, 4 unresolved discussions (waiting on @Huulivoide)


pom.xml line 21 at r1 (raw file):

        <java.version>17</java.version>

        <!-- 3.6.3 is the last version of maven 3 that has been relased. Back in 2019 😬 -->

typo: relasedreleased

Copy link
Contributor

@jarkkoka jarkkoka left a comment

Choose a reason for hiding this comment

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

Reviewable status: 21 of 22 files reviewed, 4 unresolved discussions (waiting on @Huulivoide)


src/main/kotlin/fi/hsl/jore4/auth/Jore4AuthApplication.kt line 23 at r1 (raw file):

Previously, jarkkoka (Jarkko Kaura) wrote…

To me this call seems to be ineffective. Even though a db property is empty or completely missing, the expected error messages (Assert.hasText(...) do not appear in startup at least when I tested locally.)

I tested this again more thoroughly and it seems that when the property (eg db.name) is empty it works. However, if a db property is missing, these assertion messages are ineffective.

Copy link
Contributor

@jarkkoka jarkkoka left a comment

Choose a reason for hiding this comment

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

Reviewable status: 21 of 22 files reviewed, 4 unresolved discussions (waiting on @Huulivoide)


src/main/kotlin/fi/hsl/jore4/auth/common/DatabaseProperties.kt line 21 at r1 (raw file):

Previously, jarkkoka (Jarkko Kaura) wrote…

Sorry, there is an error in the previous comment. Actually, the following would be a correct suggestion:

require(!hostname.isNullOrBlank(), "...")

...but unfortunately this correction is not so easy to read.

...or even more correctly:

require(!name.isNullOrBlank()) { "..." }

@Huulivoide Huulivoide force-pushed the DependencyUpdate branch 2 times, most recently from 4afc7af to 59c5efb Compare June 17, 2024 09:57
* Retrofit: Kept at 2.9.0, pending Spring Boot 3
* oauth2-oidc-sdk: 9.7 → 11.12
* wiremock
  - Version update: 2.27.2 → 3.6.0
  - Switched to standalone version, to prevent Netty version conflict
    with Spring updates.
* Mockito
  - Version update: 3.11.1 → 5.12.0
  - Switched to core version, as the inline-mocker has been the
    default mocker implementation since 5.0.0.
* Spring boot updated to version 3.3.0
* OpenAPI Generator upated to version 7.6.0
  - Swicthed generator to use spring-boot as lib
  - Configured Spring Boot V3 to be in use
* Updated retrofit to version 2.11.0
* Migrated from Springfox to Springdoc v2.5.0
* Support for property 'spring.session.store-type' has been
 removed from Spring. So it has been replaced with a custom
 'session.enabled' property. (Used to disable sesisons during test
  runs).
* Removed superfluous `@ExtendWith(SpringExtension::class)` annotations.
* Replaced javax imports with jakarta ones.
Copy link
Contributor Author

@Huulivoide Huulivoide left a comment

Choose a reason for hiding this comment

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

Reviewable status: 18 of 23 files reviewed, 2 unresolved discussions (waiting on @jarkkoka)


src/main/kotlin/fi/hsl/jore4/auth/Jore4AuthApplication.kt line 23 at r1 (raw file):

Previously, jarkkoka (Jarkko Kaura) wrote…

I tested this again more thoroughly and it seems that when the property (eg db.name) is empty it works. However, if a db property is missing, these assertion messages are ineffective.

Done.


src/main/kotlin/fi/hsl/jore4/auth/common/DatabaseProperties.kt line 21 at r1 (raw file):

Previously, jarkkoka (Jarkko Kaura) wrote…

...or even more correctly:

require(!name.isNullOrBlank()) { "..." }

Done.

Copy link
Contributor

@jarkkoka jarkkoka left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 22 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Huulivoide)

@Huulivoide Huulivoide merged commit 9f3325c into main Jun 17, 2024
5 checks passed
@Huulivoide Huulivoide deleted the DependencyUpdate branch June 17, 2024 13:34
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.

2 participants