fix use of read-only transactions

 in openconfig-rp-statement

-  make sure transactions are used in a proper try-with-resources block
- use Optional.map() handling rather than the explicit isPresent()/get() combo

JIRA: BGPCEP-861
Signed-off-by: Gasparini, Claudio (cg479k) <claudio.gasparini@intl.att.com>
Change-Id: Icb46adc1e4d7be70a2151f26e25229baa64c87c9

diff --git a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/AbstractExtCommunityHandler.java b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/AbstractExtCommunityHandler.java
index fcd3f4d6bac9a4bd75c7c735bab21304233ddf14..938ed1f810ee8f0f6eed950466f821468a504083 100644 (file)
--- a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/AbstractExtCommunityHandler.java
+++ b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/AbstractExtCommunityHandler.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.FluentFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collections;
 import java.util.List;
@@ -26,6 +27,7 @@ import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.policy.rev15100
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.policy.rev151009.routing.policy.defined.sets.bgp.defined.sets.ExtCommunitySets;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.policy.rev151009.routing.policy.defined.sets.bgp.defined.sets.ext.community.sets.ExtCommunitySet;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.policy.rev151009.routing.policy.defined.sets.bgp.defined.sets.ext.community.sets.ExtCommunitySetKey;
+import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.bgp.policy.rev151009.routing.policy.defined.sets.bgp.defined.sets.ext.community.sets.ext.community.set.ExtCommunityMember;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.routing.policy.rev151009.routing.policy.top.RoutingPolicy;
 import org.opendaylight.yang.gen.v1.http.openconfig.net.yang.routing.policy.rev151009.routing.policy.top.routing.policy.DefinedSets;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev200120.path.attributes.attributes.ExtendedCommunities;
@@ -55,15 +57,22 @@ public class AbstractExtCommunityHandler {
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private List<ExtendedCommunities> loadCommunitySet(final String key)
             throws ExecutionException, InterruptedException {
-        final ReadTransaction tr = this.databroker.newReadOnlyTransaction();
-        final Optional<ExtCommunitySet> result =
-                tr.read(LogicalDatastoreType.CONFIGURATION, EXT_COMMUNITY_SETS_IID
-                        .child(ExtCommunitySet.class, new ExtCommunitySetKey(key))).get();
-        if (!result.isPresent()) {
-            return Collections.emptyList();
+        final FluentFuture<Optional<ExtCommunitySet>> future;
+        try (ReadTransaction tr = this.databroker.newReadOnlyTransaction()) {
+            future = tr.read(LogicalDatastoreType.CONFIGURATION,
+                    EXT_COMMUNITY_SETS_IID.child(ExtCommunitySet.class, new ExtCommunitySetKey(key)));
         }
-        return result.get().getExtCommunityMember()
-                .stream().map(ge -> new ExtendedCommunitiesBuilder().setExtendedCommunity(ge.getExtendedCommunity())
-                        .setTransitive(ge.getTransitive()).build()).collect(Collectors.toList());
+        final Optional<ExtCommunitySet> result = future.get();
+        return result.map(AbstractExtCommunityHandler::toExtendedCommunitiesList).orElse(Collections.emptyList());
+    }
+
+    private static List<ExtendedCommunities> toExtendedCommunitiesList(ExtCommunitySet extCommunitySets) {
+        return extCommunitySets.getExtCommunityMember().stream()
+                       .map(AbstractExtCommunityHandler::toExtendedCommunities).collect(Collectors.toList());
+    }
+
+    private static ExtendedCommunities toExtendedCommunities(final ExtCommunityMember ge) {
+        return new ExtendedCommunitiesBuilder().setExtendedCommunity(ge.getExtendedCommunity())
+                .setTransitive(ge.getTransitive()).build();
     }
 }

diff --git a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchAsPathSetHandler.java b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchAsPathSetHandler.java
index 49230cd92d8b1fb34f4feacdd8fd12f3027e7dde..9818098f674375c98f603847fd66b85885735d6c 100644 (file)
--- a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchAsPathSetHandler.java
+++ b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchAsPathSetHandler.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.FluentFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.Collections;
@@ -70,10 +71,12 @@ public final class MatchAsPathSetHandler implements BgpConditionsPolicy<MatchAsP
     @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private AsPathSet loadSets(final String key) throws ExecutionException, InterruptedException {
-        final ReadTransaction tr = this.dataBroker.newReadOnlyTransaction();
-        final Optional<AsPathSet> result = tr.read(LogicalDatastoreType.CONFIGURATION,
-                AS_PATHS_SETS_IID.child(AsPathSet.class, new AsPathSetKey(key))).get();
-        return result.orElse(null);
+        final FluentFuture<Optional<AsPathSet>> future;
+        try (ReadTransaction tr = this.dataBroker.newReadOnlyTransaction()) {
+            future = tr.read(LogicalDatastoreType.CONFIGURATION,
+                    AS_PATHS_SETS_IID.child(AsPathSet.class, new AsPathSetKey(key)));
+        }
+        return future.get().orElse(null);
     }
 
     @Override
@@ -147,4 +150,4 @@ public final class MatchAsPathSetHandler implements BgpConditionsPolicy<MatchAsP
         //(matchSetOptions.equals(MatchSetOptionsType.INVERT))
         return noneInCommon;
     }
