# HG changeset patch # User Víctor Martínez Romanos # Date 1490188606 -3600 # Wed Mar 22 14:16:46 2017 +0100 # Node ID 4a8d74e36f68a900588e245107e1760897ae5052 # Parent 2933c75ab71382bc872e4178614fda319bbb4f70 Fixed issue 35590: AD_IsOrgIncluded performance improvements Created AD_Org_Tree table to store the plain parent organization tree. The unique constraint defined in this table is the key to get the performance improvement. This table is populated when the organization is set as ready. In that moment we ensure its parent organization tree won’t change anymore in the future. The AD_IsOrgIncluded function has been totally rewritten to read the AD_Org_Tree table, so there is no need to recursively iterate over the AD_TreeNode table anymore. Important: after this changeset the AD_IsOrgIncluded function will only work for organizations set as ready! A new AD_IsOrgIncluded_TreeNode is created as a copy of the previous AD_IsOrgIncluded code to be used only by the processes working with non ready organizations. The Set as Ready process (AD_ORG_READY) has been updated to use this legacy function when required. A new module script PopulateADOrgTree.java has been developed to populate the new table for existing instances. Sampledata has been updated too. A set of automatic tests have been created to ensure the function returns the same values after the refactor, and that we get a real performance improvement. The improvement heavily depends on the organization tree structure (the more levels the better it will perform), parameters (the farther the passed orgs are the better it will perform) and query/table over the function is called from (the bigger table the better it will perform). In local testings we get improvements from 2,70x to 200x diff --git a/referencedata/sampledata/F_B_International_Group/AD_ORG_TREE.xml b/referencedata/sampledata/F_B_International_Group/AD_ORG_TREE.xml new file mode 100644 --- /dev/null +++ b/referencedata/sampledata/F_B_International_Group/AD_ORG_TREE.xml @@ -0,0 +1,219 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/referencedata/sampledata/QA_Testing/AD_ORG_TREE.xml b/referencedata/sampledata/QA_Testing/AD_ORG_TREE.xml new file mode 100644 --- /dev/null +++ b/referencedata/sampledata/QA_Testing/AD_ORG_TREE.xml @@ -0,0 +1,75 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src-db/database/model/functions/AD_ISORGINCLUDED.xml b/src-db/database/model/functions/AD_ISORGINCLUDED.xml --- a/src-db/database/model/functions/AD_ISORGINCLUDED.xml +++ b/src-db/database/model/functions/AD_ISORGINCLUDED.xml @@ -22,39 +22,42 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU -* All portions are Copyright (C) 2001-2006 Openbravo SLU +* All portions are Copyright (C) 2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************/ /************************************************************************* * Title: Return 'Org Level' if orgID is a leaf of the org tree * where parentOrgID is the root. Else return -1 +* +* This function is based on AD_Org_Tree table, which is populated only +* for the organizations set as ready. For the flows where the organization +* is not set as ready yet, please use AD_ISORGINCLUDED_TREENODE instead. ************************************************************************/ - v_Level NUMBER:=0; - v_treeID varchar2(32) ; - v_Parent VARCHAR(32) ; - v_Node VARCHAR(32) ; + V_RETURN AD_ORG_TREE.LEVELNO%TYPE:=-1 ; BEGIN - SELECT MAX(AD_TREE_ORG_ID) - INTO v_treeID - FROM AD_CLIENTINFO - WHERE AD_CLIENT_ID=p_clientID; - v_Parent:=p_OrgID; - WHILE v_Parent IS NOT NULL - LOOP - v_Level:=v_Level + 1; - SELECT MAX(NODE_ID), - MAX(PARENT_ID) - INTO v_Node, - v_Parent - FROM AD_TREENODE - WHERE AD_TREE_ID=v_treeID - AND NODE_ID=v_Parent; - IF v_Node=p_parentOrgID THEN - RETURN v_Level; - END IF; - END LOOP; - RETURN(-1) ; + + -- Special case for * org + IF (p_orgid='0' and p_parentorgid='0' and p_clientid is not null) THEN + SELECT 1 + INTO V_RETURN + FROM AD_CLIENT + WHERE AD_CLIENT_ID = p_clientid; + RETURN COALESCE(V_RETURN, -1); + END IF; + + -- Take advantage of AD_ORG_TREE.AD_ORG_TREE_ORGPARENTCLIENTLEV unique constraint + SELECT LEVELNO + INTO V_RETURN + FROM AD_ORG_TREE + WHERE AD_ORG_ID = p_orgid + AND AD_PARENT_ORG_ID = p_parentorgid + AND AD_CLIENT_ID = p_clientid; + + RETURN COALESCE(V_RETURN, -1); +EXCEPTION +WHEN OTHERS THEN + RETURN -1; END AD_ISORGINCLUDED ]]> diff --git a/src-db/database/model/functions/AD_ISORGINCLUDED_TREENODE.xml b/src-db/database/model/functions/AD_ISORGINCLUDED_TREENODE.xml new file mode 100644 --- /dev/null +++ b/src-db/database/model/functions/AD_ISORGINCLUDED_TREENODE.xml @@ -0,0 +1,65 @@ + + + + + + + + + + + + + + + diff --git a/src-db/database/model/functions/AD_ORG_CHK_CALENDAR.xml b/src-db/database/model/functions/AD_ORG_CHK_CALENDAR.xml --- a/src-db/database/model/functions/AD_ORG_CHK_CALENDAR.xml +++ b/src-db/database/model/functions/AD_ORG_CHK_CALENDAR.xml @@ -13,7 +13,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) 2008-2014 Openbravo SLU +* All portions are Copyright (C) 2008-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************/ @@ -48,7 +48,7 @@ SELECT COUNT(DISTINCT c_calendar_id), COALESCE(MAX(c_calendar_id), 'NO CALENDAR') INTO v_num_calendars, v_calendar_id FROM ad_org - WHERE AD_ISORGINCLUDED(ad_org.ad_org_id, cur_org_le.ad_org_id, ad_org.ad_client_id)<>-1 + WHERE AD_ISORGINCLUDED_TREENODE(ad_org.ad_org_id, cur_org_le.ad_org_id, ad_org.ad_client_id)<>-1 AND IsReady='Y'; IF (v_num_calendars>1) THEN diff --git a/src-db/database/model/functions/AD_ORG_READY.xml b/src-db/database/model/functions/AD_ORG_READY.xml --- a/src-db/database/model/functions/AD_ORG_READY.xml +++ b/src-db/database/model/functions/AD_ORG_READY.xml @@ -16,7 +16,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) 2008-2012 Openbravo SLU +* All portions are Copyright (C) 2008-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************/ @@ -64,7 +64,7 @@ IF (v_IsRecursive='Y') THEN UPDATE AD_ORG SET ISREADY='Y' - WHERE AD_ISORGINCLUDED(ad_org.ad_org_id, v_Record_ID, ad_org.ad_client_id)<>-1 + WHERE AD_ISORGINCLUDED_TREENODE(ad_org.ad_org_id, v_Record_ID, ad_org.ad_client_id)<>-1 AND IsReady='N'; ELSE UPDATE AD_ORG @@ -123,6 +123,51 @@ v_Result:=0; RAISE_APPLICATION_ERROR(-20000, '@20539@'); END IF; + + + -- Populate AD_Org_Tree + IF (v_IsRecursive='Y') THEN + FOR Cur_Org IN + (SELECT AD_Org_ID + FROM AD_Org + WHERE AD_ISORGINCLUDED_TREENODE(AD_Org_ID, v_Record_ID, AD_Client_ID)<>-1 + ) + LOOP + INSERT INTO ad_org_tree + ( + ad_org_tree_id, ad_client_id, + ad_org_id, ad_parent_org_id, + levelno, isactive, + created, createdby, updated, updatedby + ) + select get_uuid(), oc.ad_client_id, + oc.ad_org_id, op.ad_org_id, + ad_isorgincluded_treenode(oc.ad_org_id, op.ad_org_id, oc.ad_client_id), 'Y', + now(), '100', now(), '100' + from ad_org op + cross join ad_org oc + where (oc.ad_client_id = op.ad_client_id or op.ad_org_id = '0') + and oc.ad_org_id = Cur_Org.AD_Org_ID + and ad_isorgincluded_treenode(oc.ad_org_id, op.ad_org_id, oc.ad_client_id) > 0; + END LOOP; + ELSE + INSERT INTO ad_org_tree + ( + ad_org_tree_id, ad_client_id, + ad_org_id, ad_parent_org_id, + levelno, isactive, + created, createdby, updated, updatedby + ) + select get_uuid(), oc.ad_client_id, + oc.ad_org_id, op.ad_org_id, + ad_isorgincluded_treenode(oc.ad_org_id, op.ad_org_id, oc.ad_client_id), 'Y', + now(), '100', now(), '100' + from ad_org op + cross join ad_org oc + where (oc.ad_client_id = op.ad_client_id or op.ad_org_id = '0') + and oc.ad_org_id = v_Record_ID + and ad_isorgincluded_treenode(oc.ad_org_id, op.ad_org_id, oc.ad_client_id) > 0; + END IF; -- Create PeriodControl for the organization IF (v_IsRecursive='N') THEN diff --git a/src-db/database/model/tables/AD_ORG_TREE.xml b/src-db/database/model/tables/AD_ORG_TREE.xml new file mode 100644 --- /dev/null +++ b/src-db/database/model/tables/AD_ORG_TREE.xml @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0]]> +

