-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update Spring Boot 2.7 → 3.3 & Other Java libs #139
Conversation
Syy löytyi: Ajoin Mavenia Java 21 versiolla |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really comprehensive commit message! The code style has also been greatly improved.
However, I would have liked to split this pull request into several commits. In the next PR, more attention could be paid to it.
Reviewed 76 of 236 files at r1, all commit messages.
Reviewable status: 76 of 236 files reviewed, 14 unresolved discussions (waiting on @Huulivoide)
src/main/java/fi/hsl/jore/importer/config/jobs/BatchConfig.java
line 26 at r1 (raw file):
batchDataSource = new EmbeddedDatabaseBuilder() .setType(EmbeddedDatabaseType.HSQL) .addScript("/org/springframework/batch/core/schema-hsqldb.sql")
Seems a reasonable change to use HSQLDB instead of the MapJobRepositoryFactoryBean
which according to javadocs is only meant for testing and rapid prototyping. 👍
src/main/java/fi/hsl/jore/importer/config/jooq/converter/date_range/DateRange.java
line 6 at r1 (raw file):
import java.time.LocalDate; public record DateRange(Range<LocalDate> range) {
Clever to replace the use of Immutable with a Java (immutable) record. 👍
src/main/java/fi/hsl/jore/importer/config/jooq/converter/date_range/DateRange.java
line 16 at r1 (raw file):
/* return ImmutableDateRange.builder() .range(Range.closedOpen(LocalDate.EPOCH, LocalDate.EPOCH))
IMO these commented blocks can be removed.
src/main/java/fi/hsl/jore/importer/feature/api/dto/JobStatus.java
line 5 at r1 (raw file):
import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.time.LocalDateTime;
Import statement order is no more logical.
src/main/java/fi/hsl/jore/importer/feature/batch/common/GenericImportWriter.java
line 34 at r1 (raw file):
} public void write(final FluentIterable<? extends ENTITY> items) {
I'd change this method to private
since FluentIterable
is IMO not a good choice for a parameter type in public method signatures (unlike basic Iterable
). Alternatively, I would replace FluentIterable
with basic Iterable
.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4JourneyPatternRepository.java
line 32 at r1 (raw file):
).values((UUID) null, null)); for (Jore4JourneyPattern journeyPattern : journeyPatterns) {
Missing final
here. There should be one according to "Coding Conventions":
https://github.com/HSLdevcom/jore4-jore3-importer?tab=readme-ov-file#coding-conventions
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4JourneyPatternStopRepository.java
line 47 at r1 (raw file):
); for (Jore4JourneyPatternStop journeyPatternStop : journeyPatternStops) {
missing final
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4RouteGeometryRepository.java
line 36 at r1 (raw file):
); for (Jore4RouteGeometry routeGeometry : routeGeometries) {
missing final
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4RouteRepository.java
line 50 at r1 (raw file):
); for (Jore4Route route : routes) {
missing final
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4ScheduledStopPointRepository.java
line 31 at r1 (raw file):
@Override public void insert(final Iterable<? extends Jore4ScheduledStopPoint> stopPoints) { for (Jore4ScheduledStopPoint stopPoint : stopPoints) {
missing final
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4TimingPlaceRepository.java
line 5 at r1 (raw file):
import fi.hsl.jore.importer.feature.common.converter.IJsonbConverter; import fi.hsl.jore.importer.feature.jore4.entity.Jore4TimingPlace; import java.util.stream.Stream;
These import additions seem unnecessary.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4TimingPlaceRepository.java
line 16 at r1 (raw file):
import java.util.List; import java.util.UUID; import org.springframework.util.StreamUtils;
These import addition seems unnecessary.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4TimingPlaceRepository.java
line 45 at r1 (raw file):
.values((UUID) null, null, null)); for (Jore4TimingPlace timingPlace : timingPlaces) {
missing final
src/test/java/fi/hsl/jore/importer/config/jooq/converter/geometry/LineStringConverterTest.java
line 25 at r1 (raw file):
new Coordinate(60.168896355, 24.945549266, 0)); final var output = CONVERTER.to(line);
When switching to use var
s, I think the "Coding conventions" in README.md
should be updated.
https://github.com/HSLdevcom/jore4-jore3-importer?tab=readme-ov-file#coding-conventions
* Spring Boot - `@Qualified` `TransactionManagers` for all DBs * Spring Batch 1. Updated Spring Batch config: Map based `JobRepository` is deprecated → HSQLDB used as the repo. 2. Migrated ItemWriters to use the `Chunk<>` API. 3. Migrated Repositiories to use `Iterable` as input type, so that Chunk<> Instanses can be directly passed in, without having to perform `toList()` transaformation. 4. Change `Instant` to `LocalDateTime` on `JobStatus`. 5. Swap Repo batch-insert 0-insert guard order: Before: `if (list.isEmpty()) { return; } for-loop` Now: `for-loop; if (batch.size() > 0) { execute() }` This is needed because if `List` → `Iterable` change of point 3. * jOOQ - Disabled type generation for new spatial type support → Continue using the current Binding-class. - DateRange: Newer version of jOOQ do not work with org.immutable types → Replaced `DateRange` org.immutable implementation, with java Record. - Fix config to use `<binding>` instead of `<converter>`. * Maven & other libs - Guava: 33.0.0 → 33.1.0 - vavr: 0.10.3 → 0.10.4 - immutables: 2.10.0 → 2.10.1 - geotools: 29.1 → 31.1 - wiremock: 2.27.2 → 3.6.0 And switch to -standalone version as to not have conflicting Netty versions with Spring. - HSQLDB: Added version 2.7.3 - Maven plugins: - Compiler: 3.11.0 → 3.13.0 - Properties: 1.0.0 → 1.2.0 - Enforcer: 3.0.0 → 3.5.0 - Enforced Maven version to be the latest Maven 3 version 3.6.3 - BuildHelper: 3.2.0 → 3.6.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very solid PR! :lgtm
Reviewed 13 of 13 files at r2, all commit messages.
Reviewable status: 76 of 236 files reviewed, 14 unresolved discussions (waiting on @Huulivoide)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 76 of 236 files reviewed, 14 unresolved discussions (waiting on @jarkkoka)
src/main/java/fi/hsl/jore/importer/config/jobs/BatchConfig.java
line 26 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
Seems a reasonable change to use HSQLDB instead of the
MapJobRepositoryFactoryBean
which according to javadocs is only meant for testing and rapid prototyping. 👍
Done.
src/main/java/fi/hsl/jore/importer/config/jooq/converter/date_range/DateRange.java
line 6 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
Clever to replace the use of Immutable with a Java (immutable) record. 👍
Done.
src/main/java/fi/hsl/jore/importer/config/jooq/converter/date_range/DateRange.java
line 16 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
IMO these commented blocks can be removed.
Done.
src/main/java/fi/hsl/jore/importer/feature/batch/common/GenericImportWriter.java
line 34 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
I'd change this method to
private
sinceFluentIterable
is IMO not a good choice for a parameter type in public method signatures (unlike basicIterable
). Alternatively, I would replaceFluentIterable
with basicIterable
.
Varsinainen Fluent implementaatio tehty privaks, ja lisätty public Iterable overloadi
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4JourneyPatternRepository.java
line 32 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
Missing
final
here. There should be one according to "Coding Conventions":https://github.com/HSLdevcom/jore4-jore3-importer?tab=readme-ov-file#coding-conventions
Done.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4JourneyPatternStopRepository.java
line 47 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
missing
final
Done.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4RouteGeometryRepository.java
line 36 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
missing
final
Done.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4RouteRepository.java
line 50 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
missing
final
Done.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4ScheduledStopPointRepository.java
line 31 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
missing
final
Done.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4TimingPlaceRepository.java
line 16 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
These import addition seems unnecessary.
Done.
src/main/java/fi/hsl/jore/importer/feature/jore4/repository/Jore4TimingPlaceRepository.java
line 45 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
missing
final
Done.
src/test/java/fi/hsl/jore/importer/config/jooq/converter/geometry/LineStringConverterTest.java
line 25 at r1 (raw file):
Previously, jarkkoka (Jarkko Kaura) wrote…
When switching to use
var
s, I think the "Coding conventions" inREADME.md
should be updated.https://github.com/HSLdevcom/jore4-jore3-importer?tab=readme-ov-file#coding-conventions
Vaihdettu var:it pois
@Qualified
TransactionManagers
for all DBsJobRepository
is deprecated → HSQLDB used as the repo.Chunk<>
API.Iterable
as input type, so that Chunk<> Instanses can be directly passed in, without having to performtoList()
transaformation.Instant
toLocalDateTime
onJobStatus
.Before:
if (list.isEmpty()) { return; } for-loop
Now:
for-loop; if (batch.size() > 0) { execute() }
This is needed because ifList
→Iterable
change of point 3.DateRange
org.immutable implementation, with java Record.<binding>
instead of<converter>
.This change is