r67480 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r67479‎ \| r67480 \| r67481 >
Date:	20:25, 6 June 2010
Author:	daniel
Status:	deferred
Tags:
Comment:	several test cases, more to comecd
Modified paths:	/trunk/extensions/DataTransclusion/DataTransclusion.i18n.php (modified) (history) /trunk/extensions/DataTransclusion/DataTransclusion.php (modified) (history) /trunk/extensions/DataTransclusion/DataTransclusionHandler.php (modified) (history) /trunk/extensions/DataTransclusion/DataTransclusionSource.php (modified) (history) /trunk/extensions/DataTransclusion/tests (added) (history) /trunk/extensions/DataTransclusion/tests/DataTransclusionTest.php (added) (history)

Diff [purge]

Index: trunk/extensions/DataTransclusion/DataTransclusion.i18n.php
—	—	@@ -16,6 +16,10 @@
17	17	$messages['en'] = array(
18	18	'datatransclusion-desc' => 'Import and rendering of data records from external data sources',
19	19
	20	+ 'datatransclusion-test-wikitext' => 'some <span class="test">html</span> and \'\'markup\'\'.',
	21	+ 'datatransclusion-test-evil-html' => 'some <object>evil</object> html.',
	22	+ 'datatransclusion-test-nowiki' => 'some <nowiki>{{nowiki}}</nowiki> code.',
	23	+
20	24	'datatransclusion-missing-source' => 'no data source specified (first argument is required)',
21	25	'datatransclusion-unknown-source' => 'bad data source specified ($1 is not known)',
22	26	'datatransclusion-bad-argument-by' => 'bad key field specified ($2 is not a key field in data source $1, valid keys are: $3)',
—	—	@@ -31,6 +35,10 @@
32	36	$messages['qqq'] = array(
33	37	'datatransclusion-desc' => 'Shown in [[Special:Version]] as a short description of this extension. Do not translate links.',
34	38
	39	+ 'datatransclusion-test-wikitext' => 'Used to test handling of wiki markup in messages. Do not translate!',
	40	+ 'datatransclusion-test-evil-html' => 'Used to test handling of HTML in messages. Do not translate!',
	41	+ 'datatransclusion-test-nowiki' => 'Used to test handling of nowiki-sections in messages. Do not translate!',
	42	+
35	43	'datatransclusion-missing-source' => 'issued if no data source was specified.',
36	44	'datatransclusion-unknown-source' => 'issued if an unknown data source was specified. $1 is the name of the data source.',
37	45	'datatransclusion-bad-argument-by' => 'issued if a bad value was specified for the "by" argument, that is, an unknown key field was selected. $1 is the name of the data source, $2 is the value of the by argument, $3 is a list of all valid keys for this data source.',
Index: trunk/extensions/DataTransclusion/DataTransclusionHandler.php
—	—	@@ -53,7 +53,7 @@
54	54	$argv = DataTransclusionHandler::buildAssociativeArguments( $params );
55	55
56	56	//FIXME: error messages contining special blocks like <nowiki> don't get re-substitutet correctly.
57		~~- $text = DataTransclusionHandler::handleRecordTag( null, $argv, $parser, false );~~
	57	+ $text = DataTransclusionHandler::handleRecordTransclusion( null, $argv, $parser, false );
58	58	return array( $text, 'noparse' => false, 'isHTML' => false );
59	59	}
60	60
—	—	@@ -62,17 +62,25 @@
63	63	array_shift( $params ); // $key
64	64	array_shift( $params ); // $asHTML
65	65
66		~~- if ( $asHTML ) $mode = 'parseinline'; //TESTME~~
	66	+ if ( $asHTML ) $mode = 'parseinline';
67	67	else $mode = 'parsemag';
68	68
69	69	$m = wfMsgExt( $key, $mode, $params );
70		~~- return "<span class=\"error\">$m</div>";~~
	70	+ return "<span class=\"error\">$m</span>";
71	71	}
72	72
73	73	/**
74		~~- * Entry point for the <record> tag parser hook.~~
	74	+ * Entry point for the <record> tag parser hook. Delegates to handleRecordTransclusion.
75	75	*/
76		~~- static function handleRecordTag( $key, $argv, $parser = null, $asHTML = true ) {~~
	76	+ static function handleRecordTag( $key, $argv, $parser ) {
	77	+ DataTransclusionHandler::handleRecordTransclusion( $key, $argv, $parser, true );
	78	+ }
	79	+
	80	+ /**
	81	+ * Fetches a records and renders it, according to the given array of parameters.
	82	+ * Common implementation for parser tag and parser function.
	83	+ */
	84	+ static function handleRecordTransclusion( $key, $argv, $parser, $asHTML ) {
77	85	//find out which data source to use...
78	86	if ( empty( $argv['source'] ) ) {
79	87	if ( empty( $argv[1] ) ) return DataTransclusionHandler::errorMessage( 'datatransclusion-missing-source', $asHTML ); //TESTME
—	—	@@ -137,10 +145,11 @@
138	146	}
139	147	}
140	148
141		~~- function __construct( $parser, $source, $template ) {~~
	149	+ function __construct( $parser, $source, $template, $templateText = null ) {
142	150	$this->template = $template;
143	151	$this->source = $source;
144	152	$this->parser = $parser;
	153	+ $this->templateText = $templateText;
145	154	}
146	155
147	156	function render( $record ) {
—	—	@@ -169,10 +178,16 @@
170	179	*/
171	180
172	181	//dumb and slow, but works
173		~~- $p = new Article( $this->template );~~
174		~~- if ( !$p->exists() ) return false; //TESTME~~
	182	+ if ( $this->templateText ) {
	183	+ if ( is_string( $this->templateText ) ) $text = $this->templateText;
	184	+ else $text = $this->templateText->getContent();
	185	+ } else {
	186	+ $article = new Article( $this->template );
	187	+ if ( !$article->exists() ) return false; //TESTME
175	188
176		~~- $text = $p->getContent();~~
	189	+ $text = $article->getContent();
	190	+ }
	191	+
177	192	$text = $this->parser->replaceVariables( $text, $record, true );
178	193
179	194	return $text;
—	—	@@ -187,12 +202,12 @@
188	203	if ( isset( $record[ $f ] ) ) $v = $record[ $f ];
189	204	else $v = '';
190	205
191		~~- $rec[ $f ] = $this->sanitizeValue( $v );~~
	206	+ $rec[ $f ] = $this->sanitizeValue( $v ); //TESTME
192	207	}
193	208
194	209	//add source meta info, so we can render links back to the source,
195	210	//provide license info, etc
196		~~- $info = $this->source->getSourceInfo();~~
	211	+ $info = $this->source->getSourceInfo(); //TESTME
197	212	foreach ( $info as $f => $v ) {
198	213	if ( is_array( $v ) \|\| is_object( $v ) \|\| is_resource( $v ) ) continue;
199	214	$rec[ "source.$f" ] = $this->sanitizeValue( $v );
—	—	@@ -203,22 +218,24 @@
204	219
205	220	protected static $sanitizerSubstitution = array(
206	221	# '!&!' => '&', #breaks URLs. not really needed when parsed as wiki-text...
	222	+ '!&(#?x?[\w\d]+);!' => '&$1;',
207	223	'!<!' => '<',
208	224	'!>!' => '>',
209	225	'!\[!' => '[',
210	226	'!\]!' => ']',
211	227	'!\{!' => '{',
212	228	'!\}!' => '}',
213		~~- '!\'!' => '&#apos;',~~
	229	+ '!\'!' => ''',
214	230	'!\\|!' => '\|',
215	231	'!^\!m' => '',
216	232	'!^#!m' => '#',
217	233	'!^:!m' => ':',
218	234	'!^;!m' => ';',
	235	+ '![\r\n]!' => ' ',
219	236	'!^ !m' => ' ',
220	237	);
221	238
222		~~- function sanitizeValue( $v ) {~~
	239	+ static function sanitizeValue( $v ) {
223	240	$find = array_keys( self::$sanitizerSubstitution );
224	241	$subst = array_values( self::$sanitizerSubstitution );
225	242
Index: trunk/extensions/DataTransclusion/tests/DataTransclusionTest.php
—	—	@@ -0,0 +1,230 @@
	2	+<?php
	3	+if ( getenv( 'MW_INSTALL_PATH' ) ) {
	4	+ $IP = getenv( 'MW_INSTALL_PATH' );
	5	+} else {
	6	+ $dir = dirname( __FILE__ );
	7	+
	8	+ if ( file_exists( "$dir/../../LocalSettings.php" ) ) $IP = "$dir/../..";
	9	+ else if ( file_exists( "$dir/../../../LocalSettings.php" ) ) $IP = "$dir/../../..";
	10	+ else if ( file_exists( "$dir/../../phase3/LocalSettings.php" ) ) $IP = "$dir/../../phase3";
	11	+ else if ( file_exists( "$dir/../../../phase3/LocalSettings.php" ) ) $IP = "$dir/../../../phase3";
	12	+ else $IP = $dir;
	13	+}
	14	+
	15	+if( isset( $GET_ ) ) {
	16	+ echo( "This file cannot be run from the web.\n" );
	17	+ die( 1 );
	18	+}
	19	+
	20	+require_once( "$IP/maintenance/commandLine.inc" );
	21	+
	22	+// requires PHPUnit 3.4
	23	+require_once 'PHPUnit/Framework.php';
	24	+
	25	+error_reporting(E_ALL);
	26	+
	27	+class DataTransclusionTest extends PHPUnit_Framework_TestCase {
	28	+
	29	+ function setUp()
	30	+ {
	31	+ global $wgTitle;
	32	+
	33	+ $wgTitle = Title::newFromText( "Test" );
	34	+ }
	35	+
	36	+ function runTest()
	37	+ {
	38	+ $this->testErrorMessage();
	39	+ $this->testSanitizeValue();
	40	+ $this->testNormalizeRecord();
	41	+ $this->testBuildAssociativeArguments();
	42	+ $this->testGetDataSource();
	43	+ $this->testCachedFetchRecord();
	44	+ $this->testRender();
	45	+ $this->testHandleRecordFunction();
	46	+ $this->testHandleRecordTag();
	47	+ }
	48	+
	49	+ function testErrorMessage() {
	50	+ $m = DataTransclusionHandler::errorMessage('datatransclusion-test-wikitext', false);
	51	+ $this->assertEquals( $m, '<span class="error">some <span class="test">html</span> and \'\'markup\'\'.</span>' );
	52	+
	53	+ $m = DataTransclusionHandler::errorMessage('datatransclusion-test-evil-html', false);
	54	+ $this->assertEquals( $m, '<span class="error">some <object>evil</object> html.</span>' );
	55	+
	56	+ $m = DataTransclusionHandler::errorMessage('datatransclusion-test-nowiki', false);
	57	+ $this->assertEquals( $m, '<span class="error">some <nowiki>{{nowiki}}</nowiki> code.</span>' );
	58	+
	59	+ $m = DataTransclusionHandler::errorMessage('datatransclusion-test-wikitext', true);
	60	+ $this->assertEquals( $m, '<span class="error">some <span class="test">html</span> and <i>markup</i>.</span>' );
	61	+
	62	+ $m = DataTransclusionHandler::errorMessage('datatransclusion-test-evil-html', true);
	63	+ $this->assertEquals( $m, '<span class="error">some <object>evil</object> html.</span>' );
	64	+
	65	+ $m = DataTransclusionHandler::errorMessage('datatransclusion-test-nowiki', true);
	66	+ $this->assertEquals( $m, '<span class="error">some {{nowiki}} code.</span>' );
	67	+ }
	68	+
	69	+ function testSanitizeValue() {
	70	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo bar' ), 'foo bar' );
	71	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo &bar;' ), 'foo &bar;' );
	72	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo&bar' ), 'foo&bar' );
	73	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo <bar>' ), 'foo <bar>' );
	74	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo [[bar]]' ), 'foo [[bar]]' );
	75	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo {{bar}}' ), 'foo {{bar}}' );
	76	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo \'bar\'' ), 'foo 'bar'' );
	77	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo\|bar' ), 'foo\|bar' );
	78	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( '* foo bar' ), '* foo bar' );
	79	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foobar' ), 'foobar' );
	80	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( '#foo bar' ), '#foo bar' );
	81	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo#bar' ), 'foo#bar' );
	82	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( ':foo bar' ), ':foo bar' );
	83	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo:bar' ), 'foo:bar' );
	84	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( ';foo bar' ), ';foo bar' );
	85	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( 'foo;bar' ), 'foo;bar' );
	86	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( "foo\r\nbar" ), 'foo bar' );
	87	+ $this->assertEquals( DataTransclusionHandler::sanitizeValue( ' foo bar' ), ' foo bar' );
	88	+ }
	89	+
	90	+ function testNormalizeRecord() {
	91	+ //TODO...
	92	+ }
	93	+
	94	+ function testBuildAssociativeArguments() {
	95	+ $args = array( "foo bar", "x=y", " ah = \"be\" ", "blubber bla" );
	96	+ $assoc = DataTransclusionhandler::buildAssociativeArguments( $args );
	97	+
	98	+ $this->asserttrue( !isset($assoc[0]) );
	99	+ $this->asserttrue( !isset($assoc[3]) );
	100	+ $this->asserttrue( !isset($assoc['foo']) );
	101	+ $this->asserttrue( !isset($assoc['foo bar']) );
	102	+ $this->assertEquals( $assoc[1], 'foo bar' );
	103	+ $this->assertEquals( $assoc[2], 'blubber bla' );
	104	+ $this->assertEquals( $assoc['x'], 'y' );
	105	+ $this->assertEquals( $assoc['ah'], 'be' );
	106	+ }
	107	+
	108	+ function testGetDataSource() {
	109	+ global $wgDataTransclusionSources;
	110	+
	111	+ $spec = array( 'name' => 'FOO', 'keyFields' => 'name,id', 'fieldNames' => 'id,name,info' );
	112	+ $data[] = array( "name" => "foo", "id" => 3, "info" => 'test 1');
	113	+ $data[] = array( "name" => "bar", "id" => 5, "info" => 'test 2');
	114	+ $wgDataTransclusionSources[ 'FOO' ] = new FakeDataTransclusionSource( $spec, $data );
	115	+
	116	+ $src = DataTransclusionHandler::getDataSource( 'FOO' );
	117	+ $this->assertTrue( $src instanceof FakeDataTransclusionSource );
	118	+
	119	+ $rec = $src->fetchRecord( 'id', 3 );
	120	+ $this->assertEquals( $rec['id'], 3 );
	121	+ $this->assertEquals( $rec['name'], 'foo' );
	122	+ $this->assertEquals( $rec['info'], 'test 1' );
	123	+
	124	+ $rec = $src->fetchRecord( 'name', 'bar' );
	125	+ $this->assertEquals( $rec['id'], 5 );
	126	+ $this->assertEquals( $rec['name'], 'bar' );
	127	+ $this->assertEquals( $rec['info'], 'test 2' );
	128	+
	129	+ ///////////////////////////////////////////////////////////////////////////////
	130	+ $spec[ 'class' ] = 'FakeDataTransclusionSource';
	131	+ $spec[ 'data' ] = $data;
	132	+
	133	+ $wgDataTransclusionSources[ 'BAR' ] = $spec;
	134	+
	135	+ $src = DataTransclusionHandler::getdataSource( 'BAR' );
	136	+ $this->assertTrue( $src instanceof FakeDataTransclusionSource );
	137	+ $this->assertEquals( $src->getName(), 'BAR' );
	138	+
	139	+ $rec = $src->fetchRecord( 'id', 3 );
	140	+ $this->assertEquals( $rec['id'], 3 );
	141	+ $this->assertEquals( $rec['name'], 'foo' );
	142	+ $this->assertEquals( $rec['info'], 'test 1' );
	143	+
	144	+ $rec = $src->fetchRecord( 'name', 'bar' );
	145	+ $this->assertEquals( $rec['id'], 5 );
	146	+ $this->assertEquals( $rec['name'], 'bar' );
	147	+ $this->assertEquals( $rec['info'], 'test 2' );
	148	+
	149	+ $src = DataTransclusionHandler::getdataSource( 'XYZZY' );
	150	+ $this->assertTrue( $src === null \|\| $src === false );
	151	+ }
	152	+
	153	+ function testHandleRecordFunction() {
	154	+ }
	155	+
	156	+ function testHandleRecordTag() {
	157	+ }
	158	+
	159	+ function testRender() {
	160	+ global $wgParser;
	161	+
	162	+ $source = null;
	163	+ $title = Title::newFromText( "Template:Thing" );
	164	+ $rec = array( "name" => "foo", "id" => 3, "info" => 'test X');
	165	+ $template = "{{{id}}}\|{{{name}}}\|{{{info}}}";
	166	+
	167	+ $handler = new DataTransclusionHandler( $wgParser, $source, $title, $template );
	168	+ $res = $handler->render( $rec );
	169	+
	170	+ $this->assertEquals( $res, '3\|foo\|test X' );
	171	+ }
	172	+
	173	+ function testCachedFetchRecord() {
	174	+ global $wgDataTransclusionSources;
	175	+
	176	+ $data[] = array( "name" => "foo", "id" => 3, "info" => 'test 1');
	177	+ $data[] = array( "name" => "bar", "id" => 5, "info" => 'test 2');
	178	+ $spec = array(
	179	+ 'class' => 'FakeDataTransclusionSource',
	180	+ 'data' => $data,
	181	+ 'keyFields' => 'name,id',
	182	+ 'fieldNames' => 'id,name,info',
	183	+ 'cacheDuration' => 2,
	184	+ 'cache' => new HashBagOStuff(),
	185	+ );
	186	+
	187	+ $wgDataTransclusionSources[ 'FOO' ] = $spec;
	188	+
	189	+ $src = DataTransclusionHandler::getDataSource( 'FOO' );
	190	+ $this->assertTrue( $src instanceof CachingDataTransclusionSource );
	191	+
	192	+ //get original version
	193	+ $rec = $src->fetchRecord( 'id', 3 );
	194	+ $this->assertEquals( $rec['id'], 3 );
	195	+ $this->assertEquals( $rec['name'], 'foo' );
	196	+ $this->assertEquals( $rec['info'], 'test 1' );
	197	+
	198	+ //change record
	199	+ $rec = array( "name" => "foo", "id" => 3, "info" => 'test X');
	200	+ $src->source->putRecord( $rec );
	201	+
	202	+ //fetch record - should be the cached version
	203	+ $rec = $src->fetchRecord( 'id', 3 );
	204	+ $this->assertEquals( $rec['info'], 'test 1' );
	205	+
	206	+ sleep(3);
	207	+
	208	+ //fetch record - cached version should have expired
	209	+ $rec = $src->fetchRecord( 'id', 3 );
	210	+ $this->assertEquals( $rec['info'], 'test X' );
	211	+ }
	212	+
	213	+ /*
	214	+ function testWebFetchRecord() {
	215	+ //TODO: decode, extract, etc
	216	+ }
	217	+
	218	+ function testDBFetchRecord() {
	219	+ //TODO: convert value, escape, build sql
	220	+ }
	221	+ */
	222	+}
	223	+
	224	+$wgShowExceptionDetails = true;
	225	+
	226	+$t = new DataTransclusionTest();
	227	+$t->setUp();
	228	+$t->runTest();
	229	+
	230	+echo "OK.\n";
	231	+?>
\ No newline at end of file
Index: trunk/extensions/DataTransclusion/DataTransclusion.php
—	—	@@ -36,6 +36,8 @@
37	37
38	38	$wgHooks['ParserFirstCallInit'][] = 'efDataTransclusionSetHooks';
39	39
	40	+//TODO: Special Page for displaying all configured data sources
	41	+
40	42	$wgDataTransclusionSources = array();
41	43
42	44	function efDataTransclusionSetHooks( $parser ) {
Index: trunk/extensions/DataTransclusion/DataTransclusionSource.php
—	—	@@ -170,9 +170,9 @@
171	171
172	172	$rec = $this->cache->get( $cacheKey );
173	173
174		~~- if ( !$rec ) { //TESTME~~
	174	+ if ( !$rec ) {
175	175	$rec = $this->source->fetchRecord( $key, $value );
176		~~- if ( $rec ) $this->cache->set( $cacheKey, $rev, $this->getCacheDuration() ) ; //XXX: also cache negatives??~~
	176	+ if ( $rec ) $this->cache->set( $cacheKey, $rec, $this->getCacheDuration() ) ; //XXX: also cache negatives??
177	177	}
178	178
179	179	return $rec;
—	—	@@ -193,25 +193,31 @@
194	194	*
195	195	* @param $data an array containing a list of records. Records from
196	196	* this list can be accessed via fetchRecord() using the key fields specified
197		~~- * by $spec['keyFields'].~~
	197	+ * by $spec['keyFields']. If $data is not given, $spec['data'] must contain the data array.
198	198	*/
199		~~- function __construct( $spec, $data ) {~~
200		~~- DataTransclusionSource::__construct( $spec );~~
	199	+ function __construct( $spec, $data = null ) {
	200	+ DataTransclusionSource::__construct( $spec );
201	201
202		~~- $this->lookup = array();~~
	202	+ if ( $data === null ) $data = $spec[ 'data' ];
203	203
204		~~- $fields = $this->getKeyFields();~~
205		~~- foreach ( $fields as $f ) {~~
206		~~- $this->lookup[ $f ] = array();~~
	204	+ $this->lookup = array();
207	205
208	206	foreach ( $data as $rec ) {
209		~~- $k = $rec[ $f ];~~
210		~~- $this->lookup[ $f ][ $k ] = $rec;~~
	207	+ $this->putRecord( $rec );
211	208	}
212		~~- }~~
213	209	}
214	210
	211	+ public function putRecord( $record ) {
	212	+ $fields = $this->getKeyFields();
	213	+ foreach ( $fields as $f ) {
	214	+ $k = $record[ $f ];
	215	+
	216	+ if ( !isset( $this->lookup[ $f ] ) ) $this->lookup[ $f ] = array();
	217	+ $this->lookup[ $f ][ $k ] = $record;
	218	+ }
	219	+ }
	220	+
215	221	public function fetchRecord( $key, $value ) {
216		~~- return @$this->lookup[ $key ][ $value ];~~
	222	+ return @$this->lookup[ $key ][ $value ];
217	223	}
218	224	}

Follow-up revisions

Revision	Commit summary	Author	Date
r67747	reverting stylizification in r67480; need to commmit some stuff first.	daniel	19:17, 9 June 2010

Status & tagging log

15:04, 7 June 2010 MaxSem (talk | contribs) changed the status of r67480 [removed: new added: deferred]