Skip to content

Split listener from ConfiguredClientsStrategy #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

The change log describes what is "Added", "Removed", "Changed" or "Fixed" between each release.

## 1.19.0 - unreleased

### Changed

- `ConfiguredClientsStrategy` no longer implements `EventSubscriberInterface`,
this has been moved to `ConfiguredClientsStrategyListener` to avoid initializing
the strategy on every request.

## 1.18.0 - 2020-03-30

### Added
Expand Down
33 changes: 1 addition & 32 deletions src/Discovery/ConfiguredClientsStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,14 @@
use Http\Client\HttpClient;
use Http\Discovery\HttpClientDiscovery;
use Http\Discovery\Strategy\DiscoveryStrategy;
use Symfony\Component\EventDispatcher\Event as LegacyEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Contracts\EventDispatcher\Event;

if (Kernel::MAJOR_VERSION >= 5) {
\class_alias(Event::class, 'Http\HttplugBundle\Discovery\ConfiguredClientsStrategyEventClass');
} else {
\class_alias(LegacyEvent::class, 'Http\HttplugBundle\Discovery\ConfiguredClientsStrategyEventClass');
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was unused, correct?

Copy link
Contributor Author

@wouterj wouterj Oct 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've removed the ConfiguredClientsStrategyEventClass $e argument from the onEvent() method. The event was not used, so I figured the easiest way was to remove this "bc hack".


/**
* A strategy that provide clients configured with HTTPlug bundle. With help from this strategy
* we can use the web debug toolbar for clients found with the discovery.
*
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class ConfiguredClientsStrategy implements DiscoveryStrategy, EventSubscriberInterface
class ConfiguredClientsStrategy implements DiscoveryStrategy
{
/**
* @var HttpClient
Expand Down Expand Up @@ -67,25 +57,4 @@ public static function getCandidates($type)

return [];
}

/**
* Make sure to use our custom strategy.
*/
public function onEvent(ConfiguredClientsStrategyEventClass $e)
{
HttpClientDiscovery::prependStrategy(self::class);
}

/**
* Whenever these events occur we make sure to add our strategy to the discovery.
*
* {@inheritdoc}
*/
public static function getSubscribedEvents()
{
return [
'kernel.request' => ['onEvent', 1024],
'console.command' => ['onEvent', 1024],
];
}
}
35 changes: 35 additions & 0 deletions src/Discovery/ConfiguredClientsStrategyListener.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace Http\HttplugBundle\Discovery;

use Http\Discovery\HttpClientDiscovery;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* @author Wouter de Jong <wouter@wouterj.nl>
*/
class ConfiguredClientsStrategyListener implements EventSubscriberInterface
{
/**
* Make sure to use the custom strategy.
*/
public function onEvent()
{
HttpClientDiscovery::prependStrategy(ConfiguredClientsStrategy::class);
}

/**
* Whenever these events occur we make sure to add the custom strategy to the discovery.
*
* {@inheritdoc}
*/
public static function getSubscribedEvents()
{
return [
'kernel.request' => ['onEvent', 1024],
'console.command' => ['onEvent', 1024],
];
}
}
3 changes: 3 additions & 0 deletions src/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
<service id="httplug.strategy" class="Http\HttplugBundle\Discovery\ConfiguredClientsStrategy">
<argument type="service" id="httplug.auto_discovery.auto_discovered_client" on-invalid="null"/>
<argument type="service" id="httplug.auto_discovery.auto_discovered_async" on-invalid="null"/>
</service>

<service id="httplug.strategy_listener" class="Http\HttplugBundle\Discovery\ConfiguredClientsStrategyListener">
<tag name="kernel.event_subscriber"/>
</service>

Expand Down
10 changes: 3 additions & 7 deletions tests/Functional/DiscoveredClientsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,9 @@
use Http\Discovery\HttpClientDiscovery;
use Http\Discovery\Strategy\CommonClassesStrategy;
use Http\HttplugBundle\Collector\ProfileClient;
use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
use Http\HttplugBundle\Discovery\ConfiguredClientsStrategyListener;
use Nyholm\NSA;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\EventDispatcher\Event as LegacyEvent;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Contracts\EventDispatcher\Event;

class DiscoveredClientsTest extends WebTestCase
{
Expand Down Expand Up @@ -128,9 +125,8 @@ protected function setUp(): void
parent::setUp();

// Reset values
$strategy = new ConfiguredClientsStrategy(null, null);
$strategy = new ConfiguredClientsStrategyListener(null, null);
HttpClientDiscovery::setStrategies([CommonClassesStrategy::class]);
$class = (Kernel::MAJOR_VERSION >= 5) ? Event::class : LegacyEvent::class;
$strategy->onEvent(new $class());
$strategy->onEvent();
}
}