details: https://code.openbravo.com/erp/devel/pi/rev/81e679715102 changeset: 33421:81e679715102 user: Mark <markmm82 <at> gmail.com> date: Thu Feb 08 16:23:59 2018 -0500 summary: Fixes issue 37843: OrderEventHandler does useless and repeated queries when not needed.
OrderEventHandler class has been refactorized: - Removed unused vars. - Renamed newWarehouseId to newWarehouse as it store a Warehouse object instead of an ID - Renamed oldWarehouseId to oldWarehouse as it store a Warehouse object instead of an ID - Only executed query to get preferences after verify that values have changed. This way it avoids to execute unneeded queries. - Saved the orderLineCriteria.list() in a variable to avoid execute the same query more than once. - Merged the three used loops updating fields in only one that updates fields if needed. - Only loop is executed if at least one of the expected fields is changed and it is not activated the related preference. - Created new methods to make code more cleaned and reuse in similar cases. details: https://code.openbravo.com/erp/devel/pi/rev/3caf66446c5c changeset: 33422:3caf66446c5c user: David Miguelez <david.miguelez <at> openbravo.com> date: Fri Feb 09 12:43:18 2018 +0100 summary: Related to Issue 37843. Code Review changes. Refactor code to improve redability: * Encapsulated order parameters into a sub class that can be shared between methods * Split code of methods to reduce cognitive complexity * Removed comments since the code is self explanatory enough diffstat: src/org/openbravo/event/OrderEventHandler.java | 313 ++++++++++++++++++------ 1 files changed, 232 insertions(+), 81 deletions(-) diffs (truncated from 368 to 300 lines): diff -r 8a3e9044ea5e -r 3caf66446c5c src/org/openbravo/event/OrderEventHandler.java --- a/src/org/openbravo/event/OrderEventHandler.java Fri Feb 09 17:42:37 2018 -0500 +++ b/src/org/openbravo/event/OrderEventHandler.java Fri Feb 09 12:43:18 2018 +0100 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2013-2017 Openbravo SLU + * All portions are Copyright (C) 2013-2018 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -19,9 +19,11 @@ package org.openbravo.event; import java.util.Date; +import java.util.List; import javax.enterprise.event.Observes; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.hibernate.Query; @@ -29,6 +31,7 @@ import org.openbravo.base.model.Entity; import org.openbravo.base.model.ModelProvider; import org.openbravo.base.model.Property; +import org.openbravo.base.structure.BaseOBObject; import org.openbravo.client.kernel.event.EntityDeleteEvent; import org.openbravo.client.kernel.event.EntityPersistenceEventObserver; import org.openbravo.client.kernel.event.EntityUpdateEvent; @@ -45,6 +48,9 @@ public class OrderEventHandler extends EntityPersistenceEventObserver { + private static final String DO_NOT_SYNC_WAREHOUSE_PREFERENCE = "DoNotSyncWarehouse"; + private static final String DO_NOT_SYNC_DATE_DELIVERED_PREFERENCE = "DoNotSyncDateDelivered"; + private static final String DO_NOT_SYNC_DATE_ORDERED_PREFERENCE = "DoNotSyncDateOrdered"; private static Entity[] entities = { ModelProvider.getInstance().getEntity(Order.ENTITY_NAME) }; protected Logger logger = Logger.getLogger(this.getClass()); @@ -53,95 +59,24 @@ return entities; } - public void onUpdate(@Observes - EntityUpdateEvent event) { + public void onUpdate(@Observes EntityUpdateEvent event) { if (!isValidEvent(event)) { return; } - final Entity orderEntity = ModelProvider.getInstance().getEntity(Order.ENTITY_NAME); - final Property orderDateProperty = orderEntity.getProperty(Order.PROPERTY_ORDERDATE); - final Property scheduledDateProperty = orderEntity - .getProperty(Order.PROPERTY_SCHEDULEDDELIVERYDATE); - final Property warehouseProperty = orderEntity.getProperty(Order.PROPERTY_WAREHOUSE); - final Property businessPartnerProperty = orderEntity - .getProperty(Order.PROPERTY_BUSINESSPARTNER); - String syncDateOrdered = null, syncDateDelivered = null, syncWarehouse = null; - String orderId = (String) event.getTargetInstance().getId(); - Date newOrderDate = (Date) event.getCurrentState(orderDateProperty); - Date oldOrderDate = (Date) event.getPreviousState(orderDateProperty); - Date newScheduledDate = (Date) event.getCurrentState(scheduledDateProperty); - Date oldScheduledDate = (Date) event.getPreviousState(scheduledDateProperty); - Warehouse newWarehouseId = (Warehouse) event.getCurrentState(warehouseProperty); - Warehouse oldWarehouseId = (Warehouse) event.getPreviousState(warehouseProperty); - String newBPId = ((BusinessPartner) event.getCurrentState(businessPartnerProperty)).getId(); - String oldBPId = ((BusinessPartner) event.getPreviousState(businessPartnerProperty)).getId(); - // Check whether the preference is set to sync with order header - try { - syncDateOrdered = Preferences.getPreferenceValue("DoNotSyncDateOrdered", true, OBContext - .getOBContext().getCurrentClient(), OBContext.getOBContext().getCurrentOrganization(), - OBContext.getOBContext().getUser(), OBContext.getOBContext().getRole(), null); - } catch (PropertyException e) { - // if property not found, sync the ordered date - syncDateOrdered = Preferences.NO; - } - try { - syncDateDelivered = Preferences.getPreferenceValue("DoNotSyncDateDelivered", true, OBContext - .getOBContext().getCurrentClient(), OBContext.getOBContext().getCurrentOrganization(), - OBContext.getOBContext().getUser(), OBContext.getOBContext().getRole(), null); - } catch (PropertyException e) { - // if property not found, sync the delivered date - syncDateDelivered = Preferences.NO; - } - OBCriteria<OrderLine> orderLineCriteria = OBDal.getInstance().createCriteria(OrderLine.class); - orderLineCriteria.add(Restrictions.eq(OrderLine.PROPERTY_SALESORDER, - OBDal.getInstance().get(Order.class, orderId))); - if (orderLineCriteria.count() > 0) { - if (newOrderDate.compareTo(oldOrderDate) != 0 && !Preferences.YES.equals(syncDateOrdered)) { - for (OrderLine lines : orderLineCriteria.list()) { - lines.setOrderDate(newOrderDate); - } - } - if (newScheduledDate != null && oldScheduledDate != null - && newScheduledDate.compareTo(oldScheduledDate) != 0 - && !Preferences.YES.equals(syncDateDelivered)) { - for (OrderLine lines : orderLineCriteria.list()) { - lines.setScheduledDeliveryDate(newScheduledDate); - } - } - // check preferences is set to sync warehouse in header and lines - if (newWarehouseId != null && oldWarehouseId != null - && !newWarehouseId.getId().equals(oldWarehouseId.getId())) { - try { - syncWarehouse = Preferences.getPreferenceValue("DoNotSyncWarehouse", true, OBContext - .getOBContext().getCurrentClient(), - OBContext.getOBContext().getCurrentOrganization(), - OBContext.getOBContext().getUser(), OBContext.getOBContext().getRole(), null); - } catch (PropertyException e) { - // if property not found, sync the warehouse - syncWarehouse = Preferences.NO; - } - if (!Preferences.YES.equals(syncWarehouse)) { - for (OrderLine lines : orderLineCriteria.list()) { - lines.setWarehouse(newWarehouseId); - } - } - } + OrderParameters orderParameters = getOrderParameters(event); + + List<OrderLine> orderLines = getOrderLines(orderParameters); + if (CollectionUtils.isNotEmpty(orderLines)) { + updateOrderLinesValues(orderParameters, orderLines); } - // Remove discount information - if (!StringUtils.equals(newBPId, oldBPId)) { - StringBuilder deleteHql = new StringBuilder(); - deleteHql.append(" delete from " + OrderDiscount.ENTITY_NAME); - deleteHql.append(" where " + OrderDiscount.PROPERTY_SALESORDER + ".id = :orderId"); - Query deleteQry = OBDal.getInstance().getSession().createQuery(deleteHql.toString()); - deleteQry.setParameter("orderId", orderId); - deleteQry.executeUpdate(); + if (businessPartnerHasChanged(orderParameters)) { + removeDiscountInformationFromOrder(orderParameters); } } - public void onDelete(@Observes - EntityDeleteEvent event) { + public void onDelete(@Observes EntityDeleteEvent event) { if (!isValidEvent(event)) { return; } @@ -152,4 +87,220 @@ quotation.setDocumentStatus("UE"); } } + + private OrderParameters getOrderParameters(final EntityUpdateEvent event) { + OrderParameters orderParameters = new OrderParameters(); + final Entity orderEntity = ModelProvider.getInstance().getEntity(Order.ENTITY_NAME); + final Property orderDateProperty = orderEntity.getProperty(Order.PROPERTY_ORDERDATE); + final Property scheduledDateProperty = orderEntity + .getProperty(Order.PROPERTY_SCHEDULEDDELIVERYDATE); + final Property warehouseProperty = orderEntity.getProperty(Order.PROPERTY_WAREHOUSE); + final Property businessPartnerProperty = orderEntity + .getProperty(Order.PROPERTY_BUSINESSPARTNER); + + orderParameters.setOrderId((String) event.getTargetInstance().getId()); + orderParameters.setNewOrderDate((Date) event.getCurrentState(orderDateProperty)); + orderParameters.setOldOrderDate((Date) event.getPreviousState(orderDateProperty)); + orderParameters.setNewScheduledDate((Date) event.getCurrentState(scheduledDateProperty)); + orderParameters.setOldScheduledDate((Date) event.getPreviousState(scheduledDateProperty)); + orderParameters.setNewWarehouse((Warehouse) event.getCurrentState(warehouseProperty)); + orderParameters.setOldWarehouse((Warehouse) event.getPreviousState(warehouseProperty)); + orderParameters.setNewBPId(((BusinessPartner) event.getCurrentState(businessPartnerProperty)) + .getId()); + orderParameters.setOldBPId(((BusinessPartner) event.getPreviousState(businessPartnerProperty)) + .getId()); + + return orderParameters; + } + + private List<OrderLine> getOrderLines(final OrderParameters orderParameters) { + OBCriteria<OrderLine> orderLineCriteria = OBDal.getInstance().createCriteria(OrderLine.class); + orderLineCriteria.add(Restrictions.eq(OrderLine.PROPERTY_SALESORDER, + OBDal.getInstance().get(Order.class, orderParameters.getOrderId()))); + return orderLineCriteria.list(); + } + + private void updateOrderLinesValues(final OrderParameters orderParameters, + final List<OrderLine> orderLines) { + final boolean syncOrderDate = isOrderDateChangedAndPreferenceIsNotActivated(orderParameters); + final boolean syncDeliveredDate = isScheduledDateChangedAndPreferenceIsNotActivated(orderParameters); + final boolean syncWarehouse = isWarehouseChangedAndPreferenceIsNotActivated(orderParameters); + + if (syncOrderDate || syncDeliveredDate || syncWarehouse) { + for (OrderLine lines : orderLines) { + if (syncOrderDate) { + lines.setOrderDate(orderParameters.getNewOrderDate()); + } + if (syncDeliveredDate) { + lines.setScheduledDeliveryDate(orderParameters.getNewScheduledDate()); + } + if (syncWarehouse) { + lines.setWarehouse(orderParameters.getNewWarehouse()); + } + } + } + } + + private boolean isOrderDateChangedAndPreferenceIsNotActivated( + final OrderParameters orderParameters) { + boolean syncField = false; + if (areDatesDifferent(orderParameters.getNewOrderDate(), orderParameters.getOldOrderDate())) { + syncField = !StringUtils.equals(Preferences.YES, + getPreferenceValue(DO_NOT_SYNC_DATE_ORDERED_PREFERENCE)); + } + return syncField; + } + + private boolean isScheduledDateChangedAndPreferenceIsNotActivated( + final OrderParameters orderParameters) { + boolean syncField = false; + if (areDatesDifferent(orderParameters.getNewScheduledDate(), + orderParameters.getOldScheduledDate())) { + syncField = !StringUtils.equals(Preferences.YES, + getPreferenceValue(DO_NOT_SYNC_DATE_DELIVERED_PREFERENCE)); + } + return syncField; + } + + private boolean isWarehouseChangedAndPreferenceIsNotActivated( + final OrderParameters orderParameters) { + boolean syncField = false; + if (areObjectsDifferent(orderParameters.getNewWarehouse(), orderParameters.getOldWarehouse())) { + syncField = !StringUtils.equals(Preferences.YES, + getPreferenceValue(DO_NOT_SYNC_WAREHOUSE_PREFERENCE)); + } + return syncField; + } + + private boolean areDatesDifferent(final Date newDate, final Date oldDate) { + return newDate != null && oldDate != null && newDate.compareTo(oldDate) != 0; + } + + private boolean areObjectsDifferent(final BaseOBObject newObject, final BaseOBObject oldObject) { + return newObject != null && oldObject != null + && !StringUtils.equals(newObject.getId().toString(), oldObject.getId().toString()); + } + + private String getPreferenceValue(final String preferenceKey) { + String syncField; + try { + syncField = Preferences.getPreferenceValue(preferenceKey, true, OBContext.getOBContext() + .getCurrentClient(), OBContext.getOBContext().getCurrentOrganization(), OBContext + .getOBContext().getUser(), OBContext.getOBContext().getRole(), null); + } catch (PropertyException e) { + // if property not found, sync the field + syncField = Preferences.NO; + } + return syncField; + } + + private boolean businessPartnerHasChanged(final OrderParameters orderParameters) { + return !StringUtils.equals(orderParameters.getNewBPId(), orderParameters.getOldBPId()); + } + + private void removeDiscountInformationFromOrder(final OrderParameters orderParameters) { + StringBuilder deleteHql = new StringBuilder(); + deleteHql.append(" delete from " + OrderDiscount.ENTITY_NAME); + deleteHql.append(" where " + OrderDiscount.PROPERTY_SALESORDER + ".id = :orderId"); + Query deleteQry = OBDal.getInstance().getSession().createQuery(deleteHql.toString()); + deleteQry.setParameter("orderId", orderParameters.getOrderId()); + deleteQry.executeUpdate(); + } + + private class OrderParameters { + String orderId; + Date newOrderDate; + Date oldOrderDate; + Date newScheduledDate; + Date oldScheduledDate; + Warehouse newWarehouse; + Warehouse oldWarehouse; + String newBPId; + String oldBPId; + + public OrderParameters() { + this.orderId = null; + this.newOrderDate = null; + this.oldOrderDate = null; + this.newScheduledDate = null; + this.oldScheduledDate = null; + this.newWarehouse = null; + this.oldWarehouse = null; + this.newBPId = null; + this.oldBPId = null; + } + + public String getOrderId() { + return orderId; + } + + public void setOrderId(String orderId) { + this.orderId = orderId; ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
