Make MessageTracker.Context implement AutoCloseable 08/37008/3
authorRobert Varga <rovarga@cisco.com>
Fri, 1 Apr 2016 13:16:47 +0000 (15:16 +0200)
committerRobert Varga <rovarga@cisco.com>
Sat, 2 Apr 2016 13:22:51 +0000 (15:22 +0200)
The API contract is indicative of the fact that a Context is really
a resource which needs to be closed. Express this in code and take
advantage of availability of try-with.

Change-Id: I17ebc9e458ad43888fed868a89256cbb0fca7b87
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/common/actor/MessageTracker.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/common/actor/MessageTrackerTest.java

index 0851625..a602136 100644 (file)
@@ -41,29 +41,30 @@ import org.slf4j.LoggerFactory;
  *
  *     .....
  *
- *     MessageTracker.Context context = tracker.received(message);
+ *     try (MessageTracker.Context context = tracker.received(message)) {
  *
- *     if(context.error().isPresent()){
- *         LOG.error("{}", context.error().get());
- *     }
- *
- *     // Some custom processing
- *     process(message);
+ *         if (context.error().isPresent()){
+ *             LOG.error("{}", context.error().get());
+ *         }
  *
- *     context.done();
+ *         // Some custom processing
+ *         process(message);
+ *     }
  *
  * </pre>
  */
 @Beta
 @NotThreadSafe
 public final class MessageTracker {
-    public static abstract class Context {
+    public static abstract class Context implements AutoCloseable {
         Context() {
             // Hidden to prevent outside instantiation
         }
 
-        public abstract Context done();
         public abstract Optional<Error> error();
+
+        @Override
+        public abstract void close();
     }
 
     public interface Error {
@@ -102,8 +103,8 @@ public final class MessageTracker {
     private static final Logger LOG = LoggerFactory.getLogger(MessageTracker.class);
     private static final Context NO_OP_CONTEXT = new Context() {
         @Override
-        public Context done() {
-            return this;
+        public void close() {
+            // No-op
         }
 
         @Override
@@ -236,9 +237,8 @@ public final class MessageTracker {
         abstract Stopwatch stopTimer();
 
         @Override
-        public final Context done() {
+        public final void close() {
             processed(message(), stopTimer().elapsed(NANOSECONDS));
-            return this;
         }
     }
 
index ccd0b85..a47e437 100644 (file)
@@ -209,15 +209,13 @@ public class Shard extends RaftActor {
 
     @Override
     protected void handleCommand(final Object message) {
+        try (final MessageTracker.Context context = appendEntriesReplyTracker.received(message)) {
+            final Optional<Error> maybeError = context.error();
+            if (maybeError.isPresent()) {
+                LOG.trace("{} : AppendEntriesReply failed to arrive at the expected interval {}", persistenceId(),
+                    maybeError.get());
+            }
 
-        final MessageTracker.Context context = appendEntriesReplyTracker.received(message);
-        final Optional<Error> maybeError = context.error();
-        if (maybeError.isPresent()) {
-            LOG.trace("{} : AppendEntriesReply failed to arrive at the expected interval {}", persistenceId(),
-                maybeError.get());
-        }
-
-        try {
             if (CreateTransaction.isSerializedType(message)) {
                 handleCreateTransaction(message);
             } else if (message instanceof BatchedModifications) {
@@ -264,8 +262,6 @@ public class Shard extends RaftActor {
             } else {
                 super.handleCommand(message);
             }
-        } finally {
-            context.done();
         }
     }
 
index 8a9d789..5b7d1a6 100644 (file)
@@ -52,11 +52,11 @@ public class MessageTrackerTest {
     @Test
     public void testNoTracking() {
         MessageTracker.Context context1 = messageTracker.received(new Foo());
-        context1.done();
+        context1.close();
 
         ticker.increment(MILLISECONDS.toNanos(20));
         MessageTracker.Context context2 = messageTracker.received(new Foo());
-        context2.done();
+        context2.close();
     }
 
     @Test
@@ -64,7 +64,7 @@ public class MessageTrackerTest {
         messageTracker.begin();
 
         MessageTracker.Context context1 = messageTracker.received(new Foo());
-        context1.done();
+        context1.close();
 
         ticker.increment(MILLISECONDS.toNanos(20));
 
@@ -79,15 +79,15 @@ public class MessageTrackerTest {
         messageTracker.begin();
 
         MessageTracker.Context context1 = messageTracker.received(new Foo());
-        context1.done();
+        context1.close();
 
-        messageTracker.received("A").done();
-        messageTracker.received(10L).done();
+        messageTracker.received("A").close();
+        messageTracker.received(10L).close();
         MessageTracker.Context c = messageTracker.received(100);
 
         ticker.increment(MILLISECONDS.toNanos(20));
 
-        c.done();
+        c.close();
 
         MessageTracker.Context context2 = messageTracker.received(new Foo());
 
@@ -116,7 +116,7 @@ public class MessageTrackerTest {
         messageTracker.begin();
 
         MessageTracker.Context context1 = messageTracker.received(new Foo());
-        context1.done();
+        context1.close();
 
         ticker.increment(MILLISECONDS.toNanos(1));
 
@@ -185,13 +185,12 @@ public class MessageTrackerTest {
 
         messageTracker.begin();
 
-        MessageTracker.Context context1 = messageTracker.received(Integer.valueOf(45)).done();
-
-        Assert.assertEquals(false, context1.error().isPresent());
-
-        MessageTracker.Context context2 = messageTracker.received(Long.valueOf(45)).done();
-
-        Assert.assertEquals(false, context2.error().isPresent());
+        try (MessageTracker.Context ctx = messageTracker.received(45)) {
+            Assert.assertEquals(false, ctx.error().isPresent());
+        }
+        try (MessageTracker.Context ctx = messageTracker.received(45L)) {
+            Assert.assertEquals(false, ctx.error().isPresent());
+        }
 
         List<MessageTracker.MessageProcessingTime> processingTimeList =
                 messageTracker.getMessagesSinceLastExpectedMessage();