最近发现一些同事的代码问题(java)

前言

分享几个最近在开发中发现的编码问题，这些问题也不是才入门的同事写的。都是5年以上开发经验的爪哇同事写的。 当然下面这些问题也都不是很难发现的问题。初中级的程序员还是可以仔细看看的(伪代码不用担心泄露公司代码)。

错误案例一（乐观锁的运用）

功能描述：
实现一个批量审批功能，单个审批功能是有现成的方法 approveOne。所以在做批量审批时候，就异步调用approveOne方法，异步执行批量审批的时候优先把批量审批状态更新了（先把数据锁住），打个标记数据数据再次被审批；
代码主要逻辑：
1、先查询出请求参数ids的数据是否都是待审批的状态，不是的话则抛出异常。
2、请求中的数据都是待审的数据，执行前先更新数据状态为审批中，然后异步执行审批业务逻辑

public void batchAprrove(List<Long> ids) {
  log.info("开始批量审批数据");
  //查询出待审批的数量，校验是否都是待审批的数据
  Integer waitApproveNum = service.count(APPROVE_STATUS_PRE,ids);
  if(waitApproveNum != ids.size()){
    throw new RuntimeException("数据状态有更新，请重新操作");
  }
  // 更新成审批中的状态 
  service.updateStatusByIds(APPROVE_STATUS_ING,ids);
  //循环任务丢到线程池
  for (Long id : ids) {
    approveExecutor.execute(()->{
      approveOne(id);
    });
  }
}

存在的问题

上面查询校验之后再更新数据状态，有可能查询的时候数据都是没问题的 ,更新时，数据就被其他线程给更新了，这时候再去更新已经没有意义了。

正确用法

1、执行使用数据库乐观锁思想，直接更新这批数据，多加一个待审批状态的条件，并且后台系统并发低，适合用乐观锁。
2、更新之后，更新行数和ids长度对比，如果长度不同则抛异常，让事务回滚。

@Transactional(rollbackFor = Exception.class)
public void batchAprrove(List<Long> ids) {
  log.info("开始批量审批数据");
  //查询出待审批的数量，校验是否都是待审批的数据
  // 更新成审批中的状态
  Integer row = service.updateIdsAndStatus(APPROVE_STATUS_ING,ids,APPROVE_STATUS_PRE);
  if(row.intValue() != ids.size()){
      throw new RuntimeException("数据状态有更新，请重新操作");
  }
 //循环任务丢到线程池
 for (Long id : ids) {
   approveExecutor.execute(()->{
     approveOne(id);
   });
 }
}

其实还有个问题没有解决？

approveJoinUnion(Long id) 方法在当前类中，是加了事务注解的。batchApprove方法就算加了事务注解，事务执行approveOne方法的时候，事务也失效了？ 只有在异步方法里面加编程式事务了。

<顺便吆喝一句，内推个民族企业核心岗机会；前、后端 / 测试；北京、上海、南京、西安、东莞等多地捞人>

错误案例二（锁和事务的运用）

功能描述：
主要实现一个批量导入功能，因为导入的时间可能比较长，为了防止重复导入，就在方法里面加了锁。并且加了事务，中途异常让数据回滚。

代码主要逻辑：
1、获取锁
2、获取锁之后读取导入记录，解析导入文件执行数据解析，数据验证，数据分组
3、把正确的数据入库
4、导入成功，更新导入记录为成功状态，失败则把记录更新为失败状态

@Transactional(rollbackFor = Exception.class)
public void importData2(Long importId) {
  RLock lock = redissonLockClient.getLock(RedisKeyConstant.MEMBER_IMPORT_LOCK_KEY);

  try {
    lock.lock();
    // 读取导入记录ID,获取导入文件地址，解析数据，数据校验，数据分组
    GroupData data = handleData(importId);
    //插入
    batchInsert(data.getNeedInsertData());
    //更新导入记录成功
    updateSuccessImportRecord(importId);
  } catch (Exception e){
    //表导入记录失败
    updateFailImportRecord(importId);
  }finally {
    if (lock.isHeldByCurrentThread()) {
      lock.unlock();
    }
  }
}

存在的问题

1.锁失效：方法内部执行到最后，已经解锁了。解锁之后，才开始提交事务。这个时候我又导入，在数据校验过程的时候可能上一批数据事务都没提交完成，校验过程中就查询不到上个事务的数据，导出重复导入等问题
2.事务无法回滚，代码里面用try catch，无法捕捉异常

正确用法

1.删除@Transactional(rollbackFor = Exception.class)注解，使用编程式事务；这样就解决了上面两个问题

public void importData2(Long importId) {
  RLock lock = redissonLockClient.getLock(RedisKeyConstant.MEMBER_IMPORT_LOCK_KEY);

  try {
    lock.lock();
    // 读取导入记录ID,获取导入文件地址，解析数据，数据校验，数据分组
    GroupData data = handleData(importId);
    //插入
    transactionTemplate.execute((TransactionCallback<Void>) status -> {
      try {
        //插入数据
        batchInsert(data.getNeedInsertData());
        //更新导入记录成功
        updateSuccessImportRecord(importId);
        return null;
      } catch (Exception e) {
        status.setRollbackOnly();
        throw e;
      }
    });

  } catch (Exception e){
    //表导入记录失败
    updateFailImportRecord(importId);
  }finally {
    if (lock.isHeldByCurrentThread()) {
      lock.unlock();
    }
  }
}

错误案例三（mybatis 一级缓存）

代码主要逻辑：
1、执行starFlow方法，startFlow 调用handFlow 方法（当然实际情况代码十分复杂）
2.这两个方法都调用了 service.getChildren方法参数也一样

@Transactional
public void startFlow(Long flowId) {
    //获取下级节点
    List<Long> ids = service.getChildren(flowId);
      // 把本级节点也加入到list
    ids.add(2l);
    //.....代码上省略
   handFlow(flowId);
}

public void handFlow(Long flowId){
   //查询下级节点
  List<Long> ids = service.getChildren(flowId);
  //.....代码上省略
}

存在的问题

1.第一次调用方法的时候 service.getChildren(flowId) 将结果集ids修改了，调用了ids.add(21) 。后去方法里面有调用同样的方法，同样的参数，mybatis直接从缓存里面去了。导致执行结果偏预期（同一个事务内）。

正确用法

1.尽量不要修改mapper返回的引用结果吧，不然后面执行同样的sql会直接去缓存查数据（同一个事务内）。

——转自作者：提前退休了