diff --git a/src/Model/Model_Font_Downloader.php b/src/Model/Model_Font_Downloader.php index 4039902fb..a1308c9e8 100644 --- a/src/Model/Model_Font_Downloader.php +++ b/src/Model/Model_Font_Downloader.php @@ -802,8 +802,8 @@ protected function get_size_fields( array $files, string $dir ): array { /** * Delete an unused downloaded family's own copies (see Model_Font_Sources::get_own_copy()), keeping the licence files - * and any other font's file at a family file's name. A file at the name another site's custom font uses is kept too, - * as every site shares the fonts folder. + * and any other font's file at a family file's name (a file of a known size is hashed to tell). A file at the name + * another site's custom font uses is kept too, as every site shares the fonts folder. * * @return array{id: string, removed: string[], bytes: int, font: array|null}|WP_Error `font` is its row now, if it can * be installed again. 404 when @@ -813,6 +813,15 @@ protected function get_size_fields( array $files, string $dir ): array { */ public function delete_downloaded_font( string $key ) { $family = $this->get_core_families()[ $key ] ?? null; + + /* Before the sync saves the font under its hashed name, another font's file of a known size passes for its own */ + foreach ( $family['paths'] ?? [] as $file => $path ) { + if ( $path === $this->font_sources->get_plain_path( $file ) && $this->is_unknown_build( $file ) ) { + unset( $family['paths'][ $file ] ); + $this->log->notice( 'Kept another font\'s file at a font file\'s name', [ 'name' => $file ] ); + } + } + if ( $family === null || count( $family['paths'] ) === 0 ) { return new WP_Error( 'font_not_downloaded', __( 'The font is not downloaded.', 'gravity-pdf' ), [ 'status' => 404 ] ); } diff --git a/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php b/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php index 3e49ed9db..840ef3240 100644 --- a/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php +++ b/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php @@ -381,6 +381,15 @@ public function test_delete_removes_only_the_hashed_copy_and_never_the_file_at_t $this->assertFileEquals( $this->font_fixture( 'Chewy.ttf' ), $this->font_path( 'DejaVuSerifCondensed.ttf' ) ); } + public function test_delete_keeps_another_fonts_file_of_a_known_size_at_the_name() { + $this->put_same_size_font_at( 'DejaVuSerifCondensed.ttf', 'DejaVuSerifCondensed.ttf' ); + $bytes = (string) file_get_contents( $this->font_path( 'DejaVuSerifCondensed.ttf' ) ); + + $this->assert_font_error( $this->downloader()->delete_downloaded_font( 'dejavuserifcondensed' ), 'font_not_downloaded', 404 ); + $this->assertStringEqualsFile( $this->font_path( 'DejaVuSerifCondensed.ttf' ), $bytes ); + $this->assertCount( 1, $this->log_messages( 'Kept another font\'s file at a font file\'s name' ) ); + } + public function test_a_family_sharing_a_name_with_a_record_lists_and_deletes_only_its_hashed_copy() { $sources = GPDFAPI::get_mvc_class( 'Model_Font_Sources' ); $hashed = $sources->get_hashed_name( 'DejaVuSerifCondensed.ttf' ); diff --git a/tests/playwright/core/managers/font-manager.spec.ts b/tests/playwright/core/managers/font-manager.spec.ts index 7b70de063..6c9f60cfa 100644 --- a/tests/playwright/core/managers/font-manager.spec.ts +++ b/tests/playwright/core/managers/font-manager.spec.ts @@ -4,7 +4,7 @@ import type { Page } from '@playwright/test'; import { test, resourcesPath } from '@self:playwright/fixtures/test'; import Pdf, { type FontUpload } from '@self:playwright/utils/gravitypdf'; import { wpCli } from '@self:playwright/utils/wp-cli'; -import { coreFontStandIn } from '@self:playwright/utils/core-fonts'; +import { coreFontStandIn, hashedName } from '@self:playwright/utils/core-fonts'; import * as fs from 'fs'; import * as path from 'path'; @@ -379,7 +379,7 @@ test.describe('Font Manager', () => { // Seeds a core font no other spec uses const seedCoreFont = (...files: string[]) => { - seededFiles.push(...files); + seededFiles.push(...files.map(hashedName)); wpCli( ...files.map((file) => coreFontStandIn( diff --git a/tests/playwright/multisite/site-fonts.spec.ts b/tests/playwright/multisite/site-fonts.spec.ts index 21238e7ce..7a56961cb 100644 --- a/tests/playwright/multisite/site-fonts.spec.ts +++ b/tests/playwright/multisite/site-fonts.spec.ts @@ -6,7 +6,7 @@ import { logInToSubsite as logIn, } from '@self:playwright/utils/multisite'; import { wpCli } from '@self:playwright/utils/wp-cli'; -import { coreFontStandIn } from '@self:playwright/utils/core-fonts'; +import { coreFontStandIn, hashedName } from '@self:playwright/utils/core-fonts'; import * as fs from 'fs'; import * as path from 'path'; @@ -17,6 +17,10 @@ const FILE = 'NetworkRoboto.ttf'; const CORE_NAMED_FONT = 'Core Named Roboto'; const CORE_FILE = 'DejaVuSerifCondensed.ttf'; const CORE_BOLD_FILE = 'DejaVuSerifCondensed-Bold.ttf'; +const CORE_BOLD_COPY = hashedName(CORE_BOLD_FILE); +// The manifest's own build, which the plugin hashes before deleting it from the core font's name +const CORE_FILE_IN_CONTAINER = + 'wp-content/plugins/gravity-pdf/tools/phpunit/data/fonts/DejaVuSerifCondensed.ttf'; const ROBOTO_IN_CONTAINER = 'wp-content/plugins/gravity-pdf/tools/playwright/data/fonts/Roboto-Regular.ttf'; @@ -83,7 +87,7 @@ test.describe('Fonts of a Multisite subsite', () => { } await admin.context().close(); - wpCli(`rm -f ${FONTS}/${CORE_FILE} ${FONTS}/${CORE_BOLD_FILE}`); + wpCli(`rm -f ${FONTS}/${CORE_FILE} ${FONTS}/${CORE_BOLD_COPY}`); }; test.beforeAll(deleteFonts); @@ -207,7 +211,7 @@ test.describe('Fonts of a Multisite subsite', () => { (row: { id: string }) => row.id === 'dejavuserifcondensed' ); expect(core.regular).toBe(''); - expect(core.bold).toBe(CORE_BOLD_FILE); + expect(core.bold).toBe(CORE_BOLD_COPY); const deleted = await send( page, @@ -222,7 +226,7 @@ test.describe('Fonts of a Multisite subsite', () => { const files = fontFileList(); expect(files).toContain(CORE_FILE); - expect(files).not.toContain(CORE_BOLD_FILE); + expect(files).not.toContain(CORE_BOLD_COPY); // Throws if the subsite's font no longer has its own bytes wpCli(`cmp ${ROBOTO_IN_CONTAINER} ${FONTS}/${CORE_FILE}`); @@ -249,7 +253,7 @@ test.describe('Fonts of a Multisite subsite', () => { await subsite.context().close(); // The custom font's delete removed its file, so put one back at the core font's name - wpCli(coreFontStandIn(ROBOTO_IN_CONTAINER, `${FONTS}/${CORE_FILE}`)); + wpCli(`cp ${CORE_FILE_IN_CONTAINER} ${FONTS}/${CORE_FILE}`); await page.goto('/wp-admin/index.php'); const deleted = await send( diff --git a/tools/playwright/utils/core-fonts.ts b/tools/playwright/utils/core-fonts.ts index 8f9062b8f..fb55d6159 100644 --- a/tools/playwright/utils/core-fonts.ts +++ b/tools/playwright/utils/core-fonts.ts @@ -1,26 +1,45 @@ import * as fs from 'fs'; import * as path from 'path'; -const manifest: { name: string; size: number }[] = JSON.parse( +const manifest: { name: string; size: number; sha: string }[] = JSON.parse( fs.readFileSync( path.join(__dirname, '..', '..', '..', 'fonts', 'core-fonts.json'), 'utf8' ) ); +const entryFor = (file: string) => { + const entry = manifest.find((row) => row.name === file); + if (entry === undefined) { + throw new Error(`${file} is not on the core font manifest`); + } + + return entry; +}; + +/** + * The name a core font file takes when another file holds its own: `{stem}-{first 8 of its SHA-1}{ext}` + * + * @param file The core font file's name + */ +export function hashedName(file: string) { + const { ext, name } = path.parse(file); + + return `${name}-${entryFor(file).sha.slice(0, 8)}${ext}`; +} + /** - * A shell command putting a stand-in for a core font file at `dest`: a copy of `source` at the core file's size on the - * manifest, which the plugin takes as the core font's own copy (downloads are off on the E2E sites) + * A shell command putting a stand-in for a core font file under its hashed name (see hashedName()) beside `dest`: a + * copy of `source` at the core file's size on the manifest. The plugin takes a hashed copy of that size as the core + * font's own without hashing it, while one at the plain name is hashed before it's deleted (downloads are off on the + * E2E sites). * * @param source A font file path in the container * @param dest The core font file's path in the container */ export function coreFontStandIn(source: string, dest: string) { const file = path.basename(dest); - const size = manifest.find((entry) => entry.name === file)?.size; - if (size === undefined) { - throw new Error(`${file} is not on the core font manifest`); - } + const target = path.join(path.dirname(dest), hashedName(file)); - return `cp ${source} ${dest} && truncate -s ${size} ${dest}`; + return `cp ${source} ${target} && truncate -s ${entryFor(file).size} ${target}`; }