Changing How The Relay Processes Reads

The log for reads was a lot more complicated than it needed
to be. This change makes it simplier and adds tests to verify
the reads are working.

Author: vaage

src/codeu/chat/relay/Server.java

@@ -211,29 +211,23 @@ public Collection<Relay.Bundle> read(Uuid teamId, byte[] teamSecret, Uuid root,
 
     if (authenticate(teamId, teamSecret)) {
 
-      int remaining = Math.min(range, maxRead);
-
       LOG.info(
          "Request to read from server requested=%d allowed=%d",
           range,
           maxRead);
 
-      // Writing is a one way flag (once set it will not be unset) that switches
-      // between looking for the starting message and writing all messages to the
-      // output.
-      boolean writing = root == Uuid.NULL || root == null;
-
       for (final Relay.Bundle message : history) {
 
-        if (writing && remaining > 0) {
+        // Only add a message if there is room. We cannot stop
+        // searching in case we see the root later on.
+        if (found.size() < Math.min(range, maxRead)) {
           found.add(message);
         }
 
-        remaining = Math.max(remaining - 1, 0);
-
-        // Only update "writing" after the check as the root is already known and
-        // should not be included in the output.
-        writing |= Uuid.equals(root, message.id());
+        // If the start is found, drop all previous messages.
+        if (message.id().equals(root)) {
+          found.clear();
+        }
       }
 
       LOG.info(

test/codeu/chat/relay/ServerTest.java

@@ -214,4 +214,76 @@ public void testHistoryOverwrite() {
       assertTrue(Uuid.equals(bundle.message().id(), new Uuid(7)));
     }
   }
+
+  @Test
+  public void testReadWithMissingRoot() {
+
+    final Server relay = new Server(8, 8);
+
+    final Uuid team = new Uuid(3);
+    final byte[] secret = { 0x00, 0x01, 0x02 };
+
+    assertTrue(relay.addTeam(team, secret));
+
+    assertTrue(relay.write(team,
+                           secret,
+                           relay.pack(new Uuid(4), "User", Time.now()),
+                           relay.pack(new Uuid(5), "Conversation", Time.now()),
+                           relay.pack(new Uuid(6), "Hello World", Time.now())));
+
+    final Collection<Relay.Bundle> read = relay.read(team, secret, new Uuid(7), 1);
+    assertTrue(read.size() == 1);
+
+    // By the assertion above this loop should only execute once as there should only
+    // be a single value in the collection.
+
+    for (final Relay.Bundle bundle : read) {
+      assertTrue(Uuid.equals(bundle.team(), team));
+      assertTrue(Uuid.equals(bundle.user().id(), new Uuid(4)));
+      assertTrue(Uuid.equals(bundle.conversation().id(), new Uuid(5)));
+      assertTrue(Uuid.equals(bundle.message().id(), new Uuid(6)));
+    }
+  }
+
+  @Test
+  public void testReadMidHistory() {
+
+    final Server relay = new Server(8, 8);
+
+    final Uuid team = new Uuid(3);
+    final byte[] secret = { 0x00, 0x01, 0x02 };
+
+    assertTrue(relay.addTeam(team, secret));
+
+    assertTrue(relay.write(team,
+                           secret,
+                           relay.pack(new Uuid(4), "User", Time.now()),
+                           relay.pack(new Uuid(5), "Conversation", Time.now()),
+                           relay.pack(new Uuid(6), "Hello World", Time.now())));
+
+    assertTrue(relay.write(team,
+                           secret,
+                           relay.pack(new Uuid(7), "User", Time.now()),
+                           relay.pack(new Uuid(8), "Conversation", Time.now()),
+                           relay.pack(new Uuid(9), "Hello World", Time.now())));
+
+    assertTrue(relay.write(team,
+                           secret,
+                           relay.pack(new Uuid(10), "User", Time.now()),
+                           relay.pack(new Uuid(11), "Conversation", Time.now()),
+                           relay.pack(new Uuid(12), "Hello World", Time.now())));
+
+    final Collection<Relay.Bundle> read = relay.read(team, secret, new Uuid(2), 1);
+    assertTrue(read.size() == 1);
+
+    // By the assertion above this loop should only execute once as there should only
+    // be a single value in the collection.
+
+    for (final Relay.Bundle bundle : read) {
+
+      // The relay server uses a linear id generator starting at 1 - so starting
+      // bundle 2, the id should be 3.
+      assertTrue(Uuid.equals(bundle.id(), new Uuid(3)));
+    }
+  }
 }