Fix #1235: stop overriding core's resize opt-out on client side uploads

Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php

@@ -0,0 +1,136 @@
+<?php
+declare(strict_types=1);
+
+namespace Imagify\Tests\Unit\classes\Context\WP;
+
+use Brain\Monkey\Functions;
+use Imagify\Context\WP;
+use Imagify\Tests\Unit\TestCase;
+use Mockery;
+
+/**
+ * Tests for \Imagify\Context\WP::filter_big_image_size_threshold() — WordPress 7.1 switches its
+ * own downscaling off while the browser handles the upload, because the browser supplies the
+ * scaled file itself. Overriding that leaves a conflicting "-scaled" file behind and points
+ * `original_image` at it, so the threshold has to be handed back untouched there.
+ *
+ * Only there: a `false` from anywhere else is still overridden, since nothing produced a scaled
+ * file in that case and the image would end up resized by nobody.
+ *
+ * @covers \Imagify\Context\WP::filter_big_image_size_threshold
+ * @covers \Imagify\Context\WP::maybe_flag_client_side_scaling
+ * @group  ContextWP
+ * @since  2.3.3
+ */
+class FilterBigImageSizeThresholdTest extends TestCase {
+
+	/**
+	 * Stubs the resizing option as enabled with the given width.
+	 *
+	 * @param int $width Configured resizing width.
+	 */
+	private function stubResizingOption( int $width ): void {
+		Functions\when( 'get_imagify_option' )->alias(
+			function ( $option ) use ( $width ) {
+				return 'resize_larger' === $option ? 1 : $width;
+			}
+		);
+	}
+
+	/**
+	 * Build a request stub returning the given value for the 'generate_sub_sizes' parameter.
+	 *
+	 * @param  mixed $value Value the parameter should return.
+	 * @return Mockery\MockInterface
+	 */
+	private function requestReturning( $value ) {
+		$request = Mockery::mock( 'WP_REST_Request' );
+		$request->shouldReceive( 'get_param' )->with( 'generate_sub_sizes' )->andReturn( $value );
+
+		return $request;
+	}
+
+	/**
+	 * Test: the threshold is handed back untouched for the upload the browser scaled.
+	 */
+	public function testReturnsFalseUntouchedForABrowserScaledUpload(): void {
+		Functions\when( 'set_transient' )->justReturn( true );
+		Functions\expect( 'get_imagify_option' )->never();
+
+		$context = new WP();
+
+		// The sequence WordPress produces: the upload is noted, then the filter runs.
+		$context->maybe_flag_client_side_scaling( (object) [ 'ID' => 123 ], $this->requestReturning( false ), true );
+
+		$this->assertFalse( $context->filter_big_image_size_threshold( false ) );
+	}
+
+	/**
+	 * Test: a false from anywhere else is overridden with Imagify's value, as it always was.
+	 * Nothing scaled the file in that case, so standing down would leave it unresized.
+	 */
+	public function testOverridesAFalseThatDidNotComeFromTheBrowserFlow(): void {
+		$this->stubResizingOption( 2560 );
+
+		// No upload was noted, so this false came from somewhere else.
+		$this->assertSame( 2560, ( new WP() )->filter_big_image_size_threshold( false ) );
+	}
+
+	/**
+	 * Test: Imagify's own resizing value is applied when core did not opt out.
+	 */
+	public function testReturnsImagifyThresholdWhenResizingIsEnabled(): void {
+		$this->stubResizingOption( 1200 );
+
+		$this->assertSame( 1200, ( new WP() )->filter_big_image_size_threshold( 2560 ) );
+	}
+
+	/**
+	 * Test: with the setting off, the threshold is 0 and WordPress skips its own resizing.
+	 */
+	public function testReturnsZeroWhenResizingIsDisabled(): void {
+		Functions\when( 'get_imagify_option' )->justReturn( 0 );
+
+		$this->assertSame( 0, ( new WP() )->filter_big_image_size_threshold( 2560 ) );
+	}
+
+	/**
+	 * Test: the attachment is flagged when WordPress hands the sub sizes to the browser.
+	 */
+	public function testFlagsTheAttachmentWhenTheBrowserOwnsTheSubsizes(): void {
+		$stored = [];
+
+		Functions\when( 'set_transient' )->alias(
+			function ( $name, $value ) use ( &$stored ) {
+				$stored[ $name ] = $value;
+				return true;
+			}
+		);
+
+		( new WP() )->maybe_flag_client_side_scaling( (object) [ 'ID' => 123 ], $this->requestReturning( false ), true );
+
+		$this->assertSame( [ 'imagify_client_side_scaled_123' => 1 ], $stored );
+	}
+
+	/**
+	 * Test: nothing is flagged for an ordinary upload, where WordPress builds the sub sizes,
+	 * nor when the parameter is absent entirely.
+	 */
+	public function testDoesNotFlagWhenWordPressBuildsTheSubsizes(): void {
+		Functions\expect( 'set_transient' )->never();
+
+		$attachment = (object) [ 'ID' => 123 ];
+
+		( new WP() )->maybe_flag_client_side_scaling( $attachment, $this->requestReturning( true ), true );
+		( new WP() )->maybe_flag_client_side_scaling( $attachment, $this->requestReturning( null ), true );
+	}
+
+	/**
+	 * Test: nothing is flagged when an existing attachment is updated rather than created.
+	 */
+	public function testDoesNotFlagWhenUpdatingAnAttachment(): void {
+		Functions\expect( 'set_transient' )->never();
+
+		( new WP() )->maybe_flag_client_side_scaling( (object) [ 'ID' => 123 ], $this->requestReturning( false ), false );
+	}
+}

Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php

