Page MenuHomePhabricator

Write tests for ApiHooksHandler
Closed, ResolvedPublic

Description

We acquired a mountain of code which hooks into several query APIs, and does some scary workarounds to make a batch query to the ORES cache table. This needs unit tests.

Event Timeline

Halfak triaged this task as Medium priority.Feb 11 2019, 9:29 PM
Halfak moved this task from Unsorted to Maintenance/cleanup on the Machine-Learning-Team board.
Halfak added subscribers: Ladsgroup, Halfak.

@Ladsgroup, can we add this to your backlog?

So I looked at this. We have integration tests for the API (writing unit tests for mediawiki's API modules is not easy, borderline impossible) plus all of the bad code has been deleted now. I will expand it to include watchlist too but not much can be done. OTOH, ChangesListHooksHandler can use lots of love for goodfaith bits.

Ladsgroup raised the priority of this task from Medium to High.Mar 29 2019, 3:11 PM

While writing the tests I realized it's just erroring out, then tried the watchlist module in localhost and it fataled, I was so shocked that I tried it on production and guess what? It also fatals:

{
    "error": {
        "code": "internal_api_error_DBQueryError",
        "info": "[XJ4XlwpAAK4AAKfkcskAAABN] Caught exception of type Wikimedia\\Rdbms\\DBQueryError",
        "errorclass": "Wikimedia\\Rdbms\\DBQueryError"
    },
    "servedby": "mw1312"
}

I found the fix and put it there.

Change 500062 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/ORES@master] Fix watchlist API

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

The error from logstash:

A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? 
Query: SELECT  rc_id,rc_namespace,rc_title,rc_timestamp,rc_type,rc_deleted,wl_notificationtimestamp,rc_cur_id,rc_this_oldid,rc_last_oldid,rc_type,rc_minor,rc_bot  FROM `recentchanges`,`ores_model` JOIN `watchlist` ON ((wl_namespace=rc_namespace) AND (wl_title=rc_title)) LEFT JOIN `page` ON ((rc_cur_id=page_id)) INNER JOIN `ores_classification` ON ((rc_this_oldid=oresc_rev) AND oresc_model = '12' AND oresc_class = '1')   WHERE wl_user = '9395' AND ((rc_this_oldid=page_latest) OR (rc_type=3)) AND rc_type IN ('0','1','3','6')  AND (rc_timestamp > '') AND (oresc_probability > '0.03')  ORDER BY rc_timestamp DESC,rc_id DESC LIMIT 11  
Function: WatchedItemQueryService::getWatchedItemsWithRecentChangeInfo
Error: 1054 Unknown column 'rc_namespace' in 'on clause' (10.64.16.83:3317)

Note that the query looks like SELECT ... FROM recentchanges, ores_model JOIN watchlist ON (wl_namespace=rc_namespace AND wl_title=rc_title) ..., so it's trying to join against the wrong table.

Because the ores_model table is not used directly anymore it causes this query and the subsequent error:

SELECT rc_id,rc_namespace,rc_title,rc_timestamp,rc_type,rc_deleted,wl_notificationtimestamp,rc_cur_id,rc_this_oldid,rc_last_oldid,rc_type,rc_minor,rc_bot FROM `recentchanges`,`ores_model` JOIN `watchlist` ON ((wl_namespace=rc_namespace) AND (wl_title=rc_title)) LEFT JOIN `page` ON ((rc_cur_id=page_id)) INNER JOIN `ores_classification` ON ((rc_this_oldid=oresc_rev) AND oresc_model = '49' AND oresc_class = '1') WHERE wl_user = '1' AND ((rc_this_oldid=page_latest) OR (rc_type=3)) AND rc_type IN ('0','1','3','6') AND (rc_timestamp > '') AND ((rc_type != 3) OR ((rc_deleted & 9) != 9)) AND (oresc_probability > '0.5') ORDER BY rc_timestamp DESC,rc_id DESC LIMIT 11

Change 500062 merged by jenkins-bot:
[mediawiki/extensions/ORES@master] Fix watchlist API

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