Change cache key build up to avoid needless repeats

Author: doekenorg

src/Data/CachedDataSource.php

@@ -84,7 +84,7 @@ private function get_cache_key( ...$arguments ): string {
 		$arguments[] = $this->inner->id();
 
 		try {
-			return md5( json_encode( $arguments, JSON_THROW_ON_ERROR ) );
+			return md5( json_encode( array_values( array_filter( $arguments ) ), JSON_THROW_ON_ERROR ) );
 		} catch ( JsonException $e ) {
 			throw new InvalidArgumentException(
 				'The cache key could not be generated based on the provide arguments',
@@ -105,8 +105,7 @@ private function get_cache_key( ...$arguments ): string {
 	 */
 	private function get_filter_aware_cache_key( ...$arguments ): string {
 		$arguments[] = $this->filters ? $this->filters->to_array() : null;
-		$arguments[] = $this->sort ? $this->sort->to_array() : null;
-		$arguments[] = $this->search;
+		$arguments[] = (string) $this->search;
 
 		return $this->get_cache_key( ...$arguments );
 	}
@@ -141,7 +140,12 @@ private function fetch( string $cache_key, callable $retrieve_result ) {
 	 * @since $ver$
 	 */
 	public function get_data_ids( int $limit = 20, int $offset = 0 ): array {
-		$key = $this->get_filter_aware_cache_key( __FUNCTION__, $limit, $offset );
+		$key = $this->get_filter_aware_cache_key(
+			__FUNCTION__,
+			$this->sort ? $this->sort->to_array() : null,
+			$limit,
+			$offset
+		);
 
 		return $this->fetch(
 			$key,

tests/Data/CachedDataSourceTest.php

@@ -172,7 +172,17 @@ public function testCount(): void {
 		self::assertCount( 2, $ds );
 		self::assertSame( 2, $ds->count() ); // second call
 
-		self::assertCount( 1, $this->trace->get_calls() );
+		$asc_sort = $ds->sort_by( Sort::asc( 'name' ) );
+		self::assertSame( 2, $asc_sort->count() );
+
+		$desc_sort = $ds->sort_by( Sort::desc( 'name' ) );
+		self::assertSame( 2, $desc_sort->count() );
+
+		// Count should be called only once on the trace, as the sorting does not influence the cache key.
+		self::assertSame(
+			[ 'count', 'sort_by', 'sort_by' ],
+			array_column( $this->trace->get_calls(), 0 )
+		);
 	}
 
 	/**

tests/Data/TraceableDataSource.php

@@ -16,7 +16,6 @@
  * @since $ver$
  */
 final class TraceableDataSource implements MutableDataSource {
-
 	/**
 	 * The decorated data source.
 	 *
@@ -59,7 +58,7 @@ public function id() : string {
 	 * @since $ver$
 	 */
 	public function get_data_by_id( string $id ) : array {
-		$this->calls[] = [ __METHOD__, ...func_get_args() ];
+		$this->calls[] = [ __FUNCTION__, ...func_get_args() ];
 
 		return $this->inner->get_data_by_id( $id );
 	}
@@ -69,7 +68,7 @@ public function get_data_by_id( string $id ) : array {
 	 * @since $ver$
 	 */
 	public function get_fields() : array {
-		$this->calls[] = [ __METHOD__ ];
+		$this->calls[] = [ __FUNCTION__ ];
 
 		return $this->inner->get_fields();
 	}
@@ -79,7 +78,7 @@ public function get_fields() : array {
 	 * @since $ver$
 	 */
 	public function get_data_ids( int $limit = 20, int $offset = 0 ) : array {
-		$this->calls[] = [ __METHOD__, ...func_get_args() ];
+		$this->calls[] = [ __FUNCTION__, ...func_get_args() ];
 
 		return $this->inner->get_data_ids( $limit, $offset );
 	}
@@ -89,7 +88,7 @@ public function get_data_ids( int $limit = 20, int $offset = 0 ) : array {
 	 * @since $ver$
 	 */
 	public function filter_by( ?Filters $filters ) : self {
-		$this->calls[] = [ __METHOD__, ...func_get_args() ];
+		$this->calls[] = [ __FUNCTION__, ...func_get_args() ];
 
 		$this->inner = $this->inner->filter_by( $filters );
 
@@ -101,7 +100,7 @@ public function filter_by( ?Filters $filters ) : self {
 	 * @since $ver$
 	 */
 	public function sort_by( ?Sort $sort ) : self {
-		$this->calls[] = [ __METHOD__, ...func_get_args() ];
+		$this->calls[] = [ __FUNCTION__, ...func_get_args() ];
 
 		$this->inner = $this->inner->sort_by( $sort );
 
@@ -113,7 +112,7 @@ public function sort_by( ?Sort $sort ) : self {
 	 * @since $ver$
 	 */
 	public function search_by( ?Search $search ) : self {
-		$this->calls[] = [ __METHOD__, ...func_get_args() ];
+		$this->calls[] = [ __FUNCTION__, ...func_get_args() ];
 		$this->inner   = $this->inner->search_by( $search );
 
 		return $this;
@@ -143,7 +142,7 @@ public function reset() : void {
 	 * @since $ver$
 	 */
 	public function count() : int {
-		$this->calls[] = [ __METHOD__ ];
+		$this->calls[] = [ __FUNCTION__ ];
 
 		return $this->inner->count();
 	}
@@ -156,7 +155,7 @@ public function can_delete() : bool {
 		if ( ! $this->inner instanceof MutableDataSource ) {
 			return false;
 		}
-		$this->calls[] = [ __METHOD__ ];
+		$this->calls[] = [ __FUNCTION__ ];
 
 		return $this->inner->can_delete();
 	}
@@ -170,7 +169,7 @@ public function delete_data_by_id( string ...$ids ) : void {
 			return;
 		}
 
-		$this->calls[] = [ __METHOD__ ];
+		$this->calls[] = [ __FUNCTION__ ];
 
 		$this->inner->delete_data_by_id( ...$ids );
 	}