r102947 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102946‎ | r102947 | r102948 >
Date:08:19, 14 November 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
(bug 26909) Add dir parameter for prop= API modules. Modified patch by Umherirrender
Modified paths:
  • /trunk/phase3/includes/api/ApiQueryBase.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryCategories.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryDuplicateFiles.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryIWLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryImages.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLangLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLinks.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiQueryLinks.php
@@ -48,6 +48,7 @@
4949 $this->prefix = 'pl';
5050 $this->description = 'link';
5151 $this->titlesParam = 'titles';
 52+ $this->titlesParamDescription = 'Only list links to these titles. Useful for checking whether a certain page links to a certain title.';
5253 $this->helpUrl = 'http://www.mediawiki.org/wiki/API:Properties#links_.2F_pl';
5354 break;
5455 case self::TEMPLATES:
@@ -55,6 +56,7 @@
5657 $this->prefix = 'tl';
5758 $this->description = 'template';
5859 $this->titlesParam = 'templates';
 60+ $this->titlesParamDescription = 'Only list these templates. Useful for checking whether a certain page uses a certain template.';
5961 $this->helpUrl = 'http://www.mediawiki.org/wiki/API:Properties#templates_.2F_tl';
6062 break;
6163 default:
@@ -131,6 +133,7 @@
132134 );
133135 }
134136
 137+ $dir = ( $params['dir'] == 'descending' ? ' DESC' : '' );
135138 // Here's some MySQL craziness going on: if you use WHERE foo='bar'
136139 // and later ORDER BY foo MySQL doesn't notice the ORDER BY is pointless
137140 // but instead goes and filesorts, because the index for foo was used
@@ -138,15 +141,15 @@
139142 // clause from the ORDER BY clause
140143 $order = array();
141144 if ( count( $this->getPageSet()->getGoodTitles() ) != 1 ) {
142 - $order[] = "{$this->prefix}_from";
 145+ $order[] = $this->prefix . '_from' . $dir;
143146 }
144147 if ( count( $params['namespace'] ) != 1 ) {
145 - $order[] = "{$this->prefix}_namespace";
 148+ $order[] = $this->prefix . '_namespace' . $dir;
146149 }
147150
148 - $order[] = "{$this->prefix}_title";
149 - $this->addOption( 'ORDER BY', implode( ', ', $order ) );
150 - $this->addOption( 'USE INDEX', "{$this->prefix}_from" );
 151+ $order[] = $this->prefix . "_title" . $dir;
 152+ $this->addOption( 'ORDER BY', $order );
 153+ $this->addOption( 'USE INDEX', $this->prefix . '_from' );
151154 $this->addOption( 'LIMIT', $params['limit'] + 1 );
152155
153156 $res = $this->select( __METHOD__ );
@@ -207,22 +210,25 @@
208211 $this->titlesParam => array(
209212 ApiBase::PARAM_ISMULTI => true,
210213 ),
 214+ 'dir' => array(
 215+ ApiBase::PARAM_DFLT => 'ascending',
 216+ ApiBase::PARAM_TYPE => array(
 217+ 'ascending',
 218+ 'descending'
 219+ )
 220+ ),
211221 );
212222 }
213223
214224 public function getParamDescription() {
215225 $desc = $this->description;
216 - $arr = array(
 226+ return array(
217227 'namespace' => "Show {$desc}s in this namespace(s) only",
218228 'limit' => "How many {$desc}s to return",
219229 'continue' => 'When more results are available, use this to continue',
 230+ $this->titlesParam => $this->titlesParamDescription,
 231+ 'dir' => 'The direction in which to list',
220232 );
221 - if ( $this->getModuleName() == self::LINKS ) {
222 - $arr[$this->titlesParam] = 'Only list links to these titles. Useful for checking whether a certain page links to a certain title.';
223 - } elseif ( $this->getModuleName() == self::TEMPLATES ) {
224 - $arr[$this->titlesParam] = 'Only list these templates. Useful for checking whether a certain page uses a certain template.';
225 - }
226 - return $arr;
227233 }
228234
229235 public function getDescription() {
Index: trunk/phase3/includes/api/ApiQueryBase.php
@@ -214,11 +214,10 @@
215215
216216 if ( $sort ) {
217217 $order = $field . ( $isDirNewer ? '' : ' DESC' );
218 - if ( !isset( $this->options['ORDER BY'] ) ) {
219 - $this->addOption( 'ORDER BY', $order );
220 - } else {
221 - $this->addOption( 'ORDER BY', $this->options['ORDER BY'] . ', ' . $order );
222 - }
 218+ // Append ORDER BY
 219+ $optionOrderBy = isset( $this->options['ORDER BY'] ) ? (array)$this->options['ORDER BY'] : array();
 220+ $optionOrderBy[] = $order;
 221+ $this->addOption( 'ORDER BY', $optionOrderBy );
223222 }
224223 }
225224
Index: trunk/phase3/includes/api/ApiQueryIWLinks.php
@@ -79,20 +79,27 @@
8080 );
8181 }
8282
 83+ $dir = ( $params['dir'] == 'descending' ? ' DESC' : '' );
