feat(plugins): consolidate lifecycle lock ownership

[fronzec]

Closes #84

Summary

• Consolidates dynamic plugin classloader ownership in DynamicJobLoaderService and removes duplicate ownership from PluginRegistryService.
• Makes unregister failures observable while preserving registry state, and safely rolls back late load failures before releasing classloader resources.
• Introduces reclaimable, reference-counted per-definition lifecycle locks with focused concurrency and failure-path coverage.

This is the first stacked slice of the plugin lifecycle integrity work. Full unload/reload transition serialization remains intentionally out of scope for this PR.

Type of Change

• Bug fix
• New feature
• Documentation only
• Code refactoring
• Maintenance/tooling
• Breaking change

Changes

File	Change
PluginRegistryService.java	Removes classloader reference ownership, simplifies dynamic registration, and preserves the plugin map when JobRegistry.unregister fails.
DynamicJobLoaderService.java	Adds reclaimable lifecycle lock holders and robust rollback/cleanup behavior for failures after plugin registration.
PluginRegistryServiceTest.java	Covers single-owner classloader semantics and observable unregister failure with preserved plugin state.
DynamicJobLoaderServiceTest.java	Covers rollback ordering, classloader retention on rollback failure, lock exclusion, independent IDs, queued users, and holder reclamation.

Test Plan

• Run the service test suite: mvn test -f fr-batch-service/pom.xml
• Verify registry unregister success and failure behavior.
• Verify classloader rollback and cleanup ownership on late load failures.
• Verify same-definition serialization, independent-definition concurrency, and lifecycle lock reclamation.
• Confirm GitHub CI, contract tests, and static analysis checks pass for commit b79178f.

Contributor Checklist

• Linked an approved issue with Closes #84.
• Added exactly one type:* label: type:feature.
• Ran the relevant automated tests; no shell scripts are modified by this PR.
• Tested the affected plugin lifecycle behavior in the target service.
• Updated documentation where behavior changed; no standalone documentation update is required.
• Used conventional commit format.
• Confirmed there are no Co-Authored-By trailers.

[fronzec]

• feat(plugins): serialize lifecycle transitions #87
• feat(plugins): consolidate lifecycle lock ownership #85 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

📝 Walkthrough

Walkthrough

Dynamic plugin registration no longer retains classloader references. Unregistration now preserves plugin-map state when registry removal fails. Dynamic job loading uses reference-counted, reclaimable per-definition lifecycle locks with expanded concurrency coverage.

Changes

Plugin lifecycle infrastructure

Layer / File(s)	Summary
Registry ownership and failure handling fr-batch-service/src/main/java/.../plugins/PluginRegistryService.java, fr-batch-service/src/test/.../plugins/PluginRegistryServiceTest.java	Removes stored classloader references, changes dynamic registration to accept only the plugin, and verifies unregister failures propagate while preserving the plugin entry.
Reclaimable lifecycle locking fr-batch-service/src/main/java/.../plugins/loader/DynamicJobLoaderService.java, fr-batch-service/src/test/.../plugins/loader/DynamicJobLoaderServiceTest.java	Replaces per-definition locks with reference-counted lifecycle lock holders, adds lock reclamation and concurrency coverage, and updates registry calls to the new API.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant DynamicJobLoaderService
  participant LifecycleLockHolder
  participant PluginRegistryService
  Caller->>DynamicJobLoaderService: loadJob(definitionId)
  DynamicJobLoaderService->>LifecycleLockHolder: acquire definition lock
  DynamicJobLoaderService->>PluginRegistryService: registerDynamicPlugin(plugin)
  PluginRegistryService-->>DynamicJobLoaderService: registration result
  DynamicJobLoaderService->>LifecycleLockHolder: release reference
  LifecycleLockHolder-->>DynamicJobLoaderService: remove unused holder

Loading

Possibly related PRs

