wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

Change the initialization from ServiceProviders and Subscribers

Open CrochetFeve0251 opened this issue 3 years ago • 2 comments

Is your feature request related to a problem? Please describe. My solution aims to improve architecture from the initialization from ServiceProviders and Subscribers by removing the reference to subscribers inside the Plugin.php file.

Describe the solution you'd like For that I would consider a solution using the same logic as rocket-cdn plugin to initialize the plugin. However instead of one kind of subscriber referenced on service providers we would have 4 types:

  • Front subscribers
  • Admin subscribers
  • Common subscribers
  • License subscribers

Also as subscribers names will be hidden, we should create a prefix for each service provider that add before before the id from the class that would prevent conflict between two subscribers.

One drawback that I potentially saw is that we need to initialize every service provider to check for subscribers. However as we don't use ServiceProviders constructors (we have no operations inside them) I don't think this would be a huge issue.

Another potential issue I saw was adding multiple time the same ServiceProvider to the container. However the container does a check to see if it is already there before adding it so that shouldn't be an issue.

Code

For that we could create a new base for all ServiceProvider that we will call AbstractServiceProvider with the following content:

use WP_Rocket\Dependencies\League\Container\ServiceProvider\AbstractServiceProvider as LeagueServiceProvider;


abstract class AbstractServiceProvider extends LeagueServiceProvider {
    protected $prefix = '';

   protected $front_subscribers = [];
   protected $admin_subscribers = [];
   protected $common_subscribers = [];
   protected $license_subscribers = [];

    public function provides(string $id): bool
    {
        return in_array($id, $this->provides) || in_array($id, $this->admin_subscribers) || in_array($id, $this->front_subscribers) || in_array($id, $this->common_subscribers) || in_array($id, $this->license_subscribers);
    }

  public function get_front_subscribers(): array {
    return $this->front_subscribers;
  }

  public function get_admin_subscribers(): array {
    return $this->admin_subscribers;
  }

  public function get_common_subscribers(): array {
    return $this->common_subscribers;
  }

  public function get_license_subscribers(): array {
    return $this->license_subscribers;
  }

   public function add(string $id, string $class) {
       $internal_id = $this->generate_container_id( $id );

        if( ! $this->provides( $internal_id ) && $expose ) {
            $this->provides[] = $internal_id;
        }
       return $this->getContainer()->add( $internal_id, $class); 
  }

 protected function getInternal(string $id)
    {
        return $this->getContainer()->get( $this->generate_container_id( $id ) );
    }

   protected function generate_container_id(string $id)
    {
        if ($this->prefix) {
            return $this->prefix . $id;
        }

        $class = get_class( $this );
        $class = dirname( $class );

        $class = trim( $class, '\\' );
        $class = str_replace( '\\', '.', $class );
        $class = strtolower( preg_replace( ['/([a-z])\d([A-Z])/', '/[^_]([A-Z][a-z])]/'], '$1_$2', $class ) );
        $this->prefix = $class . '.';

        return $this->prefix . $id;
    }
}

Concerning the Plugin.php we can change the logic to load subcribers like that:

 /**
     * Loads the plugin into WordPress.
     *
     * @since 3.0
     *
     * @return void
     */
    public function load() {
        $this->event_manager = new Event_Manager();
        $this->container->addShared( 'event_manager', $this->event_manager );

        $this->options_api = new Options( 'wp_rocket_' );
	$this->container->add( 'options_api', $this->options_api );
	$this->container->addServiceProvider( OptionsServiceProvider::class );
	$this->options = $this->container->get( 'options' );

        $this->load_subscribers();
    }

    /**
     * Get the subscribers to add to the event manager.
     *
     * @since 3.6
     *
     * @return array array of subscribers.
     */
    private function load_subscribers() {
        $providers = [
            new AdminDatabaseServiceProvider(),
            new SupportServiceProvider(),
            new BeaconServiceProvider(),
            new RocketCDNServiceProvider(),
            new CacheServiceProvider(),
            new CriticalPathServiceProvider(),
            new HealthCheckServiceProvider(),
            new MediaServiceProvider(),
            new DeferJSServiceProvider(),
            new SettingsServiceProvider(),
            new EngineAdminServiceProvider(),
            new OptimizationAdminServiceProvider(),
            new CapabilitiesServiceProvider(),
            new AddonServiceProvider(),
            new VarnishServiceProvider(),
            new PreloadServiceProvider(),
            new PreloadLinksServiceProvider(),
            new CDNServiceProvider(),
            new Common_Subscribers(),
            new ThirdPartyServiceProvider(),
            new HostingsServiceProvider(),
            new PluginServiceProvider(),
            new DelayJSServiceProvider(),
            new RUCSSServiceProvider(),
            new HeartbeatServiceProvider(),
            new DynamicListsServiceProvider(),
            new LicenseServiceProvider(),
            new ThemesServiceProvider(),
            new APIServiceProvider(),
        ];

        if ( is_admin() ) {
            $this->init_admin_subscribers( $providers );
        } elseif ($this->is_valid_key ) {
            $this->init_valid_key_subscribers( $providers );
        }

       if ( ! is_admin() ) {
        $this->init_front_subscribers();
      }

        $this->init_common_subscribers( $providers );
    }

    /**
     * Initializes the admin subscribers.
     *
     * @since 3.6
     *
     * @return array array of subscribers.
     */
    private function init_admin_subscribers(array $providers) {
        $this->init_subscribers( $providers, 'get_admin_subscribers');
    }

    private function init_valid_key_subscribers(array $providers) {
        $this->init_subscribers( $providers, 'get_license_subscribers');
    }

    private function init_common_subscribers(array $providers) {
        $this->init_subscribers( $providers, 'get_common_subscribers');
    }


    private function init_front_subscribers(array $providers) {
        $this->init_subscribers( $providers, 'get_front_subscribers');
    }


    /**
     * Initializes the admin subscribers.
     *
     * @since 3.6
     *
     * @return array array of subscribers.
     */
    private function init_subscribers(array $providers, string $method) {
        /** @var AbstractServiceProvider[] $providers */
        foreach ($providers as $provider) {
            if(! $provider->$method() ) {
              continue;
            }
            $this->container->addServiceProvider( $provider );
             $this->add_subscribers( $provider->$method() );
        }
    }

   protected function add_subscribers(array $subscribers) {
      foreach ( $subscribers as $subscriber ) {
			$this->event_manager->add_subscriber( $this->container->get( $subscriber ) );
		} 
    }

CrochetFeve0251 avatar Dec 15 '22 10:12 CrochetFeve0251

@MathieuLamiot What do you think about this one?

piotrbak avatar Jun 28 '24 11:06 piotrbak

To keep opened for now. We'll probably move it to WP Launchpad though. I'll make sure it's handled next week.

MathieuLamiot avatar Jun 28 '24 11:06 MathieuLamiot

@MathieuLamiot I am not sure if this needs to be closed as it will be needed to then make wpr compatible with launchpad.

However, I made a like to that issue on Launchpad: https://github.com/wp-launchpad/launchpad/issues/25

CrochetFeve0251 avatar Jul 08 '24 13:07 CrochetFeve0251