diff --git a/src/Model/Model_Font_Downloader.php b/src/Model/Model_Font_Downloader.php index 4039902fb..2f5933d15 100644 --- a/src/Model/Model_Font_Downloader.php +++ b/src/Model/Model_Font_Downloader.php @@ -164,9 +164,10 @@ class Model_Font_Downloader extends Helper_Abstract_Model { /** * When each failed file's retry is due (else 0), once the retry also failed when it gave up (else 0), and whether - * an admin dismissed the notice listing it + * an admin dismissed the notice listing it. `conflict` is the size and modified time of another font's file at its + * name when it failed, else ''. * - * @var array|null + * @var array|null * @since 6.19.0 */ protected $failures; @@ -397,6 +398,8 @@ public function download( string $file, bool $verify = false ): bool { /* A file at the name of a known size can still be another font's, which only its SHA-1 tells */ if ( $own_copy && $verify && $own_path === $font_path && $this->is_auto_install_enabled( $file ) ) { $own_copy = ! $this->is_unknown_build( $file ); + } elseif ( $own_copy && $this->is_failed_conflict( $file, $own_path ) ) { + $own_copy = false; } /* Copied in by hand or saved by another request, so any failure on record is stale */ @@ -477,6 +480,40 @@ protected function is_unknown_build( string $file ): bool { return $sha !== '' && ! in_array( $sha, $this->font_sources->get_known_builds( $file )[ $size ] ?? [], true ); } + /** + * Whether the font's own copy is on disk, as is_installed() but without the file a failed download found was another + * font's (see is_failed_conflict()) + * + * @since 6.19.0 + */ + protected function has_own_copy( string $file ): bool { + $own_path = $this->font_sources->get_own_copy( $file ); + + return $own_path !== '' && ! $this->is_failed_conflict( $file, $own_path ); + } + + /** + * Whether the own copy get_own_copy() found by size is the other font's file a failed download found at the name, + * unchanged since. Saves hashing it again on a render. + * + * @since 6.19.0 + */ + protected function is_failed_conflict( string $file, string $own_path ): bool { + $conflict = $this->get_failures()[ $file ]['conflict'] ?? ''; + + /* A hashed copy is the font's own whatever is at the name */ + return $conflict !== '' && $own_path === $this->font_sources->get_plain_path( $file ) && $conflict === $this->get_file_stamp( $own_path ); + } + + /** + * A file's size and modified time, or '' if it isn't on disk + * + * @since 6.19.0 + */ + protected function get_file_stamp( string $path ): string { + return is_file( $path ) ? filesize( $path ) . ':' . filemtime( $path ) : ''; + } + /** * Hash a font on disk, separate so tests can count the calls * @@ -1178,7 +1215,7 @@ public function get_wanted_files(): array { * @since 6.19.0 */ public function download_or_queue( string $file ): bool { - if ( ! $this->font_sources->is_large_file( $file ) || $this->font_sources->is_installed( $file ) ) { + if ( ! $this->font_sources->is_large_file( $file ) || $this->has_own_copy( $file ) ) { return $this->download( $file ); } @@ -1679,6 +1716,9 @@ protected function is_backing_off( string $file ): bool { protected function record_failure( string $file ): void { set_site_transient( self::back_off_key( $file ), time(), self::BACK_OFF ); + /* A failed download leaves the font's name as it was, so a file there is another font's */ + $conflict = $this->get_file_stamp( $this->font_sources->get_plain_path( $file ) ); + $failures = $this->get_failures(); $failure = $failures[ $file ] ?? null; @@ -1687,6 +1727,7 @@ protected function record_failure( string $file ): void { 'retry_at' => time() + wp_rand( 0, $this->retry_jitter ), 'gave_up' => 0, 'dismissed' => false, + 'conflict' => $conflict, ]; $this->save_failures( $failures ); @@ -1705,6 +1746,7 @@ protected function record_failure( string $file ): void { 'retry_at' => 0, 'gave_up' => time(), 'dismissed' => false, + 'conflict' => $conflict, ]; $this->save_failures( $failures ); @@ -1736,7 +1778,7 @@ protected function delete_back_off( string $file ): void { } /** - * @param array $failures + * @param array $failures * * @since 6.19.0 */ @@ -1749,7 +1791,7 @@ protected function save_failures( array $failures ): void { /** * Failures by file, read once per request, when any give-up whose file is now on disk is cleared from the record * - * @return array + * @return array * * @since 6.19.0 */ @@ -1768,6 +1810,7 @@ protected function get_failures(): array { 'retry_at' => $gave_up > 0 ? 0 : $retry_at, 'gave_up' => $gave_up, 'dismissed' => ( $failure['dismissed'] ?? false ) === true, + 'conflict' => is_string( $failure['conflict'] ?? null ) ? $failure['conflict'] : '', ]; } } @@ -1785,7 +1828,7 @@ protected function get_failures(): array { */ protected function clear_installed_give_ups(): void { foreach ( $this->failures as $file => $failure ) { - if ( $failure['gave_up'] > 0 && $this->font_sources->is_installed( $file ) ) { + if ( $failure['gave_up'] > 0 && $this->has_own_copy( $file ) ) { $this->clear_failure( $file ); } } diff --git a/tests/phpunit/Concerns/RecordsFontFailures.php b/tests/phpunit/Concerns/RecordsFontFailures.php index de8d228d5..4454c1d40 100644 --- a/tests/phpunit/Concerns/RecordsFontFailures.php +++ b/tests/phpunit/Concerns/RecordsFontFailures.php @@ -19,13 +19,14 @@ trait RecordsFontFailures { * @param int $gave_up When its retry also failed, or 0 while the retry is pending * @param int|null $retry_at When the pending retry is due, defaulting to now * - * @return array{retry_at: int, gave_up: int, dismissed: bool} + * @return array{retry_at: int, gave_up: int, dismissed: bool, conflict: string} */ protected function failure( int $gave_up = 0, ?int $retry_at = null ): array { return [ 'retry_at' => $gave_up > 0 ? 0 : ( $retry_at ?? time() ), 'gave_up' => $gave_up, 'dismissed' => false, + 'conflict' => '', ]; } diff --git a/tests/phpunit/integration/Model/Test_Model_Font_Downloader.php b/tests/phpunit/integration/Model/Test_Model_Font_Downloader.php index 13c3f9bbd..4a8c2b0ed 100644 --- a/tests/phpunit/integration/Model/Test_Model_Font_Downloader.php +++ b/tests/phpunit/integration/Model/Test_Model_Font_Downloader.php @@ -596,6 +596,52 @@ public function test_a_retry_for_a_file_beside_another_font_is_not_cleared() { $this->assertEmpty( $this->get_font_failures() ); } + public function test_a_same_size_font_found_when_verifying_keeps_its_failure_until_it_gives_up() { + $this->put_same_size_font_at( self::FONT, self::FONT ); + $this->fail_font_downloads(); + + $this->assertFalse( $this->new_downloader()->download( self::FONT, true ) ); + $this->assertNotSame( '', $this->get_font_failures()[ self::FONT ]['conflict'] ); + + /* A render goes by size, which the other font's file has */ + $this->assertFalse( $this->new_downloader()->download( self::FONT ) ); + $this->assertSame( 0, $this->get_font_failures()[ self::FONT ]['gave_up'] ); + + $this->delete_back_off( self::FONT ); + $this->assertFalse( $this->new_downloader()->download( self::FONT, true ) ); + + $this->assertSame( [ self::FONT ], $this->new_downloader()->get_undismissed_failed_files() ); + } + + public function test_a_render_queues_a_large_font_whose_name_a_failed_download_found_another_font_at() { + $this->put_same_size_font_at( self::FONT, self::FONT ); + $this->fail_font_downloads(); + $this->new_downloader()->download( self::FONT, true ); + $this->delete_back_off( self::FONT ); + + $manifest = GPDFAPI::get_mvc_class( 'Model_Font_Sources' )->get_manifest(); + $manifest[ self::FONT ]['size'] = Model_Font_Sources::LARGE_FILE_SIZE + 1; + $this->set_shared_manifest( $manifest ); + + $downloader = $this->new_downloader(); + $this->assertFalse( $downloader->download_or_queue( self::FONT ) ); + $this->assertCount( 1, $this->requested, 'The render leaves the download to the sync' ); + $this->assertSame( [ self::FONT ], $downloader->get_wanted_files() ); + } + + public function test_a_font_copied_over_another_fonts_file_clears_its_failure() { + $this->put_same_size_font_at( self::FONT, self::FONT ); + $this->fail_font_downloads(); + $this->new_downloader()->download( self::FONT, true ); + + $this->copy_fixture( self::FONT ); + touch( $this->font_path( self::FONT ), time() + 1 ); + + $this->assertTrue( $this->new_downloader()->download( self::FONT ) ); + $this->assertEmpty( $this->get_font_failures() ); + $this->assertFalse( $this->get_back_off( self::FONT ) ); + } + public function test_prune_never_removes_a_hashed_copy_or_a_replaced_file() { $sources = GPDFAPI::get_mvc_class( 'Model_Font_Sources' ); $hashed = $sources->get_hashed_name( self::FONT );