Single Item Search Results
Issue #23
resolved
On MAL, if you run a search and the results only match a single item, it does a 302 redirect to the details page for that item.
Currently, we don't detect this, and the parser breaks. As an example, search for "Evangelion Alone" on MAL. The site redirects to the explicit anime page for "Evangelion: 1.0 You Are (Not) Alone". If you try the same on the API, it breaks.
Comments (8)
-
-
reporter
I'm not sure how Guzzle will handle a 302. The original URL returns that, but if Guzzle fetches the provided URL, that would return a 200. There's a getRedirectCount method and also a getEffectiveURL method (http://docs.guzzlephp.org/en/latest/http-client/http-redirects.html)
We have two possibilities, we can add a method to the communicator that will allow us to determine if a redirect occurred during the operation, or we check the returned content. I'm honestly not sure which one is cleaner, but I'm leaning towards making a new method in the communicator service. It's cleaner conceptually and we don't have to change anything to detect the redirect if the source code of the page changes for some reason. Not that such a change wouldn't break other things, but it's just one less problem.
The communicator exists so we don't have to re-do boilerplate code on every call and have to do all the setup including user agent each time. It's part of making the code modular. In this case, it's abstracted away something that we could do directly, but it also saves a whole lot of work if we ever have to change the implementation of the communication service. Heck, we could rip out Guzzle and change to a different package and would only have to make the change in one location.
-
A new method is the best solution but how should the method work? What is the best way to handle this?
We could create a get/set method that records the redirects or status code. The set function could be added inside the fetch method. The get function could be called at the communicator.
-
reporter
Easiest way is to move $response to a private variable inside the communicator class and adjust the existing functions to save the response to that. This will let us create a method. that can grab the properties of the response object.
For the method, we really don't need to know the number of times the redirect happened, so we can just create wasRedirected() and have it check $this->response->getRedirectCount(). If it's 1 or more, return true, otherwise false.
Once that's done, we use this inside the search methods and we will then know which parser to call. If it's true, we call the anime parser and wrap the contents inside the expected output, otherwise, we use the existing code path.
I can probably code this later tonight, but you're welcome to handle it.
-
Thanks for asking it, I am currently really tired. (Just finished 3 reports and I have still homework to do.)
-
reporter
Convert Response to Class Member
Make the response object part of the class member. This will let us add some extra methods that callers can use to get more details on the response, if needed.
ref
#23→ <<cset f4810e340896>>
-
reporter
Add wasRedirected method for Communicator
Add a simple method that will return a boolean indicating if the request was redirected.
ref
#23→ <<cset 87e77601cf94>>
-
reporter
- changed status to resolved
Show Results for Single Item Search Returns
Configure SearchController methods to check for a redirect. If a redirect occurred, it means only one item matched and the content will be that of the title’s detail page. In that case, use the appropriate type parser and return the result. If no redirect occurred, parse as normal.
Fixes
#23→ <<cset 5b6cb1dcfeef>>
- Log in to comment
If I am right Guzzle contains a code to get the status code.
The only thing that is bugging me is that we are using the communicator. In this case (my opinion) we should check if the response contains the string "Search Results". If it doesn't we can assume that we are redirected.
I don't know any other methods.