From a3cded16db30449bdbff62c5f632d985dd9190fa Mon Sep 17 00:00:00 2001 From: Diego Pino Garcia Date: Thu, 2 Sep 2010 16:44:00 +0200 Subject: [PATCH] Fix bug detect if there are duplicated codes between the order to be saved and that order already saved in the DB It's necessary to check if any of the codes in an order that's going to be saved contain duplicated values comparing also against that very same order in the DB. Order codes must be unique. There's a case where it may not be any duplicated code in an order, neither in other orders, but a unique constraint violation still may happen. Imagine the following scenario: order |-- order1(1, 'code1') |-- order2(2, 'code2') The first time _order_ is persisted, everything goes OK. However if the value of codes are swapped, that is: order |-- order1(1, 'code2') |-- order2(2, 'code1') None code is repeated within the order, however as data is persisted one by one, when _order1_ is going to be saved into DB, the value of _order2.code_ is still 'code2', resulting into an unique code constraint violation. This is a limitation of the backend database and how unique fields work. The best thing to do is to check there's no other orderelement with the same code value in the DB (except for the one that's going to be saved). FEA: ItEr60S04ValidacionEProbasFuncionaisItEr59S04 --- .../orders/daos/IOrderElementDAO.java | 9 +++++ .../business/orders/daos/OrderElementDAO.java | 37 +++++++++++++++++++ .../navalplanner/web/orders/OrderModel.java | 26 +------------ 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/IOrderElementDAO.java b/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/IOrderElementDAO.java index 592d70014..4136a6364 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/IOrderElementDAO.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/IOrderElementDAO.java @@ -129,4 +129,13 @@ public interface IOrderElementDAO extends IIntegrationEntityDAO { */ Set getAllCodesExcluding(List orderElements); + /** + * Checks if there's another {@link OrderElement} in DB which code is the same as + * some of the ones in order (and its children) + * + * @param order + * @return + */ + OrderElement findRepeatedOrderCodeInDB(Order order); + } diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/OrderElementDAO.java b/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/OrderElementDAO.java index 73b057ae3..4b1e2a418 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/OrderElementDAO.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/orders/daos/OrderElementDAO.java @@ -23,6 +23,7 @@ package org.navalplanner.business.orders.daos; import java.math.BigDecimal; import java.math.RoundingMode; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; import java.util.Iterator; @@ -599,4 +600,40 @@ public class OrderElementDAO extends IntegrationEntityDAO return result; } + @Override + @Transactional(readOnly= true, propagation = Propagation.REQUIRES_NEW) + public OrderElement findRepeatedOrderCodeInDB(Order order) { + final Map orderElements = createMapByCode(getOrderAndAllChildren(order)); + final Map orderElementsInDB = createMapByCode(getAll()); + + for (String code : orderElements.keySet()) { + OrderElement orderElement = orderElements.get(code); + OrderElement orderElementInDB = orderElementsInDB.get(code); + + // There's an element in the DB with the same code and it's a + // different element + if (orderElementInDB != null + && !orderElementInDB.getId().equals(orderElement.getId())) { + return orderElement; + } + } + return null; + } + + private List getOrderAndAllChildren(Order order) { + List result = new ArrayList(); + result.add(order); + result.addAll(order.getAllChildren()); + return result; + } + + private Map createMapByCode(List orderElements) { + Map result = new HashMap(); + for (OrderElement each: orderElements) { + final String code = each.getCode(); + result.put(code, each); + } + return result; + } + } \ No newline at end of file diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderModel.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderModel.java index 5b866025f..5ae7b0cbd 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderModel.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/orders/OrderModel.java @@ -89,6 +89,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Scope; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import org.zkoss.ganttz.IPredicate; import org.zkoss.zul.Messagebox; @@ -550,7 +551,7 @@ public class OrderModel implements IOrderModel { repeatedOrder.getCode(), repeatedOrder.getName())); } - repeatedOrder = findRepeatedOrderCodeInAnotherOrder(order); + repeatedOrder = orderElementDAO.findRepeatedOrderCodeInDB(order); if (repeatedOrder != null) { throw new ValidationException(_( "Repeated Order code {0} in Order {1}", @@ -558,29 +559,6 @@ public class OrderModel implements IOrderModel { } } - private OrderElement findRepeatedOrderCodeInAnotherOrder(Order order) { - final List orderElements = getOrderAndAllChildren(order); - - // Codes in other orders but not in this one - Set otherCodes = orderElementDAO - .getAllCodesExcluding(orderElements); - - // Find codes in this order that are in codes of other order elements - for (OrderElement each : orderElements) { - if (otherCodes.contains(each.getCode())) { - return each; - } - } - return null; - } - - private List getOrderAndAllChildren(Order order) { - List result = new ArrayList(); - result.add(order); - result.addAll(order.getAllChildren()); - return result; - } - private void calculateAdvancePercentageIncludingChildren(OrderElement order) { calculateAdvancePercentage(order); for (OrderElement orderElement : order.getAllChildren()) {