Last modified: 2014-09-24 00:53:31 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T47652, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 45652 - API help should always give parameters' type and default
API help should always give parameters' type and default
Status: PATCH_TO_REVIEW
Product: MediaWiki
Classification: Unclassified
API (Other open bugs)
1.21.x
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: easy
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-03 01:29 UTC by spage
Modified: 2014-09-24 00:53 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description spage 2013-03-03 01:29:20 UTC
api.php auto-generates excellent API documentation. However, it omits the type of most parameters and the default for some.  It shows the type if it has a limited namespace, range, or set of values, but it doesn't simply tell you "Type: string" or "Type: integer".  Similarly, it often omits the default for a parameter.

For example, look at http://en.wikipedia.org/w/api.php for action=edit.  This has a redirect parameter, but there's no indication of its type or what the default behavior is.  The code specifies this information:
            'redirect' => array(
                ApiBase::PARAM_TYPE => 'boolean',
                ApiBase::PARAM_DFLT => false,
            ),
but makeHelpMsgParameters() in ApiBase.php doesn't always output $type and $default.
Comment 1 Brad Jorsch 2013-03-11 14:47:08 UTC
Looking at the code, it seems to me that the default will always be printed as long as it's not null or false. But many parameters use null as the default, either because there is no sane default or because the default is to skip the relevant bit entirely, which often results in the behavior of either "all" or "none"; for example, for many namespace parameters the null default is equivalent to something like 0|1|2|3|4|5|6|7|8|9|10|11|12|13|14|15|100|101|108|109|710|711|446|447|828|829 (depending on the specific namespaces available on each wiki), but actually specifying that as the default would change the "where" portion of the SQL query from something like "WHERE pl_from IN (...)" to something like "WHERE pl_from IN (...) AND pl_namespace IN (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 100, 101, 108, 109, 710, 711, 446, 447, 828, 829)".

Note that for boolean parameters the default is always false, and trying to set anything other than null or false as the default results in an error for all queries. From the client, specifying the parameter at all, even as "&redirect=no", "&redirect=false", "&redirect=0", "&redirect=", or (in GET requests) "&redirect", is interpreted as true in line with the behavior of checkboxes in HTML forms.


The indication of type should be relatively easy for someone to pick up; it should probably go somewhere in the makeHelpMsgParameters() method in includes/api/ApiBase.php.

Cleanup of any modules that use null for the default and take behavior equivalent to something that could be specified as a default should also be relatively easy, if tedious.
Comment 2 Kishan Thobhani (kishanio) 2014-05-10 10:09:00 UTC
Wording,

1.) Fix makeHelpMsgParameters() to output default & type in all cases.
2.) Go through all the modules in the API and make sure there is a relevant default and shouldn't be left null. 

Is this whats expected? Also can you please assign me to this, i would like to work on it.
Comment 3 Brad Jorsch 2014-05-10 11:08:37 UTC
(In reply to kishanio from comment #2)
> 2.) Go through all the modules in the API and make sure there is a relevant
> default and shouldn't be left null. 

While also making sure that this change doesn't result in stupid SQL queries like "WHERE pl_from IN (...) AND pl_namespace IN (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 100, 101, 108, 109, 710, 711, 446, 447, 828, 829)".

Reading over this again, I'd lean towards just skipping item #2 entirely rather than inserting checks all over the place for "was this parameter explicitly specified?".
Comment 4 Kishan Thobhani (kishanio) 2014-05-10 12:20:02 UTC
> Reading over this again, I'd lean towards just skipping item #2 entirely
> rather than inserting checks all over the place for "was this parameter
> explicitly specified?".

Alright i won't touch it. So should i run a check in makeHelpMsgParameters that in case if default is not specified it will be mentioned as <NONE> / <NOT SPECIFIED>. Do you feel a place holder would make more sense?
Comment 5 Gerrit Notification Bot 2014-05-10 14:21:05 UTC
Change 132699 had a related patch set uploaded by Kishanio:
API help should always give parameters' type and default

https://gerrit.wikimedia.org/r/132699
Comment 6 Brad Jorsch 2014-05-12 15:54:06 UTC
Looking at the result of kishanio's patch, I think this needs another iteration on the design.

I find the output needlessly cluttered by "Type: string" all over the place. This is generally useless since *everything* is a string. Sometimes the string has further restrictions like "valid title" or "user name" or "integer" which is useful to note, but just "Type: string" is more useless than asking someone what they want for lunch and they reply "Food".

I also find it useless to see "Type: boolean" and "Default: false", since there is no other default possible for a boolean type.
Comment 7 Kishan Thobhani (kishanio) 2014-05-12 19:51:48 UTC
(In reply to Brad Jorsch from comment #6)

> I find the output needlessly cluttered by "Type: string" all over the place.
> This is generally useless since *everything* is a string. Sometimes the
> string has further restrictions like "valid title" or "user name" or
> "integer" which is useful to note, but just "Type: string" is more useless
> than asking someone what they want for lunch and they reply "Food".

Seems wise. Too much redundancy. What do you suggest for exact change to-be or is there anyone who is familiar with all the modules and can guide/help me compile the set of information to be exposed here? To say a-bit more insight.

> I also find it useless to see "Type: boolean" and "Default: false", since
> there is no other default possible for a boolean type.

Thats what i thought but since i'm new here i still need to get familiar with protocol of what needs to be fixed/raising questions over discussion etc.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links