8384 if ( isset( $params['prefix'] ) ) {
8485 $this->addWhereFld( 'iwl_prefix', $params['prefix'] );
8586 if ( isset( $params['title'] ) ) {
8687 $this->addWhereFld( 'iwl_title', $params['title'] );
87 - $this->addOption( 'ORDER BY', 'iwl_from' );
 88+ $this->addOption( 'ORDER BY', 'iwl_from' . $dir );
8889 } else {
89 - $this->addOption( 'ORDER BY', 'iwl_title, iwl_from' );
 90+ $this->addOption( 'ORDER BY', array(
 91+ 'iwl_title' . $dir,
 92+ 'iwl_from' . $dir
 93+ ));
9094 }
9195 } else {
9296 // Don't order by iwl_from if it's constant in the WHERE clause
9397 if ( count( $this->getPageSet()->getGoodTitles() ) == 1 ) {
94 - $this->addOption( 'ORDER BY', 'iwl_prefix' );
 98+ $this->addOption( 'ORDER BY', 'iwl_prefix' . $dir );
9599 } else {
96 - $this->addOption( 'ORDER BY', 'iwl_from, iwl_prefix' );
 100+ $this->addOption( 'ORDER BY', array (
 101+ 'iwl_from' . $dir,
 102+ 'iwl_prefix' . $dir
 103+ ));
97104 }
98105 }
99106
@@ -142,6 +149,13 @@
143150 'continue' => null,
144151 'prefix' => null,
145152 'title' => null,
 153+ 'dir' => array(
 154+ ApiBase::PARAM_DFLT => 'ascending',
 155+ ApiBase::PARAM_TYPE => array(
 156+ 'ascending',
 157+ 'descending'
 158+ )
 159+ ),
146160 );
147161 }
148162
@@ -152,6 +166,7 @@
153167 'continue' => 'When more results are available, use this to continue',
154168 'prefix' => 'Prefix for the interwiki',
155169 'title' => "Interwiki link to search for. Must be used with {$this->getModulePrefix()}prefix",
 170+ 'dir' => 'The direction in which to list',
156171 );
157172 }
158173
Index: trunk/phase3/includes/api/ApiQueryLangLinks.php
@@ -74,20 +74,27 @@
7575 );
7676 }
7777
 78+ $dir = ( $params['dir'] == 'descending' ? ' DESC' : '' );
7879 if ( isset( $params['lang'] ) ) {
7980 $this->addWhereFld( 'll_lang', $params['lang'] );
8081 if ( isset( $params['title'] ) ) {
8182 $this->addWhereFld( 'll_title', $params['title'] );
82 - $this->addOption( 'ORDER BY', 'll_from' );
 83+ $this->addOption( 'ORDER BY', 'll_from' . $dir );
8384 } else {
84 - $this->addOption( 'ORDER BY', 'll_title, ll_from' );
 85+ $this->addOption( 'ORDER BY', array(
 86+ 'll_title' . $dir,
 87+ 'll_from' . $dir
 88+ ));
8589 }
8690 } else {
8791 // Don't order by ll_from if it's constant in the WHERE clause
8892 if ( count( $this->getPageSet()->getGoodTitles() ) == 1 ) {
89 - $this->addOption( 'ORDER BY', 'll_lang' );
 93+ $this->addOption( 'ORDER BY', 'll_lang' . $dir );
9094 } else {
91 - $this->addOption( 'ORDER BY', 'll_from, ll_lang' );
 95+ $this->addOption( 'ORDER BY', array(
 96+ 'll_from' . $dir,
 97+ 'll_lang' . $dir
 98+ ));
9299 }
93100 }
94101
@@ -135,6 +142,13 @@
136143 'url' => false,
137144 'lang' => null,
138145 'title' => null,
 146+ 'dir' => array(
 147+ ApiBase::PARAM_DFLT => 'ascending',
 148+ ApiBase::PARAM_TYPE => array(
 149+ 'ascending',
 150+ 'descending'
 151+ )
 152+ ),
139153 );
140154 }
141155
@@ -145,6 +159,7 @@
146160 'url' => 'Whether to get the full URL',
147161 'lang' => 'Language code',
148162 'title' => "Link to search for. Must be used with {$this->getModulePrefix()}lang",
 163+ 'dir' => 'The direction in which to list',
149164 );
150165 }
151166
Index: trunk/phase3/includes/api/ApiQueryCategories.php
@@ -127,11 +127,16 @@
128128 }
129129
130130 $this->addOption( 'USE INDEX', array( 'categorylinks' => 'cl_from' ) );
 131+
 132+ $dir = ( $params['dir'] == 'descending' ? ' DESC' : '' );
