Page MenuHomePhabricator

ApiTemplateData implements ApiBase::getCustomPrinter for no good reason
Closed, ResolvedPublic1 Estimated Story Points

Description

Just because the internal data structures are JSON isn't a justification for the schema data itself to not be retrievable using other supported API formats.

If you wait for Gerrit change 182858 to be merged, you won't even have to worry about getting indexed tag names right for format=xml.

In fact, you simply need to remove the implemented getCustomPrinter method and test the change.
Test by finding a TemplateData with reasonably complex structure, making the change, and then fetching it using format=xml and format=php to see if the output looks sane.

The TemplateData extension information can be found on mediawiki.org. TemplateData itself implements only one Api action, so a part of this task is, that you find the code you need to change.

Event Timeline

Anomie raised the priority of this task from to Low.
Anomie updated the task description. (Show Details)
Anomie moved this task to Non-core-API stuff on the MediaWiki-Action-API board.
Anomie subscribed.

So it just needs removing in the extension?

Probably, yes. Also remove the 'format' parameter from getAllowedParams().

Test by finding a TemplateData with reasonably complex structure, making the change, and then fetching it using format=xml and format=php to see if the output looks sane.

Change 263881 had a related patch set uploaded (by Alex Monk):
Don't restrict format parameter in the API

https://gerrit.wikimedia.org/r/263881

Krenair moved this task from Backlog to Doing on the TemplateData board.

I'm not sure I understand the value of this. Providing XML encapsulation of a JSON blob doesn't make it any more usable by something that doesn't speak JSON, surely?

Actually you end up with XML, like this:

<params>
<_1 type="unknown">
<aliases/>
</_1>
<size type="wiki-user-name">
<label en-gb="test"/>
<description en-gb="testing"/>
<example en-gb="e.g."/>
<aliases/>
</size>
<name type="unknown">
<aliases/>
</name>
<altlink type="unknown">
<aliases/>
</altlink>
<variant type="unknown">
<aliases/>
</variant>
<_v _idx="1" type="unknown">
<aliases/>
</_v>
</params>
MtDu added subscribers: Krenair, MtDu.

I'm working on this. I have claimed it on the GCI page.
Thanks,
MtDu

I already uploaded a patch, @MtDu. I claimed it here, and cannot see the URL posted above by @Florian

Actually you end up with XML, like this:

<params>
<_1 type="unknown">
<aliases/>
</_1>
<size type="wiki-user-name">
<label en-gb="test"/>
<description en-gb="testing"/>
<example en-gb="e.g."/>
<aliases/>
</size>
<name type="unknown">
<aliases/>
</name>
<altlink type="unknown">
<aliases/>
</altlink>
<variant type="unknown">
<aliases/>
</variant>
<_v _idx="1" type="unknown">
<aliases/>
</_v>
</params>

Oh dear. That's pretty useless, surely?

I put in pretty useless TemplateData, to be fair.

Oh dear. That's pretty useless, surely?

I added some suggestions on the patch to make that come out a little nicer for the XML format. But when it comes down to it,

<?xml version="1.0"?>
<api>
  <pages>
    <page id="2385">
      <title xml:space="preserve">Template:Anchor</title>
      <description en="The template {{anchor}} inserts one or more HTML anchors in a page. Those locations can then be linked to using [[#location|…]] syntax.  The parameters here are for convenience; no parameter name is required in the template itself." />
      <params>
        <param key="1">
          <label en="First anchor" />
          <description en="First anchor; Only the first anchor is required." />
          <type xml:space="preserve">string</type>
          <required xml:space="preserve">true</required>
          <suggested xml:space="preserve">false</suggested>
          <example />
          <deprecated xml:space="preserve">false</deprecated>
          <aliases />
          <autovalue />
          <default />
        </param>
        <param key="2">
          <label en="Second anchor" />
          <description en="Second anchor." />
          <type xml:space="preserve">string</type>
          <required xml:space="preserve">false</required>
          <suggested xml:space="preserve">false</suggested>
          <example />
          <deprecated xml:space="preserve">false</deprecated>
          <aliases />
          <autovalue />
          <default />
        </param>
        <param key="3">
          <label en="Third anchor" />
          <description en="Third anchor.  For additional anchors, just type in 4 as the parameter name for the next, 5 for the next after that, and so on." />
          <type xml:space="preserve">string</type>
          <required xml:space="preserve">false</required>
          <suggested xml:space="preserve">false</suggested>
          <example />
          <deprecated xml:space="preserve">false</deprecated>
          <aliases />
          <autovalue />
          <default />
        </param>
      </params>
      <format xml:space="preserve">inline</format>
      <paramOrder>
        <p>1</p>
        <p>2</p>
        <p>3</p>
      </paramOrder>
      <sets />
      <maps />
    </page>
  </pages>