• fronzec/spring-batch-projects#83: Both changes serialize concurrent dynamic loads by definition ID and update DynamicJobLoaderService locking behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies #84 by removing classloaderRefs, consolidating ownership in the loader, adding reclaimable lifecycle locks, and covering failure behavior in tests.
Out of Scope Changes check	✅ Passed	No clear out-of-scope changes are evident; the code and tests stay focused on ownership consolidation and lifecycle lock infrastructure.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly reflects the main change: consolidating plugin lifecycle lock ownership.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/plugin-lifecycle-integrity-pr-c-1

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java (1)

47-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the Javadocs after removing registry classloader ownership.

The former field comment now documents the constructor, @param classLoaderRef no longer exists, and unregister documentation still references classloaderRefs. Update these to describe the new API and failure-preserving map behavior.

As per coding guidelines, “Provide Javadoc comments for all public classes, interfaces, methods, and significant fields explaining their purpose, parameters, and return values.”

Also applies to: 185-185, 252-255

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java`
around lines 47 - 52, Update the Javadocs in PluginRegistryService to match the
API after registry classloader ownership was removed: move the former field
description off the constructor, remove the obsolete `@param` classLoaderRef
reference, and revise unregister documentation to describe the map’s
failure-preserving behavior. Ensure all public methods and significant fields
document their current purpose, parameters, and return values.

Source: Coding guidelines

🧹 Nitpick comments (1)

fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java (1)

479-507: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Wait until contenders join the holder before asserting exclusion.

Line 498 may execute before the second task starts, while Line 565 only proves one map entry exists—not that both queued callers incremented users. Wait for the holder count to reach 2/3 before asserting and releasing the first caller.

Also applies to: 545-576

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java`
around lines 479 - 507, Update
withDefinitionLock_sameIdExcludesConcurrentCallers and the analogous test around
the shared lifecycle-lock assertions to await the lock holder’s users count
reaching 2/3, confirming contenders have joined before asserting exclusion or
releasing the first caller. Use the existing lifecycle-lock inspection helpers
and retain the current cleanup and completion assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java`:
- Line 238: Track whether registration succeeded around
DynamicJobLoaderService’s pluginRegistryService.registerDynamicPlugin call, and
in the catch cleanup unregister the plugin before closing/removing its
classloader. If unregistering fails, preserve the classloader entry so the
registered plugin is not orphaned; only release the classloader after successful
rollback.

---

Outside diff comments:
In
`@fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java`:
- Around line 47-52: Update the Javadocs in PluginRegistryService to match the
API after registry classloader ownership was removed: move the former field
description off the constructor, remove the obsolete `@param` classLoaderRef
reference, and revise unregister documentation to describe the map’s
failure-preserving behavior. Ensure all public methods and significant fields
document their current purpose, parameters, and return values.

---

Nitpick comments:
In
`@fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java`:
- Around line 479-507: Update withDefinitionLock_sameIdExcludesConcurrentCallers
and the analogous test around the shared lifecycle-lock assertions to await the
lock holder’s users count reaching 2/3, confirming contenders have joined before
asserting exclusion or releasing the first caller. Use the existing
lifecycle-lock inspection helpers and retain the current cleanup and completion
assertions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b7c684c-922f-4a7b-86a8-63568c84cf70

📥 Commits

Reviewing files that changed from the base of the PR and between 8c103c7 and bb83a1a.

📒 Files selected for processing (4)

• fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryService.java
• fr-batch-service/src/main/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderService.java
• fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/PluginRegistryServiceTest.java
• fr-batch-service/src/test/java/com/fronzec/frbatchservice/batchjobs/plugins/loader/DynamicJobLoaderServiceTest.java

[fronzec]

Addressed the first two CodeRabbit findings: late load failures now roll back registration before classloader cleanup, classloader ownership is retained if rollback unregister fails, and stale registry Javadocs were updated. Focused tests: 35 passed. Full module: 134 passed. The concurrency timing nitpick was intentionally not included in this update.