Add Post Meta and Generic Term Aggregations #56
Conversation
| * Configure the Post_Meta aggregation. | ||
| * | ||
| * @param DSL $dsl The DSL object, initialized with the map from the adapter. | ||
| * @param array $args Optional. Additional arguments to pass to the aggregation. |
There was a problem hiding this comment.
I think it's important to expand the phpdoc here to document all the arguments the aggregation accepts (comment applies throughout)
Co-authored-by: Matthew Boynes <[email protected]>
…ctive/elasticsearch-extensions into feature/postmeta-aggregation
WalkthroughThe changes introduce new classes and methods for data aggregation based on post meta and generic term fields in Elasticsearch. The codebase's functionality is expanded, providing more options for data aggregation. The changes also enhance the modularity and configurability of the classes by adding new optional parameters to several methods. The PHP_CodeSniffer standard for Elasticsearch Extensions is also modified. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (6)
- lib/adapters/class-adapter.php (3 hunks)
- lib/aggregations/class-post-meta.php (1 hunks)
- lib/aggregations/class-post-type.php (3 hunks)
- lib/aggregations/class-taxonomy.php (4 hunks)
- lib/aggregations/class-term.php (1 hunks)
- lib/class-controller.php (9 hunks)
Files skipped from review due to trivial changes (1)
- lib/aggregations/class-taxonomy.php
Additional comments (Suppressed): 18
lib/adapters/class-adapter.php (3)
11-21: The new hunk introduces two new classes
Post_MetaandTermfrom theElasticsearch_Extensions\Aggregationsnamespace. Ensure these classes are implemented correctly and are available in the specified namespace.134-145: The
add_post_meta_aggregationmethod is introduced to add a new post meta aggregation to the list of active aggregations. It uses thePost_Metaclass which is new. Ensure that thePost_Metaclass is implemented correctly and that it accepts the arguments as passed in this method.172-197: The
add_term_aggregationmethod is introduced to add a new generic term aggregation to the list of active aggregations. It uses theTermclass which is new. Ensure that theTermclass is implemented correctly and that it accepts the arguments as passed in this method.lib/aggregations/class-post-type.php (3)
17-17: The
Post_Typeclass now extendsTerminstead ofAggregation. Ensure that theTermclass provides all the necessary methods and properties thatPost_Typerequires, and that it doesn't introduce any unwanted behavior.25-30: The
__construct()method now sets a new propertyterm_fieldbefore calling the parent constructor. This property seems to be used for mapping the field for term aggregations. Ensure that this property is properly used in the parent class and that the field mapping is correct.56-58: The
request()method has been removed. This method was previously responsible for getting the DSL for the aggregation to add to the Elasticsearch request object. If this functionality is now handled elsewhere, ensure that it is correctly implemented and that the removal of this method doesn't break any existing functionality.lib/aggregations/class-term.php (3)
1-89: The new
Termclass extends theAggregationclass and provides methods for building the DSL and requests for term aggregations in Elasticsearch. It introduces two new properties:$relationand$term_field. Thefilter()method generates an array of DSL fragments based on the query values and the logical relationship between them. Theparse_buckets()method parses raw Elasticsearch aggregation buckets intoBucketobjects. Therequest()method returns a DSL fragment for the aggregation to add to the Elasticsearch request object. The code seems to be logically sound and well-structured.66-77: The
parse_buckets()method creates a newBucketobject for each bucket in$buckets. It's important to ensure that each$bucketcontains the keys 'key' and 'doc_count', as these are used to create theBucketobjects. If these keys are not present in any of the$buckets, it could lead to undefined index errors. Consider adding error handling or checks to ensure these keys exist.86-88: The
request()method returns a DSL fragment for the aggregation to add to the Elasticsearch request object. Ensure that theaggregate_terms()method of the$this->dslobject accepts the$this->query_varand$this->term_fieldas parameters and returns the expected output.lib/class-controller.php (9)
57-77: The function signature for
enable_co_authors_plus_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.84-96: The function signature for
enable_custom_date_range_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.114-132: The function signature for
enable_post_date_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.139-163: A new function
enable_post_meta_aggregation()has been added. This function allows for the creation of aggregations based on post meta. Ensure that this new function is being used correctly throughout the codebase.170-190: The function signature for
enable_post_type_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.197-215: The function signature for
enable_relative_date_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.254-275: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [222-275]
The function signature for
enable_taxonomy_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
282-308: A new function
enable_term_aggregation()has been added. This function allows for the creation of generic Elasticsearch 'term' aggregations. Ensure that this new function is being used correctly throughout the codebase.332-338: The return type of
get_aggregations()has been updated fromarraytoAggregation[]. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.
| <?php | ||
| /** | ||
| * Elasticsearch Extensions: Post_Meta Aggregation Class | ||
| * | ||
| * @package Elasticsearch_Extensions | ||
| */ | ||
|
|
||
| namespace Elasticsearch_Extensions\Aggregations; | ||
|
|
||
| use Elasticsearch_Extensions\DSL; | ||
|
|
||
| /** | ||
| * Post_Meta aggregation class. Responsible for building the DSL and requests | ||
| * for aggregations as well as holding the result of the aggregation after a | ||
| * response was received. | ||
| */ | ||
| class Post_Meta extends Term { | ||
|
|
||
| /** | ||
| * The postmeta key this aggregation is associated with. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected string $meta_key; | ||
|
|
||
| /** | ||
| * Configure the Post_Meta aggregation. | ||
| * | ||
| * @param DSL $dsl The DSL object, initialized with the map from the adapter. | ||
| * @param array $args Optional. Additional arguments to pass to the aggregation. | ||
| */ | ||
| public function __construct( DSL $dsl, array $args ) { | ||
| if ( ! empty( $args['meta_key'] ) ) { | ||
| $this->label = ucwords( str_replace( [ '-', '_' ], ' ', $args['meta_key'] ) ); | ||
| $this->meta_key = $args['meta_key']; | ||
| $this->query_var = 'post_meta_' . $args['meta_key']; | ||
| $this->term_field = $dsl->map_meta_field( $args['meta_key'], $args['data_type'] ?? '' ); | ||
| unset( $args['meta_key'] ); | ||
| } | ||
|
|
||
| parent::__construct( $dsl, $args ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The Post_Meta class extends the Term class and adds a meta_key property. It overrides the constructor to accept a meta_key argument and uses it to set the label, meta_key, query_var, and term_field properties. The meta_key argument is then removed from the $args array before calling the parent constructor. This class seems to be well-structured and follows the principles of object-oriented programming. However, there is no error handling or validation for the meta_key argument. If it's not provided or is an empty string, the class properties will not be properly initialized, which could lead to unexpected behavior or errors later on. Consider adding a check for this and throwing an exception if it's not valid.
public function __construct( DSL $dsl, array $args ) {
+ if ( empty( $args['meta_key'] ) ) {
+ throw new \InvalidArgumentException( 'The meta_key argument is required and cannot be empty.' );
+ }
if ( ! empty( $args['meta_key'] ) ) {
$this->label = ucwords( str_replace( [ '-', '_' ], ' ', $args['meta_key'] ) );
$this->meta_key = $args['meta_key'];
$this->query_var = 'post_meta_' . $args['meta_key'];
$this->term_field = $dsl->map_meta_field( $args['meta_key'], $args['data_type'] ?? '' );
unset( $args['meta_key'] );
}
parent::__construct( $dsl, $args );
}
}| public function filter(): array { | ||
| if ( empty( $this->query_values ) ) { | ||
| return []; | ||
| } | ||
|
|
||
| // Fork for AND vs. OR logic. | ||
| $filters = []; | ||
| if ( 'OR' === $this->relation ) { | ||
| $filters[] = $this->dsl->terms( $this->term_field, $this->query_values ); | ||
| } else { | ||
| foreach ( $this->query_values as $query_value ) { | ||
| $filters[] = $this->dsl->terms( $this->term_field, $query_value ); | ||
| } | ||
| } | ||
|
|
||
| return $filters; | ||
| } |
There was a problem hiding this comment.
The filter() method uses an if condition to check if $this->query_values is empty and returns an empty array if true. This is a good practice as it prevents unnecessary processing when there are no query values. However, the method could be optimized by returning the array immediately after it's populated, instead of storing it in a variable and returning it at the end. This would make the code more efficient and easier to read.
public function filter(): array {
if ( empty( $this->query_values ) ) {
return [];
}
// Fork for AND vs. OR logic.
if ( 'OR' === $this->relation ) {
return [$this->dsl->terms( $this->term_field, $this->query_values )];
} else {
$filters = [];
foreach ( $this->query_values as $query_value ) {
$filters[] = $this->dsl->terms( $this->term_field, $query_value );
}
return $filters;
}
}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (4)
- lib/adapters/class-adapter.php (10 hunks)
- lib/adapters/class-generic.php (1 hunks)
- lib/class-controller.php (9 hunks)
- lib/class-factory.php (1 hunks)
Additional comments (Suppressed): 20
lib/adapters/class-generic.php (1)
- 16-22: The return type of
get_field_map()method has been updated toarray<string, string>. Ensure that all calls to this method throughout the codebase are expecting this new return type. Also, verify that the returned array always adheres to this type.lib/class-factory.php (1)
- 67-86: The order of the
searchpress_adapter()andvip_enterprise_search_adapter()methods has been swapped. Ensure that this does not affect any dependencies or the order of execution in the codebase.lib/adapters/class-adapter.php (9)
11-21: The new hunk introduces two new classes
Post_MetaandTermfor use in theAdapterclass. Ensure these classes are correctly implemented and tested.110-130: The method
add_cap_author_aggregationnow has a more detailed docstring and a more specific type hint for the$argsparameter. This is a good improvement for code readability and maintainability.133-146: The method
add_custom_date_range_aggregationnow has a more detailed docstring and a more specific type hint for the$argsparameter. This is a good improvement for code readability and maintainability.149-217: The methods
add_post_date_aggregation,add_post_meta_aggregation, andadd_post_type_aggregationnow have more detailed docstrings and more specific type hints for their parameters. This is a good improvement for code readability and maintainability.241-301: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [220-296]
The methods
add_relative_date_aggregation,add_taxonomy_aggregation, andadd_term_aggregationnow have more detailed docstrings and more specific type hints for their parameters. This is a good improvement for code readability and maintainability.
364-370: The return type hint for the
get_field_mapmethod has been updated toarray<string, string>. This is a good improvement for code readability and maintainability.410-418: The type hint for the
$post_typesparameter in theget_searchable_post_typesmethod has been updated tostring[]. This is a good improvement for code readability and maintainability.440-446: The type hint for the
$aggregationsparameter in theparse_aggregationsmethod has been updated toarray<string, mixed>. This is a good improvement for code readability and maintainability.454-460: The type hint for the
$argsparameter in thesuggest_postsmethod has been updated toarray{subtypes?: string[], page?: int, per_page?: int, include?: int[], exclude?: int[]}. This is a good improvement for code readability and maintainability.lib/class-controller.php (9)
57-77: The function signature for
enable_co_authors_plus_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.84-97: The function signature for
enable_custom_date_range_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.115-133: The function signature for
enable_post_date_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.140-169: A new function
enable_post_meta_aggregation()has been added. This function allows for the creation of aggregations based on post meta. Ensure that this new function is being used correctly throughout the codebase.171-191: The function signature for
enable_post_type_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.198-216: The function signature for
enable_relative_date_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.255-276: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [223-276]
The function signature for
enable_taxonomy_aggregation()has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
283-309: A new function
enable_term_aggregation()has been added. This function allows for the creation of generic Elasticsearch 'term' aggregations. Ensure that this new function is being used correctly throughout the codebase.333-339: The return type of
get_aggregations()has been updated toAggregation[]fromarray. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (3)
- .phpcs.xml (1 hunks)
- lib/adapters/class-adapter.php (10 hunks)
- lib/class-controller.php (9 hunks)
Files skipped from review due to trivial changes (1)
- .phpcs.xml
Additional comments (Suppressed): 19
lib/class-controller.php (9)
57-77: The function signature has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
84-97: The function signature has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
115-133: The function signature has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
140-164: A new method
enable_post_meta_aggregationhas been added. This method allows for aggregations based on post meta. Ensure that this new method is properly integrated and called in the codebase.171-191: The function signature has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
198-216: The function signature has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
255-276: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [223-276]
The function signature has been updated to include a more detailed array of arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
284-309: A new method
enable_term_aggregationhas been added. This method allows for a generic Elasticsearch 'term' aggregation. Ensure that this new method is properly integrated and called in the codebase.333-339: The return type of the
get_aggregationsmethod has been refined toAggregation[]. Ensure that all calls to this function throughout the codebase have been updated to handle the new return type.lib/adapters/class-adapter.php (10)
11-21: The new hunk introduces two new classes
Post_MetaandTermfor use in theAdapterclass. Ensure these classes are properly defined and imported.110-130: The function signature for
add_cap_author_aggregationhas been updated with a more detailed array structure for$args. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.133-146: The function signature for
add_custom_date_range_aggregationhas been updated with a more detailed array structure for$args. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.149-217: The function signatures for
add_post_date_aggregation,add_post_meta_aggregation, andadd_post_type_aggregationhave been updated with more detailed array structures for$args. Ensure that all calls to these functions throughout the codebase have been updated to match the new signatures.220-238: The function signature for
add_relative_date_aggregationhas been updated with a more detailed array structure for$args. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.241-296: The function signatures for
add_taxonomy_aggregationandadd_term_aggregationhave been updated with more detailed array structures for$args. Ensure that all calls to these functions throughout the codebase have been updated to match the new signatures.364-370: The return type of the
get_field_map()method has been refined toarray<string, string>. Ensure that all subclasses ofAdapterreturn an array of string keys and values in their implementation of this method.410-418: The function signature for
get_searchable_post_typeshas been updated to specify that$post_typesis an array of strings. Ensure that all calls to this function throughout the codebase pass an array of strings.440-446: The function signature for
parse_aggregationshas been updated to specify that$aggregationsis an array with string keys and mixed values. Ensure that all calls to this function throughout the codebase pass an array of this structure.454-460: The function signature for
suggest_postshas been updated with a more detailed array structure for$args. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
Summary by CodeRabbit