</api>

or

a:1:{s:5:"pages";a:1:{i:2385;a:7:{s:5:"title";s:15:"Template:Anchor";s:11:"description";a:1:{s:2:"en";s:233:"The template {{anchor}} inserts one or more HTML anchors in a page. Those locations can then be linked to using [[#location|…]] syntax.  The parameters here are for convenience; no parameter name is required in the template itself.";}s:6:"params";a:3:{i:1;a:10:{s:5:"label";a:1:{s:2:"en";s:12:"First anchor";}s:11:"description";a:1:{s:2:"en";s:48:"First anchor; Only the first anchor is required.";}s:4:"type";s:6:"string";s:8:"required";b:1;s:9:"suggested";b:0;s:7:"example";N;s:10:"deprecated";b:0;s:7:"aliases";a:0:{}s:9:"autovalue";N;s:7:"default";N;}i:2;a:10:{s:5:"label";a:1:{s:2:"en";s:13:"Second anchor";}s:11:"description";a:1:{s:2:"en";s:14:"Second anchor.";}s:4:"type";s:6:"string";s:8:"required";b:0;s:9:"suggested";b:0;s:7:"example";N;s:10:"deprecated";b:0;s:7:"aliases";a:0:{}s:9:"autovalue";N;s:7:"default";N;}i:3;a:10:{s:5:"label";a:1:{s:2:"en";s:12:"Third anchor";}s:11:"description";a:1:{s:2:"en";s:127:"Third anchor.  For additional anchors, just type in 4 as the parameter name for the next, 5 for the next after that, and so on.";}s:4:"type";s:6:"string";s:8:"required";b:0;s:9:"suggested";b:0;s:7:"example";N;s:10:"deprecated";b:0;s:7:"aliases";a:0:{}s:9:"autovalue";N;s:7:"default";N;}}s:6:"format";s:6:"inline";s:10:"paramOrder";a:3:{i:0;s:1:"1";i:1;s:1:"2";i:2;s:1:"3";}s:4:"sets";a:0:{}s:4:"maps";a:0:{}}}}

isn't inherently more or less useful than

{
    "pages": {
        "2385": {
            "title": "Template:Anchor",
            "description": {
                "en": "The template {{anchor}} inserts one or more HTML anchors in a page. Those locations can then be linked to using [[#location|…]] syntax.  The parameters here are for convenience; no parameter name is required in the template itself."
            },
            "params": {
                "1": {
                    "label": {
                        "en": "First anchor"
                    },
                    "description": {
                        "en": "First anchor; Only the first anchor is required."
                    },
                    "type": "string",
                    "required": true,
                    "suggested": false,
                    "example": null,
                    "deprecated": false,
                    "aliases": [],
                    "autovalue": null,
                    "default": null
                },
                "2": {
                    "label": {
                        "en": "Second anchor"
                    },
                    "description": {
                        "en": "Second anchor."
                    },
                    "type": "string",
                    "required": false,
                    "suggested": false,
                    "example": null,
                    "deprecated": false,
                    "aliases": [],
                    "autovalue": null,
                    "default": null
                },
                "3": {
                    "label": {
                        "en": "Third anchor"
                    },
                    "description": {
                        "en": "Third anchor.  For additional anchors, just type in 4 as the parameter name for the next, 5 for the next after that, and so on."
                    },
                    "type": "string",
                    "required": false,
                    "suggested": false,
                    "example": null,
                    "deprecated": false,
                    "aliases": [],
                    "autovalue": null,
                    "default": null
                }
            },
            "format": "inline",
            "paramOrder": [
                "1",
                "2",
                "3"
            ],
            "sets": [],
            "maps": {}
        }
    }
}

It all depends on which serialization format is easiest for the client to parse.

Change 264570 had a related patch set uploaded (by IoannisKydonis):
Remove getCustomPrinter() from ApiTemplateData

https://gerrit.wikimedia.org/r/264570

Change 264570 abandoned by TTO:
Remove getCustomPrinter() from ApiTemplateData

https://gerrit.wikimedia.org/r/264570

We should remove the Google Code In tag. This can be confusing, as Krenair has already pushed a patch for this, so there is no need for this to remain a GCI task. This confused me, so I'm sure if left as is, will confuse another student.
Thanks,
MtDu

Change 263881 merged by jenkins-bot:
Don't restrict format parameter in the API

https://gerrit.wikimedia.org/r/263881