设计模式简记-通过重构增强代码可测试性实战

Posted wod-y

tags:

篇首语：本文由小常识网(cha138.com)小编为大家整理，主要介绍了设计模式简记-通过重构增强代码可测试性实战相关的知识，希望对你有一定的参考价值。

4.1 通过重构增强代码可测试性实战

  • 代码可测试性：

    针对代码编写单元测试的难易程度。对于一段代码，如果很难为其编写单元测试，或者单元测试写起来很费劲，需要依靠单元测试框架中很高级的特性，那往往就意味着代码设计得不够合理，代码的可测试性不好

4.1.1 需求

Transaction 是经过我抽象简化之后的一个电商系统的交易类，用来记录每笔订单交易的情况。Transaction 类中的 execute() 函数负责执行转账操作，将钱从买家的钱包转到卖家的钱包中。真正的转账操作是通过调用 WalletRpcService RPC 服务来完成的。除此之外，代码中还涉及一个分布式锁 DistributedLock 单例类，用来避免 Transaction 并发执行，导致用户的钱被重复转出。

public class Transaction {
  private String id;
  private Long buyerId;
  private Long sellerId;
  private Long productId;
  private String orderId;
  private Long createTimestamp;
  private Double amount;
  private STATUS status;
  private String walletTransactionId;
  
  // ...get() methods...
  
  public Transaction(String preAssignedId, Long buyerId, Long sellerId, Long productId, String orderId) {
    if (preAssignedId != null && !preAssignedId.isEmpty()) {
      this.id = preAssignedId;
    } else {
      this.id = IdGenerator.generateTransactionId();
    }
    if (!this.id.startWith("t_")) {
      this.id = "t_" + preAssignedId;
    }
    this.buyerId = buyerId;
    this.sellerId = sellerId;
    this.productId = productId;
    this.orderId = orderId;
    this.status = STATUS.TO_BE_EXECUTD;
    this.createTimestamp = System.currentTimestamp();
  }
  
