重构遗留应用：案例研究
本文是InfoQ中的一篇关于遗留系统重构的文章，该文基于一个真实案例，讲述了如何在重构遗留系统时编写单元测试，以及单元测试又是如何确保了重构的正确性。(2013.03.03最后更新) 遗留代码臭不可闻。每个优秀的开发者都想对这样的代码进行重构，但为了重构，理想情况下，应该有一组单元测试用例以防止程序退化。但为遗留程序编写单元测试绝非易事；遗留代码往往是一团乱麻。对遗留程序编写高效的单元，可能需要先对它进行重构；而为了重构它，你又需要单元测试去确保重构不会破坏任何功能。所以这是一个先有鸡，还是先有蛋的问题。通过对我曾经工作过的一个真实案例的分享，本文描述了一种对遗留程序进行安全重构的方法论。问题陈述 在本文中，我使用一个真实案例阐述了在测试与重构遗留系统时的一种实用的实践过程。本案例由Java编写，但这一做法应该也适用于其它语言。为了照顾无经验者，我对本案例的场景进行了修改，并且对其进行了简化以使其更易于理解。本文介绍的实践均有助于最近我对遗留系统的重构。 这不是一篇 关于单元测试与重构基本技能的文章。要对关于这方面的主题进行更多学习，你可以阅读：Martin Fowler的Refactoring: Improving the Design of Existing Code，以及Joshua Kerievsky的Refactoring to Patterns。本文不会描述在现实项目中时常会遇到的复杂场景，但我希望本文能为处理这些复杂场景提供一些有用的方法。 在该案例研究中，我将使用的例子是一个虚构的资源管理系统，在该系统中，一个资源代表一个能被分配一些任务的人。可以认为，一个资源能被分配一个Ticket--HR Ticket或IT Ticket。同时，一个资源也能被分配一个Request--HR Request或IT Request。资源经理可以记录资源要完成某项任务的评估时间。资源可以记录他或她在完全这个Ticket和Request时所耗费的实际时间。
柱状图中展示了资源的使用情况，包括评估时间与实际时间。
不复杂吧？然而，在真实系统中，资源会被分配到许多不同类型的任务。即使如此，从技术上看，这个设计并不复杂。然而，当第一次读到这些代码，我感觉我好像是在看一块化石。我能够看到这些代码是如何进化的(或者更应该说是退化)。最开始时，该系统只能处理Request，然后加入了处理Ticket和其它类型任务的功能。然后来一个开发工程师，他编写了处理Request的代码：从数据库中解析数据，将收集到的数据展示到柱状图中。他甚至都没有将数据信息置于一个合适的对象中：class ResourceBreakdownService {
public Map search (Session context) throws SearchException{
//omitted twenty or so lines of code to pull search criteria out of context and verify them, such as the below:
if(resourceIds==null || resourceIds.size ()==0){
throw new SearchException(“Resource list is not provided”);
}
if(resourceId!=null || resourceIds.size()>0){
resourceObjs=resourceDAO.getResourceByIds(resourceIds);
}
//get workload for all requests
Map requestBreakDown=getResourceRequestsLoadBreakdown (resourceObjs,startDate,finishDate);
return requestBreakDown;
}
}
我能肯定你会立即被这段代码的恶臭逼退。例如，你可能会马上想到search不是一个有意义的名字，应该使用Apache Commons类库的CollectionUtil.isEmpty()去测试这个集合对象，你可能也会对返回结果Map提出质疑。 但先等等，臭味还在继续集聚中。又来了一个开发工程师，并使用相同的方法来处理Ticket，所以你会看到这样的代码：// get workload for all tickets
Map ticketBreakdown =getResourceRequestsLoadBreakdown(resourceObjs,startDate,finishDate,ticketSeverity);
Map result=new HashMap();
for(Iterator i = resourceObjs.iterator(); i.hasNext();) {
Resource resource=(Resource)i.next();
Map requestBreakdown2=(Map)requestBreakdown.get(resource);
List ticketBreakdown2=(List)ticketBreakdown.get(resource);
Map resourceWorkloadBreakdown=combineRequestAndTicket(requestBreakdown2, ticketBreakdown2);
result.put(resource,resourceWorkloadBreakdown)
}
return result;
先不必考虑命名，结构的平衡性，以及其它美学方面的关切，最臭不可闻的就是返回的Map对象。Map是一个黑洞，它可以吸入任何对象，但不会为你提供足够的线索去弄清楚它所包含的确切对象。 在这个例子中，{}表示一个Map对象，=>代表一个键-值对象，而[]代表一个集合对象：{resource with id 30000=> [
SummaryOfActualWorkloadForRequestType,
SummaryOfEstimatedWorkloadForRequestType,
{30240=>[
ActualWorkloadForReqeustWithId_30240,
EstimatedWorkloadForRequestWithId_30240],
30241=>[
ActualWorkloadForReqeustWithId_30241,
EstimatedWorkloadForRequestWithId_30241]
}
SummaryOfActualWorkloadForTicketType,
SummaryOfEstimatedWorkloadForTicketType,
{20000=>[
ActualWorkloadForTicketWithId_2000,
EstimatedWorkloadForTicketWithId_2000],
}
]
}
使用如此糟糕的数据结构，数据的组装与分解逻辑确实不易读懂，而且极其冗长。集成测试 希望到目前为止，我已经让你相信这些代码确实很复杂。如果在开始重构之前，我要首先解开这一团乱麻，还得理解每一处代码，那我可能会发疯。为了保持头脑清醒，我决定去理解程序的逻辑，最好是使用由上至下的途径去进行理解。即，不去阅读代码，也不去推演程序逻辑，而最好是去使用该系统并对其进行调试，以便在总体上对其进行理解。 在编写单元测试时，我也使用相同的方法。传统观点是编写小的单元测试去验证每一块程序，如果每个测试都能正常通过，那么当你把所有的程序汇集到一块儿时，有很大的可能性，整个应用都能够正常运行。但该方法不适用此处。ResourceBreakdownService是一个"上帝类"。如果在只是大体认知的情况下，我一开始就想着去分割这个类，那么我会范下很多错误--遗留系统的每个犄角旮旯中都可能隐藏着许多秘密。 我编写了一个简单的单元测试，它反映了我对应用整体结构的理解：public void testResourceBreakdown(){
Resource resource=createResource();
List requests=createRequests();
assignRequestToResource(resource, requests);
List tickets=createTickets();
assignTicketToResource(resource, tickets);
Map result=new ResourceBreakdownService().search(resource);
verifyResult(result,resource,requests,tickets);
}
请注意verifyResult()方法。首先，我通过递归地打印返回结果的内容来了解它的结构。verifyResult()方法使用这一结构去验证返回值是否包含有正确的数据：private void verifyResult(Map result, Resource rsc, List<Request> requests, List<Ticket> tickets){
assertTrue(result.containsKey(rsc.getId()));
// in this simple test case, actual workload is empty
UtilizationBean emptyActualLoad=createDummyWorkload();
List resourceWorkLoad=result.get(rsc.getId());
UtilizationBean scheduleWorkload=calculateWorkload(rsc,requests);
assertEquals(emptyActualLoad,resourceWorkLoad.get(0));
assertEquals(scheduleWorkload,resourceWorkLoad.get(1));
Map requestDetailWorkload = (Map)resourceWorkLoad.get(3);
for (Request request : requests) {
assertTrue(requestDetailWorkload.containsKey(request.getId());
UtilizationBean scheduleWorkload0=calculateWorkload(rsc,request);
assertEquals(emptyActualLoad,requestDetailWorkload.get(request.getId()).get(0));
assertEquals(scheduleWorkload0,requestDetailWorkload.get(request.getId()).get(1));
}
// omit code to check tickets
} 绕开障碍 上述单元测试开起来简单，但实际上复杂。首先，ResourceBreakdownService.search()方法与运行时环境紧密绑定，它需要访问数据库，其它服务，鬼知道还需要些什么。像许多遗留系统一样，该资源管理系统没有任何单元测试基础架构。为了访问运行时服务，只能开启整个系统，这不仅昂贵，而也不方便。 启动系统服务器端的类是ServerMain。该类也像一块化石，通过它你能看到程序的演进过程。该系统是10年前写成的，那时还没有Spring和Hibernate，只有JBoss和Tomcat的早期版本。在那时，勇敢的先行者们不得不新手动手做很多事情，所以他们创建了自用的集群，缓存服务，以及连接池。后来，他们以某种方式加入了JBoss和Tomcat(但不幸地是，他们留下了自己创建的服务，所以程序遗有了两套事务管理机制和三种连接池。) 我决定将ServerMain复制到TestServerMain中，当调用TestServerMain.main()时却失败了：org.springframework.beans.factory.BeanInitializationException: Could not load properties; nested exception is
java.io.FileNotFoundException: class path resource [database.properties] cannot be opened because it does not exist
at
org.springframework.beans.factory.config.PropertyResourceConfigurer.postProcessBeanFactory(PropertyResourceConfigurer.java:78)
是的，很容易就能搞定！我弄来一个database.properties属性文件，并把它置入测试类路径中，再次启动测试程序。这次则抛出一个异常：java.io.FileNotFoundException: .\server.conf (The system cannot find the file specified)
at java.io.FileInputStream.open(Native Method)
at java.io.FileInputStream.<init>(FileInputStream.java:106)
at java.io.FileInputStream.<init>(FileInputStream.java:66)
at java.io.FileReader.<init>(FileReader.java:41)
at com.foo.bar.config.ServerConfigAgent.parseFile(ServerConfigAgent.java:1593)
at com.foo.bar.config.ServerConfigAgent.parseConfigFile(ServerConfigAgent.java:1720)
at com.foo.bar.config.ServerConfigAgent.parseConfigFile(ServerConfigAgent.java:1712)
at com.foo.bar.config.ServerConfigAgent.readServerConf(ServerConfigAgent.java:1581)
at com.foo.bar.ServerConfigFactory.initServerConfig(ServerConfigFactory.java:38)
at com.foo.bar.util.HibernateUtil.setupDatabaseProperties(HibernateUtil.java:207)
at com.foo.bar.util.HibernateUtil.doStart(HibernateUtil.java:135)
at com.foo.bar.util.HibernateUtil.<clinit>(HibernateUtil.java:125)
server.conf是存在于某处，但这种方法让我很不爽。只不过是写个单元测试，就马上暴露出了程序中的问题。HibernateUtil，顾名思义，应该只关注由database.properties文件提供的数据库信息。那为什么它还需要访问server.conf文件呢？该文件是用来配置服务器端参数的。有一线索可用来判断代码是否有异味：如果你感觉你是在读一部侦探小说，并不停在问"为什么"，那么这段代码通常就不好。我已经花了大量时间去阅读ServerConfigFactory，HibernateUtil和ServerConfigAgent中的代码，并且还尝试着去弄清楚HibernateUtil是如何使用database.properties，但在这个问题上，使我急切地想得到一个运行的服务器。除了这种方法以外，还有一种解决途径，这个武器就是AspectJ。void around():
call(public static void com.foo.bar.ServerConfigFactory.initServerConfig()){
System.out.println("bypassing com.foo.bar.ServerConfigFactory.initServerConfig");
}
对不懂AspectJ的伙计们来说，上面那段话的意思是：当运行到对ServerConfigFactory.initServerConfig()的调用时，AspectJ会打印一条信息，然后直接返回，而不再进入该方法本身。感觉这就像是骇客入侵，但这非常高效。遗留系统中满是谜团与问题。在任一给定的时刻，每个人都需要作出他具有战略性的一击。当时，依据用户满意度，我做的最值得称道的事情就是修复了资源管理系统中的缺陷，并改进了它的性能。矫正其它方面的问题并不是我需要关注的。但是，我会把它记在头里，之后我会转回来解决ServerMain中的乱麻。 在HibernateUtil从server.conf中读取必要信息这个调用点，我指示它从database.properties中读取这些信息：String around():call(public String com.foo.bar.config.ServerConfig.getJDBCUrl()){
// code omitted, reading from database.properties
}
String around():call(public String com.foo.bar.config.ServerConfig.getDBUser()){
// code omitted, reading from database.properties
}
你可能已经猜到剩下的工作了：解决运行时障碍，使它做起来更方便或更自然些。但如果已经现成的Mock对象，那就复用它。例如，TestServerMain.main()会在如下位置失败：- Factory name: java:comp/env/hibernate/SessionFactory
- JNDI InitialContext properties:{}
- Could not bind factory to JNDI
javax.naming.NoInitialContextException: Need to specify class name in environment or system property, or as an applet
parameter, or in an application resource file: java.naming.factory.initial
at javax.naming.spi.NamingManager.getInitialContext(NamingManager.java:645)
at javax.naming.InitialContext.getDefaultInitCtx(InitialContext.java:288)
这是因为没有启动JBoss的命名服务。我可以使用相同的AspectJ技术去解决这个问题，但InitialContext是一个很大的Java接口，它包含有许多方法，我不想去实现每个方法--那实在太繁冗了。随便查了一下，发现Spring已经提供一个Mock类SimpleNamingContext，所以就在测试用例使用这个类：SimpleNamingContextBuilder builder = new SimpleNamingContextBuilder();
builder.bind(“java:comp/env/hibernate/SessionFactory”,sessionFactory);
builder.activate();
经过几轮尝试后，我已能成功执行TestServerMain.main()方法了。与ServerMain相比，它要简单多了，它模拟了许多JBoss服务，并且它没有纠结于集群管理。
创建构建模块 TestServerMain连接到一个真实的数据库。遗留系统会将意想不到的逻辑隐藏于存储过程中，更糟糕地是，甚至会隐藏于触发器中。基于相同的整体结构考虑，我认为当时试图去理解隐藏于数据库中的神秘逻辑并去模拟这样的一个数据库是不明智的，所以我决定让测试用例去访问真实的数据库。 需要重复地执行测试用例，以确保我对产品做的每一点儿改变都能够通过测试。在每次执行时，测试程序将在数据库中创建资源与请求。不同于单元测试的传统套路，有时候你并不想在每个用例执行完成之后销毁由该用例创建的数据，以使运行环境变得干净。到目前为止，测试与重构实践是一种实情调查的探索方式--通过尝试着进行测试去学习遗留系统。你可能想检查由测试用例在数据库中创建的数据，或者为了确定每一项功能都能如期运行，你可能还想在运行时系统中使用这些数据。这就意味着，测试用例必须在数据库中创建独一无二的数据实例，以避免与其它用例冲突。应该有一些工具类去方便地创建这些数据实例。 此处有一个简单的创建资源的构建模块：public static ResourceBuilder newResource (String userName) {
ResourceBuilder rb = new ResourceBuilder();
rb.userName = userName + UnitTestThreadContext.getUniqueSuffix();
return rb; }
public ResourceBuilder assignRole(String roleName) {
this.roleName = roleName + UnitTestThreadContext.getUniqueSuffix();
return this;
}
public Resource create() {
ResourceDAO resourceDAO = new ResourceDAO(UnitTestThreadContext.getSession());
Resource rs;
if (StringUtils.isNotBlank(userName)) {
rs = resourceDAO.createResource(this.userName);
} else {
throw new RuntimeException("must have a user name to create a resource");
}
if (StringUtils.isNotBlank(roleName)) {
Role role = RoleBuilder.newRole(roleName).create();
rs.addRole(role);
}
return rs;
}
public static void delete(Resource rs, boolean cascadeToRole) {
Session session = UnitTestThreadContext.getSession();
ResourceDAO resourceDAO = new ResourceDAO(session);
resourceDAO.delete(rs);
if (cascadeToRole) {
RoleDAO roleDAO = new RoleDAO(session);
List roles = rs.getRoles();
for (Object role : roles) {
roleDAO.delete((Role)role);
}
}
}
ResourceBuilder是Builder与Factory模式的实现；你得悠着点儿去用它：ResourceBuilder.newResource(“Tom”).assignRole(“Developer”).create();
该类也包含一个战场清扫方法：delete()。在早期的重构实践中，我没有经常调用这个方法，因为我经常启动该系统，然后引入测试用例中的数据，并检验柱状图是否正确。此处非常有用的一个类就是UnitTestThreadContext，它按照线程的不同去存储Hibernate会话，并为每个你想创建的数据实体的名称的后面加上一个唯一的字符串，因此保证了实体的唯一性。public class UnitTestThreadContext {
private static ThreadLocal<Session> threadSession=new ThreadLocal<Session>();
private static ThreadLocal<String> threadUniqueId=new ThreadLocal<String>();
private final static SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH_mm_ss_S");
public static Session getSession(){>
Session session = threadSession.get();
if (session==null) {
throw new RuntimeException("Hibernate Session not set!");
}
return session;
}
public static void setSession(Session session) {
threadSession.set(session);
}
public static String getUniqueSuffix() {
String uniqueId = threadUniqueId.get();
if (uniqueId==null){
uniqueId = "-"+dateFormat.format(new Date());
threadUniqueId.set(uniqueId);
}
return uniqueId;
}
…
}
集中起来看 现在我可以启动这个最低限度的基础框架了，并能运行简单的测试用例：protected void setUp() throws Exception {
TestServerMain.run(); //setup a minimum running infrastructure
}
public void testResourceBreakdown(){
Resource resource=createResource(); //use ResourceBuilder to build unique resources
List requests=createRequests(); //use RequestBuilder to build unique requests
assignRequestToResource(resource, requests);
List tickets=createTickets(); //use TicketBuilder to build unique tickets
assignTicketToResource(resource, tickets);
Map result=new ResourceBreakdownService().search(resource);
verifyResult(result);
}
protected void tearDown() throws Exception {
// use TicketBuilder.delete() to delete tickets
// use RequestBuilder.delete() to delete requests
// use ResourceBuilder.delete() to delete resources
使用该方法，我继续编写更复杂的测试用例，重构产品程序与测试程序。 靠着这些测试用例的武装，我对"上帝类"ResourceBreakdownService一点点儿地进行分解。我不会透露更多的细节，这会使你难以理解；有许多书都可以教你如何安全地进行重构。
糟糕的Map_Of_Array_Of_Map_Of…数据结构现在被组装成ResourceLoadBucket，该类使用了Composite模式。它包含有特定级别的评估工效与实际工效，下一级别的工效可通过aggregate()方法聚合而成。最终程序要清洁的多，性能也更优。它甚至暴露了一些隐藏于原有代码复杂逻辑中的缺陷。当然，我也根据这种方法改进了单元测试。 贯穿于这次重构实践，我始终坚持的关键原则就是大局观思想。我挑选着工作方向并持续坚持着大局观原则，绕开当前并不重要的任务，然后构建一个最低限度的测试基础架构，我的团队则会继续使用这个基础架构去重构系统的其它部分。仍然在测试基础架构中保留了一些骇客侵入式的AspectJ程序，因为在业务上没有必要去清除它们。我不仅重构了一个非常复杂的功能领域，而且还对遗留系统进行了深入的认知。对待一个遗留应用，就如同对待一件易碎的瓷器，它不会给你带来任何安全感。深入其中，并对其进行重构，如此，遗留应用才可能在未来得以幸存。