Responding to review comments for: Setting order numbers in the api

needs to be safe in a clustered environment - TRUNK-4183
openmrs · Dec 23, 2013 · 683e3bf · 683e3bf
1 parent 9e62a20
commit 683e3bf
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 12 deletions.
diff --git a/api/src/main/java/org/openmrs/api/OrderService.java b/api/src/main/java/org/openmrs/api/OrderService.java
@@ -180,5 +180,5 @@ public <Ord extends Order> List<Ord> getOrders(Class<Ord> orderClassType, List<P
 	 * @return the order number seed
 	 */
 	@Authorized(PrivilegeConstants.ADD_ORDERS)
-	public Long getNextOrderNumberSeed();
+	public Long getNextOrderNumberSeedSequenceValue();
 }
diff --git a/api/src/main/java/org/openmrs/api/db/OrderDAO.java b/api/src/main/java/org/openmrs/api/db/OrderDAO.java
@@ -80,5 +80,5 @@ public <Ord extends Order> List<Ord> getOrders(Class<Ord> orderClassType, List<P
 	 * 
 	 * @return the order number seed
 	 */
-	public Long getNextOrderNumberSeed();
+	public Long getNextOrderNumberSeedSequenceValue();
 }
diff --git a/api/src/main/java/org/openmrs/api/db/hibernate/HibernateOrderDAO.java b/api/src/main/java/org/openmrs/api/db/hibernate/HibernateOrderDAO.java
@@ -48,8 +48,6 @@ public class HibernateOrderDAO implements OrderDAO {
 
 	protected static final Log log = LogFactory.getLog(HibernateOrderDAO.class);
 
-	private static final String ORDER_NUMBER_START_VALUE = "1";
-
 	/**
 	 * Hibernate session factory
 	 */
@@ -152,18 +150,18 @@ public Order getOrderByOrderNumber(String orderNumber) {
 	}
 
 	/**
-	 * @see org.openmrs.api.db.OrderDAO#getNextOrderNumberSeed()
+	 * @see org.openmrs.api.db.OrderDAO#getNextOrderNumberSeedSequenceValue()
 	 */
 	@Override
-	public Long getNextOrderNumberSeed() {
+	public Long getNextOrderNumberSeedSequenceValue() {
 		Criteria searchCriteria = sessionFactory.getCurrentSession().createCriteria(GlobalProperty.class);
 		searchCriteria.add(Restrictions.eq("property", OpenmrsConstants.GLOBAL_PROPERTY_NEXT_ORDER_NUMBER_SEED));
 		searchCriteria.setLockMode(LockMode.PESSIMISTIC_WRITE);
 
 		GlobalProperty globalProperty = (GlobalProperty) searchCriteria.uniqueResult();
 		if (globalProperty == null) {
-			globalProperty = new GlobalProperty(OpenmrsConstants.GLOBAL_PROPERTY_NEXT_ORDER_NUMBER_SEED,
-			        ORDER_NUMBER_START_VALUE, "The next order number available for assignment");
+			throw new APIException("Missing global property named: "
+			        + OpenmrsConstants.GLOBAL_PROPERTY_NEXT_ORDER_NUMBER_SEED);
 		}
 
 		String gpTextValue = globalProperty.getPropertyValue();

diff --git a/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java
@@ -175,7 +175,7 @@ public Order getOrderByUuid(String uuid) throws APIException {
 	 */
 	@Override
 	public synchronized String getNewOrderNumber() {
-		return ORDER_NUMBER_PREFIX + Context.getOrderService().getNextOrderNumberSeed();
+		return ORDER_NUMBER_PREFIX + Context.getOrderService().getNextOrderNumberSeedSequenceValue();
 	}
 
 	/**
@@ -205,10 +205,10 @@ public List<Order> getOrderHistoryByConcept(Patient patient, Concept concept) {
 	}
 
 	/**
-	 * @see org.openmrs.api.OrderService#getNextOrderNumberSeed()
+	 * @see org.openmrs.api.OrderService#getNextOrderNumberSeedSequenceValue()
 	 */
 	@Override
-	public Long getNextOrderNumberSeed() {
-		return dao.getNextOrderNumberSeed();
+	public Long getNextOrderNumberSeedSequenceValue() {
+		return dao.getNextOrderNumberSeedSequenceValue();
 	}
 }
diff --git a/api/src/test/java/org/openmrs/api/OrderServiceTest.java b/api/src/test/java/org/openmrs/api/OrderServiceTest.java
@@ -125,6 +125,9 @@ public void purgeOrder_shouldDeleteObsThatReference() throws Exception {
 	@Verifies(value = "should always return unique orderNumbers when called multiple times without saving orders", method = "getNewOrderNumber()")
 	public void getNewOrderNumber_shouldAlwaysReturnUniqueOrderNumbersWhenCalledMultipleTimesWithoutSavingOrders()
 	        throws Exception {
+
+		executeDataSet("org/openmrs/api/include/OrderServiceTest-globalProperties.xml");
+
 		int N = 50;
 		final Set<String> uniqueOrderNumbers = new HashSet<String>(50);
 		List<Thread> threads = new ArrayList<Thread>();

diff --git a/api/src/test/resources/org/openmrs/api/include/OrderServiceTest-globalProperties.xml b/api/src/test/resources/org/openmrs/api/include/OrderServiceTest-globalProperties.xml
@@ -0,0 +1,4 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<dataset>
+	<global_property property="order.nextOrderNumberSeed" property_value="1" uuid="1d55827e-26fe-102b-80cb-0017a47879y5"/>
+</dataset>