Problem/Motivation

In Drupal\feeds\Feeds\Fetcher\HttpFetcher::get() a http request gets made using Guzzle. The request gets made with the following line:

$response = $this->client->getAsync($url, $options)->wait();

Sometimes you want to set extra options for the request to be made, there are a lot of these available: https://docs.guzzlephp.org/en/stable/request-options.html

Right now a class must extend HttpFetcher and override the whole get() method just to specify an extra option. So this causes a lot of code duplication.

Proposed resolution

There is more than one way to allow extra options to be set. The easiest solution is to specify an additional parameter for the get() method called $options. This way, subclasses could still override the get() method if they want, but then they only have to specify the extra options they want and handle the rest of via the parent:

use Drupal\feeds\Feeds\Fetcher\HttpFetcher;

class MyHttpFetcher extends HttpFetcher {

  /**
   * {@inheritdoc}
   */
  protected function get($url, $sink, $cache_key = FALSE, array $options = []) {
    $options += [
      'auth' => ['**user**','**pass**','digest'],
    ];
    return parent::get($url, $sink, $cache_key, $options);
  }

}

Or the subclass can add the extra option by calling the get() method themselves somewhere. For example, by overriding the whole fetch() method:

use Drupal\feeds\Exception\EmptyFeedException;
use Drupal\feeds\FeedInterface;
use Drupal\feeds\Feeds\Fetcher\HttpFetcher;
use Drupal\feeds\Result\HttpFetcherResult;
use Drupal\feeds\StateInterface;
use Symfony\Component\HttpFoundation\Response;

class MyHttpFetcher extends HttpFetcher {

  /**
   * {@inheritdoc}
   */
  public function fetch(FeedInterface $feed, StateInterface $state) {
    $sink = $this->feedsFileSystem->tempnam($feed, 'http_fetcher_');

    // Get cache key if caching is enabled.
    $cache_key = $this->useCache() ? $this->getCacheKey($feed) : FALSE;

    $response = $this->get($feed->getSource(), $sink, $cache_key, [
      'auth' => ['**user**','**pass**','digest'],
    ]);

    // 304, nothing to see here.
    if ($response->getStatusCode() == Response::HTTP_NOT_MODIFIED) {
      $state->setMessage($this->t('The feed has not been updated.'));
      throw new EmptyFeedException();
    }

    return new HttpFetcherResult($sink, $response->getHeaders(), $this->fileSystem);
  }

}

This looks less ideal though, as you are then still copying the contents of a whole method.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork feeds-3401002

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

MegaChriz created an issue. See original summary.

megachriz’s picture

Status: Active » Needs review

  • MegaChriz committed 1b534372 on 8.x-3.x
    Issue #3401002 by MegaChriz, agn507: Added easier way to override...

MegaChriz credited agn507.

megachriz’s picture

Status: Needs review » Fixed

Merged the code!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.