Skip to content

Commit 6a66e7d

Browse files
authored
[jpa] Don't rollback failed transactions (#20645)
Signed-off-by: Cody Cutrer <cody@cutrer.us>
1 parent 9145b68 commit 6a66e7d

2 files changed

Lines changed: 121 additions & 3 deletions

File tree

bundles/org.openhab.persistence.jpa/src/main/java/org/openhab/persistence/jpa/internal/JpaPersistenceService.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,9 @@ public void store(Item item, @Nullable String alias) {
169169
// a duplicate timestamp. Just ignore
170170
logger.debug("Failed to persist item {} because of duplicate timestamp", name);
171171
} else {
172-
logger.error("Error while persisting item! Rolling back!", e);
172+
logger.error("Error while persisting item! Rolling back if needed.", e);
173173
}
174-
em.getTransaction().rollback();
174+
rollbackIfActive(em, "persisting item " + name);
175175
} finally {
176176
em.close();
177177
}
@@ -266,7 +266,7 @@ public Iterable<HistoricItem> query(FilterCriteria filter, @Nullable String alia
266266
return historicList;
267267
} catch (Exception e) {
268268
logger.error("Error while querying database!", e);
269-
em.getTransaction().rollback();
269+
rollbackIfActive(em, "querying historic items");
270270
} finally {
271271
em.close();
272272
}
@@ -325,6 +325,16 @@ protected boolean isEntityManagerFactoryOpen() {
325325
return emf != null && emf.isOpen();
326326
}
327327

328+
private void rollbackIfActive(EntityManager em, String operation) {
329+
try {
330+
if (em.getTransaction().isActive()) {
331+
em.getTransaction().rollback();
332+
}
333+
} catch (Exception rollbackException) {
334+
logger.warn("Rollback after {} failed", operation, rollbackException);
335+
}
336+
}
337+
328338
/**
329339
* Return the persistence unit as in persistence.xml file.
330340
*
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright (c) 2010-2026 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.persistence.jpa.internal;
14+
15+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
16+
import static org.mockito.Mockito.mock;
17+
import static org.mockito.Mockito.never;
18+
import static org.mockito.Mockito.verify;
19+
import static org.mockito.Mockito.when;
20+
21+
import java.util.Map;
22+
23+
import org.eclipse.jdt.annotation.NonNullByDefault;
24+
import org.eclipse.jdt.annotation.Nullable;
25+
import org.junit.jupiter.api.Test;
26+
import org.openhab.core.items.Item;
27+
import org.openhab.core.items.ItemRegistry;
28+
import org.openhab.core.library.types.DecimalType;
29+
import org.osgi.framework.BundleContext;
30+
31+
import jakarta.persistence.EntityManager;
32+
import jakarta.persistence.EntityManagerFactory;
33+
import jakarta.persistence.EntityTransaction;
34+
35+
/**
36+
* Tests for {@link JpaPersistenceService}.
37+
*
38+
* @author Cody Cutrer
39+
*/
40+
@NonNullByDefault
41+
class JpaPersistenceServiceTest {
42+
43+
@Test
44+
void storeDoesNotRollbackInactiveTransactionAfterCommitFailure() {
45+
EntityManagerFactory entityManagerFactory = mock(EntityManagerFactory.class);
46+
EntityManager entityManager = mock(EntityManager.class);
47+
EntityTransaction transaction = mock(EntityTransaction.class);
48+
Item item = mock(Item.class);
49+
50+
when(entityManagerFactory.createEntityManager()).thenReturn(entityManager);
51+
when(entityManager.getTransaction()).thenReturn(transaction);
52+
when(item.getName()).thenReturn("Kitchen_Persons");
53+
when(item.getState()).thenReturn(new DecimalType(1));
54+
when(transaction.isActive()).thenReturn(false);
55+
56+
RuntimeException commitFailure = new RuntimeException("commit failed");
57+
org.mockito.Mockito.doThrow(commitFailure).when(transaction).commit();
58+
59+
JpaPersistenceService service = new TestJpaPersistenceService(entityManagerFactory);
60+
61+
assertDoesNotThrow(() -> service.store(item));
62+
63+
verify(transaction, never()).rollback();
64+
verify(entityManager).close();
65+
}
66+
67+
@Test
68+
void storeRollsBackActiveTransactionAfterCommitFailure() {
69+
EntityManagerFactory entityManagerFactory = mock(EntityManagerFactory.class);
70+
EntityManager entityManager = mock(EntityManager.class);
71+
EntityTransaction transaction = mock(EntityTransaction.class);
72+
Item item = mock(Item.class);
73+
74+
when(entityManagerFactory.createEntityManager()).thenReturn(entityManager);
75+
when(entityManager.getTransaction()).thenReturn(transaction);
76+
when(item.getName()).thenReturn("Kitchen_Persons");
77+
when(item.getState()).thenReturn(new DecimalType(1));
78+
when(transaction.isActive()).thenReturn(true);
79+
80+
RuntimeException commitFailure = new RuntimeException("commit failed");
81+
org.mockito.Mockito.doThrow(commitFailure).when(transaction).commit();
82+
83+
JpaPersistenceService service = new TestJpaPersistenceService(entityManagerFactory);
84+
85+
assertDoesNotThrow(() -> service.store(item));
86+
87+
verify(transaction).rollback();
88+
verify(entityManager).close();
89+
}
90+
91+
private static final class TestJpaPersistenceService extends JpaPersistenceService {
92+
private final EntityManagerFactory entityManagerFactory;
93+
94+
TestJpaPersistenceService(EntityManagerFactory entityManagerFactory) {
95+
super(mock(BundleContext.class), validConfig(), mock(ItemRegistry.class));
96+
this.entityManagerFactory = entityManagerFactory;
97+
}
98+
99+
@Override
100+
protected EntityManagerFactory getEntityManagerFactory() {
101+
return entityManagerFactory;
102+
}
103+
}
104+
105+
private static Map<String, @Nullable Object> validConfig() {
106+
return Map.of("url", "jdbc:derby:memory:test;create=true", "driver", "org.apache.derby.jdbc.EmbeddedDriver");
107+
}
108+
}

0 commit comments

Comments
 (0)