From 48b20fb5541d6469137f090dc86f950866f56e62 Mon Sep 17 00:00:00 2001
From: Nicolas Capponi <nicolas.capponi@forgerock.com>
Date: Wed, 11 May 2016 15:40:19 +0000
Subject: [PATCH] OPENDJ-2969 Fix BlockLogReader#checkLogIsValid() method

---
 opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/BlockLogReader.java           |   24 ++++++++++++++----------
 opendj-server-legacy/src/test/java/org/opends/server/replication/server/changelog/file/BlockLogReaderWriterTest.java |   35 +++++++++++++++++++++++++++++++++--
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/BlockLogReader.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/BlockLogReader.java
index 7f8df32..8b33a59 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/BlockLogReader.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/server/changelog/file/BlockLogReader.java
@@ -29,6 +29,7 @@
 import org.forgerock.opendj.ldap.ByteStringBuilder;
 import org.forgerock.util.Pair;
 import org.forgerock.util.Reject;
+import org.forgerock.util.annotations.VisibleForTesting;
 import org.opends.server.replication.server.changelog.api.ChangelogException;
 import org.opends.server.replication.server.changelog.api.DBCursor.KeyMatchingStrategy;
 import org.opends.server.replication.server.changelog.api.DBCursor.PositionStrategy;
@@ -530,12 +531,22 @@
    *          The position of reader on file.
    * @return the file position of the block start.
    */
+  @VisibleForTesting
   long getClosestBlockStartBeforeOrAtPosition(final long filePosition)
   {
     final int dist = getDistanceToNextBlockStart(filePosition, blockSize);
     return dist == 0 ? filePosition : filePosition + dist - blockSize;
   }
 
+  @VisibleForTesting
+  long getClosestBlockStartToEndOfFile() throws ChangelogException
+  {
+    final long fileLength = getFileLength();
+    long lastBlockStart = getClosestBlockStartBeforeOrAtPosition(fileLength);
+    // block start is invalid if equal to file length
+    return lastBlockStart < fileLength ? lastBlockStart :  getClosestBlockStartBeforeOrAtPosition(fileLength-1);
+  }
+
   /**
    * Returns the closest start of block which has a position strictly
    * higher than the provided file position.
@@ -589,8 +600,7 @@
  {
    try
    {
-     final long fileSize = getFileLength();
-     final long lastBlockStart = getClosestBlockStartBeforeOrAtPosition(fileSize);
+     final long lastBlockStart = getClosestBlockStartToEndOfFile();
      positionToRecordFromBlockStart(lastBlockStart);
 
      long lastValidPosition = lastBlockStart;
@@ -599,7 +609,7 @@
        lastValidPosition = reader.getFilePointer();
      }
 
-     final boolean isFileValid = lastValidPosition == fileSize;
+     final boolean isFileValid = lastValidPosition == getFileLength();
      return isFileValid ? -1 : lastValidPosition;
    }
    catch (Exception e)
@@ -613,13 +623,7 @@
 Record<K, V> getNewestRecord() throws ChangelogException
  {
    try {
-     final long fileLength = getFileLength();
-     long lastBlockStart = getClosestBlockStartBeforeOrAtPosition(fileLength);
-     if (lastBlockStart == fileLength)
-     {
-       // this is not a valid block start, find the previous block start
-       lastBlockStart = getClosestBlockStartBeforeOrAtPosition(fileLength-1);
-     }
+     long lastBlockStart = getClosestBlockStartToEndOfFile();
      positionToRecordFromBlockStart(lastBlockStart);
      ByteString candidate = readNextRecord();
      ByteString record = candidate;
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/replication/server/changelog/file/BlockLogReaderWriterTest.java b/opendj-server-legacy/src/test/java/org/opends/server/replication/server/changelog/file/BlockLogReaderWriterTest.java
index c473aa2..5cf7943 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/replication/server/changelog/file/BlockLogReaderWriterTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/replication/server/changelog/file/BlockLogReaderWriterTest.java
@@ -15,7 +15,8 @@
  */
 package org.opends.server.replication.server.changelog.file;
 
-import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assertions.assertThat;
+
 import static org.opends.server.replication.server.changelog.api.DBCursor.KeyMatchingStrategy.*;
 import static org.opends.server.replication.server.changelog.api.DBCursor.PositionStrategy.*;
 import static org.opends.server.replication.server.changelog.file.BlockLogReader.*;
@@ -251,7 +252,7 @@
   }
 
   @Test
-  public void testGetClosestMarkerBeforeOrAtPosition() throws Exception
+  public void testGetClosestBlockStartBeforeOrAtPosition() throws Exception
   {
     final int blockSize = 10;
     BlockLogReader<Integer, Integer> reader = newReaderWithNullFile(blockSize);
@@ -295,6 +296,36 @@
     }
   }
 
+  @DataProvider
+  Object[][] recordsForEndOfFile()
+  {
+    return new Object[][]
+        {
+      // raw size taken by each record is: 4 (record size) + 4 (key) + 4 (value) = 12 bytes
+
+      // size of block, records to write, expected closest block start to end of file
+      { 12, records(1), 0 }, // zero block marker
+      { 10, records(1), 10 }, // one block marker
+      { 8, records(1), 8 },  // one block marker
+      { 7, records(1), 14 },  // two block markers
+      { 6, records(1), 18 },  // three block markers
+      { 5, records(1), 35 },  // seven block markers
+      { 16, records(1,2), 16 }, // one block marker
+      { 12, records(1,2), 24 }, // two block markers
+      { 10, records(1,2), 30 }, // three block markers
+        };
+  }
+  @Test(dataProvider="recordsForEndOfFile")
+  public void testGetClosestBlockStartToEndOfFile(int blockSize, List<Record<Integer, Integer>> records,
+      int expectedBlockStart) throws Exception
+  {
+    writeRecords(blockSize, records);
+    try(BlockLogReader<Integer, Integer> reader = newReader(blockSize))
+    {
+      assertThat(reader.getClosestBlockStartToEndOfFile()).isEqualTo(expectedBlockStart);
+    }
+  }
+
   @Test
   public void testGetClosestMarkerStrictlyAfterPosition() throws Exception
   {

--
Gitblit v1.10.0