-
Notifications
You must be signed in to change notification settings - Fork 633
Fix uncaught PDOException in manual lookup when sqlite is unavailable #1969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace { | ||
| // include/manual-lookup.inc defines global functions and depends on the global | ||
| // get_manual_search_sections() (normally from include/site.inc). Provide the same | ||
| // section list here so the file can be exercised without pulling in all of site.inc. | ||
| if (!function_exists('get_manual_search_sections')) { | ||
| function get_manual_search_sections(): array | ||
| { | ||
| return ['', 'book.', 'ref.', 'function.', 'class.', 'enum.', 'features.', 'control-structures.', 'language.', 'about.', 'faq.']; | ||
| } | ||
| } | ||
|
|
||
| require_once __DIR__ . '/../../../include/manual-lookup.inc'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are now specific functions to get the right path, see (for example): 26b50b7#diff-e735bce93cbdaeabecf58805fc81589f114ea0a640e756195967983a91812fa6L5
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also saying that because the paths have now changed for some of the |
||
| } | ||
|
|
||
| namespace phpweb\Test\Unit\ManualLookup { | ||
|
|
||
| use PHPUnit\Framework; | ||
|
|
||
| #[Framework\Attributes\CoversFunction('find_manual_page')] | ||
| #[Framework\Attributes\CoversFunction('find_manual_page_slow')] | ||
| final class FindManualPageTest extends Framework\TestCase | ||
| { | ||
| private string $root; | ||
|
|
||
| private ?string $originalDocumentRoot; | ||
|
|
||
| protected function setUp(): void | ||
| { | ||
| $this->root = sys_get_temp_dir() . '/phpweb-ml-' . uniqid('', true); | ||
| mkdir($this->root . '/backend', 0777, true); | ||
| mkdir($this->root . '/manual/en', 0777, true); | ||
| // Filesystem (slow-path) target for the keyword "echo". | ||
| file_put_contents($this->root . '/manual/en/function.echo.php', '<?php'); | ||
|
|
||
| $this->originalDocumentRoot = $_SERVER['DOCUMENT_ROOT'] ?? null; | ||
| $_SERVER['DOCUMENT_ROOT'] = $this->root; | ||
| } | ||
|
|
||
| protected function tearDown(): void | ||
| { | ||
| if ($this->originalDocumentRoot === null) { | ||
| unset($_SERVER['DOCUMENT_ROOT']); | ||
| } else { | ||
| $_SERVER['DOCUMENT_ROOT'] = $this->originalDocumentRoot; | ||
| } | ||
|
|
||
| array_map('unlink', glob($this->root . '/backend/*') ?: []); | ||
| array_map('unlink', glob($this->root . '/manual/en/*') ?: []); | ||
| @rmdir($this->root . '/backend'); | ||
| @rmdir($this->root . '/manual/en'); | ||
| @rmdir($this->root . '/manual'); | ||
| @rmdir($this->root); | ||
| } | ||
|
|
||
| /** | ||
| * Regression test for the production fatal: | ||
| * Uncaught PDOException: SQLSTATE[HY000]: General error: 8 | ||
| * attempt to write a readonly database in include/manual-lookup.inc | ||
| * | ||
| * When the sqlite fast-path fails for ANY reason (a read-only/locked database, | ||
| * or a corrupt/truncated one from an interrupted rsync), find_manual_page() must | ||
| * fall back to the filesystem search instead of throwing an uncaught exception. | ||
| */ | ||
| public function testFallsBackToSlowSearchWhenSqliteQueryFails(): void | ||
| { | ||
| file_put_contents($this->root . '/backend/manual-lookup.sqlite', 'this is not a sqlite database'); | ||
|
|
||
| $result = find_manual_page('en', 'echo'); | ||
|
|
||
| self::assertSame('/manual/en/function.echo.php', $result); | ||
| } | ||
|
|
||
| public function testFallsBackToSlowSearchForDottedKeywordWhenSqliteQueryFails(): void | ||
| { | ||
| // A dotted keyword takes the other SQL branch in find_manual_page(); it must | ||
| // fall back to the filesystem search on a broken database too. | ||
| file_put_contents($this->root . '/backend/manual-lookup.sqlite', 'this is not a sqlite database'); | ||
|
|
||
| $result = find_manual_page('en', 'function.echo'); | ||
|
|
||
| self::assertSame('/manual/en/function.echo.php', $result); | ||
| } | ||
|
|
||
| #[Framework\Attributes\RequiresPhpExtension('pdo_sqlite')] | ||
| public function testUsesSqliteFastPathWhenDatabaseIsValid(): void | ||
| { | ||
| $this->buildValidDatabase(); | ||
|
|
||
| $result = find_manual_page('en', 'function.echo'); | ||
|
|
||
| self::assertSame('/manual/en/function.echo.php', $result); | ||
| } | ||
|
|
||
| public function testFallsBackToSlowSearchWhenNoDatabasePresent(): void | ||
| { | ||
| // No backend/manual-lookup.sqlite at all -> slow (filesystem) search only. | ||
| $result = find_manual_page('en', 'echo'); | ||
|
|
||
| self::assertSame('/manual/en/function.echo.php', $result); | ||
| } | ||
|
|
||
| private function buildValidDatabase(): void | ||
| { | ||
| $dbh = new \PDO('sqlite:' . $this->root . '/backend/manual-lookup.sqlite'); | ||
| $dbh->exec('CREATE TABLE fs (lang TEXT, prefix TEXT, keyword TEXT, name TEXT, prio INT)'); | ||
| $dbh->exec("INSERT INTO fs (lang, prefix, keyword, name, prio) VALUES ('en', 'function.', 'echo', '/manual/en/function.echo.php', 3)"); | ||
| $dbh = null; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raise my suspicions that you wrote this yourself — an
sin optimisation?