Fix bug that true is always returned even in failure cases
Additionally, remove catch block handling TransactionException, which
doesn't have to be caught in user side any more.
Change-Id: I359b50dbe0e1074a2bc4c8850459cb4463669aa8
diff --git a/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java b/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
index 687576c..c9aaba5 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/newresource/impl/ConsistentResourceStore.java
@@ -29,7 +29,6 @@
import org.onosproject.store.service.Serializer;
import org.onosproject.store.service.StorageService;
import org.onosproject.store.service.TransactionContext;
-import org.onosproject.store.service.TransactionException;
import org.onosproject.store.service.TransactionalMap;
import org.onosproject.store.service.Versioned;
import org.slf4j.Logger;
@@ -100,29 +99,24 @@
TransactionContext tx = service.transactionContextBuilder().build();
tx.begin();
- try {
- TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
- tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+ TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+ tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
- Map<ResourcePath, List<ResourcePath>> resourceMap = resources.stream()
- .filter(x -> x.parent().isPresent())
- .collect(Collectors.groupingBy(x -> x.parent().get()));
+ Map<ResourcePath, List<ResourcePath>> resourceMap = resources.stream()
+ .filter(x -> x.parent().isPresent())
+ .collect(Collectors.groupingBy(x -> x.parent().get()));
- for (Map.Entry<ResourcePath, List<ResourcePath>> entry: resourceMap.entrySet()) {
- if (!isRegistered(childTxMap, entry.getKey())) {
- return abortTransaction(tx);
- }
-
- if (!appendValues(childTxMap, entry.getKey(), entry.getValue())) {
- return abortTransaction(tx);
- }
+ for (Map.Entry<ResourcePath, List<ResourcePath>> entry: resourceMap.entrySet()) {
+ if (!isRegistered(childTxMap, entry.getKey())) {
+ return abortTransaction(tx);
}
- return commitTransaction(tx);
- } catch (TransactionException e) {
- log.error("Exception thrown, abort the transaction", e);
- return abortTransaction(tx);
+ if (!appendValues(childTxMap, entry.getKey(), entry.getValue())) {
+ return abortTransaction(tx);
+ }
}
+
+ return tx.commit();
}
@Override
@@ -132,33 +126,28 @@
TransactionContext tx = service.transactionContextBuilder().build();
tx.begin();
- try {
- TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
- tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
- TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
- tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
+ TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+ tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+ TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+ tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
- Map<ResourcePath, List<ResourcePath>> resourceMap = resources.stream()
- .filter(x -> x.parent().isPresent())
- .collect(Collectors.groupingBy(x -> x.parent().get()));
+ Map<ResourcePath, List<ResourcePath>> resourceMap = resources.stream()
+ .filter(x -> x.parent().isPresent())
+ .collect(Collectors.groupingBy(x -> x.parent().get()));
- // even if one of the resources is allocated to a consumer,
- // all unregistrations are regarded as failure
- for (Map.Entry<ResourcePath, List<ResourcePath>> entry: resourceMap.entrySet()) {
- if (entry.getValue().stream().anyMatch(x -> consumerTxMap.get(x) != null)) {
- return abortTransaction(tx);
- }
-
- if (!removeValues(childTxMap, entry.getKey(), entry.getValue())) {
- return abortTransaction(tx);
- }
+ // even if one of the resources is allocated to a consumer,
+ // all unregistrations are regarded as failure
+ for (Map.Entry<ResourcePath, List<ResourcePath>> entry: resourceMap.entrySet()) {
+ if (entry.getValue().stream().anyMatch(x -> consumerTxMap.get(x) != null)) {
+ return abortTransaction(tx);
}
- return commitTransaction(tx);
- } catch (TransactionException e) {
- log.error("Exception thrown, abort the transaction", e);
- return abortTransaction(tx);
+ if (!removeValues(childTxMap, entry.getKey(), entry.getValue())) {
+ return abortTransaction(tx);
+ }
}
+
+ return tx.commit();
}
@Override
@@ -169,28 +158,23 @@
TransactionContext tx = service.transactionContextBuilder().build();
tx.begin();
- try {
- TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
- tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
- TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
- tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
+ TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+ tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+ TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+ tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
- for (ResourcePath resource: resources) {
- if (!isRegistered(childTxMap, resource)) {
- return abortTransaction(tx);
- }
-
- ResourceConsumer oldValue = consumerTxMap.put(resource, consumer);
- if (oldValue != null) {
- return abortTransaction(tx);
- }
+ for (ResourcePath resource: resources) {
+ if (!isRegistered(childTxMap, resource)) {
+ return abortTransaction(tx);
}
- return commitTransaction(tx);
- } catch (TransactionException e) {
- log.error("Exception thrown, abort the transaction", e);
- return abortTransaction(tx);
+ ResourceConsumer oldValue = consumerTxMap.put(resource, consumer);
+ if (oldValue != null) {
+ return abortTransaction(tx);
+ }
}
+
+ return tx.commit();
}
@Override
@@ -202,28 +186,23 @@
TransactionContext tx = service.transactionContextBuilder().build();
tx.begin();
- try {
- TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
- tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
- Iterator<ResourcePath> resourceIte = resources.iterator();
- Iterator<ResourceConsumer> consumerIte = consumers.iterator();
+ TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+ tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
+ Iterator<ResourcePath> resourceIte = resources.iterator();
+ Iterator<ResourceConsumer> consumerIte = consumers.iterator();
- while (resourceIte.hasNext() && consumerIte.hasNext()) {
- ResourcePath resource = resourceIte.next();
- ResourceConsumer consumer = consumerIte.next();
+ while (resourceIte.hasNext() && consumerIte.hasNext()) {
+ ResourcePath resource = resourceIte.next();
+ ResourceConsumer consumer = consumerIte.next();
- // if this single release fails (because the resource is allocated to another consumer,
- // the whole release fails
- if (!consumerTxMap.remove(resource, consumer)) {
- return abortTransaction(tx);
- }
+ // if this single release fails (because the resource is allocated to another consumer,
+ // the whole release fails
+ if (!consumerTxMap.remove(resource, consumer)) {
+ return abortTransaction(tx);
}
-
- return commitTransaction(tx);
- } catch (TransactionException e) {
- log.error("Exception thrown, abort the transaction", e);
- return abortTransaction(tx);
}
+
+ return tx.commit();
}
@Override
@@ -278,17 +257,6 @@
}
/**
- * Commit the transaction.
- *
- * @param tx transaction context
- * @return always true
- */
- private boolean commitTransaction(TransactionContext tx) {
- tx.commit();
- return true;
- }
-
- /**
* Appends the values to the existing values associated with the specified key.
* If the map already has all the given values, appending will not happen.
*
diff --git a/core/store/dist/src/main/java/org/onosproject/store/resource/impl/ConsistentLinkResourceStore.java b/core/store/dist/src/main/java/org/onosproject/store/resource/impl/ConsistentLinkResourceStore.java
index c332ada5..a38550e 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/resource/impl/ConsistentLinkResourceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/resource/impl/ConsistentLinkResourceStore.java
@@ -59,7 +59,6 @@
import org.onosproject.store.service.Serializer;
import org.onosproject.store.service.StorageService;
import org.onosproject.store.service.TransactionContext;
-import org.onosproject.store.service.TransactionException;
import org.onosproject.store.service.TransactionalMap;
import org.onosproject.store.service.Versioned;
@@ -294,7 +293,7 @@
intentAllocs.put(allocations.intentId(), allocations);
allocations.links().forEach(link -> allocateLinkResource(tx, link, allocations));
tx.commit();
- } catch (TransactionException | ResourceAllocationException e) {
+ } catch (ResourceAllocationException e) {
log.error("Exception thrown, rolling back", e);
tx.abort();
} catch (Exception e) {
@@ -407,12 +406,8 @@
after.remove(allocations);
linkAllocs.replace(linkId, before, after);
});
- tx.commit();
- success = true;
- } catch (TransactionException e) {
- log.debug("Transaction failed, retrying", e);
- tx.abort();
- } catch (Exception e) {
+ success = tx.commit();
+ } catch (Exception e) {
log.error("Exception thrown during releaseResource {}", allocations, e);
tx.abort();
throw e;