brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [10/19] brooklyn-server git commit: add many of the code review comments
Date Thu, 02 Mar 2017 17:01:54 GMT
add many of the code review comments


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/7476d3b5
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/7476d3b5
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/7476d3b5

Branch: refs/heads/master
Commit: 7476d3b5f830d80fdd872ccfd70c8f0f7bf98015
Parents: 3f3e3d6
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Sat Feb 18 01:30:55 2017 +0000
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Sat Feb 18 01:31:32 2017 +0000

----------------------------------------------------------------------
 .../brooklyn/api/mgmt/ExecutionContext.java     |  14 ++
 .../brooklyn/camp/brooklyn/ConfigYamlTest.java  |   5 +-
 .../task/InterruptingImmediateSupplier.java     |   2 +-
 .../brooklyn/core/entity/EntityConfigTest.java  |  24 ++--
 .../task/InterruptingImmediateSupplierTest.java | 133 +++++++++++++++++++
 .../util/exceptions/ExceptionsTest.java         |  20 +++
 6 files changed, 186 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7476d3b5/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java b/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
index d8e538c..f8a963a 100644
--- a/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
+++ b/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
@@ -26,6 +26,8 @@ import java.util.concurrent.Executor;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.util.guava.Maybe;
 
+import com.google.common.annotations.Beta;
+
 /**
  * This is a Brooklyn extension to the Java {@link Executor}.
  * 
@@ -65,6 +67,18 @@ public interface ExecutionContext extends Executor {
 
     boolean isShutdown();
 
+    /**
+     * Gets the value promptly, or returns {@link Maybe#absent()} if the value is not yet
available.
+     * It may throw an error if it cannot be determined whether a value is available immediately
or not.
+     * <p>
+     * Implementations will typically attempt to execute in the current thread, with appropriate
+     * tricks to make it look like it is in a sub-thread, and will attempt to be non-blocking
but
+     * if needed they may block.
+     * <p>
+     * Supports {@link Callable} and {@link Runnable} targets to be evaluated with "immediate"
semantics.
+     */
+    // TODO reference ImmediateSupplier when that class is moved to utils project
+    @Beta
     <T> Maybe<T> getImmediately(Object callableOrSupplier);
 
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7476d3b5/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
index 64700de..1686f55 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java
@@ -103,7 +103,6 @@ public class ConfigYamlTest extends AbstractYamlTest {
         doTestRecursiveConfigFailsGracefully(false);
     }
     
-    // TODO this test fails because entities aren't available when evaluating immediately
     @Test
     public void testRecursiveConfigImmediateFailsGracefully() throws Exception {
         doTestRecursiveConfigFailsGracefully(true);
@@ -127,9 +126,9 @@ public class ConfigYamlTest extends AbstractYamlTest {
                     // error, loop wasn't interrupted or detected
                     LOG.warn("Timeout elapsed, destroying items; usage: "+
                             ((LocalManagementContext)mgmt()).getGarbageCollector().getUsageString());
-                    //Entities.destroy(app);
+                    Entities.destroy(app);
                 } catch (RuntimeInterruptedException e) {
-                    // expected on normal execution
+                    // expected on normal execution; clear the interrupted flag to prevent
ugly further warnings being logged
                     Thread.interrupted();
                 }
             }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7476d3b5/core/src/main/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplier.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplier.java
b/core/src/main/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplier.java
index ced001e..a92a641 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplier.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplier.java
@@ -44,7 +44,7 @@ import com.google.common.base.Supplier;
 @Beta
 public class InterruptingImmediateSupplier<T> implements ImmediateSupplier<T>,
