From 18eecbfda25b608526d8999f2577e8c286a8785d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Kucha=C5=99?= <honza.kuchar@grifart.cz>
Date: Fri, 6 May 2016 20:54:47 +0200
Subject: [PATCH] Cursor now has boundary checks

---
 src/PostgresDriver/CursorException.php        |  6 ++
 src/PostgresDriver/FastCursor.php             | 23 ++++++--
 src/PostgresDriver/ICursor.php                |  2 +-
 .../Store/PostgresDriver/FastCursorTest.phpt  | 59 +++++++++++++++++++
 4 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/src/PostgresDriver/CursorException.php b/src/PostgresDriver/CursorException.php
index 268f278..b5fa8e4 100644
--- a/src/PostgresDriver/CursorException.php
+++ b/src/PostgresDriver/CursorException.php
@@ -10,6 +10,7 @@ use Exception;
 class CursorException extends \LogicException
 {
 	const MESSAGE_NO_DATA_TO_FETCH = "There was not data to fetch. Haven't you reached end of cursor?";
+	const MESSAGE_OVERFLOW = "Cursor overflow. You've hit end or beginning of the cursor.";
 
 	/**
 	 * @inheritDoc
@@ -24,4 +25,9 @@ class CursorException extends \LogicException
 		return new static(self::MESSAGE_NO_DATA_TO_FETCH);
 	}
 
+	public static function cursorOverflow()
+	{
+		return new static(self::MESSAGE_OVERFLOW);
+	}
+
 }
\ No newline at end of file
diff --git a/src/PostgresDriver/FastCursor.php b/src/PostgresDriver/FastCursor.php
index 36c68bf..808309f 100644
--- a/src/PostgresDriver/FastCursor.php
+++ b/src/PostgresDriver/FastCursor.php
@@ -52,20 +52,26 @@ class FastCursor implements ICursor
 
 	public function moveTo(int $index)
 	{
-		$this->getConnection()->query(
+		$result = $this->getConnection()->query(
 			"MOVE ABSOLUTE %i IN %n",
 			$index,
 			$this->getCursorName()
 		);
+		if($result !== 1) {
+			throw CursorException::cursorOverflow();
+		}
 	}
 
 	public function moveBy(int $rows)
 	{
-		$this->getConnection()->query(
+		$result = $this->getConnection()->query(
 			"MOVE RELATIVE %i IN %n",
 			$rows,
 			$this->getCursorName()
 		);
+		if($result !== 1) {
+			throw CursorException::cursorOverflow();
+		}
 	}
 
 	public function moveToBeginning()
@@ -78,18 +84,24 @@ class FastCursor implements ICursor
 
 	public function moveToFirst()
 	{
-		$this->getConnection()->query(
+		$result = $this->getConnection()->query(
 			"MOVE FIRST IN %n;", // == ABSOLUTE 1
 			$this->getCursorName()
 		);
+		if($result !== 1) {
+			throw CursorException::cursorOverflow();
+		}
 	}
 
 	public function moveToLast()
 	{
-		$this->getConnection()->query(
+		$result = $this->getConnection()->query(
 			"MOVE LAST IN %n", // == ABSOLUTE -1
 			$this->getCursorName()
 		);
+		if($result !== 1) {
+			throw CursorException::cursorOverflow();
+		}
 	}
 
 	public function moveToEnd()
@@ -101,6 +113,8 @@ class FastCursor implements ICursor
 		);
 	}
 
+	// ----------------- FETCH ---------------------
+
 	public function fetch(int $rows)
 	{
 		$forward = $rows >= 0;
@@ -110,6 +124,7 @@ class FastCursor implements ICursor
 			$rows,
 			$this->getCursorName()
 		)->fetchAll();
+
 	}
 
 	public function fetchNext()
diff --git a/src/PostgresDriver/ICursor.php b/src/PostgresDriver/ICursor.php
index 8f2aa04..1fe4f8e 100644
--- a/src/PostgresDriver/ICursor.php
+++ b/src/PostgresDriver/ICursor.php
@@ -56,7 +56,7 @@ interface ICursor
 	 * @return void
 	 */
 	public function moveToEnd();
-
+	
 	/**
 	 * Fetch the next/previous count rows. FORWARD 0 re-fetches the current row.
 	 * @param int $rows
diff --git a/tests/Store/PostgresDriver/FastCursorTest.phpt b/tests/Store/PostgresDriver/FastCursorTest.phpt
index 9066b2f..7d6da26 100644
--- a/tests/Store/PostgresDriver/FastCursorTest.phpt
+++ b/tests/Store/PostgresDriver/FastCursorTest.phpt
@@ -233,7 +233,66 @@ class FastCursorTest extends BaseTest
 			$this->uut->fetchCurrentSingle();
 		}, CursorException::class, CursorException::MESSAGE_NO_DATA_TO_FETCH);
 	}
+
+
+	// edge cases:
+	public function test_givenInitialPosition_whenMoveToLeft_thenGetError()
+	{
+		// start: index === 0
+		Assert::exception(function() {
+			$this->uut->moveBy(-1);
+		}, CursorException::class, CursorException::MESSAGE_OVERFLOW);
+
+		// index === 0
+		$data = $this->uut->fetch(-1);
+		Assert::count(0, $data);
+
+		$firstValue = $this->uut->fetchNextSingle();
+		Assert::equal(1, $firstValue);
+
+	}
+
+	public function test_givenEndPosition_whenMoveToRight_thenGetError()
+	{
+		$this->uut->moveToEnd();
+
+		Assert::exception(function() {
+			$this->uut->moveBy(1);
+		}, CursorException::class, CursorException::MESSAGE_OVERFLOW);
+	}
+
+	public function test_givenFirstPosition_whenMoveToLeft_thenGetError()
+	{
+		$this->uut->moveToFirst();
+
+		Assert::exception(function() {
+			$this->uut->moveBy(-1);
+		}, CursorException::class, CursorException::MESSAGE_OVERFLOW);
+
+		// todo: unfortunately moveBy moved cursor into index=0 even when exception occured
+		// todo: this should be supported or should not modify state
+
+		Assert::exception(function() {
+			$this->uut->fetchCurrentSingle();
+		}, CursorException::class, CursorException::MESSAGE_NO_DATA_TO_FETCH);
+	}
 	
+	public function test_givenLastPosition_whenMoveToRight_thenGetError()
+	{
+		$this->uut->moveToLast();
+
+		Assert::exception(function() {
+			$this->uut->moveBy(1);
+		}, CursorException::class, CursorException::MESSAGE_OVERFLOW);
+
+		// todo: unfortunately moveBy moved cursor into index=0 even when exception occured
+		// todo: this should be supported or should not modify state
+
+		Assert::exception(function() {
+			$this->uut->fetchCurrentSingle();
+		}, CursorException::class, CursorException::MESSAGE_NO_DATA_TO_FETCH);
+	}
+
 }
 
 (new FastCursorTest())->run();
-- 
GitLab