-
Notifications
You must be signed in to change notification settings - Fork 3
PR for v1.5.9 #387
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
base: main
Are you sure you want to change the base?
PR for v1.5.9 #387
Conversation
…ndencycheck-12.x Update plugin org.owasp.dependencycheck to v12.0.2
…mbok-8.x Update plugin io.freefair.lombok to v8.12.1
WalkthroughUpgrades Gradle, plugins, and many dependencies; sets Java source/target to 17; updates Gradle wrapper to 9.1.0 and changes wrapper scripts to use Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant Wrapper as gradlew / gradlew.bat
participant Java as java
participant GradleJar as gradle/wrapper/gradle-wrapper.jar
participant GradleDist as Gradle distribution
Dev->>Wrapper: ./gradlew <task>
Wrapper->>Java: exec "java -jar $APP_HOME/gradle/wrapper/gradle-wrapper.jar" <args>
Java->>GradleJar: run wrapper JAR (wrapper main)
GradleJar->>GradleDist: download/bootstrap Gradle distribution if needed
GradleDist->>Dev: execute requested build task / return status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ework.data-spring-data-elasticsearch-5.x Update dependency org.springframework.data:spring-data-elasticsearch to v5.4.3
…ework-spring-messaging-6.x Update dependency org.springframework:spring-messaging to v6.2.3
…ndencycheck-12.x Update plugin org.owasp.dependencycheck to v12.1.0
Update dependency org.javers:javers-core to v7.8.0
Update springDocVersion to v2.8.5
…ven-publish-java-8.x Update plugin io.freefair.maven-publish-java to v8.12.2
…-spring-boot-admin-starter-client-3.x Update dependency de.codecentric:spring-boot-admin-starter-client to v3.4.4
…ework.boot-3.x Update plugin org.springframework.boot to v3.4.3
…ush-action-digest Update docker/build-push-action digest to b16f42f
…mbok-8.x Update plugin io.freefair.lombok to v8.12.2
…a-action-digest Update docker/metadata-action digest to 902fa8e
…ush-action-digest Update docker/build-push-action digest to 471d1dc
…mbok-9.x Update plugin io.freefair.lombok to v9
…-h2-2.x Update dependency com.h2database:h2 to v2.4.240
…ush-action-digest Update docker/build-push-action digest to 9e436ba
…ndencycheck-12.x Update plugin org.owasp.dependencycheck to v12.1.6
Add a test for monitoring supported by new repo-core library.
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.
Actionable comments posted: 1
🧹 Nitpick comments (7)
.github/workflows/docker-publish.yml (2)
16-22: Add top‑level concurrency to avoid overlapping image publishes on rapid pushesPrevents racing pushes for the same ref.
env: REGISTRY: ghcr.io IMAGE_NAME: ${{ github.repository }} +concurrency: + group: docker-publish-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + # Only one job for creating and pushing Docker image jobs:
59-65: Enable Buildx remote cache to speed up subsequent buildsUses the GitHub Actions cache backend; safe and commonly used with build-push-action.
- name: Build and push Docker image uses: docker/build-push-action@9e436ba9f2d7bcd1d038c8e55d039d37896ddc5d with: context: . push: true tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=maxsrc/test/java/edu/kit/datamanager/repo/test/integration/ActuatorPrometheusTest.java (5)
78-80: Remove unused JUnitRestDocumentation RuleThe Rule is declared but not used; drop it to reduce noise.
- @Rule - public JUnitRestDocumentation restDocumentation = new JUnitRestDocumentation();
68-74: Consolidate repeated @TestPropertySource annotationsCombine into a single annotation for readability and easier maintenance.
-@TestPropertySource(properties = {"spring.datasource.url=jdbc:h2:mem:db_prometheus;DB_CLOSE_DELAY=-1;MODE=LEGACY;NON_KEYWORDS=VALUE"}) -@TestPropertySource(properties = {"spring.jpa.database-platform=org.hibernate.dialect.H2Dialect"}) -@TestPropertySource(properties = {"spring.jpa.defer-datasource-initialization=true"}) -@TestPropertySource(properties = {"repo.monitoring.enabled=true"}) -@TestPropertySource(properties = {"repo.monitoring.serviceName=base_repo_test"}) -@TestPropertySource(properties = {"management.endpoints.web.exposure.include=prometheus"}) +@TestPropertySource(properties = { + "spring.datasource.url=jdbc:h2:mem:db_prometheus;DB_CLOSE_DELAY=-1;MODE=LEGACY;NON_KEYWORDS=VALUE", + "spring.jpa.database-platform=org.hibernate.dialect.H2Dialect", + "spring.jpa.defer-datasource-initialization=true", + "repo.monitoring.enabled=true", + "repo.monitoring.serviceName=base_repo_test", + "management.endpoints.web.exposure.include=prometheus" +})
96-107: Trim .andDo(print()) to keep CI logs cleanPrinting every request clutters logs and slows builds. Keep prints only when debugging failures.
- this.mockMvc.perform(get("/actuator/beans")).andDo(print()).andExpect(status().isNotFound()); + this.mockMvc.perform(get("/actuator/beans")).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/caches")).andDo(print()).andExpect(status().isNotFound()); + this.mockMvc.perform(get("/actuator/caches")).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/conditions")).andDo(print()).andExpect(status().isNotFound()); + this.mockMvc.perform(get("/actuator/conditions")).andExpect(status().isNotFound()); ...apply similarly to the remaining endpoints...
110-116: Also assert Prometheus content type; drop printPrometheus should respond with text/plain (OpenMetrics v0.0.4 exposition). Check compatibility to avoid parameter/charset brittleness.
+import org.springframework.http.MediaType; @@ - this.mockMvc.perform(get("/actuator/prometheus")).andDo(print()).andExpect(status().isOk()) + this.mockMvc.perform(get("/actuator/prometheus")).andExpect(status().isOk()) + .andExpect(content().contentTypeCompatibleWith(MediaType.TEXT_PLAIN)) .andExpect(content().string(Matchers.containsString("# TYPE base_repo_test_requests_served_total"))) .andExpect(content().string(Matchers.containsString("# TYPE base_repo_test_unique_users"))) .andExpect(content().string(Matchers.containsString("# TYPE base_repo_test_registered_users"))) .andReturn();
93-107: Reduce duplication by looping over non‑exposed endpointsKeeps the test short and easier to extend.
- // Check that all other endpoints are not exposed - this.mockMvc.perform(get("/actuator/beans")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/caches")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/conditions")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/configprops")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/env")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/loggers")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/heapdump")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/threaddump")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/metrics")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/scheduledtasks")).andDo(print()).andExpect(status().isNotFound()); - this.mockMvc.perform(get("/actuator/mappings")).andDo(print()).andExpect(status().isNotFound()); + for (String ep : new String[]{ + "/actuator/beans", "/actuator/caches", "/actuator/conditions", "/actuator/configprops", + "/actuator/env", "/actuator/loggers", "/actuator/heapdump", "/actuator/threaddump", + "/actuator/metrics", "/actuator/scheduledtasks", "/actuator/mappings"}) { + this.mockMvc.perform(get(ep)).andExpect(status().isNotFound()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/docker-publish.yml(2 hunks)build.gradle(5 hunks)src/test/java/edu/kit/datamanager/repo/test/integration/ActuatorPrometheusTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- build.gradle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: Analyze (java)
- GitHub Check: build (ubuntu-latest, 17)
🔇 Additional comments (1)
.github/workflows/docker-publish.yml (1)
35-35: Pin checkout action to a specific SHA and enable full fetch-depth
- Replace
actions/checkout@v5withactions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8to avoid supply-chain risks.- Add
fetch-depth: 0so docker/metadata-action can access full tag history.Apply:
- name: Checkout repository - uses: actions/checkout@v5 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 + with: + fetch-depth: 0
| @RunWith(SpringRunner.class) | ||
| @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) | ||
| @EntityScan("edu.kit.datamanager") | ||
| @EnableJpaRepositories("edu.kit.datamanager") | ||
| @ComponentScan({"edu.kit.datamanager"}) | ||
| @AutoConfigureMockMvc |
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.
🧩 Analysis chain
JUnit 4 runner in a Spring Boot 3.x stack — confirm Vintage engine presence or plan Jupiter migration
You’re using JUnit 4 (SpringRunner, @before, @rule). Ensure the build includes junit-vintage-engine, or migrate tests to JUnit Jupiter to align with modern Spring Test support. Based on learnings.
Run to confirm dependencies and usage:
🏁 Script executed:
#!/bin/bash
# Find explicit JUnit dependencies and engines
rg -nP -C2 '(junit-?vintage|junit-?jupiter|org\.junit\.jupiter|org\.junit\.vintage|junit[:"]\s*4\.)' \
-g '!**/build/**' -g '!**/out/**'
# Spot JUnit4 annotations in tests
rg -nP -C2 'org\.junit\.(Test|Before|Rule)|@RunWith\(SpringRunner\.class\)' src/testLength of output: 10993
🏁 Script executed:
#!/bin/bash
# Search for JUnit dependencies in Maven pom.xml
rg -nP '<artifactId>junit' -C1 -g 'pom.xml'
rg -nP 'org\.junit\.vintage|org\.junit\.jupiter' -g 'pom.xml'
# If Gradle build script present, search for junit dependencies
rg -nP 'junit' -g 'build.gradle' -g 'build.gradle.kts'Length of output: 358
Add JUnit Vintage engine or migrate tests to JUnit Jupiter
Your integration tests (e.g. using SpringRunner, @rule, @before) rely on JUnit 4, but build.gradle only declares testImplementation "junit:junit:4.13.2" (no vintage engine). Spring Boot 3.x uses JUnit Platform, so either add the Vintage engine:
testRuntimeOnly "org.junit.vintage:junit-vintage-engine"or migrate your tests to JUnit Jupiter.
🤖 Prompt for AI Agents
In
src/test/java/edu/kit/datamanager/repo/test/integration/ActuatorPrometheusTest.java
around lines 56 to 61, the test class uses JUnit 4 constructs (SpringRunner,
@RunWith) but the build lacks the JUnit Vintage engine so tests will not run
under JUnit Platform; either add the Vintage engine to build.gradle as a test
runtime dependency (org.junit.vintage:junit-vintage-engine) so JUnit4 tests run
on the JUnit Platform, or migrate the test to JUnit Jupiter by replacing
@RunWith(SpringRunner.class) with @ExtendWith(SpringExtension.class) and
updating any @Before/@After/@Rule usages to their JUnit 5 equivalents.
…ction-digest Update docker/login-action digest to 5e57cd1
Enabled actuator for testing
Update dependency org.javers:javers-core to v7.9.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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/resources/test-config/application-test.properties (1)
38-39: Remove duplicate management property entries.These two keys are already defined later in this file (Lines 57 and 59), so the later values override these additions. Please drop one set to avoid confusion.
- management.endpoints.web.exposure.include=* - management.endpoint.health.show-details=always
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
build.gradle(5 hunks)src/test/resources/test-config/application-test.properties(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Analyze (java)
- GitHub Check: build (macOS-latest, 17)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: build (macOS-latest, 17)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: GitGuardian Security Checks
| set('javersVersion', "7.9.0") | ||
| set('springBootVersion', "3.2.1") | ||
| set('springDocVersion', "2.8.4") | ||
| set('springDocVersion', "2.8.13") |
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.
Update springBootVersion property to match the plugin
The build now applies org.springframework.boot 3.5.6, but the springBootVersion property still advertises 3.2.1. Downstream modules or dependency management imports that consume this property will continue pulling the 3.2.x BOM, creating a version split from the Gradle plugin and the managed dependencies you just upgraded. Please bump the property to 3.5.6 (or remove it if truly unused) so all consumers stay aligned.
- set('springBootVersion', "3.2.1")
+ set('springBootVersion', "3.5.6")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set('javersVersion', "7.9.0") | |
| set('springBootVersion', "3.2.1") | |
| set('springDocVersion', "2.8.4") | |
| set('springDocVersion', "2.8.13") | |
| set('javersVersion', "7.9.0") | |
| set('springBootVersion', "3.5.6") | |
| set('springDocVersion', "2.8.13") |
🤖 Prompt for AI Agents
In build.gradle around lines 35 to 37, the springBootVersion property is set to
3.2.1 while the project applies org.springframework.boot plugin version 3.5.6,
causing a mismatch between the plugin and any consumers of the property; update
the property value to "3.5.6" (or remove the property if it is unused) so
dependency management and downstream modules use the same Spring Boot BOM as the
applied plugin.
…ework.data-spring-data-elasticsearch-5.x Update dependency org.springframework.data:spring-data-elasticsearch to v5.5.5
…ction-digest Update docker/login-action digest to 28fdb31
…ework-spring-messaging-6.x Update dependency org.springframework:spring-messaging to v6.2.12
…ework.cloud-spring-cloud-gateway-mvc-4.x Update dependency org.springframework.cloud:spring-cloud-gateway-mvc to v4.3.2
…ndencycheck-12.x Update plugin org.owasp.dependencycheck to v12.1.8
Update dependency jacoco to v0.8.14
…action-4.x Update github/codeql-action action to v4
Summary by CodeRabbit
Chores
Tests
Documentation