brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [2/4] brooklyn-library git commit: Improvements for PR #66
Date Tue, 11 Oct 2016 21:16:57 GMT
Improvements for PR #66

* Avoid NPE in in AbstractController.computePortsAndUrls() if inferUrl
  returns null.
* Remove some duplication from inferUrl*()
* Renames inferUrlPublic() to inferUrlForPublic() - and same for subnet
* testMainUriSensorsCorrectlyComputedWithoutDomain to use machineLocation
  extending HasSubnetHostname, so subnet address set as expected.
* Tidy deprecated code and generics in test.

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-library/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-library/commit/023e4928
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-library/tree/023e4928
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-library/diff/023e4928

Branch: refs/heads/master
Commit: 023e49287c8acdb6daf3af171e7e7277b145f63a
Parents: 313203f
Author: Aled Sage <aled.sage@gmail.com>
Authored: Tue Oct 11 20:16:47 2016 +0100
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Tue Oct 11 20:16:47 2016 +0100

----------------------------------------------------------------------
 .../entity/proxy/AbstractControllerImpl.java    | 72 +++++++++++---------
 .../entity/proxy/AbstractControllerTest.java    | 46 ++++++++-----
 2 files changed, 69 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/023e4928/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
index be60cf2..3277e38 100644
--- a/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
+++ b/software/webapp/src/main/java/org/apache/brooklyn/entity/proxy/AbstractControllerImpl.java
@@ -54,6 +54,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Objects;
+import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -230,7 +231,15 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl
impleme
     public String getDomain() {
         return getAttribute(DOMAIN_NAME);
     }
-    
+
+    protected String getDomainWithoutWildcard() {
+        String domain = getDomain();
+        if (domain != null && domain.startsWith("*.")) {
+            domain = domain.replace("*.", ""); // Strip wildcard
+        }
+        return domain;
+    }
+
     @Override
     public Integer getPort() {
         if (isSsl())
@@ -267,39 +276,21 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl
impleme
         return isSsl() ? "https" : "http";
     }
 
