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.

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

Jump to bottom

Improve connector registration logic #1546

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Improve connector registration logic #1546

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d1a439
Check connector dependencies in registry and loop only once
delawski Aug 7, 2024
459abce
Remove redundant check
delawski Aug 9, 2024
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
84 changes: 35 additions & 49 deletions classes/class-connectors.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ public function load_connectors() {
'wordpress-seo',
);

// Get excluded connectors.
$excluded_connectors = array();

$classes = array();
foreach ( $connectors as $connector ) {
// Load connector class file.
Expand All @@ -106,57 +109,43 @@ public function load_connectors() {
// Set fully qualified class name.
$class_name = sprintf( '\WP_Stream\Connector_%s', str_replace( '-', '_', $connector ) );

// Bail if no class loaded.
if ( ! class_exists( $class_name ) ) {
// Bail if no class loaded or it does not extend WP_Stream\Connector.
if ( ! class_exists( $class_name ) || ! is_subclass_of( $class_name, 'WP_Stream\Connector' ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 thought
I keep going back and forth on this. There's a check on is_subclass_of() below, is there a performance reason for it here also? (I'm asking because I don't know!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! No, I don't think we need that check here. I think it must have slipped through my fingers when doing another iteration on this logic. I'm going to remove it 👍

continue;
}

// Initialize connector.
$class = new $class_name( $this->plugin->log );
$class = new $class_name();
$classes[ $class->name ] = $class;
}

/**
* Allows for adding additional connectors via classes that extend Connector.
*
* @param array $classes An array of Connector objects.
*/
$connector_classes = apply_filters( 'wp_stream_connectors', $classes );

foreach ( $connector_classes as $connector ) {
// Check if the connector class extends WP_Stream\Connector.
if ( ! is_subclass_of( $class, 'WP_Stream\Connector' ) ) {
if ( ! is_subclass_of( $connector, 'WP_Stream\Connector' ) ) {
continue;
}

// Check if the connector events are allowed to be registered in the WP Admin.
if ( is_admin() && ! $class->register_admin ) {
if ( is_admin() && ! $connector->register_admin ) {
continue;
}

// Check if the connector events are allowed to be registered in the WP Frontend.
if ( ! is_admin() && ! $class->register_frontend ) {
if ( ! is_admin() && ! $connector->register_frontend ) {
continue;
}

// Run any final validations the connector may have before used.
if ( $class->is_dependency_satisfied() ) {
$classes[ $class->name ] = $class;
}
}

/**
* Allows for adding additional connectors via classes that extend Connector.
*
* @param array $classes An array of Connector objects.
*/
$this->connectors = apply_filters( 'wp_stream_connectors', $classes );

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

foreach ( $this->connectors as $connector ) {
if ( ! method_exists( $connector, 'get_label' ) ) {
if ( ! $connector->is_dependency_satisfied() ) {
continue;
}
$this->term_labels['stream_connector'][ $connector->name ] = $connector->get_label();
}

// Get excluded connectors.
$excluded_connectors = array();

foreach ( $this->connectors as $connector ) {

// Register error for invalid any connector class.
if ( ! method_exists( $connector, 'get_label' ) ) {
Expand All @@ -180,39 +169,36 @@ public function load_connectors() {
continue;
}

// Check if the connectors extends the Connector class, if not skip it.
if ( ! is_subclass_of( $connector, '\WP_Stream\Connector' ) ) {
/* translators: %s: connector class name, intended to provide help to developers (e.g. "Connector_BuddyPress") */
$this->plugin->admin->notice( sprintf( __( '%1$s class wasn\'t loaded because it doesn\'t extends the %2$s class.', 'stream' ), $connector->name, 'Connector' ), true );
continue;
}

// Store connector label.
if ( ! in_array( $connector->name, $this->term_labels['stream_connector'], true ) ) {
$this->term_labels['stream_connector'][ $connector->name ] = $connector->get_label();
}

$connector_name = $connector->name;
$is_excluded = in_array( $connector_name, $excluded_connectors, true );

/**
* Allows excluded connectors to be overridden and registered.
*
* @param bool $is_excluded True if excluded, otherwise false.
* @param string $connector The current connector's slug.
* @param array $excluded_connectors An array of all excluded connector slugs.
*/
$is_excluded_connector = apply_filters( 'wp_stream_check_connector_is_excluded', $is_excluded, $connector_name, $excluded_connectors );
$is_excluded_connector = apply_filters(
'wp_stream_check_connector_is_excluded',
in_array( $connector->name, $excluded_connectors, true ),
$connector->name,
$excluded_connectors
);

if ( $is_excluded_connector ) {
continue;
}

// Add connector to the registry.
$this->connectors[ $connector->name ] = $connector;

// Register the connector.
$connector->register();

// Link context labels to their connector.
$this->contexts[ $connector->name ] = $connector->get_context_labels();

// Store connector label.
$this->term_labels['stream_connector'][ $connector->name ] = $connector->get_label();

// Add new terms to our label lookup array.
$this->term_labels['stream_action'] = array_merge(
$this->term_labels['stream_action'],
Expand All @@ -229,8 +215,8 @@ public function load_connectors() {
/**
* Fires after all connectors have been registered.
*
* @param array $labels All register connectors labels array
* @param Connectors $connectors The Connectors object
* @param array $labels All register connectors labels array
* @param Connectors $connector_classes The Connectors object
*/
do_action( 'wp_stream_after_connectors_registration', $labels, $this );
}
Expand Down
Loading