mirror: remove unused helper functions (#1351)
Summary: The functions `isSqlSafe` and `_nontransactionallyFindUnusedTableName` are unused, because we no longer need to dynamically generate SQL, and all operations are clearly safe by construction. Test Plan: That `yarn flow` passes suffices. wchargin-branch: mirror-prune-helpers
This commit is contained in:
parent
b0b911cec4
commit
01bdb2e94a
|
@ -2081,23 +2081,6 @@ export function _inTransaction<R>(db: Database, fn: () => R): R {
|
|||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* In some cases, we need to interpolate user input in SQL queries in
|
||||
* positions that do not allow bound variables in prepared statements
|
||||
* (e.g., table and column names). In these cases, we manually sanitize.
|
||||
*
|
||||
* If this function returns `true`, then its argument may be safely
|
||||
* included in a SQL identifier. If it returns `false`, then no such
|
||||
* guarantee is made (this function is overly conservative, so it is
|
||||
* possible that the argument may in fact be safe).
|
||||
*
|
||||
* For instance, the function will return `true` if passed "col", but
|
||||
* will return `false` if passed "'); DROP TABLE objects; --".
|
||||
*/
|
||||
function isSqlSafe(token: string) {
|
||||
return !token.match(/[^A-Za-z0-9_]/);
|
||||
}
|
||||
|
||||
/**
|
||||
* Convert a prepared statement into a JS function that executes that
|
||||
* statement and asserts that it makes exactly one change to the
|
||||
|
@ -2148,48 +2131,3 @@ export function _makeSingleUpdateFunction<Args: BindingDictionary>(
|
|||
}
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Find a name for a new table (or index) that starts with the given
|
||||
* prefix and is not used by any current table or index.
|
||||
*
|
||||
* This function does not actually create any tables. Consider including
|
||||
* it in a transaction that subsequently creates the table.
|
||||
*
|
||||
* The provided prefix must be a SQL-safe string, or an error will be
|
||||
* thrown.
|
||||
*
|
||||
* The result will be a SQL-safe string, and will not need to be quoted
|
||||
* unless the provided prefix does.
|
||||
*
|
||||
* See: `isSqlSafe`.
|
||||
*/
|
||||
export function _nontransactionallyFindUnusedTableName(
|
||||
db: Database,
|
||||
prefix: string
|
||||
) {
|
||||
if (!isSqlSafe(prefix)) {
|
||||
throw new Error("Unsafe table name prefix: " + JSON.stringify(prefix));
|
||||
}
|
||||
const result: string = db
|
||||
.prepare(
|
||||
dedent`\
|
||||
SELECT :prefix || (IFNULL(MAX(CAST(suffix AS INTEGER)), 0) + 1)
|
||||
FROM (
|
||||
SELECT SUBSTR(name, LENGTH(:prefix) + 1) AS suffix
|
||||
FROM sqlite_master
|
||||
WHERE SUBSTR(name, 1, LENGTH(:prefix)) = :prefix
|
||||
)
|
||||
`
|
||||
)
|
||||
.pluck()
|
||||
.get({prefix});
|
||||
// istanbul ignore if: should not be possible---it only has the
|
||||
// prefix (which is safe as defined above) and a trailing integer.
|
||||
if (!isSqlSafe(result)) {
|
||||
throw new Error(
|
||||
"Invariant violation: unsafe table name: " + JSON.stringify(result)
|
||||
);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
|
|
@ -12,7 +12,6 @@ import {
|
|||
_buildSchemaInfo,
|
||||
_inTransaction,
|
||||
_makeSingleUpdateFunction,
|
||||
_nontransactionallyFindUnusedTableName,
|
||||
Mirror,
|
||||
} from "./mirror";
|
||||
|
||||
|
@ -3692,73 +3691,4 @@ describe("graphql/mirror", () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("_nontransactionallyFindUnusedTableName", () => {
|
||||
it("throws if the name is not SQL-safe", () => {
|
||||
const db = new Database(":memory:");
|
||||
expect(() => {
|
||||
_nontransactionallyFindUnusedTableName(db, "w a t");
|
||||
}).toThrow('Unsafe table name prefix: "w a t"');
|
||||
});
|
||||
it("does not actually create any tables or indices", () => {
|
||||
const db = new Database(":memory:");
|
||||
db.prepare("CREATE TABLE tab (col)").run();
|
||||
db.prepare("CREATE INDEX idx ON tab (col)").run();
|
||||
const getMaster = db.prepare(
|
||||
dedent`
|
||||
SELECT
|
||||
type, name, tbl_name, rootpage, sql
|
||||
FROM sqlite_master
|
||||
ORDER BY
|
||||
type, name, tbl_name, rootpage, sql
|
||||
`
|
||||
);
|
||||
const pre = getMaster.all();
|
||||
expect(pre).toHaveLength(2); // one table, one index
|
||||
_nontransactionallyFindUnusedTableName(db, "hello");
|
||||
const post = getMaster.all();
|
||||
expect(post).toEqual(pre);
|
||||
});
|
||||
it("behaves when there are no conflicts", () => {
|
||||
const db = new Database(":memory:");
|
||||
db.prepare("CREATE TABLE three (col)").run();
|
||||
expect(_nontransactionallyFindUnusedTableName(db, "two_")).toEqual(
|
||||
"two_1"
|
||||
);
|
||||
});
|
||||
it("behaves when there are table-name conflicts", () => {
|
||||
const db = new Database(":memory:");
|
||||
db.prepare("CREATE TABLE two_1 (col)").run();
|
||||
expect(_nontransactionallyFindUnusedTableName(db, "two_")).toEqual(
|
||||
"two_2"
|
||||
);
|
||||
});
|
||||
it("behaves when there are index-name conflicts", () => {
|
||||
const db = new Database(":memory:");
|
||||
db.prepare("CREATE TABLE tab (col)").run();
|
||||
db.prepare("CREATE INDEX idx_1 ON tab (col)").run();
|
||||
expect(_nontransactionallyFindUnusedTableName(db, "idx_")).toEqual(
|
||||
"idx_2"
|
||||
);
|
||||
});
|
||||
it("behaves when there are discontinuities", () => {
|
||||
const db = new Database(":memory:");
|
||||
db.prepare("CREATE TABLE two_1 (col)").run();
|
||||
db.prepare("CREATE TABLE two_3 (col)").run();
|
||||
expect(_nontransactionallyFindUnusedTableName(db, "two_")).toEqual(
|
||||
// It would also be fine for this to return `two_2`.
|
||||
"two_4"
|
||||
);
|
||||
});
|
||||
it("behaves in the face of lexicographical discontinuities", () => {
|
||||
const db = new Database(":memory:");
|
||||
for (let i = 1; i <= 10; i++) {
|
||||
db.prepare(`CREATE TABLE two_${i} (col)`).run();
|
||||
}
|
||||
expect(_nontransactionallyFindUnusedTableName(db, "two_")).toEqual(
|
||||
"two_11"
|
||||
);
|
||||
});
|
||||
//
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue