r76214 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r76213‎ \| r76214 \| r76215 >
Date:	20:11, 6 November 2010
Author:	demon
Status:	ok
Tags:
Comment:	General code cleanup to CR: * Remove more silly 'm' prefixes from member vars * Got rid of entry point detection stuff, not necessary in files that only have classes * Get rid of dupe authorWikiUser() from CodeView and children * Made getViewFrom() non-static. It was also using $mRepo * Tried to privatize some variables that didn't need to be public and had accessors
Modified paths:	/trunk/extensions/CodeReview/backend/CodeComment.php (modified) (history) /trunk/extensions/CodeReview/backend/CodePropChange.php (modified) (history) /trunk/extensions/CodeReview/backend/CodeRepository.php (modified) (history) /trunk/extensions/CodeReview/backend/CodeRevision.php (modified) (history) /trunk/extensions/CodeReview/backend/Subversion.php (modified) (history) /trunk/extensions/CodeReview/svnImport.php (modified) (history) /trunk/extensions/CodeReview/ui/CodeRepoListView.php (modified) (history) /trunk/extensions/CodeReview/ui/CodeRevisionAuthorView.php (modified) (history) /trunk/extensions/CodeReview/ui/CodeRevisionListView.php (modified) (history) /trunk/extensions/CodeReview/ui/SpecialCode.php (modified) (history) /trunk/extensions/CodeReview/ui/SpecialRepoAdmin.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CodeReview/backend/CodeRevision.php
—	—	@@ -1,5 +1,4 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4	3
5	4	class CodeRevision {
6	5	public $mRepoId, $mRepo, $mId, $mAuthor, $mTimestamp, $mMessage, $mPaths, $mStatus, $mOldStatus, $mCommonPath;
Index: trunk/extensions/CodeReview/backend/CodeRepository.php
—	—	@@ -1,12 +1,43 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4	3
	4	+/**
	5	+ * Core class for interacting with a repository of code.
	6	+ */
5	7	class CodeRepository {
6		~~- static $userLinks = array();~~
7		~~- static $authorLinks = array();~~
8	8
9		~~- public $mId, $mName, $mPath, $mViewVc, $mBugzilla;~~
	9	+ /**
	10	+ * Local cache of Wiki user -> SVN user mappings
	11	+ * @var Array
	12	+ */
	13	+ private static $userLinks = array();
10	14
	15	+ /**
	16	+ * Sort of the same, but looking it up for the other direction
	17	+ * @var Array
	18	+ */
	19	+ private static $authorLinks = array();
	20	+
	21	+ /**
	22	+ * Various data about the repo
	23	+ */
	24	+ private $id, $name, $path, $viewVc, $bugzilla;
	25	+
	26	+ /**
	27	+ * Constructor, can't use it. Call one of the static newFrom* methods
	28	+ * @param $id Int Database id for the repo
	29	+ * @param $name String User-defined name for the repository
	30	+ * @param $path String Path to SVN
	31	+ * @param $viewVc String Base path to ViewVC URLs
	32	+ * @param $bugzilla String Base path to Bugzilla
	33	+ */
	34	+ public function __construct( $id, $name, $path, $viewvc, $bugzilla ) {
	35	+ $this->id = $id;
	36	+ $this->name = $name;
	37	+ $this->path = $path;
	38	+ $this->viewVc = $viewvc;
	39	+ $this->bugzilla = $bugzilla;
	40	+ }
	41	+
11	42	public static function newFromName( $name ) {
12	43	$dbw = wfGetDB( DB_MASTER );
13	44	$row = $dbw->selectRow(
—	—	@@ -48,13 +79,13 @@
49	80	}
50	81
51	82	static function newFromRow( $row ) {
52		~~- $repo = new CodeRepository();~~
53		~~- $repo->mId = intval( $row->repo_id );~~
54		~~- $repo->mName = $row->repo_name;~~
55		~~- $repo->mPath = $row->repo_path;~~
56		~~- $repo->mViewVc = $row->repo_viewvc;~~
57		~~- $repo->mBugzilla = $row->repo_bugzilla;~~
58		~~- return $repo;~~
	83	+ return new CodeRepository(
	84	+ intval( $row->repo_id ),
	85	+ $row->repo_name,
	86	+ $row->repo_path,
	87	+ $row->repo_viewvc,
	88	+ $row->repo_bugzilla
	89	+ );
59	90	}
60	91
61	92	static function getRepoList() {
—	—	@@ -68,28 +99,28 @@
69	100	}
70	101
71	102	public function getId() {
72		~~- return intval( $this->mId );~~
	103	+ return intval( $this->id );
73	104	}
74	105
75	106	public function getName() {
76		~~- return $this->mName;~~
	107	+ return $this->name;
77	108	}
78	109
79	110	public function getPath() {
80		~~- return $this->mPath;~~
	111	+ return $this->path;
81	112	}
82	113
83	114	public function getViewVcBase() {
84		~~- return $this->mViewVc;~~
	115	+ return $this->viewVc;
85	116	}
86	117
87	118	/**
88	119	* Return a bug URL or false.
89	120	*/
90	121	public function getBugPath( $bugId ) {
91		~~- if ( $this->mBugzilla ) {~~
	122	+ if ( $this->bugzilla ) {
92	123	return str_replace( '$1',
93		~~- urlencode( $bugId ), $this->mBugzilla );~~
	124	+ urlencode( $bugId ), $this->bugzilla );
94	125	}
95	126	return false;
96	127	}
—	—	@@ -229,7 +260,7 @@
230	261	}
231	262
232	263	# Try memcached...
233		~~- $key = wfMemcKey( 'svn', md5( $this->mPath ), 'diff', $rev1, $rev2 );~~
	264	+ $key = wfMemcKey( 'svn', md5( $this->path ), 'diff', $rev1, $rev2 );
234	265	if ( $useCache === 'skipcache' ) {
235	266	$data = null;
236	267	} else {
—	—	@@ -241,7 +272,7 @@
242	273	$dbr = wfGetDB( DB_SLAVE );
243	274	$row = $dbr->selectRow( 'code_rev',
244	275	array( 'cr_diff', 'cr_flags' ),
245		~~- array( 'cr_repo_id' => $this->mId, 'cr_id' => $rev, 'cr_diff IS NOT NULL' ),~~
	276	+ array( 'cr_repo_id' => $this->id, 'cr_id' => $rev, 'cr_diff IS NOT NULL' ),
246	277	__METHOD__
247	278	);
248	279	if ( $row ) {
—	—	@@ -259,7 +290,7 @@
260	291
261	292	# Generate the diff as needed...
262	293	if ( !$data && $useCache !== 'cached' ) {
263		~~- $svn = SubversionAdaptor::newFromRepo( $this->mPath );~~
	294	+ $svn = SubversionAdaptor::newFromRepo( $this->path );
264	295	$data = $svn->getDiff( '', $rev1, $rev2 );
265	296	// Store to cache
266	297	$wgMemc->set( $key, $data, 3600 * 24 * 3 );
—	—	@@ -269,7 +300,7 @@
270	301	$dbw = wfGetDB( DB_MASTER );
271	302	$dbw->update( 'code_rev',
272	303	array( 'cr_diff' => $storedData, 'cr_flags' => $flags ),
273		~~- array( 'cr_repo_id' => $this->mId, 'cr_id' => $rev ),~~
	304	+ array( 'cr_repo_id' => $this->id, 'cr_id' => $rev ),
274	305	__METHOD__
275	306	);
276	307	}
—	—	@@ -288,10 +319,10 @@
289	320	$rev1 = $codeRev->getId() - 1;
290	321	$rev2 = $codeRev->getId();
291	322
292		~~- $svn = SubversionAdaptor::newFromRepo( $this->mPath );~~
	323	+ $svn = SubversionAdaptor::newFromRepo( $this->path );
293	324	$data = $svn->getDiff( '', $rev1, $rev2 );
294	325	// Store to cache
295		~~- $key = wfMemcKey( 'svn', md5( $this->mPath ), 'diff', $rev1, $rev2 );~~
	326	+ $key = wfMemcKey( 'svn', md5( $this->path ), 'diff', $rev1, $rev2 );
296	327	$wgMemc->set( $key, $data, 3600 * 24 * 3 );
297	328	// Permanent DB storage
298	329	$storedData = $data;
—	—	@@ -299,7 +330,7 @@
300	331	$dbw = wfGetDB( DB_MASTER );
301	332	$dbw->update( 'code_rev',
302	333	array( 'cr_diff' => $storedData, 'cr_flags' => $flags ),
303		~~- array( 'cr_repo_id' => $this->mId, 'cr_id' => $codeRev->getId() ),~~
	334	+ array( 'cr_repo_id' => $this->id, 'cr_id' => $codeRev->getId() ),
304	335	__METHOD__
305	336	);
306	337	wfProfileOut( __METHOD__ );
Index: trunk/extensions/CodeReview/backend/CodeComment.php
—	—	@@ -1,5 +1,4 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4	3
5	4	class CodeComment {
6	5	public $id, $text, $user, $userText, $timestamp, $review, $sortkey, $attrib, $removed, $added;
Index: trunk/extensions/CodeReview/backend/Subversion.php
—	—	@@ -1,6 +1,6 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4	3
	4	+
5	5	abstract class SubversionAdaptor {
6	6	protected $mRepo;
7	7
Index: trunk/extensions/CodeReview/backend/CodePropChange.php
—	—	@@ -1,5 +1,4 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4	3
5	4	class CodePropChange {
6	5	function __construct( $rev ) {
Index: trunk/extensions/CodeReview/svnImport.php
—	—	@@ -113,7 +113,7 @@
114	114	$codeRev->save();
115	115
116	116	$this->output( sprintf( "%d %s %s (%0.1f revs/sec)\n",
117		~~- $codeRev->mId,~~
	117	+ $codeRev->getId(),
118	118	wfTimestamp( TS_DB, $codeRev->mTimestamp ),
119	119	$codeRev->mAuthor,
120	120	$revSpeed ) );
Index: trunk/extensions/CodeReview/ui/CodeRevisionAuthorView.php
—	—	@@ -4,7 +4,7 @@
5	5	function __construct( $repoName, $author ) {
6	6	parent::__construct( $repoName );
7	7	$this->mAuthor = $author;
8		~~- $this->mUser = $this->authorWikiUser( $author );~~
	8	+ $this->mUser = $this->mRepo->authorWikiUser( $author );
9	9	$this->mAppliedFilter = wfMsg( 'code-revfilter-cr_author', $author );
10	10	}
11	11
Index: trunk/extensions/CodeReview/ui/SpecialCode.php
—	—	@@ -1,12 +1,14 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4	3
	4	+/**
	5	+ * Main UI entry point. This calls the appropriate CodeView subclass and runs it
	6	+ */
5	7	class SpecialCode extends SpecialPage {
6		~~- function __construct() {~~
	8	+ public function __construct() {
7	9	parent::__construct( 'Code' , 'codereview-use' );
8	10	}
9	11
10		~~- function execute( $subpage ) {~~
	12	+ public function execute( $subpage ) {
11	13	global $wgOut, $wgUser, $wgExtensionAssetsPath, $wgCodeReviewStyleVersion;
12	14
13	15	if ( !$this->userCanExecute( $wgUser ) ) {
—	—	@@ -17,7 +19,7 @@
18	20	$this->setHeaders();
19	21	$wgOut->addStyle( "$wgExtensionAssetsPath/CodeReview/codereview.css?$wgCodeReviewStyleVersion" );
20	22
21		~~- $view = self::getViewFrom( $subpage );~~
	23	+ $view = $this->getViewFrom( $subpage );
22	24	if( $view ) {
23	25	$view->execute();
24	26	} else {
—	—	@@ -38,7 +40,7 @@
39	41	* Get a view object from a sub page path.
40	42	* @return View object or null if no valid action could be found
41	43	*/
42		~~- private static function getViewFrom( $subpage ) {~~
	44	+ private function getViewFrom( $subpage ) {
43	45	global $wgRequest;
44	46
45	47	# Remove stray slashes
—	—	@@ -149,16 +151,6 @@
150	152
151	153	abstract function execute();
152	154
153		- /*
154		~~- * returns a User object if $author has a wikiuser associated,~~
155		~~- * of false~~
156		~~- */~~
157		~~- function authorWikiUser( $author ) {~~
158		~~- if ( $this->mRepo )~~
159		~~- return $this->mRepo->authorWikiUser( $author );~~
160		~~- return false;~~
161		~~- }~~
162		-
163	155	function authorLink( $author, $extraParams = array() ) {
164	156	$repo = $this->mRepo->getName();
165	157	$special = SpecialPage::getTitleFor( 'Code', "$repo/author/$author" );
—	—	@@ -234,7 +226,7 @@
235	227	function formatRow( $row ) {
236	228	global $wgWikiSVN;
237	229	$css = "mw-codereview-status-{$row->cr_status}";
238		~~- if ( $this->mRepo->mName == $wgWikiSVN ) {~~
	230	+ if ( $this->mRepo->getName() == $wgWikiSVN ) {
239	231	$css .= " mw-codereview-" . ( $row-> { $this->getDefaultSort() } <= $this->mCurSVN ? 'live' : 'notlive' );
240	232	}
241	233	$s = "<tr class=\"$css\">\n";
Index: trunk/extensions/CodeReview/ui/SpecialRepoAdmin.php
—	—	@@ -1,12 +1,13 @@
2	2	<?php
3		~~-if ( !defined( 'MEDIAWIKI' ) ) die();~~
4		-
	3	+/**
	4	+ * Repository administration
	5	+ */
5	6	class SpecialRepoAdmin extends SpecialPage {
6		~~- function __construct() {~~
	7	+ public function __construct() {
7	8	parent::__construct( 'RepoAdmin', 'repoadmin' );
8	9	}
9	10
10		~~- function execute( $subpage ) {~~
	11	+ public function execute( $subpage ) {
11	12	global $wgRequest, $wgUser;
12	13
13	14	$this->setHeaders();
—	—	@@ -18,32 +19,47 @@
19	20
20	21	$repo = $wgRequest->getVal( 'repo', $subpage );
21	22	if ( $repo == '' ) {
22		~~- $view = new RepoAdminListView( $this );~~
	23	+ $view = new RepoAdminListView( $this->getTitle() );
23	24	} else {
24		~~- $view = new RepoAdminRepoView( $this, $repo );~~
	25	+ $view = new RepoAdminRepoView( $this->getTitle( $repo ), $repo );
25	26	}
26	27	$view->execute();
27	28	}
28	29	}
29	30
	31	+/**
	32	+ * View for viewing all of the repositories
	33	+ */
30	34	class RepoAdminListView {
31		~~- var $mPage;~~
	35	+ /**
	36	+ * Reference to Special:RepoAdmin
	37	+ * @var Title
	38	+ */
	39	+ private $title;
32	40
33		~~- function __construct( $page ) {~~
34		~~- $this->mPage = $page;~~
	41	+ /**
	42	+ * Constructor
	43	+ * @param $t Title object referring to Special:RepoAdmin
	44	+ */
	45	+ public function __construct( Title $t ) {
	46	+ $this->title = $t;
35	47	}
36	48
37		~~- function getForm() {~~
	49	+ /**
	50	+ * Get "create new repo" form
	51	+ * @return String
	52	+ */
	53	+ private function getForm() {
38	54	global $wgScript;
39	55	return Xml::fieldset( wfMsg( 'repoadmin-new-legend' ) ) .
40	56	Xml::openElement( 'form', array( 'method' => 'get', 'action' => $wgScript ) ) .
41		~~- Html::hidden( 'title', $this->mPage->getTitle()->getPrefixedDBKey() ) .~~
	57	+ Html::hidden( 'title', $this->title->getPrefixedDBKey() ) .
42	58	Xml::inputLabel( wfMsg( 'repoadmin-new-label' ), 'repo', 'repo' ) .
43	59	Xml::submitButton( wfMsg( 'repoadmin-new-button' ) ) .
44	60	'</form></fieldset>';
45	61	}
46	62
47		~~- function execute() {~~
	63	+ public function execute() {
48	64	global $wgOut;
49	65	$wgOut->addHTML( $this->getForm() );
50	66	$repos = CodeRepository::getRepoList();
—	—	@@ -59,22 +75,45 @@
60	76	}
61	77	}
62	78
	79	+/**
	80	+ * View for editing a single repository
	81	+ */
63	82	class RepoAdminRepoView {
64		~~- var $mPage;~~
	83	+ /**
	84	+ * Reference to Special:RepoAdmin
	85	+ * @var Title
	86	+ */
	87	+ private $title;
65	88
66		~~- function __construct( $page, $repo ) {~~
67		~~- $this->mPage = $page;~~
68		~~- $this->mRepoName = $repo;~~
69		~~- $this->mRepo = CodeRepository::newFromName( $repo );~~
	89	+ /**
	90	+ * Human-readable name of the repository
	91	+ * @var String
	92	+ */
	93	+ private $repoName;
	94	+
	95	+ /**
	96	+ * Actual repository object
	97	+ */
	98	+ private $repo;
	99	+
	100	+ /**
	101	+ * @
	102	+ * @param $page Title Special page title (with repo subpage)
	103	+ * @param $repo
	104	+ */
	105	+ public function __construct( Title $t, $repo ) {
	106	+ $this->title = $t;
	107	+ $this->repoName = $repo;
	108	+ $this->repo = CodeRepository::newFromName( $repo );
70	109	}
71	110
72	111	function execute() {
73	112	global $wgOut, $wgRequest, $wgUser;
74		~~- $repoExists = (bool)$this->mRepo;~~
75		~~- $repoPath = $wgRequest->getVal( 'wpRepoPath', $repoExists ? $this->mRepo->mPath : '' );~~
76		~~- $bugPath = $wgRequest->getVal( 'wpBugPath', $repoExists ? $this->mRepo->mBugzilla : '' );~~
77		~~- $viewPath = $wgRequest->getVal( 'wpViewPath', $repoExists ? $this->mRepo->mViewVc : '' );~~
78		~~- if ( $wgRequest->wasPosted() && $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ), $this->mRepoName ) ) {~~
	113	+ $repoExists = (bool)$this->repo;
	114	+ $repoPath = $wgRequest->getVal( 'wpRepoPath', $repoExists ? $this->repo->getPath() : '' );
	115	+ $bugPath = $wgRequest->getVal( 'wpBugPath', $repoExists ? $this->repo->getBugzillaBase() : '' );
	116	+ $viewPath = $wgRequest->getVal( 'wpViewPath', $repoExists ? $this->repo->getViewVcBase() : '' );
	117	+ if ( $wgRequest->wasPosted() && $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ), $this->repoName ) ) {
79	118	// @todo log
80	119	$dbw = wfGetDB( DB_MASTER );
81	120	if ( $repoExists ) {
—	—	@@ -92,7 +131,7 @@
93	132	$dbw->insert(
94	133	'code_repo',
95	134	array(
96		~~- 'repo_name' => $this->mRepoName,~~
	135	+ 'repo_name' => $this->repoName,
97	136	'repo_path' => $repoPath,
98	137	'repo_viewvc' => $viewPath,
99	138	'repo_bugzilla' => $bugPath
—	—	@@ -100,12 +139,12 @@
101	140	__METHOD__
102	141	);
103	142	}
104		~~- $wgOut->wrapWikiMsg( '<div class="successbox">$1</div>', array( 'repoadmin-edit-sucess', $this->mRepoName ) );~~
	143	+ $wgOut->wrapWikiMsg( '<div class="successbox">$1</div>', array( 'repoadmin-edit-sucess', $this->repoName ) );
105	144	return;
106	145	}
107	146	$wgOut->addHTML(
108		~~- Xml::fieldset( wfMsg( 'repoadmin-edit-legend', $this->mRepoName ) ) .~~
109		~~- Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->mPage->getTitle( $this->mRepoName )->getLocalURL() ) ) .~~
	147	+ Xml::fieldset( wfMsg( 'repoadmin-edit-legend', $this->repoName ) ) .
	148	+ Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->title->getLocalURL() ) ) .
110	149	Xml::buildForm(
111	150	array(
112	151	'repoadmin-edit-path' =>
—	—	@@ -114,7 +153,7 @@
115	154	Xml::input( 'wpBugPath', 60, $bugPath ),
116	155	'repoadmin-edit-view' =>
117	156	Xml::input( 'wpViewPath', 60, $viewPath ) ) ) .
118		~~- Html::hidden( 'wpEditToken', $wgUser->editToken( $this->mRepoName ) ) .~~
	157	+ Html::hidden( 'wpEditToken', $wgUser->editToken( $this->repoName ) ) .
119	158	Xml::submitButton( wfMsg( 'repoadmin-edit-button' ) ) .
120	159	'</form></fieldset>'
121	160	);
Index: trunk/extensions/CodeReview/ui/CodeRevisionListView.php
—	—	@@ -348,7 +348,7 @@
349	349	function formatRow( $row ) {
350	350	global $wgWikiSVN;
351	351	$css = "mw-codereview-status-{$row->cr_status}";
352		~~- if ( $this->mRepo->mName == $wgWikiSVN ) {~~
	352	+ if ( $this->mRepo->getName() == $wgWikiSVN ) {
353	353	$css .= " mw-codereview-" . ( $row-> { $this->getDefaultSort() } <= $this->mCurSVN ? 'live' : 'notlive' );
354	354	}
355	355	$s = "<tr class=\"$css\">\n";
Index: trunk/extensions/CodeReview/ui/CodeRepoListView.php
—	—	@@ -1,9 +1,10 @@
2	2	<?php
3	3
4		~~-// Special:Code~~
	4	+/**
	5	+ * Class for showing the list of repositories, if none was specified
	6	+ */
5	7	class CodeRepoListView {
6		-
7		~~- function execute() {~~
	8	+ public function execute() {
8	9	global $wgOut;
9	10	$repos = CodeRepository::getRepoList();
10	11	if ( !count( $repos ) ) {

Follow-up revisions

Revision	Commit summary	Author	Date
r76879	Fix for r76214: added missing method CodeRepository::getBugzillaBase(); was t...	ialex	11:43, 17 November 2010
r80481	(bug 26780) Fix fatal from refactoring in r76214	demon	01:00, 18 January 2011

Status & tagging log

12:43, 8 December 2010 Reedy (talk | contribs) changed the status of r76214 [removed: new added: ok]