-    protected String inferUrlSubnet() {
-        String protocol = checkNotNull(getProtocol(), "no protocol configured");
-        String domain = getDomain();
-        if (domain != null && domain.startsWith("*.")) {
-            domain = domain.replace("*.", ""); // Strip wildcard
-        }
-        Integer port = checkNotNull(getPort(), "no port configured (the requested port may
be in use)");
-
+    protected String inferUrlForSubnet() {
+        String domain = getDomainWithoutWildcard();
         if (domain==null) domain = getAttribute(Attributes.SUBNET_ADDRESS);
-        if (domain==null) return null;
-        return protocol+"://"+domain+":"+port+"/"+getConfig(SERVICE_UP_URL_PATH);
+        return inferUrl(domain, Optional.<Integer>absent());
     }
 
-    protected String inferUrlPublic() {
-        String protocol = checkNotNull(getProtocol(), "no protocol configured");
-        String domain = getDomain();
-        if (domain != null && domain.startsWith("*.")) {
-            domain = domain.replace("*.", ""); // Strip wildcard
-        }
-        Integer port = checkNotNull(getPort(), "no port configured (the requested port may
be in use)");
-
+    protected String inferUrlForPublic() {
+        String domain = getDomainWithoutWildcard();
         if (domain==null) domain = getAttribute(Attributes.ADDRESS);
-        if (domain==null) return null;
-        return protocol+"://"+domain+":"+port+"/"+getConfig(SERVICE_UP_URL_PATH);
+        return inferUrl(domain, Optional.<Integer>absent());
     }
 
     /** returns URL, if it can be inferred; null otherwise */
     protected String inferUrl(boolean requireManagementAccessible) {
-        String protocol = checkNotNull(getProtocol(), "no protocol configured");
-        String domain = getDomain();
-        if (domain != null && domain.startsWith("*.")) {
-            domain = domain.replace("*.", ""); // Strip wildcard
-        }
+        String domain = getDomainWithoutWildcard();
         Integer port = checkNotNull(getPort(), "no port configured (the requested port may
be in use)");
         if (requireManagementAccessible) {
             HostAndPort accessible = BrooklynAccessUtils.getBrooklynAccessibleAddress(this,
port);
@@ -309,8 +300,15 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl
impleme
             }
         }
         if (domain==null) domain = Machines.findSubnetHostname(this).orNull();
-        if (domain==null) return null;
-        return protocol+"://"+domain+":"+port+"/"+getConfig(SERVICE_UP_URL_PATH);
+        return inferUrl(domain, Optional.of(port));
+    }
+
+    protected String inferUrl(String host, Optional<Integer> portOverride) {
+        if (host == null) return null;
+        String protocol = checkNotNull(getProtocol(), "no protocol configured");
+        int port = portOverride.isPresent() ? portOverride.get() : checkNotNull(getPort(),
"no port configured (the requested port may be in use)");
+        String path = getConfig(SERVICE_UP_URL_PATH);
+        return protocol+"://"+host+":"+port+"/"+path;
     }
 
     protected String inferUrl() {
@@ -343,14 +341,26 @@ public abstract class AbstractControllerImpl extends SoftwareProcessImpl
impleme
         ConfigToAttributes.apply(this);
 
         sensors().set(PROTOCOL, inferProtocol());
-        sensors().set(MAIN_URI, URI.create(inferUrl()));
-        sensors().set(MAIN_URI_MAPPED_SUBNET, URI.create(inferUrlSubnet()));
-        sensors().set(MAIN_URI_MAPPED_PUBLIC, URI.create(inferUrlPublic()));
+        sensors().set(MAIN_URI, createUriOrNull(inferUrl()));
+        sensors().set(MAIN_URI_MAPPED_SUBNET, createUriOrNull(inferUrlForSubnet()));
+        sensors().set(MAIN_URI_MAPPED_PUBLIC, createUriOrNull(inferUrlForPublic()));
         sensors().set(ROOT_URL, inferUrl());
  
         checkNotNull(getPortNumberSensor(), "no sensor configured to infer port number");
     }
     
+    private URI createUriOrNull(String val) {
+        if (val == null) {
+            return null;
+        }
+        try {
+            return URI.create(val);
+        } catch (IllegalArgumentException e) {
+            LOG.warn("Invalid URI for {}: {}", this, val);
+            return null;
+        }
+    }
+    
     @Override
     protected void connectSensors() {
         super.connectSensors();

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/023e4928/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
index 45397be..831df4c 100644
--- a/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
+++ b/software/webapp/src/test/java/org/apache/brooklyn/entity/proxy/AbstractControllerTest.java
@@ -31,7 +31,6 @@ import java.util.Map;
 import java.util.Set;
 
 import org.apache.brooklyn.api.entity.Entity;
-import org.apache.brooklyn.api.entity.EntityLocal;
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.location.Location;
 import org.apache.brooklyn.api.location.LocationSpec;
@@ -43,6 +42,7 @@ import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.EntityAsserts;
 import org.apache.brooklyn.core.entity.factory.EntityFactory;
 import org.apache.brooklyn.core.entity.trait.Startable;
+import org.apache.brooklyn.core.location.HasSubnetHostname;
 import org.apache.brooklyn.core.location.PortRanges;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestEntity;
@@ -155,7 +155,7 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
     public void testUpdateCalledWithAddressesOfNewChildren() {
         // First child
         cluster.resize(1);
-        EntityLocal child = (EntityLocal) Iterables.getOnlyElement(cluster.getMembers());
+        Entity child = Iterables.getOnlyElement(cluster.getMembers());
         
         List<Collection<String>> u = Lists.newArrayList(controller.getUpdates());
         assertTrue(u.isEmpty(), "expected empty list but got "+u);
@@ -171,7 +171,7 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
             public void run() {
                 assertEquals(cluster.getMembers().size(), 2);
             }});
-        EntityLocal child2 = (EntityLocal) Iterables.getOnlyElement(MutableSet.builder().addAll(cluster.getMembers()).remove(child).build());
+        Entity child2 = Iterables.getOnlyElement(MutableSet.<Entity>builder().addAll(cluster.getMembers()).remove(child).build());
         
         child2.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
         child2.sensors().set(Startable.SERVICE_UP, true);
@@ -192,8 +192,8 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
         // Get some children, so we can remove one...
         cluster.resize(2);
         for (Entity it: cluster.getMembers()) { 
-            ((EntityLocal)it).sensors().set(ClusteredEntity.HTTP_PORT, 1234);
-            ((EntityLocal)it).sensors().set(Startable.SERVICE_UP, true);
+            it.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
+            it.sensors().set(Startable.SERVICE_UP, true);
         }
         assertEventuallyAddressesMatchCluster();
 
@@ -208,17 +208,17 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
         // Get some children, so we can remove one...
         cluster.resize(2);
         for (Entity it: cluster.getMembers()) { 
-            ((EntityLocal)it).sensors().set(ClusteredEntity.HTTP_PORT, 1234);
-            ((EntityLocal)it).sensors().set(Startable.SERVICE_UP, true);
+            it.sensors().set(ClusteredEntity.HTTP_PORT, 1234);
+            it.sensors().set(Startable.SERVICE_UP, true);
         }
         assertEventuallyAddressesMatchCluster();
 
         // Now unset host/port, and remove children
         // Note the unsetting of hostname is done in SoftwareProcessImpl.stop(), so this
is realistic
         for (Entity it : cluster.getMembers()) {
-            ((EntityLocal)it).sensors().set(ClusteredEntity.HTTP_PORT, null);
-            ((EntityLocal)it).sensors().set(ClusteredEntity.HOSTNAME, null);
-            ((EntityLocal)it).sensors().set(Startable.SERVICE_UP, false);
+            it.sensors().set(ClusteredEntity.HTTP_PORT, null);
+            it.sensors().set(ClusteredEntity.HOSTNAME, null);
+            it.sensors().set(Startable.SERVICE_UP, false);
         }
         assertEventuallyAddressesMatch(ImmutableList.<Entity>of());
     }
