refactor: make AbstractConnectionResolver::should_execute() non-abstract and add ::pre_should_execute()

src/Data/Connection/AbstractConnectionResolver.php

@@ -64,9 +64,13 @@ abstract class AbstractConnectionResolver {
 	/**
 	 * Whether the connection resolver should execute.
 	 *
-	 * @var bool
+	 * If `false`, the connection resolve will short-circuit and return an empty array.
+	 *
+	 * Filterable by `graphql_connection_pre_should_execute` and `graphql_connection_should_execute`.
+	 *
+	 * @var ?bool
 	 */
-	protected $should_execute = true;
+	protected $should_execute;
 
 	/**
 	 * The loader name.
@@ -172,8 +176,8 @@ public function __construct( $source, array $args, AppContext $context, ResolveI
 		 */
 		$this->args = $args;
 
-		// Bail if the Post->ID is empty, as that indicates a private post.
-		if ( $source instanceof Post && empty( $source->ID ) ) {
+		// Pre-check if the connection should execute so we can skip expensive logic if we already know it shouldn't execute.
+		if ( ! $this->get_pre_should_execute( $this->source, $this->unfiltered_args, $this->context, $this->info ) ) {
 			$this->should_execute = false;
 		}
 
@@ -257,22 +261,29 @@ abstract public function get_query_args();
 	abstract public function get_query();
 
 	/**
-	 * Should_execute
-	 *
-	 * Determine whether or not the query should execute.
-	 *
-	 * Return true to execute, return false to prevent execution.
-	 *
-	 * Various criteria can be used to determine whether a Connection Query should
-	 * be executed.
+	 * Used to determine whether the connection query should be executed. This is useful for short-circuiting the connection resolver before executing the query.
 	 *
-	 * For example, if a user is requesting revisions of a Post, and the user doesn't have
-	 * permission to edit the post, they don't have permission to view the revisions, and therefore
-	 * we can prevent the query to fetch revisions from executing in the first place.
+	 * When `pre_should_excecute()` returns false, that's a sign the Resolver shouldn't execute the query. Otherwise, the more expensive logic logic in `should_execute()` will run later in the lifecycle.
 	 *
-	 * @return bool
+	 * @param mixed                                $source  Source passed down from the resolve tree
+	 * @param array<string,mixed>                  $args    Array of arguments input in the field as part of the GraphQL query.
+	 * @param \WPGraphQL\AppContext                $context The app context that gets passed down the resolve tree.
+	 * @param \GraphQL\Type\Definition\ResolveInfo $info    Info about fields passed down the resolve tree.
 	 */
-	abstract public function should_execute();
+	protected function pre_should_execute( $source, array $args, AppContext $context, ResolveInfo $info ): bool {
+		$should_execute = true;
+
+		/**
+		 * If the source is a Post and the ID is empty, we should not execute the query.
+		 *
+		 * @todo this can probably be moved to the PostObjectConnectionResolver when we don't care about b/c.
+		 */
+		if ( $source instanceof Post && empty( $source->ID ) ) {
+			$should_execute = false;
+		}
+
+		return $should_execute;
+	}
 
 	/**
 	 * The maximum number of items that should be returned by the query.
@@ -322,6 +333,25 @@ public function get_ids_from_query() {
 		);
 	}
 
+	/**
+	 * Determine whether or not the query should execute.
+	 *
+	 * Return true to exeucte, return false to prevent execution.
+	 *
+	 * Various criteria can be used to determine whether a Connection Query should be executed.
+	 *
+	 * For example, if a user is requesting revisions of a Post, and the user doesn't have permission to edit the post, they don't have permission to view the revisions, and therefore we can prevent the query to fetch revisions from executing in the first place.
+	 *
+	 * Runs only if `pre_should_execute()` returns true.
+	 *
+	 * @todo This is public for b/c but it should be protected.
+	 *
+	 * @return bool
+	 */
+	public function should_execute() {
+		return true;
+	}
+
 	/**
 	 * Returns the offset for a given cursor.
 	 *
@@ -428,12 +458,6 @@ public function get_loader_name() {
 		return $this->loader_name;
 	}
 
-	/**
-	 * Returns whether the connection should execute.
-	 */
-	public function get_should_execute(): bool {
-		return $this->should_execute;
-	}
 
 	/**
 	 * Returns the $args passed to the connection, before any modifications.
@@ -496,6 +520,19 @@ public function get_query_amount() {
 		return $this->query_amount;
 	}
 
+	/**
+	 * Returns whether the connection should execute.
+	 *
+	 * If conditions are met that should prevent the execution, we can bail from resolving early, before the query is executed.
+	 */
+	public function get_should_execute(): bool {
+		// If `pre_should_execute()` or other logic has yet to run, we should run the full `should_execute()` logic.
+		if ( ! isset( $this->should_execute ) ) {
+			$this->should_execute = $this->should_execute();
+		}
+
+		return $this->should_execute;
+	}
 
 	/**
 	 * Returns an array of IDs for the connection.
@@ -649,6 +686,33 @@ public function one_to_one() {
 		return $this;
 	}
 
+	/**
+	 * Gets whether or not the query should execute, BEFORE any data is fetched or altered, filtered by 'graphql_connection_pre_should_execute'.
+	 *
+	 * @param mixed                                $source  The source that's passed down the GraphQL queries.
+	 * @param array<string,mixed>                  $args    The inputArgs on the field.
+	 * @param \WPGraphQL\AppContext                $context The AppContext passed down the GraphQL tree.
+	 * @param \GraphQL\Type\Definition\ResolveInfo $info    The ResolveInfo passed down the GraphQL tree.
+	 */
+	protected function get_pre_should_execute( $source, array $args, AppContext $context, ResolveInfo $info ): bool {
+		$should_execute = $this->pre_should_execute( $source, $args, $context, $info );
+
+		/**
+		 * Filters whether or not the query should execute, BEFORE any data is fetched or altered.
+		 *
+		 * This is evaluated based solely on the values passed to the constructor, before any data is fetched or altered, and is useful for shortcircuiting the Connection Resolver before any heavy logic is executed.
+		 *
+		 * For more in-depth checks, use the `graphql_connection_should_execute` filter instead.
+		 *
+		 * @param bool                                 $should_execute Whether or not the query should execute.
+		 * @param mixed                                $source         The source that's passed down the GraphQL queries.
+		 * @param array                                $args           The inputArgs on the field.
+		 * @param \WPGraphQL\AppContext                $context        The AppContext passed down the GraphQL tree.
+		 * @param \GraphQL\Type\Definition\ResolveInfo $info           The ResolveInfo passed down the GraphQL tree.
+		 */
+		return apply_filters( 'graphql_connection_pre_should_execute', $should_execute, $source, $args, $context, $info );
+	}
+
 	/**
 	 * Returns the loader.
 	 *
@@ -792,12 +856,9 @@ function () {
 	 * @throws \Exception
 	 */
 	public function execute_and_get_ids() {
-
 		/**
-		 * If should_execute is explicitly set to false already, we can
-		 * prevent execution quickly. If it's not, we need to
-		 * call the should_execute() method to execute any situational logic
-		 * to determine if the connection query should execute or not
+		 * If should_execute is explicitly set to false already, we can prevent execution quickly.
+		 * If it's not, we need to call the should_execute() method to execute any situational logic to determine if the connection query should execute.
 		 */
 		$should_execute = false === $this->should_execute ? false : $this->should_execute();
 

src/Data/Connection/CommentConnectionResolver.php

@@ -311,11 +311,4 @@ public function sanitize_input_fields( array $args ) {
 	public function is_valid_offset( $offset ) {
 		return ! empty( get_comment( $offset ) );
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/ContentTypeConnectionResolver.php

@@ -73,11 +73,4 @@ protected function loader_name(): string {
 	public function is_valid_offset( $offset ) {
 		return (bool) get_post_type_object( $offset );
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/EnqueuedScriptsConnectionResolver.php

@@ -72,11 +72,4 @@ public function is_valid_offset( $offset ) {
 		global $wp_scripts;
 		return isset( $wp_scripts->registered[ $offset ] );
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/EnqueuedStylesheetConnectionResolver.php

@@ -79,11 +79,4 @@ public function is_valid_offset( $offset ) {
 		global $wp_styles;
 		return isset( $wp_styles->registered[ $offset ] );
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/PostObjectConnectionResolver.php

@@ -113,33 +113,30 @@ public function get_ids_from_query() {
 	 * {@inheritDoc}
 	 */
 	public function should_execute() {
-		if ( false === $this->should_execute ) {
-			return false;
-		}
-
 		/**
-		 * For revisions, we only want to execute the connection query if the user
-		 * has access to edit the parent post.
+		 * If the post_type is not revision we can just return the parent::should_execute().
 		 *
-		 * If the user doesn't have permission to edit the parent post, then we shouldn't
-		 * even execute the connection
-		 */
-		if ( isset( $this->post_type ) && 'revision' === $this->post_type ) {
-			if ( $this->source instanceof Post ) {
-				$parent_post_type_obj = get_post_type_object( $this->source->post_type );
-				if ( ! isset( $parent_post_type_obj->cap->edit_post ) || ! current_user_can( $parent_post_type_obj->cap->edit_post, $this->source->ID ) ) {
-					$this->should_execute = false;
-				}
-				/**
-				 * If the connection is from the RootQuery, check if the user
-				 * has the 'edit_posts' capability
-				 */
-			} elseif ( ! current_user_can( 'edit_posts' ) ) {
-					$this->should_execute = false;
+		 * @todo This works because AbstractConnectionResolver::pre_should_execute does a permission check on the `Post` model )
+		 */
+		if ( ! isset( $this->post_type ) || 'revision' !== $this->post_type ) {
+			return parent::should_execute();
+		}
+
+		// If the connection is from the RootQuery (i.e. it doesn't have a `Post` source), check if the user has the 'edit_posts' capability.
+		if ( ! $this->source instanceof Post && current_user_can( 'edit_posts' ) ) {
+			return true;
+		}
+
+		// For revisions, we only want to execute the connection query if the user has access to edit the parent post.
+		if ( $this->source instanceof Post ) {
+			$parent_post_type_obj = get_post_type_object( $this->source->post_type );
+
+			if ( isset( $parent_post_type_obj->cap->edit_post ) && current_user_can( $parent_post_type_obj->cap->edit_post, $this->source->ID ) ) {
+				return true;
 			}
 		}
 
-		return $this->should_execute;
+		return false;
 	}
 
 	/**

src/Data/Connection/TaxonomyConnectionResolver.php

@@ -74,11 +74,4 @@ protected function loader_name(): string {
 	public function is_valid_offset( $offset ) {
 		return (bool) get_taxonomy( $offset );
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/TermObjectConnectionResolver.php

@@ -294,13 +294,4 @@ static function ( $id ) {
 	public function is_valid_offset( $offset ) {
 		return get_term( absint( $offset ) ) instanceof \WP_Term;
 	}
-
-	/**
-	 * {@inheritDoc}
-	 *
-	 * Default is true, meaning any time a TermObjectConnection resolver is asked for, it will execute.
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/ThemeConnectionResolver.php

@@ -67,11 +67,4 @@ public function is_valid_offset( $offset ) {
 		$theme = wp_get_theme( $offset );
 		return $theme->exists();
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }

src/Data/Connection/UserConnectionResolver.php

@@ -281,11 +281,4 @@ protected function sanitize_input_fields( array $args ) {
 	public function is_valid_offset( $offset ) {
 		return (bool) get_user_by( 'ID', absint( $offset ) );
 	}
-
-	/**
-	 * {@inheritDoc}
-	 */
-	public function should_execute() {
-		return true;
-	}
 }