r97223 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97222‎ | r97223 | r97224 >
Date:00:26, 16 September 2011
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
changing param handling to happen on query processor level instead of in the query printers; high risk commit, might break stuff somewhere
Modified paths:
  • /trunk/extensions/SemanticMediaWiki/RELEASE-NOTES (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/SMW_QueryProcessor.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/api/ApiSMWQuery.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/params/SMW_ParamFormat.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/parserhooks/SMW_Concept.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/queryprinters/SMW_QP_Auto.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/queryprinters/SMW_QueryPrinter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticMediaWiki/RELEASE-NOTES
——@@ -5,6 +5,7 @@
66
77 * Use of native MediaWiki sortable tables for the table formats.
88 * Fixed separator and filename parameters for the DSV format.
 9+* Fixed display of properties of type URL (bug 30912).
910 * Added definitions for missing params (sort, order, searchlabel) to the base query printer.
1011 * Added validation and manipulation of the format paremeter using Validator.
1112
Index: trunk/extensions/SemanticMediaWiki/includes/SMW_QueryProcessor.php
——@@ -22,6 +22,26 @@
2323 const CONCEPT_DESC = 2; // query for concept definition
2424
2525 /**
 26+ * Takes an array of unprocessed parameters,
 27+ * processes them using Validator, and returns them.
 28+ *
 29+ * Both input and output arrays are
 30+ * param name (string) => param value (mixed)
 31+ *
 32+ * @since 1.6.2
 33+ *
 34+ * @param array $params
 35+ *
 36+ * @return array
 37+ */
 38+ public static function getProcessedParams( array $params ) {
 39+ $validator = new Validator();
 40+ $validator->setParameters( $params, self::getParameters(), false );
 41+ $validator->validateParameters();
 42+ return $validator->getParameterValues();
 43+ }
 44+
 45+ /**
2646 * Parse a query string given in SMW's query language to create
2747 * an SMWQuery. Parameters are given as key-value-pairs in the
2848 * given array. The parameter $context defines in what context the
——@@ -32,18 +52,17 @@
3353 * known. Otherwise it will be determined from the parameters when
3454 * needed. This parameter is just for optimisation in a common case.
3555 *
 56+ * @param string $querystring
 57+ * @param array $params These need to be the result of a list fed to getProcessedParams
 58+ * @param $context
 59+ * @param string $format
 60+ * @param array $extraprintouts
 61+ *
3662 * @return SMWQuery
3763 */
38 - static public function createQuery( $querystring, array $params, $context = self::INLINE_QUERY, $format = '', $extraprintouts = array() ) {
 64+ static public function createQuery( $querystring, array $params, $context = self::INLINE_QUERY, $format = '', array $extraprintouts = array() ) {
3965 global $smwgQDefaultNamespaces, $smwgQFeatures, $smwgQConceptFeatures;
4066
41 - // TODO: this is a hack
42 - // Idially one round of Validation would be done through Validator on this level
43 - // instead of on query printer level and then have weirdness here.
44 - $formatParam = new SMWParamFormat();
45 - $p = array();
46 - $formatParam->doManipulation( $format, new Parameter( 'format' ), $p );
47 -
4867 // parse query:
4968 $queryfeatures = ( $context == self::CONCEPT_DESC ) ? $smwgQConceptFeatures : $smwgQFeatures;
5069 $qp = new SMWQueryParser( $queryfeatures );
——@@ -95,8 +114,13 @@
96115 }
97116
98117 // determine sortkeys and ascendings:
99 - if ( array_key_exists( 'order', $params ) ) {
100 - $orders = explode( ',', $params['order'] );
 118+ if ( array_key_exists( 'order', $params ) && !is_null( $params['order'] ) ) {
 119+ // Compatibility with query printers not using Validator yet
 120+ if ( is_string( $params['order'] ) ) {
 121+ $params['order'] = explode( ',', $params['order'] );
 122+ }
 123+
 124+ $orders = $params['order'];
101125
102126 foreach ( $orders as $key => $order ) { // normalise
103127 $order = strtolower( trim( $order ) );
——@@ -114,11 +138,16 @@
115139
116140 reset( $orders );
117141
118 - if ( array_key_exists( 'sort', $params ) ) {
 142+ if ( array_key_exists( 'sort', $params ) && !is_null( $params['sort'] ) ) {
 143+ // Compatibility with query printers not using Validator yet
 144+ if ( is_string( $params['sort'] ) ) {
 145+ $params['sort'] = explode( ',', $params['sort'] );
 146+ }
 147+
119148 $query->sort = true;
120149 $query->sortkeys = array();
121150
122 - foreach ( explode( ',', $params['sort'] ) as $sort ) {
 151+ foreach ( $params['sort'] as $sort ) {
123152 $propertyValue = SMWPropertyValue::makeUserProperty( trim( $sort ) );
124153 if ( $propertyValue->isValid() ) {
125154 $sortkey = $propertyValue->getDataItem()->getKey();
——@@ -256,6 +285,7 @@
257286 */
258287 static public function getResultFromFunctionParams( array $rawparams, $outputmode, $context = self::INLINE_QUERY, $showmode = false ) {
259288 self::processFunctionParams( $rawparams, $querystring, $params, $printouts, $showmode );
 289+ $params = self::getProcessedParams( $params );
260290 return self::getResultFromQueryString( $querystring, $params, $printouts, SMW_OUTPUT_WIKI, $context );
261291 }
262292
——@@ -266,11 +296,17 @@
267297 * parameter $context defines in what context the query is used, which affects
268298 * certain general settings. Finally, $extraprintouts supplies additional
269299 * printout requests for the query results.
 300+ *
 301+ * @param string $querystring
 302+ * @param array $params These need to be the result of a list fed to getProcessedParams
 303+ * @param $extraprintouts
 304+ * @param $outputmode
 305+ * @param $context
270306 */
271307 static public function getResultFromQueryString( $querystring, array $params, $extraprintouts, $outputmode, $context = self::INLINE_QUERY ) {
272308 wfProfileIn( 'SMWQueryProcessor::getResultFromQueryString (SMW)' );
273309
274 - $format = array_key_exists( 'format', $params ) ? $params['format'] : '';
 310+ $format = $params['format']; // This is rather silly to do now...
275311 $query = self::createQuery( $querystring, $params, $context, $format, $extraprintouts );
276312 $result = self::getResultFromQuery( $query, $params, $extraprintouts, $outputmode, $context, $format );
277313
——@@ -279,15 +315,22 @@
280316 return $result;
281317 }
282318
 319+ /**
 320+ * TODO: document
 321+ *
 322+ * @param SMWQuery $query
 323+ * @param array $params These need to be the result of a list fed to getProcessedParams
 324+ * @param $extraprintouts
 325+ * @param $outputmode
 326+ * @param $context
 327+ * @param $format
 328+ *
 329+ * @return string
 330+ */
283331 static public function getResultFromQuery( SMWQuery $query, array $params, $extraprintouts, $outputmode, $context = self::INLINE_QUERY, $format = '' ) {
284332 wfProfileIn( 'SMWQueryProcessor::getResultFromQuery (SMW)' );
285333
286 - // TODO: this is a hack
287 - // Idially one round of Validation would be done through Validator on this level
288 - // instead of on query printer level and then have weirdness here.
289 - $formatParam = new SMWParamFormat();
290 - $p = array();
291 - $formatParam->doManipulation( $format, new Parameter( 'format' ), $p );
 334+ //$params = self::getProcessedParams( $params );
292335
293336 // Query routing allows extensions to provide alternative stores as data sources
294337 // The while feature is experimental and is not properly integrated with most of SMW's architecture. For instance, some query printers just fetch their own store.
——@@ -356,5 +399,97 @@
357400
358401 return new $formatClass( $format, ( $context != self::SPECIAL_PAGE ) );
359402 }
 403+
 404+ /**
 405+ * Determines the format from an array of parameters, and returns it.
 406+ *
 407+ * @deprecated since 1.6.2, removal in 1.8
 408+ *
 409+ * @param array $params
 410+ *
 411+ * @return string
 412+ */
 413+ static protected function getResultFormat( array $params ) {
 414+ $format = 'auto';
360415
 416+ if ( array_key_exists( 'format', $params ) ) {
 417+ global $smwgResultFormats;
 418+
 419+ $format = strtolower( trim( $params['format'] ) );
 420+
 421+ if ( !array_key_exists( $format, $smwgResultFormats ) ) {
 422+ $isAlias = SMWParamFormat::resolveFormatAliases( $format );
 423+ if ( !$isAlias ) {
 424+ $format = 'auto'; // If it is an unknown format, defaults to list/table again
 425+ }
 426+ }
 427+ }
 428+
 429+ return $format;
 430+ }
 431+
 432+ /**
 433+ * A function to describe the allowed parameters of a query using
 434+ * any specific format - most query printers should override this
 435+ * function.
 436+ *
 437+ * TODO: refactor non-printer params up to the query processor
 438+ * and do all param handling there.
 439+ *
 440+ * @since 1.6.2
 441+ *
 442+ * @return array
 443+ */
 444+ public static function getParameters() {
 445+ $params = array();
 446+
 447+ $allowedFormats = $GLOBALS['smwgResultFormats'];
 448+
 449+ foreach ( $GLOBALS['smwgResultAliases'] as $aliases ) {
 450+ $allowedFormats += $aliases;
 451+ }
 452+
 453+ $params['format'] = new Parameter( 'format' );
 454+ $params['format']->setDefault( 'auto' );
 455+ //$params['format']->addCriteria( new CriterionInArray( $allowedFormats ) );
 456+ $params['format']->addManipulations( new SMWParamFormat() );
 457+
 458+ $params['limit'] = new Parameter( 'limit', Parameter::TYPE_INTEGER );
 459+ $params['limit']->setMessage( 'smw_paramdesc_limit' );
 460+ $params['limit']->setDefault( 20 );
 461+
 462+ $params['sort'] = new ListParameter( 'sort' );
 463+ $params['sort']->setMessage( 'smw-paramdesc-sort' );
 464+ $params['sort']->setDefault( array() );
 465+
 466+ $params['order'] = new ListParameter( 'order' );
 467+ $params['order']->setMessage( 'smw-paramdesc-order' );
 468+ $params['order']->setDefault( array() );
 469+ $params['order']->addCriteria( new CriterionInArray( 'descending', 'desc', 'asc', 'ascending', 'rand', 'random' ) );
 470+
 471+ $params['offset'] = new Parameter( 'offset', Parameter::TYPE_INTEGER );
 472+ $params['offset']->setMessage( 'smw_paramdesc_offset' );
 473+ $params['offset']->setDefault( 0 );
 474+
 475+ $params['headers'] = new Parameter( 'headers' );
 476+ $params['headers']->setMessage( 'smw_paramdesc_headers' );
 477+ $params['headers']->addCriteria( new CriterionInArray( 'show', 'hide', 'plain' ) );
 478+ $params['headers']->setDefault( 'show' );
 479+
 480+ $params['mainlabel'] = new Parameter( 'mainlabel' );
 481+ $params['mainlabel']->setMessage( 'smw_paramdesc_mainlabel' );
 482+ $params['mainlabel']->setDefault( false, false );
 483+
 484+ $params['link'] = new Parameter( 'link' );
 485+ $params['link']->setMessage( 'smw_paramdesc_link' );
 486+ $params['link']->addCriteria( new CriterionInArray( 'all', 'subject', 'none' ) );
 487+ $params['link']->setDefault( 'all' );
 488+
 489+ $params['searchlabel'] = new Parameter( 'searchlabel' );
 490+ $params['searchlabel']->setDefault( '' );
 491+ $params['searchlabel']->setMessage( 'smw-paramdesc-searchlabel' );
 492+
 493+ return $params;
 494+ }
 495+
361496 }
\ No newline at end of file
Index: trunk/extensions/SemanticMediaWiki/includes/params/SMW_ParamFormat.php
——@@ -26,9 +26,29 @@
2727 /**
2828 * @see ItemParameterManipulation::doManipulation
2929 *
30 - * @since 0.7
 30+ * @since 1.6.2
3131 */
3232 public function doManipulation( &$value, Parameter $parameter, array &$parameters ) {
 33+ // Make sure the format value is valid.
 34+ $value = self::getValidFormatName( $value );
 35+
 36+ // Add the formats parameters to the parameter list.
 37+ $queryPrinter = SMWQueryProcessor::getResultPrinter( $value );
 38+ $parameters = array_merge( $parameters, $queryPrinter->getValidatorParameters() );
 39+ }
 40+
 41+ /**
 42+ * Takes a format name, which can be an alias and returns something
 43+ * that certainly valid. Aliases are resvolved. Invalid formats
 44+ * will result into 'auto' being returned.
 45+ *
 46+ * @since 1.6.2
 47+ *
 48+ * @param string $value
 49+ *
 50+ * @return string
 51+ */
 52+ protected function getValidFormatName( $value ) {
3353 global $smwgResultFormats;
3454
3555 $value = trim( $value );
——@@ -40,16 +60,20 @@
4161 $value = 'auto'; // If it is an unknown format, defaults to list/table again
4262 }
4363 }
 64+
 65+ return $value;
4466 }
4567
4668 /**
4769 * Turns format aliases into main formats.
4870 *
 71+ * @since 1.6.2
 72+ *
4973 * @param string $format
5074 *
5175 * @return boolean Indicates if the passed format was an alias, and thus was changed.
5276 */
53 - static protected function resolveFormatAliases( &$format ) {
 77+ public static function resolveFormatAliases( &$format ) {
5478 global $smwgResultAliases;
5579
5680 $isAlias = false;
Index: trunk/extensions/SemanticMediaWiki/includes/queryprinters/SMW_QP_Auto.php
——@@ -66,5 +66,16 @@
6767 smwfLoadExtensionMessages( 'SemanticMediaWiki' );
6868 return wfMsg( 'smw_printername_auto' );
6969 }
 70+
 71+ public function getParameters() {
 72+ // TODO: this assumes table, which is not correct when it should be list
 73+
 74+ $printer = SMWQueryProcessor::getResultPrinter(
 75+ 'table',
 76+ $this->mInline ? SMWQueryProcessor::INLINE_QUERY : SMWQueryProcessor::SPECIAL_PAGE
 77+ );
 78+
 79+ return $printer->getParameters();
 80+ }
7081
7182 }
Index: trunk/extensions/SemanticMediaWiki/includes/queryprinters/SMW_QueryPrinter.php
——@@ -158,15 +158,7 @@
159159 $this->hasTemplates = false;
160160
161161 if ( $this->useValidator ) {
162 - $paramValidationResult = $this->handleRawParameters( $params );
163 -
164 - if ( is_array( $paramValidationResult ) ) {
165 - $this->handleParameters( $paramValidationResult, $outputmode );
166 - }
167 - else {
168 - $this->addError( $paramValidationResult );
169 - return $this->getErrorString( $results );
170 - }
 162+ $this->handleParameters( $params, $outputmode );
171163 }
172164 else {
173165 $this->readParameters( $params, $outputmode );
——@@ -291,31 +283,6 @@
292284
293285 return $result;
294286 }
295 -
296 - /**
297 - * Handles the user-provided parameters and returns the processes key-value pairs.
298 - * If there is a fatal error, a string with the error message will be returned intsead.
299 - *
300 - * @since 1.6
301 - *
302 - * @param array $keyValuePairs
303 - *
304 - * @return array or string
305 - */
306 - protected function handleRawParameters( array $keyValuePairs ) {
307 - $validator = new Validator();
308 - $validator->setParameters( $keyValuePairs, $this->getParameters() );
309 - $validator->validateParameters();
310 - $fatalError = $validator->hasFatalError();
311 -
312 - if ( $fatalError === false ) {
313 - $this->mErrors = $validator->getErrorMessages();
314 - return $validator->getParameterValues();
315 - }
316 - else {
317 - return $fatalError->getMessage();
318 - }
319 - }
320287
321288 /**
322289 * Read an array of parameter values given as key-value-pairs and
——@@ -613,77 +580,14 @@
614581 $params = array();
615582
616583 foreach ( $this->getParameters() as $param ) {
617 - $params[] = $this->toValidatorParam( $param );
 584+ $param = $this->toValidatorParam( $param );
 585+ $params[$param->getName()] = $param;
618586 }
619587
620588 return $params;
621589 }
622590
623591 /**
624 - * A function to describe the allowed parameters of a query using
625 - * any specific format - most query printers should override this
626 - * function.
627 - *
628 - * TODO: refactor non-printer params up to the query processor
629 - * and do all param handling there.
630 - *
631 - * @since 1.5.0
632 - *
633 - * @return array
634 - */
635 - public function getParameters() {
636 - $params = array();
637 -
638 - $allowedFormats = $GLOBALS['smwgResultFormats'];
639 -
640 - foreach ( $GLOBALS['smwgResultAliases'] as $aliases ) {
641 - $allowedFormats += $aliases;
642 - }
643 -
644 - $params['format'] = new Parameter( 'format' );
645 - $params['format']->setDefault( 'auto' );
646 - //$params['format']->addCriteria( new CriterionInArray( $allowedFormats ) );
647 - $params['format']->addManipulations( new SMWParamFormat() );
648 -
649 - $params['limit'] = new Parameter( 'limit', Parameter::TYPE_INTEGER );
650 - $params['limit']->setMessage( 'smw_paramdesc_limit' );
651 - $params['limit']->setDefault( 20 );
652 -
653 - $params['sort'] = new ListParameter( 'sort' );
654 - $params['sort']->setMessage( 'smw-paramdesc-sort' );
655 - $params['sort']->setDefault( '' );
656 -
657 - $params['order'] = new ListParameter( 'order' );
658 - $params['order']->setMessage( 'smw-paramdesc-order' );
659 - $params['order']->setDefault( '' );
660 - $params['order']->addCriteria( new CriterionInArray( 'descending', 'desc', 'asc', 'ascending', 'rand', 'random' ) );
661 -
662 - $params['offset'] = new Parameter( 'offset', Parameter::TYPE_INTEGER );
663 - $params['offset']->setMessage( 'smw_paramdesc_offset' );
664 - $params['offset']->setDefault( 0 );
665 -
666 - $params['headers'] = new Parameter( 'headers' );
667 - $params['headers']->setMessage( 'smw_paramdesc_headers' );
668 - $params['headers']->addCriteria( new CriterionInArray( 'show', 'hide', 'plain' ) );
669 - $params['headers']->setDefault( 'show' );
670 -
671 - $params['mainlabel'] = new Parameter( 'mainlabel' );
672 - $params['mainlabel']->setMessage( 'smw_paramdesc_mainlabel' );
673 - $params['mainlabel']->setDefault( false, false );
674 -
675 - $params['link'] = new Parameter( 'link' );
676 - $params['link']->setMessage( 'smw_paramdesc_link' );
677 - $params['link']->addCriteria( new CriterionInArray( 'all', 'subject', 'none' ) );
678 - $params['link']->setDefault( 'all' );
679 -
680 - $params['searchlabel'] = new Parameter( 'searchlabel' );
681 - $params['searchlabel']->setDefault( '' );
682 - $params['searchlabel']->setMessage( 'smw-paramdesc-searchlabel' );
683 -
684 - return $params;
685 - }
686 -
687 - /**
688592 * Returns a Validator style Parameter definition.
689593 * SMW 1.5.x style definitions are converted.
690594 *
——@@ -722,5 +626,21 @@
723627 return $param;
724628 }
725629 }
 630+
 631+ /**
 632+ * A function to describe the allowed parameters of a query using
 633+ * any specific format - most query printers should override this
 634+ * function.
 635+ *
 636+ * TODO: refactor non-printer params up to the query processor
 637+ * and do all param handling there.
 638+ *
 639+ * @since 1.5
 640+ *
 641+ * @return array
 642+ */
 643+ public function getParameters() {
 644+ return array();
 645+ }
726646
727647 }
Index: trunk/extensions/SemanticMediaWiki/includes/parserhooks/SMW_Concept.php
——@@ -54,7 +54,12 @@
5555 $concept_docu = array_shift( $params );
5656
5757 // NOTE: the str_replace above is required in MediaWiki 1.11, but not in MediaWiki 1.14
58 - $query = SMWQueryProcessor::createQuery( $concept_input, array( 'limit' => 20, 'format' => 'list' ), SMWQueryProcessor::CONCEPT_DESC );
 58+ $query = SMWQueryProcessor::createQuery(
 59+ $concept_input,
 60+ SMWQueryProcessor::getProcessedParams( array( 'limit' => 20, 'format' => 'list' ) ),
 61+ SMWQueryProcessor::CONCEPT_DESC
 62+ );
 63+
5964 $concept_text = $query->getDescription()->getQueryString();
6065
6166 if ( SMWParseData::getSMWData( $parser ) !== null ) {
Index: trunk/extensions/SemanticMediaWiki/includes/api/ApiSMWQuery.php
——@@ -36,7 +36,7 @@
3737 */
3838 protected function getQuery( $queryString ) {
3939 // SMWQueryProcessor::processFunctionParams( $rawparams, $queryString, $m_params, $m_printouts);
40 - return SMWQueryProcessor::createQuery( $queryString, $this->parameters, SMWQueryProcessor::SPECIAL_PAGE );
 40+ return SMWQueryProcessor::createQuery( $queryString, SMWQueryProcessor::getProcessedParams( $this->parameters ), SMWQueryProcessor::SPECIAL_PAGE );
4141 }
4242
4343 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r97225Follow up to r97223;jeroendedauw00:53, 16 September 2011
r97255Follow up to r97223;jeroendedauw12:24, 16 September 2011
r97256Follow up to r97223;jeroendedauw12:24, 16 September 2011

Comments

#Comment by Nikerabbit (talk | contribs)   07:57, 16 September 2011

And it did: 

[16-Sep-2011 07:48:32] PHP Notice:  Undefined index: mappingservice in /www/w/extensions/Maps/includes/manipulations/Maps_ParamGeoService.php on line 53
[16-Sep-2011 07:48:32] PHP Fatal error:  Call to a member function getValue() on a non-object in /www/w/extensions/Maps/includes/manipulations/Maps_ParamGeoService.php on line 53
#Comment by Jeroen De Dauw (talk | contribs)   12:25, 16 September 2011

Should be fixed by follow up :)

Status & tagging log