Skip to content

Commit

Permalink
[Blueprints] Skip empty lines in the runSql step (#1939)
Browse files Browse the repository at this point in the history
Skips the empty SQL lines in the runSql Blueprint step. Before this PR,
the step handler was very strict and would error out on the first empty
or semicolon-only line. While I was on it, I split the unit tests into
three distinct cases and simplified the SQL running logic.

This SQL file wouldn't work before this PR but it works after it:

```sql
SELECT * FROM wp_users;

;

SELECT * FROM wp_users;
```

 ## Testing instructions

CI

cc @akirk
  • Loading branch information
adamziel authored Oct 24, 2024
1 parent b25c7a4 commit b1c6d5a
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 50 deletions.
102 changes: 59 additions & 43 deletions packages/playground/blueprints/src/lib/steps/run-sql.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { PHP } from '@php-wasm/universal';
import { phpVars, randomFilename } from '@php-wasm/util';
import { phpVars } from '@php-wasm/util';
import { runSql } from './run-sql';
import { PHPRequestHandler } from '@php-wasm/universal';
import { loadNodeRuntime } from '@php-wasm/node';
Expand All @@ -8,24 +8,25 @@ const phpVersion = '8.0';
describe('Blueprint step runSql', () => {
let php: PHP;
let handler: PHPRequestHandler;
const documentRoot = '/wordpress';

const outputLogPath = `/tmp/sql-execution-log.json`;
beforeEach(async () => {
handler = new PHPRequestHandler({
phpFactory: async () => new PHP(await loadNodeRuntime(phpVersion)),
documentRoot: '/wordpress',
documentRoot,
});
php = await handler.getPrimaryPhp();
});

it('should split and "run" sql queries', async () => {
const docroot = '/wordpress';
const sqlFilename = `/tmp/${randomFilename()}.sql`;
const resFilename = `/tmp/${randomFilename()}.json`;
const js = phpVars({ docroot, sqlFilename, resFilename });
await php.mkdir(docroot);

php.mkdir(documentRoot);
// Create an object that will log all function calls
await php.writeFile(
`${docroot}/wp-load.php`,
const js = phpVars({ documentRoot, outputLogPath });
/**
* The run-sql step loads WordPress by including wp-load.php.
* We don't need the rest of WordPress for this test, so we
* create a minimal wp-load.php that just logs the sql queries.
*/
php.writeFile(
`${documentRoot}/wp-load.php`,
`<?php
class MockLogger
{
Expand All @@ -37,51 +38,66 @@ describe('Blueprint step runSql', () => {
'args' => $args,
];
file_put_contents(${js.resFilename}, json_encode($entry) . "\n", FILE_APPEND);
file_put_contents(${js.outputLogPath}, json_encode($entry) . "\n", FILE_APPEND);
}
}
global $wpdb;
$wpdb = new MockLogger();
file_put_contents(${js.resFilename}, '');
file_put_contents(${js.outputLogPath}, '');
`
);
});

it('should split and "run" sql queries', async () => {
// Test a single query
const mockFileSingle = new File(
['SELECT * FROM wp_users;'],
'single-query.sql'
);

await runSql(php, { sql: mockFileSingle });

const singleQueryResult = await php.readFileAsText(resFilename);
const singleQueryExpect = `{"type":"CALL","function":"query","args":["SELECT * FROM wp_users;"]}\n`;
expect(singleQueryResult).toBe(singleQueryExpect);
await runSql(php, {
sql: new File(['SELECT * FROM wp_users;'], 'single-query.sql'),
});

// Test a multiple queries
const mockFileMultiple = new File(
[`SELECT * FROM wp_users;\nSELECT * FROM wp_posts;\n`],
'multiple-queries.sql'
const result = php.readFileAsText(outputLogPath);
expect(result).toBe(
`{"type":"CALL","function":"query","args":["SELECT * FROM wp_users;"]}\n`
);
});

await runSql(php, { sql: mockFileMultiple });

const multiQueryResult = await php.readFileAsText(resFilename);
const multiQueryExpect = `{"type":"CALL","function":"query","args":["SELECT * FROM wp_users;\\n"]}\n{"type":"CALL","function":"query","args":["SELECT * FROM wp_posts;\\n"]}\n`;
expect(multiQueryResult).toBe(multiQueryExpect);
it('should split and "run" multiple sql queries', async () => {
await runSql(php, {
sql: new File(
[
['SELECT * FROM wp_users;', 'SELECT * FROM wp_posts;'].join(
'\n'
),
],
'multiple-queries.sql'
),
});

// Ensure it works the same if the last query is missing a trailing newline
const mockFileNoTrailingSpace = new File(
[`SELECT * FROM wp_users;\nSELECT * FROM wp_posts;`],
'no-trailing-newline.sql'
const result = php.readFileAsText(outputLogPath);
expect(result).toBe(
`{"type":"CALL","function":"query","args":["SELECT * FROM wp_users;\\n"]}\n{"type":"CALL","function":"query","args":["SELECT * FROM wp_posts;"]}\n`
);
});

it('should support inputs with empty lines and semicolon-only lines', async () => {
await runSql(php, {
sql: new File(
[
[
'SELECT * FROM wp_users;',
';',
'',
'SELECT * FROM wp_posts;',
'',
].join('\n'),
],
'no-trailing-newline.sql'
),
});

await runSql(php, { sql: mockFileNoTrailingSpace });
const noTrailingNewlineQueryResult = await php.readFileAsText(
resFilename
const result = php.readFileAsText(outputLogPath);
expect(result).toBe(
`{"type":"CALL","function":"query","args":["SELECT * FROM wp_users;\\n"]}\n{"type":"CALL","function":"query","args":["SELECT * FROM wp_posts;\\n"]}\n`
);
const noTrailingNewlineQueryExpect = `{"type":"CALL","function":"query","args":["SELECT * FROM wp_users;\\n"]}\n{"type":"CALL","function":"query","args":["SELECT * FROM wp_posts;"]}\n`;
expect(noTrailingNewlineQueryResult).toBe(noTrailingNewlineQueryExpect);
});
});
10 changes: 3 additions & 7 deletions packages/playground/blueprints/src/lib/steps/run-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,15 @@ export const runSql: StepHandler<RunSqlStep<File>> = async (
require_once ${js.docroot} . '/wp-load.php';
$handle = fopen(${js.sqlFilename}, 'r');
$buffer = '';
global $wpdb;
while ($bytes = fgets($handle)) {
$buffer .= $bytes;
if (!feof($handle) && substr($buffer, -1, 1) !== "\n") {
while ($line = fgets($handle)) {
if(trim($line, " \n;") === '') {
continue;
}
$wpdb->query($buffer);
$buffer = '';
$wpdb->query($line);
}
`,
});
Expand Down

0 comments on commit b1c6d5a

Please sign in to comment.