fix: prevent NPE in stats and redirects endpoints by restoring extractor initialization #805
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a new Bash test script for validating stats and redirects endpoints across all languages, integrates it into the CI/CD workflow, reconfigures the server to reference languages via a mappings alias instead of explicit codes, and consolidates the extractor initialization in the Server class. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/src/main/bash/stats-redirects-test.sh (2)
15-18: Address shellcheck warnings for safer array handling.The script has two shellcheck warnings that should be addressed:
- Line 15 (SC2207): Using command substitution for array population is discouraged. While functional, consider using
mapfilefor safer handling.- Line 18 (SC2145): Mixing string and array without proper expansion. Use
"${LANGUAGES[*]}"instead of"${LANGUAGES[@]}"when displaying as a string.Apply these improvements:
-LANGUAGES=($(echo "$MAPPINGS_PAGE" | grep -oP 'href="\K[^/"]+(?=/">)' | sort -u)) +mapfile -t LANGUAGES < <(echo "$MAPPINGS_PAGE" | grep -oP 'href="\K[^/"]+(?=/">)' | sort -u) [ ${#LANGUAGES[@]} -eq 0 ] && { echo "❌ ERROR: No languages found"; exit 1; } -echo "Found ${#LANGUAGES[@]} languages: ${LANGUAGES[@]}" +echo "Found ${#LANGUAGES[@]} languages: ${LANGUAGES[*]}"
22-36: Consider adding timeout to curl for robustness.The test function correctly validates HTTP 200 responses and tracks failures. However, consider adding a timeout to prevent the test from hanging indefinitely if an endpoint becomes unresponsive.
- http_code=$(curl -s -o /dev/null -w "%{http_code}" "$url") + http_code=$(curl -s --max-time 30 -o /dev/null -w "%{http_code}" "$url")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/server-web-api-test.yml(1 hunks)scripts/src/main/bash/stats-redirects-test.sh(1 hunks)server/server.default.properties(1 hunks)server/src/main/scala/org/dbpedia/extraction/server/Server.scala(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/src/main/scala/org/dbpedia/extraction/server/Server.scala (1)
server/src/main/scala/org/dbpedia/extraction/server/ExtractionManager.scala (1)
updateAll(64-66)
🪛 Shellcheck (0.11.0)
scripts/src/main/bash/stats-redirects-test.sh
[warning] 15-15: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
[error] 18-18: Argument mixes string and array. Use * or separate argument.
(SC2145)
⏰ 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). (4)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: deploy
🔇 Additional comments (5)
server/server.default.properties (1)
21-21: LGTM! Dynamic language configuration improves maintainability.The change from hardcoded "de,en" to "@mappings" makes the configuration more dynamic and maintainable, automatically including all languages with mappings support without manual updates.
server/src/main/scala/org/dbpedia/extraction/server/Server.scala (1)
42-44: LGTM! Proper extractor initialization fixes the NPE.The restoration of
extractor.updateAllafter instantiation correctly initializes the extractor's internal state, which is essential for the statistics and redirects endpoints to function without NullPointerException. This pattern is consistent with the single extractor cache initialization at lines 56-68.Note: The extractor field is now public. Verify this is intentional for access from resources/endpoints.
.github/workflows/server-web-api-test.yml (1)
54-57: LGTM! Test step properly integrated into CI/CD workflow.The new test step follows the established pattern and is correctly positioned after server startup and before shutdown. It will automatically validate stats and redirects endpoints for all mappings languages.
scripts/src/main/bash/stats-redirects-test.sh (2)
38-44: LGTM! Test loop comprehensively validates both endpoints.The loop correctly tests both statistics and redirects endpoints for each discovered language, providing clear output for each test.
46-59: LGTM! Clear summary and proper exit code handling.The summary provides comprehensive test results with clear pass/fail indicators and exits with appropriate status codes for CI/CD integration.
haniyakonain
force-pushed
the
add-stats-redirects-test
branch
2 times, most recently
from
5e4b2ae to
7731575
Compare
…ages - Create stats-redirects-test.sh to verify HTTP 200 responses - Restore extractor initialization with updateAll() call - Configure server to use @mappings languages - Test validates stats and redirects endpoints for each language - Reports clear pass/fail results with detailed error messages
The @mappings token may be causing server startup issues. Using explicit language list (de,en) to fix CI/CD timeout.
haniyakonain
force-pushed
the
add-stats-redirects-test
branch
2 times, most recently
from
c26c2ab to
6fb6409
Compare
|
1 participant
Overview
Fixes NullPointerException (NPE) errors when browsing mapping statistics and redirects endpoints by restoring proper extractor initialization. Adds automated testing to validate that statistics and redirects endpoints return HTTP 200 for all languages with mappings support.
Changes
Fixed NPE: Restored extractor.updateAll() call after instantiation to properly initialize extractors and statistics
Configured @mappings: Updated server.default.properties to use @mappings languages instead of hardcoded de,en
Added validation tests: Created stats-redirects-test.sh script that:
Automatically discovers all languages with @mappings support
Tests /statistics/{lang}/ endpoint for each language
Tests /mappings/{lang}/redirects/ endpoint for each language
Reports clear pass/fail results with detailed error messages
Integrated test into GitHub Actions workflow (server-web-api-test.yml)
Testing
To run manually:
The script validates both endpoints for each language and exits with error code if any tests fail.
Summary by CodeRabbit
Tests
Chores