@@ -314,24 +314,34 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
 
     @Test
     public void testMainUriSensorsCorrectlyComputedWithoutDomain() throws Exception {
+        // The MachineLocation needs to implement HasSubnetHostname for the Attributes.SUBNET_HOSTNAME

+        // to be set with the subnet addresss (otherwise it will fall back to using machine.getAddress()).
+        // See Machines.getSubnetHostname. 
+        
         TrackingAbstractController controller2 = app.addChild(EntitySpec.create(TrackingAbstractController.class)
                 .configure(TrackingAbstractController.SERVER_POOL, cluster)
                 .configure(TrackingAbstractController.PROXY_HTTP_PORT, PortRanges.fromInteger(8081))
-                .location(LocationSpec.create(SshMachineLocation.class)
+                .location(LocationSpec.create(SshMachineLocationWithSubnetHostname.class)
                         .configure("address", Inet4Address.getByName("1.1.1.1"))
                         .configure(SshMachineLocation.PRIVATE_ADDRESSES, ImmutableList.of("2.2.2.2"))));
         controller2.start(ImmutableList.<Location>of());
 
-        // Unfortunately the Attributes.SUBNET_HOSTNAME is 1.1.1.1, because SshMachineLocation
does not implement
-        // HasSubnetHostname (see Machines.getSubnetHostname). It falls back to using the
machine.getAddress().
-        // Hence the MAIN_URI uses 1.1.1.1.
         EntityAsserts.assertAttributeEquals(controller2, Attributes.ADDRESS, "1.1.1.1");
         EntityAsserts.assertAttributeEquals(controller2, Attributes.SUBNET_ADDRESS, "2.2.2.2");
-        EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI, URI.create("http://1.1.1.1:8081/"));
+        EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI, URI.create("http://2.2.2.2:8081/"));
         EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI_MAPPED_PUBLIC,
URI.create("http://1.1.1.1:8081/"));
         EntityAsserts.assertAttributeEquals(controller2, Attributes.MAIN_URI_MAPPED_SUBNET,
URI.create("http://2.2.2.2:8081/"));
     }
-
+    public static class SshMachineLocationWithSubnetHostname extends SshMachineLocation implements
HasSubnetHostname {
+        @Override public String getSubnetHostname() {
+            return getSubnetIp();
+        }
+        @Override public String getSubnetIp() {
+            Set<String> addrs = getPrivateAddresses();
+            return (addrs.isEmpty()) ? getAddress().getHostAddress() : Iterables.get(addrs,
0);
+        }
+    }
+    
     private void assertEventuallyAddressesMatchCluster() {
         assertEventuallyAddressesMatch(cluster.getMembers());
     }
@@ -388,10 +398,10 @@ public class AbstractControllerTest extends BrooklynAppUnitTestSupport
{
         @SetFromFlag("hostAndPort")
         public static final AttributeSensor<String> HOST_AND_PORT = Attributes.HOST_AND_PORT;
         
-        MachineProvisioningLocation provisioner;
+        MachineProvisioningLocation<MachineLocation> provisioner;
         
         public void start(Collection<? extends Location> locs) {
-            provisioner = (MachineProvisioningLocation) locs.iterator().next();
+            provisioner = (MachineProvisioningLocation<MachineLocation>) locs.iterator().next();
             MachineLocation machine;
             try {
                 machine = provisioner.obtain(MutableMap.of());


Mime
View raw message