UNOMI-955: Fix CI Javadoc failures and a dead shutdown crash-recovery path#780
Merged
Conversation
… path Rewrites Javadoc across api, services-common, persistence-spi, and the router extensions so the build's doclint pass goes clean, and wires a --javadoc flag into build.sh (enabled by default under --ci) to catch regressions going forward. While reviewing the diff, also fixed a real bug that had snuck into SchedulerServiceImpl: the new shutdown logic meant to mark RUNNING tasks as CRASHED was reading and saving through wrapper methods that silently no-op once the shutdown flag is set, so it never actually persisted anything. It now goes through the persistence provider directly. Added two unit tests that reproduce the bug against the old code and pass against the fix. Also replaced a hardcoded retry count in GraphQLListIT with a multiple of the shared DEFAULT_TRYING_TRIES constant.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves CI reliability and documentation build stability by (1) increasing test resiliency on loaded CI runners, (2) fixing scheduler shutdown crash-recovery persistence so RUNNING tasks don’t poison subsequent runs, and (3) cleaning up Javadoc/doclint issues and adding a build-script option to validate Javadocs locally.
Changes:
- Fix
SchedulerServiceImpl.preDestroy()to persistently mark RUNNING tasks as CRASHED during shutdown, and add regression tests. - Reduce CI flakiness by increasing scheduler test timeouts and increasing GraphQL list IT retry attempts.
- Clean up malformed/invalid Javadoc across multiple modules and add
--javadoc(and related logging flags) tobuild.sh.
Reviewed changes
Copilot reviewed 55 out of 56 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/src/main/java/org/apache/unomi/services/impl/scheduler/SchedulerServiceImpl.java | Adds shutdown-time CRASHED marking for RUNNING tasks. |
| services/src/test/java/org/apache/unomi/services/impl/scheduler/SchedulerServiceImplTest.java | Increases timeout and adds shutdown crash-marking regression tests. |
| itests/src/test/java/org/apache/unomi/itests/graphql/GraphQLListIT.java | Increases retry budget for async rule-engine processing on CI. |
| build.sh | Adds --javadoc validation and log-file output options; enables Javadoc in --ci. |
| services-common/src/main/java/org/apache/unomi/services/common/service/AbstractContextAwareService.java | Javadoc cleanup/clarification for tenant-context services. |
| services-common/src/main/java/org/apache/unomi/services/common/security/KarafSecurityService.java | Adds/cleans Javadoc for security service behavior. |
| services-common/src/main/java/org/apache/unomi/services/common/security/ExecutionContextManagerImpl.java | Adds/cleans Javadoc for thread-local execution context handling. |
| services-common/src/main/java/org/apache/unomi/services/common/security/AuditServiceImpl.java | Adds/cleans Javadoc for audit/persistence behavior. |
| services-common/src/main/java/org/apache/unomi/services/common/cache/AbstractMultiTypeCachingService.java | Javadoc cleanup for cache lifecycle and tenant/system inheritance behavior. |
| persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/PastEventConditionPersistenceQueryBuilder.java | Removes invalid/fragile Javadoc reference. |
| persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/evaluator/ConditionEvaluatorDispatcher.java | Removes invalid/fragile Javadoc reference. |
| persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionContextHelper.java | Fixes malformed Javadoc and clarifies overload behavior. |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/strategy/StringLinesAggregationStrategy.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/strategy/ArrayListAggregationStrategy.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportToUnomiRouteBuilder.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportOneShotRouteBuilder.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportProducerRouteBuilder.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/UnomiStorageProcessor.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/LineSplitProcessor.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/LineSplitFailureHandler.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/LineBuildProcessor.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ImportRouteCompletionProcessor.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ImportConfigByFileNameProcessor.java | Fixes malformed Javadoc structure around <p> blocks. |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ExportRouteCompletionProcessor.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/bean/CollectProfileBean.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/services/ProfileImportService.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/services/ProfileExportService.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/services/ImportExportConfigurationService.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/ProfileToImport.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/IRouterCamelContext.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/ImportExportConfiguration.java | Fixes malformed Javadoc (</p>). |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/exceptions/BadProfileDataFormatException.java | Fixes malformed Javadoc (</p>). |
| api/src/main/java/org/apache/unomi/api/utils/ConditionBuilder.java | Adds missing field/class Javadoc to satisfy doclint. |
| api/src/main/java/org/apache/unomi/api/tenants/security/TenantSecurityService.java | Removes invalid @throws Javadoc entry. |
| api/src/main/java/org/apache/unomi/api/tasks/ScheduledTask.java | Javadoc cleanup and clarification of scheduling/task model fields. |
| api/src/main/java/org/apache/unomi/api/services/SchedulerService.java | Escapes <= in Javadoc to satisfy doclint. |
| api/src/main/java/org/apache/unomi/api/services/ProfileService.java | Adds missing @return Javadoc. |
| api/src/main/java/org/apache/unomi/api/services/ConfigSharingService.java | Expands/cleans Javadoc for cross-bundle config sharing and events. |
| api/src/main/java/org/apache/unomi/api/services/ConditionValidationService.java | Expands/cleans Javadoc for validation behavior and error models. |
| api/src/main/java/org/apache/unomi/api/services/cache/CacheableTypeConfig.java | Adds Javadoc for TriConsumer.accept. |
| api/src/main/java/org/apache/unomi/api/query/Aggregate.java | Expands/cleans Javadoc for aggregation configuration. |
| api/src/main/java/org/apache/unomi/api/Item.java | Adds missing setter/getter Javadoc to satisfy doclint. |
| api/src/main/java/org/apache/unomi/api/exceptions/BadSegmentConditionException.java | Adds constructor Javadoc. |
| api/src/main/java/org/apache/unomi/api/CustomItem.java | Adds getter/setter Javadoc. |
| api/src/main/java/org/apache/unomi/api/ContextResponse.java | Adds Javadoc for getters/setters and deprecated methods. |
| api/src/main/java/org/apache/unomi/api/ContextRequest.java | Adds Javadoc for getters/setters. |
| api/src/main/java/org/apache/unomi/api/ConsentStatus.java | Adds enum value comments. |
| api/src/main/java/org/apache/unomi/api/conditions/ConditionValidation.java | Adds enum and accessor Javadoc. |
| api/src/main/java/org/apache/unomi/api/conditions/ConditionType.java | Adds item-type constant Javadoc. |
| api/src/main/java/org/apache/unomi/api/ClusterNode.java | Adds constant comment. |
| api/src/main/java/org/apache/unomi/api/actions/ActionType.java | Adds item-type constant Javadoc. |
| .gitignore | Ignores local venvs and generated Javadoc artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The lock-owner-only query skipped stale RUNNING tasks with no lock owner. Scan RUNNING tasks and mark those owned by this node or unowned as CRASHED, while still leaving tasks locked by other cluster nodes untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary for non-technical readers
This change fixes two things that were making our automated tests unreliable on busy build servers, and fixes a documentation build error that was breaking our nightly publish job.
--javadocoption to our build script so developers can catch this kind of error on their own machine before pushing, instead of finding out from a failed CI job.Linked ticket
UNOMI-955 — Fix recurring CI failures and add local Javadoc validation to build script
Ticket scope:
ProfileServiceImplTest,SegmentServiceImplTest,RulesServiceImplTest).</p>closing tags in their Javadoc comments, breaking the nightly Javadoc publish job.What changed
api,services-common,persistence-spi, and the router extensions somvn javadoc:javadocruns clean (no doclint errors), removing the malformed</p>tags called out in the ticket.--javadocflag tobuild.sh(also implied by--ci) that runsjavadoc:javadocand fails the build on doclint errors, so this class of failure is now catchable locally.SchedulerServiceImpl's shutdown sequence was meant to mark any task stillRUNNINGasCRASHEDso it can be safely rescheduled, but it queried/saved tasks through wrapper methods (getAllTasks()/saveTask()) that silently no-op once the shutdown flag is set — so the crash-marking never actually persisted. Reworked it to go through the persistence provider directly.SchedulerServiceImplTestthat reproduce the bug against the old code (both fail withexpected: <CRASHED> but was: <RUNNING>) and pass against the fix.SchedulerServiceImplTest.TEST_TIMEOUTfrom 5s to 15s, and replaced a hardcoded retry count of30inGraphQLListITwithDEFAULT_TRYING_TRIES * 3, to give both suites more margin on loaded CI runners.Test plan
mvn javadoc:javadoc -DskipTestsonapi,services,services-common,persistence-spi,extensions/router/router-api,extensions/router/router-core— no doclint errorsmvn -pl services test -Dtest=SchedulerServiceImplTest— 33/33 passing, including the two new regression testsmvn -pl itests -P integration-tests test-compile—GraphQLListITchange compiles