davsclaus commented on code in PR #25726:
URL: https://github.com/apache/camel/pull/25726#discussion_r3861624950
##########
components/camel-file/src/main/java/org/apache/camel/component/file/strategy/FileIdempotentRepositoryReadLockStrategy.java:
##########
@@ -95,8 +95,9 @@ public boolean
acquireExclusiveReadLock(GenericFileOperations<File> operations,
public void releaseExclusiveReadLockOnAbort(
GenericFileOperations<File> operations, GenericFile<File> file,
Exchange exchange)
throws Exception {
Review Comment:
Confirmed and fixed in 42173c8. You're right that
`releaseExclusiveReadLockOnAbort` can also run after a successful acquire —
`GenericFileConsumer.processExchange()` calls `abort()` whenever `begin()`
returns `false` *or throws*, and `GenericFileRenameProcessStrategy.begin()`
calls `super.begin()` (which acquires the lock) before running
`beginRenamer.renameFile()` for `preMove`, which can throw afterwards.
Applied your suggested approach: added
`Exchange.FILE_LOCK_IDEMPOTENT_ACQUIRED`, set at the end of
`acquireExclusiveReadLock` (mirroring `MarkerFileExclusiveReadLockStrategy`'s
`FILE_LOCK_FILE_ACQUIRED` pattern), so `releaseExclusiveReadLockOnAbort` only
removes the key when we actually own it — correctly handling both this PR's
pre-existing-key scenario and CAMEL-24093's abort-after-acquire scenario. Added
`FileIdempotentRepositoryReadLockStrategyAbortTest` which directly exercises
both branches (verified it fails against the previous no-op version).
_Claude Sonnet 5 on behalf of @davsclaus_
##########
components/camel-file/src/main/java/org/apache/camel/component/file/strategy/FileIdempotentChangedRepositoryReadLockStrategy.java:
##########
@@ -115,8 +115,10 @@ public boolean
acquireExclusiveReadLock(GenericFileOperations<File> operations,
public void releaseExclusiveReadLockOnAbort(
Review Comment:
Confirmed and fixed in 42173c8. You're right that
`releaseExclusiveReadLockOnAbort` can also run after a successful acquire —
`GenericFileConsumer.processExchange()` calls `abort()` whenever `begin()`
returns `false` *or throws*, and `GenericFileRenameProcessStrategy.begin()`
calls `super.begin()` (which acquires the lock) before running
`beginRenamer.renameFile()` for `preMove`, which can throw afterwards.
Applied your suggested approach: added
`Exchange.FILE_LOCK_IDEMPOTENT_ACQUIRED`, set at the end of
`acquireExclusiveReadLock` (mirroring `MarkerFileExclusiveReadLockStrategy`'s
`FILE_LOCK_FILE_ACQUIRED` pattern), so `releaseExclusiveReadLockOnAbort` only
removes the key when we actually own it — correctly handling both this PR's
pre-existing-key scenario and CAMEL-24093's abort-after-acquire scenario. Added
`FileIdempotentRepositoryReadLockStrategyAbortTest` which directly exercises
both branches (verified it fails against the previous no-op version).
_Claude Sonnet 5 on behalf of @davsclaus_
##########
components/camel-file/src/main/java/org/apache/camel/component/file/strategy/FileIdempotentRenameRepositoryReadLockStrategy.java:
##########
@@ -107,8 +107,10 @@ public boolean
acquireExclusiveReadLock(GenericFileOperations<File> operations,
public void releaseExclusiveReadLockOnAbort(
Review Comment:
Confirmed and fixed in 42173c8. You're right that
`releaseExclusiveReadLockOnAbort` can also run after a successful acquire —
`GenericFileConsumer.processExchange()` calls `abort()` whenever `begin()`
returns `false` *or throws*, and `GenericFileRenameProcessStrategy.begin()`
calls `super.begin()` (which acquires the lock) before running
`beginRenamer.renameFile()` for `preMove`, which can throw afterwards.
Applied your suggested approach: added
`Exchange.FILE_LOCK_IDEMPOTENT_ACQUIRED`, set at the end of
`acquireExclusiveReadLock` (mirroring `MarkerFileExclusiveReadLockStrategy`'s
`FILE_LOCK_FILE_ACQUIRED` pattern), so `releaseExclusiveReadLockOnAbort` only
removes the key when we actually own it — correctly handling both this PR's
pre-existing-key scenario and CAMEL-24093's abort-after-acquire scenario. Added
`FileIdempotentRepositoryReadLockStrategyAbortTest` which directly exercises
both branches (verified it fails against the previous no-op version).
_Claude Sonnet 5 on behalf of @davsclaus_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