  public boolean execute() throws InvalidTransactionException {
    if ((buyerId == null || (sellerId == null || amount < 0.0) {
      throw new InvalidTransactionException(...);
    }
    if (status == STATUS.EXECUTED) return true;
    boolean isLocked = false;
    try {
      isLocked = RedisDistributedLock.getSingletonIntance().lockTransction(id);
      if (!isLocked) {
        return false; // 锁定未成功，返回false，job兜底执行
      }
      if (status == STATUS.EXECUTED) return true; // double check
      long executionInvokedTimestamp = System.currentTimestamp();
      if (executionInvokedTimestamp - createdTimestap > 14days) {
        this.status = STATUS.EXPIRED;
        return false;
      }
      WalletRpcService walletRpcService = new WalletRpcService();
      String walletTransactionId = walletRpcService.moveMoney(id, buyerId, sellerId, amount);
      if (walletTransactionId != null) {
        this.walletTransactionId = walletTransactionId;
        this.status = STATUS.EXECUTED;
        return true;
      } else {
        this.status = STATUS.FAILED;
        return false;
      }
    } finally {
      if (isLocked) {
       RedisDistributedLock.getSingletonIntance().unlockTransction(id);
      }
    }
  }
}

4.1.2 测试用例设计

  1. 正常情况下，交易执行成功，回填用于对账（交易与钱包的交易流水）用的 walletTransactionId，交易状态设置为 EXECUTED，函数返回 true。
  2. buyerId、sellerId 为 null、amount 小于 0，返回 InvalidTransactionException。
  3. 交易已过期（createTimestamp 超过 14 天），交易状态设置为 EXPIRED，返回 false。
  4. 交易已经执行了（status==EXECUTED），不再重复执行转钱逻辑，返回 true。
  5. 钱包（WalletRpcService）转钱失败，交易状态设置为 FAILED，函数返回 false。
  6. 交易正在执行着，不会被重复执行，函数直接返回 false。

4.1.3 测试用例实现

4.1.3.1 测试用例1
public void testExecute() {
  Long buyerId = 123L;
  Long sellerId = 234L;
  Long productId = 345L;
  Long orderId = 456L;
  Transction transaction = new Transaction(null, buyerId, sellerId, productId, orderId);
  boolean executedResult = transaction.execute();
  assertTrue(executedResult);
}

  • 问题

    execute() 函数的执行依赖两个外部的服务，一个是 RedisDistributedLock，一个 WalletRpcService。这就导致上面的单元测试代码存在下面几个问题。

    • 如果要让这个单元测试能够运行，需要搭建 Redis 服务和 Wallet RPC 服务。搭建和维护的成本比较高。
    • 还需要保证将伪造的 transaction 数据发送给 Wallet RPC 服务之后，能够正确返回我们期望的结果，然而 Wallet RPC 服务有可能是第三方（另一个团队开发维护的）的服务，并不是可控的。换句话说，并不是我们想让它返回什么数据就返回什么。
    • Transaction 的执行跟 Redis、RPC 服务通信，需要走网络，耗时可能会比较长，对单元测试本身的执行性能也会有影响。
    • 网络的中断、超时、Redis、RPC 服务的不可用，都会影响单元测试的执行。

  • 解决WalletRpcService mock的问题

    • 继承 WalletRpcService 类，并且重写其中的 moveMoney() 函数的方式来实现 mock。具体的代码实现如下所示。通过 mock 的方式，可以让 moveMoney() 返回任意我们想要的数据，完全在我们的控制范围内，并且不需要真正进行网络通信

      public class MockWalletRpcServiceOne extends WalletRpcService {
  public String moveMoney(Long id, Long fromUserId, Long toUserId, Double amount) {
    return "123bac";
  } 
}

public class MockWalletRpcServiceTwo extends WalletRpcService {
  public String moveMoney(Long id, Long fromUserId, Long toUserId, Double amount) {
    return null;
  } 
}

    • 重构使WalletRpcService 可动态配置，增强 execute() 方法的可测试性

      public class Transaction {
  //...
  // 添加一个成员变量及其set方法
  private WalletRpcService walletRpcService;
  
  public void setWalletRpcService(WalletRpcService walletRpcService) {
    this.walletRpcService = walletRpcService;
  }
  // ...
  public boolean execute() {
    // ...
    // 删除下面这一行代码
    // WalletRpcService walletRpcService = new WalletRpcService();
    // ...
  }
}

    • 测试实现

      public void testExecute() {
  Long buyerId = 123L;
  Long sellerId = 234L;
  Long productId = 345L;
  Long orderId = 456L;
  Transction transaction = new Transaction(null, buyerId, sellerId, productId, orderId);
  // 使用mock对象来替代真正的RPC服务
  transaction.setWalletRpcService(new MockWalletRpcServiceOne()):
  boolean executedResult = transaction.execute();
  assertTrue(executedResult);
  assertEquals(STATUS.EXECUTED, transaction.getStatus());
}

  • 解决RedisDistributedLock的替换和mock问题

    RedisDistributedLock 是一个单例类。单例相当于一个全局变量，无法 mock（无法继承和重写方法），也无法通过依赖注入的方式来替换，且可能不是本方维护的类。

    • 可以对 transaction 上锁这部分逻辑重新封装一下：

      public class TransactionLock {
  public boolean lock(String id) {
    return RedisDistributedLock.getSingletonIntance().lockTransction(id);
  }
  
  public void unlock() {
    RedisDistributedLock.getSingletonIntance().unlockTransction(id);
  }
}

public class Transaction {
  //...
  private TransactionLock lock;
  
  public void setTransactionLock(TransactionLock lock) {
    this.lock = lock;
  }
 
  public boolean execute() {
    //...
    try {
      isLocked = lock.lock();
      //...
    } finally {
      if (isLocked) {
        lock.unlock();
      }
    }
    //...
  }
}

    • 测试实现

      public void testExecute() {
  Long buyerId = 123L;
  Long sellerId = 234L;
  Long productId = 345L;
  Long orderId = 456L;
  
  TransactionLock mockLock = new TransactionLock() {
    public boolean lock(String id) {
      return true;
    }
  
    public void unlock() {}
  };
  
  Transction transaction = new Transaction(null, buyerId, sellerId, productId, orderId);
  transaction.setWalletRpcService(new MockWalletRpcServiceOne());
  transaction.setTransactionLock(mockLock);
  boolean executedResult = transaction.execute();
  assertTrue(executedResult);
  assertEquals(STATUS.EXECUTED, transaction.getStatus());
}
4.1.3.2 测试用例3

测试用例 3：交易已过期（createTimestamp 超过 14 天），交易状态设置为 EXPIRED，返回 false。上代码：

public void testExecute_with_TransactionIsExpired() {
  Long buyerId = 123L;
  Long sellerId = 234L;
  Long productId = 345L;
  Long orderId = 456L;
  Transction transaction = new Transaction(null, buyerId, sellerId, productId, orderId);
  transaction.setCreatedTimestamp(System.currentTimestamp() - 14days);
  boolean actualResult = transaction.execute();
  assertFalse(actualResult);
  assertEquals(STATUS.EXPIRED, transaction.getStatus());
}

  • 问题

    如果在 Transaction 类中，并没有暴露修改 createdTimestamp 成员变量的 set 方法，上述代码是有问题的也不能随便给类添加setCreatedTimestamp方法，违反封装性：createTimestamp 是在交易生成时（也就是构造函数中）自动获取的系统时间，本来就不应该人为地轻易修改

  • 解决问题

    处理方式是将这种未决行为逻辑重新封装。

    • 重构

      针对 Transaction 类，只需要将交易是否过期的逻辑，封装到 isExpired() 函数中即可，具体的代码实现如下所示：

      public class Transaction {
  protected boolean isExpired() {
    long executionInvokedTimestamp = System.currentTimestamp();
    return executionInvokedTimestamp - createdTimestamp > 14days;
  }
  
  public boolean execute() throws InvalidTransactionException {
    //...
      if (isExpired()) {
        this.status = STATUS.EXPIRED;
        return false;
      }
    //...
  }
}

    • 测试实现

      public void testExecute_with_TransactionIsExpired() {
  Long buyerId = 123L;
  Long sellerId = 234L;
  Long productId = 345L;
  Long orderId = 456L;
  Transction transaction = new Transaction(null, buyerId, sellerId, productId, orderId) {
    protected boolean isExpired() {
      return true;
    }
  };
  boolean actualResult = transaction.execute();
  assertFalse(actualResult);
  assertEquals(STATUS.EXPIRED, transaction.getStatus());
}
4.1.3.3 其他问题

  • Transaction 类的构造函数的设计还有点不妥：交易 id 的赋值逻辑稍微复杂。最好也要测试一下，以保证这部分逻辑的正确性

    public void testExecute_with_TransactionIsExpired() {
  Long buyerId = 123L;
  Long sellerId = 234L;
  Long productId = 345L;
  Long orderId = 456L;
  Transction transaction = new Transaction(null, buyerId, sellerId, productId, orderId) {
    protected boolean isExpired() {
      return true;
    }
  };
  boolean actualResult = transaction.execute();
  assertFalse(actualResult);
  assertEquals(STATUS.EXPIRED, transaction.getStatus());
}

  • 重构一下，这样在测试代码中就可以通过重写fillTransactionId来mock测试

      public Transaction(String preAssignedId, Long buyerId, Long sellerId, Long productId, String orderId) {
    //...
    fillTransactionId(preAssignId);
    //...
  }
  
  protected void fillTransactionId(String preAssignedId) {
    if (preAssignedId != null && !preAssignedId.isEmpty()) {
      this.id = preAssignedId;
    } else {
      this.id = IdGenerator.generateTransactionId();
    }
    if (!this.id.startWith("t_")) {
      this.id = "t_" + preAssignedId;
    }
  }

4.1.4 其他常见的 Anti-Patterns

4.1.4.1 未决行为

代码的输出是随机或者说不确定的，比如，跟时间、随机数有关的代码，

4.1.4.2 全局变量

滥用全局变量也让编写单元测试变得困难：多个测试用例涉及到全局变量时会互相影响预期初始值

4.1.4.3 静态方法

静态方法跟全局变量一样，也是一种面向过程的编程思维。在代码中调用静态方法，有时候会导致代码不易测试

Math.abs() 这样的简单静态方法，本身并不需要 mock。复杂的耗时长的静态方法需要mock

4.1.4.4 复杂继承
4.1.4.5 高耦合代码

以上是关于设计模式简记-通过重构增强代码可测试性实战的主要内容，如果未能解决你的问题，请参考以下文章

代码质量重构可测试性解耦杂谈

代码质量重构可测试性解耦杂谈

优秀代码规范

仅可测试性是依赖注入的理由吗？

可测试性战术

如何编写具有可测试性的代码