Page MenuHomePhabricator

API allusers: Is it intended that all users are returned when an invalid value for augroup is specified?
Open, LowestPublic

Description

Consider this query for augroup=Image-reviewer. The group Image-reviewer was recently renamed to image-reviewer. If I wanted avoid falsely stating all users would be Image-reviewers, I would either have to consider the warning emitted or query valid user groups in advance.

Given that augroup is a filter, I would rather expect 0 results if the group specified is invalid, not all users.

Consider augroup=Image-reviewer|sysop -- there only sysops are returned, in spite of the invalid Image-reviewer.

The current behavior is:

  • valid filter -> users filtered
  • invalid filter value -> users not filtered
  • partially invalid filter value -> users filtered by valid part

Was this behavior chosen intentionally?

Event Timeline

Rillke created this task.Jul 28 2019, 11:04 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 28 2019, 11:04 AM
Rillke renamed this task from Is it intended that all users are returned when an invalid value for augroup is specified? to API allusers: Is it intended that all users are returned when an invalid value for augroup is specified?.Jul 28 2019, 11:05 AM
Restricted Application added a project: Core Platform Team. · View Herald TranscriptJul 28 2019, 11:27 AM

@Rillke is this intended to be a discussion, or a request to change the logic to return an error?

CCicalese_WMF triaged this task as Lowest priority.Jul 30 2019, 5:38 PM
This comment was removed by Rillke.
Rillke added a comment.EditedJul 30 2019, 6:45 PM

@DannyS712 I consider it unexpected behaviour. However phrasing it as a question I was hoping to get more productive comments compared to a "request" because nobody has to be defensive. Now please let's, according to mw:Bug_management/Phabricator_etiquette, focus on the issue?

Anomie moved this task from Unsorted to Needs Code on the MediaWiki-API board.Aug 13 2019, 5:11 PM
Anomie added a subscriber: Anomie.

Consider augroup=Image-reviewer|sysop -- there only sysops are returned, in spite of the invalid Image-reviewer.

This is expected behavior. augroup is "return users in any of these groups", not "return users in all of these groups". Compare, for example, augroup=bot|sysop.

Consider this query for augroup=Image-reviewer. The group Image-reviewer was recently renamed to image-reviewer. If I wanted avoid falsely stating all users would be Image-reviewers, I would either have to consider the warning emitted or query valid user groups in advance.

It's a side effect of how the parameter validation works: when you supply an invalid value for augroup, it produces a warning along the lines of "Unrecognized value for parameter "augroup": foobar." and ignores the invalid value. In this case that's resulting in an empty list being passed for augroups.

For historical reasons,[1] multi-valued filtering parameters like this tend to treat supplying an empty list as being the same as not filtering at all. I wouldn't oppose the addition of some sort of check to treat an augroup with all invalid groups as meaning to return no results (or as an error), as was done for example in T162816: ApiQueryLinks ignores pltitles/tltemplates when empty. Changing the meaning of an actual empty list or having augroup treat any invalid value as an error (rather than a warning) should probably go through a deprecation process.

[1]: Mainly "it was accidentally coded this way a decade ago, now clients might be depending on the behavior".

as meaning to return no results (or as an error)
go through a deprecation process

That'd be great :) I can't think of a single scenario where clients supply (solely) invalid group(s) and expect to get a list of all users, or at least, depend on getting that list.