DeferredSupplier<T> {
 
-    final Supplier<T> nestedSupplier;
+    private final Supplier<T> nestedSupplier;
     
     public InterruptingImmediateSupplier(Supplier<T> nestedSupplier) {
         this.nestedSupplier = nestedSupplier;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7476d3b5/core/src/test/java/org/apache/brooklyn/core/entity/EntityConfigTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/EntityConfigTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/EntityConfigTest.java
index 672924f..4882b7c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/entity/EntityConfigTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/entity/EntityConfigTest.java
@@ -372,31 +372,39 @@ public class EntityConfigTest extends BrooklynAppUnitTestSupport {
     
     @Test(groups="Integration") // because takes 1s+
     public void testGetTaskNonBlockingKey() throws Exception {
-        new ConfigNonBlockingFixture().usingTask().runGetConfigNonBlockingInKey(); }
+        new ConfigNonBlockingFixture().usingTask().runGetConfigNonBlockingInKey(); 
+    }
     @Test(groups="Integration") // because takes 1s+
     public void testGetTaskNonBlockingMap() throws Exception {
-        new ConfigNonBlockingFixture().usingTask().runGetConfigNonBlockingInMap(); }
+        new ConfigNonBlockingFixture().usingTask().runGetConfigNonBlockingInMap(); 
+    }
     
     @Test(groups="Integration") // because takes 1s+
     public void testGetTaskFactoryNonBlockingKey() throws Exception {
-        new ConfigNonBlockingFixture().usingTaskFactory().runGetConfigNonBlockingInKey();
}
+        new ConfigNonBlockingFixture().usingTaskFactory().runGetConfigNonBlockingInKey();

+    }
     @Test(groups="Integration") // because takes 1s+
     public void testGetTaskFactoryNonBlockingMap() throws Exception {
-        new ConfigNonBlockingFixture().usingTaskFactory().runGetConfigNonBlockingInMap();
}
+        new ConfigNonBlockingFixture().usingTaskFactory().runGetConfigNonBlockingInMap();

+    }
     
     @Test(groups="Integration") // because takes 1s+
     public void testGetSupplierNonBlockingKey() throws Exception {
-        new ConfigNonBlockingFixture().usingDeferredSupplier().runGetConfigNonBlockingInKey();
}
+        new ConfigNonBlockingFixture().usingDeferredSupplier().runGetConfigNonBlockingInKey();

+    }
     @Test(groups="Integration") // because takes 1s+
     public void testGetSuppierNonBlockingMap() throws Exception {
-        new ConfigNonBlockingFixture().usingDeferredSupplier().runGetConfigNonBlockingInMap();
}
+        new ConfigNonBlockingFixture().usingDeferredSupplier().runGetConfigNonBlockingInMap();

+    }
     
     @Test // fast 
     public void testGetImmediateSupplierNonBlockingKey() throws Exception {
-        new ConfigNonBlockingFixture().usingImmediateSupplier().runGetConfigNonBlockingInKey();
}
+        new ConfigNonBlockingFixture().usingImmediateSupplier().runGetConfigNonBlockingInKey();

+    }
     @Test(groups="Integration") // because takes 1s+
     public void testGetImmediateSupplierNonBlockingMap() throws Exception {
-        new ConfigNonBlockingFixture().usingImmediateSupplier().runGetConfigNonBlockingInMap();
}
+        new ConfigNonBlockingFixture().usingImmediateSupplier().runGetConfigNonBlockingInMap();

