Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OptimisticLockException does not give any details if batch is active #1936

Merged
merged 5 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.eclipse.persistence.queries.ObjectLevelModifyQuery;
import org.eclipse.persistence.sessions.SessionProfiler;

import java.util.List;
import java.util.Vector;

/**
Expand All @@ -40,6 +41,7 @@ public class OptimisticLockException extends EclipseLinkException {
public final static int UNWRAPPING_OBJECT_DELETED_SINCE_LAST_READ = 5009;
public final static int OBJECT_CHANGED_SINCE_LAST_MERGE = 5010;
public final static int STATEMENT_NOT_EXECUTED_IN_BATCH = 5011;
public final static int STATEMENT_NOT_EXECUTED_IN_BATCH_WITH_PARAMETERS_LIST = 5012;

/**
* INTERNAL:
Expand Down Expand Up @@ -92,6 +94,14 @@ public static OptimisticLockException batchStatementExecutionFailure(){

}

public static OptimisticLockException batchStatementExecutionFailureWithParametersList(Object object, List<List> parameters, String queryString){
Object[] args = { object.getClass().getName(), parameters.toString(), queryString};

OptimisticLockException optimisticLockException = new OptimisticLockException(ExceptionMessageGenerator.buildMessage(OptimisticLockException.class, STATEMENT_NOT_EXECUTED_IN_BATCH_WITH_PARAMETERS_LIST, args));
optimisticLockException.setErrorCode(STATEMENT_NOT_EXECUTED_IN_BATCH_WITH_PARAMETERS_LIST);
return optimisticLockException;
}

public static OptimisticLockException mustHaveMappingWhenStoredInObject(Class<?> aClass) {
Object[] args = { aClass };

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2021 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
Expand Down Expand Up @@ -33,7 +33,8 @@ public final class OptimisticLockExceptionResource extends ListResourceBundle {
{ "5008", "Must map the version lock field to java.sql.Timestamp when using Timestamp Locking" },
{ "5009", "The object of class [{1}] with primary key [{0}] cannot be unwrapped because it was deleted since it was last read." },
{ "5010", "The object [{0}] cannot be merged because it has changed or been deleted since it was last read. {2}Class> {1}" },
{ "5011", "One or more objects cannot be updated because it has changed or been deleted since it was last read" }
{ "5011", "One or more objects cannot be updated because it has changed or been deleted since it was last read" },
{ "5012", "One or more objects of class {0} with parameters list {1} cannot be updated for SQL query {2} because it has changed or been deleted since it was last read" }
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ public class DatabasePlatform extends DatasourcePlatform {
/** JSON support for ResultSet data retrieval. */
private transient volatile DatabaseJsonPlatform jsonPlatform;

private int[] executeBatchRowCounts;
/**
* Creates an instance of default database platform.
*/
Expand Down Expand Up @@ -308,6 +309,7 @@ public DatabasePlatform() {
this.useJDBCStoredProcedureSyntax = null;
this.storedProcedureTerminationToken = ";";
this.jsonPlatform = null;
this.executeBatchRowCounts = new int[0];
}

