You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2018/01/31 02:14:58 UTC

[GitHub] merlimat closed pull request #1128: Use Optional instead of null checks when reading isolation policies

merlimat closed pull request #1128: Use Optional instead of null checks when reading isolation policies
URL: https://github.com/apache/incubator-pulsar/pull/1128
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LoadManagerShared.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LoadManagerShared.java
index ffdaac64f..e0af8ec0d 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LoadManagerShared.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/LoadManagerShared.java
@@ -90,7 +90,7 @@ public static synchronized void applyNamespacePolicies(final ServiceUnitId servi
         primariesCache.clear();
         secondaryCache.clear();
         NamespaceName namespace = serviceUnit.getNamespaceObject();
-        boolean isIsolationPoliciesPresent = policies.IsIsolationPoliciesPresent(namespace);
+        boolean isIsolationPoliciesPresent = policies.areIsolationPoliciesPresent(namespace);
         boolean isNonPersistentTopic = (serviceUnit instanceof NamespaceBundle)
                 ? ((NamespaceBundle) serviceUnit).hasNonPersistentTopic() : false;
         if (isIsolationPoliciesPresent) {
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/SimpleResourceAllocationPolicies.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/SimpleResourceAllocationPolicies.java
index c8322f68c..fa67c0d7b 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/SimpleResourceAllocationPolicies.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/SimpleResourceAllocationPolicies.java
@@ -19,6 +19,7 @@
 package org.apache.pulsar.broker.loadbalance.impl;
 
 import java.util.Map;
+import java.util.Optional;
 
 import org.apache.pulsar.broker.PulsarService;
 import org.apache.pulsar.broker.admin.AdminResource;
@@ -47,57 +48,62 @@ public boolean canAssign(ServiceUnit srvUnit, ResourceUnit rescrUnit, Map<Resour
         return true;
     }
 
-    private NamespaceIsolationPolicies getIsolationPolicies(String clusterName) {
-        NamespaceIsolationPolicies policies = null;
+    private Optional<NamespaceIsolationPolicies> getIsolationPolicies(String clusterName) {
         try {
-            policies = namespaceIsolationPolicies
-                    .get(AdminResource.path("clusters", clusterName, "namespaceIsolationPolicies")).orElse(null);
+            return namespaceIsolationPolicies
+                    .get(AdminResource.path("clusters", clusterName, "namespaceIsolationPolicies"));
         } catch (Exception e) {
             LOG.warn("GetIsolationPolicies: Unable to get the namespaceIsolationPolicies [{}]", e);
+            return Optional.empty();
         }
-        return policies;
     }
 
-    public boolean IsIsolationPoliciesPresent(NamespaceName namespace) {
+    public boolean areIsolationPoliciesPresent(NamespaceName namespace) {
         try {
-            NamespaceIsolationPolicies policies = null;
-            policies = getIsolationPolicies(pulsar.getConfiguration().getClusterName());
-            if (policies != null)
-                return policies.getPolicyByNamespace(namespace) != null;
+            Optional<NamespaceIsolationPolicies> policies = getIsolationPolicies(pulsar.getConfiguration().getClusterName());
+            if (policies.isPresent()) {
+                return policies.get().getPolicyByNamespace(namespace) != null;
+            } else {
+                return false;
+            }
         } catch (Exception e) {
             LOG.warn("IsIsolationPoliciesPresent: Unable to get the namespaceIsolationPolicies [{}]", e);
+            return false;
         }
-        return false;
     }
 
-    private NamespaceIsolationPolicy getNamespaceIsolationPolicy(NamespaceName namespace) {
-        NamespaceIsolationPolicies policies = null;
-        NamespaceIsolationPolicy policy = null;
+    private Optional<NamespaceIsolationPolicy> getNamespaceIsolationPolicy(NamespaceName namespace) {
         try {
-            policies = getIsolationPolicies(pulsar.getConfiguration().getClusterName());
-            policy = policies.getPolicyByNamespace(namespace);
+            Optional<NamespaceIsolationPolicies> policies =getIsolationPolicies(pulsar.getConfiguration().getClusterName());
+            if (!policies.isPresent()) {
+                return Optional.empty();
+            }
+
+            return Optional.ofNullable(policies.get().getPolicyByNamespace(namespace));
         } catch (Exception e) {
             LOG.warn("Unable to get the namespaceIsolationPolicies [{}]", e);
+            return Optional.empty();
         }
-        return policy;
     }
 
     public boolean isPrimaryBroker(NamespaceName namespace, String broker) {
-		NamespaceIsolationPolicy nsPolicy = getNamespaceIsolationPolicy(namespace);
-		return (nsPolicy != null) ? nsPolicy.isPrimaryBroker(broker) : false;
+		Optional<NamespaceIsolationPolicy> nsPolicy = getNamespaceIsolationPolicy(namespace);
+		return nsPolicy.isPresent() && nsPolicy.get().isPrimaryBroker(broker);
     }
 
     public boolean isSecondaryBroker(NamespaceName namespace, String broker) {
-		NamespaceIsolationPolicy nsPolicy = getNamespaceIsolationPolicy(namespace);
-		return (nsPolicy != null) ? nsPolicy.isSecondaryBroker(broker) : false;
+		Optional<NamespaceIsolationPolicy> nsPolicy = getNamespaceIsolationPolicy(namespace);
+		return nsPolicy.isPresent() && nsPolicy.get().isSecondaryBroker(broker);
     }
 
     public boolean isSharedBroker(String broker) {
         try {
-            NamespaceIsolationPolicies policies = getIsolationPolicies(pulsar.getConfiguration().getClusterName());
-            if (policies == null)
+            Optional<NamespaceIsolationPolicies> policies = getIsolationPolicies(pulsar.getConfiguration().getClusterName());
+            if (!policies.isPresent()) {
                 return true;
-            return policies.isSharedBroker(broker);
+            }
+
+            return policies.get().isSharedBroker(broker);
         } catch (Exception e) {
             LOG.warn("isPrimaryForAnyNamespace: [{}]", e);
         }
@@ -105,7 +111,7 @@ public boolean isSharedBroker(String broker) {
     }
 
     public boolean shouldFailoverToSecondaries(NamespaceName namespace, int totalPrimaryCandidates) {
-		NamespaceIsolationPolicy nsPolicy = getNamespaceIsolationPolicy(namespace);
-		return (nsPolicy != null) ? nsPolicy.shouldFailover(totalPrimaryCandidates) : false;
+		Optional<NamespaceIsolationPolicy> nsPolicy = getNamespaceIsolationPolicy(namespace);
+		return nsPolicy.isPresent() && nsPolicy.get().shouldFailover(totalPrimaryCandidates);
     }
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services