Mwjames has uploaded a new change for review.
https://gerrit.wikimedia.org/r/95176
Change subject: Remove MW dependency from PredefinedPropertyAnnotator
......................................................................
Remove MW dependency from PredefinedPropertyAnnotator
Using a PredefinedPropertyAdapter to encapsulate necessary MW objects
Change-Id: Icd2c39394e5f37f8f78bfce147f562212a45d6f6
---
M SemanticMediaWiki.classes.php
A includes/PredefinedPropertyAdapter.php
M includes/annotator/PredefinedPropertyAnnotator.php
M includes/dic/SharedDependencyContainer.php
A tests/phpunit/includes/PredefinedPropertyAdapterTest.php
M tests/phpunit/includes/annotator/ChainablePropertyAnnotatorTest.php
M tests/phpunit/includes/annotator/PredefinedPropertyAnnotatorTest.php
M tests/phpunit/mocks/MockObjectRepository.php
8 files changed, 332 insertions(+), 114 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/SemanticMediaWiki
refs/changes/76/95176/1
diff --git a/SemanticMediaWiki.classes.php b/SemanticMediaWiki.classes.php
index b719538..b5952c1 100644
--- a/SemanticMediaWiki.classes.php
+++ b/SemanticMediaWiki.classes.php
@@ -47,6 +47,7 @@
'SMW\SimpleDictionary' => 'includes/SimpleDictionary.php',
'SMW\StoreUpdater' => 'includes/StoreUpdater.php',
'SMW\LazyDBConnectionProvider' =>
'includes/LazyDBConnectionProvider.php',
+ 'SMW\PredefinedPropertyAdapter' =>
'includes/PredefinedPropertyAdapter.php',
// Annotator
'SMW\PropertyAnnotator' =>
'includes/annotator/PropertyAnnotator.php',
diff --git a/includes/PredefinedPropertyAdapter.php
b/includes/PredefinedPropertyAdapter.php
new file mode 100644
index 0000000..8067f9e
--- /dev/null
+++ b/includes/PredefinedPropertyAdapter.php
@@ -0,0 +1,85 @@
+<?php
+
+namespace SMW;
+
+use WikiPage;
+use Revision;
+use User;
+
+/**
+ * Adapter class to provide access to MediaWiki objects relevant for the
+ * predefined property annotation process
+ *
+ * @ingroup SMW
+ *
+ * @licence GNU GPL v2+
+ * @since 1.9
+ *
+ * @author mwjames
+ */
+class PredefinedPropertyAdapter {
+
+ /** @var WikiPage */
+ protected $wikiPage;
+
+ /** @var Revision */
+ protected $revision;
+
+ /** @var User */
+ protected $user;
+
+ /**
+ * @since 1.9
+ *
+ * @param WikiPage $wikiPage
+ * @param Revision $revision
+ * @param User $user
+ */
+ public function __construct( WikiPage $wikiPage, Revision $revision,
User $user ) {
+ $this->wikiPage = $wikiPage;
+ $this->revision = $revision;
+ $this->user = $user;
+ }
+
+ /**
+ * @since 1.9
+ *
+ * @return integer
+ */
+ public function getModificationDate() {
+ return $this->wikiPage->getTimestamp();
+ }
+
+ /**
+ * @note getFirstRevision() is expensive as it initiates a read on the
+ * revision table which is not cached
+ *
+ * @since 1.9
+ *
+ * @return integer
+ */
+ public function getCreationDate() {
+
return$this->wikiPage->getTitle()->getFirstRevision()->getTimestamp();
+ }
+
+ /**
+ * @note Using isNewPage() is expensice due to access to the database
+ *
+ * @since 1.9
+ *
+ * @return boolean
+ */
+ public function isNewPage() {
+ return $this->revision->getParentId() !== '';
+ }
+
+ /**
+ * @since 1.9
+ *
+ * @return Title
+ */
+ public function getLastEditor() {
+ return $this->user->getUserPage();
+ }
+
+}
\ No newline at end of file
diff --git a/includes/annotator/PredefinedPropertyAnnotator.php
b/includes/annotator/PredefinedPropertyAnnotator.php
index b46534a..b3c4e71 100644
--- a/includes/annotator/PredefinedPropertyAnnotator.php
+++ b/includes/annotator/PredefinedPropertyAnnotator.php
@@ -7,10 +7,6 @@
use SMWDIBoolean as DIBoolean;
use SMWDITime as DITime;
-use WikiPage;
-use Revision;
-use User;
-
/**
* Handling predefined property annotations
*
@@ -23,28 +19,18 @@
*/
class PredefinedPropertyAnnotator extends PropertyAnnotatorDecorator {
- /** @var WikiPage */
- protected $wikiPage;
-
- /** @var Revision */
- protected $revision;
-
- /** @var User */
- protected $user;
+ /** @var PredefinedPropertyAdapter */
+ protected $predefinedProperty;
/**
* @since 1.9
*
* @param PropertyAnnotator $propertyAnnotator
- * @param WikiPage $wikiPage
- * @param Revision $revision
- * @param User $user
+ * @param PredefinedPropertyAdapter $predefinedProperty
*/
- public function __construct( PropertyAnnotator $propertyAnnotator,
WikiPage $wikiPage, Revision $revision, User $user ) {
+ public function __construct( PropertyAnnotator $propertyAnnotator,
PredefinedPropertyAdapter $predefinedProperty ) {
parent::__construct( $propertyAnnotator );
- $this->wikiPage = $wikiPage;
- $this->revision = $revision;
- $this->user = $user;
+ $this->predefinedProperty = $predefinedProperty;
}
/**
@@ -59,41 +45,18 @@
foreach ( $predefinedProperties as $propertyId ) {
- // Ensure that only special properties are added that
are registered
- // and only added once
- if ( ( DIProperty::getPredefinedPropertyTypeId(
$propertyId ) === '' ) ||
- ( array_key_exists( $propertyId,
$cachedProperties ) ) ) {
+ if ( $this->assertRegisteredPropertyId( $propertyId,
$cachedProperties ) ) {
continue;
}
$propertyDI = new DIProperty( $propertyId );
- // Don't do a double round
if ( $this->getSemanticData()->getPropertyValues(
$propertyDI ) !== array() ) {
$cachedProperties[ $propertyId ] = true;
continue;
}
- switch ( $propertyId ) {
- case DIProperty::TYPE_MODIFICATION_DATE :
- $dataItem = DITime::newFromTimestamp(
$this->wikiPage->getTimestamp() );
- break;
- case DIProperty::TYPE_CREATION_DATE :
- // Expensive getFirstRevision()
initiates a revision table
- // read and is not cached
- $dataItem = DITime::newFromTimestamp(
-
$this->wikiPage->getTitle()->getFirstRevision()->getTimestamp()
- );
- break;
- case DIProperty::TYPE_NEW_PAGE :
- // Expensive isNewPage() does a
database read
- // $dataValue = new SMWDIBoolean(
$this->title->isNewPage() );
- $dataItem = new DIBoolean(
$this->revision->getParentId() !== '' );
- break;
- case DIProperty::TYPE_LAST_EDITOR :
- $dataItem = DIWikiPage::newFromTitle(
$this->user->getUserPage() );
- break;
- }
+ $dataItem = $this->createDataItemEntry( $propertyId );
if ( $dataItem instanceof DataItem ) {
$cachedProperties[ $propertyId ] = true;
@@ -106,4 +69,37 @@
return $this;
}
+ /**
+ * @since 1.9
+ */
+ protected function assertRegisteredPropertyId( $propertyId,
$cachedProperties ) {
+ return ( DIProperty::getPredefinedPropertyTypeId( $propertyId )
=== '' ) ||
+ array_key_exists( $propertyId, $cachedProperties );
+ }
+
+ /**
+ * @since 1.9
+ */
+ protected function createDataItemEntry( $propertyId ) {
+
+ $dataItem = null;
+
+ switch ( $propertyId ) {
+ case DIProperty::TYPE_MODIFICATION_DATE :
+ $dataItem = DITime::newFromTimestamp(
$this->predefinedProperty->getModificationDate() );
+ break;
+ case DIProperty::TYPE_CREATION_DATE :
+ $dataItem = DITime::newFromTimestamp(
$this->predefinedProperty->getCreationDate() );
+ break;
+ case DIProperty::TYPE_NEW_PAGE :
+ $dataItem = new DIBoolean(
$this->predefinedProperty->isNewPage() );
+ break;
+ case DIProperty::TYPE_LAST_EDITOR :
+ $dataItem = DIWikiPage::newFromTitle(
$this->predefinedProperty->getLastEditor() );
+ break;
+ }
+
+ return $dataItem;
+ }
+
}
diff --git a/includes/dic/SharedDependencyContainer.php
b/includes/dic/SharedDependencyContainer.php
index 5b36fd2..38eedae 100644
--- a/includes/dic/SharedDependencyContainer.php
+++ b/includes/dic/SharedDependencyContainer.php
@@ -422,8 +422,6 @@
}
/**
- * PredefinedPropertyAnnotator object definition
- *
* @since 1.9
*
* @return PredefinedPropertyAnnotator
@@ -433,12 +431,13 @@
$annotator = $builder->newObject(
'NullPropertyAnnotator' );
- return new PredefinedPropertyAnnotator(
- $annotator,
+ $adapter = new PredefinedPropertyAdapter(
$builder->getArgument( 'WikiPage' ),
$builder->getArgument( 'Revision' ),
$builder->getArgument( 'User' )
);
+
+ return new PredefinedPropertyAnnotator( $annotator,
$adapter );
};
}
diff --git a/tests/phpunit/includes/PredefinedPropertyAdapterTest.php
b/tests/phpunit/includes/PredefinedPropertyAdapterTest.php
new file mode 100644
index 0000000..baab247
--- /dev/null
+++ b/tests/phpunit/includes/PredefinedPropertyAdapterTest.php
@@ -0,0 +1,162 @@
+<?php
+
+namespace SMW\Test;
+
+use SMW\PredefinedPropertyAdapter;
+
+/**
+ * @covers \SMW\PredefinedPropertyAdapter
+ *
+ * @ingroup Test
+ *
+ * @group SMW
+ * @group SMWExtension
+ *
+ * @licence GNU GPL v2+
+ * @since 1.9
+ *
+ * @author mwjames
+ */
+class PredefinedPropertyAdapterTest extends SemanticMediaWikiTestCase {
+
+ /**
+ * @return string|false
+ */
+ public function getClass() {
+ return '\SMW\PredefinedPropertyAdapter';
+ }
+
+ /**
+ * @since 1.9
+ *
+ * @return PredefinedPropertyAdapter
+ */
+ private function newInstance( $wikiPage = null, $revision = null, $user
= null ) {
+
+ if ( $wikiPage === null ) {
+ $wikiPage = $this->newMockBuilder()->newObject(
'WikiPage' );
+ }
+
+ if ( $revision === null ) {
+ $revision = $this->newMockBuilder()->newObject(
'Revision' );
+ }
+
+ if ( $user === null ) {
+ $user = $this->newMockBuilder()->newObject( 'User' );
+ }
+
+ return new PredefinedPropertyAdapter( $wikiPage, $revision,
$user );
+ }
+
+ /**
+ * @since 1.9
+ */
+ public function testConstructor() {
+ $this->assertInstanceOf( $this->getClass(),
$this->newInstance() );
+ }
+
+ /**
+ * @dataProvider instanceDataProvider
+ *
+ * @since 1.9
+ */
+ public function testMethodAccess( array $setup, $expected ) {
+
+ $method = $setup['method'];
+
+ $instance = $this->newInstance(
+ $this->newMockBuilder()->newObject( 'WikiPage',
$setup['wikiPage'] ),
+ $this->newMockBuilder()->newObject( 'Revision',
$setup['revision'] ),
+ $this->newMockBuilder()->newObject( 'User',
$setup['user'] )
+ );
+
+ $this->assertEquals(
+ $expected['result'],
+ $instance->{ $method }(),
+ "Asserts that {$method} returns an expected result"
+ );
+
+ }
+
+ /**
+ * @return array
+ */
+ public function instanceDataProvider() {
+
+ $provider = array();
+
+ // TYPE_MODIFICATION_DATE
+ $provider[] = array(
+ array(
+ 'wikiPage' => array( 'getTimestamp' =>
1272508903 ),
+ 'revision' => array(),
+ 'user' => array(),
+ 'method' => 'getModificationDate'
+ ),
+ array(
+ 'result' => 1272508903
+ )
+ );
+
+ // TYPE_CREATION_DATE
+ $revision = $this->newMockBuilder()->newObject( 'Revision',
array(
+ 'getTimestamp' => 1272508903
+ ) );
+
+ $title = $this->newMockBuilder()->newObject( 'Title', array(
+ 'getDBkey' => 'Lula',
+ 'getNamespace' => NS_MAIN,
+ 'getFirstRevision' => $revision
+ ) );
+
+ $subject = $this->newMockBuilder()->newObject( 'DIWikiPage',
array(
+ 'getTitle' => $this->newMockBuilder()->newObject(
'Title' )
+ ) );
+
+ $provider[] = array(
+ array(
+ 'wikiPage' => array( 'getTitle' => $title ),
+ 'revision' => array(),
+ 'user' => array(),
+ 'method' => 'getCreationDate'
+ ),
+ array(
+ 'result' => 1272508903
+ )
+ );
+
+ // TYPE_NEW_PAGE
+ $provider[] = array(
+ array(
+ 'wikiPage' => array(),
+ 'revision' => array( 'getParentId' => 9001 ),
+ 'user' => array(),
+ 'method' => 'isNewPage'
+ ),
+ array(
+ 'result' => true
+ )
+ );
+
+ // TYPE_LAST_EDITOR
+ $userPage = $this->newMockBuilder()->newObject( 'Title', array(
+ 'getDBkey' => 'Lula',
+ 'getNamespace' => NS_USER,
+ ) );
+
+ $provider[] = array(
+ array(
+ 'wikiPage' => array(),
+ 'revision' => array(),
+ 'user' => array( 'getUserPage' => $userPage
),
+ 'method' => 'getLastEditor'
+ ),
+ array(
+ 'result' => $userPage
+ )
+ );
+
+ return $provider;
+ }
+
+}
diff --git
a/tests/phpunit/includes/annotator/ChainablePropertyAnnotatorTest.php
b/tests/phpunit/includes/annotator/ChainablePropertyAnnotatorTest.php
index 6d47557..8d85d01 100644
--- a/tests/phpunit/includes/annotator/ChainablePropertyAnnotatorTest.php
+++ b/tests/phpunit/includes/annotator/ChainablePropertyAnnotatorTest.php
@@ -63,9 +63,7 @@
$instance = new PredefinedPropertyAnnotator(
$instance,
- $this->newMockBuilder()->newObject( 'WikiPage',
$setup['wikiPage'] ),
- $this->newMockBuilder()->newObject( 'Revision',
$setup['revision'] ),
- $this->newMockBuilder()->newObject( 'User',
$setup['user'] )
+ $this->newMockBuilder()->newObject(
'PredefinedPropertyAdapter', $setup['adapter'] )
);
$instance->addAnnotation();
@@ -91,9 +89,7 @@
'namespace' => NS_MAIN,
'categories' => array( 'Foo', 'Bar' ),
'sortkey' => 'Lala',
- 'wikiPage' => array( 'getTimestamp' =>
1272508903 ),
- 'revision' => array(),
- 'user' => array(),
+ 'adapter' => array( 'getModificationDate' =>
1272508903 ),
'settings' => array(
'smwgUseCategoryHierarchy' => false,
'smwgCategoriesAsInstances' => true,
diff --git
a/tests/phpunit/includes/annotator/PredefinedPropertyAnnotatorTest.php
b/tests/phpunit/includes/annotator/PredefinedPropertyAnnotatorTest.php
index e85c844..039f722 100644
--- a/tests/phpunit/includes/annotator/PredefinedPropertyAnnotatorTest.php
+++ b/tests/phpunit/includes/annotator/PredefinedPropertyAnnotatorTest.php
@@ -6,7 +6,6 @@
use SMW\NullPropertyAnnotator;
use SMW\EmptyContext;
use SMW\SemanticData;
-use SMW\DIWikiPage;
use SMW\DIProperty;
/**
@@ -50,23 +49,13 @@
*
* @return PredefinedPropertyAnnotator
*/
- private function newInstance( $semanticData = null, $settings =
array(), $wikiPage = null, $revision = null, $user = null ) {
+ private function newInstance( $semanticData = null, $settings =
array(), $adapter = array() ) {
if ( $semanticData === null ) {
$semanticData = $this->newMockBuilder()->newObject(
'SemanticData' );
}
- if ( $wikiPage === null ) {
- $wikiPage = $this->newMockBuilder()->newObject(
'WikiPage' );
- }
-
- if ( $revision === null ) {
- $revision = $this->newMockBuilder()->newObject(
'Revision' );
- }
-
- if ( $user === null ) {
- $user = $this->newMockBuilder()->newObject( 'User' );
- }
+ $predefinedProperty = $this->newMockBuilder()->newObject(
'PredefinedPropertyAdapter', $adapter );
$settings = $this->newSettings( $settings );
@@ -74,12 +63,8 @@
$context->getDependencyBuilder()->getContainer()->registerObject( 'Settings',
$settings );
return new PredefinedPropertyAnnotator(
- new NullPropertyAnnotator( $semanticData, $context ),
- $wikiPage,
- $revision,
- $user
+ new NullPropertyAnnotator( $semanticData, $context ),
$predefinedProperty
);
-
}
/**
@@ -98,14 +83,7 @@
$semanticData = new SemanticData( $setup['subject'] );
- $instance = $this->newInstance(
- $semanticData,
- $setup['settings'],
- $this->newMockBuilder()->newObject( 'WikiPage',
$setup['wikiPage'] ),
- $this->newMockBuilder()->newObject( 'Revision',
$setup['revision'] ),
- $this->newMockBuilder()->newObject( 'User',
$setup['user'] )
- );
-
+ $instance = $this->newInstance( $semanticData,
$setup['settings'], $setup['adapter'] );
$instance->attach( $this->newObserver() )->addAnnotation();
$this->assertSemanticData(
@@ -149,9 +127,7 @@
'settings' => array(
'smwgPageSpecialProperties' => array(
'Lala', '_Lula', '-Lila', '' )
),
- 'wikiPage' => array(),
- 'revision' => array(),
- 'user' => array()
+ 'adapter' => array(),
),
array(
'propertyCount' => 0,
@@ -165,9 +141,7 @@
'settings' => array(
'smwgPageSpecialProperties' => array(
DIProperty::TYPE_MODIFICATION_DATE )
),
- 'wikiPage' => array( 'getTimestamp' =>
1272508903 ),
- 'revision' => array(),
- 'user' => array()
+ 'adapter' => array( 'getModificationDate' =>
1272508903 )
),
array(
'propertyCount' => 1,
@@ -177,29 +151,13 @@
);
// TYPE_CREATION_DATE
- $revision = $this->newMockBuilder()->newObject( 'Revision',
array(
- 'getTimestamp' => 1272508903
- ) );
-
- $title = $this->newMockBuilder()->newObject( 'Title', array(
- 'getDBkey' => 'Lula',
- 'getNamespace' => NS_MAIN,
- 'getFirstRevision' => $revision
- ) );
-
- $subject = $this->newMockBuilder()->newObject( 'DIWikiPage',
array(
- 'getTitle' => $this->newMockBuilder()->newObject(
'Title' )
- ) );
-
$provider[] = array(
array(
- 'subject' => $subject,
+ 'subject' => $this->newSubject(
$this->newTitle() ),
'settings' => array(
'smwgPageSpecialProperties' => array(
DIProperty::TYPE_CREATION_DATE )
),
- 'wikiPage' => array( 'getTitle' => $title ),
- 'revision' => array(),
- 'user' => array()
+ 'adapter' => array( 'getCreationDate' =>
1272508903 )
),
array(
'propertyCount' => 1,
@@ -215,9 +173,7 @@
'settings' => array(
'smwgPageSpecialProperties' => array(
DIProperty::TYPE_NEW_PAGE )
),
- 'wikiPage' => array(),
- 'revision' => array( 'getParentId' => 9001 ),
- 'user' => array()
+ 'adapter' => array( 'isNewPage' => true )
),
array(
'propertyCount' => 1,
@@ -238,9 +194,7 @@
'settings' => array(
'smwgPageSpecialProperties' => array(
DIProperty::TYPE_LAST_EDITOR )
),
- 'wikiPage' => array(),
- 'revision' => array(),
- 'user' => array( 'getUserPage' => $userPage
)
+ 'adapter' => array( 'getLastEditor' =>
$userPage )
),
array(
'propertyCount' => 1,
@@ -249,16 +203,17 @@
)
);
- // Combine entries
+ // Combined entries
$provider[] = array(
array(
'subject' => $this->newSubject(
$this->newTitle() ),
'settings' => array(
'smwgPageSpecialProperties' => array(
'_MDAT', '_LEDT' )
),
- 'wikiPage' => array( 'getTimestamp' =>
1272508903 ),
- 'revision' => array(),
- 'user' => array( 'getUserPage' => $userPage
)
+ 'adapter' => array(
+ 'getModificationDate' => 1272508903,
+ 'getLastEditor' => $userPage
+ )
),
array(
'propertyCount' => 2,
diff --git a/tests/phpunit/mocks/MockObjectRepository.php
b/tests/phpunit/mocks/MockObjectRepository.php
index d248223..485438c 100644
--- a/tests/phpunit/mocks/MockObjectRepository.php
+++ b/tests/phpunit/mocks/MockObjectRepository.php
@@ -1104,4 +1104,28 @@
return $language;
}
+ /**
+ * @since 1.9
+ *
+ * @return PredefinedPropertyAdapter
+ */
+ public function PredefinedPropertyAdapter() {
+
+ $methods = $this->builder->getInvokedMethods();
+
+ $adapter = $this->getMockBuilder(
'SMW\PredefinedPropertyAdapter' )
+ ->disableOriginalConstructor()
+ ->getMock();
+
+ foreach ( $methods as $method ) {
+
+ $adapter->expects( $this->any() )
+ ->method( $method )
+ ->will( $this->builder->setCallback( $method )
);
+
+ }
+
+ return $adapter;
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/95176
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd2c39394e5f37f8f78bfce147f562212a45d6f6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <jamesin.hongkon...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
