Page MenuHomePhabricator

Consider deprecating section editing API in RESTBase
Closed, ResolvedPublic

Description

Currently RESTBase exposes endpoints get the sections and save the edited sections of the article. This API was built for VE section edits to save bandwidth by not transmitting the whole article to the client. However, the API is currently not used and it's not clear whether VisualEditor team has plans to use it in the future. If not, we should deprecate and remove the API which will also help with Parsoid PHP porting project since section offsets calculated in Parsoid will be incompatible between PHP and JS.

Question for the VisualEditor team - are you going to use the section editing API?

Event Timeline

Pchelolo created this task.Feb 20 2019, 5:44 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 20 2019, 5:44 PM
cscott added a subscriber: cscott.Feb 26 2019, 4:12 PM

I'd like to see the concept of section editing be replaced by "DOM subtree editing", since sections are just one DOM tag we could subtree.

Then there's the broader question of how character indexing should be done: JS (UCS-2), unicode codepoint, vs PHP (UTF-8 byte offsets). We were using JS offsets originally without thinking too hard about it, but we should probably think a bit.

matmarex renamed this task from Consider deprecating session editing API in RESTBase to Consider deprecating section editing API in RESTBase.Mar 19 2019, 2:17 PM

Hehe, indeed, it seems so, but @Esanders wasn't clear on that ticket. @Esanders would VisualEditor be ok without section editing support from RESTBase / Parsoid?

ssastry added a comment.EditedApr 5 2019, 1:51 PM

I read both tickets closely now and here are my thoughts.

I think there will be some form of subtree ( not just section ) editing support in Parsoid (and a corresponding endpoint) in RESTBase which Ed affirms in T211461: Section edit endpoints in RESTBase: Still needed?. I anticipate two endpoints: one to get, and another to save.

The question more is about what those endpoints would look like. Will the GET endpoint rely on HTML offsets that Parsoid provides and RESTBase doing a substring operation to fetch the subtree? Or will RB go back to Parsoid to fetch the relevent subtree AND postprocess it to ensure there is adequate references support. If the latter, then the HTML offsets are not relevant, which is what this ticket is asking about.

So, the only confirmation we need from Ed is that fast substring operations to fetch a subtree from RESTBase storage is not going to be useful since it would still need postprocessing for references (and any other global state because of other such extensions).

No blockers from Readers Engineering to deprecating/removing this.

Seems like we have a consensus on the backend. @Esanders could you please confirm that VE will not need this in future?

We had a nearly-working implementation, however it was blocked by references. With references support being moved into Parsoid we would be able to bring this back. This feature would make saving a lot quicker on long articles. It is unlikely to be a priority in the next few quarters.

Basically this is a nice-to-have but we are unlikely to come back to it any time soon. Ideally this will be possible in the future, but doesn't need to be kept available it the current form.

Thank you for the info, @Esanders . I propose we deprecate and remove it in that case. @Pchelolo @ssastry thoughts?

bearND updated the task description. (Show Details)Apr 25 2019, 5:35 PM
bearND added a subscriber: bearND.

+1 to "DOM subtree editing" from me as well.

Mentioned in SAL (#wikimedia-operations) [2019-04-25T19:25:35Z] <mobrovac@deploy1001> Started deploy [restbase/deploy@7187e0c]: Bump HTML content version in docs, remove Parsoid stash fall-back and start logging all sections requests - T221432 T215956 T216636

Mentioned in SAL (#wikimedia-operations) [2019-04-25T19:45:39Z] <mobrovac@deploy1001> Finished deploy [restbase/deploy@7187e0c]: Bump HTML content version in docs, remove Parsoid stash fall-back and start logging all sections requests - T221432 T215956 T216636 (duration: 20m 04s)

After adding the warn logging, there were 15 requests recorded for getSections all in a span of a few minutes, from a browser, so I believe that was someone playing with it from the docs UI. I think we can safely remove the endpoints and the code.

Indeed, the calls were for /en.wikipedia.org/v1/page/html/List_of_Nintendo_Switch_games_(M-Z)?sections=header&stash=false (and footer respectively), so they are not legitimate (as in, there are no such sections in the Parsoid HTML). I propose to leave the logging on for the rest of the week. If we see the same picture Friday we can safely remove it.

Mentioned in SAL (#wikimedia-operations) [2019-05-07T07:26:38Z] <mobrovac@deploy1001> Started deploy [restbase/deploy@d91ee4c]: Remove section functionality from the REST API - T216636

Mentioned in SAL (#wikimedia-operations) [2019-05-07T07:51:24Z] <mobrovac@deploy1001> Finished deploy [restbase/deploy@d91ee4c]: Remove section functionality from the REST API - T216636 (duration: 24m 46s)

mobrovac closed this task as Resolved.May 7 2019, 7:57 AM
mobrovac triaged this task as Medium priority.

Deployed, section functionality is no more (for now). Resolving.

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMay 7 2019, 7:57 AM