Skip to content

Commit edeaf98

Browse files
ibuildthecloudibuildthecloud
committed
Cleaner and more type safe Transaction API for checked exceptions
1 parent 323bbcc commit edeaf98

28 files changed

Lines changed: 240 additions & 254 deletions

‎engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@
167167
import com.cloud.utils.db.TransactionCallback;
168168
import com.cloud.utils.db.TransactionCallbackNoReturn;
169169
import com.cloud.utils.db.TransactionCallbackWithException;
170+
import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
170171
import com.cloud.utils.db.TransactionStatus;
171172
import com.cloud.utils.exception.CloudRuntimeException;
172173
import com.cloud.utils.exception.ExecutionException;
@@ -337,9 +338,9 @@ public void allocate(String vmInstanceName, final VirtualMachineTemplate templat
337338

338339
final VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vmFinal, template, serviceOffering, null, null);
339340

340-
Transaction.executeWithException(new TransactionCallbackWithException<Object>() {
341+
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<InsufficientCapacityException>() {
341342
@Override
342-
public Object doInTransaction(TransactionStatus status) throws InsufficientCapacityException {
343+
public void doInTransactionWithoutResult(TransactionStatus status) throws InsufficientCapacityException {
343344
if (s_logger.isDebugEnabled()) {
344345
s_logger.debug("Allocating nics for " + vmFinal);
345346
}
@@ -365,10 +366,8 @@ public Object doInTransaction(TransactionStatus status) throws InsufficientCapac
365366
for (Map.Entry<? extends DiskOffering, Long> offering : dataDiskOfferingsFinal.entrySet()) {
366367
volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + vmFinal.getId(), offering.getKey(), offering.getValue(), vmFinal, template, owner);
367368
}
368-
369-
return null;
370369
}
371-
}, InsufficientCapacityException.class);
370+
});
372371

