details: https://code.openbravo.com/erp/devel/pi/rev/ebd6611474d3 changeset: 35216:ebd6611474d3 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Tue Dec 11 15:27:05 2018 +0100 summary: related to issue 39672: improved javadoc
details: https://code.openbravo.com/erp/devel/pi/rev/7e4c318ad6a3 changeset: 35217:7e4c318ad6a3 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Tue Dec 11 15:37:30 2018 +0100 summary: related to issue 39672: added test cases diffstat: src-test/src/org/openbravo/test/AllAntTaskTests.java | 4 +- src-test/src/org/openbravo/test/dal/DalLockingTest.java | 223 ++++++++++++++++ src/org/openbravo/dal/service/OBDal.java | 31 +- 3 files changed, 248 insertions(+), 10 deletions(-) diffs (truncated from 313 to 300 lines): diff -r 4c4039a9c917 -r 7e4c318ad6a3 src-test/src/org/openbravo/test/AllAntTaskTests.java --- a/src-test/src/org/openbravo/test/AllAntTaskTests.java Tue Dec 11 12:26:44 2018 +0000 +++ b/src-test/src/org/openbravo/test/AllAntTaskTests.java Tue Dec 11 15:37:30 2018 +0100 @@ -50,6 +50,7 @@ import org.openbravo.test.dal.DalComplexQueryRequisitionTest; import org.openbravo.test.dal.DalComplexQueryTestOrderLine; import org.openbravo.test.dal.DalConnectionProviderTest; +import org.openbravo.test.dal.DalLockingTest; import org.openbravo.test.dal.DalPerformanceInventoryLineTest; import org.openbravo.test.dal.DalPerformanceProductTest; import org.openbravo.test.dal.DalPerformanceProxyTest; @@ -151,6 +152,7 @@ DalPerformanceProxyTest.class, // DalQueryTest.class, // DalTest.class, // + DalLockingTest.class, // CentralBrokerTest.class, // DalUtilTest.class, // IssuesTest.class, // @@ -317,7 +319,7 @@ // AD_Org Persist Information ADOrgPersistInfoTestSuite.class, - + // Automatic Invoice from Goods Shipment InvoiceFromShipmentTest.class diff -r 4c4039a9c917 -r 7e4c318ad6a3 src-test/src/org/openbravo/test/dal/DalLockingTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src-test/src/org/openbravo/test/dal/DalLockingTest.java Tue Dec 11 15:37:30 2018 +0100 @@ -0,0 +1,223 @@ +/* + ************************************************************************* + * The contents of this file are subject to the Openbravo Public License + * Version 1.1 (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) 2018 Openbravo SLU + * All Rights Reserved. + * Contributor(s): ______________________________________. + ************************************************************************ + */ + +package org.openbravo.test.dal; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.sameInstance; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.hibernate.LazyInitializationException; +import org.hibernate.LockMode; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.openbravo.base.exception.OBException; +import org.openbravo.base.provider.OBProvider; +import org.openbravo.dal.core.OBContext; +import org.openbravo.dal.service.OBDal; +import org.openbravo.model.ad.alert.AlertRecipient; +import org.openbravo.model.ad.alert.AlertRule; +import org.openbravo.test.base.OBBaseTest; + +/** + * Test cases covering + * {@link OBDal#getObjectLockForNoKeyUpdate(org.openbravo.base.structure.BaseOBObject)} + */ +public class DalLockingTest extends OBBaseTest { + private static final Logger log = LogManager.getLogger(); + private static String testingRuleId; + + private List<String> executionOrder; + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @BeforeClass + public static void createTestEnvironment() { + OBContext.setAdminMode(); + try { + AlertRule newAlertRule = OBProvider.getInstance().get(AlertRule.class); + newAlertRule.setName(DalLockingTest.class.getName() + " - Testing Alert Rule"); + OBDal.getInstance().save(newAlertRule); + + testingRuleId = newAlertRule.getId(); + OBDal.getInstance().commitAndClose(); + } finally { + OBContext.restorePreviousMode(); + } + } + + @Test + public void lockedObjectShouldBeANewInstance() { + AlertRule originalInstance = getTestingAlertRule(); + AlertRule lockedAlert = OBDal.getInstance().getObjectLockForNoKeyUpdate(originalInstance); + + assertThat(lockedAlert, not(sameInstance(originalInstance))); + } + + @Test + public void originalObjectShouldBeDetachedFromSession() { + AlertRule originalInstance = getTestingAlertRule(); + OBDal.getInstance().getObjectLockForNoKeyUpdate(originalInstance); + + // originalInstance is now detached from DAL session. If any of its proxies is tried to be + // initialized, it should fail. + thrown.expect(LazyInitializationException.class); + originalInstance.getADAlertRecipientList().size(); + } + + @Test + public void objectShouldBeLockedInDB() throws InterruptedException, ExecutionException { + List<Callable<Void>> threads = Arrays.asList( + // + getDalCallable(this::acquireLock, "T1", 0, 200), + getDalCallable(this::acquireLock, "T2", 100, 0)); + + executeAndGetResults(threads); + assertThat( + "Execution Order: Thread that acquired lock (T1) should finish before the one trying to acquire it afterwards (T2).", + executionOrder, is(Arrays.asList("T1", "T2"))); + } + + @Test + public void lockedObjectShouldAllowChildrenCreation() throws InterruptedException, + ExecutionException { + List<Callable<Void>> threads = Arrays.asList( // + getDalCallable(this::acquireLock, "T1", 0, 200), // + getDalCallable(() -> { + AlertRecipient recipient = OBProvider.getInstance().get(AlertRecipient.class); + recipient.setRole(OBContext.getOBContext().getRole()); + recipient.setAlertRule(getTestingAlertRule()); + OBDal.getInstance().save(recipient); + }, "T2", 100, 0)); + + executeAndGetResults(threads); + assertThat( + "Execution Order: Even T1 acquired a lock on parent, T2 can insert children without waiting for it", + executionOrder, is(Arrays.asList("T2", "T1"))); + } + + @Test + public void lockedInstanceGetsRefreshedFromDB() throws InterruptedException, ExecutionException { + StringBuilder originalName = new StringBuilder(); + StringBuilder lockedName = new StringBuilder(); + List<Callable<Void>> threads = Arrays.asList( // + getDalCallable(() -> { + AlertRule ar = getTestingAlertRule(); + originalName.append(ar.getName()); + + try { + TimeUnit.MILLISECONDS.sleep(200); + } catch (InterruptedException e) { + } + + lockedName.append(OBDal.getInstance().getObjectLockForNoKeyUpdate(ar).getName()); + }, "T1", 0, 0), // + getDalCallable(() -> { + getTestingAlertRule().setName("Modified"); + }, "T2", 100, 0)); + + executeAndGetResults(threads); + + assertThat("Execution Order", executionOrder, is(Arrays.asList("T2", "T1"))); + assertThat("Name changed", lockedName.toString(), not(originalName.toString())); + assertThat("Name changed", lockedName.toString(), is("Modified")); + } + + /** Note once HHH-13135 is fixed and Dal lock adapted to make use of it, this test should fail. */ + @Test + public void dalLockIsNotAHibernateLock() { + AlertRule lockedRule = acquireLock(); + LockMode lm = OBDal.getInstance().getSession().getCurrentLockMode(lockedRule); + assertTrue("Hibernate lock mode doesn't match DAL's one", lm.lessThan(LockMode.WRITE)); + } + + @AfterClass + public static void cleanUpTestEnvironment() { + OBContext.setAdminMode(); + try { + OBDal.getInstance().remove(getTestingAlertRule()); + OBDal.getInstance().commitAndClose(); + } finally { + OBContext.restorePreviousMode(); + } + } + + private Callable<Void> getDalCallable(Runnable r, String name, long waitBefore, long waitAfter) { + return () -> { + boolean errorOccurred = false; + try { + OBContext.setAdminMode(); + TimeUnit.MILLISECONDS.sleep(waitBefore); + r.run(); + TimeUnit.MILLISECONDS.sleep(waitAfter); + return null; + } catch (Exception e) { + log.error("Error occurred executing dal action", e); + OBDal.getInstance().rollbackAndClose(); + errorOccurred = true; + throw new OBException(e); + } finally { + if (!errorOccurred) { + OBDal.getInstance().commitAndClose(); + } + OBContext.restorePreviousMode(); + log.info("Completed thread {}", name); + executionOrder.add(name); + } + }; + } + + private <T extends Object> void executeAndGetResults(List<Callable<T>> threads) + throws InterruptedException, ExecutionException { + executionOrder = new ArrayList<>(); + ExecutorService executorService = Executors.newFixedThreadPool(2); + List<Future<T>> results = executorService.invokeAll(threads); + for (Future<T> result : results) { + // get to throw exception if it failed + result.get(); + } + } + + private AlertRule acquireLock() { + return OBDal.getInstance().getObjectLockForNoKeyUpdate( + OBDal.getInstance().getProxy(AlertRule.class, testingRuleId)); + } + + private static AlertRule getTestingAlertRule() { + return OBDal.getInstance().get(AlertRule.class, testingRuleId); + } +} diff -r 4c4039a9c917 -r 7e4c318ad6a3 src/org/openbravo/dal/service/OBDal.java --- a/src/org/openbravo/dal/service/OBDal.java Tue Dec 11 12:26:44 2018 +0000 +++ b/src/org/openbravo/dal/service/OBDal.java Tue Dec 11 15:37:30 2018 +0100 @@ -27,6 +27,8 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import javax.persistence.LockModeType; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.hibernate.ObjectNotFoundException; @@ -730,16 +732,24 @@ } /** - * Retrieves an object from the database getting a lock "for no key update" for the indicated - * object. Note that the object entering in the method is evicted and a new object is created. - * Before calling this method, is a must check if changes have been made to the object previously. - * In this case a flush must be done. + * Creates a WRITE lock in database for the DAL persistence instance {@code object} parameter and + * returns a new instance representing the same database object. + * <p> + * Note the original instance that is passed as parameter is evicted from Hibernate's 1st level. + * Therefore, any state not persisted before invoking this method will be ignored, after invoking + * this method the parameter instance shouldn't be used anymore using instead the returned one. + * <p> + * Whereas this is similar to JPA's {@link LockModeType#PESSIMISTIC_WRITE}, it decreases lock + * level in PostgreSQL implemented by Hibernate from {@code FOR UPDATE} to + * {@code FOR NO KEY UPDATE} allowing insertions of children records while a lock on its parent is + * acquired by a different transaction. This is a workaround until Hibernate issue HHH-13135 is + * fixed. Unlike locks acquired by Hibernate, the ones created by this method are only present in + * Database and cannot be detected by Hibernate (eg. {@link Session#getCurrentLockMode(Object)}. * * @param object - * the type to create the query for - * @return the new object getting a lock "for no key update" + * DAL instance to acquire a database lock for. + * @return A new DAL instance that represents the same database object than the parameter. */ - @SuppressWarnings("unchecked") public <T extends BaseOBObject> T getObjectLockForNoKeyUpdate(T object) { Entity entity = object.getEntity(); @@ -756,7 +766,10 @@ _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