-}
\ No newline at end of file
+}

diff --git a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchBgpNeighborSetHandler.java b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchBgpNeighborSetHandler.java
index 73c45e4aa1904cd852fd761893458d8ef9008fb1..e8c16bbd8f6bc79e30d13b06e95a4793090465b9 100644 (file)
--- a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchBgpNeighborSetHandler.java
+++ b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchBgpNeighborSetHandler.java
@@ -12,8 +12,8 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.FluentFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
@@ -52,7 +52,7 @@ public final class MatchBgpNeighborSetHandler
             .child(NeighborSets.class);
     private final DataBroker dataBroker;
     private final LoadingCache<String, List<PeerId>> peerSets = CacheBuilder.newBuilder()
-            .build(new CacheLoader<String, List<PeerId>>() {
+            .build(new CacheLoader<>() {
                 @Override
                 public List<PeerId> load(final String key) throws ExecutionException, InterruptedException {
                     return loadRoleSets(key);
@@ -66,15 +66,15 @@ public final class MatchBgpNeighborSetHandler
     @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private List<PeerId> loadRoleSets(final String key) throws ExecutionException, InterruptedException {
-        final ReadTransaction tr = this.dataBroker.newReadOnlyTransaction();
-        final Optional<NeighborSet> result = tr.read(LogicalDatastoreType.CONFIGURATION,
-                NEIGHBOR_SET_IID.child(NeighborSet.class, new NeighborSetKey(key))).get();
-        if (!result.isPresent()) {
-            return Collections.emptyList();
+        final FluentFuture<Optional<NeighborSet>> future;
+        try (ReadTransaction tr = this.dataBroker.newReadOnlyTransaction()) {
+            future = tr.read(LogicalDatastoreType.CONFIGURATION,
+                    NEIGHBOR_SET_IID.child(NeighborSet.class, new NeighborSetKey(key)));
+
         }
-        return result.get().getNeighbor().values().stream()
-                .map(nei -> RouterIds.createPeerId(nei.getAddress()))
-                .collect(Collectors.toList());
+        return future.get().map(neighboursSet -> neighboursSet.getNeighbor().values().stream()
+                                                         .map(nei -> RouterIds.createPeerId(nei.getAddress()))
+                                                         .collect(Collectors.toUnmodifiableList())).orElse(List.of());
     }
 
     @Override

diff --git a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchClusterIdSetHandler.java b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchClusterIdSetHandler.java
index af93772ef0ca13b6c89488f0b8767e0ed4b9ce67..47f382b571adc710e438bb17c2af984ba6e34e65 100644 (file)
--- a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchClusterIdSetHandler.java
+++ b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchClusterIdSetHandler.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.FluentFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -53,7 +54,7 @@ public final class MatchClusterIdSetHandler
             .augmentation(BgpClusterIdSets.class).child(ClusterIdSets.class);
     private final DataBroker dataBroker;
     private final LoadingCache<String, ClusterIdSet> sets = CacheBuilder.newBuilder()
-            .build(new CacheLoader<String, ClusterIdSet>() {
+            .build(new CacheLoader<>() {
                 @Override
                 public ClusterIdSet load(final String key) throws ExecutionException, InterruptedException {
                     return loadSets(key);
@@ -67,10 +68,12 @@ public final class MatchClusterIdSetHandler
     @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private ClusterIdSet loadSets(final String key) throws ExecutionException, InterruptedException {
-        final ReadTransaction tr = this.dataBroker.newReadOnlyTransaction();
-        final Optional<ClusterIdSet> result = tr.read(LogicalDatastoreType.CONFIGURATION,
-                CLUSTERS_ID_SETS_IID.child(ClusterIdSet.class, new ClusterIdSetKey(key))).get();
-        return result.orElse(null);
+        final FluentFuture<Optional<ClusterIdSet>> future;
+        try (ReadTransaction tr = this.dataBroker.newReadOnlyTransaction()) {
+            future = tr.read(LogicalDatastoreType.CONFIGURATION,
+                    CLUSTERS_ID_SETS_IID.child(ClusterIdSet.class, new ClusterIdSetKey(key)));
+        }
+        return  future.get().orElse(null);
     }
 
     @Override

diff --git a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchOriginatorIdSetHandler.java b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchOriginatorIdSetHandler.java
index bef45d96e6f968b1451f710205a9e30e05b3cb79..106b18338914d4ac6aceb7d5e08ec1f020762681 100644 (file)
--- a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchOriginatorIdSetHandler.java
+++ b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchOriginatorIdSetHandler.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.FluentFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
@@ -50,7 +51,7 @@ public final class MatchOriginatorIdSetHandler
             .augmentation(BgpOriginatorIdSets.class).child(OriginatorIdSets.class);
     private final DataBroker dataBroker;
     private final LoadingCache<String, OriginatorIdSet> sets = CacheBuilder.newBuilder()
-            .build(new CacheLoader<String, OriginatorIdSet>() {
+            .build(new CacheLoader<>() {
                 @Override
                 public OriginatorIdSet load(final String key) throws ExecutionException, InterruptedException {
                     return loadSets(key);
@@ -64,10 +65,12 @@ public final class MatchOriginatorIdSetHandler
     @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private OriginatorIdSet loadSets(final String key) throws ExecutionException, InterruptedException {
-        final ReadTransaction tr = this.dataBroker.newReadOnlyTransaction();
-        final Optional<OriginatorIdSet> result = tr.read(LogicalDatastoreType.CONFIGURATION,
-                ORIGINATOR_ID_SETS_IID.child(OriginatorIdSet.class, new OriginatorIdSetKey(key))).get();
-        return result.orElse(null);
+        final FluentFuture<Optional<OriginatorIdSet>> future;
+        try (ReadTransaction tr = this.dataBroker.newReadOnlyTransaction()) {
+            future = tr.read(LogicalDatastoreType.CONFIGURATION,
+                    ORIGINATOR_ID_SETS_IID.child(OriginatorIdSet.class, new OriginatorIdSetKey(key)));
+        }
+        return  future.get().orElse(null);
     }
 
     @Override

diff --git a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchRoleSetHandler.java b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchRoleSetHandler.java
index 5b631726e053cf339ecf30e34b461c1f25689c69..8a263ee1321e6157862904686d056e33fb269ca8 100644 (file)
--- a/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchRoleSetHandler.java
+++ b/bgp/openconfig-rp-statement/src/main/java/org/opendaylight/protocol/bgp/openconfig/routing/policy/statement/conditions/MatchRoleSetHandler.java
@@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.FluentFuture;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collections;
 import java.util.List;
@@ -53,7 +54,7 @@ public final class MatchRoleSetHandler implements BgpConditionsAugmentationPolic
             .augmentation(BgpRoleSets.class).child(RoleSets.class);
     private final DataBroker dataBroker;
     private final LoadingCache<String, List<PeerRole>> roleSets = CacheBuilder.newBuilder()
-            .build(new CacheLoader<String, List<PeerRole>>() {
+            .build(new CacheLoader<>() {
                 @Override
                 public List<PeerRole> load(final String key) throws ExecutionException, InterruptedException {
                     return loadRoleSets(key);
@@ -67,13 +68,12 @@ public final class MatchRoleSetHandler implements BgpConditionsAugmentationPolic
     @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
             justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private List<PeerRole> loadRoleSets(final String key) throws ExecutionException, InterruptedException {
-        final ReadTransaction tr = this.dataBroker.newReadOnlyTransaction();
-        final Optional<RoleSet> result = tr.read(LogicalDatastoreType.CONFIGURATION,
-                ROLE_SET_IID.child(RoleSet.class, new RoleSetKey(key))).get();
-        if (!result.isPresent()) {
-            return Collections.emptyList();
+        final  FluentFuture<Optional<RoleSet>> future;
+        try (ReadTransaction tr = this.dataBroker.newReadOnlyTransaction()) {
+            future = tr.read(LogicalDatastoreType.CONFIGURATION,
+                    ROLE_SET_IID.child(RoleSet.class, new RoleSetKey(key)));
         }
-        return result.get().getRole();
+        return future.get().map(RoleSet::getRole).orElse(Collections.emptyList());
     }
 
     @Override