@@ -0,0 +1,107 @@
+<?php
+declare(strict_types=1);
+
+namespace Imagify\Tests\Unit\classes\Optimization\Process\WP;
+
+use Brain\Monkey\Functions;
+use Imagify\Optimization\Process\WP;
+use Imagify\Tests\Unit\TestCase;
+use Mockery;
+
+/**
+ * Tests for \Imagify\Optimization\Process\WP::can_resize() — when the browser handled the upload
+ * it produced the scaled version itself, using the threshold Imagify configured, so resizing on
+ * the server would only shrink the untouched original WordPress keeps aside as `original_image`.
+ *
+ * Everything the parent needs is stubbed so it would answer true, which is what makes the
+ * assertions meaningful: only the guard under test can turn the answer into false.
+ *
+ * @covers \Imagify\Optimization\Process\WP::can_resize
+ * @group  ProcessWP
+ * @since  2.3.3
+ */
+class CanResizeTest extends TestCase {
+
+	/**
+	 * Invoke the protected method on a process whose media reports the given ID, with every
+	 * condition the parent checks satisfied.
+	 *
+	 * @param  int|null $media_id Media ID, or null for no media at all.
+	 * @return bool
+	 */
+	private function canResize( $media_id ): bool {
+		$media = false;
+
+		if ( null !== $media_id ) {
+			$context = Mockery::mock( 'Imagify\Context\ContextInterface' );
+			$context->shouldReceive( 'can_resize' )->andReturn( true );
+
+			$media = Mockery::mock( 'Imagify\Media\MediaInterface' );
+			$media->shouldReceive( 'get_id' )->andReturn( $media_id );
+			$media->shouldReceive( 'get_context_instance' )->andReturn( $context );
+		}
+
+		$file = Mockery::mock( 'Imagify\Optimization\File' );
+		$file->shouldReceive( 'is_image' )->andReturn( true );
+
+		/*
+		 * A partial mock leaves protected methods alone, so the real can_resize() runs while
+		 * get_media() is stubbed. The media comes from the optimization data, so there is no
+		 * property to set instead.
+		 */
+		$process = Mockery::mock( WP::class )->makePartial();
+		$process->shouldReceive( 'get_media' )->andReturn( $media );
+		// is_valid() is get_media() && get_media()->is_valid(), so it cannot be true without a media.
+		$process->shouldReceive( 'is_valid' )->andReturn( null !== $media_id );
+
+		$method = new \ReflectionMethod( get_class( $process ), 'can_resize' );
+		$method->setAccessible( true );
+
+		return $method->invoke( $process, 'full', $file );
+	}
+
+	/**
+	 * Test: a media the browser already scaled is not resized again. Without the guard this
+	 * returns true, since every condition the parent checks is satisfied.
+	 */
+	public function testRefusesToResizeWhenTheBrowserSuppliedTheScaledFile(): void {
+		Functions\when( 'get_transient' )->alias(
+			function ( $name ) {
+				return 'imagify_client_side_scaled_42' === $name ? 1 : false;
+			}
+		);
+
+		$this->assertFalse( $this->canResize( 42 ) );
+	}
+
+	/**
+	 * Test: an ordinary media is still resized, so the guard does not block everything.
+	 */
+	public function testStillResizesWhenTheBrowserDidNotScaleTheFile(): void {
+		Functions\when( 'get_transient' )->justReturn( false );
+
+		$this->assertTrue( $this->canResize( 42 ) );
+	}
+
+	/**
+	 * Test: the decision is per attachment, so another media is not caught by the flag.
+	 */
+	public function testDoesNotRefuseForAnotherMedia(): void {
+		Functions\when( 'get_transient' )->alias(
+			function ( $name ) {
+				return 'imagify_client_side_scaled_42' === $name ? 1 : false;
+			}
+		);
+
+		$this->assertTrue( $this->canResize( 99 ) );
+	}
+
+	/**
+	 * Test: a process without a media does not blow up looking for an ID.
+	 */
+	public function testHandlesAProcessWithoutMedia(): void {
+		Functions\when( 'get_transient' )->justReturn( false );
+
+		$this->assertFalse( $this->canResize( null ) );
+	}
+}

