r77767 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r77766‎ | r77767 | r77768 >
Date:05:11, 5 December 2010
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
Some cleanup
Modified paths:
  • /trunk/extensions/DSMW/DSMW.php (modified) (history)
  • /trunk/extensions/DSMW/jobs/DSMWPropertyTypeJob.php (modified) (history)
  • /trunk/extensions/DSMW/patch/Patch.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DSMW/jobs/DSMWPropertyTypeJob.php
——@@ -1,80 +1,53 @@
22 <?php
33 /**
44 * @copyright 2009 INRIA-LORIA-Score Team
 5+ *
56 * @author jean-philippe muller
 7+ * @author Jeroen De Dauw
68 */
79
810 /**
911 * Job that assigns a type to some properties used in the DSMW ontology
10 - *
1112 */
1213 class DSMWPropertyTypeJob extends Job {
1314
14 - function __construct( $title ) {
15 - parent::__construct( 'DSMWPropertyTypeJob', $title );
 15+ public function __construct( $title ) {
 16+ parent::__construct( __CLASS__, $title );
1617 }
1718
18 - function run() {
 19+ public function run() {
1920 wfProfileIn( 'DSMWPropertyTypeJob::run()' );
2021
21 - $title = Title::newFromText( 'changeSetID', SMW_NS_PROPERTY );
22 - if ( !$title->exists() ) {
23 - $article = new Article( $title );
24 - $editpage = new EditPage( $article );
25 - $editpage->textbox1 = '[[has type::String]]';
26 - $editpage->attemptSave();
27 - }
 22+ $this->addProperty( 'changeSetID', '[[has type::String]]' );
 23+ $this->addProperty( 'hasSemanticQuery', '[[has type::String]]' );
 24+ $this->addProperty( 'patchID', '[[has type::String]]' );
 25+ $this->addProperty( 'siteID', '[[has type::String]]' );
 26+ $this->addProperty( 'pushFeedServer', '[[has type::URL]]' );
 27+ $this->addProperty( 'pushFeedName', '[[has type::String]]' );
 28+ $this->addProperty( 'hasOperation', '[[has type::Record]] [[has fields::String;String;Text;Text]]' );
2829
29 - $title = Title::newFromText( 'hasSemanticQuery', SMW_NS_PROPERTY );
30 - if ( !$title->exists() ) {
31 - $article = new Article( $title );
32 - $editpage = new EditPage( $article );
33 - $editpage->textbox1 = '[[has type::String]]';
34 - $editpage->attemptSave();
35 - }
36 -
37 - $title = Title::newFromText( 'patchID', SMW_NS_PROPERTY );
38 - if ( !$title->exists() ) {
39 - $article = new Article( $title );
40 - $editpage = new EditPage( $article );
41 - $editpage->textbox1 = '[[has type::String]]';
42 - $editpage->attemptSave();
43 - }
44 -
45 - $title = Title::newFromText( 'siteID', SMW_NS_PROPERTY );
46 - if ( !$title->exists() ) {
47 - $article = new Article( $title );
48 - $editpage = new EditPage( $article );
49 - $editpage->textbox1 = '[[has type::String]]';
50 - $editpage->attemptSave();
51 - }
52 -
53 - $title = Title::newFromText( 'pushFeedServer', SMW_NS_PROPERTY );
54 - if ( !$title->exists() ) {
55 - $article = new Article( $title );
56 - $editpage = new EditPage( $article );
57 - $editpage->textbox1 = '[[has type::URL]]';
58 - $editpage->attemptSave();
59 - }
60 -
61 - $title = Title::newFromText( 'pushFeedName', SMW_NS_PROPERTY );
62 - if ( !$title->exists() ) {
63 - $article = new Article( $title );
64 - $editpage = new EditPage( $article );
65 - $editpage->textbox1 = '[[has type::String]]';
66 - $editpage->attemptSave();
67 - }
68 -
69 - $title = Title::newFromText( 'hasOperation', SMW_NS_PROPERTY );
70 - if ( !$title->exists() ) {
71 - $article = new Article( $title );
72 - $editpage = new EditPage( $article );
73 - $editpage->textbox1 = '[[has type::Record]]
74 -[[has fields::String;String;Text;Text]]';
75 - $editpage->attemptSave();
76 - }
77 -
7830 wfProfileOut( 'DSMWPropertyTypeJob::run()' );
 31+
7932 return true;
8033 }
 34+
 35+ /**
 36+ * Adds a property.
 37+ *
 38+ * @since 1.1
 39+ *
 40+ * @param string $titleName The name of the property
 41+ * @param string $propertyProperties A string of properties to assign to the propery (such as "has type")
 42+ */
 43+ protected function addProperty( $titleName, $propertyProperties ) {
 44+ $title = Title::newFromText( $titleName, SMW_NS_PROPERTY );
 45+
 46+ if ( !$title->exists() ) {
 47+ $article = new Article( $title );
 48+ $editpage = new EditPage( $article );
 49+ $editpage->textbox1 = $propertyProperties;
 50+ $editpage->attemptSave();
 51+ }
 52+ }
 53+
8154 }
Index: trunk/extensions/DSMW/patch/Patch.php
——@@ -40,8 +40,10 @@
4141 */
4242 public function __construct( $remote, $attachment, $operations, $siteUrl = '', $causalLink = '', $patchid = '', $previousPatch = '', $siteID = '', $Mime = '', $Size = '', $Url = '', $Date = '' ) {
4343 global $wgServer;
 44+
4445 $this->mRemote = $remote;
4546 $this->mID = utils::generateID();
 47+
4648 if ( $remote == true ) {
4749 $this->mPatchId = $patchid;
4850 $this->mSiteId = $siteID;
——@@ -50,6 +52,7 @@
5153 $this->mPatchId = "Patch:" . $this->mID;
5254 $this->mSiteId = DSMWSiteId::getInstance()->getSiteId();
5355 }
 56+
5457 $this->mOperations = $operations;
5558 $this->mPrevPatch = $previousPatch;
5659 $this->mSiteUrl = $siteUrl;
——@@ -142,11 +145,8 @@
143146 foreach ( $this->mOperations as $operation ) {
144147 $lineContent = $operation->getLineContent();
145148 $lineContent1 = utils::contentEncoding( $lineContent ); // base64 encoding
146 - $type = "";
147 - if ( $operation instanceof LogootIns )
148 - $type = "Insert";
149 - else
150 - $type = "Delete";
 149+ $type = $operation instanceof LogootIns ? 'Insert' : 'Delete';
 150+
151151 $operationID = utils::generateID();
152152 $text .= '|[[hasOperation::' . $operationID . ';' . $type . ';'
153153 . $operation->getLogootPosition()->toString() . ';' . $lineContent1 . '| ]]' . $type;
——@@ -187,13 +187,8 @@
188188 $article->doEdit( $text, $summary = "" );
189189 }
190190
191 - private function splitLine( $line ) {
192 - $text = "";
193 - $arr = str_split( $line, 150 );
194 - foreach ( $arr as $element ) {
195 - $text .= $element . '<br>';
196 - }
197 - return $text;
 191+ protected function splitLine( $line ) {
 192+ return implode( '<br>', str_split( $line, 150 ) ) . '<br />';
198193 }
199194
200195 }
Index: trunk/extensions/DSMW/DSMW.php
——@@ -57,11 +57,6 @@
5858
5959 $wgAutoloadClasses['DSMWHooks'] = dirname( __FILE__ ) . '/DSMW.hooks.php';
6060
61 -$wgHooks['UnknownAction'][] = 'DSMWHooks::onUnknownAction';
62 -$wgHooks['EditPage::attemptSave'][] = 'DSMWHooks::onAttemptSave';
63 -$wgHooks['EditPageBeforeConflictDiff'][] = 'DSMWHooks::onEditConflict';
64 -$wgHooks['UploadComplete'][] = 'DSMWHooks::onUploadComplete';
65 -
6661 $wgAutoloadClasses['logootEngine'] = "$wgDSMWIP/logootComponent/logootEngine.php";
6762 $wgAutoloadClasses['logoot'] = "$wgDSMWIP/logootComponent/logoot.php";
6863 $wgAutoloadClasses['LogootId'] = "$wgDSMWIP/logootComponent/LogootId.php";
——@@ -104,6 +99,11 @@
105100
106101 $wgExtensionFunctions[] = 'dsmwgSetupFunction';
107102
 103+$wgHooks['UnknownAction'][] = 'DSMWHooks::onUnknownAction';
 104+$wgHooks['EditPage::attemptSave'][] = 'DSMWHooks::onAttemptSave';
 105+$wgHooks['EditPageBeforeConflictDiff'][] = 'DSMWHooks::onEditConflict';
 106+$wgHooks['UploadComplete'][] = 'DSMWHooks::onUploadComplete';
 107+
108108 $wgExtensionCredits[defined( 'SEMANTIC_EXTENSION_TYPE' ) ? 'semantic' : 'other'][] = array(
109109 'path' => __FILE__,
110110 'name' => 'Distributed&nbsp;Semantic&nbsp;MediaWiki',

Follow-up revisions

RevisionCommit summaryAuthorDate
r77820Follow up to suggestion by Nikerabbit at r77767jeroendedauw18:57, 5 December 2010

Comments

#Comment by Nikerabbit (talk | contribs)   09:45, 5 December 2010

+ $article = new Article( $title ); 

+	        $editpage = new EditPage( $article );
+	        $editpage->textbox1 = $propertyProperties;
+	        $editpage->attemptSave();

Why is it using editpage to edit pages? Also, not passing 0 as second parameter to Article is sure way to get hard to debug bugs when jobs are run along web requests.

#Comment by Jeroen De Dauw (talk | contribs)   16:49, 5 December 2010

> Why is it using editpage to edit pages? No idea, it's not my code. What should be used here though?

> ...not passing 0 as second parameter to Article is sure way to get hard to debug bugs... Can you elaborate on that or point me to some example of correct usage?

#Comment by Nikerabbit (talk | contribs)   16:54, 5 December 2010

Article::doEdit() 

$foo = new Article( $title, 0 );

Otherwise it pulls any random unrelated oldid from the current request.

#Comment by Jeroen De Dauw (talk | contribs)   16:58, 5 December 2010

Ok, makes sense, I'll fix :)

Status & tagging log