From e46dbec294cd254f087574e72ec773a65001785d Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 4 Jul 2020 23:08:03 +0100 Subject: [PATCH] Move EventSource to SharedWorker (#12095) (#12130) * Move EventSource to SharedWorker (#12095) Backport #12095 Move EventSource to use a SharedWorker. This prevents issues with HTTP/1.1 open browser connections from preventing gitea from opening multiple tabs. Also allow setting EVENT_SOURCE_UPDATE_TIME to disable EventSource updating Fix #11978 Signed-off-by: Andrew Thornton Co-authored-by: silverwind Co-authored-by: techknowlogick * Bugfix for shared event source For some reason our eslint configuration is not working correctly and a bug has become apparent when trying to backport this to 1.12. Signed-off-by: Andrew Thornton * Re-fix #12095 again Unfortunately some of the suggested changes to #12095 introduced bugs which due to caching behaviour of sharedworkers were not caught on simple tests. These are as follows: * Changing from simple for loop to use includes here: ```js register(port) { if (!this.clients.includes(port)) return; this.clients.push(port); port.postMessage({ type: 'status', message: `registered to ${this.url}`, }); } ``` The additional `!` prevents any clients from being added and should read: ```js if (this.clients.includes(port)) return; ``` * Dropping the use of jQuery `$(...)` selection and using DOM `querySelector` here: ```js async function receiveUpdateCount(event) { try { const data = JSON.parse(event.data); const notificationCount = document.querySelector('.notification_count'); if (data.Count > 0) { notificationCount.classList.remove('hidden'); } else { notificationCount.classList.add('hidden'); } notificationCount.text() = `${data.Count}`; await updateNotificationTable(); } catch (error) { console.error(error, event); } } ``` Requires that `notificationCount.text()` be changed to use `textContent` instead. Signed-off-by: Andrew Thornton Co-authored-by: silverwind Co-authored-by: techknowlogick --- .eslintrc | 2 +- custom/conf/app.ini.sample | 2 +- .../doc/advanced/config-cheat-sheet.en-us.md | 3 +- modules/eventsource/manager_run.go | 3 + modules/templates/helper.go | 4 +- .../js/features/eventsource.sharedworker.js | 135 ++++++++++++++++++ web_src/js/features/notification.js | 92 ++++++++---- web_src/js/index.js | 2 +- webpack.config.js | 3 + 9 files changed, 210 insertions(+), 36 deletions(-) create mode 100644 web_src/js/features/eventsource.sharedworker.js diff --git a/.eslintrc b/.eslintrc index 3156f88375..346ae71c49 100644 --- a/.eslintrc +++ b/.eslintrc @@ -25,7 +25,7 @@ globals: Tribute: false overrides: - - files: ["web_src/**/*.worker.js", "web_src/js/serviceworker.js"] + - files: ["web_src/**/*worker.js"] env: worker: true rules: diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index aef5972f6b..a5570136ba 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -211,7 +211,7 @@ MIN_TIMEOUT = 10s MAX_TIMEOUT = 60s TIMEOUT_STEP = 10s ; This setting determines how often the db is queried to get the latest notification counts. -; If the browser client supports EventSource, it will be used in preference to polling notification. +; If the browser client supports EventSource and SharedWorker, a SharedWorker will be used in preference to polling notification. Set to -1 to disable the EventSource EVENT_SOURCE_UPDATE_TIME = 10s [markdown] diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index cb5aad3558..952d5a2938 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -148,8 +148,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `MIN_TIMEOUT`: **10s**: These options control how often notification endpoint is polled to update the notification count. On page load the notification count will be checked after `MIN_TIMEOUT`. The timeout will increase to `MAX_TIMEOUT` by `TIMEOUT_STEP` if the notification count is unchanged. Set MIN_TIMEOUT to 0 to turn off. - `MAX_TIMEOUT`: **60s**. - `TIMEOUT_STEP`: **10s**. -- `EVENT_SOURCE_UPDATE_TIME`: **10s**: This setting determines how often the database is queried to update notification counts. If the browser client supports `EventSource`, it will be used in preference to polling notification endpoint. - +- `EVENT_SOURCE_UPDATE_TIME`: **10s**: This setting determines how often the database is queried to update notification counts. If the browser client supports `EventSource` and `SharedWorker`, a `SharedWorker` will be used in preference to polling notification endpoint. Set to **-1** to disable the `EventSource`. ## Markdown (`markdown`) diff --git a/modules/eventsource/manager_run.go b/modules/eventsource/manager_run.go index 75d3ee5b01..ccfe2e0709 100644 --- a/modules/eventsource/manager_run.go +++ b/modules/eventsource/manager_run.go @@ -17,6 +17,9 @@ import ( // Init starts this eventsource func (m *Manager) Init() { + if setting.UI.Notification.EventSourceUpdateTime <= 0 { + return + } go graceful.GetManager().RunWithShutdownContext(m.Run) } diff --git a/modules/templates/helper.go b/modules/templates/helper.go index bcd22f79f4..ddcb1baa99 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -289,8 +289,8 @@ func NewFuncMap() []template.FuncMap { return "" } }, - "NotificationSettings": func() map[string]int { - return map[string]int{ + "NotificationSettings": func() map[string]interface{} { + return map[string]interface{}{ "MinTimeout": int(setting.UI.Notification.MinTimeout / time.Millisecond), "TimeoutStep": int(setting.UI.Notification.TimeoutStep / time.Millisecond), "MaxTimeout": int(setting.UI.Notification.MaxTimeout / time.Millisecond), diff --git a/web_src/js/features/eventsource.sharedworker.js b/web_src/js/features/eventsource.sharedworker.js new file mode 100644 index 0000000000..bb4f628f6c --- /dev/null +++ b/web_src/js/features/eventsource.sharedworker.js @@ -0,0 +1,135 @@ +self.name = 'eventsource.sharedworker.js'; + +const sourcesByUrl = {}; +const sourcesByPort = {}; + +class Source { + constructor(url) { + this.url = url; + this.eventSource = new EventSource(url); + this.listening = {}; + this.clients = []; + this.listen('open'); + this.listen('logout'); + this.listen('notification-count'); + this.listen('error'); + } + + register(port) { + if (this.clients.includes(port)) return; + + this.clients.push(port); + + port.postMessage({ + type: 'status', + message: `registered to ${this.url}`, + }); + } + + deregister(port) { + const portIdx = this.clients.indexOf(port); + if (portIdx < 0) { + return this.clients.length; + } + this.clients.splice(portIdx, 1); + return this.clients.length; + } + + close() { + if (!this.eventSource) return; + + this.eventSource.close(); + this.eventSource = null; + } + + listen(eventType) { + if (this.listening[eventType]) return; + this.listening[eventType] = true; + const self = this; + this.eventSource.addEventListener(eventType, (event) => { + self.notifyClients({ + type: eventType, + data: event.data + }); + }); + } + + notifyClients(event) { + for (const client of this.clients) { + client.postMessage(event); + } + } + + status(port) { + port.postMessage({ + type: 'status', + message: `url: ${this.url} readyState: ${this.eventSource.readyState}`, + }); + } +} + +self.onconnect = (e) => { + for (const port of e.ports) { + port.addEventListener('message', (event) => { + if (event.data.type === 'start') { + const url = event.data.url; + if (sourcesByUrl[url]) { + // we have a Source registered to this url + const source = sourcesByUrl[url]; + source.register(port); + sourcesByPort[port] = source; + return; + } + let source = sourcesByPort[port]; + if (source) { + if (source.eventSource && source.url === url) return; + + // How this has happened I don't understand... + // deregister from that source + const count = source.deregister(port); + // Clean-up + if (count === 0) { + source.close(); + sourcesByUrl[source.url] = null; + } + } + // Create a new Source + source = new Source(url); + source.register(port); + sourcesByUrl[url] = source; + sourcesByPort[port] = source; + } else if (event.data.type === 'listen') { + const source = sourcesByPort[port]; + source.listen(event.data.eventType); + } else if (event.data.type === 'close') { + const source = sourcesByPort[port]; + + if (!source) return; + + const count = source.deregister(port); + if (count === 0) { + source.close(); + sourcesByUrl[source.url] = null; + sourcesByPort[port] = null; + } + } else if (event.data.type === 'status') { + const source = sourcesByPort[port]; + if (!source) { + port.postMessage({ + type: 'status', + message: 'not connected', + }); + return; + } + source.status(port); + } else { + // just send it back + port.postMessage({ + type: 'error', + message: `received but don't know how to handle: ${event.data}`, + }); + } + }); + port.start(); + } +}; diff --git a/web_src/js/features/notification.js b/web_src/js/features/notification.js index 0ea7f93c8d..b0cc1cda44 100644 --- a/web_src/js/features/notification.js +++ b/web_src/js/features/notification.js @@ -18,7 +18,25 @@ export function initNotificationsTable() { }); } -export function initNotificationCount() { +async function receiveUpdateCount(event) { + try { + const data = JSON.parse(event.data); + + const notificationCount = document.querySelector('.notification_count'); + if (data.Count > 0) { + notificationCount.classList.remove('hidden'); + } else { + notificationCount.classList.add('hidden'); + } + + notificationCount.textContent = `${data.Count}`; + await updateNotificationTable(); + } catch (error) { + console.error(error, event); + } +} + +export async function initNotificationCount() { const notificationCount = $('.notification_count'); if (!notificationCount.length) { @@ -26,36 +44,52 @@ export function initNotificationCount() { } if (NotificationSettings.EventSourceUpdateTime > 0 && !!window.EventSource) { - // Try to connect to the event source first - const source = new EventSource(`${AppSubUrl}/user/events`); - source.addEventListener('notification-count', async (e) => { - try { - const data = JSON.parse(e.data); - - const notificationCount = $('.notification_count'); - if (data.Count === 0) { - notificationCount.addClass('hidden'); - } else { - notificationCount.removeClass('hidden'); + // Try to connect to the event source via the shared worker first + if (window.SharedWorker) { + const worker = new SharedWorker(`${__webpack_public_path__}js/eventsource.sharedworker.js`, 'notification-worker'); + worker.addEventListener('error', (event) => { + console.error(event); + }); + worker.port.onmessageerror = () => { + console.error('Unable to deserialize message'); + }; + worker.port.postMessage({ + type: 'start', + url: `${window.location.origin}${AppSubUrl}/user/events`, + }); + worker.port.addEventListener('message', (event) => { + if (!event.data || !event.data.type) { + console.error(event); + return; } + if (event.data.type === 'notification-count') { + receiveUpdateCount(event.data); + } else if (event.data.type === 'error') { + console.error(event.data); + } else if (event.data.type === 'logout') { + if (event.data !== 'here') { + return; + } + worker.port.postMessage({ + type: 'close', + }); + worker.port.close(); + window.location.href = AppSubUrl; + } + }); + worker.port.addEventListener('error', (e) => { + console.error(e); + }); + worker.port.start(); + window.addEventListener('beforeunload', () => { + worker.port.postMessage({ + type: 'close', + }); + worker.port.close(); + }); - notificationCount.text(`${data.Count}`); - await updateNotificationTable(); - } catch (error) { - console.error(error); - } - }); - source.addEventListener('logout', async (e) => { - if (e.data !== 'here') { - return; - } - source.close(); - window.location.href = AppSubUrl; - }); - window.addEventListener('beforeunload', () => { - source.close(); - }); - return; + return; + } } if (NotificationSettings.MinTimeout <= 0) { diff --git a/web_src/js/index.js b/web_src/js/index.js index fc581baaef..886520db63 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -2455,7 +2455,6 @@ $(document).ready(async () => { initTemplateSearch(); initContextPopups(); initNotificationsTable(); - initNotificationCount(); initTribute(); // Repo clone url. @@ -2502,6 +2501,7 @@ $(document).ready(async () => { initClipboard(), initUserHeatmap(), initServiceWorker(), + initNotificationCount(), ]); }); diff --git a/webpack.config.js b/webpack.config.js index 70a5029e63..358b5ffbbf 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -38,6 +38,9 @@ module.exports = { serviceworker: [ resolve(__dirname, 'web_src/js/serviceworker.js'), ], + 'eventsource.sharedworker': [ + resolve(__dirname, 'web_src/js/features/eventsource.sharedworker.js'), + ], icons: glob('node_modules/@primer/octicons/build/svg/**/*.svg'), ...themes, },