Skip to content

Remove container dependency #13

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Remove container dependency #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4b3aaf
Removes client container connection
jaapio Jan 23, 2017
3012b77
Remove php-di as hard dependency
jaapio Jan 23, 2017
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
1 change: 0 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"api-clients/hydrator": "^1.0.1",
"api-clients/middleware": "^4.0 || ^3.0 || ^2.0 || ^1.1",
"api-clients/transport": "^3.0 || ^2.0 || ^1.0",
"php-di/php-di": "^5.4.3",
Copy link
Member

Choose a reason for hiding this comment

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

Can you revert this, PHP-DI was specifically picked for the autowiring of dependencies for services and handlers used throughout the entire @php-api-clients project. External definitions are supported through https://github.com/php-api-clients/foundation/blob/master/src/Factory.php#L53 but I would like to support wrappoing an external PSR-11 container in the future.

"psr/container": "^1.0"
},
"require-dev": {
Expand Down
190 changes: 24 additions & 166 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 3 additions & 25 deletions src/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,17 @@

final class Client implements ClientInterface
{
/**
* @var ContainerInterface
*/
private $container;

/**
* @var CommandBusInterface
*/
private $commandBus;

/**
* @param ContainerInterface $container
*/
public function __construct(ContainerInterface $container)
{
$this->container = $container;
$this->commandBus = $this->container->get(CommandBusInterface::class);
}

/**
* @return ContainerInterface
*/
public function getContainer(): ContainerInterface
{
return $this->container;
}

/**
* @return mixed
* @param CommandBusInterface $commandBus
*/
public function getFromContainer(string $id)
public function __construct(CommandBusInterface $commandBus)
{
return $this->container->get($id);
$this->commandBus = $commandBus;
}

/**
Expand Down
10 changes: 0 additions & 10 deletions src/ClientInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,6 @@

interface ClientInterface
{
/**
* @return ContainerInterface
*/
public function getContainer(): ContainerInterface;

/**
* @return mixed
*/
public function getFromContainer(string $id);
Copy link
Member

Choose a reason for hiding this comment

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

This is only used in two clients and from I've gathered it can easily updated after this fix 👍


/**
* @param $command
* @return CancellablePromiseInterface
Expand Down
3 changes: 2 additions & 1 deletion src/Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ public static function create(
LoopInterface $loop,
array $options = []
): Client {
$container = self::createContainer($loop, $options);
return new Client(
Copy link
Author

Choose a reason for hiding this comment

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

Do we actually need this method? This factory should return the commandbus, not the client?

Copy link
Member

Choose a reason for hiding this comment

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

Actually returning the command bus instead of the client makes sense. Clients should using it building their client and abstract all operations into it 👍 .

Copy link
Member

Choose a reason for hiding this comment

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

Ignore this comment for now. It would be something for a follow up PR, like to explore the idea though.

self::createContainer($loop, $options)
$container->get(CommandBusInterface::class)
);
}

Expand Down
7 changes: 1 addition & 6 deletions tests/ClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,8 @@ public function handle($command)
);

$commandBus = new CommandBus($loop, $handlerMiddleware);
$client = new Client($commandBus);

$container = ContainerBuilder::buildDevContainer();
$container->set(CommandBusInterface::class, $commandBus);
$client = new Client($container);

$this->assertSame($container, $client->getContainer());
$this->assertSame($commandBus, $client->getFromContainer(CommandBusInterface::class));
$this->assertSame($command, await($client->handle($command), $loop));
}

Expand Down
Loading