+    }
     
     @Test
     public void testGetConfigKeysReturnsFromSuperAndInterfacesAndSubClass() throws Exception
{

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7476d3b5/core/src/test/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplierTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplierTest.java
b/core/src/test/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplierTest.java
new file mode 100644
index 0000000..fe83225
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/util/core/task/InterruptingImmediateSupplierTest.java
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.util.core.task;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+import java.util.concurrent.Callable;
+
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.time.Duration;
+import org.apache.brooklyn.util.time.Time;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.google.common.util.concurrent.Callables;
+import com.google.common.util.concurrent.Runnables;
+
+public class InterruptingImmediateSupplierTest {
+
+    @Test(expectedExceptions=UnsupportedOperationException.class)
+    public void testOfInvalidType() throws Exception {
+        InterruptingImmediateSupplier.of("myval");
+    }
+    
+    @Test
+    public void testRunnable() throws Exception {
+        assertImmediatelyPresent(Runnables.doNothing(), null);
+        assertImmediatelyAbsent(new SleepingRunnable());
+        assertImmediatelyFails(new FailingRunnable(), MarkerException.class);
+    }
+    
+    @Test
+    public void testCallable() throws Exception {
+        assertImmediatelyPresent(Callables.returning("myval"), "myval");
+        assertImmediatelyAbsent(new SleepingCallable());
+        assertImmediatelyFails(new FailingCallable(), MarkerException.class);
+    }
+    
+    @Test
+    public void testSupplier() throws Exception {
+        assertImmediatelyPresent(Suppliers.ofInstance("myval"), "myval");
+        assertImmediatelyAbsent(new SleepingSupplier());
+        assertImmediatelyFails(new FailingSupplier(), MarkerException.class);
+    }
+    
+    private void assertImmediatelyPresent(Object orig, Object expected) {
+        Maybe<Object> result = getImmediately(orig);
+        assertEquals(result.get(), expected);
+        assertFalse(Thread.currentThread().isInterrupted());
+    }
+    
+    private void assertImmediatelyAbsent(Object orig) {
+        Maybe<Object> result = getImmediately(orig);
+        assertTrue(result.isAbsent(), "result="+result);
+        assertFalse(Thread.currentThread().isInterrupted());
+    }
+    
+    private void assertImmediatelyFails(Object orig, Class<? extends Exception> expected)
{
+        try {
+            Maybe<Object> result = getImmediately(orig);
+            Asserts.shouldHaveFailedPreviously("result="+result);
+        } catch (Exception e) {
+            Asserts.expectedFailureOfType(e, expected);
+        }
+        assertFalse(Thread.currentThread().isInterrupted());
+    }
+    
+    private Maybe<Object> getImmediately(Object val) {
+        InterruptingImmediateSupplier<Object> supplier = InterruptingImmediateSupplier.of(val);
+        return supplier.getImmediately();
+    }
+    
+    public static class SleepingRunnable implements Runnable {
+        @Override public void run() {
+            Time.sleep(Duration.ONE_MINUTE);
+        }
+    }
+    
+    public static class SleepingCallable implements Callable<Void> {
+        @Override public Void call() {
+            Time.sleep(Duration.ONE_MINUTE);
+            return null;
+        }
+    }
+    
+    public static class SleepingSupplier implements Supplier<Void> {
+        @Override public Void get() {
+            Time.sleep(Duration.ONE_MINUTE);
+            return null;
+        }
+    }
+    
+    public static class FailingRunnable implements Runnable {
+        @Override public void run() {
+            throw new MarkerException();
+        }
+    }
+    
+    public static class FailingCallable implements Callable<Void> {
+        @Override public Void call() {
+            throw new MarkerException();
+        }
+    }
+    
+    public static class FailingSupplier implements Supplier<Void> {
+        @Override public Void get() {
+            throw new MarkerException();
+        }
+    }
+    
+    public static class MarkerException extends RuntimeException {
+        private static final long serialVersionUID = -3395361406478634652L;
+    }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7476d3b5/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
b/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
index 65b5d91..272d3f6 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/exceptions/ExceptionsTest.java
@@ -34,6 +34,7 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
 
 public class ExceptionsTest {
 
@@ -307,6 +308,25 @@ public class ExceptionsTest {
     }
     
     @Test
+    public void testGetCausalChain() throws Exception {
+        Exception e1 = new Exception("e1");
+        Exception e2 = new Exception("e2", e1);
+        assertEquals(Exceptions.getCausalChain(e2), ImmutableList.of(e2, e1));
+    }
+    
+    @Test
+    public void testGetCausalChainRecursive() throws Exception {
+        Exception e1 = new Exception("e1") {
+            private static final long serialVersionUID = 1L;
+            public synchronized Throwable getCause() {
+                return this;
+            }
+        };
+        Exception e2 = new Exception("e2", e1);
+        assertEquals(Exceptions.getCausalChain(e2), ImmutableList.of(e2, e1));
+    }
+    
+    @Test
     public void testComplexJcloudsExample() {
         Throwable t;
         t = new IOException("POST https://ec2.us-east-1.amazonaws.com/ HTTP/1.1 -> HTTP/1.1
401 Unauthorized");


Mime
View raw message