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

Fixes #6865 Container V4 service provider error about providing (options_debug) service #6902

Merged
merged 4 commits into from
Aug 30, 2024

Conversation

Khadreal
Copy link
Contributor

@Khadreal Khadreal commented Aug 23, 2024

Description

Fixes #6865

Documentation

User documentation

No impact.

Technical documentation

Add options_debug directly to the boot method since it doesn't have a service registered.

Type of change

  • Bug fix (non-breaking change which fixes an issue).

Detailed scenario

Check issue #6865 for detailed scenario

Documentation

Fix service provider error for container without a registered service

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide sreenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.

Documentation

  • I prepared the user documentation for the feature/enhancement and shared it in the PR or the GitHub issue.
  • I prepared the technical documentation if needed, and shared it in the PR or the GitHub issue.

Code style

  • I wrote self-explanatory code about what it does.
  • I did not introduce unecessary complexity.

Copy link

codacy-production bot commented Aug 23, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 7999ed21 0.00% (target: 50.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (7999ed2) Report Missing Report Missing Report Missing
Head commit (27eaccb) 37545 9455 25.18%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#6902) 1 0 0.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@CrochetFeve0251
Copy link
Contributor

hmmm I don't really get the solution provided on this branch as it doesn't seem to match what was said during grooming.

Instead, doing this would fix the issue and match the grooming:

<?php
namespace WP_Rocket\Engine\Debug;

use WP_Rocket\Dependencies\League\Container\ServiceProvider\{AbstractServiceProvider, BootableServiceProviderInterface};
use WP_Rocket\Admin\Options_Data;

/**
 * Service provider for Debug
 */
class ServiceProvider extends AbstractServiceProvider implements BootableServiceProviderInterface {
	/**
	 * Array of services provided by this service provider
	 *
	 * @var array
	 */
	protected $provides = [
		'debug_subscriber',
	];

	/**
	 * Array of available debug services.
	 *
	 * @var array
	 */
	protected $services = [];

	/**
	 * Check if the service provider provides a specific service.
	 *
	 * @param string $id The id of the service.
	 *
	 * @return bool
	 */
	public function provides( string $id ): bool {
		return in_array( $id, $this->provides, true );
	}

	/**
	 * Register the service in the provider array
	 *
	 * @return void
	 */
	public function boot(): void {
		$this->services = $this->getContainer()->get( 'debug_resolver' )->get_services();

		if ( empty( $this->services ) ) {
			return;
		}

		$this->provides []= 'options_debug';

		foreach ( $this->services as $service ) {
			$this->provides[] = $service['service'];
		}
	}

	/**
	 * Registers items with the container
	 *
	 * @return void
	 */
	public function register(): void {
		$this->container->add( 'debug_subscriber', DebugSubscriber::class );

		if ( empty( $this->services ) ) {
			return;
		}

		$this->container->add( 'options_debug', Options_Data::class )
			->addArgument( $this->container->get( 'options_api' )->get( 'debug', [] ) );

		foreach ( $this->services as $service ) {
			$this->getContainer()->add( $service['service'], $service['class'] )
				->addArgument( $this->getContainer()->get( 'options_debug' ) )
				->addArgument( $this->getContainer()->get( 'options_api' ) );
		}
	}
}

@Khadreal Khadreal marked this pull request as ready for review August 27, 2024 08:09
@Khadreal Khadreal requested a review from a team August 27, 2024 09:20
@Khadreal Khadreal self-assigned this Aug 27, 2024
@mostafa-hisham
Copy link
Contributor

mostafa-hisham commented Aug 30, 2024

@Khadreal
It didn't work with rucss-debug-tool in my end
but after removing options_debug from the protected $provides array like @CrochetFeve0251 suggested here it worked

@MathieuLamiot
Copy link
Contributor

I just pushed the change showed by @mostafa-hisham.
It is working on my side as well.

@MathieuLamiot
Copy link
Contributor

Tested with the RUCSS Debug tool and working fine.

@MathieuLamiot MathieuLamiot added this pull request to the merge queue Aug 30, 2024
Merged via the queue into develop with commit 1d52f9d Aug 30, 2024
12 of 13 checks passed
@MathieuLamiot MathieuLamiot deleted the fix/6865-container-v4-service-provider branch August 30, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Container V4 service provider lied about providing (options_debug) service
6 participants