Skip to content

Messaging - IDB Cleanup #523

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

Merged
merged 4 commits into from
Feb 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions packages/messaging/src/models/clean-v1-undefined.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* Copyright 2017 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* There seems to have been a bug in the messaging SDK versions <= 4.9.x
* where the IndexedDB model was using a database name of 'undefined'.
*
* In 4.10.x we changed the model implementation, but kept the database
* name as it should have been. This however introduced an issue where
* two tokens were pointing to the same underlying PushSubscription.
*
* This code will look for the undefined database and delete any of the
* underlying tokens.
*/

import IIDModel from '../models/iid-model';

const OLD_DB_NAME = 'undefined';
const OLD_OBJECT_STORE_NAME = 'fcm_token_object_Store';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OBJECT_STORE_NAME_V1 and DB_NAME_V1?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is purely cleaning up the old model (which shouldn't have been named 'undefined' but it was). So this, while related to the token-details-model, they are separate databases and I am just using the v1 -> v2 change to perform this one off clean up. So the version shouldn't every be included or needed here.


function handleDb(db: IDBDatabase) {
if (!db.objectStoreNames.contains(OLD_OBJECT_STORE_NAME)) {
// We found a database with the name 'undefined', but our expected object
// store isn't defined.
return;
}

const transaction = db.transaction(OLD_OBJECT_STORE_NAME);
const objectStore = transaction.objectStore(OLD_OBJECT_STORE_NAME);

const iidModel = new IIDModel();

const openCursorRequest: IDBRequest = objectStore.openCursor();
openCursorRequest.onerror = event => {
// NOOP - Nothing we can do.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be helpful to log something to the console?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a warning log.

console.warn('Unable to cleanup old IDB.', event);
};

openCursorRequest.onsuccess = () => {
const cursor = openCursorRequest.result;
if (cursor) {
// cursor.value contains the current record being iterated through
// this is where you'd do something with the result
const tokenDetails = cursor.value;

iidModel.deleteToken(
tokenDetails.fcmSenderId,
tokenDetails.fcmToken,
tokenDetails.fcmPushSet
);

cursor.continue();
} else {
db.close();
indexedDB.deleteDatabase(OLD_DB_NAME);
}
};
}

function cleanV1() {
const request: IDBOpenDBRequest = indexedDB.open(OLD_DB_NAME);
request.onerror = function(event) {
// NOOP - Nothing we can do.
};
request.onsuccess = function(event) {
const db = request.result;
handleDb(db);
};
}

export { cleanV1 };
4 changes: 2 additions & 2 deletions packages/messaging/src/models/db-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default class DBInterface {
request.onupgradeneeded = event => {
try {
var db = (<IDBRequest>event.target).result;
this.onDBUpgrade(db);
this.onDBUpgrade(db, event);
} catch (err) {
// close the database as it can't be used.
db.close();
Expand Down Expand Up @@ -90,7 +90,7 @@ export default class DBInterface {
* @protected
* @param {!IDBDatabase} db
*/
onDBUpgrade(db) {
onDBUpgrade(db: IDBDatabase, event: IDBVersionChangeEvent) {
throw this.errorFactory_.create(Errors.codes.SHOULD_BE_INHERITED);
}
}
43 changes: 24 additions & 19 deletions packages/messaging/src/models/token-details-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
import DBInterface from './db-interface';
import Errors from './errors';
import arrayBufferToBase64 from '../helpers/array-buffer-to-base64';
import { cleanV1 } from './clean-v1-undefined';

const FCM_TOKEN_OBJ_STORE = 'fcm_token_object_Store';
const DB_VERSION = 1;
const DB_NAME = 'fcm_token_details_db';
const DB_VERSION = 2;

/** @record */
function ValidateInput() {}
Expand All @@ -39,29 +41,32 @@ ValidateInput.prototype.fcmPushSet;

export default class TokenDetailsModel extends DBInterface {
constructor() {
super(TokenDetailsModel.DB_NAME, DB_VERSION);
super(DB_NAME, DB_VERSION);
}

static get DB_NAME() {
return 'fcm_token_details_db';
}
onDBUpgrade(db: IDBDatabase, evt: IDBVersionChangeEvent) {
if (evt.oldVersion < 1) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means it's brand new, right? can you add a quick comment?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment

// New IDB instance
var objectStore = db.createObjectStore(FCM_TOKEN_OBJ_STORE, {
keyPath: 'swScope'
});

/**
* @override
*/
onDBUpgrade(db) {
var objectStore = db.createObjectStore(FCM_TOKEN_OBJ_STORE, {
keyPath: 'swScope'
});
// Make sure the sender ID can be searched
objectStore.createIndex('fcmSenderId', 'fcmSenderId', {
unique: false
});

// Make sure the sender ID can be searched
objectStore.createIndex('fcmSenderId', 'fcmSenderId', {
unique: false
});
objectStore.createIndex('fcmToken', 'fcmToken', {
unique: true
});
}

objectStore.createIndex('fcmToken', 'fcmToken', {
unique: true
});
if (evt.oldVersion < 2) {
// Prior to version 2, we were using either 'fcm_token_details_db'
// or 'undefined' as the database name due to bug in the SDK
// So remove the old tokens and databases.
cleanV1();
}
}

/**
Expand Down
7 changes: 2 additions & 5 deletions packages/messaging/src/models/vapid-details-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,13 @@ import DBInterface from './db-interface';
import Errors from './errors';

const FCM_VAPID_OBJ_STORE = 'fcm_vapid_object_Store';
const DB_NAME = 'fcm_vapid_details_db';
const DB_VERSION = 1;
const UNCOMPRESSED_PUBLIC_KEY_SIZE = 65;

export default class VapidDetailsModel extends DBInterface {
constructor() {
super(VapidDetailsModel.DB_NAME, DB_VERSION);
}

static get DB_NAME() {
return 'fcm_vapid_details_db';
super(DB_NAME, DB_VERSION);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/messaging/test/controller-delete-token.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Firebase Messaging > *Controller.deleteToken()', function() {
deletePromises.push(globalMessagingService.delete());
}
return Promise.all(deletePromises)
.then(() => deleteDatabase(TokenDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_token_details_db'))
.then(() => (globalMessagingService = null));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Firebase Messaging > TokenDetailsModel.deleteToken()', function() {
}

return Promise.all(promises)
.then(() => deleteDatabase(TokenDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_token_details_db'))
.then(() => (globalTokenModel = null));
};

Expand Down
2 changes: 1 addition & 1 deletion packages/messaging/test/tokenDetailsModel-getToken.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('Firebase Messaging > TokenDetailsModel.getTokenDetailsFromToken()', fu
}

return Promise.all(promises)
.then(() => deleteDatabase(TokenDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_token_details_db'))
.then(() => (globalTokenModel = null));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('Firebase Messaging > TokenDetailsModel.saveToken()', function() {
}

return Promise.all(promises)
.then(() => deleteDatabase(TokenDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_token_details_db'))
.then(() => (globalTokenModel = null));
};

Expand Down
2 changes: 1 addition & 1 deletion packages/messaging/test/vapid-details-model-delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('Firebase Messaging > VapidDetailsModel.deleteToken()', function() {
}

return promiseChain
.then(() => deleteDatabase(VapidDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_vapid_details_db'))
.then(() => (vapidModel = null));
};

Expand Down
2 changes: 1 addition & 1 deletion packages/messaging/test/vapid-details-model-get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('Firebase Messaging > VapidDetailsModel.getVapidFromSWScope()', functio
}

return promiseChain
.then(() => deleteDatabase(VapidDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_vapid_details_db'))
.then(() => (vapidModel = null));
};

Expand Down
2 changes: 1 addition & 1 deletion packages/messaging/test/vapid-details-model-save.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('Firebase Messaging > VapidDetailsModel.saveVapidDetails()', function()
}

return promiseChain
.then(() => deleteDatabase(VapidDetailsModel.DB_NAME))
.then(() => deleteDatabase('fcm_vapid_details_db'))
.then(() => (vapidModel = null));
};

Expand Down