373372
if (s_logger.isDebugEnabled()) {
374373
s_logger.debug("Allocation completed for VM: " + vmFinal);
@@ -566,7 +565,7 @@ protected Ternary<VMInstanceVO, ReservationContext, ItWorkVO> changeToStartState
566565
try {
567566
final ItWorkVO workFinal = work;
568567
Ternary<VMInstanceVO, ReservationContext, ItWorkVO> result =
569-
Transaction.executeWithException(new TransactionCallbackWithException<Ternary<VMInstanceVO, ReservationContext, ItWorkVO>>() {
568+
Transaction.execute(new TransactionCallbackWithException<Ternary<VMInstanceVO, ReservationContext, ItWorkVO>, NoTransitionException>() {
570569
@Override
571570
public Ternary<VMInstanceVO, ReservationContext, ItWorkVO> doInTransaction(TransactionStatus status) throws NoTransitionException {
572571
Journal journal = new Journal.LogJournal("Creating " + vm, s_logger);
@@ -582,7 +581,7 @@ public Ternary<VMInstanceVO, ReservationContext, ItWorkVO> doInTransaction(Trans
582581

583582
return new Ternary<VMInstanceVO, ReservationContext, ItWorkVO>(null, null, work);
584583
}
585-
}, NoTransitionException.class);
584+
});
586585

587586
work = result.third();
588587
if (result.first() != null)

‎engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@
169169
import com.cloud.utils.db.GlobalLock;
170170
import com.cloud.utils.db.TransactionCallback;
171171
import com.cloud.utils.db.TransactionCallbackNoReturn;
172-
import com.cloud.utils.db.TransactionCallbackWithException;
172+
import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
173173
import com.cloud.utils.db.TransactionStatus;
174174
import com.cloud.utils.db.JoinBuilder.JoinType;
175175
import com.cloud.utils.db.SearchBuilder;
@@ -687,9 +687,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
687687
public void allocate(final VirtualMachineProfile vm, final LinkedHashMap<? extends Network, ? extends NicProfile> networks) throws InsufficientCapacityException,
688688
ConcurrentOperationException {
689689

690-
Transaction.executeWithException(new TransactionCallbackWithException<Object>() {
690+
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<InsufficientCapacityException>() {
691691
@Override
692-
public Object doInTransaction(TransactionStatus status) throws InsufficientCapacityException {
692+
public void doInTransactionWithoutResult(TransactionStatus status) throws InsufficientCapacityException {
693693
int deviceId = 0;
694694

695695
boolean[] deviceIds = new boolean[networks.size()];
@@ -750,10 +750,8 @@ public Object doInTransaction(TransactionStatus status) throws InsufficientCapac
750750
if (nics.size() == 1) {
751751
nics.get(0).setDefaultNic(true);
752752
}
753-
754-
return null;
755753
}
756-
}, InsufficientCapacityException.class);
754+
});
757755
}
758756

759757
@DB
@@ -2470,15 +2468,13 @@ public boolean reallocate(final VirtualMachineProfile vm, DataCenterDeployment d
24702468
final LinkedHashMap<Network, NicProfile> profiles = new LinkedHashMap<Network, NicProfile>();
24712469
profiles.put(network, null);
24722470

2473-
Transaction.executeWithException(new TransactionCallbackWithException<Object>() {
2471+
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<InsufficientCapacityException>() {
24742472
@Override
2475-
public Object doInTransaction(TransactionStatus status) throws Exception {
2473+
public void doInTransactionWithoutResult(TransactionStatus status) throws InsufficientCapacityException {
24762474
cleanupNics(vm);
24772475
allocate(vm, profiles);
2478-
2479-
return null;
24802476
}
2481-
}, InsufficientCapacityException.class);
2477+
});
24822478
}
24832479
return true;
24842480
}

‎framework/db/src/com/cloud/utils/db/Transaction.java‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,13 @@
1818

1919
import java.util.concurrent.atomic.AtomicLong;
2020

21-
import com.cloud.utils.exception.ExceptionUtil;
22-
2321
public class Transaction {
2422
private final static AtomicLong counter = new AtomicLong(0);
2523
private final static TransactionStatus STATUS = new TransactionStatus() {
2624
};
2725

28-
public static <T> T execute(TransactionCallback<T> callback) {
26+
@SuppressWarnings("deprecation")
27+
public static <T,E extends Throwable> T execute(TransactionCallbackWithException<T,E> callback) throws E {
2928
String name = "tx-" + counter.incrementAndGet();
3029
short databaseId = TransactionLegacy.CLOUD_DB;
3130
TransactionLegacy currentTxn = TransactionLegacy.currentTxn(false);
@@ -43,24 +42,13 @@ public static <T> T execute(TransactionCallback<T> callback) {
4342
}
4443
}
4544

46-
public static <T,X extends Exception> T executeWithException(final TransactionCallbackWithException<T> callback, Class<X> exception) throws X {
47-
try {
48-
return execute(new TransactionCallback<T>() {
49-
@Override
50-
public T doInTransaction(TransactionStatus status) {
51-
try {
52-
return callback.doInTransaction(status);
53-
} catch (Exception e) {
54-
ExceptionUtil.rethrowRuntime(e);
55-
throw new TransactionWrappedExeception(e);
56-
}
57-
}
58-
});
59-
} catch (TransactionWrappedExeception e) {
60-
ExceptionUtil.rethrowRuntime(e.getWrapped());
61-
ExceptionUtil.rethrow(e.getWrapped(), exception);
62-
throw e;
63-
}
45+
public static <T> T execute(final TransactionCallback<T> callback) {
46+
return execute(new TransactionCallbackWithException<T, RuntimeException>() {
47+
@Override
48+
public T doInTransaction(TransactionStatus status) throws RuntimeException {
49+
return callback.doInTransaction(status);
50+
}
51+
});
6452
}
6553

6654
}

‎framework/db/src/com/cloud/utils/db/TransactionCallbackNoReturn.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
public abstract class TransactionCallbackNoReturn implements TransactionCallback<Object> {
44

55
@Override
6-
public Object doInTransaction(TransactionStatus status) {
6+
public final Object doInTransaction(TransactionStatus status) {
77
doInTransactionWithoutResult(status);
88
return null;
99
}

‎framework/db/src/com/cloud/utils/db/TransactionCallbackWithException.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package com.cloud.utils.db;
22

3-
public interface TransactionCallbackWithException<T> {
3+
public interface TransactionCallbackWithException<T,E extends Throwable> {
44

5-
public T doInTransaction(TransactionStatus status) throws Exception;
5+
public T doInTransaction(TransactionStatus status) throws E;
66

77
}

framework/db/src/com/cloud/utils/db/TransactionWrappedExeception.java renamed to framework/db/src/com/cloud/utils/db/TransactionCallbackWithExceptionNoReturn.java

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,15 @@
1818
*/
1919
package com.cloud.utils.db;
2020

21-
import com.cloud.utils.exception.CloudRuntimeException;
21+
public abstract class TransactionCallbackWithExceptionNoReturn<E extends Throwable> implements TransactionCallbackWithException<Boolean, E> {
2222

23-
public class TransactionWrappedExeception extends CloudRuntimeException {
24-
25-
private static final long serialVersionUID = -3254037624055143300L;
26-
27-
Exception e;
28-
29-
public TransactionWrappedExeception(Exception e) {
30-
this.e = e;
23+
@Override
24+
public final Boolean doInTransaction(TransactionStatus status) throws E {
25+
doInTransactionWithoutResult(status);
26+
return true;
3127
}
3228

33-
public Exception getWrapped() {
34-
return e;
35-
}
29+
public abstract void doInTransactionWithoutResult(TransactionStatus status) throws E;
30+
3631
}
32+

‎framework/db/test/com/cloud/utils/db/TestTransaction.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import java.io.FileNotFoundException;
2525
import java.sql.Connection;
26+
import java.util.concurrent.atomic.AtomicInteger;
2627

2728
import org.junit.After;
2829
import org.junit.Before;
@@ -88,14 +89,14 @@ public Object doInTransaction(TransactionStatus status) {
8889
@Test
8990
public void testRollbackWithException() throws Exception {
9091
try {
91-
Transaction.executeWithException(new TransactionCallbackWithException<Object>() {
92+
Transaction.execute(new TransactionCallbackWithException<Object,FileNotFoundException>() {
9293
@Override
9394
public Object doInTransaction(TransactionStatus status) throws FileNotFoundException {
9495
assertEquals(TransactionLegacy.CLOUD_DB, TransactionLegacy.currentTxn().getDatabaseId().shortValue());
9596

9697
throw new FileNotFoundException("Panic!");
9798
}
98-
}, FileNotFoundException.class);
99+
});
99100
fail();
100101
} catch (FileNotFoundException e) {
101102
assertEquals("Panic!", e.getMessage());
@@ -107,6 +108,23 @@ public Object doInTransaction(TransactionStatus status) throws FileNotFoundExcep
107108
verify(conn, times(1)).close();
108109
}
109110

111+
@Test
112+
public void testWithExceptionNoReturn() throws Exception {
113+
final AtomicInteger i = new AtomicInteger(0);
114+
assertTrue(Transaction.execute(new TransactionCallbackWithExceptionNoReturn<FileNotFoundException>() {
115+
@Override
116+
public void doInTransactionWithoutResult(TransactionStatus status) throws FileNotFoundException {
117+
i.incrementAndGet();
118+
}
119+
}));
120+
121+
assertEquals(1, i.get());
122+
verify(conn).setAutoCommit(false);
123+
verify(conn, times(1)).commit();
124+
verify(conn, times(0)).rollback();
125+
verify(conn, times(1)).close();
126+
}
127+
110128
@Test
111129
public void testOtherdatabaseRollback() throws Exception {
112130
after();

‎plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@
124124
import com.cloud.utils.db.EntityManager;
125125
import com.cloud.utils.db.Transaction;
126126
import com.cloud.utils.db.TransactionCallback;
127-
import com.cloud.utils.db.TransactionCallbackWithException;
127+
import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
128128
import com.cloud.utils.db.TransactionStatus;
129129
import com.cloud.utils.exception.CloudRuntimeException;
130130
import com.cloud.utils.exception.ExceptionUtil;
@@ -317,9 +317,9 @@ public boolean implement(final Network network, final NetworkOffering offering,
317317
}
318318

319319
try {
320-
Transaction.executeWithException(new TransactionCallbackWithException<Object>() {
320+
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<Exception>() {
321321
@Override
322-
public Object doInTransaction(TransactionStatus status) throws InsufficientAddressCapacityException, ResourceUnavailableException {
322+
public void doInTransactionWithoutResult(TransactionStatus status) throws InsufficientAddressCapacityException, ResourceUnavailableException {
323323

324324
// ensure that there is an ASA 1000v assigned to this network
325325
CiscoAsa1000vDevice assignedAsa = assignAsa1000vToNetwork(network);
@@ -419,10 +419,8 @@ public Object doInTransaction(TransactionStatus status) throws InsufficientAddre
419419
throw new CloudRuntimeException("Failed to associate Cisco ASA 1000v (" + assignedAsa.getManagementIp() +
420420
") with logical edge firewall in VNMC for network " + network.getName());
421421
}
422-
423-
return null;
424422
}
425-
}, Exception.class);
423+
});
426424
} catch (CloudRuntimeException e) {
427425
s_logger.error("CiscoVnmcElement failed", e);
428426
return false;

‎plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,9 @@ protected List<LoadBalancerVO> findExistingLoadBalancers(String lbName, Long ipI
587587

588588
@DB
589589
public PublicIp allocDirectIp(final Account account, final long guestNetworkId) throws InsufficientAddressCapacityException {
590-
return Transaction.executeWithException(new TransactionCallbackWithException<PublicIp>() {
590+
return Transaction.execute(new TransactionCallbackWithException<PublicIp,InsufficientAddressCapacityException>() {
591591
@Override
592-
public PublicIp doInTransaction(TransactionStatus status) throws Exception {
592+
public PublicIp doInTransaction(TransactionStatus status) throws InsufficientAddressCapacityException {
593593
Network frontEndNetwork = _networkModel.getNetwork(guestNetworkId);
594594

595595
PublicIp ip = _ipAddrMgr.assignPublicIpAddress(frontEndNetwork.getDataCenterId(), null, account, VlanType.DirectAttached, frontEndNetwork.getId(), null, true);
@@ -600,7 +600,7 @@ public PublicIp doInTransaction(TransactionStatus status) throws Exception {
600600

601601
return ip;
602602
}
603-
}, InsufficientAddressCapacityException.class);
603+
});
604604
}
605605

606606
public void releaseIp(long ipId, long userId, Account caller) {

‎server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,9 +422,9 @@ protected ExternalLoadBalancerDeviceVO allocateLoadBalancerForNetwork(final Netw
422422
final boolean dedicatedLB = offering.getDedicatedLB(); // does network offering supports a dedicated load balancer?
423423

424424
try {
425-
lbDevice = Transaction.executeWithException(new TransactionCallbackWithException<ExternalLoadBalancerDeviceVO>() {
425+
lbDevice = Transaction.execute(new TransactionCallbackWithException<ExternalLoadBalancerDeviceVO,InsufficientCapacityException>() {
426426
@Override
427-
public ExternalLoadBalancerDeviceVO doInTransaction(TransactionStatus status) throws Exception {
427+
public ExternalLoadBalancerDeviceVO doInTransaction(TransactionStatus status) throws InsufficientCapacityException {
428428
// FIXME: should the device allocation be done during network implement phase or do a
429429
// lazy allocation when first rule for the network is configured??
430430

@@ -442,7 +442,7 @@ public ExternalLoadBalancerDeviceVO doInTransaction(TransactionStatus status) th
442442
_externalLoadBalancerDeviceDao.update(lbDeviceId, lbDevice);
443443
return lbDevice;
444444
}
445-
}, InsufficientCapacityException.class);
445+
});
446446

447447
// allocated load balancer for the network, so skip retry
448448
tryLbProvisioning = false;

0 commit comments

Comments
 (0)