You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
73 lines
3.3 KiB
73 lines
3.3 KiB
4 months ago
|
### TaskUtils.java 代码评审
|
||
|
|
|
||
|
|
**优点：**
|
||
|
|
1. 使用了RedissonClient来获取分布式锁，确保了在高并发环境下的锁的正确性。
|
||
|
|
2. 使用tryLock方法尝试获取锁，并且设置了超时时间，这是一个好的实践，可以防止死锁。
|
||
|
|
3. 使用Spring的PlatformTransactionManager来管理事务，确保了方法内部的代码在一个事务中执行，要么全部成功，要么全部回滚。
|
||
|
|
|
||
|
|
**改进点：**
|
||
|
|
1. **空指针检查：** 在获取Spring bean时，没有进行空指针检查。`SpringContextHolder.getBean(RedissonClient.class)`可能返回null，应该添加空指针检查。
|
||
|
|
2. **代码重复：** `taskLock`方法中，获取RedissonClient和PlatformTransactionManager的bean被重复调用，应该提取到方法外部或者使用依赖注入来避免重复。
|
||
|
|
3. **异常处理：** 在finally块中，只检查了tryLock是否为true来决定是否释放锁。如果tryLock为false，则不应该尝试释放锁，因为这会导致异常。
|
||
|
|
4. **事务传播行为：** 使用了`PROPAGATION_REQUIRES_NEW`，这可能会导致事务隔离问题。如果这个方法需要与其他事务隔离，应该考虑使用其他传播行为。
|
||
|
|
5. **异常处理：** 如果在执行目标方法时抛出异常，应该在finally块中回滚事务，而不是在try块中。这样可以确保事务的完整性。
|
||
|
|
|
||
|
|
**具体代码改进：**
|
||
|
|
```java
|
||
|
|
public static void taskLock(String code, Runnable function) {
|
||
|
|
RedissonClient redissonClient = SpringContextHolder.getBean(RedissonClient.class);
|
||
|
|
if (redissonClient == null) {
|
||
|
|
throw new IllegalStateException("RedissonClient bean not found");
|
||
|
|
}
|
||
|
|
PlatformTransactionManager txManager = SpringContextHolder.getBean(PlatformTransactionManager.class);
|
||
|
|
if (txManager == null) {
|
||
|
|
throw new IllegalStateException("PlatformTransactionManager bean not found");
|
||
|
|
}
|
||
|
|
RLock lock = redissonClient.getLock(code);
|
||
|
|
boolean tryLock = lock.tryLock(5, TimeUnit.SECONDS);
|
||
|
|
TransactionStatus status = txManager.getTransaction(new DefaultTransactionDefinition());
|
||
|
|
try {
|
||
|
|
if (tryLock) {
|
||
|
|
function.run();
|
||
|
|
txManager.commit(status);
|
||
|
|
} else {
|
||
|
|
throw new BadRequestException("其他设备占用锁，等待结束！");
|
||
|
|
}
|
||
|
|
} catch (Exception e) {
|
||
|
|
txManager.rollback(status);
|
||
|
|
throw e;
|
||
|
|
} finally {
|
||
|
|
if (tryLock) {
|
||
|
|
lock.unlock();
|
||
|
|
}
|
||
|
|
}
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
### ApplicationTest.java 代码评审
|
||
|
|
|
||
|
|
**优点：**
|
||
|
|
1. 使用了多线程来模拟并发访问，这有助于测试锁的正确性。
|
||
|
|
|
||
|
|
**改进点：**
|
||
|
|
1. **资源关闭：** 测试方法中没有关闭RedissonClient，这可能会导致资源泄露。
|
||
|
|
2. **异常处理：** 在测试方法中抛出的异常没有被捕获和处理，可能会导致测试失败。
|
||
|
|
|
||
|
|
**具体代码改进：**
|
||
|
|
```java
|
||
|
|
@Test
|
||
|
|
void testSQLException() throws InterruptedException {
|
||
|
|
for (int i = 0; i < 100; i++) {
|
||
|
|
new Thread(() -> {
|
||
|
|
try {
|
||
|
|
doExec();
|
||
|
|
} catch (Exception e) {
|
||
|
|
// Log the exception or handle it as needed
|
||
|
|
}
|
||
|
|
}).start();
|
||
|
|
Thread.sleep(1000);
|
||
|
|
}
|
||
|
|
}
|
||
|
|
```
|
||
|
|
|
||
|
|
请注意，以上评审是基于提供的代码片段，可能需要根据整个代码库和业务逻辑进行调整。
|