use before and after update hooks to delete items only after new ones have been fetched

7 files changed, 59 insertions, 17 deletions

diff --git a/appinfo/routes.php b/appinfo/routes.php
index fd54794b5..de8f5a47a 100644
--- a/appinfo/routes.php
+++ b/appinfo/routes.php
@@ -239,9 +239,15 @@ $this->create('news_api_version', '/api/v1-2/version')->get()->action(
 	}
 );
 
-$this->create('news_api_cleanup', '/api/v1-2/cleanup')->get()->action(
+$this->create('news_api_before_update', '/api/v1-2/cleanup/before-update')->get()->action(
 	function($params) {
-		return App::main('NewsAPI', 'cleanUp', $params, new DIContainer());
+		return App::main('NewsAPI', 'beforeUpdate', $params, new DIContainer());
+	}
+);
+
+$this->create('news_api_after_update', '/api/v1-2/cleanup/after-update')->get()->action(
+	function($params) {
+		return App::main('NewsAPI', 'afterUpdate', $params, new DIContainer());
 	}
 );
 
diff --git a/backgroundjob/task.php b/backgroundjob/task.php
index d2eaab6bd..587259378 100644
--- a/backgroundjob/task.php
+++ b/backgroundjob/task.php
@@ -38,8 +38,9 @@ class Task {
 		// make it possible to turn off cron updates if you use an external
 		// script to execute updates in paralell
 		if ($container['useCronUpdates']) {
-			$container['Updater']->cleanUp();
+			$container['Updater']->beforeUpdate();
 			$container['Updater']->update();
+			$container['Updater']->afterUpdate();
 		}
 	}
 
diff --git a/bin/updater.py b/bin/updater.py
index e951c27c7..149ef5b3d 100644
--- a/bin/updater.py
+++ b/bin/updater.py
@@ -94,7 +94,8 @@ class Updater:
             self.base_url += '/'
         self.base_url += 'index.php/apps/news/api/v1-2'
 
-        self.cleanup_url = '%s/cleanup' % self.base_url
+        self.before_cleanup_url = '%s/cleanup/before-update' % self.base_url
+        self.after_cleanup_url = '%s/cleanup/after-update' % self.base_url
         self.all_feeds_url = '%s/feeds/all' % self.base_url
         self.update_url = '%s/feeds/update' % self.base_url
 
@@ -105,7 +106,7 @@ class Updater:
                 opener = get_basic_auth_opener(self.base_url, self.user,
                     self.password)
                 # run the cleanup request and get all the feeds to update
-                opener.open(self.cleanup_url)
+                opener.open(self.before_cleanup_url)
                 feeds_response = opener.open(self.all_feeds_url)
                 feeds_json = feeds_response.read().decode('utf-8')
                 feeds = json.loads(feeds_json)['feeds']
@@ -121,6 +122,8 @@ class Updater:
                 for thread in threads:
                     thread.join()
 
+                opener.open(self.after_cleanup_url)
+
                 if self.run_once:
                     return
                 
diff --git a/external/newsapi.php b/external/newsapi.php
index 1121f3400..51a2d9cf2 100644
--- a/external/newsapi.php
+++ b/external/newsapi.php
@@ -64,8 +64,19 @@ class NewsAPI extends Controller {
 	 * @Ajax
 	 * @API
 	 */
-	public function cleanUp() {
-		$this->updater->cleanUp();
+	public function beforeUpdate() {
+		$this->updater->beforeUpdate();
+		return new JSONResponse();
+	}
+
+
+	/**
+	 * @CSRFExemption
+	 * @Ajax
+	 * @API
+	 */
+	public function afterUpdate() {
+		$this->updater->afterUpdate();
 		return new JSONResponse();
 	}
 
diff --git a/tests/unit/external/NewsAPITest.php b/tests/unit/external/NewsAPITest.php
index a14faafc7..0f3031090 100644
--- a/tests/unit/external/NewsAPITest.php
+++ b/tests/unit/external/NewsAPITest.php
@@ -66,9 +66,14 @@ class NewsAPITest extends ControllerTestUtility {
 		$this->assertDefaultAnnotations('version');
 	}
 
-	public function testCleanUpAnnotations(){
+	public function testBeforeUpdateAnnotations(){
 		$annotations = array('Ajax', 'CSRFExemption', 'API');
-		$this->assertAnnotations($this->newsAPI, 'cleanUp', $annotations);
+		$this->assertAnnotations($this->newsAPI, 'beforeUpdate', $annotations);
+	}
+
+	public function testAfterUpdateAnnotations(){
+		$annotations = array('Ajax', 'CSRFExemption', 'API');
+		$this->assertAnnotations($this->newsAPI, 'afterUpdate', $annotations);
 	}
 
 	public function testGetVersion(){
@@ -85,10 +90,18 @@ class NewsAPITest extends ControllerTestUtility {
 	}
 
 
-	public function testCleanUp(){
+	public function testBeforeUpdate(){
+		$this->updater->expects($this->once())
+			->method('beforeUpdate');
+		$response = $this->newsAPI->beforeUpdate();
+		$this->assertTrue($response instanceof JSONResponse);
+	}
+
+
+	public function testAfterUpdate(){
 		$this->updater->expects($this->once())
-			->method('cleanUp');
-		$response = $this->newsAPI->cleanUp();
+			->method('afterUpdate');
+		$response = $this->newsAPI->afterUpdate();
 		$this->assertTrue($response instanceof JSONResponse);
 	}
 
diff --git a/tests/unit/utility/UpdaterTest.php b/tests/unit/utility/UpdaterTest.php
index 8a900d15e..2e6b03351 100644
--- a/tests/unit/utility/UpdaterTest.php
+++ b/tests/unit/utility/UpdaterTest.php
@@ -58,17 +58,21 @@ class UpdaterTest extends \PHPUnit_Framework_TestCase {
 			$this->itemBusinessLayer);
 	}
 
-	public function testCleanUp() {
+	public function testBeforeUpdate() {
 		$this->folderBusinessLayer->expects($this->once())
 			->method('purgeDeleted');
 		$this->feedBusinessLayer->expects($this->once())
 			->method('purgeDeleted');
+		$this->updater->beforeUpdate();
+	}
+
+
+	public function testAfterUpdate() {
 		$this->itemBusinessLayer->expects($this->once())
 			->method('autoPurgeOld');
-		$this->updater->cleanUp();
+		$this->updater->afterUpdate();
 	}
 
-
 	public function testUpdate() {
 		$this->feedBusinessLayer->expects($this->once())
 			->method('updateAll');
diff --git a/utility/updater.php b/utility/updater.php
index 0321f994d..cfe9cc16a 100644
--- a/utility/updater.php
+++ b/utility/updater.php
@@ -47,10 +47,9 @@ class Updater {
 	}
 
 
-	public function cleanUp() {
+	public function beforeUpdate() {
 		$this->folderBusinessLayer->purgeDeleted();
 		$this->feedBusinessLayer->purgeDeleted();
-		$this->itemBusinessLayer->autoPurgeOld();
 	}
 
 
@@ -59,4 +58,9 @@ class Updater {
 	}
 
 
+	public function afterUpdate() {
+		$this->itemBusinessLayer->autoPurgeOld();
+	}
+
+
 }
\ No newline at end of file