Refactor the code

cat-core/src/main/java/com/dianping/cat/abtest/spi/ABTestEntity.java

@@ -5,46 +5,45 @@ import java.util.Date;
 import com.dianping.cat.abtest.model.entity.Entity;
 
 public class ABTestEntity {
-
-	private Entity entity;
+	private Entity m_entity;
 
 	public ABTestEntity() {
-		this.entity = new Entity();
+		m_entity = new Entity();
 	}
 
 	public ABTestEntity(Entity entity) {
-		this.entity = entity;
+		m_entity = entity;
 	}
 
 	public String getGroupStrategy() {
-		return entity.getGroupStrategy().getName();
+		return m_entity.getGroupStrategy().getName();
 	}
 
 	public String getGroupStrategyConfiguration() {
-		return entity.getGroupStrategy().getConfiguration();
+		return m_entity.getGroupStrategy().getConfiguration();
 	}
 
 	public int getId() {
-		return entity.getId();
+		return m_entity.getId();
 	}
 
 	public String getName() {
-		return entity.getName();
+		return m_entity.getName();
 	}
 
 	public boolean isEligible(Date date) {
-		if (entity.getDisabled()) {
+		if (m_entity.getDisabled()) {
 			return false;
 		}
 
-		Date startDate = entity.getStartDate();
+		Date startDate = m_entity.getStartDate();
 		if (startDate != null) {
 			if (date.before(startDate)) {
 				return false;
 			}
 		}
 
-		Date endDate = entity.getEndDate();
+		Date endDate = m_entity.getEndDate();
 		if (endDate != null) {
 			if (date.after(endDate)) {
 				return false;
@@ -55,32 +54,55 @@ public class ABTestEntity {
 	}
 
 	public boolean isDisabled() {
-		return entity.isDisabled();
+		return m_entity.isDisabled();
 	}
 
 	public void setDisabled(boolean disabled) {
-		entity.setDisabled(disabled);
+		m_entity.setDisabled(disabled);
 	}
 
 	public void setGroupStrategy(String groupStrategy) {
-		entity.getGroupStrategy().setName(groupStrategy);
+		m_entity.getGroupStrategy().setName(groupStrategy);
 	}
 
 	public void setGroupStrategyConfiguration(String groupStrategyConfiguration) {
-		entity.getGroupStrategy().setConfiguration(groupStrategyConfiguration);
+		m_entity.getGroupStrategy().setConfiguration(groupStrategyConfiguration);
 	}
 
 	public void setId(int id) {
-		entity.setId(id);
+		m_entity.setId(id);
 	}
 
 	public void setName(String name) {
-		entity.setName(name);
+		m_entity.setName(name);
 	}
 
 	@Override
 	public String toString() {
-		return "ABTestEntity [entity=" + entity + "]";
+		return String.format("%s[id=%s, name=%s, groupStrategy=%s, configuation=%s]", getClass().getSimpleName(),
+		      getId(), getName(), getGroupStrategy(), getGroupStrategyConfiguration());
+	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = 1;
+		result = prime * result + getId();
+		return result;
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		ABTestEntity other = (ABTestEntity) obj;
+		if (getId() != other.getId())
+			return false;
+		return true;
 	}
 
 }

cat-core/src/main/java/com/dianping/cat/abtest/spi/internal/ABTestVisitor.java

-package com.dianping.cat.abtest.spi.internal;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import com.dianping.cat.abtest.model.entity.Entity;
-import com.dianping.cat.abtest.model.transform.BaseVisitor;
-import com.dianping.cat.abtest.spi.ABTestEntity;
-
-public class ABTestVisitor extends BaseVisitor {
-
-	private Map<Integer, ABTestEntity> m_entities = new HashMap<Integer, ABTestEntity>();
-
-	@Override
-	public void visitEntity(Entity entity) {
-		if (entity != null) {
-			ABTestEntity abTestEntity = new ABTestEntity(entity);
-			m_entities.put(abTestEntity.getId(), abTestEntity);
-		}
-	}
-
-	public Map<Integer, ABTestEntity> getABTestEntitys() {
-		return m_entities;
-	}
-
-}

cat-core/src/main/java/com/dianping/cat/abtest/spi/internal/DefaultABTestEntityManager.java

 package com.dianping.cat.abtest.spi.internal;
 
 import java.io.InputStream;
+import java.util.HashMap;
 import java.util.Map;
 
 import org.codehaus.plexus.personality.plexus.lifecycle.phase.Initializable;
@@ -8,12 +9,14 @@ import org.codehaus.plexus.personality.plexus.lifecycle.phase.InitializationExce
 
 import com.dianping.cat.abtest.ABTestId;
 import com.dianping.cat.abtest.model.entity.Abtest;
+import com.dianping.cat.abtest.model.entity.Entity;
+import com.dianping.cat.abtest.model.transform.BaseVisitor;
 import com.dianping.cat.abtest.model.transform.DefaultSaxParser;
 import com.dianping.cat.abtest.spi.ABTestEntity;
 import com.dianping.cat.abtest.spi.ABTestEntityManager;
 
 public class DefaultABTestEntityManager implements ABTestEntityManager, Initializable {
-	private Map<Integer, ABTestEntity> m_entities;
+	private Map<Integer, ABTestEntity> m_entities = new HashMap<Integer, ABTestEntity>();
 
 	@Override
 	public ABTestEntity getEntity(ABTestId id) {
@@ -22,6 +25,7 @@ public class DefaultABTestEntityManager implements ABTestEntityManager, Initiali
 		if (entity == null) {
 			entity = new ABTestEntity();
 			entity.setDisabled(true);
+
 			m_entities.put(id.getValue(), entity);
 		}
 
@@ -33,13 +37,26 @@ public class DefaultABTestEntityManager implements ABTestEntityManager, Initiali
 		try {
 			InputStream in = getClass().getResourceAsStream("abtest.xml");
 			Abtest abtest = DefaultSaxParser.parse(in);
-			ABTestVisitor visitor = new ABTestVisitor();
-			abtest.accept(visitor);
-			m_entities = visitor.getABTestEntitys();
+			ABTestVisitor visitor = new ABTestVisitor(m_entities);
 
+			abtest.accept(visitor);
 		} catch (Exception e) {
 			throw new InitializationException("Error when loading resource(abtest.xml)", e);
 		}
 	}
 
+	static class ABTestVisitor extends BaseVisitor {
+		private Map<Integer, ABTestEntity> m_entities;
+
+		public ABTestVisitor(Map<Integer, ABTestEntity> entities) {
+			m_entities = entities;
+		}
+
+		@Override
+		public void visitEntity(Entity entity) {
+			ABTestEntity abTestEntity = new ABTestEntity(entity);
+
+			m_entities.put(abTestEntity.getId(), abTestEntity);
+		}
+	}
 }

cat-core/src/main/java/com/dianping/cat/build/ABTestComponentConfigurator.java

@@ -8,8 +8,6 @@ import org.unidal.lookup.configuration.Component;
 
 import com.dianping.cat.abtest.spi.ABTestContextManager;
 import com.dianping.cat.abtest.spi.ABTestEntityManager;
-import com.dianping.cat.abtest.spi.ABTestGroupStrategy;
-import com.dianping.cat.abtest.spi.RoundRobinGroupStrategy;
 import com.dianping.cat.abtest.spi.internal.DefaultABTestContextManager;
 import com.dianping.cat.abtest.spi.internal.DefaultABTestEntityManager;
 
@@ -22,8 +20,6 @@ class ABTestComponentConfigurator extends AbstractResourceConfigurator {
 		      .req(ABTestEntityManager.class));
 
 		all.add(C(ABTestEntityManager.class, DefaultABTestEntityManager.class));
-		
-		all.add(C(ABTestGroupStrategy.class,"roundrobin",RoundRobinGroupStrategy.class));
 
 		return all;
 	}

cat-core/src/main/resources/META-INF/plexus/components.xml

@@ -315,10 +315,5 @@
 			<role>com.dianping.cat.abtest.spi.ABTestEntityManager</role>
 			<implementation>com.dianping.cat.abtest.spi.internal.DefaultABTestEntityManager</implementation>
 		</component>
-		<component>
-			<role>com.dianping.cat.abtest.spi.ABTestGroupStrategy</role>
-			<role-hint>roundrobin</role-hint>
-			<implementation>com.dianping.cat.abtest.spi.RoundRobinGroupStrategy</implementation>
-		</component>
 	</components>
 </plexus>

cat-core/src/test/java/com/dianping/cat/abtest/ABTestServer.java

-package com.dianping.cat.abtest;
-
-import java.io.File;
-
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.mortbay.jetty.Handler;
-import org.mortbay.jetty.Server;
-import org.mortbay.jetty.servlet.Context;
-import org.mortbay.jetty.servlet.ServletHolder;
-import org.unidal.test.jetty.JettyServer;
-
-import com.dianping.cat.Cat;
-import com.dianping.cat.servlet.CatFilter;
-
-@RunWith(JUnit4.class)
-public class ABTestServer extends JettyServer {
-
-	@Test
-	public void startWebApp() throws Exception {
-		File file = new File("/data/appdatas/cat/client.xml");
-		Cat.initialize(file);
-
-		Server server = new Server(getServerPort());
-		Context root = new Context(server, "/", Context.SESSIONS);
-		root.addFilter(CatFilter.class, "/*", Handler.REQUEST | Handler.FORWARD);
-		root.addServlet(new ServletHolder(SimpleRoundRobinWebPage.class), "/abtest");
-		server.start();
-		// open the page in the default browser
-		display("/abtest");
-		waitForAnyKey();
-	}
-
-	@Override
-	protected int getServerPort() {
-		return 8081;
-	}
-
-	@Override
-	protected String getContextPath() {
-		// TODO Auto-generated method stub
-		return null;
-	}
-
-}

cat-core/src/test/java/com/dianping/cat/abtest/demo/roundrobin/ABTestServer.java

+package com.dianping.cat.abtest.demo.roundrobin;
+
+import java.io.File;
+import java.util.HashMap;
+
+import org.codehaus.plexus.DefaultContainerConfiguration;
+import org.codehaus.plexus.PlexusContainer;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mortbay.jetty.Handler;
+import org.mortbay.jetty.servlet.ServletHolder;
+import org.mortbay.jetty.webapp.WebAppContext;
+import org.unidal.lookup.ContainerLoader;
+import org.unidal.test.jetty.JettyServer;
+
+import com.dianping.cat.Cat;
+import com.dianping.cat.servlet.CatFilter;
+
+@RunWith(JUnit4.class)
+public class ABTestServer extends JettyServer {
+	public static void main(String[] args) throws Exception {
+		ABTestServer server = new ABTestServer();
+		System.setProperty("devMode", "true");
+		server.setupContainer();
+		server.startServer();
+		server.startWebApp();
+		server.stopServer();
+	}
+
+	@Override
+	protected File getWarRoot() {
+		String path = getClass().getResource("/abtest/roundrobin").getPath();
+
+		return new File(path);
+	}
+
+	@Before
+	public void before() throws Exception {
+		setupContainer();
+
+		System.setProperty("devMode", "true");
+		super.startServer();
+	}
+
+	private void setupContainer() {
+		DefaultContainerConfiguration configuration = new DefaultContainerConfiguration();
+		String defaultConfigurationName = getClass().getName().replace('.', '/') + ".xml";
+
+		configuration.setName("test").setContext(new HashMap<Object, Object>());
+		configuration.setContainerConfiguration(defaultConfigurationName);
+		PlexusContainer container = ContainerLoader.getDefaultContainer(configuration);
+
+		Cat.initialize(container, null);
+	}
+
+	@Override
+	protected String getContextPath() {
+		return "/abtest";
+	}
+
+	@Override
+	protected int getServerPort() {
+		return 8081;
+	}
+
+	@Override
+	protected void postConfigure(WebAppContext context) {
+		 context.addFilter(CatFilter.class, "/*", Handler.REQUEST | Handler.FORWARD);
+		 context.addServlet(new ServletHolder(SimpleRoundRobinServlet.class), "/roundrobin/*");
+	}
+
+	@Test
+	public void startWebApp() throws Exception {
+		// open the page in the default browser
+		display("/abtest/roundrobin");
+		waitForAnyKey();
+	}
+}

cat-core/src/test/java/com/dianping/cat/abtest/demo/roundrobin/ABTestServerConfigurator.java

+package com.dianping.cat.abtest.demo.roundrobin;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.unidal.lookup.configuration.AbstractResourceConfigurator;
+import org.unidal.lookup.configuration.Component;
+
+import com.dianping.cat.abtest.spi.ABTestGroupStrategy;
+
+public class ABTestServerConfigurator extends AbstractResourceConfigurator {
+	@Override
+	public List<Component> defineComponents() {
+		List<Component> all = new ArrayList<Component>();
+
+		all.add(C(ABTestGroupStrategy.class, "roundrobin", RoundRobinGroupStrategy.class));
+
+		return all;
+	}
+
+	@Override
+	protected Class<?> getTestClass() {
+		return ABTestServer.class;
+	}
+
+	public static void main(String[] args) {
+		generatePlexusComponentsXmlFile(new ABTestServerConfigurator());
+	}
+}

cat-core/src/main/java/com/dianping/cat/abtest/spi/RoundRobinGroupStrategy.java → cat-core/src/test/java/com/dianping/cat/abtest/demo/roundrobin/RoundRobinGroupStrategy.java

-package com.dianping.cat.abtest.spi;
+package com.dianping.cat.abtest.demo.roundrobin;
 
 import java.util.concurrent.atomic.AtomicInteger;
 
-public class RoundRobinGroupStrategy implements ABTestGroupStrategy {
+import com.dianping.cat.abtest.spi.ABTestContext;
+import com.dianping.cat.abtest.spi.ABTestGroupStrategy;
 
+public class RoundRobinGroupStrategy implements ABTestGroupStrategy {
 	private AtomicInteger m_pvCounter = new AtomicInteger();
 
 	@Override
 	public void apply(ABTestContext ctx) {
-		if (m_pvCounter.incrementAndGet() % 2 == 0) {
+		int value = m_pvCounter.incrementAndGet();
+
+		if (value % 3 == 0) {
 			ctx.setGroupName("A");
-		} else {
+		} else if (value % 3 == 1) {
 			ctx.setGroupName("B");
+		} else {
+			// do nothing
 		}
 	}
 }

cat-core/src/test/java/com/dianping/cat/abtest/SimpleRoundRobinWebPage.java → cat-core/src/test/java/com/dianping/cat/abtest/demo/roundrobin/SimpleRoundRobinServlet.java

-package com.dianping.cat.abtest;
+package com.dianping.cat.abtest.demo.roundrobin;
 
 import java.io.IOException;
 
@@ -7,22 +7,23 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import com.dianping.cat.abtest.ABTest;
+import com.dianping.cat.abtest.ABTestManager;
 import com.dianping.cat.abtest.sample.SampleTest.MyABTestId;
 
-public class SimpleRoundRobinWebPage extends HttpServlet {
-	/**
-    * 
-    */
-   private static final long serialVersionUID = -6472784609174835547L;
+public class SimpleRoundRobinServlet extends HttpServlet {
+	private static final long serialVersionUID = -6472784609174835547L;
 
-	public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
-		ABTest abtest = ABTestManager.getTest(MyABTestId.CASE1);
-		if (abtest.isGroupA()) {
+	private ABTest m_abtest = ABTestManager.getTest(MyABTestId.CASE1);
+
+	@Override
+	public void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+		if (m_abtest.isGroupA()) {
 			String a = "This is group A";
 			byte[] aByte = a.getBytes();
 			response.getOutputStream().write(aByte);
 			// Cat.logMetric(...);
-		} else if (abtest.isGroupB()) {
+		} else if (m_abtest.isGroupB()) {
 			String b = "This is group B";
 			byte[] bByte = b.getBytes();
 			response.getOutputStream().write(bByte);

cat-core/src/test/java/com/dianping/cat/abtest/spi/internal/DefaultABTestEntityManagerTest.java

 package com.dianping.cat.abtest.spi.internal;
 
-import java.util.Calendar;
+import java.text.SimpleDateFormat;
 import java.util.Date;
 
 import junit.framework.Assert;
@@ -14,40 +14,48 @@ import com.dianping.cat.abtest.spi.ABTestEntityManager;
 
 public class DefaultABTestEntityManagerTest extends ComponentTestCase {
 
+	private static final String PATTERN = "yyyy-MM-dd hh:mm:ss";
+
 	@Test
 	public void testGetEntity() throws Exception {
-		ABTestEntityManager abTestEntityManager = lookup(ABTestEntityManager.class);
-		// 获取不到entity的情况
-		ABTestId id = new ABTestId() {
-			@Override
-			public int getValue() {
-				return -1;
-			}
-		};
-		ABTestEntity entity = abTestEntityManager.getEntity(id);
-		Assert.assertNotNull(entity);
-		Assert.assertEquals(entity.getId(), 0);
-		Assert.assertEquals(entity.isDisabled(), true);
-		// 获取具体的entity
-		id = new ABTestId() {
+		checkEntity(-1, null, null, null, null, false, true);
+		checkEntity(1001, "Mock1", "roundrobin", "This is the configuration", "2012-01-01 00:00:00", false, false);
+		checkEntity(1001, "Mock1", "roundrobin", "This is the configuration", "2013-04-10 18:00:00", true, false);
+	}
+
+	private void checkEntity(final int id, String expectedEntityName, String expectedGroupStrategy,
+	      String expectedGroupStrategyConfiguration, String expectedDateStr, boolean expectedEligible,
+	      boolean expectedDisabled) throws Exception {
+		ABTestEntityManager manager = lookup(ABTestEntityManager.class);
+		ABTestId testId = new ABTestId() {
 			@Override
 			public int getValue() {
-				return 1001;
+				return id;
 			}
 		};
-		entity = abTestEntityManager.getEntity(id);
+		ABTestEntity entity = manager.getEntity(testId);
+
 		Assert.assertNotNull(entity);
-		Assert.assertEquals(entity.getId(), id.getValue());
-		Assert.assertEquals(entity.getGroupStrategy(), "mock");
-		Assert.assertEquals(entity.getGroupStrategyConfiguration(), "This is the configuration");
-		Assert.assertEquals(entity.getName(), "Mock1");
-		Assert.assertEquals(entity.isDisabled(), false);
-		Calendar cal = Calendar.getInstance();
-		cal.set(2013, 0, 1, 0, 0, 0);
-		Date date = cal.getTime();
-		Assert.assertEquals(entity.isEligible(date), false);
-		cal.set(2013, 3, 10, 17, 1, 0);
-		date = cal.getTime();
-		Assert.assertEquals(entity.isEligible(date), true);
+
+		Assert.assertEquals(id < 0 ? 0 : id, entity.getId());
+
+		if (expectedGroupStrategy != null) {
+			Assert.assertEquals(expectedGroupStrategy, entity.getGroupStrategy());
+		}
+
+		if (expectedGroupStrategy != null) {
+			Assert.assertEquals(expectedGroupStrategyConfiguration, entity.getGroupStrategyConfiguration());
+		}
+
+		Assert.assertEquals(expectedEntityName, entity.getName());
+
+		if (expectedDateStr != null) {
+			SimpleDateFormat sdf = new SimpleDateFormat(PATTERN);
+			Date date = sdf.parse(expectedDateStr);
+			Assert.assertEquals(expectedEligible, entity.isEligible(date));
+		}
+
+		Assert.assertEquals(expectedDisabled, entity.isDisabled());
+
 	}
 }

cat-core/src/test/resources/abtest/roundrobin/WEB-INF/web.xml

+<?xml version="1.0" encoding="UTF-8"?>
+<web-app xmlns="http://java.sun.com/xml/ns/javaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_2_5.xsd"
+	version="2.5">
+</web-app>
+

cat-core/src/test/resources/com/dianping/cat/abtest/demo/roundrobin/ABTestServer.xml

+<plexus>
+	<components>
+		<component>
+			<role>com.dianping.cat.abtest.spi.ABTestGroupStrategy</role>
+			<role-hint>roundrobin</role-hint>
+			<implementation>com.dianping.cat.abtest.demo.roundrobin.RoundRobinGroupStrategy</implementation>
+		</component>
+	</components>
+</plexus>

cat-core/src/test/resources/com/dianping/cat/abtest/spi/internal/abtest.xml

 <abtest>
-	<entity id="1001" name="Mock1" start-date="2013-04-10 17:00:00" end-date="2013-04-30 18:00:00" disabled="false">
+	<entity id="1001" name="Mock1" start-date="2013-04-10 17:00:00" end-date="2013-12-30 18:00:00" disabled="false">
 		<description>This is a mock ABTest for test</description>
 		<group-strategy name="roundrobin">
 			<configuration><![CDATA[This is the configuration]]></configuration>