Tests/Unit/classes/Tools/InternalStateList/sharedList.php

@@ -68,6 +68,8 @@ public function testGetLockedTransientPatternsReturnsExpectedArray(): void {
 			'_transient_%imagify_rpc_%',
 			'_transient_imagify_%_process_locked',
 			'_site_transient_imagify_%_process_lock%',
+			'_transient_imagify_client_side_scaled_%',
+			'_transient_timeout_imagify_client_side_scaled_%',
 		];
 
 		$this->assertSame( $expected, InternalStateList::get_locked_transient_patterns() );

Tests/Unit/classes/Tools/ResetInternalState/reset.php

@@ -4,6 +4,7 @@
 namespace Imagify\Tests\Unit\classes\Tools\ResetInternalState;
 
 use Imagify\Tests\Unit\TestCase;
+use Imagify\Tools\InternalStateList;
 use Imagify\Tools\ResetInternalState;
 use Mockery;
 use Brain\Monkey\Functions;
@@ -161,7 +162,7 @@ function ( string $sql, string $pattern ) use ( &$patterns_queried ) {
 				}
 			);
 
-		$this->wpdb->shouldReceive( 'query' )->times( 4 )->andReturn( 0 );
+		$this->wpdb->shouldReceive( 'query' )->times( count( InternalStateList::get_locked_transient_patterns() ) )->andReturn( 0 );
 
 		( new ResetInternalState() )->reset();
 
@@ -171,6 +172,8 @@ function ( string $sql, string $pattern ) use ( &$patterns_queried ) {
 			'\_transient\_%imagify\_rpc\_%',
 			'\_transient\_imagify\_%\_process\_locked',
 			'\_site\_transient\_imagify\_%\_process\_lock%',
+			'\_transient\_imagify\_client\_side\_scaled\_%',
+			'\_transient\_timeout\_imagify\_client\_side\_scaled\_%',
 		];
 
 		foreach ( $expected_patterns as $pattern ) {
@@ -263,7 +266,7 @@ function () use ( &$query_calls ) {
 
 		( new ResetInternalState() )->reset();
 
-		// 4 options-pattern queries prove reset() ran to completion.
-		$this->assertSame( 4, $query_calls );
+		// One options-pattern query per registered pattern proves reset() ran to completion.
+		$this->assertSame( count( InternalStateList::get_locked_transient_patterns() ), $query_calls );
 	}
 }