Clean up distribution lock

[avgustinmm]

No description provided.

[Copilot]

Pull request overview

This PR refactors the JPA distributed lock implementation to simplify DistributedLockRepository construction/usage and align the lock “refresh” mechanism with newer Spring Integration APIs (renewal with explicit TTL).

Changes:

• Remove PlatformTransactionManager wiring from distributed lock repository beans (main config + test config).
• Simplify DistributedLockRepository by computing refresh scheduling parameters once and switching scheduled refresh logic to “renew”.
• Add JSpecify nullness annotations and update related comments.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockTest.java	Updates test Spring config to construct the lock repository without a transaction manager.
hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java	Updates distributed lock LockRepository bean wiring to match the simplified repository constructor.
hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockRepository.java	Refactors distributed lock repository: removes tx-manager workaround, introduces nullness annotations, and switches scheduled refresh to renew semantics.

Comments suppressed due to low confidence (3)

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockRepository.java:69

• The superclass TTL is no longer being initialized (previously via setTimeToLive(lockProperties.getTtl())). This can cause JdbcLockRegistry instances that rely on the repository’s configured TTL (e.g., constructed without an explicit TTL) to use DefaultLockRepository’s default instead, leading to premature lock expiry and broken renew scheduling. Consider calling setTimeToLive(lockProperties.getTtl()) (and keeping refreshAfterMillis/renewTtl aligned with that value), or otherwise ensuring all callers pass an explicit TTL consistently.

    public DistributedLockRepository(final DataSource dataSource, final LockProperties lockProperties) {
        super(dataSource);

        renewTtl = Duration.ofMillis(lockProperties.getTtl());

        final int timeToLive = lockProperties.getTtl();
        final int triggerOnRemainMS = Math.max(
                lockProperties.getRefreshOnRemainMS(),
                timeToLive * lockProperties.getRefreshOnRemainPercent() / 100);
        final int refreshAfterMS = timeToLive - triggerOnRemainMS;
        refreshAfterMillis = refreshAfterMS <= 0 ? null : refreshAfterMS;

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockRepository.java:40

• Spelling in Javadoc: "longer then ttl" should be "longer than ttl".

 * Repository for {@link JdbcLockRegistry}. This class is not thread safe.
 * Adds support for keeping lock longer then ttl if really used by the instance (renew).
 */

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/cluster/DistributedLockTest.java:93

• This test constructs JdbcLockRegistry via the single-arg constructor (see keepLockAlive()), which typically relies on the LockRepository’s configured TTL. Since DistributedLockRepository no longer calls setTimeToLive(lockProperties.getTtl()), the registry may fall back to DefaultLockRepository’s default TTL, making this test (and any similar usage) flaky or incorrect. Consider passing the TTL explicitly to JdbcLockRegistry here, or ensuring the repository initializes the superclass TTL.

        private LockRepository lockRepository(final DataSource dataSource, final LockProperties lockProperties) {
            final DefaultLockRepository repository = new DistributedLockRepository(dataSource, lockProperties);
            repository.setPrefix("SP_");
            return repository;
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[vasilchev]

old comment, not in new transaction anymore

[vasilchev]

(optional) maybe setTimeToLive to remain here in this constructor? it is not used it seems in current flow, but it exist in src, and would prefer having it alligned with others rather then magically again involved?