Skip to content

Commit

Permalink
Merge 57a433f into a66f3e1
Browse files Browse the repository at this point in the history
  • Loading branch information
justlevine committed Apr 24, 2024
2 parents a66f3e1 + 57a433f commit a0c8157
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 108 deletions.
117 changes: 89 additions & 28 deletions src/Data/Connection/AbstractConnectionResolver.php
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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();

Expand Down
7 changes: 0 additions & 7 deletions src/Data/Connection/CommentConnectionResolver.php
Expand Up @@ -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;
}
}
7 changes: 0 additions & 7 deletions src/Data/Connection/ContentTypeConnectionResolver.php
Expand Up @@ -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;
}
}
7 changes: 0 additions & 7 deletions src/Data/Connection/EnqueuedScriptsConnectionResolver.php
Expand Up @@ -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;
}
}
7 changes: 0 additions & 7 deletions src/Data/Connection/EnqueuedStylesheetConnectionResolver.php
Expand Up @@ -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;
}
}
41 changes: 19 additions & 22 deletions src/Data/Connection/PostObjectConnectionResolver.php
Expand Up @@ -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;
}

/**
Expand Down
7 changes: 0 additions & 7 deletions src/Data/Connection/TaxonomyConnectionResolver.php
Expand Up @@ -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;
}
}
9 changes: 0 additions & 9 deletions src/Data/Connection/TermObjectConnectionResolver.php
Expand Up @@ -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;
}
}
7 changes: 0 additions & 7 deletions src/Data/Connection/ThemeConnectionResolver.php
Expand Up @@ -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;
}
}
7 changes: 0 additions & 7 deletions src/Data/Connection/UserConnectionResolver.php
Expand Up @@ -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;
}
}

0 comments on commit a0c8157

Please sign in to comment.