Bsitu has uploaded a new change for review.
https://gerrit.wikimedia.org/r/153651
Change subject: Add TargetPage model and mapper to notifications
......................................................................
Add TargetPage model and mapper to notifications
This will be used for marking a notificaiton as read when
a user visits a target page. The new table should keep the
volume as low as possible for fast data loopup. records
should be removed from the table once it's marked as read.
Change-Id: I605cbc79adfc12d22bd889c5bb513d05c479fe6e
---
M Echo.php
M Hooks.php
A db_patches/echo_target_page.sql
M echo.sql
A includes/mapper/TargetPageMapper.php
A model/TargetPage.php
A tests/includes/mapper/TargetPageMapperTest.php
A tests/model/TargetPageTest.php
8 files changed, 610 insertions(+), 0 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/51/153651/1
diff --git a/Echo.php b/Echo.php
index ac8f765..30ac6b4 100644
--- a/Echo.php
+++ b/Echo.php
@@ -52,6 +52,7 @@
$wgAutoloadClasses['EchoAbstractEntity'] = $dir . 'model/AbstractEntity.php';
$wgAutoloadClasses['EchoEvent'] = $dir . 'model/Event.php';
$wgAutoloadClasses['EchoNotification'] = $dir . 'model/Notification.php';
+$wgAutoloadClasses['EchoTargetPage'] = $dir . 'model/TargetPage.php';
$wgAutoloadClasses['MWEchoEmailBatch'] = $dir . 'includes/EmailBatch.php';
$wgAutoloadClasses['MWDbEchoEmailBatch'] = $dir . 'includes/DbEmailBatch.php';
$wgAutoloadClasses['MWEchoEmailBundler'] = $dir . 'includes/EmailBundler.php';
@@ -63,6 +64,7 @@
$wgAutoloadClasses['EchoAbstractMapper'] = $dir .
'includes/mapper/AbstractMapper.php';
$wgAutoloadClasses['EchoEventMapper'] = $dir .
'includes/mapper/EventMapper.php';
$wgAutoloadClasses['EchoNotificationMapper'] = $dir .
'includes/mapper/NotificationMapper.php';
+$wgAutoloadClasses['EchoTargetPageMapper'] = $dir .
'includes/mapper/TargetPageMapper.php';
$wgAutoloadClasses['EchoUserNotificationGateway'] = $dir .
'includes/gateway/UserNotificationGateway.php';
// Output formatters
diff --git a/Hooks.php b/Hooks.php
index 8eb741b..8e12509 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -110,6 +110,7 @@
$baseSQLFile = "$dir/echo.sql";
$updater->addExtensionTable( 'echo_event', $baseSQLFile );
$updater->addExtensionTable( 'echo_email_batch',
"$dir/db_patches/echo_email_batch.sql" );
+ $updater->addExtensionTable( 'echo_target_page',
"$dir/db_patches/echo_target_page.sql" );
if ( $updater->getDB()->getType() === 'sqlite' ) {
$updater->modifyExtensionField( 'echo_event',
'event_agent', "$dir/db_patches/patch-event_agent-split.sqlite.sql" );
diff --git a/db_patches/echo_target_page.sql b/db_patches/echo_target_page.sql
new file mode 100644
index 0000000..f6e5c4a
--- /dev/null
+++ b/db_patches/echo_target_page.sql
@@ -0,0 +1,8 @@
+CREATE TABLE /*_*/echo_target_page (
+ etp_user int unsigned not null default 0,
+ etp_page int unsigned not null default 0,
+ etp_event int unsigned not null default 0
+) /*$wgDBTableOptions*/;
+
+CREATE UNIQUE INDEX /*i*/echo_target_page_user_event ON /*_*/echo_target_page
(etp_user, etp_event);
+CREATE INDEX /*i*/echo_target_page_user_page_event ON /*_*/echo_target_page
(etp_user, etp_page, etp_event);
diff --git a/echo.sql b/echo.sql
index 7b3f388..02871a6 100644
--- a/echo.sql
+++ b/echo.sql
@@ -41,3 +41,12 @@
CREATE UNIQUE INDEX /*i*/echo_email_batch_user_event ON /*_*/echo_email_batch
(eeb_user_id,eeb_event_id);
CREATE INDEX /*i*/echo_email_batch_user_hash_priority ON /*_*/echo_email_batch
(eeb_user_id, eeb_event_hash, eeb_event_priority);
+
+CREATE TABLE /*_*/echo_target_page (
+ etp_user int unsigned not null default 0,
+ etp_page int unsigned not null default 0,
+ etp_event int unsigned not null default 0
+) /*$wgDBTableOptions*/;
+
+CREATE UNIQUE INDEX /*i*/echo_target_page_user_event ON /*_*/echo_target_page
(etp_user, etp_event);
+CREATE INDEX /*i*/echo_target_page_user_page_event ON /*_*/echo_target_page
(etp_user, etp_page, etp_event);
diff --git a/includes/mapper/TargetPageMapper.php
b/includes/mapper/TargetPageMapper.php
new file mode 100644
index 0000000..9083e44
--- /dev/null
+++ b/includes/mapper/TargetPageMapper.php
@@ -0,0 +1,162 @@
+<?php
+
+/**
+ * Database mapper for EchoTargetPage model
+ */
+class EchoTargetPageMapper extends EchoAbstractMapper {
+
+ /**
+ * List of db fields used to construct an EchoTargetPage model
+ * @var string[]
+ */
+ protected static $fields = array(
+ 'etp_user',
+ 'etp_page',
+ 'etp_event'
+ );
+
+ /**
+ * Fetch an EchoTargetPage instance by user & page_id
+ *
+ * @param User $user
+ * @param int $pageId
+ * @return EchoTargetPage[]|boolean
+ */
+ public function fetchByUserPageId( User $user, $pageId ) {
+ $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
+
+ $res = $dbr->select(
+ array( 'echo_target_page' ),
+ self::$fields,
+ array(
+ 'etp_user' => $user->getId(),
+ 'etp_page' => $pageId
+ ),
+ __METHOD__
+ );
+ if ( $res ) {
+ $targetPages = array();
+ foreach ( $res as $row ) {
+ $targetPages[] = EchoTargetPage::newFromRow(
$row );
+ }
+ return $targetPages;
+ } else {
+ return false;
+ }
+ }
+
+ /**
+ * Fetch EchoTargetPage records by user and set of event_id
+ *
+ * @param User $user
+ * @param int[] $eventIds
+ * @return EchoTargetPage[]|boolean
+ */
+ public function fetchByUserEventIds( User $user, array $eventIds ) {
+ if ( !$eventIds ) {
+ return array();
+ }
+ $dbr = $this->dbFactory->getEchoDb( DB_SLAVE );
+
+ $res = $dbr->select(
+ array( 'echo_target_page' ),
+ self::$fields,
+ array(
+ 'etp_user' => $user->getId(),
+ 'etp_event' => $eventIds
+ ),
+ __METHOD__
+ );
+ if ( $res ) {
+ $targetPages = array();
+ foreach ( $res as $row ) {
+ $targetPages[] = EchoTargetPage::newFromRow(
$row );
+ }
+ return $targetPages;
+ } else {
+ return false;
+ }
+ }
+
+ /**
+ * Insert an EchoTargetPage instance into the database
+ *
+ * @param EchoTargetPage $targetPage
+ * @return boolean
+ */
+ public function insert( EchoTargetPage $targetPage ) {
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+
+ $row = $targetPage->toDbArray();
+
+ $res = $dbw->insert( 'echo_target_page', $row, __METHOD__ );
+
+ return $res;
+ }
+
+ /**
+ * Delete an EchoTargetPage instance from the database
+ *
+ * @param EchoTargetPage
+ * @return boolean
+ */
+ public function delete( EchoTargetPage $targetPage ) {
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+
+ $res = $dbw->delete(
+ 'echo_target_page',
+ array(
+ 'etp_user' => $targetPage->getUser()->getId(),
+ 'etp_page' => $targetPage->getPageId(),
+ 'etp_event' => $targetPage->getEventId()
+ ),
+ __METHOD__
+ );
+ return $res;
+ }
+
+ /**
+ * Delete multiple EchoTargetPage records by user & set of event_id
+ *
+ * @param User $user
+ * @param int[] $eventIds
+ * @return boolean
+ */
+ public function deleteByUserEvents( User $user, array $eventIds ) {
+ if ( !$eventIds ) {
+ return true;
+ }
+
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+
+ $res = $dbw->delete(
+ 'echo_target_page',
+ array(
+ 'etp_user' => $user->getId(),
+ 'etp_event' => $eventIds
+ ),
+ __METHOD__
+ );
+ return $res;
+ }
+
+ /**
+ * Delete multiple EchoTargetPage records by user
+ *
+ * @param User $user
+ * @return boolean
+ */
+ public function deleteByUser( User $user ) {
+ $dbw = $this->dbFactory->getEchoDb( DB_MASTER );
+
+ $res = $dbw->delete(
+ 'echo_target_page',
+ array(
+ 'etp_user' => $user->getId()
+ ),
+ __METHOD__
+ );
+ return $res;
+ }
+
+}
diff --git a/model/TargetPage.php b/model/TargetPage.php
new file mode 100644
index 0000000..cc84660
--- /dev/null
+++ b/model/TargetPage.php
@@ -0,0 +1,139 @@
+<?php
+
+/**
+ * Map a title to an echo notification so that we can mark a notification as
read
+ * when visiting the page. This only supports titles with ids because majority
+ * of notifications have page_id and searching by namespace and title is slow
+ */
+class EchoTargetPage extends EchoAbstractEntity {
+
+ /**
+ * @var User
+ */
+ protected $user;
+
+ /**
+ * @var Title|null
+ */
+ protected $title;
+
+ /**
+ * @var int
+ */
+ protected $pageId;
+
+ /**
+ * @var EchoEvent|null
+ */
+ protected $event;
+
+ /**
+ * @var int
+ */
+ protected $eventId;
+
+ /**
+ * Only allow creating instance internally
+ */
+ protected function __construct() {}
+
+ /**
+ * Create a EchoTargetPage instance from User, Title and EchoEvent
+ *
+ * @param User $user
+ * @param Title $title
+ * @param EchoEvent $event
+ * @return TargetPage|null
+ */
+ public static function create( User $user, Title $title, EchoEvent
$event ) {
+ // This only support title with a page_id
+ if ( $user->isAnon() || !$title->getArticleID() ) {
+ return null;
+ }
+ $obj = new self();
+ $obj->user = $user;
+ $obj->event = $event;
+ $obj->eventId = $event->getId();
+ $obj->title = $title;
+ $obj->pageId = $title->getArticleID();
+ return $obj;
+ }
+
+ /**
+ * Create a EchoTargetPage instance from stdClass object
+ *
+ * @param stdClass $row
+ * @return EchoTargetPage
+ * @throws MWException
+ */
+ public static function newFromRow( $row ) {
+ $requiredFields = array (
+ 'etp_user',
+ 'etp_page',
+ 'etp_event'
+ );
+ foreach ( $requiredFields as $field ) {
+ if ( !isset( $row->$field ) || !$row->$field ) {
+ throw new MWException( $field . ' is not set in
the row!' );
+ }
+ }
+ $obj = new self();
+ $obj->user = User::newFromId( $row->etp_user );
+ $obj->pageId = $row->etp_page;
+ $obj->eventId = $row->etp_event;
+ return $obj;
+ }
+
+ /**
+ * @return User
+ */
+ public function getUser() {
+ return $this->user;
+ }
+
+ /**
+ * @return Title|null
+ */
+ public function getTitle() {
+ if ( !$this->title ) {
+ $this->title = Title::newFromId( $this->pageId );
+ }
+ return $this->title;
+ }
+
+ /**
+ * @return int
+ */
+ public function getPageId() {
+ return $this->pageId;
+ }
+
+ /**
+ * @return EchoEvent
+ */
+ public function getEvent() {
+ if ( !$this->event ) {
+ $this->event = EchoEvent::newFromID( $this->eventId );
+ }
+ return $this->event;
+ }
+
+ /**
+ * @return int
+ */
+ public function getEventId() {
+ return $this->eventId;
+ }
+
+ /**
+ * Convert the properties to a database row
+ * @return array
+ */
+ public function toDbArray() {
+ return array (
+ 'etp_user' => $this->user->getId(),
+ 'etp_page' => $this->pageId,
+ 'etp_event' => $this->eventId
+ );
+ }
+}
diff --git a/tests/includes/mapper/TargetPageMapperTest.php
b/tests/includes/mapper/TargetPageMapperTest.php
new file mode 100644
index 0000000..dcf9b35
--- /dev/null
+++ b/tests/includes/mapper/TargetPageMapperTest.php
@@ -0,0 +1,190 @@
+<?php
+
+class EchoTargetPageMapperTest extends MediaWikiTestCase {
+
+ public function testFetchByUserPageId() {
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array ( 'select' => false ) ) );
+ $res = $targetMapper->fetchByUserPageId( User::newFromId( 1 ),
1 );
+ $this->assertFalse( $res );
+
+ $dbResult = array (
+ (object)array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 3
+ ),
+ (object)array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 7,
+ )
+ );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array ( 'select' => $dbResult ) ) );
+ $res = $targetMapper->fetchByUserPageId( User::newFromId( 1 ),
2 );
+ $this->assertTrue( is_array( $res ) );
+ foreach ( $res as $row ) {
+ $this->assertInstanceOf( 'EchoTargetPage', $row );
+ }
+ }
+
+ public function testFetchByUserEventIds() {
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array ( 'select' => false ) ) );
+ $res = $targetMapper->fetchByUserEventIds( User::newFromId( 1
), array( 1, 2, 3 ) );
+ $this->assertFalse( $res );
+
+ $dbResult = array (
+ (object)array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 2
+ ),
+ (object)array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 3,
+ )
+ );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( array ( 'select' => $dbResult ) ) );
+ $res = $targetMapper->fetchByUserEventIds( User::newFromId( 1
), array( 1, 2, 3 ) );
+ $this->assertTrue( is_array( $res ) );
+ $this->assertCount( 2, $res );
+ foreach ( $res as $row ) {
+ $this->assertInstanceOf( 'EchoTargetPage', $row );
+ }
+ }
+
+ public function provideDataTestInsert() {
+ return array (
+ array (
+ 'successful insert with next sequence = 1',
+ array ( 'nextSequenceValue' => 1, 'insert' =>
true, 'insertId' => 2 ),
+ 1
+ ),
+ array (
+ 'successful insert with insert id = 2',
+ array ( 'nextSequenceValue' => null, 'insert'
=> true, 'insertId' => 2 ),
+ 2
+ ),
+ array (
+ 'unsuccessful insert',
+ array ( 'nextSequenceValue' => null, 'insert'
=> false, 'insertId' => 2 ),
+ false
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideDataTestInsert
+ */
+ public function testInsert( $message, $dbResult, $result ) {
+ $target = $this->mockEchoTargetPage();
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
+ $this->assertEquals( $result, $targetMapper->insert( $target ),
$message );
+ }
+
+ public function testDelete() {
+ $dbResult = array( 'delete' => true );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
+ $this->assertTrue( $targetMapper->delete(
$this->mockEchoTargetPage() ) );
+
+ $dbResult = array( 'delete' => false );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
+ $this->assertFalse( $targetMapper->delete(
$this->mockEchoTargetPage() ) );
+ }
+
+ public function provideDataTestDeleteByUserEvents() {
+ return array(
+ array( true, array( 1 ), true ),
+ array( false, array( 1 ), false ),
+ array( true, array(), true ),
+ array( false, array(), true ),
+ );
+ }
+
+ /**
+ * @dataProvider provideDataTestDeleteByUserEvents
+ */
+ public function testDeleteByUserEvents( $deleteResult, $eventIds,
$result ) {
+ $dbResult = array( 'delete' => $deleteResult );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
+ $this->assertSame( $targetMapper->deleteByUserEvents(
User::newFromId( 1 ), $eventIds ), $result );
+ }
+
+ public function testDeleteByUser() {
+ $dbResult = array( 'delete' => true );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
+ $this->assertSame( $targetMapper->deleteByUser(
User::newFromId( 1 ) ), true );
+
+ $dbResult = array( 'delete' => false );
+ $targetMapper = new EchoTargetPageMapper(
$this->mockMWEchoDbFactory( $dbResult ) );
+ $this->assertSame( $targetMapper->deleteByUser(
User::newFromId( 1 ) ), false );
+ }
+
+ /**
+ * Mock object of EchoTargetPage
+ */
+ protected function mockEchoTargetPage() {
+ $target = $this->getMockBuilder( 'EchoTargetPage' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $target->expects( $this->any() )
+ ->method( 'toDbArray' )
+ ->will( $this->returnValue( array() ) );
+ $target->expects( $this->any() )
+ ->method( 'getUser' )
+ ->will( $this->returnValue( User::newFromId( 1 ) ) );
+ $target->expects( $this->any() )
+ ->method( 'getPageId' )
+ ->will( $this->returnValue( 2 ) );
+ $target->expects( $this->any() )
+ ->method( 'getEventId' )
+ ->will( $this->returnValue( 3 ) );
+ return $target;
+ }
+
+ /**
+ * Mock object of MWEchoDbFactory
+ */
+ protected function mockMWEchoDbFactory( $dbResult ) {
+ $dbFactory = $this->getMockBuilder( 'MWEchoDbFactory' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $dbFactory->expects( $this->any() )
+ ->method( 'getEchoDb' )
+ ->will( $this->returnValue( $this->mockDb( $dbResult )
) );
+ return $dbFactory;
+ }
+
+ /**
+ * Mock object of DatabaseMysql ( DatabaseBase )
+ */
+ protected function mockDb( array $dbResult ) {
+ $dbResult += array(
+ 'nextSequenceValue' => '',
+ 'insert' => '',
+ 'insertId' => '',
+ 'select' => '',
+ 'delete' => ''
+ );
+ $db = $this->getMockBuilder( 'DatabaseMysql' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $db->expects( $this->any() )
+ ->method( 'nextSequenceValue' )
+ ->will( $this->returnValue(
$dbResult['nextSequenceValue'] ) );
+ $db->expects( $this->any() )
+ ->method( 'insert' )
+ ->will( $this->returnValue( $dbResult['insert'] ) );
+ $db->expects( $this->any() )
+ ->method( 'insertId' )
+ ->will( $this->returnValue( $dbResult['insertId'] ) );
+ $db->expects( $this->any() )
+ ->method( 'select' )
+ ->will( $this->returnValue( $dbResult['select'] ) );
+ $db->expects( $this->any() )
+ ->method( 'delete' )
+ ->will( $this->returnValue( $dbResult['delete'] ) );
+ return $db;
+ }
+
+}
diff --git a/tests/model/TargetPageTest.php b/tests/model/TargetPageTest.php
new file mode 100644
index 0000000..29670ff
--- /dev/null
+++ b/tests/model/TargetPageTest.php
@@ -0,0 +1,99 @@
+<?php
+
+class EchoTargetPageTest extends MediaWikiTestCase {
+
+ public function testCreate() {
+ $this->assertNull(
+ EchoTargetPage::create(
+ User::newFromId( 0 ),
+ $this->mockTitle( 1 ),
+ $this->mockEchoEvent()
+ )
+ );
+
+ $this->assertNull(
+ EchoTargetPage::create(
+ User::newFromId( 1 ),
+ $this->mockTitle( 0 ),
+ $this->mockEchoEvent()
+ )
+ );
+
+ $this->assertNull(
+ EchoTargetPage::create(
+ User::newFromId( 0 ),
+ $this->mockTitle( 0 ),
+ $this->mockEchoEvent()
+ )
+ );
+
+ $this->assertInstanceOf(
+ 'EchoTargetPage',
+ EchoTargetPage::create(
+ User::newFromId( 1 ),
+ $this->mockTitle( 1 ),
+ $this->mockEchoEvent()
+ )
+ );
+ }
+
+ public function testNewFromRow() {
+ $row = (object) array (
+ 'etp_user' => 1,
+ 'etp_page' => 2,
+ 'etp_event' => 3
+ );
+ $obj = EchoTargetPage::newFromRow( $row );
+ $this->assertInstanceOf( 'EchoTargetPage', $obj );
+ return $obj;
+ }
+
+ /**
+ * @expectedException MWException
+ */
+ public function testNewFromRowWithException() {
+ $row = (object) array (
+ 'etp_page' => 2,
+ 'etp_event' => 3
+ );
+ $this->assertInstanceOf( 'EchoTargetPage',
EchoTargetPage::newFromRow( $row ) );
+ }
+
+ /**
+ * @depends testNewFromRow
+ */
+ public function testToDbArray( $obj ) {
+ $row = $obj->toDbArray();
+ $this->assertTrue( is_array( $row ) );
+ $this->assertArrayHasKey( 'etp_user', $row );
+ $this->assertArrayHasKey( 'etp_page', $row );
+ $this->assertArrayHasKey( 'etp_event', $row );
+ }
+
+ /**
+ * Mock object of Title
+ */
+ protected function mockTitle( $pageId ) {
+ $event = $this->getMockBuilder( 'Title' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $event->expects( $this->any() )
+ ->method( 'getArticleID' )
+ ->will( $this->returnValue( $pageId ) );
+ return $event;
+ }
+
+ /**
+ * Mock object of EchoEvent
+ */
+ protected function mockEchoEvent( $eventId = 1 ) {
+ $event = $this->getMockBuilder( 'EchoEvent' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $event->expects( $this->any() )
+ ->method( 'getId' )
+ ->will( $this->returnValue( $eventId ) );
+ return $event;
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/153651
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I605cbc79adfc12d22bd889c5bb513d05c479fe6e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: two_tabs
Gerrit-Owner: Bsitu <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
