Skip to content

Commit 9669eaf

Browse files
skizzerzskizzerz
committed
Address CI issues
- Use tabs instead of spaces - Make unit tests use the updated hook interfaces
1 parent 55365bb commit 9669eaf

File tree

6 files changed

+178
-163
lines changed

6 files changed

+178
-163
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ jobs:
3131
php: '7.4'
3232
mediawiki: REL1_39
3333
- os: ubuntu-latest
34-
php: '8.2'
34+
php: '8.1'
3535
mediawiki: REL1_39
3636
steps:
3737
- name: Setup PHP
@@ -78,7 +78,7 @@ jobs:
7878
php: '7.4'
7979
mediawiki: REL1_39
8080
- os: ubuntu-latest
81-
php: '8.2'
81+
php: '8.1'
8282
mediawiki: REL1_39
8383
steps:
8484
- name: Setup PHP

‎.phan/config.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,8 @@
88
$cfg['suppress_issue_types'][] = 'UnusedSuppression';
99
$cfg['suppress_issue_types'][] = 'UnusedPluginSuppression';
1010

11+
// we make use of class aliases for backwards compat, but phan doesn't honor version checks surrounding them
12+
$cfg['suppress_issue_types'][] = 'PhanUndeclaredClassAliasOriginal';
13+
$cfg['suppress_issue_types'][] = 'PhanRedefineClassAlias';
14+
1115
return $cfg;

‎extension.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,5 @@
2727
"MessagesDirs": {
2828
"CrawlerProtection": "i18n"
2929
},
30-
"callback": "MediaWiki\\Extension\\CrawlerProtection\\Setup::registerClassAliases",
3130
"manifest_version": 2
3231
}

‎includes/Hooks.php‎

Lines changed: 98 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,28 @@
22

33
namespace MediaWiki\Extension\CrawlerProtection;
44

5+
// Class aliases for multi-version compatibility.
6+
// These need to be in global scope so phan can pick up on them,
7+
// and before any use statements that make use of the namespaced names.
8+
if ( version_compare( MW_VERSION, '1.40', '<' ) ) {
9+
class_alias( '\Title', '\MediaWiki\Title\Title' );
10+
}
11+
12+
if ( version_compare( MW_VERSION, '1.41', '<' ) ) {
13+
class_alias( '\OutputPage', '\MediaWiki\Output\OutputPage' );
14+
class_alias( '\SpecialPage', '\MediaWiki\SpecialPage\SpecialPage' );
15+
class_alias( '\User', '\MediaWiki\User\User' );
16+
class_alias( '\WebRequest', '\MediaWiki\Request\WebRequest' );
17+
}
18+
19+
if ( version_compare( MW_VERSION, '1.42', '<' ) ) {
20+
class_alias( '\MediaWiki', '\MediaWiki\Actions\ActionEntryPoint' );
21+
}
22+
23+
if ( version_compare( MW_VERSION, '1.44', '<' ) ) {
24+
class_alias( '\Article', '\MediaWiki\Page\Article' );
25+
}
26+
527
use MediaWiki\Actions\ActionEntryPoint;
628
use MediaWiki\Hook\MediaWikiPerformActionHook;
729
use MediaWiki\Output\OutputPage;
@@ -13,85 +35,86 @@
1335
use MediaWiki\User\User;
1436

