Page MenuHomePhabricator

API action=wbgetentities does not handle formatversion=2
Open, Needs TriagePublic

Description

Both of this requests return identical result, ignoring JSON v2:

{
    "entities": {
        "-1": {
            "site": "enwiki",
            "title": "Bug12345",
            "missing": ""
        }
    },
    "success": 1
}

expected for v2:

{
    "entities": [
        {
            "site": "enwiki",
            "title": "Bug12345",
            "missing": true
        }
    ],
    "success": true
}

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

Yurik created this task.Dec 16 2018, 4:36 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 16 2018, 4:36 AM
Yurik updated the task description. (Show Details)Dec 16 2018, 4:37 AM

Relevant code for this specific example: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Wikibase/+/6eea7f8d5e52a528877748391e3fae965f752018/repo/includes/Api/ResultBuilder.php#1019. Changing that "" to true should do it, although with how complicated Wikibase's code is I can't say for sure.

There may, of course, be other places in the code with the same or similar issues.

Mvolz added a subscriber: Mvolz.May 24 2019, 11:03 AM

Seconding, I just ran into this.

Relevant code for this specific example: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Wikibase/+/6eea7f8d5e52a528877748391e3fae965f752018/repo/includes/Api/ResultBuilder.php#1019. Changing that "" to true should do it, although with how complicated Wikibase's code is I can't say for sure.

That would only change "missing" from "" to true, I think, whereas the example in the task description also has entities change from an object to an array. This is analogous to the change of query’s titles in MediaWiki core (which the JSON v2 documentation describes as specific to that API module).

However, that would be a massive breaking change to wbgetentities, even more so than changing claims to statements, which was previously discussed in T149410 – I can’t imagine any use of wbgetentities that would be unaffected by this. It might be something to consider for a hypothetical formatversion=3 – though even then, I’m not convinced of its usefulness, since entity IDs are much less internal than query/titles’ page IDs, and once you have multiple entities in the response an array is arguably less useful than an object – but I don’t think we should make such a change for formatversion=2.

Restricted Application added a project: Core Platform Team. · View Herald TranscriptMon, Oct 21, 4:58 PM

Relevant code for this specific example: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/Wikibase/+/6eea7f8d5e52a528877748391e3fae965f752018/repo/includes/Api/ResultBuilder.php#1019. Changing that "" to true should do it, although with how complicated Wikibase's code is I can't say for sure.

That would only change "missing" from "" to true, I think, whereas the example in the task description also has entities change from an object to an array. This is analogous to the change of query’s titles in MediaWiki core (which the JSON v2 documentation describes as specific to that API module).
However, that would be a massive breaking change to wbgetentities, even more so than changing claims to statements, which was previously discussed in T149410 – I can’t imagine any use of wbgetentities that would be unaffected by this. It might be something to consider for a hypothetical formatversion=3 – though even then, I’m not convinced of its usefulness, since entity IDs are much less internal than query/titles’ page IDs, and once you have multiple entities in the response an array is arguably less useful than an object – but I don’t think we should make such a change for formatversion=2.

My understanding /interpretation behind formatversion 2 is that it isn't expected to be stable (https://www.mediawiki.org/wiki/API:JSON_version_2) and that by default everyone gets formatversion 1 anyway. So this would be more bringing up the api up to spec for version 2, not adding breaking changes, because most people shouldn't be requesting the format version or requesting 2 if they want the format specified by default/1. I don't think there are any plans on doing a version 3 in particular.

formatversion=2 has been available for four years. If it’s still not stable after all this time, then when does it ever become stable? And does that mean that I’ve actually been relying on an unstable API in most of my tools, without being aware of it?

If I read the documentation correctly, the intention was that formatversion=2 is unstable in MediaWiki 1.25, leaving one version where problems and shortcomings could be discovered and fixed, so that 1.26 could make breaking changes if necessary. I don’t know if any such changes were made in the end (none are listed in the documentation), but I certainly hope that since MediaWiki 1.26, formatversion=2 has been a stable interface that users can rely on.

@Lucas_Werkmeister_WMDE is correct. It was unstable in 1.25, maybe even 1.26, to give time for people to find and report places where we missed making the needed changes. It's not unstable anymore.

It "officially" became stable in 1.33 with rMW8ec833f7e0c8: JSON formatversion=2 is no longer experimental, but that's more of a "we forgot to update the docs for several releases" than it actually being unstable in 1.32 and several earlier versions.

It's unlikely we'll do a formatversion=3 any time soon. That would be appropriate for API-wide changes to the structure of JSON output, not just a change in structure of one individual module. The module itself is welcome to introduce its own "wbformatversion" parameter, or (better IMO) a parameter targeted to the specific format change it wants to make.

We can also make breaking changes to the Wikibase API without a new parameter, as long as we follow the process set out in the stable interface policy.

I propose to close this task as “invalid” (formatversion=2 does not mandate the output expected in the task description, in my opinion), and open more specific tasks instead:

  • Change missing from "" to true. This should be a relatively safe change to make, since I expect most consumers will only check for presence of the key. (We would never add missing=false, just like action=query never adds missing=false or invalid=false.)
  • Change entities from object to array.
  • Any other inconsistencies?