EBernhardson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/168547
Change subject: Hygiene: Static analysis cleanups
......................................................................
Hygiene: Static analysis cleanups
Change-Id: I770e47cd92f4f854db7717b0ae484f49261edcf3
---
M container.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
M includes/Block/TopicSummary.php
M includes/Content/Content.php
M includes/Data/BufferedCache.php
M includes/Data/Index.php
M includes/Data/Index/BoardHistoryIndex.php
M includes/Data/Index/FeatureIndex.php
M includes/Data/Index/TopicHistoryIndex.php
M includes/Data/ObjectLocator.php
M includes/Data/RecentChanges/PostRevisionRecentChanges.php
M includes/Formatter/AbstractFormatter.php
M includes/Formatter/Contributions.php
M includes/Formatter/IRCLineUrlFormatter.php
M includes/Formatter/RecentChanges.php
M includes/Formatter/RevisionFormatter.php
M includes/Formatter/TopicFormatter.php
M includes/Model/UUID.php
M includes/Parsoid/Fixer/BadImageRemover.php
M includes/TalkpageManager.php
M includes/UrlGenerator.php
M tests/phpunit/Data/IndexTest.php
M tests/phpunit/Repository/TreeRepositoryTest.php
24 files changed, 109 insertions(+), 59 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/47/168547/1
diff --git a/container.php b/container.php
index 1a2d90a..00c70f0 100644
--- a/container.php
+++ b/container.php
@@ -39,9 +39,7 @@
} );
$c['url_generator'] = $c->share( function( $c ) {
- return new Flow\UrlGenerator(
- $c['occupation_controller']
- );
+ return new Flow\UrlGenerator();
} );
// listener is attached to storage.workflow, it
// notifies the url generator about all loaded workflows.
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index 2afbd0f..ca45dcc 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -410,7 +410,7 @@
'workflow' => $this->workflow,
'topic-title' => $this->loadTopicTitle(),
);
- if ( !$metadata['topic-title'] ) {
+ if ( !$metadata['topic-title'] instanceof PostRevision
) {
// permissions failure, should never have
gotten this far
throw new PermissionException( 'Not Allowed',
'insufficient-permission' );
}
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index 911552f..f5bd2fc 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -283,6 +283,7 @@
$postStorage = $this->storage->getStorage( 'PostRevision' );
return $pager->getPage( function( array $found ) use (
$postStorage ) {
$queries = array();
+ /** @var TopicListEntry[] $found */
foreach ( $found as $entry ) {
$queries[] = array( 'rev_type_id' =>
$entry->getId() );
}
diff --git a/includes/Block/TopicSummary.php b/includes/Block/TopicSummary.php
index cbf22d4..091407e 100644
--- a/includes/Block/TopicSummary.php
+++ b/includes/Block/TopicSummary.php
@@ -60,8 +60,8 @@
);
/**
- * @param string
- * @param User
+ * @param IContextSource $context
+ * @param string $action
*/
public function init( IContextSource $context, $action ) {
parent::init( $context, $action );
diff --git a/includes/Content/Content.php b/includes/Content/Content.php
index 3d3110b..d148076 100644
--- a/includes/Content/Content.php
+++ b/includes/Content/Content.php
@@ -56,7 +56,9 @@
$newRev = $occupationController->ensureFlowRevision(
$article, $loader->getWorkflow() );
if ( $newRev ) {
+ /** @noinspection PhpUndefinedFieldInspection */
$article->getPage()->mRevision = $newRev;
+ /** @noinspection PhpUndefinedFieldInspection */
$article->getPage()->mContentObject =
$newRev->getContent();
$contentObject = $newRev->getContent();
}
diff --git a/includes/Data/BufferedCache.php b/includes/Data/BufferedCache.php
index 0c50023..12c73f8 100644
--- a/includes/Data/BufferedCache.php
+++ b/includes/Data/BufferedCache.php
@@ -2,7 +2,7 @@
namespace Flow\Data;
-use BagOStuff;
+use Flow\Data\BagOStuff\BufferedBagOStuff;
use Closure;
/**
@@ -12,7 +12,7 @@
*/
class BufferedCache {
/**
- * @var BagOStuff
+ * @var BufferedBagOStuff
*/
protected $cache;
@@ -22,10 +22,10 @@
protected $exptime = 0;
/**
- * @param BagOStuff $cache The cache implementation to back this buffer
with
+ * @param BufferedBagOStuff $cache The cache implementation to back
this buffer with
* @param int $exptime The default length of time to cache data. 0 for
LRU.
*/
- public function __construct( BagOStuff $cache, $exptime = 0 ) {
+ public function __construct( BufferedBagOStuff $cache, $exptime = 0 ) {
$this->exptime = $exptime;
$this->cache = $cache;
}
@@ -103,6 +103,30 @@
}
/**
+ * Initiate a transaction: this will defer all writes to real cache
until
+ * commit() is called.
+ */
+ public function begin() {
+ $this->cache->begin();
+ }
+
+ /**
+ * Commits all deferred updates to real cache.
+ *
+ * @return bool
+ */
+ public function commit() {
+ return $this->cache->commit();
+ }
+
+ /**
+ * Roll back all scheduled changes.
+ */
+ public function rollback() {
+ $this->cache->rollback();
+ }
+
+ /**
* Catches all other method calls & passes them on to the real cache.
*
* @param string $name
diff --git a/includes/Data/Index.php b/includes/Data/Index.php
index ebf7f6c..ed05bb2 100644
--- a/includes/Data/Index.php
+++ b/includes/Data/Index.php
@@ -85,4 +85,10 @@
* greater than $offset
*/
function compareRowToOffset( array $row, $offset );
+
+ /**
+ * @param object $object
+ * @param array $row
+ */
+ function cachePurge( $object, array $row );
}
diff --git a/includes/Data/Index/BoardHistoryIndex.php
b/includes/Data/Index/BoardHistoryIndex.php
index 6300a19..4baeda6 100644
--- a/includes/Data/Index/BoardHistoryIndex.php
+++ b/includes/Data/Index/BoardHistoryIndex.php
@@ -4,6 +4,7 @@
use Flow\Container;
use Flow\Data\BufferedCache;
+use Flow\Data\ManagerGroup;
use Flow\Data\Storage\BoardHistoryStorage;
use Flow\Exception\DataModelException;
use Flow\Exception\InvalidInputException;
@@ -44,7 +45,8 @@
}
/**
- * {@inheritDoc}
+ * @param Header|PostRevision $object
+ * @param string[] $row
*/
public function cachePurge( $object, array $row ) {
$row['topic_list_id'] = $this->findTopicListId( $object, $row );
@@ -94,7 +96,10 @@
* Find a topic list id related to an abstract revision
*
* @param AbstractRevision $object
- * @return string|boolean False when object is not root post or topic
is not found
+ * @param array $row
+ * @return string|false Alphadecimal uid of the related board. False
when object is not root post or topic is not found
+ * @throws InvalidInputException when $object is not a Header,
PostRevision or
+ * PostSummary instance.
*/
protected function findTopicListId( AbstractRevision $object, array
$row ) {
if ( $object instanceof Header ) {
@@ -108,8 +113,9 @@
} else {
throw new InvalidInputException( 'Unexpected object
type: ' . get_class( $object ) );
}
-
- $found = Container::get( 'storage' )->find(
+ /** @var ManagerGroup $storage */
+ $storage = Container::get( 'storage' );
+ $found = $storage->find(
'TopicListEntry',
array( 'topic_id' => $post->getRootPost()->getPostId() )
);
diff --git a/includes/Data/Index/FeatureIndex.php
b/includes/Data/Index/FeatureIndex.php
index bab97b9..dcdd10f 100644
--- a/includes/Data/Index/FeatureIndex.php
+++ b/includes/Data/Index/FeatureIndex.php
@@ -267,11 +267,12 @@
*
* @param object $object
* @param array $row
+ * @throws DataModelException
*/
public function cachePurge( $object, array $row ) {
$indexed = ObjectManager::splitFromRow( $row, $this->indexed );
if ( !$indexed ) {
- throw new DataModelException( 'Un-indexable row: ' .
FormatJson::encode( $new ), 'process-data' );
+ throw new DataModelException( 'Un-indexable row: ' .
FormatJson::encode( $row ), 'process-data' );
}
// We don't want to just remove this object from the index,
then the index would be incorrect.
// We want to delete the bucket that contains this object.
diff --git a/includes/Data/Index/TopicHistoryIndex.php
b/includes/Data/Index/TopicHistoryIndex.php
index 8aace2d..4cb52df 100644
--- a/includes/Data/Index/TopicHistoryIndex.php
+++ b/includes/Data/Index/TopicHistoryIndex.php
@@ -27,7 +27,8 @@
}
/**
- * {@inheritDoc}
+ * @param PostRevision|PostSummary $object
+ * @param array $row
*/
public function cachePurge( $object, array $row ) {
$row['topic_root_id'] = $this->findTopicRootId( $object );
@@ -35,7 +36,7 @@
}
/**
- * @param PostRevision $object
+ * @param PostRevision|PostSummary $object
* @param string[] $new
* @param array $metadata
*/
@@ -45,7 +46,7 @@
}
/**
- * @param PostRevision $object
+ * @param PostRevision|PostSummary $object
* @param string[] $old
* @param string[] $new
* @param array $metadata
@@ -56,7 +57,7 @@
}
/**
- * @param PostRevision $object
+ * @param PostRevision|PostSummary $object
* @param string[] $old
* @param array $metadata
*/
@@ -68,6 +69,7 @@
/**
* @param PostRevision|PostSummary $object
* @return string alphadecimal uuid
+ * @throws InvalidInputException When $object is not PostRevision or
PostSummary
*/
protected function findTopicRootId( $object ) {
if ( $object instanceof PostRevision ) {
diff --git a/includes/Data/ObjectLocator.php b/includes/Data/ObjectLocator.php
index 4f8d8fb..ef4b011 100644
--- a/includes/Data/ObjectLocator.php
+++ b/includes/Data/ObjectLocator.php
@@ -268,6 +268,7 @@
*/
public function getIndexFor( array $keys, array $options = array() ) {
sort( $keys );
+ /** @var Index|null $current */
$current = null;
foreach ( $this->indexes as $index ) {
// @var Index $index
diff --git a/includes/Data/RecentChanges/PostRevisionRecentChanges.php
b/includes/Data/RecentChanges/PostRevisionRecentChanges.php
index c03e2b6..949b0d1 100644
--- a/includes/Data/RecentChanges/PostRevisionRecentChanges.php
+++ b/includes/Data/RecentChanges/PostRevisionRecentChanges.php
@@ -37,7 +37,7 @@
if ( !isset( $metadata['workflow'] ) ) {
throw new FlowException( 'Missing required metadata:
workflow' );
}
- if ( !isset( $metadata['topic-title'] ) ) {
+ if ( !isset( $metadata['topic-title'] ) ||
!$metadata['topic-title'] instanceof PostRevision ) {
throw new FlowException( 'Missing required metadata:
topic-title' );
}
diff --git a/includes/Formatter/AbstractFormatter.php
b/includes/Formatter/AbstractFormatter.php
index 8d74217..cc18d77 100644
--- a/includes/Formatter/AbstractFormatter.php
+++ b/includes/Formatter/AbstractFormatter.php
@@ -72,7 +72,7 @@
$anchor = null;
foreach ( $linkKeys as $linkKey ) {
if ( isset( $data['links'][$linkKey] ) ) {
- $anchor = $data['links'][$linkKey]->toHTML(
$formattedTime );
+ $anchor = $data['links'][$linkKey];
break;
}
}
diff --git a/includes/Formatter/Contributions.php
b/includes/Formatter/Contributions.php
index 7fe3866..1b9165e 100644
--- a/includes/Formatter/Contributions.php
+++ b/includes/Formatter/Contributions.php
@@ -38,6 +38,7 @@
* @param FormatterRow $row
* @param IContextSource $ctx
* @return string
+ * @throws FlowException
*/
protected function formatHtml( FormatterRow $row, IContextSource $ctx )
{
$this->serializer->setIncludeHistoryProperties( true );
diff --git a/includes/Formatter/IRCLineUrlFormatter.php
b/includes/Formatter/IRCLineUrlFormatter.php
index 6fe8058..9554a0d 100644
--- a/includes/Formatter/IRCLineUrlFormatter.php
+++ b/includes/Formatter/IRCLineUrlFormatter.php
@@ -133,6 +133,7 @@
throw new FlowException( 'Unknown revision type in
recent change row ' . $rc->getAttribute( 'rc_id' ) );
}
+ /** @noinspection PhpUndefinedMethodInspection */
return $class::fromStorageRow( $row + array(
'rev_id' => $change['revision'],
'rev_change_type' => $change['action'],
diff --git a/includes/Formatter/RecentChanges.php
b/includes/Formatter/RecentChanges.php
index 853471e..6f7f4ba 100644
--- a/includes/Formatter/RecentChanges.php
+++ b/includes/Formatter/RecentChanges.php
@@ -2,6 +2,7 @@
namespace Flow\Formatter;
+use Flow\Exception\FlowException;
use Flow\Model\PostRevision;
use Flow\Parsoid\Utils;
use ChangesList;
@@ -17,6 +18,7 @@
* @param IContextSource $ctx
* @param bool $linkOnly
* @return string|false Output line, or false on failure
+ * @throws FlowException
*/
public function format( RecentChangesRow $row, IContextSource $ctx,
$linkOnly = false ) {
if ( !$this->permissions->isAllowed( $row->revision,
'recentchanges' ) ) {
diff --git a/includes/Formatter/RevisionFormatter.php
b/includes/Formatter/RevisionFormatter.php
index 6ac5217..723872c 100644
--- a/includes/Formatter/RevisionFormatter.php
+++ b/includes/Formatter/RevisionFormatter.php
@@ -430,20 +430,19 @@
case 'thank':
if (
// thanks extension must be available
- !class_exists( 'ThanksHooks' ) ||
- // anons can't thank
- $user->isAnon() ||
+ class_exists( 'ThanksHooks' ) &&
+ // anons can't give a thank
+ !$user->isAnon() &&
// can only thank for PostRevisions
- // (other revision objects have mo
getCreator* methods)
- !$revision instanceof PostRevision ||
- // can't thank an anon user
- $revision->getCreatorIp() ||
+ // (other revision objects have no
getCreator* methods)
+ $revision instanceof PostRevision &&
+ // only thank a logged in user
+ $revision->getCreatorId() > 0 &&
// can't thank self
- $user->getId() ===
$revision->getCreatorId()
+ $user->getId() !==
$revision->getCreatorId()
) {
- continue;
+ $links['thank'] =
$this->urlGenerator->thankAction( $postId );
}
- $links['thank'] =
$this->urlGenerator->thankAction( $postId );
break;
case 'reply':
diff --git a/includes/Formatter/TopicFormatter.php
b/includes/Formatter/TopicFormatter.php
index a49c4ab..258a3db 100644
--- a/includes/Formatter/TopicFormatter.php
+++ b/includes/Formatter/TopicFormatter.php
@@ -99,7 +99,7 @@
}
/**
- * @param array Map from alphadecimal postId to list of alphadecimal
revisionId's
+ * @param array $posts Map from alphadecimal postId to list of
alphadecimal revisionId's
* for that postId contained within $revisions.
* @param array $revisions Map from alphadecimal revisionId to
serialized representation
* of that revision.
diff --git a/includes/Model/UUID.php b/includes/Model/UUID.php
index bffc080..19d6637 100644
--- a/includes/Model/UUID.php
+++ b/includes/Model/UUID.php
@@ -272,7 +272,7 @@
/**
* Returns the timestamp in the desired format (defaults to TS_MW)
*
- * @param string $format Desired format
+ * @param int $format Desired format (TS_MW, TS_UNIX, etc.)
* @return string
*/
public function getTimestamp( $format = TS_MW ) {
diff --git a/includes/Parsoid/Fixer/BadImageRemover.php
b/includes/Parsoid/Fixer/BadImageRemover.php
index 24d57ba..e590858 100644
--- a/includes/Parsoid/Fixer/BadImageRemover.php
+++ b/includes/Parsoid/Fixer/BadImageRemover.php
@@ -22,6 +22,7 @@
* // Before outputting content
* $content = $badImageRemover->apply( $foo->getContent(), $title );
*/
+
class BadImageRemover implements Fixer {
/**
* @var callable
@@ -29,7 +30,7 @@
protected $isFiltered;
/**
- * @var callable $isFiltered (string, Title) returning bool. First
+ * @param callable $isFiltered (string, Title) returning bool. First
* argument is the image name to check. Second argument is the page on
* which the image occurs. Returns true when the image should be
filtered.
*/
diff --git a/includes/TalkpageManager.php b/includes/TalkpageManager.php
index c8b9b2b..699c6de 100644
--- a/includes/TalkpageManager.php
+++ b/includes/TalkpageManager.php
@@ -91,7 +91,7 @@
if ( $doing ) {
return null;
}
- $doing = true;
+
// Comment to add to the Revision to indicate Flow taking over
$comment = '/* Taken over by Flow */';
@@ -99,30 +99,32 @@
$page = $article->getPage();
$revision = $page->getRevision();
- // Add a revision only if a Flow revision has not yet been
inserted.
- if (
- $revision === null ||
- $revision->getComment( Revision::RAW ) != $comment ||
- (
- ! $revision->getContent() instanceof
BoardContent ||
- ! $revision->getContent()->getWorkflowId()
- )
- ) {
- $status = $page->doEditContent(
- new BoardContent( 'flow-board', $workflow ),
- $comment,
- EDIT_FORCE_BOT | EDIT_SUPPRESS_RC,
- false,
- $this->getTalkpageManager()
- );
-
- if ( $status->isGood() && isset(
$status->value['revision'] ) ) {
- $doing = false;
- return $status->value['revision'];
+ if ( $revision !== null ) {
+ if ( $revision->getComment( Revision::RAW ) == $comment
) {
+ // Revision was created by this process
+ return null;
+ }
+ $content = $revision->getContent();
+ if ( $content instanceof BoardContent &&
$content->getWorkflowId() ) {
+ // Revision is already a valid BoardContent
+ return null;
}
}
+ $doing = true;
+ $status = $page->doEditContent(
+ new BoardContent( 'flow-board', $workflow ),
+ $comment,
+ EDIT_FORCE_BOT | EDIT_SUPPRESS_RC,
+ false,
+ $this->getTalkpageManager()
+ );
$doing = false;
+
+ if ( $status->isGood() && isset( $status->value['revision'] ) )
{
+ return $status->value['revision'];
+ }
+
return null;
}
diff --git a/includes/UrlGenerator.php b/includes/UrlGenerator.php
index 7e1a305..bb9701e 100644
--- a/includes/UrlGenerator.php
+++ b/includes/UrlGenerator.php
@@ -247,6 +247,7 @@
* @param UUID $workflowId
* @param UUID $oldRevId
* @return Anchor
+ * @throws FlowException When $revision is not PostRevision, Header or
PostSummary
*/
public function diffLink( AbstractRevision $revision, Title $title =
null, UUID $workflowId, UUID $oldRevId = null ) {
if ( $revision instanceof PostRevision ) {
diff --git a/tests/phpunit/Data/IndexTest.php b/tests/phpunit/Data/IndexTest.php
index c1c5a98..02f6949 100644
--- a/tests/phpunit/Data/IndexTest.php
+++ b/tests/phpunit/Data/IndexTest.php
@@ -3,6 +3,7 @@
namespace Flow\Tests\Data;
use Flow\Container;
+use Flow\Data\BagOStuff\BufferedBagOStuff;
use Flow\Data\BufferedCache;
use Flow\Data\Index\FeatureIndex;
use Flow\Data\Index\TopKIndex;
@@ -17,7 +18,7 @@
public function testShallow() {
global $wgFlowCacheTime;
- $bag = new \HashBagOStuff;
+ $bag = new BufferedBagOStuff( new \HashBagOStuff );
$cache = new BufferedCache( $bag, $wgFlowCacheTime );
// As we are only testing the cached result, storage should
never be called
@@ -62,7 +63,7 @@
public function testCompositeShallow() {
global $wgFlowCacheTime;
- $bag = new \HashBagOStuff;
+ $bag = new BufferedBagOStuff( new \HashBagOStuff );
$cache = new BufferedCache( $bag, $wgFlowCacheTime );
$storage = $this->getMock( 'Flow\\Data\\ObjectStorage' );
diff --git a/tests/phpunit/Repository/TreeRepositoryTest.php
b/tests/phpunit/Repository/TreeRepositoryTest.php
index 8a64d73..6c25d01 100644
--- a/tests/phpunit/Repository/TreeRepositoryTest.php
+++ b/tests/phpunit/Repository/TreeRepositoryTest.php
@@ -2,6 +2,7 @@
namespace Flow\Tests\Repository;
+use Flow\Data\BagOStuff\BufferedBagOStuff;
use Flow\Data\BufferedCache;
use Flow\Model\UUID;
use Flow\Repository\TreeRepository;
@@ -24,7 +25,7 @@
public function testSuccessfulInsert() {
global $wgFlowCacheTime;
- $cache = new BufferedCache( new \HashBagOStuff(),
$wgFlowCacheTime );
+ $cache = new BufferedCache( new BufferedBagOStuff( new
\HashBagOStuff() ), $wgFlowCacheTime );
$treeRepository = new TreeRepository( $this->mockDbFactory(
true ), $cache );
$this->assertTrue( $treeRepository->insert( $this->descendant,
$this->ancestor ) );
@@ -44,7 +45,7 @@
global $wgFlowCacheTime;
// Catch the exception and test the cache result then re-throw
the exception,
// otherwise the exception would skip the cache result test
- $cache = new BufferedCache( new \HashBagOStuff(),
$wgFlowCacheTime );
+ $cache = new BufferedCache( new BufferedBagOStuff( new
\HashBagOStuff() ), $wgFlowCacheTime );
try {
$treeRepository = new TreeRepository(
$this->mockDbFactory( false ), $cache );
$this->assertNull( $treeRepository->insert(
$this->descendant, $this->ancestor ) );
--
To view, visit https://gerrit.wikimedia.org/r/168547
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I770e47cd92f4f854db7717b0ae484f49261edcf3
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