1537
class Hooks implements MediaWikiPerformActionHook, SpecialPageBeforeExecuteHook {
16-
/**
17-
* Block sensitive page views for anonymous users via MediaWikiPerformAction.
18-
* Handles:
19-
* - ?type=revision
20-
* - ?action=history
21-
* - ?diff=1234
22-
* - ?oldid=1234
23-
*
24-
* Special pages (e.g. Special:WhatLinksHere) are handled separately.
25-
*
26-
* @param OutputPage $output
27-
* @param Article $article
28-
* @param Title $title
29-
* @param User $user
30-
* @param WebRequest $request
31-
* @param ActionEntryPoint $mediaWiki
32-
* @return bool False to abort further action
33-
*/
34-
public function onMediaWikiPerformAction(
35-
$output,
36-
$article,
37-
$title,
38-
$user,
39-
$request,
40-
$mediaWiki
41-
) {
42-
$type = $request->getVal( 'type' );
43-
$action = $request->getVal( 'action' );
44-
$diffId = (int)$request->getVal( 'diff' );
45-
$oldId = (int)$request->getVal( 'oldid' );
38+
/**
39+
* Block sensitive page views for anonymous users via MediaWikiPerformAction.
40+
* Handles:
41+
* - ?type=revision
42+
* - ?action=history
43+
* - ?diff=1234
44+
* - ?oldid=1234
45+
*
46+
* Special pages (e.g. Special:WhatLinksHere) are handled separately.
47+
*
48+
* @param OutputPage $output
49+
* @param Article $article
50+
* @param Title $title
51+
* @param User $user
52+
* @param WebRequest $request
53+
* @param ActionEntryPoint $mediaWiki
54+
* @return bool False to abort further action
55+
*/
56+
public function onMediaWikiPerformAction(
57+
$output,
58+
$article,
59+
$title,
60+
$user,
61+
$request,
62+
$mediaWiki
63+
) {
64+
$type = $request->getVal( 'type' );
65+
$action = $request->getVal( 'action' );
66+
$diffId = (int)$request->getVal( 'diff' );
67+
$oldId = (int)$request->getVal( 'oldid' );
4668

47-
if (
48-
!$user->isRegistered()
49-
&& (
50-
$type === 'revision'
51-
|| $action === 'history'
52-
|| $diffId > 0
53-
|| $oldId > 0
54-
)
55-
) {
56-
self::denyAccess( $output );
57-
return false;
58-
}
69+
if (
70+
!$user->isRegistered()
71+
&& (
72+
$type === 'revision'
73+
|| $action === 'history'
74+
|| $diffId > 0
75+
|| $oldId > 0
76+
)
77+
) {
78+
self::denyAccess( $output );
79+
return false;
80+
}
5981

60-
return true;
61-
}
82+
return true;
83+
}
6284

63-
/**
64-
* Block Special:RecentChangesLinked and Special:WhatLinksHere for anonymous users.
65-
*
66-
* @param SpecialPage $special
67-
* @param string|null $subPage
68-
* @return bool False to abort execution
69-
*/
70-
public function onSpecialPageBeforeExecute( $special, $subPage ) {
71-
$user = $special->getContext()->getUser();
72-
if ( $user->isRegistered() ) {
73-
return true; // logged-in users: allow
74-
}
85+
/**
86+
* Block Special:RecentChangesLinked and Special:WhatLinksHere for anonymous users.
87+
*
88+
* @param SpecialPage $special
89+
* @param string|null $subPage
90+
* @return bool False to abort execution
91+
*/
92+
public function onSpecialPageBeforeExecute( $special, $subPage ) {
93+
$user = $special->getContext()->getUser();
94+
if ( $user->isRegistered() ) {
95+
// logged-in users: allow
96+
return true;
97+
}
7598

76-
$name = strtolower( $special->getName() );
77-
if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere' ], true ) ) {
78-
$out = $special->getContext()->getOutput();
79-
self::denyAccess( $out );
80-
return false;
81-
}
99+
$name = strtolower( $special->getName() );
100+
if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere' ], true ) ) {
101+
$out = $special->getContext()->getOutput();
102+
self::denyAccess( $out );
103+
return false;
104+
}
82105

83-
return true;
84-
}
106+
return true;
107+
}
85108

86-
/**
87-
* Helper: output 403 Access Denied page using i18n messages.
88-
*
89-
* @param OutputPage $output
90-
* @return void
91-
*/
92-
private static function denyAccess( OutputPage $output ): void {
93-
$output->setStatusCode( 403 );
94-
$output->setPageTitle( wfMessage( 'crawlerprotection-accessdenied-title' )->text() );
95-
$output->addWikiTextAsInterface( wfMessage( 'crawlerprotection-accessdenied-text' )->text() );
96-
}
109+
/**
110+
* Helper: output 403 Access Denied page using i18n messages.
111+
*
112+
* @param OutputPage $output
113+
* @return void
114+
*/
115+
private static function denyAccess( OutputPage $output ): void {
116+
$output->setStatusCode( 403 );
117+
$output->setPageTitle( wfMessage( 'crawlerprotection-accessdenied-title' )->text() );
118+
$output->addWikiTextAsInterface( wfMessage( 'crawlerprotection-accessdenied-text' )->text() );
119+
}
97120
}

‎includes/Setup.php‎

Lines changed: 0 additions & 26 deletions
This file was deleted.

‎tests/phpunit/HooksTest.php‎

Lines changed: 74 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2,67 +2,82 @@
22

33
namespace MediaWiki\Extension\CrawlerProtection\Tests;
44

5+
use MediaWiki\Actions\ActionEntryPoint;
56
use MediaWiki\Extension\CrawlerProtection\Hooks;
6-
use OutputPage;
7-
use Title;
7+
use MediaWiki\Output\OutputPage;
8+
use MediaWiki\Page\Article;
9+
use MediaWiki\Title\Title;
10+
use PHPUnit\Framework\TestCase;
811
use User;
912
use WebRequest;
10-
use MediaWiki\MediaWiki;
11-
use Article;
12-
use PHPUnit\Framework\TestCase;
1313

