mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-01-06 22:40:14 -05:00
Refactored local revisions to avoid QuotaExceededErrors (#21128)
ref https://ghost-foundation.sentry.io/issues/5908152800/ - In the current state, we are maintaining an 'index' key for all revisions in localStorage. This gives us quick and easy access to all the revisions in localStorage, but it requires additional "bookkeeping" to update the index each time we add/remove a key. - In some obscure edge cases, this results in the `remove()` method throwing a `QuotaExceededError` (since removing a revision also requires updating the index with `localStorage.setItem()`). If the `remove()` call fails, we are sort of stuck — the only way to reduce our storage usage is to remove items, but if the `remove()` method throws errors, we can't do that. - This change removes the whole index concept, and instead loops over all the keys in localStorage, filtering by the prefix to find all our revisions. This makes the `keys()` method slightly more complex, as it has to filter out keys in localStorage that aren't related to revisions, but it simplifies saving and removing revisions. - Critically, this also means that `remove()` should never throw a `QuotaExceededError`, since it no longer needs to call `localStorage.setItem()` — it now simply calls `localStorage.removeItem()` for the revision, which should never fail.
This commit is contained in:
parent
75afbb4f2a
commit
046b06fe72
2 changed files with 25 additions and 25 deletions
|
@ -23,9 +23,6 @@ export default class LocalRevisionsService extends Service {
|
|||
_prefix = 'post-revision';
|
||||
latestRevisionTime = null;
|
||||
|
||||
// key to store a simple index of all revisions
|
||||
_indexKey = 'ghost-revisions';
|
||||
|
||||
/**
|
||||
*
|
||||
* @param {object} data - serialized post data, must include id and revisionTimestamp
|
||||
|
@ -74,11 +71,7 @@ export default class LocalRevisionsService extends Service {
|
|||
data.revisionTimestamp = Date.now();
|
||||
const key = this.generateKey(data);
|
||||
try {
|
||||
const allKeys = this.keys();
|
||||
allKeys.push(key);
|
||||
this.storage.setItem(this._indexKey, JSON.stringify(allKeys));
|
||||
this.storage.setItem(key, JSON.stringify(data));
|
||||
|
||||
// Apply the filter after saving
|
||||
this.filterRevisions(data.id);
|
||||
|
||||
|
@ -152,12 +145,6 @@ export default class LocalRevisionsService extends Service {
|
|||
*/
|
||||
remove(key) {
|
||||
this.storage.removeItem(key);
|
||||
const keys = this.keys();
|
||||
let index = keys.indexOf(key);
|
||||
if (index !== -1) {
|
||||
keys.splice(index, 1);
|
||||
}
|
||||
this.storage.setItem(this._indexKey, JSON.stringify(keys));
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -186,11 +173,15 @@ export default class LocalRevisionsService extends Service {
|
|||
* @returns {string[]}
|
||||
*/
|
||||
keys(prefix = undefined) {
|
||||
let keys = JSON.parse(this.storage.getItem(this._indexKey) || '[]');
|
||||
if (prefix) {
|
||||
keys = keys.filter(key => key.startsWith(prefix));
|
||||
const allKeys = [];
|
||||
const filterPrefix = prefix || this._prefix;
|
||||
for (let i = 0; i < this.storage.length; i++) {
|
||||
const key = this.storage.key(i);
|
||||
if (key.startsWith(filterPrefix)) {
|
||||
allKeys.push(key);
|
||||
}
|
||||
}
|
||||
return keys;
|
||||
return allKeys;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -30,8 +30,7 @@ describe('Unit: Service: local-revisions', function () {
|
|||
// Mock localStorage
|
||||
sinon.restore();
|
||||
localStore = {};
|
||||
getItemStub = sinon.stub().callsFake(key => localStore[key] || null
|
||||
);
|
||||
getItemStub = sinon.stub().callsFake(key => localStore[key] || null);
|
||||
setItemStub = sinon.stub().callsFake((key, value) => localStore[key] = value + '');
|
||||
removeItemStub = sinon.stub().callsFake(key => delete localStore[key]);
|
||||
clearStub = sinon.stub().callsFake(() => localStore = {});
|
||||
|
@ -41,6 +40,16 @@ describe('Unit: Service: local-revisions', function () {
|
|||
removeItem: removeItemStub,
|
||||
clear: clearStub
|
||||
};
|
||||
Object.defineProperty(localStorageMock, 'length', {
|
||||
get: function () {
|
||||
return Object.keys(localStore).length;
|
||||
}
|
||||
});
|
||||
Object.defineProperty(localStorageMock, 'key', {
|
||||
value: function (n) {
|
||||
return Object.keys(localStore)[n];
|
||||
}
|
||||
});
|
||||
|
||||
// Create the service
|
||||
this.service = this.owner.lookup('service:local-revisions');
|
||||
|
@ -116,9 +125,7 @@ describe('Unit: Service: local-revisions', function () {
|
|||
quotaError.name = 'QuotaExceededError';
|
||||
|
||||
setItemStub.onCall(setItemStub.callCount).throws(quotaError);
|
||||
// remove calls setItem() to remove the key from the index
|
||||
// it's called twice for each quota error, hence the + 3
|
||||
setItemStub.onCall(setItemStub.callCount + 3).throws(quotaError);
|
||||
setItemStub.onCall(setItemStub.callCount + 1).throws(quotaError);
|
||||
const keyToAdd = this.service.performSave('post', {id: 'test-id-3', lexical: 'data-3', status: 'draft'});
|
||||
|
||||
// Ensure the oldest revision was removed
|
||||
|
@ -319,11 +326,13 @@ describe('Unit: Service: local-revisions', function () {
|
|||
expect(result).to.deep.equal([]);
|
||||
});
|
||||
|
||||
it('returns the keys for all revisions if not prefix is provided', function () {
|
||||
it('returns the keys for all revisions if no prefix is provided', async function () {
|
||||
// save revision
|
||||
this.service.performSave('post', {id: 'test-id', lexical: 'data', status: 'draft'});
|
||||
await sleep(1);
|
||||
this.service.performSave('post', {id: 'draft', lexical: 'data', status: 'draft'});
|
||||
const result = this.service.keys();
|
||||
expect(Object.keys(result)).to.have.lengthOf(1);
|
||||
expect(result).to.have.lengthOf(2);
|
||||
expect(result[0]).to.match(/post-revision-test-id-\d+/);
|
||||
});
|
||||
|
||||
|
@ -332,7 +341,7 @@ describe('Unit: Service: local-revisions', function () {
|
|||
this.service.performSave('post', {id: 'test-id', lexical: 'data', status: 'draft'});
|
||||
this.service.performSave('post', {id: 'draft', lexical: 'data', status: 'draft'});
|
||||
const result = this.service.keys('post-revision-test-id');
|
||||
expect(Object.keys(result)).to.have.lengthOf(1);
|
||||
expect(result).to.have.lengthOf(1);
|
||||
expect(result[0]).to.match(/post-revision-test-id-\d+/);
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in a new issue