Page MenuHomePhabricator

Implement recursive calls to the MediaWiki API until a certain number of categories are obtained
Closed, ResolvedPublic

Description

Essential additional task that needs to be completed before the next step (caching).

Current plan: Start with 100m ggsradius and if <10 categories found, recurse with ggsradius of 10x the last one. There appears to be a limit of <=10,000m for the ggsradius (after which the API throws an error[0]) so stop at that point regardless.

[0]:

"error": {
    "code": "ggsintegeroutofrange",
    "info": "ggsradius may not be over 10000 (set to 10001) for users",

2/1/16: First draft of implementation done at https://github.com/nicolas-raoul/apps-android-commons/pull/36 , but there is an issue where categorySet hasn't actually been called yet when the categorySet.size()>=10 check to break out of the for loop is done, so it calls the API three times always. Probably needs to be handled differently.

3/1/16: I attempted to refactor the code according to Niedzielski's recommendations:

(1) ShareActivity.java should supply the Listener instead.
(2) For sending multiple API requests, RequestQueue should be refactored into a singleton pattern.
(3) The 'for' loop in my current code is running infinite loops because it isn't extracting the current request's radius. There is actually no need for a 'for' loop, we just need to check if there are enough categories and if the radius is within limits.
(4) If there are issues with static/non-static methods, refactor.

So I put the ResponseListener<T> class in ShareActivity.java, and passed it into the QueryRequest constructor. I wrote a VolleyRequestQueue class to implement the singleton pattern, according to this tutorial. I rewrote MwVolleyApi.java (now called MwVolley.java) to try and avoid use of nested static classes where possible, as I was previously encountering issues with the methods in static classes not being able to access non-static variables or methods.

As the QueryResponse, Query, Page, and Category classes are all non static, I received the 'no arg constructor' error and fixed that by registering InstanceCreators for them as mentioned in this tutorial. That fixed that particular issue.

However, now I am encountering this [1] error:

[1]

01-03 04:08:06.443: E/ACRA(11486): fr.free.nrw.commons fatal error : Attempt to invoke virtual method 'int fr.free.nrw.commons.upload.MwVolley.getRadius()' on a null object reference
01-03 04:08:06.443: E/ACRA(11486): java.lang.NullPointerException: Attempt to invoke virtual method 'int fr.free.nrw.commons.upload.MwVolley.getRadius()' on a null object reference
01-03 04:08:06.443: E/ACRA(11486): 	at fr.free.nrw.commons.upload.ShareActivity$ResponseListener.onResponse(ShareActivity.java:226)

However I am pretty sure I have instantiated apiCall (the MwVolley object) in the onCreate() method of ShareActivity, and have also tried instantiating it in other places while receiving the same error. If I comment out the getRadius() statement and replace it with an arbitrary radius, it runs for a bit, prints [2], and then crashes with the error message [3]

[2]:

01-03 04:57:35.865: D/Cat(14858): gpsCatExists=true
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858): query=pages=
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858):  CATEGORIES= Category:2010 in New Zealand, Category:Construction sites in Auckland, Category:Footbridges in Auckland, Category:Keep left road signs in New Zealand, Category:Night in Auckland, Category:Red and white stripe traffic cones, Category:Remote views of the Sky Tower, Category:Roadworks in New Zealand, Category:Traffic cones in New Zealand, Category:Traffic signals in New Zealand,
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858):  CATEGORIES= no categories exist
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858):  CATEGORIES= no categories exist
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858):  CATEGORIES= no categories exist
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858):  CATEGORIES= no categories exist
01-03 04:57:35.866: D/fr.free.nrw.commons.upload.ShareActivity$ResponseListener(14858): CATEGORIES FOUND[2010 in New Zealand, Roadworks in New Zealand, Night in Auckland, Footbridges in Auckland, Remote views of the Sky Tower, Traffic signals in New Zealand, Keep left road signs in New Zealand, Red and white stripe traffic cones, Traffic cones in New Zealand, Construction sites in Auckland]

[3]:

01-03 04:57:36.228: E/ACRA(14858): fr.free.nrw.commons fatal error : Attempt to read from field 'java.util.Set fr.free.nrw.commons.upload.MwVolley.categorySet' on a null object reference
01-03 04:57:36.228: E/ACRA(14858): java.lang.NullPointerException: Attempt to read from field 'java.util.Set fr.free.nrw.commons.upload.MwVolley.categorySet' on a null object reference
01-03 04:57:36.228: E/ACRA(14858): 	at fr.free.nrw.commons.upload.ShareActivity$ResponseListener.onResponse(ShareActivity.java:230)

This is a pretty big refactor so perhaps even though the error is listed in ShareActivity, it may actually stem from problems with the way I wrote my MwVolley or VolleyRequestQueue classes instead, although I really don't know what is wrong. @Nicolas_Raoul or @Niedzielski , would appreciate it if you took a look at my current code. Thanks!

Event Timeline

josephine_l claimed this task.
josephine_l raised the priority of this task from to High.
josephine_l updated the task description. (Show Details)
josephine_l moved this task from Backlog to Doing on the Commons-App-Android-Upload board.

@Nicolas_Raoul , @Niedzielski - I'm having problems with this task, that are described above. Sorry for pinging twice, I'm not sure if pings in the task description show up in email notifications the same way as pings in comments do?

Hi @josephine_l, yup, task description gives you a ping as well. I believe the delay in response is due to the holiday season and upcoming MediaWiki Dev Summit. I'm sure they'll reply soon. Try and work on other tasks which don't need mentor input meanwhile, if possible. :)

Thanks @NiharikaKohli :) No worries, I just didn't know if the description was the right place to ping since I've only ever gotten email notifications for comment pings myself. I'll try and work on other tasks while looking for a solution to this, but I'm sure my mentors will respond soon too.

I have seen the Stack Overflow question but I don't see what's going wrong... feel free to start working on caching and come back to this task when more feedback comes.

@Nicolas_Raoul - Okay, will do. Still no answers on StackOverflow today, hahah. :(

@josephine_l, @NiharikaKohli, I do not get a ping of any kind for @ tags in a description change AFAIK. I do get an email that the description for a task has changed because I'm a subscriber but there's no diff supplied or any special behavior associated with the ping.

@josephine_l, I've left a comment on your PR that I think will be quite helpful in resolving this task. In short, I think the recursive requests may be flattened to a single call which I hope should make the operation much faster as well as greatly simplifying the code.

@Niedzielski , thanks! That is a huge help indeed. :)

josephine_l lowered the priority of this task from High to Medium.Jan 8 2016, 5:15 AM

@Nicolas_Raoul @Niedzielski - Moving forward, shall I just do (1) and (2), and ditch the recursive calls entirely? I have already set the ggsradius to maximum allowed by MW API (10,000m) and there seem to be no problems with that so far.

(1) ShareActivity.java should supply the Listener instead.
(2) For sending multiple API requests, RequestQueue should be refactored into a singleton pattern.

Stephen will know better than me for the Java part, but one thing needed about this new way to use the API is to evaluate it... sorry it takes time but would you mind adding the necessary paragraphs at https://github.com/nicolas-raoul/apps-android-commons/wiki/Location-based-category-search ? To check whether there are tricky cases where this new idea actually does not work, in such case better realize it now than after implementation.
Thanks a lot! :-)

Hmmm... but the new method has already been implemented, it just involved changing the ggsradius to 10000. :) But I can still add the results to the GitHub wiki if needed.

Added 10k radius test results to the GitHub wiki.

josephine_l moved this task from Doing to Done on the Commons-App-Android-Upload board.