/**
Expand Down Expand Up @@ -1476,6 +1478,14 @@ public boolean isLobCompatibleWithDistinct() {
return true;
}

public int[] getExecuteBatchRowCounts() {
return executeBatchRowCounts;
}

public void setExecuteBatchRowCounts(int[] rowCounts) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new methods should have some documentation

executeBatchRowCounts = rowCounts;
}

/**
* Builds a table of maximum numeric values keyed on java class. This is used for type testing but
* might also be useful to end users attempting to sanitize values.
Expand Down Expand Up @@ -2266,6 +2276,7 @@ public boolean supportsDeleteOnCascade() {
*/
public int executeBatch(Statement statement, boolean isStatementPrepared) throws java.sql.SQLException {
int[] rowCounts = statement.executeBatch();
setExecuteBatchRowCounts(rowCounts);
int rowCount = 0;
// Otherwise check if the row counts were returned.
for (int count : rowCounts) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import org.eclipse.persistence.internal.sessions.AbstractSession;
import org.eclipse.persistence.logging.SessionLog;
import org.eclipse.persistence.queries.ModifyQuery;
import org.eclipse.persistence.queries.DatabaseQuery;
import org.eclipse.persistence.queries.DeleteObjectQuery;
import org.eclipse.persistence.queries.UpdateObjectQuery;
import org.eclipse.persistence.sessions.SessionProfiler;

import java.io.StringWriter;
Expand Down Expand Up @@ -155,7 +158,14 @@ private void executeBatch(AbstractSession session) {
Object rowCount = this.databaseAccessor.basicExecuteCall(this.previousCall, null, session, false);
if (this.previousCall.hasOptimisticLock() && rowCount instanceof Integer) {
if ((Integer)rowCount != 1) {
throw OptimisticLockException.batchStatementExecutionFailure();
Object object = null;
DatabaseQuery query = this.previousCall.getQuery();
if (query.isUpdateObjectQuery()) {
object = ((UpdateObjectQuery) query).getObject();
} else if (query.isDeleteObjectQuery()) {
object = ((DeleteObjectQuery) query).getObject();
}
throw OptimisticLockException.batchStatementExecutionFailureWithParametersList(object, parameters, query.getSQLString());
}
}
} finally {
Expand Down Expand Up @@ -186,7 +196,21 @@ private void executeBatch(AbstractSession session) {
this.databaseAccessor.writeStatementsCount++;

if (this.previousCall.hasOptimisticLock() && (this.executionCount != this.statementCount)) {
throw OptimisticLockException.batchStatementExecutionFailure();
int[] rowCounts = this.databaseAccessor.getPlatform().getExecuteBatchRowCounts();
List<List> failureParametersList = new ArrayList();
for (int i = 0; i < rowCounts.length; i++) {
if (rowCounts[i] != 1 ) {
failureParametersList.add(parameters.get(i));
}
}
Object object = null;
DatabaseQuery query = this.previousCall.getQuery();
if (query.isUpdateObjectQuery()) {
object = ((UpdateObjectQuery) query).getObject();
} else if (query.isDeleteObjectQuery()) {
object = ((DeleteObjectQuery) query).getObject();
}
throw OptimisticLockException.batchStatementExecutionFailureWithParametersList(object, failureParametersList, query.getSQLString());
}
} finally {
// Reset the batched sql string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!--

Copyright (c) 2018, 2022 Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2018, 2023 Oracle and/or its affiliates. All rights reserved.

This program and the accompanying materials are made available under the
terms of the Eclipse Public License v. 2.0 which is available at
Expand Down Expand Up @@ -55,4 +55,21 @@
<property name="eclipselink.logging.parameters" value="${eclipselink.logging.parameters}"/>
</properties>
</persistence-unit>
<persistence-unit name="PU-BatchWriting" transaction-type="RESOURCE_LOCAL">
<provider>org.eclipse.persistence.jpa.PersistenceProvider</provider>
<exclude-unlisted-classes>false</exclude-unlisted-classes>
<properties>
<property name="eclipselink.sessions-xml" value="META-INF/sessions.xml"/>
<property name="eclipselink.session-name" value="BatchWritingSession"/>
<property name="eclipselink.exception-handler" value="org.eclipse.persistence.testing.tests.jpa.jpaadvancedproperties.CustomizedExceptionHandler"/>
<property name="eclipselink.session-event-listener" value="org.eclipse.persistence.testing.tests.jpa.jpaadvancedproperties.CustomizedSessionEventListener"/>
<property name="eclipselink.jdbc.batch-writing" value="JDBC"/>
<property name="eclipselink.logging.logger" value="JavaLogger"/>
<property name="eclipselink.logging.file" value="JPAAdvancedProperties.log"/>
<property name="eclipselink.profiler" value="NoProfiler"/>
<property name="eclipselink.logging.level" value="${eclipselink.logging.level}"/>
<property name="eclipselink.logging.level.sql" value="${eclipselink.logging.sql.level}"/>
<property name="eclipselink.logging.parameters" value="${eclipselink.logging.parameters}"/>
</properties>
</persistence-unit>
</persistence>
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="US-ASCII"?>
<!--

Copyright (c) 2018, 2022 Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2018, 2023 Oracle and/or its affiliates. All rights reserved.

This program and the accompanying materials are made available under the
terms of the Eclipse Public License v. 2.0 which is available at
Expand All @@ -19,4 +19,9 @@
<logging xsi:type="java-log"/>
<primary-project xsi:type="xml">META-INF/RelationshipProject.xml</primary-project>
</session>
<session xsi:type="server-session">
<name>BatchWritingSession</name>
<logging xsi:type="java-log"/>
<primary-project xsi:type="xml">META-INF/RelationshipProject.xml</primary-project>
</session>
</sessions>
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0,
* or the Eclipse Distribution License v. 1.0 which is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
*/

package org.eclipse.persistence.testing.tests.jpa.jpaadvancedproperties;

import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.Persistence;

import org.junit.Assert;
import org.eclipse.persistence.exceptions.OptimisticLockException;
import org.eclipse.persistence.testing.framework.jpa.junit.JUnitTestCase;
import org.eclipse.persistence.testing.models.jpa.jpaadvancedproperties.Customer;
import org.eclipse.persistence.testing.models.jpa.jpaadvancedproperties.ModelExamples;

public class OptimisticLockingBatchWritingTest extends JUnitTestCase {

public OptimisticLockingBatchWritingTest() {
}

public OptimisticLockingBatchWritingTest(String name) {
super(name);
setPuName(getPersistenceUnitName());
}

@Override
public String getPersistenceUnitName() {
return "PU-BatchWriting";
}

public void testSingleStatementBatch() {
EntityManager em = createEntityManager();
try {
beginTransaction(em);
Customer customer = ModelExamples.customerExample1();
em.persist(customer);
Integer customerId = customer.getCustomerId();
commitTransaction(em);
callTestImpl(customerId, new Integer[] { customerId });
} catch (RuntimeException e) {
if (isTransactionActive(em)){
rollbackTransaction(em);
}
throw e;
}finally{
closeEntityManager(em);
}
}

public void testMultipleStatementsBatch() {
EntityManager em = createEntityManager();
try {
beginTransaction(em);
Customer customer1 = ModelExamples.customerExample1();
em.persist(customer1);
Integer customerId1 = customer1.getCustomerId();
Customer customer2 = ModelExamples.customerExample2();
em.persist(customer2);
Integer customerId2 = customer2.getCustomerId();
commitTransaction(em);
callTestImpl(customerId2, new Integer[] { customerId1, customerId2 });
} catch (RuntimeException e) {
if (isTransactionActive(em)){
rollbackTransaction(em);
}
throw e;
}finally{
closeEntityManager(em);
}
}

protected void callTestImpl(Integer customerId, Integer[] customerIds) {

new Thread(() -> changeOrderAndWaitInTransaction(500, new Integer[] { customerId })).start();

try {
changeOrderAndWaitInTransaction(1000, customerIds);
Assert.fail("There should have been an optimistic lock");
} catch (Throwable e) {
Assert.assertTrue(e.getMessage(),
e.getMessage().contains("One or more objects of class"));
lukasj marked this conversation as resolved.
Show resolved Hide resolved
Assert.assertTrue(e.getMessage(),
e.getMessage().contains("org.eclipse.persistence.testing.models.jpa.jpaadvancedproperties.Customer"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using Customer.class.getName() would be more friendly to possible refactoring

}

}

protected void changeOrderAndWaitInTransaction(int sleepInMillis, Integer[] customerIds) {
EntityManager em = createEntityManager();
try {
beginTransaction(em);
for (Integer customerId : customerIds) {
Customer customer = em.find(org.eclipse.persistence.testing.models.jpa.jpaadvancedproperties.Customer.class, customerId);
customer.setName(customer.getName() + "-modified");
}
try {
Thread.sleep(sleepInMillis);
} catch (InterruptedException e) {
// ignore for this test
}

commitTransaction(em);
} catch (RuntimeException e) {
if (isTransactionActive(em)){
rollbackTransaction(em);
}
throw e;
}finally{
closeEntityManager(em);
}

}

}