jikan-me/jikan

[Enhancement] Unnecessary double request for getting anime episodes

Closed this issue · 1 comments

ToshY commented

Problem

There seems to be an unnecessary additional request going to the endpoint /anime/X/_/episode.
I've noticed this when looking at my logging:

[2022-10-27T21:42:56.597680+02:00] http_client.INFO: Request: "GET https://myanimelist.net/anime/19/"
[2022-10-27T21:42:57.234499+02:00] http_client.INFO: Response: "200 https://myanimelist.net/anime/19/"
[2022-10-27T21:42:57.288015+02:00] http_client.INFO: Request: "GET https://myanimelist.net/anime/19/_/characters"
[2022-10-27T21:42:58.002126+02:00] http_client.INFO: Response: "200 https://myanimelist.net/anime/19/_/characters"
[2022-10-27T21:42:58.875553+02:00] http_client.INFO: Request: "GET https://myanimelist.net/anime/19/_/episode?offset=0"
[2022-10-27T21:42:59.823240+02:00] http_client.INFO: Response: "200 https://myanimelist.net/anime/19/_/episode?offset=0"
[2022-10-27T21:43:00.226814+02:00] http_client.INFO: Request: "GET https://myanimelist.net/anime/19/_/episode?offset=0"
[2022-10-27T21:43:00.849468+02:00] http_client.INFO: Response: "200 https://myanimelist.net/anime/19/_/episode?offset=0"
[2022-10-27T21:43:00.872698+02:00] http_client.INFO: Request: "GET https://myanimelist.net/anime/19/_/video"
[2022-10-27T21:43:01.434744+02:00] http_client.INFO: Response: "200 https://myanimelist.net/anime/19/_/video"
[2022-10-27T21:43:01.701029+02:00] http_client.INFO: Request: "GET https://myanimelist.net/anime/19/jikan/pics"
[2022-10-27T21:43:02.006953+02:00] http_client.INFO: Response: "200 https://myanimelist.net/anime/19/jikan/pics"

It does 2 requests to the /episode?offset=0.

Reproduction

$this->malClient->getAnimeEpisodes(
    new AnimeEpisodesRequest(19)
);

Expectation

Single request made for episode endpoint.

Possible solution

Currently the code for getAnimeEpisodes looks like this:

public function getAnimeEpisodes(Request\Anime\AnimeEpisodesRequest $request): Model\Anime\Episodes
{
    // Episode page returns 404 when there are no results
    try {
        $crawler = $this->ghoutte->request('GET', $request->getPath());
    } catch (\Exception $e) {
        if ($e->getCode() === 404) {
            return new Model\Anime\Episodes();
        }

        throw $e;
    }

    $crawler = $this->ghoutte->request('GET', $request->getPath());
    try {
        $parser = new Parser\Anime\EpisodesParser($crawler);

        return $parser->getModel();
    } catch (\Exception $e) {
        throw ParserException::fromRequest($request, $e);
    }
}

The second crawler request is redundant. I can create a PR to remove it if you don't have any objections.

Great catch! Please go ahead