Create TargetPage data if specified by EchoEvent

Change-Id: I71039eb03b4b7e617ce03d515a6d51c4f3666ab8

includes/mapper/AbstractMapper.php

@@ -11,6 +11,12 @@ abstract class EchoAbstractMapper {
 	 */
 	protected $dbFactory;
 
+	/**
+	 * Event listeners for method like insert/delete
+	 * @var array
+	 */
+	protected $listeners;
+
 	/**
 	 * @param MWEchoDbFactory|null
 	 */
@@ -21,4 +27,50 @@ abstract class EchoAbstractMapper {
 		$this->dbFactory = $dbFactory;
 	}
 
+	/**
+	 * Attach a listener
+	 *
+	 * @param string $method Method name
+	 * @param string $key Identification of the callable
+	 * @param callable $callable
+	 */
+	public function attachListener( $method, $key, $callable ) {
+		if ( !method_exists( $this, $method ) ) {
+			throw new MWException( $method . ' does not exist in ' . get_class( $this ) );
+		}
+		if ( !isset( $this->listeners[$method] ) ) {
+			$this->listeners[$method] = array();
+		}
+
+		$this->listeners[$method][$key] = $callable;
+	}
+
+	/**
+	 * Detach a listener
+	 *
+	 * @param string $method Method name
+	 * @param string $key identification of the callable
+	 */
+	public function detachListener( $method, $key ) {
+		if ( isset( $this->listeners[$method] ) ) {
+			unset( $this->listeners[$method][$key] );
+		}
+	}
+
+	/**
+	 * Get the listener for a method
+	 *
+	 * @return array
+	 */
+	public function getMethodListeners( $method ) {
+		if ( !method_exists( $this, $method ) ) {
+			throw new MWException( $method . ' does not exist in ' . get_class( $this ) );
+		}
+		if ( isset( $this->listeners[$method] ) ) {
+			return $this->listeners[$method];
+		} else {
+			return array();
+		}
+	}
+
 }

includes/mapper/NotificationMapper.php