131133 // Don't order by cl_from if it's constant in the WHERE clause
132134 if ( count( $this->getPageSet()->getGoodTitles() ) == 1 ) {
133 - $this->addOption( 'ORDER BY', 'cl_to' );
 135+ $this->addOption( 'ORDER BY', 'cl_to' . $dir );
134136 } else {
135 - $this->addOption( 'ORDER BY', "cl_from, cl_to" );
 137+ $this->addOption( 'ORDER BY', array(
 138+ 'cl_from' . $dir,
 139+ 'cl_to' . $dir
 140+ ));
136141 }
137142
138143 $res = $this->select( __METHOD__ );
@@ -213,6 +218,13 @@
214219 'categories' => array(
215220 ApiBase::PARAM_ISMULTI => true,
216221 ),
 222+ 'dir' => array(
 223+ ApiBase::PARAM_DFLT => 'ascending',
 224+ ApiBase::PARAM_TYPE => array(
 225+ 'ascending',
 226+ 'descending'
 227+ )
 228+ ),
217229 );
218230 }
219231
@@ -228,6 +240,7 @@
229241 'show' => 'Which kind of categories to show',
230242 'continue' => 'When more results are available, use this to continue',
231243 'categories' => 'Only list these categories. Useful for checking whether a certain page is in a certain category',
 244+ 'dir' => 'The direction in which to list',
232245 );
233246 }
234247
Index: trunk/phase3/includes/api/ApiQueryDuplicateFiles.php
@@ -94,7 +94,8 @@
9595 );
9696 }
9797
98 - $this->addOption( 'ORDER BY', 'i1.img_name' );
 98+ $dir = ( $params['dir'] == 'descending' ? ' DESC' : '' );
 99+ $this->addOption( 'ORDER BY', 'i1.img_name' . $dir );
99100 $this->addOption( 'LIMIT', $params['limit'] + 1 );
100101
101102 $res = $this->select( __METHOD__ );
@@ -141,6 +142,13 @@
142143 ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2
143144 ),
144145 'continue' => null,
 146+ 'dir' => array(
 147+ ApiBase::PARAM_DFLT => 'ascending',
 148+ ApiBase::PARAM_TYPE => array(
 149+ 'ascending',
 150+ 'descending'
 151+ )
 152+ ),
145153 );
146154 }
147155
@@ -148,6 +156,7 @@
149157 return array(
150158 'limit' => 'How many files to return',
151159 'continue' => 'When more results are available, use this to continue',
 160+ 'dir' => 'The direction in which to list',
152161 );
153162 }
154163
Index: trunk/phase3/includes/api/ApiQueryImages.php
@@ -79,11 +79,15 @@
8080 );
8181 }
8282
 83+ $dir = ( $params['dir'] == 'descending' ? ' DESC' : '' );
8384 // Don't order by il_from if it's constant in the WHERE clause
8485 if ( count( $this->getPageSet()->getGoodTitles() ) == 1 ) {
85 - $this->addOption( 'ORDER BY', 'il_to' );
 86+ $this->addOption( 'ORDER BY', 'il_to' . $dir );
8687 } else {
87 - $this->addOption( 'ORDER BY', 'il_from, il_to' );
 88+ $this->addOption( 'ORDER BY', array(
 89+ 'il_from' . $dir,
 90+ 'il_to' . $dir
 91+ ));
8892 }
8993 $this->addOption( 'LIMIT', $params['limit'] + 1 );
9094
@@ -154,7 +158,14 @@
155159 'continue' => null,
156160 'images' => array(
157161 ApiBase::PARAM_ISMULTI => true,
158 - )
 162+ ),
 163+ 'dir' => array(
 164+ ApiBase::PARAM_DFLT => 'ascending',
 165+ ApiBase::PARAM_TYPE => array(
 166+ 'ascending',
 167+ 'descending'
 168+ )
 169+ ),
159170 );
160171 }
161172
@@ -163,6 +174,7 @@
164175 'limit' => 'How many images to return',
165176 'continue' => 'When more results are available, use this to continue',
166177 'images' => 'Only list these images. Useful for checking whether a certain page has a certain Image.',
 178+ 'dir' => 'The direction in which to list',
167179 );
168180 }
169181

Comments

#Comment by Nikerabbit (talk | contribs)   09:40, 16 December 2011

dir is a bit ambiguous - it is probably too late to change it now. Still it's inconsistent that the values are not abbreviated.

#Comment by Krinkle (talk | contribs)   15:58, 2 January 2012

It is indeed inconsistent in that regard. But it is in consistency with the "dir" property of the list modules such as "action=query&list=allpages" which also use "dir" with ascending/descending values.

#Comment by Duplicatebug (talk | contribs)   22:40, 6 January 2012

list=categorymembers is using the abbreviation "asc" and "desc" ...

#Comment by Duplicatebug (talk | contribs)   22:41, 6 January 2012

There is a problem with continue queries and dir, see the reopened bug.

Status & tagging log