14+
/**
15+
* @coversDefaultClass \MediaWiki\Extension\CrawlerProtection\Hooks
16+
*/
1417
class HooksTest extends TestCase {
15-
public function testRevisionTypeBlocksAnonymous() {
16-
$output = $this->createMock( OutputPage::class );
17-
$output->expects( $this->once() )->method( 'setPageTitle' );
18-
$output->expects( $this->once() )->method( 'addWikiTextAsInterface' );
19-
$output->expects( $this->once() )->method( 'setStatusCode' )->with( 403 );
20-
21-
$request = $this->createMock( WebRequest::class );
22-
$request->method( 'getVal' )->with( 'type' )->willReturn( 'revision' );
23-
24-
$user = $this->createMock( User::class );
25-
$user->method( 'isRegistered' )->willReturn( false );
26-
27-
$article = $this->createMock( Article::class );
28-
$title = $this->createMock( Title::class );
29-
$wiki = $this->createMock( MediaWiki::class );
30-
31-
$result = Hooks::onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
32-
$this->assertFalse( $result );
33-
}
34-
35-
public function testRevisionTypeAllowsLoggedIn() {
36-
$output = $this->createMock( OutputPage::class );
37-
38-
$request = $this->createMock( WebRequest::class );
39-
$request->method( 'getVal' )->with( 'type' )->willReturn( 'revision' );
40-
41-
$user = $this->createMock( User::class );
42-
$user->method( 'isRegistered' )->willReturn( true );
43-
44-
$article = $this->createMock( Article::class );
45-
$title = $this->createMock( Title::class );
46-
$wiki = $this->createMock( MediaWiki::class );
47-
48-
$result = Hooks::onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
49-
$this->assertTrue( $result );
50-
}
51-
52-
public function testNonRevisionTypeAlwaysAllowed() {
53-
$output = $this->createMock( OutputPage::class );
54-
55-
$request = $this->createMock( WebRequest::class );
56-
$request->method( 'getVal' )->with( 'type' )->willReturn( 'view' );
57-
58-
$user = $this->createMock( User::class );
59-
$user->method( 'isRegistered' )->willReturn( false );
60-
61-
$article = $this->createMock( Article::class );
62-
$title = $this->createMock( Title::class );
63-
$wiki = $this->createMock( MediaWiki::class );
64-
65-
$result = Hooks::onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
66-
$this->assertTrue( $result );
67-
}
68-
}
18+
/**
19+
* @covers ::onMediaWikiPerformAction
20+
*/
21+
public function testRevisionTypeBlocksAnonymous() {
22+
$output = $this->createMock( OutputPage::class );
23+
$output->expects( $this->once() )->method( 'setPageTitle' );
24+
$output->expects( $this->once() )->method( 'addWikiTextAsInterface' );
25+
$output->expects( $this->once() )->method( 'setStatusCode' )->with( 403 );
26+
27+
$request = $this->createMock( WebRequest::class );
28+
$request->method( 'getVal' )->with( 'type' )->willReturn( 'revision' );
29+
30+
$user = $this->createMock( User::class );
31+
$user->method( 'isRegistered' )->willReturn( false );
32+
33+
$article = $this->createMock( Article::class );
34+
$title = $this->createMock( Title::class );
35+
$wiki = $this->createMock( ActionEntryPoint::class );
36+
37+
$runner = new Hooks();
38+
$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
39+
$this->assertFalse( $result );
40+
}
41+
42+
/**
43+
* @covers ::onMediaWikiPerformAction
44+
*/
45+
public function testRevisionTypeAllowsLoggedIn() {
46+
$output = $this->createMock( OutputPage::class );
47+
48+
$request = $this->createMock( WebRequest::class );
49+
$request->method( 'getVal' )->with( 'type' )->willReturn( 'revision' );
50+
51+
$user = $this->createMock( User::class );
52+
$user->method( 'isRegistered' )->willReturn( true );
53+
54+
$article = $this->createMock( Article::class );
55+
$title = $this->createMock( Title::class );
56+
$wiki = $this->createMock( ActionEntryPoint::class );
57+
58+
$runner = new Hooks();
59+
$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
60+
$this->assertTrue( $result );
61+
}
62+
63+
/**
64+
* @covers ::onMediaWikiPerformAction
65+
*/
66+
public function testNonRevisionTypeAlwaysAllowed() {
67+
$output = $this->createMock( OutputPage::class );
68+
69+
$request = $this->createMock( WebRequest::class );
70+
$request->method( 'getVal' )->with( 'type' )->willReturn( 'view' );
71+
72+
$user = $this->createMock( User::class );
73+
$user->method( 'isRegistered' )->willReturn( false );
74+
75+
$article = $this->createMock( Article::class );
76+
$title = $this->createMock( Title::class );
77+
$wiki = $this->createMock( ActionEntryPoint::class );
78+
79+
$runner = new Hooks();
80+
$result = $runner->onMediaWikiPerformAction( $output, $article, $title, $user, $request, $wiki );
81+
$this->assertTrue( $result );
82+
}
83+
}

0 commit comments

Comments
 (0)