@@ -15,8 +15,9 @@ class EchoNotificationMapper extends EchoAbstractMapper {
 
 		$fname = __METHOD__;
 		$row = $notification->toDbArray();
+		$listeners = $this->getMethodListeners( __FUNCTION__ );
 
-		$dbw->onTransactionIdle( function() use ( $dbw, $row, $fname ) {
+		$dbw->onTransactionIdle( function() use ( $dbw, $row, $fname, $listeners ) {
 			$dbw->startAtomic( $fname );
 			// reset the bundle base if this notification has a display hash
 			// the result of this operation is that all previous notifications
@@ -39,11 +40,13 @@ class EchoNotificationMapper extends EchoAbstractMapper {
 			$res = $dbw->insert( 'echo_notification', $row, $fname );
 			$dbw->endAtomic( $fname );
 
-			// @todo - This is simple and easy but the proper way is to build a notification
-			// observer to notify all listeners on creating a new notification
 			if ( $res ) {
+				// @Todo - move the reset notification count logic to a listener
 				$user = User::newFromId( $row['notification_user'] );
 				MWEchoNotifUser::newFromUser( $user )->resetNotificationCount( DB_MASTER );
+				foreach ( $listeners as $listener ) {
+					call_user_func( $listener );
+				}
 			}
 		} );
 	}

model/Event.php

@@ -136,6 +136,7 @@ class EchoEvent extends EchoAbstractEntity{
 			return false;
 		}
 
+		//@Todo - Database insert logic should not be inside the model
 		$obj->insert();
 
 		wfRunHooks( 'EchoEventInsertComplete', array( $obj ) );

model/Notification.php

@@ -81,6 +81,7 @@ class EchoNotification extends EchoAbstractEntity {
 			$obj->timestamp = wfTimestampNow();
 		}
 
+		//@Todo - Database insert logic should not be inside the model
 		$obj->insert();
 
 		return $obj;
@@ -99,6 +100,11 @@ class EchoNotification extends EchoAbstractEntity {
 		if ( !empty( $wgEchoNotifications[$this->event->getType()]['bundle']['web'] ) ) {
 			wfRunHooks( 'EchoGetBundleRules', array( $this->event, &$bundleKey ) );
 		}
+
+		// The list of event ids to be removed from echo_target_page,
+		// this is mainly for bundled notifications when an event is
+		// no longer the bundle base
+		$eventIds = array();
 		if ( $bundleKey ) {
 			$hash = md5( $bundleKey );
 			$this->bundleHash = $hash;
@@ -109,11 +115,35 @@ class EchoNotification extends EchoAbstractEntity {
 			// 2. last bundle notification with the same hash was read
 			if ( $lastNotif && !$lastNotif->getReadTimestamp() ) {
 				$this->bundleDisplayHash = $lastNotif->getBundleDisplayHash();
+				$lastEvent = $lastNotif->getEvent();
+				if ( $lastEvent ) {
+					$eventIds[] = $lastEvent->getId();
+				}
 			} else {
 				$this->bundleDisplayHash = md5( $bundleKey . '-display-hash-' . wfTimestampNow() );
 			}
 		}
 
+		// Create a target page object if specified by event
+		$event = $this->event;
+		$user = $this->user;
+		if ( $event->getExtraParam( 'target-page' ) ) {
+			$notifMapper->attachListener( 'insert', 'add-target-page', function() use ( $event, $user, $eventIds ) {
+				// Make sure the target-page id is a valid id
+				$title = Title::newFromID( $event->getExtraParam( 'target-page' ) );
+				if ( $title ) {
+					$targetMapper = new EchoTargetPageMapper();
+					if ( $eventIds ) {
+						$targetMapper->deleteByUserEvents( $user, $eventIds );
+					}
+					$targetPage = EchoTargetPage::create( $user, $title, $event );
+					if ( $targetPage ) {
+						$targetMapper->insert( $targetPage );
+					}
+				}
+			} );
+		}
+
 		$notifMapper->insert( $this );
 
 		// Clear applicable section status from cache upon new notification creation

tests/includes/mapper/AbstractMapperTest.php

@@ -0,0 +1,75 @@
+<?php
+
+class EchoAbstractMapperTest extends MediaWikiTestCase {
+
+	public function testAttachListener() {
+		$mapper = new EchoAbstractMapperStub();
+		$mapper->attachListener( 'testMethod', 'key_a', function() {} );
+
+		$class = new ReflectionClass( 'EchoAbstractMapperStub' );
+		$property = $class->getProperty( 'listeners' );
+		$property->setAccessible( true );
+		$listeners = $property->getValue( $mapper );
+
+		$this->assertArrayHasKey( 'testMethod', $listeners );
+		$this->assertArrayHasKey( 'key_a', $listeners['testMethod'] );
+		$this->assertTrue( is_callable( $listeners['testMethod']['key_a'] ) );
+		return array( 'mapper' => $mapper, 'property' => $property );
+	}
+
+	/**
+	 * @expectedException MWException
+	 */
+	public function testAttachListenerWithException() {
+		$mapper = new EchoAbstractMapperStub();
+		$mapper->attachListener( 'nonExistingMethod', 'key_a', function() {} );
+	}
+
+	/**
+	 * @depends testAttachListener
+	 */
+	public function testGetMethodListeners( $data ) {
+		$mapper = $data['mapper'];
+		$property = $data['property'];
+
+		$listeners = $mapper->getMethodListeners( 'testMethod' );
+		$this->assertArrayHasKey( 'key_a', $listeners );
+		$this->assertTrue( is_callable( $listeners['key_a'] ) );
+	}
+
+	/**
+	 * @depends testAttachListener
+	 * @expectedException MWException
+	 */
+	public function testGetMethodListenersWithException( $data ) {
+		$mapper = $data['mapper'];
+		$property = $data['property'];
+
+		$listeners = $mapper->getMethodListeners( 'nonExistingMethod' );
+	}
+
+	/**
+	 * @depends testAttachListener
+	 */
+	public function testDetachListener( $data ) {
+		$mapper = $data['mapper'];
+		$property = $data['property'];
+
+		$mapper->detachListener( 'testMethod', 'key_a' );
+		$listeners = $property->getValue( $mapper );
+		$this->assertArrayHasKey( 'testMethod', $listeners );
+		$this->assertTrue( !isset( $listeners['testMethod']['key_a'] ) );
+	}
+
+}
+
+/**
+ * Create a stub class for testing the abstract class
+ */
+class EchoAbstractMapperStub extends EchoAbstractMapper {
+
+	public function testMethod() {
+
+	}
+
+}