Skip to content

Commit 1d55212

Browse files
author
Benjamin Pasero
committed
storage - back to PRAGMA check on startup
1 parent 08d3833 commit 1d55212

7 files changed

Lines changed: 30 additions & 38 deletions

File tree

‎src/vs/base/node/storage.ts‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { Database, Statement, OPEN_READWRITE, OPEN_CREATE } from 'vscode-sqlite3';
6+
import { Database, Statement } from 'vscode-sqlite3';
77
import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
88
import { Emitter, Event } from 'vs/base/common/event';
99
import { ThrottledDelayer, timeout } from 'vs/base/common/async';
@@ -14,7 +14,6 @@ import { mark } from 'vs/base/common/performance';
1414

1515
export interface IStorageOptions {
1616
path: string;
17-
createPath: boolean;
1817

1918
logging?: IStorageLoggingOptions;
2019
}
@@ -341,10 +340,10 @@ export class SQLiteStorageImpl {
341340

342341
// In case of any error to open the DB, use an in-memory
343342
// DB so that we always have a valid DB to talk to.
344-
this.doOpen(':memory:', true).then(resolve, reject);
343+
this.doOpen(':memory:').then(resolve, reject);
345344
};
346345

347-
this.doOpen(this.options.path, this.options.createPath).then(resolve, error => {
346+
this.doOpen(this.options.path).then(resolve, error => {
348347

349348
// TODO@Ben check if this is still happening. This error code should only arise if
350349
// another process is locking the same DB we want to open at that time. This typically
@@ -355,7 +354,7 @@ export class SQLiteStorageImpl {
355354
this.logger.error(`[storage ${this.name}] open(): Retrying after ${SQLiteStorageImpl.BUSY_OPEN_TIMEOUT}ms due to SQLITE_BUSY`);
356355

357356
// Retry after 2s if the DB is busy
358-
timeout(SQLiteStorageImpl.BUSY_OPEN_TIMEOUT).then(() => this.doOpen(this.options.path, this.options.createPath).then(resolve, fallbackToInMemoryDatabase));
357+
timeout(SQLiteStorageImpl.BUSY_OPEN_TIMEOUT).then(() => this.doOpen(this.options.path).then(resolve, fallbackToInMemoryDatabase));
359358
}
360359

361360
// Otherwise give up and fallback to in-memory DB
@@ -366,7 +365,7 @@ export class SQLiteStorageImpl {
366365
});
367366
}
368367

369-
private doOpen(path: string, createPath: boolean): Promise<Database> {
368+
private doOpen(path: string): Promise<Database> {
370369
return new Promise((resolve, reject) => {
371370
let measureRequireDuration = false;
372371
if (!SQLiteStorageImpl.measuredRequireDuration) {
@@ -380,17 +379,14 @@ export class SQLiteStorageImpl {
380379
mark('didRequireSQLite');
381380
}
382381

383-
const db = new (this.logger.verbose ? sqlite3.verbose().Database : sqlite3.Database)(path, createPath ? OPEN_READWRITE | OPEN_CREATE : OPEN_READWRITE, error => {
382+
const db = new (this.logger.verbose ? sqlite3.verbose().Database : sqlite3.Database)(path, error => {
384383
if (error) {
385384
return reject(error);
386385
}
387386

388-
// Return early if we did not create the DB
389-
if (!createPath) {
390-
return resolve(db);
391-
}
392-
393-
// Otherwise: Setup schema
387+
// The following exec() statement serves two purposes:
388+
// - create the DB if it does not exist yet
389+
// - validate that the DB is not corrupt (the open() call does not throw otherwise)
394390
mark('willSetupSQLiteSchema');
395391
this.exec(db, [
396392
'PRAGMA user_version = 1;',

‎src/vs/base/test/node/storage/storage.test.ts‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ suite('Storage Library', () => {
2323
const storageDir = uniqueStorageDir();
2424
await mkdirp(storageDir);
2525

26-
const storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: true });
26+
const storage = new Storage({ path: join(storageDir, 'storage.db') });
2727

2828
await storage.init();
2929

@@ -97,7 +97,7 @@ suite('Storage Library', () => {
9797
const storageDir = uniqueStorageDir();
9898
await mkdirp(storageDir);
9999

100-
let storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: true });
100+
let storage = new Storage({ path: join(storageDir, 'storage.db') });
101101
await storage.init();
102102

103103
const set1Promise = storage.set('foo', 'bar');
@@ -113,15 +113,15 @@ suite('Storage Library', () => {
113113

114114
equal(setPromiseResolved, true);
115115

116-
storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: false });
116+
storage = new Storage({ path: join(storageDir, 'storage.db') });
117117
await storage.init();
118118

119119
equal(storage.get('foo'), 'bar');
120120
equal(storage.get('bar'), 'foo');
121121

122122
await storage.close();
123123

124-
storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: false });
124+
storage = new Storage({ path: join(storageDir, 'storage.db') });
125125
await storage.init();
126126

127127
const delete1Promise = storage.delete('foo');
@@ -137,7 +137,7 @@ suite('Storage Library', () => {
137137

138138
equal(deletePromiseResolved, true);
139139

140-
storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: false });
140+
storage = new Storage({ path: join(storageDir, 'storage.db') });
141141
await storage.init();
142142

143143
ok(!storage.get('foo'));
@@ -151,7 +151,7 @@ suite('Storage Library', () => {
151151
const storageDir = uniqueStorageDir();
152152
await mkdirp(storageDir);
153153

154-
let storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: true });
154+
let storage = new Storage({ path: join(storageDir, 'storage.db') });
155155
await storage.init();
156156

157157
let changes = new Set<string>();
@@ -206,7 +206,7 @@ suite('SQLite Storage Library', () => {
206206
}
207207

208208
async function testDBBasics(path, errorLogger?: (error) => void) {
209-
const options: IStorageOptions = { path, createPath: true };
209+
const options: IStorageOptions = { path };
210210
if (errorLogger) {
211211
options.logging = {
212212
errorLogger
@@ -305,8 +305,7 @@ suite('SQLite Storage Library', () => {
305305
await mkdirp(storageDir);
306306

307307
let storage = new SQLiteStorageImpl({
308-
path: join(storageDir, 'storage.db'),
309-
createPath: true
308+
path: join(storageDir, 'storage.db')
310309
});
311310

312311
const items1 = new Map<string, string>();
@@ -382,8 +381,7 @@ suite('SQLite Storage Library', () => {
382381
await storage.close();
383382

384383
storage = new SQLiteStorageImpl({
385-
path: join(storageDir, 'storage.db'),
386-
createPath: true
384+
path: join(storageDir, 'storage.db')
387385
});
388386

389387
storedItems = await storage.getItems();
@@ -400,8 +398,7 @@ suite('SQLite Storage Library', () => {
400398
await mkdirp(storageDir);
401399

402400
let storage = new SQLiteStorageImpl({
403-
path: join(storageDir, 'storage.db'),
404-
createPath: true
401+
path: join(storageDir, 'storage.db')
405402
});
406403

407404
const items = new Map<string, string>();
@@ -454,7 +451,7 @@ suite('SQLite Storage Library', () => {
454451
const storageDir = uniqueStorageDir();
455452
await mkdirp(storageDir);
456453

457-
const storage = new Storage({ path: join(storageDir, 'storage.db'), createPath: true });
454+
const storage = new Storage({ path: join(storageDir, 'storage.db') });
458455

459456
await storage.init();
460457

‎src/vs/platform/storage/node/storageService.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ export class StorageService extends Disposable implements IStorageService {
6262

6363
constructor(
6464
workspaceStoragePath: string,
65-
workspaceStoragePathExists: boolean,
6665
disableGlobalStorage: boolean,
6766
@ILogService logService: ILogService,
6867
@IEnvironmentService environmentService: IEnvironmentService
@@ -86,21 +85,21 @@ export class StorageService extends Disposable implements IStorageService {
8685
};
8786

8887
this.globalStorageWorkspacePath = workspaceStoragePath === StorageService.IN_MEMORY_PATH ? StorageService.IN_MEMORY_PATH : StorageService.IN_MEMORY_PATH;
89-
this.globalStorage = disableGlobalStorage ? new NullStorage() : new Storage({ path: this.globalStorageWorkspacePath, createPath: true, logging: this.loggingOptions });
88+
this.globalStorage = disableGlobalStorage ? new NullStorage() : new Storage({ path: this.globalStorageWorkspacePath, logging: this.loggingOptions });
9089
this._register(this.globalStorage.onDidChangeStorage(key => this.handleDidChangeStorage(key, StorageScope.GLOBAL)));
9190

92-
this.createWorkspaceStorage(workspaceStoragePath, workspaceStoragePathExists);
91+
this.createWorkspaceStorage(workspaceStoragePath);
9392
}
9493

95-
private createWorkspaceStorage(workspaceStoragePath: string, workspaceStoragePathExists: boolean): void {
94+
private createWorkspaceStorage(workspaceStoragePath: string): void {
9695

9796
// Dispose old (if any)
9897
this.workspaceStorage = dispose(this.workspaceStorage);
9998
this.workspaceStorageListener = dispose(this.workspaceStorageListener);
10099

101100
// Create new
102101
this.workspaceStoragePath = workspaceStoragePath;
103-
this.workspaceStorage = new Storage({ path: workspaceStoragePath, createPath: !workspaceStoragePathExists, logging: this.loggingOptions });
102+
this.workspaceStorage = new Storage({ path: workspaceStoragePath, logging: this.loggingOptions });
104103
this.workspaceStorageListener = this.workspaceStorage.onDidChangeStorage(key => this.handleDidChangeStorage(key, StorageScope.WORKSPACE));
105104
}
106105

@@ -235,7 +234,7 @@ export class StorageService extends Disposable implements IStorageService {
235234
// Close workspace DB to be able to copy
236235
return this.workspaceStorage.close().then(() => {
237236
return copy(this.workspaceStoragePath, newWorkspaceStoragePath).then(() => {
238-
this.createWorkspaceStorage(newWorkspaceStoragePath, false);
237+
this.createWorkspaceStorage(newWorkspaceStoragePath);
239238

240239
return this.workspaceStorage.init();
241240
});

‎src/vs/platform/storage/test/node/storageService.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ suite('StorageService', () => {
8484
const storageDir = uniqueStorageDir();
8585
await mkdirp(storageDir);
8686

87-
const storage = new StorageService(join(storageDir, 'storage.db'), false, false, new NullLogService(), TestEnvironmentService);
87+
const storage = new StorageService(join(storageDir, 'storage.db'), false, new NullLogService(), TestEnvironmentService);
8888
await storage.init();
8989

9090
storage.store('bar', 'foo', StorageScope.WORKSPACE);

‎src/vs/platform/telemetry/test/electron-browser/commonProperties.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ suite('Telemetry - common properties', function () {
2424
let nestStorage2Service: IStorageService;
2525

2626
setup(() => {
27-
nestStorage2Service = new StorageService(':memory:', false, false, new NullLogService(), TestEnvironmentService);
27+
nestStorage2Service = new StorageService(':memory:', false, new NullLogService(), TestEnvironmentService);
2828
});
2929

3030
teardown(done => {

‎src/vs/workbench/electron-browser/main.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ function createStorageService(workspaceStorageFolder: string, payload: IWorkspac
284284
// Return early if we are using in-memory storage
285285
const useInMemoryStorage = !!environmentService.extensionTestsPath; /* never keep any state when running extension tests */
286286
if (useInMemoryStorage) {
287-
const storageService = new StorageService(StorageService.IN_MEMORY_PATH, false, true, logService, environmentService);
287+
const storageService = new StorageService(StorageService.IN_MEMORY_PATH, true, logService, environmentService);
288288

289289
return storageService.init().then(() => storageService);
290290
}
@@ -296,7 +296,7 @@ function createStorageService(workspaceStorageFolder: string, payload: IWorkspac
296296
return exists(workspaceStorageDBPath).then(exists => {
297297
perf.mark('didCheckWorkspaceStorageExists');
298298

299-
const storageService = new StorageService(workspaceStorageDBPath, exists, true, logService, environmentService);
299+
const storageService = new StorageService(workspaceStorageDBPath, true, logService, environmentService);
300300

301301
return storageService.init().then(() => {
302302
if (exists) {

‎src/vs/workbench/test/workbenchTestServices.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ export class TestPartService implements IPartService {
531531
export class TestStorageService extends StorageService {
532532

533533
constructor() {
534-
super(':memory:', false, false, new NullLogService(), TestEnvironmentService);
534+
super(':memory:', false, new NullLogService(), TestEnvironmentService);
535535
}
536536
}
537537

0 commit comments

Comments
 (0)