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;