Skip to content

Commit f925cda

Browse files
committed
Attend to review comments:
1 parent b2be35e commit f925cda

File tree

1 file changed

+43
-14
lines changed

1 file changed

+43
-14
lines changed

glean/src/platform/qt/storage.ts

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,15 @@ import log, { LoggingLevel } from "../../core/log.js";
1212
const LOG_TAG = "platform.qt.Storage";
1313
// The name of the file that will hold the SQLite database.
1414
const DATABASE_NAME = "Glean";
15+
// Estimated size of database file.
16+
const ESTIMATED_DATABASE_SIZE = 150 * 2 * 10**3; // 300Kb
1517
// Since we cannot have nesting in SQL databases,
1618
// we will have a database with only two columns: `key` and `value`.
1719
// The `key` column will contain the StorageIndex as a string, joined by SEPARATOR.
1820
//
19-
// !IMPORTANT! This separator cannot be "." or "#" because these values
20-
// are already used as separators for label and category in other places of the code.
21+
// !IMPORTANT! This separator cannot be ".", "#" or "/" because these values
22+
// are already used as separators for label, category and inside internal metric names
23+
// (e.g. sequence numbers) in other places of the code.
2124
const SEPARATOR = "+";
2225

2326
/**
@@ -94,6 +97,7 @@ function queryResultToJSONObject(
9497

9598
class QMLStore implements Store {
9699
private initialized: Promise<unknown>;
100+
private dbHandle?: LocalStorage.DatabaseHandle;
97101

98102
constructor(
99103
private tableName: string,
@@ -111,9 +115,6 @@ class QMLStore implements Store {
111115
this._executeQuery(
112116
`CREATE TABLE IF NOT EXISTS ${tableName}(key VARCHAR(255), value VARCHAR(255));`
113117
),
114-
// This allows for `REPLACE ITEM` to work.
115-
// See: https://www.sqlitetutorial.net/sqlite-replace-statement/
116-
this._executeQuery(`CREATE UNIQUE INDEX IF NOT EXISTS idx ON ${tableName}(key);`),
117118
);
118119

119120
this.initialized = Promise.all(startQueries);
@@ -123,26 +124,47 @@ class QMLStore implements Store {
123124
return index.join(SEPARATOR);
124125
}
125126

126-
private _getDbHandle(): LocalStorage.DatabaseHandle {
127-
return LocalStorage.LocalStorage.openDatabaseSync(
128-
this.name, "1.0", `${this.name} Storage`, 1000000
129-
);
127+
/**
128+
* Best effort at getting the database handle.
129+
*
130+
* @returns The database handle or `undefined`.
131+
*/
132+
private get _dbHandle(): LocalStorage.DatabaseHandle | undefined {
133+
try {
134+
const handle = LocalStorage.LocalStorage.openDatabaseSync(
135+
this.name, "1.0", `${this.name} Storage`, ESTIMATED_DATABASE_SIZE
136+
);
137+
138+
this.dbHandle = handle;
139+
return handle;
140+
} catch(e) {
141+
log(
142+
LOG_TAG,
143+
["Error while attempting to access LocalStorage.\n", e],
144+
LoggingLevel.Debug
145+
);
146+
} finally {
147+
return this.dbHandle;
148+
}
130149
}
131150

132151
protected _executeQuery(query: string): Promise<LocalStorage.QueryResult | undefined> {
133-
const handle = this._getDbHandle();
152+
const handle = this._dbHandle;
134153

135154
return new Promise((resolve, reject) => {
136155
try {
137-
handle.transaction((tx: LocalStorage.DatabaseTransaction): void => {
156+
// In case the handle is undefined we want to throw and land
157+
// in the `catch` block below.
158+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
159+
handle!.transaction((tx: LocalStorage.DatabaseTransaction): void => {
138160
const result = tx.executeSql(query);
139161
resolve(result);
140162
});
141163
} catch (e) {
142164
log(
143165
LOG_TAG,
144166
[`Error executing LocalStorage query: ${query}.\n`, e],
145-
LoggingLevel.Error
167+
LoggingLevel.Debug
146168
);
147169
reject();
148170
}
@@ -181,9 +203,16 @@ class QMLStore implements Store {
181203
const updates = getKeyValueArrayFromNestedObject(transformedResult);
182204
for (const update of updates) {
183205
const [ key, value ] = update;
184-
await this._executeOnceInitialized(
185-
`REPLACE INTO ${this.tableName}(key, value) VALUES('${key}', '${value.replace("'", "''")}');`
206+
const escapedValue = value.replace("'", "''");
207+
const updateResult = await this._executeOnceInitialized(
208+
`UPDATE ${this.tableName} SET value='${escapedValue}' WHERE key='${key}'`
186209
);
210+
211+
if (!updateResult?.rows.length) {
212+
await this._executeOnceInitialized(
213+
`INSERT INTO ${this.tableName}(key, value) VALUES('${key}', '${escapedValue}');`
214+
);
215+
}
187216
}
188217
}
189218

0 commit comments

Comments
 (0)