Skip to content

PR for v1.2.3 #287

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

Merged
merged 139 commits into from
Nov 8, 2024
Merged

PR for v1.2.3 #287

merged 139 commits into from
Nov 8, 2024

Conversation

ThomasJejkal
Copy link
Contributor

@ThomasJejkal ThomasJejkal commented Apr 23, 2024

  • Update CHANGELOG.md
    • Update links at the end
  • Update dependencies

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced searching capabilities for related identifiers with new methods in the RelatedIdentifierSpec class.
    • Comprehensive unit tests added for the RelatedIdentifierSpec class to ensure robust functionality.
  • Bug Fixes

    • Improved error messaging for missing publisher during updates in the DataResourceService.
  • Chores

    • Updated various dependencies and plugins to their latest versions for improved performance and security.
    • Updated Gradle wrapper to version 8.10.2 for better build management.
    • Removed outdated configuration for GitHub Actions in the project setup.
  • Documentation

    • Updated CHANGELOG.md to reflect recent changes and fixes.

ThomasJejkal and others added 30 commits April 23, 2024 09:47
…ate_REVOKED_returns_VOLATILE_and_FIXED

Search for state REVOKED returns VOLATILE and FIXED
Bumps com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider from 2.17.0 to 2.17.1.

---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [io.spring.dependency-management](https://github.com/spring-gradle-plugins/dependency-management-plugin) from 1.1.4 to 1.1.5.
- [Release notes](https://github.com/spring-gradle-plugins/dependency-management-plugin/releases)
- [Commits](spring-gradle-plugins/dependency-management-plugin@v1.1.4...v1.1.5)

---
updated-dependencies:
- dependency-name: io.spring.dependency-management
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [com.fasterxml.jackson.module:jackson-module-afterburner](https://github.com/FasterXML/jackson-modules-base) from 2.17.0 to 2.17.1.
- [Commits](FasterXML/jackson-modules-base@jackson-modules-base-2.17.0...jackson-modules-base-2.17.1)

---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.module:jackson-module-afterburner
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…opment/com.fasterxml.jackson.module-jackson-module-afterburner-2.17.1
…opment/io.spring.dependency-management-1.1.5
…terxml.jackson.jaxrs-jackson-jaxrs-json-provider-2.17.1
Bumps com.fasterxml.jackson.datatype:jackson-datatype-jsr310 from 2.17.0 to 2.17.1.

---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.datatype:jackson-datatype-jsr310
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…opment/com.fasterxml.jackson.jaxrs-jackson-jaxrs-json-provider-2.17.1
…opment/com.fasterxml.jackson.datatype-jackson-datatype-jsr310-2.17.1
Bumps [com.fasterxml.jackson.datatype:jackson-datatype-joda](https://github.com/FasterXML/jackson-datatype-joda) from 2.17.0 to 2.17.1.
- [Commits](FasterXML/jackson-datatype-joda@jackson-datatype-joda-2.17.0...jackson-datatype-joda-2.17.1)

---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.datatype:jackson-datatype-joda
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…opment/com.fasterxml.jackson.datatype-jackson-datatype-joda-2.17.1
Bumps org.owasp.dependencycheck from 9.1.0 to 9.2.0.

---
updated-dependencies:
- dependency-name: org.owasp.dependencycheck
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.javers:javers-spring-boot-starter-sql](https://github.com/javers/javers) from 7.4.2 to 7.4.3.
- [Release notes](https://github.com/javers/javers/releases)
- [Commits](javers/javers@javers-7.4.2...javers-7.4.3)

---
updated-dependencies:
- dependency-name: org.javers:javers-spring-boot-starter-sql
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…opment/org.javers-javers-spring-boot-starter-sql-7.4.3
Bumps [org.javers:javers-spring-boot-starter-sql](https://github.com/javers/javers) from 7.4.3 to 7.4.6.
- [Release notes](https://github.com/javers/javers/releases)
- [Commits](javers/javers@javers-7.4.3...javers-7.4.6)

---
updated-dependencies:
- dependency-name: org.javers:javers-spring-boot-starter-sql
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…opment/org.javers-javers-spring-boot-starter-sql-7.4.6
Bumps [org.javers:javers-spring-boot-starter-sql](https://github.com/javers/javers) from 7.4.6 to 7.5.0.
- [Release notes](https://github.com/javers/javers/releases)
- [Commits](javers/javers@javers-7.4.6...javers-7.5.0)

---
updated-dependencies:
- dependency-name: org.javers:javers-spring-boot-starter-sql
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps com.fasterxml.jackson.datatype:jackson-datatype-jsr310 from 2.17.1 to 2.17.2.

---
updated-dependencies:
- dependency-name: com.fasterxml.jackson.datatype:jackson-datatype-jsr310
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
…opment/org.javers-javers-spring-boot-starter-sql-7.5.0
…_if_publisher_is_missing_is_wrong

Error message if publisher is missing is wrong.
Bumps org.owasp.dependencycheck from 9.2.0 to 10.0.3.

---
updated-dependencies:
- dependency-name: org.owasp.dependencycheck
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps `springDocVersion` from 2.5.0 to 2.6.0.

Updates `org.springdoc:springdoc-openapi-starter-webmvc-ui` from 2.5.0 to 2.6.0
- [Release notes](https://github.com/springdoc/springdoc-openapi/releases)
- [Changelog](https://github.com/springdoc/springdoc-openapi/blob/main/CHANGELOG.md)
- [Commits](springdoc/springdoc-openapi@v2.5.0...v2.6.0)

Updates `org.springdoc:springdoc-openapi-starter-common` from 2.5.0 to 2.6.0
- [Release notes](https://github.com/springdoc/springdoc-openapi/releases)
- [Changelog](https://github.com/springdoc/springdoc-openapi/blob/main/CHANGELOG.md)
- [Commits](springdoc/springdoc-openapi@v2.5.0...v2.6.0)

Updates `org.springdoc:springdoc-openapi-starter-webmvc-api` from 2.5.0 to 2.6.0
- [Release notes](https://github.com/springdoc/springdoc-openapi/releases)
- [Changelog](https://github.com/springdoc/springdoc-openapi/blob/main/CHANGELOG.md)
- [Commits](springdoc/springdoc-openapi@v2.5.0...v2.6.0)

---
updated-dependencies:
- dependency-name: org.springdoc:springdoc-openapi-starter-webmvc-ui
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.springdoc:springdoc-openapi-starter-common
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: org.springdoc:springdoc-openapi-starter-webmvc-api
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
ThomasJejkal and others added 15 commits November 1, 2024 13:34
…jackson.datatype-jackson-datatype-jsr310-2.x

Update dependency com.fasterxml.jackson.datatype:jackson-datatype-jsr310 to v2.18.1
…java-4.x

Update actions/setup-java action to v4.5.0
Update dependency org.springframework.boot:spring-boot-dependencies to v3.3.5
…ndencycheck-11.x

Update plugin org.owasp.dependencycheck to v11
…-spring-boot-admin-starter-client-3.x

Update dependency de.codecentric:spring-boot-admin-starter-client to v3.3.5
…jackson.jaxrs-jackson-jaxrs-json-provider-2.x

Update dependency com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider to v2.18.1
…jackson.datatype-jackson-datatype-joda-2.x

Update dependency com.fasterxml.jackson.datatype:jackson-datatype-joda to v2.18.1
…ntifierSpec

New Features
    Enhanced functionality for specifying related identifiers with new methods for complex queries.
    Improved query construction for DataResource entities based on related identifiers and types.

Tests
    Introduced integration tests for the RelatedIdentifierSpec class to validate querying capabilities and ensure expected results.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (5)
build.gradle (1)

72-72: Ensure Spring dependency versions are aligned.

The spring-messaging (6.1.14) and spring-data-elasticsearch (5.3.5) versions should be compatible with Spring Boot 3.3.5. Consider using Spring Boot's dependency management to handle these versions automatically.

Consider removing explicit versions and letting Spring Boot's dependency management handle them:

-    implementation 'org.springframework:spring-messaging:6.1.14'
+    implementation 'org.springframework:spring-messaging'
-    implementation 'org.springframework.data:spring-data-elasticsearch:5.3.5'
+    implementation 'org.springframework.data:spring-data-elasticsearch'

Also applies to: 80-80

CHANGELOG.md (1)

Update changelog to reflect all significant changes

Based on the verification results, the following changes should be documented in the changelog:

  • Dependencies and Plugins:

    • Spring Boot upgraded to 3.3.5
    • SpringDoc upgraded to 2.6.0
    • Javers upgraded to 7.6.3
    • Gradle wrapper upgraded to 8.10.2
    • Multiple Gradle plugin updates (freefair.lombok 8.10.2, spring dependency-management 1.1.6, etc.)
    • GSON upgraded to 2.11.0
  • Build/CI:

    • Changes in GitHub Actions workflows (setup-java action)
    • Dependabot configuration updates

These changes should be properly categorized under "Changed" and "Security" (dependency updates) sections in the Unreleased changelog.

🔗 Analysis chain

Line range hint 1-24: Verify completeness of changelog entries.

The AI summary mentions multiple changes that should be reflected in the changelog:

  • Removal of "github-actions" from Dependabot config
  • Updates to Java setup action version in workflows
  • Enhancements to RelatedIdentifierSpec class
  • Multiple plugin and dependency version upgrades
  • Gradle wrapper updates

These changes should be documented in appropriate sections (Changed, Security, etc.) of the Unreleased section.

Let's verify if there are any dependency updates that should be documented:


Let me gather more information about the changes mentioned in the AI summary:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Look for dependency-related changes in build files
# Expected: Find gradle dependency updates that should be documented

# Search for version changes in build.gradle
echo "Checking build.gradle for version updates..."
rg -A 1 "version|Version" build.gradle

# Search for wrapper property changes
echo "Checking gradle-wrapper.properties for updates..."
rg "distributionUrl" gradle/wrapper/gradle-wrapper.properties

Length of output: 2424


Script:

#!/bin/bash
# Check for workflow changes
echo "Checking workflow changes..."
rg -l "actions/setup-java" .github/workflows/

# Check for Dependabot config changes
echo "Checking Dependabot config..."
cat .github/dependabot.yml

# Check for RelatedIdentifierSpec changes
echo "Checking RelatedIdentifierSpec changes..."
rg -A 3 "class RelatedIdentifierSpec" 

Length of output: 1774

🧰 Tools
🪛 LanguageTool

[duplication] ~11-~11: Possible typo: you repeated a word
Context: ...html). ## [Unreleased] ### Added ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)

src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java (3)

116-117: Remove duplicate test case.

The test case for UNKNOWN_RELATED_RESOURCE and RELATED_RESOURCE_1 is duplicated.

-    testTwoValues(UNKNOWN_RELATED_RESOURCE, RELATED_RESOURCE_1, 2);
     testTwoValues(UNKNOWN_RELATED_RESOURCE, RELATED_RESOURCE_1, 2);

102-164: Consider enhancing test coverage.

While the current tests are good, consider adding these scenarios:

  1. Test with empty strings for related resource values
  2. Test with special characters in resource values
  3. Test with multiple relation types simultaneously
  4. Test error cases (e.g., invalid relation types)

Would you like me to help generate additional test cases for these scenarios?


166-198: Enhance helper methods with validation and documentation.

Consider these improvements to the helper methods:

  1. Add input validation in createTestResource
  2. Make assertion messages more descriptive by including the test scenario
  3. Add Javadoc to document the test data setup logic

Example improvements:

+ /**
+  * Creates a test resource with the specified related identifier.
+  *
+  * @param name The unique name for the resource
+  * @param relationType The type of relation (can be null for no relation)
+  * @param relatedResource The related resource value
+  * @return The created and persisted DataResource
+  * @throws IllegalArgumentException if name or relatedResource is null
+  */
 private DataResource createTestResource(String name, RelatedIdentifier.RELATION_TYPES relationType, String relatedResource) {
+    if (name == null || relatedResource == null) {
+        throw new IllegalArgumentException("name and relatedResource must not be null");
+    }
     DataResource resource = DataResource.factoryNewDataResource(name);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9f48b03 and f474809.

⛔ Files ignored due to path filters (1)
  • gradle/wrapper/gradle-wrapper.jar is excluded by !**/*.jar
📒 Files selected for processing (12)
  • .github/dependabot.yml (0 hunks)
  • .github/workflows/codeql-analysis.yml (1 hunks)
  • .github/workflows/gradle.yml (2 hunks)
  • .github/workflows/publishRelease.yml (1 hunks)
  • CHANGELOG.md (1 hunks)
  • build.gradle (6 hunks)
  • gradle/wrapper/gradle-wrapper.properties (1 hunks)
  • gradlew (5 hunks)
  • gradlew.bat (3 hunks)
  • src/main/java/edu/kit/datamanager/repo/dao/spec/dataresource/RelatedIdentifierSpec.java (1 hunks)
  • src/main/java/edu/kit/datamanager/repo/service/impl/DataResourceService.java (3 hunks)
  • src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java (1 hunks)
💤 Files with no reviewable changes (1)
  • .github/dependabot.yml
🔇 Additional comments (23)
gradle/wrapper/gradle-wrapper.properties (1)

5-5: LGTM! Good security practice.

Adding validateDistributionUrl=true is a great security enhancement that helps prevent supply chain attacks by validating the Gradle distribution URL.

.github/workflows/publishRelease.yml (1)

11-11: LGTM! Safe minor version update of setup-java action.

The update from v4.2.1 to v4.5.0 is a backward-compatible change that likely includes bug fixes and improvements.

Let's verify version consistency across all workflow files:

✅ Verification successful

✅ Version consistency verified across all workflow files

All GitHub Actions workflow files are using the same version (v4.5.0) of the setup-java action:

  • .github/workflows/publishRelease.yml
  • .github/workflows/gradle.yml (used twice)
  • .github/workflows/codeql-analysis.yml
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if setup-java action version is consistent across all workflow files
# Expected: All files should use v4.5.0

echo "Checking setup-java action versions in workflow files:"
rg "uses: actions/setup-java@v" .github/workflows/

Length of output: 458

.github/workflows/gradle.yml (1)

27-27: LGTM! Good practice keeping GitHub Actions up to date.

The update of actions/setup-java to v4.5.0 across both jobs is a good security practice. The change is consistent and aligns with updates in other workflow files.

Let's verify the consistency of the setup-java action version across all workflow files:

Also applies to: 53-53

✅ Verification successful

All workflow files consistently use setup-java@v4.5.0 ✓

The verification confirms that all instances of actions/setup-java across the workflow files (gradle.yml, codeql-analysis.yml, and publishRelease.yml) are using the same version v4.5.0, indicating proper version consistency throughout the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify setup-java action version consistency across workflow files
# Expected: All setup-java actions should use v4.5.0

echo "Checking setup-java action versions in workflow files:"
rg "uses: actions/setup-java@v" .github/workflows/

Length of output: 458

.github/workflows/codeql-analysis.yml (1)

45-45: LGTM! Verify the Java setup action version.

The update to actions/setup-java@v4.5.0 is consistent with the changes mentioned in the PR summary.

Let's verify if this is the latest stable version and check for any breaking changes:

✅ Verification successful

✅ The Java setup action version is up to date

The workflow is using actions/setup-java@v4.5.0 which matches the latest stable release from October 24, 2024.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the latest release version and changelog for actions/setup-java

# Get the latest release version and compare
gh release list --repo actions/setup-java --limit 1

Length of output: 95

gradlew.bat (4)

16-17: LGTM: SPDX license identifier addition

The SPDX license identifier correctly matches the detailed Apache 2.0 license header above it.


48-52: LGTM: Improved error message handling

Error messages are now properly redirected to stderr using 1>&2, which is the correct approach for error handling in Windows batch scripts.


62-66: LGTM: Consistent error handling improvement

Error messages for invalid JAVA_HOME are now properly redirected to stderr, maintaining consistency with the previous error handling block.


48-52: Verify error handling compatibility with Gradle 8.10.2

The error handling improvements look good, but let's verify they align with Gradle's conventions and work correctly with the updated Gradle version mentioned in the AI summary.

Also applies to: 62-66

✅ Verification successful

Error handling in gradlew.bat is consistent with Gradle conventions

The error handling patterns in gradlew.bat are properly implemented and align with the project's configuration:

  • The error messages follow Windows batch file conventions using 1>&2 for stderr
  • The JAVA_HOME validation is consistent with the Java versions (17, 19) specified in the workflow files
  • The Gradle version (8.10.2) and error handling are compatible with the project's CI/CD setup across different operating systems
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if similar error handling patterns exist in other Gradle wrapper scripts
# and verify the Gradle version being used.

# Check the Gradle version in wrapper properties
echo "Checking Gradle version:"
cat gradle/wrapper/gradle-wrapper.properties

# Look for similar error handling patterns in other Gradle wrapper scripts
echo -e "\nChecking error handling patterns in other Gradle scripts:"
rg "echo.*1>&2" --type bat

Length of output: 546


Script:

#!/bin/bash
# Let's verify the error handling patterns in the Gradle wrapper script
# and check for any related configuration files

# First, check if the gradlew.bat file exists and show its error handling section
echo "Checking gradlew.bat error handling:"
rg "echo.*ERROR.*JAVA_HOME" gradlew.bat -A 3

# Check for any workflow files that might reference Java or Gradle
echo -e "\nChecking workflow files for Java/Gradle configuration:"
fd -e yml -e yaml . .github/workflows/ --exec cat {}

# Look for any other batch files that might have similar patterns
echo -e "\nChecking for other batch files:"
fd -e bat -e cmd

Length of output: 6434

build.gradle (5)

126-126: LGTM!

The spring-restdocs-mockmvc version 3.0.2 is compatible with Spring Boot 3.3.5.


158-158: LGTM!

JaCoCo 0.8.12 is compatible with Java 17 and includes various improvements and bug fixes.


89-89: Review major version upgrades carefully.

Several significant version upgrades require attention:

  1. Tika Core upgrade from 2.9.2 to 3.0.0 is a major version change
  2. Jackson libraries upgraded to 2.18.1
  3. H2 database upgraded to 2.3.232

These changes might introduce breaking changes or require additional configuration updates.

#!/bin/bash
# Description: Check for potential breaking changes in configuration
# Test: Search for Tika and Jackson related configuration
rg -l "tika|jackson|h2|postgresql" 

Also applies to: 94-94, 100-101, 103-104, 106-107, 111-111


35-37: Verify Spring Boot 3.3.5 compatibility with existing dependencies.

The upgrade from Spring Boot 3.1.0 to 3.3.5 is significant. Please ensure:

  1. All Spring dependencies are aligned with this version
  2. Third-party libraries are compatible with Spring Boot 3.3.5
#!/bin/bash
# Description: Check for potential Spring Boot version conflicts
# Test: Search for Spring Boot related dependencies
rg "org\.springframework\.boot" 

17-22: Verify plugin compatibility with Java 17.

The significant version updates in plugins should be tested thoroughly:

  • freefair plugins (8.10.2)
  • spring dependency management (1.1.6)
  • owasp dependencycheck (11.1.0)
  • asciidoctor (4.0.3)
✅ Verification successful

Plugin versions are compatible with Java 17 configuration

The codebase is explicitly configured for Java 17 with proper compatibility checks in place:

  • Source and target compatibility are set to Java 17
  • Build file includes various Java version checks for different compatibility scenarios
  • All updated plugin versions (freefair 8.10.2, spring 1.1.6, owasp 11.1.0, asciidoctor 4.0.3) are recent releases that support Java 17
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if any of these plugin versions have known compatibility issues with Java 17
# Test: Search for Java version compatibility information in gradle files
rg -l "sourceCompatibility|targetCompatibility|JavaVersion" 

Length of output: 1879


Script:

#!/bin/bash
# Let's check the actual Java version configuration in the build files
rg "sourceCompatibility|targetCompatibility|JavaVersion" build.gradle gradle/profile-deploy.gradle

Length of output: 564

gradlew (5)

18-19: LGTM: Documentation updates are accurate and beneficial.

The addition of the SPDX license identifier and the updated Groovy template source URL improve documentation clarity and maintainability.

Also applies to: 60-60


88-90: LGTM: Improved APP_HOME handling.

The change properly addresses gradle/gradle#25036 by suppressing the cd command's output when $CDPATH is set. This is a good fix that prevents unwanted output from interfering with the script's operation.


137-143: LGTM: Enhanced Java command resolution.

The change improves Java command detection by using command -v, which is more reliable than direct path checks and follows POSIX shell best practices.


151-162: LGTM: Improved file descriptor handling.

The changes enhance the robustness of file descriptor limit handling:

  • Added appropriate shellcheck directives
  • Improved error handling for ulimit commands
  • Maintained POSIX shell compatibility

204-212: LGTM: Enhanced documentation for JVM options handling.

The changes improve the documentation by:

  • Adding clearer guidance about shell fragment restrictions in environment variables
  • Including a helpful example about ${Hostname} expansion
  • Enhancing readability with better formatting
CHANGELOG.md (1)

12-12: LGTM: Fix properly documented.

The fix for the unprivileged find functionality is properly documented in the Unreleased section.

src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java (1)

59-59: Verify H2 database configuration.

The H2 database URL includes special configurations that might not be necessary:

  • MODE=LEGACY: Consider if this is needed as it might mask compatibility issues with your production database
  • NON_KEYWORDS=VALUE: This suggests there's a column named "VALUE". Consider renaming the column to avoid keyword conflicts
✅ Verification successful

Let me try another search strategy to verify the H2 configuration necessity.


Let me check the RelatedIdentifier entity to understand if the H2 configuration is necessary.


Based on the examination of the RelatedIdentifier entity and the H2 configurations across test files, I can now provide a final response:

H2 database configuration is appropriate and necessary

The H2 configuration settings are justified:

  • NON_KEYWORDS=VALUE is necessary because the RelatedIdentifier entity has a field named value which could conflict with H2's VALUE keyword
  • MODE=LEGACY is consistently used across all test classes in the codebase, indicating it's a standardized test configuration
  • Each test class uses a unique database name to prevent interference between tests
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for column names that might conflict with H2 keywords
echo "Searching for potential column name conflicts..."
rg -g "*.java" -B 2 -A 2 '\@Column\(.*name.*=.*"VALUE".*\)' || echo "No direct column named 'VALUE' found."

# Search for entity definitions that might use 'value' as a field name
echo "Searching for entity fields named 'value'..."
ast-grep --pattern 'class $_ {
  $$$
  private $_ value;
  $$$
}'

Length of output: 531


Script:

#!/bin/bash
# Search for entity classes and their column definitions
echo "Searching for entity classes and column definitions..."
rg -g "*.java" -B 2 -A 2 "@Column.*name.*=" 

# Search for H2 database configurations in test files
echo "Searching for other H2 database configurations..."
rg -g "*.java" "jdbc:h2:mem:" -B 1 -A 1

# Search for related identifier entity definition
echo "Searching for related identifier entity..."
fd -e java -E test --exec grep -l "class.*RelatedIdentifier" {} \;

Length of output: 3209


Script:

#!/bin/bash
# Check RelatedIdentifier entity for any VALUE column or field
echo "Checking RelatedIdentifier entity..."
cat ./src/main/java/edu/kit/datamanager/repo/domain/RelatedIdentifier.java

# Check if any other test classes use different H2 configurations
echo "Checking other test configurations..."
rg -g "test/**/*.java" "spring.datasource.url" --no-filename | sort | uniq

Length of output: 7554

src/main/java/edu/kit/datamanager/repo/service/impl/DataResourceService.java (2)

505-507: LGTM! Improved state handling logic

The condition now correctly checks against example.getState() for including revoked states, which is more accurate than the previous implementation.


605-605: LGTM! Fixed incorrect error message

The error message now correctly reflects that it's checking for an empty publisher, not publication year.

src/main/java/edu/kit/datamanager/repo/dao/spec/dataresource/RelatedIdentifierSpec.java (1)

46-48: Well-designed overloads enhance API flexibility

The addition of overloaded toSpecification methods improves usability by allowing queries with either relationType, identifierValues, or both.

Also applies to: 56-58

VolkerHartmann and others added 5 commits November 8, 2024 15:07
…nager-service-base-1.x

Update dependency edu.kit.datamanager:service-base to v1.3.2
…dIdentifierSpecificationTest.java

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…elatedIdentifierSpec.java

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (3)
src/main/java/edu/kit/datamanager/repo/dao/spec/dataresource/RelatedIdentifierSpec.java (1)

67-87: Consider the following improvements to enhance robustness and maintainability.

  1. Add null-safety improvements:
  2. Document the usage of DISTINCT in the method's Javadoc
  3. Consider using a more descriptive join alias
  4. Simplify predicate construction

Here's the suggested implementation:

   public static Specification<DataResource> toSpecification(final RelatedIdentifier.RELATION_TYPES relationType, final String... identifierValues) {
-    Specification<DataResource> newSpec = Specification.where(null);
-    if ((identifierValues == null || identifierValues.length == 0) && (relationType == null)) {
-      return newSpec;
+    // Return empty specification if no search criteria provided
+    if (isEmpty(identifierValues) && relationType == null) {
+      return Specification.where(null);
     }

     return (Root<DataResource> root, CriteriaQuery<?> query, CriteriaBuilder builder) -> {
+      // Use distinct to prevent duplicate results when multiple criteria match
       query.distinct(true);
-      //join dataresource table with related identifiers table
-      Join<DataResource, RelatedIdentifier> relatedIdentifierJoin = root.join("relatedIdentifiers", JoinType.INNER);
-      //get all related identifiers of type relationType with one of the provided values
-      Predicate allPredicates = builder.conjunction();
-      if (identifierValues != null && identifierValues.length != 0) {
-        allPredicates = builder.and(allPredicates, relatedIdentifierJoin.get("value").in(identifierValues));
-      }
-      if (relationType != null) {
-        allPredicates = builder.and(allPredicates, builder.equal(relatedIdentifierJoin.get("relationType"), relationType));
-      }
 
-      return allPredicates;
+      // Join with related identifiers
+      Join<DataResource, RelatedIdentifier> relatedIds = root.join("relatedIdentifiers", JoinType.INNER);
+      
+      // Build predicates based on provided criteria
+      Predicate valuePredicate = !isEmpty(identifierValues) 
+          ? relatedIds.get("value").in(identifierValues)
+          : null;
+      Predicate typePredicate = relationType != null 
+          ? builder.equal(relatedIds.get("relationType"), relationType)
+          : null;
+          
+      return builder.and(
+          valuePredicate != null ? valuePredicate : builder.conjunction(),
+          typePredicate != null ? typePredicate : builder.conjunction()
+      );
     };
   }
+  
+  private static boolean isEmpty(String... values) {
+      return values == null || values.length == 0;
+  }

The improvements include:

  • Better null handling with a helper method
  • More descriptive variable names
  • Cleaner predicate construction
  • Better code organization and readability
CHANGELOG.md (2)

15-16: Remove word repetition in the Fixed section.

The word "Fixed" is redundant since it's already in the section header.

Apply this diff to improve readability:

### Fixed
-* Fixed potential issue with unprivileged find
-* Fixed issue with privileged find for state 'REVOKED'
+* Potential issue with unprivileged find
+* Issue with privileged find for state 'REVOKED'

Line range hint 284-285: Update version comparison links for v1.2.3.

Since this PR is for version 1.2.3, the version comparison links need to be updated.

Apply this diff to update the links:

-[Unreleased]: https://github.com/kit-data-manager/repo-core/compare/v1.2.2...HEAD
+[Unreleased]: https://github.com/kit-data-manager/repo-core/compare/v1.2.3...HEAD
+[1.2.3]: https://github.com/kit-data-manager/repo-core/compare/v1.2.2...v1.2.3
 [1.2.2]: https://github.com/kit-data-manager/repo-core/compare/v1.2.1...v1.2.2
🧰 Tools
🪛 LanguageTool

[duplication] ~14-~14: Possible typo: you repeated a word
Context: ...pdates in the DataResourceService. ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 32e832e and 6ddd0b2.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • src/main/java/edu/kit/datamanager/repo/dao/spec/dataresource/RelatedIdentifierSpec.java (1 hunks)
  • src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[duplication] ~14-~14: Possible typo: you repeated a word
Context: ...pdates in the DataResourceService. ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)

🔇 Additional comments (5)
src/main/java/edu/kit/datamanager/repo/dao/spec/dataresource/RelatedIdentifierSpec.java (4)

19-24: LGTM! Imports are appropriate and well-organized.


40-48: LGTM! Well-documented convenience method.

The method provides a clean interface for searching by identifier values only.


50-58: LGTM! Well-documented convenience method.

The method provides a clean interface for searching by relation type only.


67-87: Verify test coverage for the specification logic.

The implementation looks correct, but let's verify that all cases are properly tested.

✅ Verification successful

Let's examine the test file content to verify the test coverage.


Test coverage is comprehensive and implementation is verified

The test file demonstrates thorough coverage of the specification logic with tests for:

  • Single and multiple identifier values
  • Empty and null value arrays
  • Different relation types
  • Combined filtering by value and relation type
  • Edge cases (case sensitivity, unknown values)
  • Null and empty scenarios

The implementation correctly handles all test cases and matches the expected behavior.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for test files related to RelatedIdentifierSpec
echo "Searching for related test files..."
fd -e java -e groovy "RelatedIdentifier.*Test"

# Search for test methods testing the specification
echo "Searching for test methods..."
rg -l "class.*RelatedIdentifier.*Test|test.*RelatedIdentifier|test.*Specification"

Length of output: 777


Script:

#!/bin/bash
# Check the content of the test file
cat src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java

Length of output: 10500

CHANGELOG.md (1)

10-21: LGTM! Well-structured changelog entries.

The changelog entries are well-organized, following the Keep a Changelog convention, with clear and descriptive content in each section.

🧰 Tools
🪛 LanguageTool

[duplication] ~14-~14: Possible typo: you repeated a word
Context: ...pdates in the DataResourceService. ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (1)
CHANGELOG.md (1)

33-55: Consider grouping dependency updates by type

The Security section contains a long list of dependency updates. Consider grouping them by type (e.g., GitHub Actions, Spring dependencies, Testing dependencies) for better readability.

### Security
-* Update actions/setup-java action to v4.5.0
-* Update dependency com.fasterxml.jackson.datatype:jackson-datatype-joda to v2.18.1.
+# GitHub Actions
+* Update actions/setup-java action to v4.5.0
+
+# Spring Dependencies
+* Update dependency org.springframework.boot:spring-boot-dependencies to v3.3.5
+* Update dependency org.springframework.data:spring-data-elasticsearch to v5.3.5
+* Update dependency org.springframework.restdocs:spring-restdocs-mockmvc to v3.0.2
+* Update dependency org.springframework:spring-messaging to v6.1.14
+
+# Jackson Dependencies
+* Update dependency com.fasterxml.jackson.datatype:jackson-datatype-joda to v2.18.1
+* Update dependency com.fasterxml.jackson.datatype:jackson-datatype-jsr310 to v2.18.1
+* Update dependency com.fasterxml.jackson.module:jackson-module-afterburner to v2.18.1
+* Update dependency com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider to v2.18.1
[remaining dependencies...]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 6ddd0b2 and 388dbb8.

📒 Files selected for processing (2)
  • CHANGELOG.md (3 hunks)
  • src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/edu/kit/datamanager/repo/test/integration/RelatedIdentifierSpecificationTest.java
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[duplication] ~28-~28: Possible typo: you repeated a word
Context: ...ss to ensure robust functionality. ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)

🪛 Markdownlint
CHANGELOG.md

61-61: null
Multiple headings with the same content

(MD024, no-duplicate-heading)

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (1)
CHANGELOG.md (1)

58-59: Remove duplicate empty Security section

There's already a populated Security section above (lines 29-52) containing the dependency updates. This empty duplicate section should be removed.

-### Security
-
🧰 Tools
🪛 Markdownlint

58-58: null
Multiple headings with the same content

(MD024, no-duplicate-heading)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 388dbb8 and 121e798.

📒 Files selected for processing (1)
  • CHANGELOG.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[duplication] ~25-~25: Possible typo: you repeated a word
Context: ...ss to ensure robust functionality. ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)


[duplication] ~55-~55: Possible typo: you repeated a word
Context: ...ssaging to v6.1.14 ### Deprecated ### Removed * Removed outdated configuration for GitHub Actio...

(ENGLISH_WORD_REPEAT_RULE)

🪛 Markdownlint
CHANGELOG.md

58-58: null
Multiple headings with the same content

(MD024, no-duplicate-heading)

🔇 Additional comments (1)
CHANGELOG.md (1)

Line range hint 7-18: LGTM! Unreleased section is properly structured.

The empty sections are correctly maintained for future changes.

🧰 Tools
🪛 LanguageTool

[duplication] ~25-~25: Possible typo: you repeated a word
Context: ...ss to ensure robust functionality. ### Fixed * Fixed potential issue with unprivileged find ...

(ENGLISH_WORD_REPEAT_RULE)


[duplication] ~55-~55: Possible typo: you repeated a word
Context: ...ssaging to v6.1.14 ### Deprecated ### Removed * Removed outdated configuration for GitHub Actio...

(ENGLISH_WORD_REPEAT_RULE)

🪛 Markdownlint

58-58: null
Multiple headings with the same content

(MD024, no-duplicate-heading)

Comment on lines +320 to +321
[Unreleased]: https://github.com/kit-data-manager/repo-core/compare/v1.2.3...HEAD
[1.2.3]: https://github.com/kit-data-manager/repo-core/compare/v1.2.2...v1.2.3
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Version comparison links need to be updated

The version v1.2.3 referenced in the changelog links does not exist in the repository. The latest available tag is v1.2.2. The comparison links should be updated to use existing versions.

  • Update the unreleased comparison link to: [Unreleased]: https://github.com/kit-data-manager/repo-core/compare/v1.2.2...HEAD
  • Remove the non-existent v1.2.3 comparison link as this version hasn't been released yet
🔗 Analysis chain

Verify version comparison links

The version comparison links have been updated correctly for v1.2.3. Let's verify these links are accessible.


Let me verify the repository and tag information first to understand if there's a mismatch in the versions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the version comparison links are valid
# Test: Check if the comparison URLs return valid responses

echo "Verifying comparison URLs..."
gh api repos/kit-data-manager/repo-core/compare/v1.2.2...v1.2.3 --jq '.status' || echo "Invalid comparison: v1.2.2...v1.2.3"
gh api repos/kit-data-manager/repo-core/compare/v1.2.3...HEAD --jq '.status' || echo "Invalid comparison: v1.2.3...HEAD"

Length of output: 680


Script:

#!/bin/bash
# Check repository existence and available tags
echo "Checking repository information..."
gh repo view kit-data-manager/repo-core --json name,url 2>/dev/null || echo "Repository not found"

echo -e "\nChecking available tags..."
gh api repos/kit-data-manager/repo-core/tags --jq '.[].name' 2>/dev/null || echo "Unable to fetch tags"

Length of output: 441

@ThomasJejkal ThomasJejkal merged commit 1250105 into main Nov 8, 2024
17 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 24, 2025
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