+ diff --git a/src-db/database/sourcedata/AD_COLUMN.xml b/src-db/database/sourcedata/AD_COLUMN.xml --- a/src-db/database/sourcedata/AD_COLUMN.xml +++ b/src-db/database/sourcedata/AD_COLUMN.xml @@ -248784,6 +248784,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -254472,6 +254510,47 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -261588,6 +261667,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -319468,6 +319586,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -325344,6 +325500,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -326998,6 +327193,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -328645,6 +328879,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -354375,6 +354648,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -374089,6 +374401,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -381321,6 +381671,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src-db/database/sourcedata/AD_DATASET_TABLE.xml b/src-db/database/sourcedata/AD_DATASET_TABLE.xml --- a/src-db/database/sourcedata/AD_DATASET_TABLE.xml +++ b/src-db/database/sourcedata/AD_DATASET_TABLE.xml @@ -1215,6 +1215,20 @@ + + + + + + + + + + + + + + diff --git a/src-db/database/sourcedata/AD_ELEMENT.xml b/src-db/database/sourcedata/AD_ELEMENT.xml --- a/src-db/database/sourcedata/AD_ELEMENT.xml +++ b/src-db/database/sourcedata/AD_ELEMENT.xml @@ -26070,6 +26070,20 @@ + + + + + + + + + + + + + + @@ -27694,6 +27708,20 @@ + + + + + + + + + + + + + + @@ -31706,6 +31734,20 @@ + + + + + + + + + + + + + + diff --git a/src-db/database/sourcedata/AD_TABLE.xml b/src-db/database/sourcedata/AD_TABLE.xml --- a/src-db/database/sourcedata/AD_TABLE.xml +++ b/src-db/database/sourcedata/AD_TABLE.xml @@ -9607,6 +9607,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src-test/src/org/openbravo/test/IsOrgIncluded/ADOrgTreeTest.java b/src-test/src/org/openbravo/test/IsOrgIncluded/ADOrgTreeTest.java new file mode 100644 --- /dev/null +++ b/src-test/src/org/openbravo/test/IsOrgIncluded/ADOrgTreeTest.java @@ -0,0 +1,408 @@ +/* + ************************************************************************* + * The contents of this file are subject to the Openbravo Public License + * Version 1.0 (the "License"), being the Mozilla Public License + * Version 1.1 with a permitted attribution clause; you may not use this + * file except in compliance with the License. You may obtain a copy of + * the License at http://www.openbravo.com/legal/license.html + * Software distributed under the License is distributed on an "AS IS" + * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the + * License for the specific language governing rights and limitations + * under the License. + * The Original Code is Openbravo ERP. + * The Initial Developer of the Original Code is Openbravo SLU + * All portions are Copyright (C) 2017 Openbravo SLU + * All Rights Reserved. + * Contributor(s): ______________________________________. + ************************************************************************* + */ + +package org.openbravo.test.IsOrgIncluded; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; + +import org.apache.commons.lang.StringUtils; +import org.apache.log4j.Logger; +import org.hibernate.Query; +import org.hibernate.Session; +import org.junit.Test; +import org.openbravo.base.exception.OBException; +import org.openbravo.base.session.OBPropertiesProvider; +import org.openbravo.dal.core.OBContext; +import org.openbravo.dal.service.OBDal; +import org.openbravo.dal.service.OBQuery; +import org.openbravo.erpCommon.businessUtility.InitialOrgSetup; +import org.openbravo.model.ad.process.ProcessInstance; +import org.openbravo.model.common.enterprise.Organization; +import org.openbravo.model.common.enterprise.OrganizationTree; +import org.openbravo.service.db.CallProcess; +import org.openbravo.test.db.model.functions.Ad_isorgincludedTest; + +/** + * Tests related to AD_Org_Tree table + * + */ +public class ADOrgTreeTest extends Ad_isorgincludedTest { + private static final Logger log = Logger.getLogger(ADOrgTreeTest.class); + + private static final String ORGTYPE_ORGANIZATION = "0"; + private static final String CLIENT_0 = "0"; + + private final static String[] allOrganizations = { ORG_FB_NORTE, ORG_FB_SUR, ORG_FB_FBGROUP, + ORG_0, ORG_FB_SPAIN, ORG_FB_US, ORG_FB_WEST, ORG_FB_EAST, ORG_QA_MAIN, ORG_QA_SPAIN, null, + "XX", "YY" }; + private final static String[] allClients = { CLIENT_FB, CLIENT_QA, CLIENT_0, null, "XX", "XY" }; + private final static int ALL_COMBINATIONS = allOrganizations.length * allOrganizations.length + * allClients.length; + + @Override + public void testIsOrgIncluded() { + // Don't launch this test again, it will be launched by Ad_isorgincludedTest class itself + } + + /** + * Creates a new organization just below * and do not cascade + */ + @Test + public void testSingleNewOrganization() { + singleOrgTestCreator("N"); + } + + /** + * Creates a new organization just below * and do cascade + */ + @Test + public void testSingleNewOrganizationCascade() { + singleOrgTestCreator("Y"); + } + + private void singleOrgTestCreator(final String isCascade) { + try { + OBContext.setAdminMode(true); + final long number = System.currentTimeMillis(); + final String newOrgId = createOrganization("Test_" + number, ORGTYPE_ORGANIZATION, ORG_0); + + List orgTreeRecords = getOrganizationTreeRecords(newOrgId, null); + assertEquals("Records found at OrgTree at this point", 0, orgTreeRecords.size()); + + setAsReady(newOrgId, isCascade); + orgTreeRecords = getOrganizationTreeRecords(newOrgId, null); + assertEquals("Records found at OrgTree at this point", 2, orgTreeRecords.size()); + + orgTreeRecords = getOrganizationTreeRecords(newOrgId, newOrgId); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 1, orgTreeRecords.get(0).getLevelno().longValue()); + + orgTreeRecords = getOrganizationTreeRecords(newOrgId, ORG_0); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 2, orgTreeRecords.get(0).getLevelno().longValue()); + } finally { + OBContext.restorePreviousMode(); + } + } + + /** + * Test creation of two new organizations (one below the other) and set as ready individually: + *

*
|__firstOrg
___|__secondOrg

+ */ + @Test + public void testTwoNewOrgsIndividually() { + twoOrgTestBuilder(false); + } + + /** + * Test creation of two new organizations (one below the other) and set as ready in Cascade: + *

*
|__firstOrg
___|__secondOrg

+ */ + @Test + public void testTwoNewOrgsCascade() { + twoOrgTestBuilder(true); + } + + private void twoOrgTestBuilder(boolean isCascade) { + try { + OBContext.setAdminMode(true); + long number = System.currentTimeMillis(); + // First Org + final String firstOrgId = createOrganization("Test_" + number, ORGTYPE_ORGANIZATION, ORG_0); + + List orgTreeRecords = getOrganizationTreeRecords(firstOrgId, null); + assertEquals("Records found at OrgTree at this point", 0, orgTreeRecords.size()); + + String secondOrgId = null; + if (!isCascade) { + setAsReady(firstOrgId, "N"); + } else { // Second Org + number = System.currentTimeMillis(); + secondOrgId = createOrganization("Test_" + number, ORGTYPE_ORGANIZATION, firstOrgId); + + orgTreeRecords = getOrganizationTreeRecords(secondOrgId, null); + assertEquals("Records found at OrgTree at this point", 0, orgTreeRecords.size()); + + setAsReady(firstOrgId, "Y"); + } + + // First org checks + orgTreeRecords = getOrganizationTreeRecords(firstOrgId, null); + assertEquals("Records found at OrgTree at this point", 2, orgTreeRecords.size()); + + orgTreeRecords = getOrganizationTreeRecords(firstOrgId, firstOrgId); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 1, orgTreeRecords.get(0).getLevelno().longValue()); + + orgTreeRecords = getOrganizationTreeRecords(firstOrgId, ORG_0); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 2, orgTreeRecords.get(0).getLevelno().longValue()); + + // Second org checks + if (!isCascade) { + number = System.currentTimeMillis(); + secondOrgId = createOrganization("Test_" + number, ORGTYPE_ORGANIZATION, firstOrgId); + + orgTreeRecords = getOrganizationTreeRecords(secondOrgId, null); + assertEquals("Records found at OrgTree at this point", 0, orgTreeRecords.size()); + + setAsReady(secondOrgId, "N"); + } + + orgTreeRecords = getOrganizationTreeRecords(secondOrgId, null); + assertEquals("Records found at OrgTree at this point", 3, orgTreeRecords.size()); + + orgTreeRecords = getOrganizationTreeRecords(secondOrgId, secondOrgId); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 1, orgTreeRecords.get(0).getLevelno().longValue()); + + orgTreeRecords = getOrganizationTreeRecords(secondOrgId, firstOrgId); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 2, orgTreeRecords.get(0).getLevelno().longValue()); + + orgTreeRecords = getOrganizationTreeRecords(secondOrgId, ORG_0); + assertEquals("Records found at OrgTree at this point", 1, orgTreeRecords.size()); + assertEquals("Level found", 3, orgTreeRecords.get(0).getLevelno().longValue()); + } finally { + OBContext.restorePreviousMode(); + } + } + + /** + * Test we get expected records in AD_ORG_Tree table when the org tree has few levels + */ + @Test + public void testCreateExtendedOrgTreeFewLevels() { + checkExtendedOrgTreeBuilder(7, 2); + } + + /** + * Test we get expected records in AD_ORG_Tree table when the org tree has many levels + */ + @Test + public void testCreateExtendedOrgTreeManyLevels() { + checkExtendedOrgTreeBuilder(23, 6); + } + + private void checkExtendedOrgTreeBuilder(int totalOrgs, int maxLevel) { + try { + OBContext.setAdminMode(true); + String superParentOrgId = createOrganizationTree(totalOrgs, maxLevel); + setAsReady(superParentOrgId, "Y"); + List orgTreeRecords = getOrganizationTreeRecords(null, superParentOrgId); + assertEquals("Records found at OrgTree at this point", totalOrgs, orgTreeRecords.size()); + } finally { + OBContext.restorePreviousMode(); + } + } + + /** + * Test all the possible combinations for calling AD_IsOrgIncluded function and compares the + * result to the original AD_IsOrgIncluded_TreeNode + */ + @Test + public void testCallAllCombinations() { + int i = 0; + for (final String client : allClients) { + for (final String organizationChild : allOrganizations) { + for (final String organizationParent : allOrganizations) { + assertEquals("Failed combination with parameters: " + organizationChild + ", " + + organizationParent + ", " + client, + isOrgIncludedLegacy(organizationChild, organizationParent, client), + isOrgIncluded(organizationChild, organizationParent, client)); + i++; + } + } + } + assertEquals("Combinations tested", ALL_COMBINATIONS, i); + } + + /** + * Test performance of new function is better than legacy one when used as a filter criteria in + * the where clause. Verify we get the same results. + */ + @Test + public void testPerformanceI() { + try { + OBContext.setAdminMode(true); + final String hql = "select count(*) from FinancialMgmtAccountingFact fa where ad_isorgincluded(fa.organization.id, :parentOrgId, :clientId) <> -1"; + final Session session = OBDal.getInstance().getSession(); + final Query hqlQuery = session.createQuery(hql.toString()); + hqlQuery.setParameter("parentOrgId", ORG_FB_SPAIN); + hqlQuery.setParameter("clientId", CLIENT_FB); + long start = System.currentTimeMillis(); + final long hqlCount = (long) hqlQuery.uniqueResult(); + long hqlTime = System.currentTimeMillis() - start; + log.info("AD_IsOrgIncluded time: " + hqlTime); + + final String hqlLegacy = "select count(*) from FinancialMgmtAccountingFact fa where ad_isorgincluded_treenode(fa.organization.id, :parentOrgId, :clientId) <> -1"; + final Query hqlLegacyQuery = session.createQuery(hqlLegacy.toString()); + hqlLegacyQuery.setParameter("parentOrgId", ORG_FB_SPAIN); + hqlLegacyQuery.setParameter("clientId", CLIENT_FB); + start = System.currentTimeMillis(); + final long hqlLegacyCount = (long) hqlLegacyQuery.uniqueResult(); + long hqlLegacyTime = System.currentTimeMillis() - start; + log.info("AD_IsOrgIncluded_TreeNode time: " + hqlLegacyTime); + + assertEquals(hqlCount, hqlLegacyCount); + assertTrue("ad_isorgincluded_treenode ( " + hqlLegacyTime + + ") should be slower than ad_isorgincluded (" + hqlTime + ")", hqlLegacyTime > hqlTime); + // Set to 2 to be conservative (in local testing about 2,70x) + assertTrue("ad_isorgincluded_treenode ( " + hqlLegacyTime + + ") should be much slower than ad_isorgincluded (" + hqlTime + ")", + hqlLegacyTime > hqlTime * 2); + log.info(String.format("Performance gain: %.2fx", (double) hqlLegacyTime / hqlTime)); + } finally { + OBContext.restorePreviousMode(); + } + } + + /** + * Test performance of new function is equivalent to legacy one when using individually. 300 + * consecutive calls done in a batch to have better range. + * + * Verify we get the same results + */ + @Test + public void testPerformanceII() { + final int numberOfCalls = 300; + int[] result = new int[numberOfCalls]; + + int i = 0; + long init = System.currentTimeMillis(); + do { + for (final String client : allClients) { + for (final String organizationChild : allOrganizations) { + for (final String organizationParent : allOrganizations) { + if (i >= numberOfCalls) { + break; + } + result[i++] = isOrgIncluded(organizationChild, organizationParent, client); + } + } + } + } while (i < numberOfCalls); + long time = System.currentTimeMillis() - init; + + int[] resultLegacy = new int[numberOfCalls]; + i = 0; + init = System.currentTimeMillis(); + do { + for (final String client : allClients) { + for (final String organizationChild : allOrganizations) { + for (final String organizationParent : allOrganizations) { + if (i >= numberOfCalls) { + break; + } + resultLegacy[i++] = isOrgIncludedLegacy(organizationChild, organizationParent, client); + } + } + } + } while (i < numberOfCalls); + long legacyTime = System.currentTimeMillis() - init; + + assertTrue(Arrays.equals(result, resultLegacy)); + // Set to < 50 ms to be conservative (in local testing about -5ms) + assertTrue("ad_isorgincluded_treenode ( " + legacyTime + + ") should be more or less equal than ad_isorgincluded (" + time + ")", + time - legacyTime < 50); + log.info("Difference actual (" + time + ") - legacy (" + legacyTime + ") = " + + (time - legacyTime)); + } + + private List getOrganizationTreeRecords(final String newOrgId, + final String parentOrgId) { + final Map parameters = new HashMap(3); + String whereClause = "client.id = :clientId "; + parameters.put("clientId", OBContext.getOBContext().getCurrentClient().getId()); + if (!StringUtils.isBlank(newOrgId)) { + whereClause += " and organization.id = :orgId "; + parameters.put("orgId", newOrgId); + } + if (!StringUtils.isBlank(parentOrgId)) { + whereClause += " and parentOrganization.id = :parentOrgId "; + parameters.put("parentOrgId", parentOrgId); + } + OBQuery query = OBDal.getInstance().createQuery(OrganizationTree.class, + whereClause); + query.setNamedParameters(parameters); + return query.list(); + } + + private String createOrganizationTree(int totalOrgs, int maxLevel) { + long number = System.currentTimeMillis(); + // Org just below * (the first created) + final String superParent = createOrganization("SuperParent_" + number, ORGTYPE_ORGANIZATION, + ORG_0); + String strParentOrg = superParent; + int i = 1; + while (i < totalOrgs) { + for (int j = 0; i < totalOrgs && j < maxLevel; j++) { + strParentOrg = createOrganization("Test" + number + "_" + i + "_" + j, + ORGTYPE_ORGANIZATION, strParentOrg); + i++; + } + strParentOrg = superParent; + } + + return superParent; + } + + private String createOrganization(String newOrgName, String newOrgType, String strParentOrg) { + Properties properties = OBPropertiesProvider.getInstance().getOpenbravoProperties(); + String strSourcePath = properties.getProperty("source.path"); + InitialOrgSetup initialOrg = new InitialOrgSetup(OBContext.getOBContext().getCurrentClient()); + initialOrg.createOrganization(newOrgName, "", newOrgType, strParentOrg, "", "", "", false, + null, "", false, false, false, false, false, strSourcePath); + OBDal.getInstance().get(Organization.class, initialOrg.getOrgId()).setSummaryLevel(true); + return initialOrg.getOrgId(); + } + + private void setAsReady(final String orgId, final String isCascade) { + final Map parameters = new HashMap(1); + parameters.put("Cascade", isCascade); + final ProcessInstance pinstance = CallProcess.getInstance().call("AD_Org_Ready", orgId, + parameters); + if (pinstance.getResult() == 0L) { + throw new OBException(pinstance.getErrorMsg()); + } + + OBDal.getInstance().flush(); + } + + private int isOrgIncludedLegacy(String orgId, String parentOrgId, String clientId) { + return callOrgIncludedFunction(orgId, parentOrgId, clientId, "AD_ISORGINCLUDED_TREENODE"); + } + +} diff --git a/src-test/src/org/openbravo/test/db/model/functions/Ad_isorgincludedTest.java b/src-test/src/org/openbravo/test/db/model/functions/Ad_isorgincludedTest.java --- a/src-test/src/org/openbravo/test/db/model/functions/Ad_isorgincludedTest.java +++ b/src-test/src/org/openbravo/test/db/model/functions/Ad_isorgincludedTest.java @@ -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) 2015-2016 Openbravo SLU + * All portions are Copyright (C) 2015-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -35,62 +35,62 @@ /** * All Organization * */ - private static final String ORG_O = "0"; + protected static final String ORG_0 = "0"; /** * QA Testing Client */ - private static final String CLIENT_A = "4028E6C72959682B01295A070852010D"; + protected static final String CLIENT_QA = "4028E6C72959682B01295A070852010D"; /** * Main Organization */ - private static final String ORG_A = "43D590B4814049C6B85C6545E8264E37"; + protected static final String ORG_QA_MAIN = "43D590B4814049C6B85C6545E8264E37"; /** * Spain Organization */ - private static final String ORG_A1 = "357947E87C284935AD1D783CF6F099A1"; + protected static final String ORG_QA_SPAIN = "357947E87C284935AD1D783CF6F099A1"; /** * F&B International Group Client */ - private static final String CLIENT_B = "23C59575B9CF467C9620760EB255B389"; + protected static final String CLIENT_FB = "23C59575B9CF467C9620760EB255B389"; /** * F&B International Group Organization */ - private static final String ORG_B = "19404EAD144C49A0AF37D54377CF452D"; + protected static final String ORG_FB_FBGROUP = "19404EAD144C49A0AF37D54377CF452D"; /** * F&B US, Inc. */ - private static final String ORG_B1 = "2E60544D37534C0B89E765FE29BC0B43"; + protected static final String ORG_FB_US = "2E60544D37534C0B89E765FE29BC0B43"; /** * F&B US East Coast */ - private static final String ORG_B11 = "7BABA5FF80494CAFA54DEBD22EC46F01"; + protected static final String ORG_FB_EAST = "7BABA5FF80494CAFA54DEBD22EC46F01"; /** * F&B US West Coast */ - private static final String ORG_B12 = "BAE22373FEBE4CCCA24517E23F0C8A48"; + protected static final String ORG_FB_WEST = "BAE22373FEBE4CCCA24517E23F0C8A48"; /** * F&B España, S.A. */ - private static final String ORG_B2 = "B843C30461EA4501935CB1D125C9C25A"; + protected static final String ORG_FB_SPAIN = "B843C30461EA4501935CB1D125C9C25A"; /** * F&B España - Región Norte */ - private static final String ORG_B21 = "E443A31992CB4635AFCAEABE7183CE85"; + protected static final String ORG_FB_NORTE = "E443A31992CB4635AFCAEABE7183CE85"; /** * F&B España - Región Sur */ - private static final String ORG_B22 = "DC206C91AA6A4897B44DA897936E0EC3"; + protected static final String ORG_FB_SUR = "DC206C91AA6A4897B44DA897936E0EC3"; /** * Case I: Distinct Organization in the same branch with different levels. @@ -120,42 +120,47 @@ public void testIsOrgIncluded() { // Case I - assertEquals("Level 1 Organization", 1, isOrgIncluded(ORG_O, ORG_O, CLIENT_B)); - assertEquals("Level 2 Organization", 2, isOrgIncluded(ORG_B, ORG_O, CLIENT_B)); - assertEquals("Level 3 Organization", 3, isOrgIncluded(ORG_B1, ORG_O, CLIENT_B)); - assertEquals("Level 4 Organization", 4, isOrgIncluded(ORG_B12, ORG_O, CLIENT_B)); + assertEquals("Level 1 Organization", 1, isOrgIncluded(ORG_0, ORG_0, CLIENT_FB)); + assertEquals("Level 2 Organization", 2, isOrgIncluded(ORG_FB_FBGROUP, ORG_0, CLIENT_FB)); + assertEquals("Level 3 Organization", 3, isOrgIncluded(ORG_FB_US, ORG_0, CLIENT_FB)); + assertEquals("Level 4 Organization", 4, isOrgIncluded(ORG_FB_WEST, ORG_0, CLIENT_FB)); // Case II - assertTrue(isOrgIncluded(ORG_B12, ORG_B2, CLIENT_A) == -1); - assertTrue(isOrgIncluded(ORG_B11, ORG_B2, CLIENT_A) == -1); - assertTrue(isOrgIncluded(ORG_B21, ORG_B1, CLIENT_A) == -1); - assertTrue(isOrgIncluded(ORG_B22, ORG_B1, CLIENT_A) == isOrgIncluded(ORG_B11, ORG_B2, CLIENT_A)); + assertTrue(isOrgIncluded(ORG_FB_WEST, ORG_FB_SPAIN, CLIENT_QA) == -1); + assertTrue(isOrgIncluded(ORG_FB_EAST, ORG_FB_SPAIN, CLIENT_QA) == -1); + assertTrue(isOrgIncluded(ORG_FB_NORTE, ORG_FB_US, CLIENT_QA) == -1); + assertTrue(isOrgIncluded(ORG_FB_SUR, ORG_FB_US, CLIENT_QA) == isOrgIncluded(ORG_FB_EAST, + ORG_FB_SPAIN, CLIENT_QA)); // Case III - assertTrue(isOrgIncluded(ORG_A, ORG_A1, CLIENT_A) == -1); - assertTrue(isOrgIncluded(ORG_B1, ORG_B12, CLIENT_A) == -1); + assertTrue(isOrgIncluded(ORG_QA_MAIN, ORG_QA_SPAIN, CLIENT_QA) == -1); + assertTrue(isOrgIncluded(ORG_FB_US, ORG_FB_WEST, CLIENT_QA) == -1); // Case IV - assertTrue(isOrgIncluded(ORG_A1, ORG_A, CLIENT_B) == -1); - assertTrue(isOrgIncluded(ORG_B1, ORG_B, CLIENT_A) == -1); + assertTrue(isOrgIncluded(ORG_QA_SPAIN, ORG_QA_MAIN, CLIENT_FB) == -1); + assertTrue(isOrgIncluded(ORG_FB_US, ORG_FB_FBGROUP, CLIENT_QA) == -1); // Case V - assertTrue(isOrgIncluded(ORG_A, ORG_A, CLIENT_A) == 1); + assertTrue(isOrgIncluded(ORG_QA_MAIN, ORG_QA_MAIN, CLIENT_QA) == 1); // Case VI - assertTrue(isOrgIncluded("ABC", ORG_B, CLIENT_A) == -1); - + assertTrue(isOrgIncluded("ABC", ORG_FB_FBGROUP, CLIENT_QA) == -1); } - private int isOrgIncluded(String orgId, String parentOrgId, String clientId) { + protected int isOrgIncluded(String orgId, String parentOrgId, String clientId) { + return callOrgIncludedFunction(orgId, parentOrgId, clientId, "AD_ISORGINCLUDED"); + } + + protected int callOrgIncludedFunction(String orgId, String parentOrgId, String clientId, + String functionName) { int value = 0; try { final List