Skip to content
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

Fix conditional services referencing not-yet registered services #6784

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft
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
9 changes: 9 additions & 0 deletions tests/php/src/Fixture/DummyService.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,13 @@

final class DummyService implements Service {

/**
* A method that can be triggered for tests.
*
* @return bool Always returns true.
*/
public function method_to_trigger()
{
return true;
}
}
19 changes: 19 additions & 0 deletions tests/php/src/Fixture/DummyServiceWithCondition.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace AmpProject\AmpWP\Tests\Fixture;

use AmpProject\AmpWP\Infrastructure\Conditional;
use AmpProject\AmpWP\Infrastructure\Service;

class DummyServiceWithCondition implements Service, Conditional {

/**
* Check whether the conditional object is currently needed.
*
* @return bool Whether the conditional object is needed.
*/
public static function is_needed()
{
return true;
}
}
23 changes: 23 additions & 0 deletions tests/php/src/Fixture/DummyServiceWithReferencingCondition.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace AmpProject\AmpWP\Tests\Fixture;

use AmpProject\AmpWP\Infrastructure\Conditional;
use AmpProject\AmpWP\Infrastructure\Service;
use AmpProject\AmpWP\Services;

class DummyServiceWithReferencingCondition implements Service, Conditional {

/**
* Check whether the conditional object is currently needed.
*
* @return bool Whether the conditional object is needed.
*/
public static function is_needed()
{
/** @var DummyService $service */
$service = Services::get( 'service_a' );

return $service->method_to_trigger();
}
}
84 changes: 83 additions & 1 deletion tests/php/src/Infrastructure/ServiceBasedPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
use AmpProject\AmpWP\Infrastructure\ServiceContainer\SimpleServiceContainer;
use AmpProject\AmpWP\Tests\Fixture\DummyService;
use AmpProject\AmpWP\Tests\Fixture\DummyServiceBasedPlugin;
use AmpProject\AmpWP\Tests\Fixture\DummyServiceWithCondition;
use AmpProject\AmpWP\Tests\Fixture\DummyServiceWithDelay;
use AmpProject\AmpWP\Tests\Fixture\DummyServiceWithReferencingCondition;
use AmpProject\AmpWP\Tests\Fixture\DummyServiceWithRequirements;
use AmpProject\AmpWP\Tests\TestCase;

Expand Down Expand Up @@ -63,7 +65,6 @@ public function test_it_registers_default_services() {
$plugin = $this->getMockBuilder( DummyServiceBasedPlugin::class )
->enableOriginalConstructor()
->setConstructorArgs( [ true, null, $container ] )
->setMethods()
->setMethodsExcept(
[
'register',
Expand Down Expand Up @@ -238,6 +239,87 @@ public function test_it_handles_delays_for_requirements() {
$this->assertInstanceof( DummyServiceWithRequirements::class, $container->get( 'service_with_requirements' ) );
}

public function test_it_handles_conditions_for_requirements() {
$container = new SimpleServiceContainer();
$plugin = $this->getMockBuilder( DummyServiceBasedPlugin::class )
->enableOriginalConstructor()
->setConstructorArgs( [ true, null, $container ] )
->setMethodsExcept(
[
'collect_missing_requirements',
'register',
'register_services',
'requirements_are_met',
'get_container',
'get_service_classes',
]
)
->getMock();

$service_callback = static function ( $services ) {
return array_merge(
$services,
[
'service_with_requirements' => DummyServiceWithRequirements::class,
'service_a' => DummyServiceWithCondition::class,
]
);
};

add_filter( 'services', $service_callback );

$plugin->register();

$this->assertEquals( 4, count( $container ) );
$this->assertTrue( $container->has( 'service_a' ) );
$this->assertInstanceof( DummyServiceWithCondition::class, $container->get( 'service_a' ) );
$this->assertTrue( $container->has( 'service_b' ) );
$this->assertInstanceof( DummyService::class, $container->get( 'service_b' ) );
$this->assertTrue( $container->has( 'service_with_requirements' ) );
$this->assertInstanceof( DummyServiceWithRequirements::class, $container->get( 'service_with_requirements' ) );
}

public function test_it_handles_referencing_conditions_for_requirements() {
$container = new SimpleServiceContainer();
$plugin = $this->getMockBuilder( DummyServiceBasedPlugin::class )
->enableOriginalConstructor()
->setConstructorArgs( [ true, null, $container ] )
->setMethodsExcept(
[
'collect_missing_requirements',
'register',
'register_services',
'requirements_are_met',
'get_container',
'get_service_classes',
]
)
->getMock();

$service_callback = static function ( $services ) {
return array_merge(
$services,
[
'service_with_requirements' => DummyServiceWithRequirements::class,
'service_b' => DummyServiceWithReferencingCondition::class,
'service_a' => DummyService::class,
]
);
};

add_filter( 'services', $service_callback );

$plugin->register();

$this->assertEquals( 4, count( $container ) );
$this->assertTrue( $container->has( 'service_a' ) );
$this->assertInstanceof( DummyServiceWithReferencingCondition::class, $container->get( 'service_b' ) );
$this->assertTrue( $container->has( 'service_b' ) );
$this->assertInstanceof( DummyService::class, $container->get( 'service_a' ) );
$this->assertTrue( $container->has( 'service_with_requirements' ) );
$this->assertInstanceof( DummyServiceWithRequirements::class, $container->get( 'service_with_requirements' ) );
}

public function test_it_throws_an_exception_if_unrecognized_service_is_required() {
$container = new SimpleServiceContainer();
$plugin = $this->getMockBuilder( DummyServiceBasedPlugin::class )
Expand Down