Skip to content

Commit cc688a5

Browse files
alexanderbyalexanderby
authored
Storage refactoring and fixes (darkreader#3605)
- Fixed forcing users from local to sync storage. - Fixed unexpected behavior when sync storage fails to load. - Removed old settings migrations (v4.6.2). - Wrapped storage API into Promises. - Another fix for darkreader#3297 and darkreader#3424.
1 parent cdf9bef commit cc688a5

5 files changed

Lines changed: 145 additions & 156 deletions

File tree

‎src/background/extension.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ export class Extension {
103103

104104
this.startAutoTimeCheck();
105105
this.news.subscribe();
106-
this.user.cleanup();
107106
}
108107

109108
private popupOpeningListener: () => void = null;

‎src/background/newsmaker.ts‎

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {getBlogPostURL} from '../utils/links';
22
import {getDuration} from '../utils/time';
33
import {News} from '../definitions';
4+
import {readSyncStorage, readLocalStorage, writeSyncStorage, writeLocalStorage} from './utils/extension-api';
45

56
export default class Newsmaker {
67
static UPDATE_INTERVAL = getDuration({hours: 4});
@@ -26,56 +27,58 @@ export default class Newsmaker {
2627
}
2728
}
2829

30+
private async getReadNews(): Promise<string[]> {
31+
let sync = await readSyncStorage({readNews: []});
32+
let local = await readLocalStorage({readNews: []});
33+
return Array.from(new Set([
34+
...sync ? sync.readNews : [],
35+
...local ? local.readNews : [],
36+
]));
37+
}
38+
2939
private async getNews() {
3040
try {
3141
const response = await fetch(`https://darkreader.github.io/blog/posts.json?date=${(new Date()).toISOString().substring(0, 10)}`, {cache: 'no-cache'});
3242
const $news = await response.json();
33-
return new Promise<News[]>((resolve, reject) => {
34-
chrome.storage.sync.get({readNews: []}, ({readNews}) => {
35-
const news: News[] = $news.map(({id, date, headline, important}) => {
36-
const url = getBlogPostURL(id);
37-
const read = this.isRead(id, readNews);
38-
return {id, date, headline, url, important, read};
39-
});
40-
for (let i = 0; i < news.length; i++) {
41-
const date = new Date(news[i].date);
42-
if (isNaN(date.getTime())) {
43-
reject(new Error(`Unable to parse date ${date}`));
44-
return;
45-
}
46-
}
47-
resolve(news);
48-
});
43+
const readNews = await this.getReadNews();
44+
const news: News[] = $news.map(({id, date, headline, important}) => {
45+
const url = getBlogPostURL(id);
46+
const read = this.isRead(id, readNews);
47+
return {id, date, headline, url, important, read};
4948
});
49+
for (let i = 0; i < news.length; i++) {
50+
const date = new Date(news[i].date);
51+
if (isNaN(date.getTime())) {
52+
throw new Error(`Unable to parse date ${date}`);
53+
}
54+
}
55+
return news;
5056
} catch (err) {
5157
console.error(err);
5258
return null;
5359
}
5460
}
5561

56-
markAsRead(...ids: string[]) {
57-
return new Promise((resolve) => {
58-
chrome.storage.sync.get({readNews: []}, ({readNews}) => {
59-
const results = readNews.slice();
60-
let changed = false;
61-
ids.forEach((id) => {
62-
if (readNews.indexOf(id) < 0) {
63-
results.push(id);
64-
changed = true;
65-
}
66-
});
67-
if (changed) {
68-
this.latest = this.latest.map(({id, date, url, headline, important}) => {
69-
const read = this.isRead(id, results);
70-
return {id, date, url, headline, important, read};
71-
});
72-
this.onUpdate(this.latest);
73-
chrome.storage.sync.set({readNews: results}, () => resolve());
74-
} else {
75-
resolve();
76-
}
77-
});
62+
async markAsRead(...ids: string[]) {
63+
const readNews = await this.getReadNews();
64+
const results = readNews.slice();
65+
let changed = false;
66+
ids.forEach((id) => {
67+
if (readNews.indexOf(id) < 0) {
68+
results.push(id);
69+
changed = true;
70+
}
7871
});
72+
if (changed) {
73+
this.latest = this.latest.map(({id, date, url, headline, important}) => {
74+
const read = this.isRead(id, results);
75+
return {id, date, url, headline, important, read};
76+
});
77+
this.onUpdate(this.latest);
78+
const obj = {readNews: results};
79+
await writeLocalStorage(obj);
80+
await writeSyncStorage(obj);
81+
}
7982
}
8083

8184
isRead(id: string, readNews: string[]) {

‎src/background/user-storage.ts‎

Lines changed: 56 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {DEFAULT_SETTINGS, DEFAULT_THEME} from '../defaults';
2+
import {debounce} from '../utils/debounce';
23
import {isURLMatched} from '../utils/url';
34
import {UserSettings} from '../definitions';
5+
import {readSyncStorage, readLocalStorage, writeSyncStorage, writeLocalStorage} from './utils/extension-api';
46

57
const SAVE_TIMEOUT = 1000;
68

@@ -15,91 +17,71 @@ export default class UserStorage {
1517
this.settings = await this.loadSettingsFromStorage();
1618
}
1719

18-
cleanup() {
19-
chrome.storage.local.remove(['activationTime', 'deactivationTime']);
20-
chrome.storage.sync.remove(['activationTime', 'deactivationTime']);
20+
private fillDefaults(settings: UserSettings) {
21+
settings.theme = {...DEFAULT_THEME, ...settings.theme};
22+
settings.time = {...DEFAULT_SETTINGS.time, ...settings.time};
23+
settings.presets.forEach((preset) => {
24+
preset.theme = {...DEFAULT_THEME, ...preset.theme};
25+
});
26+
settings.customThemes.forEach((site) => {
27+
site.theme = {...DEFAULT_THEME, ...site.theme};
28+
});
2129
}
2230

23-
private loadSettingsFromStorage() {
24-
return new Promise<UserSettings>((resolve) => {
25-
chrome.storage.local.get(DEFAULT_SETTINGS, (local: UserSettings) => {
26-
local.syncSettings = local.syncSettings || DEFAULT_SETTINGS.syncSettings;
27-
if (!local.syncSettings) {
28-
local.theme = {...DEFAULT_SETTINGS.theme, ...local.theme};
29-
local.time = {...DEFAULT_SETTINGS.time, ...local.time};
30-
local.customThemes.forEach((site) => {
31-
site.theme = {...DEFAULT_SETTINGS.theme, ...site.theme};
32-
});
33-
resolve(local);
34-
return;
35-
}
31+
private async loadSettingsFromStorage() {
32+
const local = await readLocalStorage(DEFAULT_SETTINGS);
33+
if (local.syncSettings == null) {
34+
local.syncSettings = DEFAULT_SETTINGS.syncSettings;
35+
}
36+
if (!local.syncSettings) {
37+
this.fillDefaults(local);
38+
return local;
39+
}
3640

37-
chrome.storage.sync.get({...DEFAULT_SETTINGS, config: 'empty'}, ($sync: UserSettings & {config: any}) => {
38-
let sync: UserSettings;
39-
if (!$sync) {
40-
this.saveSyncSetting(false);
41-
resolve(this.loadSettingsFromStorage());
42-
return;
43-
}
44-
if ($sync.config === 'empty') {
45-
delete $sync.config;
46-
sync = $sync;
47-
} else {
48-
sync = this.migrateSettings_4_6_2($sync) as UserSettings;
49-
}
50-
sync.theme = {...DEFAULT_SETTINGS.theme, ...sync.theme};
51-
sync.time = {...DEFAULT_SETTINGS.time, ...sync.time};
52-
sync.presets.forEach((preset) => {
53-
preset.theme = {...DEFAULT_SETTINGS.theme, ...preset.theme};
54-
});
55-
sync.customThemes.forEach((site) => {
56-
site.theme = {...DEFAULT_SETTINGS.theme, ...site.theme};
57-
});
58-
resolve(sync);
59-
});
60-
});
61-
});
62-
}
41+
const $sync = await readSyncStorage(DEFAULT_SETTINGS);
42+
if (!$sync) {
43+
console.warn('Sync settings are missing');
44+
local.syncSettings = false;
45+
this.set({syncSettings: false});
46+
this.saveSyncSetting(false);
47+
return local;
48+
}
6349

64-
async saveSettings() {
65-
const saved = await this.saveSettingsIntoStorage(this.settings);
66-
this.settings = saved;
50+
const sync = await readSyncStorage(DEFAULT_SETTINGS);
51+
this.fillDefaults(sync);
52+
return sync;
6753
}
6854

69-
saveSyncSetting(sync: boolean) {
70-
chrome.storage.sync.set({syncSettings: sync}, () => {
71-
if (chrome.runtime.lastError) {
72-
console.warn('Settings synchronization was disabled due to error:', chrome.runtime.lastError);
73-
}
74-
});
75-
chrome.storage.local.set({syncSettings: sync});
55+
async saveSettings() {
56+
await this.saveSettingsIntoStorage();
7657
}
7758

78-
private saveSettingsIntoStorage(settings: UserSettings) {
79-
if (this.timeout) {
80-
clearInterval(this.timeout);
59+
async saveSyncSetting(sync: boolean) {
60+
const obj = {syncSettings: sync};
61+
await writeLocalStorage(obj);
62+
try {
63+
await writeSyncStorage(obj);
64+
} catch (err) {
65+
console.warn('Settings synchronization was disabled due to error:', chrome.runtime.lastError);
66+
this.set({syncSettings: false});
8167
}
82-
return new Promise<UserSettings>((resolve) => {
83-
this.timeout = setTimeout(() => {
84-
this.timeout = null;
85-
if (settings.syncSettings) {
86-
chrome.storage.sync.set(settings, () => {
87-
if (chrome.runtime.lastError) {
88-
console.warn('Settings synchronization was disabled due to error:', chrome.runtime.lastError);
89-
const local: UserSettings = {...settings, syncSettings: false};
90-
chrome.storage.local.set(local, () => resolve(local));
91-
} else {
92-
resolve(settings);
93-
}
94-
});
95-
} else {
96-
chrome.storage.local.set(settings, () => resolve(settings));
97-
}
98-
}, SAVE_TIMEOUT);
99-
});
10068
}
10169

102-
private timeout: number = null;
70+
private saveSettingsIntoStorage = debounce(SAVE_TIMEOUT, async () => {
71+
const settings = this.settings;
72+
if (settings.syncSettings) {
73+
try {
74+
await writeSyncStorage(settings);
75+
} catch (err) {
76+
console.warn('Settings synchronization was disabled due to error:', chrome.runtime.lastError);
77+
this.set({syncSettings: false});
78+
await this.saveSyncSetting(false);
79+
await writeLocalStorage(settings);
80+
}
81+
} else {
82+
await writeLocalStorage(settings);
83+
}
84+
});
10385

10486
set($settings: Partial<UserSettings>) {
10587
if ($settings.siteList) {
@@ -128,47 +110,4 @@ export default class UserStorage {
128110
}
129111
this.settings = {...this.settings, ...$settings};
130112
}
131-
132-
private migrateSettings_4_6_2(settings_4_6_2: any) {
133-
function migrateTheme(filterConfig_4_6_2: any) {
134-
const f = filterConfig_4_6_2;
135-
return {
136-
...DEFAULT_THEME,
137-
mode: f.mode,
138-
brightness: f.brightness,
139-
contrast: f.contrast,
140-
grayscale: f.grayscale,
141-
sepia: f.sepia,
142-
useFont: f.useFont,
143-
fontFamily: f.fontFamily,
144-
textStroke: f.textStroke,
145-
engine: f.engine,
146-
stylesheet: f.stylesheet,
147-
};
148-
}
149-
150-
try {
151-
const s = settings_4_6_2;
152-
const settings: UserSettings = {
153-
...DEFAULT_SETTINGS,
154-
enabled: s.enabled,
155-
theme: migrateTheme(s.config),
156-
customThemes: s.config.custom ? s.config.custom.map((c) => {
157-
return {
158-
url: c.url,
159-
theme: migrateTheme(c.config),
160-
};
161-
}) : [],
162-
siteList: s.config.siteList,
163-
applyToListedOnly: s.config.invertListed,
164-
changeBrowserTheme: s.config.changeBrowserTheme,
165-
};
166-
chrome.storage.sync.remove('config');
167-
chrome.storage.sync.set(settings);
168-
return settings;
169-
} catch (err) {
170-
console.error('Settings migration error:', err, 'Loaded settings:', settings_4_6_2);
171-
return DEFAULT_SETTINGS;
172-
}
173-
}
174113
}

‎src/background/utils/extension-api.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,42 @@ export function canInjectScript(url: string) {
3131
);
3232
}
3333

34+
export function readSyncStorage<T extends {[key: string]: any}>(defaults: T): Promise<T> {
35+
return new Promise<T>((resolve) => {
36+
chrome.storage.sync.get(defaults, (sync: T) => {
37+
resolve(sync);
38+
});
39+
});
40+
}
41+
42+
export function readLocalStorage<T extends {[key: string]: any}>(defaults: T): Promise<T> {
43+
return new Promise<T>((resolve) => {
44+
chrome.storage.local.get(defaults, (local: T) => {
45+
resolve(local);
46+
});
47+
});
48+
}
49+
50+
export function writeSyncStorage<T extends {[key: string]: any}>(values: T): Promise<void> {
51+
return new Promise<void>((resolve, reject) => {
52+
chrome.storage.sync.set(values, () => {
53+
if (chrome.runtime.lastError) {
54+
reject(chrome.runtime.lastError);
55+
return;
56+
}
57+
resolve();
58+
});
59+
});
60+
}
61+
62+
export function writeLocalStorage<T extends {[key: string]: any}>(values: T): Promise<void> {
63+
return new Promise<void>((resolve) => {
64+
chrome.storage.local.set(values, () => {
65+
resolve();
66+
});
67+
});
68+
}
69+
3470
export function getFontList() {
3571
return new Promise<string[]>((resolve) => {
3672
if (!chrome.fontSettings) {

‎src/utils/debounce.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export function debounce<F extends (...args: any[]) => any>(delay: number, fn: F): F {
2+
let timeoutId: number = null;
3+
return ((...args: any[]) => {
4+
if (timeoutId) {
5+
clearTimeout(timeoutId);
6+
}
7+
timeoutId = setTimeout(() => {
8+
timeoutId = null;
9+
fn(...args);
10+
}, delay);
11+
}) as any;
12+
}

0 commit comments

Comments
 (0)