Skip to content
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* Translation strings added for the layout- and layout-column-widgets.
* When switching locale from the doc editor, ask if the user wants to localize the current document in the target locale or want to start a blank document.
* Introduced a new `longPolling: false` option for the `@apostrophecms/notification` module. This eliminates long-pending requests when logged in, but also slows down the delivery of notifications. The behavior can be tuned further via the `pollingInterval` option, which defaults to `5000` milliseconds.

### Changes

Expand Down
30 changes: 25 additions & 5 deletions modules/@apostrophecms/notification/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
//
// ## Options
//
// ### `longPolling`: by default, to provide a swift response, ApostropheCMS
// keeps a request for new notifications alive until the long polling
// timeout expires (see below). However, `longPolling: false` can be used
// to give an immediate response, in which case the front end will poll
// the old-fashioned way, respecting the `pollingInterval`.
//
// ### `queryInterval`: interval in milliseconds between MongoDB
// queries while long polling for notifications. Defaults to 500
// (1/2 second). Set it longer if you prefer fewer queries, however
Expand All @@ -15,12 +21,22 @@
// Defaults to 10000 (10 seconds) to avoid typical proxy server timeouts.
// Until it times out the request will keep making MongoDB queries to
// see if any new notifications are available (long polling).
//
// ### `pollingInterval`: when `longPolling` is set to `false`, this
// option determines how often the browser polls for new notifications.
// Not used when `longPolling` is `true` (the default).
// `pollingInterval` defaults to 5000 (5 seconds).

const delay = require('bluebird').delay;

module.exports = {
options: {
alias: 'notification'
alias: 'notification',
longPolling: true,
longPollingTimeout: 10000,
queryInterval: 1000,
// Used only when longPolling is false
pollingInterval: 5000
},
extend: '@apostrophecms/module',
async init(self) {
Expand Down Expand Up @@ -64,7 +80,10 @@ module.exports = {
return await attempt();

async function attempt() {
if (Date.now() - start >= (self.options.longPollingTimeout || 10000)) {
if (
self.options.longPolling &&
(Date.now() - start >= self.options.longPollingTimeout)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No fallback needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Correct, no fallback needed, because this is code that preempts making a database query at all if the long polling timer runs out, which isn't relevant in vanilla polling.

) {
return {
notifications: [],
dismissed: []
Expand All @@ -75,11 +94,10 @@ module.exports = {
modifiedOnOrSince,
seenIds
});
if (!notifications.length && !dismissed.length) {
if (self.options.longPolling && !notifications.length && !dismissed.length) {
await delay(self.options.queryInterval || 1000);
return attempt();
}

return {
notifications,
dismissed
Expand Down Expand Up @@ -208,7 +226,9 @@ module.exports = {
return {
getBrowserData(req) {
return {
action: self.action
action: self.action,
longPolling: self.options.longPolling,
pollingInterval: self.options.pollingInterval
};
},
// When used server-side, call with `req` as the first argument,
Expand Down
7 changes: 5 additions & 2 deletions modules/@apostrophecms/ui/ui/apos/stores/notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,13 @@ export const useNotificationStore = defineStore('notification', () => {
return !res.dismissed.some((element) => notif._id === element._id);
});
}
// Long polling, we should reconnect promptly, the server
// If using long polling we should reconnect promptly, the server
// is responsible for keeping that request open for a reasonable
// amount of time if there are no new messages, not us
setTimeout(poll, 50);
const timeout = apos.notification.longPolling
? 50
: apos.notification.pollingInterval;
setTimeout(poll, timeout);
}
} catch (err) {
// eslint-disable-next-line no-console
Expand Down