[ https://issues.apache.org/jira/browse/JCR-2874?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12995366#comment-12995366 ]
Thomas Mueller commented on JCR-2874: ------------------------------------- OK, when I try to build everything (on trunk), I also get the exception: "The projects in the reactor contain a cyclic reference..." > Locked helper class improvements > -------------------------------- > > Key: JCR-2874 > URL: https://issues.apache.org/jira/browse/JCR-2874 > Project: Jackrabbit Content Repository > Issue Type: Improvement > Components: jackrabbit-jcr-commons, jackrabbit-spi-commons > Reporter: Alex Parvulescu > Priority: Minor > Attachments: lock_upgrade.diff, lock_upgrade_2_tests.patch, > lock_upgrade_2_wrapper.patch > > > Currently there are 2 helper classes called Locked, that serve the same > purpose: > one in jcr-commons > http://jackrabbit.apache.org/api/2.2/org/apache/jackrabbit/util/Locked.html > and the other one in spi-commons > http://jackrabbit.apache.org/api/2.2/org/apache/jackrabbit/spi/commons/lock/Locked.html > There was a discussion a while back about deprecating one of them. the issue > affected JR 1.4 and it is now closed. I guess the old patches have not been > applied: https://issues.apache.org/jira/browse/JCR-1169 > Anyway, I propose to keep the one in jcr-commons and deprecate the other one. > Also, while we are on the matter, I'd like to propose some improvements to > this class: > 1. make the lock configurable, basically add a flag to say if it is session > scoped or not (currently it is hard coded as a session wide lock). > 2. upgrade the lock code to use the LockManager > (http://www.day.com/maven/javax.jcr/javadocs/jcr-2.0/javax/jcr/lock/LockManager.html) > - this means that the class will rely only on the JCR 2.0. this is a > potentially breaking change, although a lot of the classes in this module > depend directly on the 2.0 API > 3. allow the Locked class to use generics. The biggest issue here is the > timeout behavior: on timeout you get the TIMED_OUT token object, and you have > to compare the response to that, to make sure that the code ran properly. I > think the simplest solution would be to not touch the class directly and > build a wrapper class that is generified, and throws a RepositoryException in > case of a timeout. > This would turn this code( from the javadoc) > Node counter = ...; > Object ret = new Locked() { > protected Object run(Node counter) throws RepositoryException { > ... > } > }.with(counter, false); > if (ret == Locked.TIMED_OUT) { > // do whatever you think is appropriate in this case > } else { > // get the value > long nextValue = ((Long) ret).longValue(); > } > into > Node counter = ...; > Long ret = new LockedWrapper<Long>() { > protected Object run(Node counter) throws RepositoryException { > ... > } > }.with(counter, false); > // handle the timeout as a RepositoryException > 4. lock tests location? (this is more of an observation than an actual issue > it came to me via 'find . -name *Lock*Test.java') > There are some lock tests in jackrabbit-core: > - jackrabbit-core/src/test/java/org/apache/jackrabbit/core/LockTest.java > - > jackrabbit-core/src/test/java/org/apache/jackrabbit/core/lock/LockTimeoutTest.java > - > jackrabbit-core/src/test/java/org/apache/jackrabbit/core/lock/ConcurrentLockingWithTransactionsTest.java > - > jackrabbit-core/src/test/java/org/apache/jackrabbit/core/lock/ExtendedLockingTest.java > - > jackrabbit-core/src/test/java/org/apache/jackrabbit/core/lock/ConcurrentLockingTest.java > ...some in jackrabbit-jcr2spi: > - > jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/OpenScopedLockTest.java > - > jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/SessionScopedLockTest.java > - > jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/DeepLockTest.java > - > jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/AbstractLockTest.java > and others in jackrabbit-jcr-tests: > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/observation/LockingTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/SetValueLockExceptionTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/LockManagerTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/OpenScopedLockTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/LockTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/SessionScopedLockTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/DeepLockTest.java > - > jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/lock/AbstractLockTest.java > I'd like to move this one: org.apache.jackrabbit.core.LockTest from the > jackrabbit-core to the jackrabbit-jcr-commons where the implementation class > lives. > I'll add a patch for 1 & 2, but the rest I need your opinion. -- This message is automatically generated by JIRA. - For more information on JIRA, see: http://www.atlassian.com/software/jira