diff --git a/src/Controller/Controller_Custom_Fonts.php b/src/Controller/Controller_Custom_Fonts.php index 7943b3dcd..0aa997f0f 100644 --- a/src/Controller/Controller_Custom_Fonts.php +++ b/src/Controller/Controller_Custom_Fonts.php @@ -85,7 +85,7 @@ class Controller_Custom_Fonts extends Helper_Abstract_Controller { * @var string[] List of the standard font keys used when saving settings * @since 6.0 */ - protected $font_keys = [ 'regular', 'italics', 'bold', 'bolditalics' ]; + protected $font_keys = Model_Custom_Fonts::FONT_STYLES; public function __construct( Model_Custom_Fonts $model, LoggerInterface $log, Helper_Abstract_Form $gform, string $font_dir_path, string $filesystem = 'GFPDF_Vendor\\GravityPdf\\Upload\\Storage\\FileSystem', string $file = 'GFPDF_Vendor\\GravityPdf\\Upload\\File' ) { $this->model = $model; @@ -428,7 +428,7 @@ public function update_item( WP_REST_Request $request ) { } /* Not one an upload was just saved under, which happens when the old file had gone missing */ - array_map( [ $this, 'delete_font_file' ], array_diff( $unused_files, $uncommitted_files ) ); + $this->delete_unused_font_files( array_diff( $unused_files, $uncommitted_files ) ); $uncommitted_files = []; @@ -554,15 +554,16 @@ public function delete_item( WP_REST_Request $request ) { } /* Delete TTF files from disk */ - $font = $this->model->get_font_by_id( $id ); + $font = $this->model->get_font_by_id( $id ); + $files = []; foreach ( $this->font_keys as $font_id ) { - if ( empty( $font[ $font_id ] ) ) { - continue; + if ( ! empty( $font[ $font_id ] ) ) { + $files[] = basename( $font[ $font_id ] ); } - - $this->delete_font_file( basename( $font[ $font_id ] ) ); } + $this->delete_unused_font_files( $files ); + /* Update DB */ if ( ! $this->model->delete_font( $id ) ) { throw new GravityPdfDatabaseUpdateException(); @@ -739,6 +740,30 @@ protected function get_upload_error_message( File $file, string $error_code = Er return __( 'The upload is not a valid TTF file', 'gravity-pdf' ); } + /** + * Delete font files, keeping any another site's fonts use (the fonts folder is network-wide) + * + * @param string[] $files The filenames + * + * @since 6.19.0 + */ + protected function delete_unused_font_files( array $files ): void { + if ( $files === [] ) { + return; + } + + $shared = $this->model->get_files_used_by_other_sites(); + + foreach ( $files as $file ) { + if ( in_array( $file, $shared, true ) ) { + $this->log->debug( 'Kept a font file another site uses', [ 'file' => $file ] ); + continue; + } + + $this->delete_font_file( $file ); + } + } + /** * @param string $file The filename of the font to be deleted * diff --git a/src/Model/Model_Custom_Fonts.php b/src/Model/Model_Custom_Fonts.php index debe2b553..713004a7f 100644 --- a/src/Model/Model_Custom_Fonts.php +++ b/src/Model/Model_Custom_Fonts.php @@ -28,12 +28,24 @@ */ class Model_Custom_Fonts extends Helper_Abstract_Model { + /** + * @var string[] The keys a font record stores its files under + * @since 6.19.0 + */ + public const FONT_STYLES = [ 'regular', 'italics', 'bold', 'bolditalics' ]; + /** * @var Helper_Abstract_Options * @since 6.0 */ protected $options; + /** + * @var array The files other sites' fonts use, keyed by the current site ID + * @since 6.19.0 + */ + protected $files_used_by_other_sites = []; + public function __construct( Helper_Abstract_Options $options ) { $this->options = $options; } @@ -158,6 +170,70 @@ public function delete_font( string $id ): bool { return $this->options->update_option( 'custom_fonts', $fonts ); } + /** + * The filenames the other sites' fonts use, read from each site's options table to avoid switch_to_blog() + * + * @return string[] + * + * @since 6.19.0 + */ + public function get_files_used_by_other_sites(): array { + global $wpdb; + + if ( ! is_multisite() ) { + return []; + } + + $current_site = get_current_blog_id(); + if ( isset( $this->files_used_by_other_sites[ $current_site ] ) ) { + return $this->files_used_by_other_sites[ $current_site ]; + } + + $site_ids = get_sites( + [ + 'fields' => 'ids', + 'number' => 0, + 'site__not_in' => [ $current_site ], + ] + ); + + $files = []; + + foreach ( $site_ids as $site_id ) { + $prefix = $wpdb->get_blog_prefix( $site_id ); + $settings = maybe_unserialize( $wpdb->get_var( "SELECT option_value FROM {$prefix}options WHERE option_name = 'gfpdf_settings' LIMIT 1" ) ); //phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared + + if ( ! is_array( $settings ) || ! is_array( $settings['custom_fonts'] ?? null ) ) { + continue; + } + + foreach ( $settings['custom_fonts'] as $font ) { + if ( ! is_array( $font ) ) { + continue; + } + + foreach ( self::FONT_STYLES as $style ) { + if ( is_string( $font[ $style ] ?? null ) && $font[ $style ] !== '' ) { + $files[] = basename( $font[ $style ] ); + } + } + } + } + + $this->files_used_by_other_sites[ $current_site ] = array_values( array_unique( $files ) ); + + return $this->files_used_by_other_sites[ $current_site ]; + } + + /** + * Forget the other sites' font files read this request + * + * @since 6.19.0 + */ + public function flush(): void { + $this->files_used_by_other_sites = []; + } + /** * Checks if the ID already exists and, if so, suffixes the ID with a string until unique * diff --git a/src/Model/Model_Font_Sources.php b/src/Model/Model_Font_Sources.php index d8d4c01f6..624852743 100644 --- a/src/Model/Model_Font_Sources.php +++ b/src/Model/Model_Font_Sources.php @@ -4,7 +4,6 @@ namespace GFPDF\Model; -use GFPDF\Helper\Fonts\FlushCache; use GFPDF\Helper\Helper_Abstract_Model; use GFPDF\Helper\Helper_Data; use GFPDF\Statics\Git_Blob; @@ -668,7 +667,7 @@ protected function get_record_file_names(): array { $names = []; foreach ( $records as $record ) { - foreach ( [ 'regular', 'italics', 'bold', 'bolditalics' ] as $style ) { + foreach ( Model_Custom_Fonts::FONT_STYLES as $style ) { $file = is_string( $record[ $style ] ?? null ) ? basename( $record[ $style ] ) : ''; if ( isset( $family_files[ $file ] ) ) { $names[ $file ] = $file; @@ -1032,39 +1031,30 @@ public function adopt_records( string $source_id ): array { } /** - * Remove a custom font record, and outside Multisite its files too: another site's record may use the shared fonts - * folder. Logs and returns false on failure + * Remove a custom font record and the files no other site uses. Logs and returns false on failure * * @since 6.19.0 */ protected function remove_record( string $id ): bool { - $removed = false; - $error = ''; + $error = ''; try { - if ( ! is_multisite() ) { - $removed = GPDFAPI::delete_pdf_font( $id ) === true; - } elseif ( $this->custom_fonts->delete_font( $id ) ) { - FlushCache::flush( $id ); - $removed = true; + if ( GPDFAPI::delete_pdf_font( $id ) === true ) { + return true; } } catch ( Throwable $e ) { $error = $e->getMessage(); } - if ( ! $removed ) { - $this->log->error( - 'Could not remove a custom font record replaced by a font source', - [ - 'font' => $id, - 'exception' => $error, - ] - ); - - return false; - } + $this->log->error( + 'Could not remove a custom font record replaced by a font source', + [ + 'font' => $id, + 'exception' => $error, + ] + ); - return true; + return false; } /** diff --git a/tests/phpunit/Concerns/CopiesFontFixtures.php b/tests/phpunit/Concerns/CopiesFontFixtures.php index 0a587afad..602e3bbdc 100644 --- a/tests/phpunit/Concerns/CopiesFontFixtures.php +++ b/tests/phpunit/Concerns/CopiesFontFixtures.php @@ -5,7 +5,6 @@ namespace GFPDF\Tests\Concerns; use GFPDF\Helper\Fonts\FlushCache; -use GFPDF\Model\Model_Custom_Fonts; use GFPDF\Model\Model_Font_Sources; use GFPDF\Statics\Git_Blob; use GPDFAPI; @@ -98,8 +97,8 @@ protected function add_record( string $id, array $files ): void { /** * A model whose bundled source accepts the fixture as a legacy Arimo copy */ - protected function get_model_with_legacy( string $fixture, ?Model_Custom_Fonts $custom_fonts = null ): Model_Font_Sources { - $model = new class( GPDFAPI::get_data_class(), $custom_fonts ?? GPDFAPI::get_mvc_class( 'Model_Custom_Fonts' ), GPDFAPI::get_log_class() ) extends Model_Font_Sources { + protected function get_model_with_legacy( string $fixture ): Model_Font_Sources { + $model = new class( GPDFAPI::get_data_class(), GPDFAPI::get_mvc_class( 'Model_Custom_Fonts' ), GPDFAPI::get_log_class() ) extends Model_Font_Sources { /** @var array */ public $legacy = []; @@ -294,6 +293,7 @@ protected function remove_font_fixtures(): void { array_map( [ FlushCache::class, 'flush' ], array_unique( $this->recorded_fonts ) ); GPDFAPI::get_mvc_class( 'Model_Font_Sources' )->flush(); GPDFAPI::get_mvc_class( 'Model_Required_Fonts' )->flush(); + GPDFAPI::get_mvc_class( 'Model_Custom_Fonts' )->flush(); /* A download or its fallback caches metrics under every font key the render asked for, substitutes included */ if ( $this->requested !== [] ) { diff --git a/tests/phpunit/integration/Controller/Test_Controller_Custom_Fonts.php b/tests/phpunit/integration/Controller/Test_Controller_Custom_Fonts.php index 1c80a1ca0..64e1d2002 100644 --- a/tests/phpunit/integration/Controller/Test_Controller_Custom_Fonts.php +++ b/tests/phpunit/integration/Controller/Test_Controller_Custom_Fonts.php @@ -883,6 +883,101 @@ public function test_delete_item_with_font_reference_gone_success() { $this->assertSame( 200, $response->get_status() ); } + /** + * @group ms-required + */ + public function test_deleting_a_font_keeps_the_files_another_site_uses() { + global $gfpdf; + + $this->skipWithoutMultisite(); + + [ $shared, $own, $other_site ] = $this->share_a_file_with_another_site(); + + $this->assertNull( $this->controller->delete_item( $this->delete_request( 'font' ) ) ); + + $this->assertSame( [], $this->model->get_custom_fonts() ); + $this->assertFileExists( $shared ); + $this->assertFileDoesNotExist( $own ); + $this->assertArrayHasKey( 'other', get_blog_option( $other_site, 'gfpdf_settings' )['custom_fonts'] ); + + /* The last site using the file removes it */ + switch_to_blog( $other_site ); + $gfpdf->options->set_plugin_settings(); + $response = $this->controller->delete_item( $this->delete_request( 'other' ) ); + restore_current_blog(); + $gfpdf->options->set_plugin_settings(); + + $this->assertNull( $response ); + $this->assertFileDoesNotExist( $shared ); + } + + /** + * @group ms-required + */ + public function test_update_item_keeps_a_replaced_file_another_site_uses() { + $this->skipWithoutMultisite(); + + [ $shared, $own, $other_site ] = $this->share_a_file_with_another_site(); + + $request = new WP_REST_Request( 'POST' ); + $request->set_body_params( + [ + 'id' => 'font', + 'bold' => '', + ] + ); + $this->set_font_files( $request, [ 'regular' => [ 'DejaVuSans.ttf', self::$test_fonts[0] ] ] ); + + $updated = $this->controller->update_item( $request ); + + $this->assertNotSame( $shared, $updated['regular'] ); + $this->assertSame( '', $updated['bold'] ); + $this->assertFileExists( $shared ); + $this->assertFileDoesNotExist( $own ); + } + + /** + * Save a font whose regular file another site's font also uses, and whose bold file is only its own + * + * @return array{0: string, 1: string, 2: int} The shared file, the font's own file and the other site's ID + */ + protected function share_a_file_with_another_site(): array { + $shared = $this->copy_font( 0, 'Shared.ttf' ); + $own = $this->copy_font( 1, 'Own.ttf' ); + $this->model->add_font( $this->font_record( 'font', $shared, $own ) ); + + $other_site = self::factory()->blog->create(); + update_blog_option( $other_site, 'gfpdf_settings', [ 'custom_fonts' => [ 'other' => $this->font_record( 'other', $shared ) ] ] ); + + return [ $shared, $own, $other_site ]; + } + + protected function copy_font( int $fixture, string $name ): string { + copy( self::$test_fonts[ $fixture ], $this->tmp_font_location . $name ); + + return $this->tmp_font_location . $name; + } + + protected function font_record( string $id, string $regular, string $bold = '' ): array { + return [ + 'id' => $id, + 'font_name' => $id, + 'regular' => $regular, + 'italics' => '', + 'bold' => $bold, + 'bolditalics' => '', + 'useOTL' => 0x00, + 'useKashida' => 0, + ]; + } + + protected function delete_request( string $id ): WP_REST_Request { + $request = new WP_REST_Request( 'DELETE' ); + $request->set_param( 'id', $id ); + + return $request; + } + public function test_delete_item_invalid_font_id() { wp_set_current_user( $this->admin_user ); diff --git a/tests/phpunit/integration/Model/Test_Model_Custom_Fonts.php b/tests/phpunit/integration/Model/Test_Model_Custom_Fonts.php index 8c6aa2e52..3504d7231 100644 --- a/tests/phpunit/integration/Model/Test_Model_Custom_Fonts.php +++ b/tests/phpunit/integration/Model/Test_Model_Custom_Fonts.php @@ -198,4 +198,64 @@ public function data_has_unique_font_id(): array { [ true, 'dejavusans1' ], ]; } + + /** + * @group ms-excluded + */ + public function test_a_single_site_reads_no_other_sites_fonts() { + global $wpdb; + + $this->skipWithMultisite(); + + $queries = $wpdb->num_queries; + + $this->assertSame( [], $this->model->get_files_used_by_other_sites() ); + $this->assertSame( $queries, $wpdb->num_queries ); + } + + /** + * @group ms-required + */ + public function test_get_files_used_by_other_sites() { + global $wpdb; + + $this->skipWithoutMultisite(); + + $this->model->add_font( + [ + 'id' => 'mine', + 'regular' => '/fonts/Mine.ttf', + ] + ); + + $fonts = [ + 'one' => [ + 'regular' => '/fonts/One.ttf', + 'bolditalics' => '/fonts/Shared.ttf', + ], + 'two' => [ + 'regular' => '', + 'italics' => '/fonts/Shared.ttf', + 'bold' => [ 'Array.ttf' ], + ], + 'corrupt' => 'font', + ]; + + update_blog_option( self::factory()->blog->create(), 'gfpdf_settings', [ 'custom_fonts' => $fonts ] ); + update_blog_option( self::factory()->blog->create(), 'gfpdf_settings', [ 'custom_fonts' => 'fonts' ] ); + update_blog_option( self::factory()->blog->create(), 'gfpdf_settings', 'settings' ); + self::factory()->blog->create(); + + /* A truncated serialized value */ + $truncated = self::factory()->blog->create(); + update_blog_option( $truncated, 'gfpdf_settings', [ 'custom_fonts' => [] ] ); + $wpdb->update( $wpdb->get_blog_prefix( $truncated ) . 'options', [ 'option_value' => 'a:1:{s:12:"custom_fonts";a:1:{' ], [ 'option_name' => 'gfpdf_settings' ] ); + + $this->assertSame( [ 'One.ttf', 'Shared.ttf' ], $this->model->get_files_used_by_other_sites() ); + + /* Read once a request */ + $queries = $wpdb->num_queries; + $this->model->get_files_used_by_other_sites(); + $this->assertSame( $queries, $wpdb->num_queries ); + } } diff --git a/tests/phpunit/integration/Model/Test_Model_Font_Sources.php b/tests/phpunit/integration/Model/Test_Model_Font_Sources.php index 8cac6882a..dd20ff3a2 100644 --- a/tests/phpunit/integration/Model/Test_Model_Font_Sources.php +++ b/tests/phpunit/integration/Model/Test_Model_Font_Sources.php @@ -4,7 +4,6 @@ namespace GFPDF\Model; -use GFPDF\Exceptions\GravityPdfIdException; use GFPDF\Tests\Concerns\CopiesFontFixtures; use GFPDF\Tests\Concerns\Seamed_Font_Downloader; use GFPDF\Tests\Concerns\UnregistersMvcClasses; @@ -427,10 +426,8 @@ public function test_adopt_records_removes_a_record_matching_the_manifest() { $this->assertSame( [ 'dejavusanscondensed' ], $this->model->adopt_records( 'core' ) ); $this->assertSame( [], $this->custom_fonts->get_custom_fonts() ); - - /* Multisite keeps the files in the shared fonts folder */ - $this->assertSame( is_multisite(), is_file( $this->font_dir . 'DejaVuSansCondensed85274.ttf' ) ); - $this->assertSame( is_multisite(), is_file( $this->font_dir . 'DejaVuSans-Bold.ttf' ) ); + $this->assertFileDoesNotExist( $this->font_dir . 'DejaVuSansCondensed85274.ttf' ); + $this->assertFileDoesNotExist( $this->font_dir . 'DejaVuSans-Bold.ttf' ); } public function test_adopt_records_removes_a_record_of_the_core_font_installers_files() { @@ -486,7 +483,7 @@ public function test_adopt_records_removes_a_record_matching_legacy_hashes() { $this->assertSame( [ 'arimo' ], $model->adopt_records( 'bundled' ) ); $this->assertSame( [], $this->custom_fonts->get_custom_fonts() ); - $this->assertSame( is_multisite(), is_file( $this->font_dir . 'Arimo-Regular85274.ttf' ) ); + $this->assertFileDoesNotExist( $this->font_dir . 'Arimo-Regular85274.ttf' ); } public function test_adopt_records_keeps_an_edited_record() { @@ -933,10 +930,6 @@ public function test_adopt_records_logs_a_record_it_could_not_remove() { } public function test_adopt_records_keeps_the_record_when_the_custom_fonts_controller_is_unhooked() { - if ( is_multisite() ) { - $this->markTestSkipped( 'Multisite removes the record through the model' ); - } - $model = $this->get_model_with_legacy( 'Chewy.ttf' ); $this->copy_fixture( 'Chewy.ttf', 'Arimo-Regular.ttf' ); $this->add_record( 'arimo', [ 'regular' => 'Arimo-Regular.ttf' ] ); @@ -952,22 +945,22 @@ function () use ( $model ) { $this->assertFileExists( $this->font_dir . 'Arimo-Regular.ttf' ); } - public function test_adopt_records_logs_a_record_removed_elsewhere_on_multisite() { - if ( ! is_multisite() ) { - $this->markTestSkipped( 'Single site removes the record through the API' ); - } - - $custom_fonts = new class( GPDFAPI::get_options_class() ) extends Model_Custom_Fonts { - public function delete_font( string $id ): bool { - throw new GravityPdfIdException(); - } - }; + /** + * @group ms-required + */ + public function test_adopt_records_keeps_a_file_another_site_uses() { + $this->skipWithoutMultisite(); - $model = $this->get_model_with_legacy( 'Chewy.ttf', $custom_fonts ); + $model = $this->get_model_with_legacy( 'Chewy.ttf' ); $this->copy_fixture( 'Chewy.ttf', 'Arimo-Regular.ttf' ); $this->add_record( 'arimo', [ 'regular' => 'Arimo-Regular.ttf' ] ); - $this->assert_record_kept( $model->adopt_records( 'bundled' ) ); + $other_site = self::factory()->blog->create(); + update_blog_option( $other_site, 'gfpdf_settings', [ 'custom_fonts' => $this->custom_fonts->get_custom_fonts() ] ); + + $this->assertSame( [ 'arimo' ], $model->adopt_records( 'bundled' ) ); + $this->assertSame( [], $this->custom_fonts->get_custom_fonts() ); + $this->assertFileExists( $this->font_dir . 'Arimo-Regular.ttf' ); } public function test_adopt_records_moves_on_after_an_error_removing_a_record() {