Skip to content
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

Implement SSE notifications #9451

Merged
merged 29 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7c85dd1
Implement PoC Polyfill
lookacat Jul 19, 2023
642f864
WIP
lookacat Jul 21, 2023
45ea160
make basic sse notification handling work
lookacat Jul 21, 2023
ef1e377
Add changelog, remove dev leftover
lookacat Jul 21, 2023
f767f3c
remove unused code
lookacat Jul 21, 2023
d95a98a
Fix lots of retries on error
lookacat Jul 24, 2023
a559215
Remove console log
lookacat Jul 24, 2023
30770af
Add async await
lookacat Jul 24, 2023
dc9d51e
refactor into composable
lookacat Jul 27, 2023
7155c3c
Move dependency into web-pkg package.json
lookacat Jul 27, 2023
9841af2
Simplify notifications onOpen error
lookacat Jul 27, 2023
60c4ea7
Add additional required headers
lookacat Jul 27, 2023
d4730f7
Fix x-request-id
lookacat Jul 27, 2023
568b9f4
remove unused import
lookacat Jul 27, 2023
fe59bd7
Make vite work
lookacat Jul 27, 2023
f5eb15e
Fix unittests
lookacat Jul 27, 2023
9930747
fix url join
lookacat Jul 27, 2023
6868b25
fix lockfile
lookacat Jul 27, 2023
54f0c23
Fix 401 error
lookacat Jul 28, 2023
39b623c
remove await to not block
lookacat Jul 28, 2023
2fec745
Implement retry limit
lookacat Jul 31, 2023
b078644
reestablish sse connection on language change
lookacat Jul 31, 2023
4ee4a41
Fix unittests
lookacat Jul 31, 2023
634f77e
Use capability sse
lookacat Aug 1, 2023
a78e0c5
Fix build error
lookacat Aug 1, 2023
614b97e
reintroduce polling if sse isn't enabled
lookacat Aug 2, 2023
77a896a
do initial notification request on see open
lookacat Aug 2, 2023
ef368f1
Fix language switch multiple connections
lookacat Aug 2, 2023
a131e36
fix language switched multiple times
lookacat Aug 3, 2023
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Enhancement: Add SSE to get notifications instantly

We've added SSE to the notifications which allows us to be notified
about new notifications instantly and from the server without polling
every few seconds.

https://github.com/owncloud/web/pull/9451
https://github.com/owncloud/web/issues/9434
1 change: 1 addition & 0 deletions packages/web-pkg/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"directory": "packages/web-pkg"
},
"peerDependencies": {
"@microsoft/fetch-event-source": "^2.0.1",
"@casl/ability": "^6.3.3",
"@casl/vue": "^2.2.1",
"axios": "^0.27.2 || ^1.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const useCapabilityCoreSupportUrlSigning = createCapabilityComposable(
'core.support-url-signing',
false
)
export const useCapabilityCoreSSE = createCapabilityComposable('core.support-sse', false)
export const useCapabilityGraphPersonalDataExport = createCapabilityComposable(
'graph.personal-data-export',
false
Expand Down
1 change: 1 addition & 0 deletions packages/web-pkg/src/composables/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ export * from './store'
export * from './fileListHeaderPosition'
export * from './viewMode'
export * from './search'
export * from './sse'
1 change: 1 addition & 0 deletions packages/web-pkg/src/composables/sse/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './useServerSentEvents'
64 changes: 64 additions & 0 deletions packages/web-pkg/src/composables/sse/useServerSentEvents.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { EventSourceMessage, fetchEventSource } from '@microsoft/fetch-event-source'
import { ref, unref, watch } from 'vue'
import { v4 as uuidV4 } from 'uuid'
import { useGettext } from 'vue3-gettext'
import { configurationManager, useAccessToken, useStore } from 'web-pkg/src'

export interface ServerSentEventsOptions {
url: string
onOpen?: (response: Response) => void
onMessage?: (msg: EventSourceMessage) => void
}

export const useServerSentEvents = (options: ServerSentEventsOptions) => {
const store = useStore()
const language = useGettext()
const accessToken = useAccessToken({ store })
const ctrl = ref(new AbortController())
const retryCounter = ref(0)

watch(
() => language.current,
() => {
unref(ctrl).abort()
}
)
const setupServerSentEvents = () => {
if (unref(retryCounter) >= 5) {
unref(ctrl).abort()
throw new Error('Too many retries')
}
const setupSSE = async () => {
retryCounter.value++
try {
ctrl.value = new AbortController()
await fetchEventSource(new URL(options.url, configurationManager.serverUrl).href, {
signal: unref(ctrl).signal,
headers: {
Authorization: `Bearer ${accessToken.value}`,
'Accept-Language': unref(language).current,
'X-Request-ID': uuidV4()
},
async onopen(response) {
if (response.status === 401) {
unref(ctrl).abort()
return
}
retryCounter.value = 0
await options.onOpen?.(response)
},
onmessage(msg) {
options.onMessage?.(msg)
}
})
} catch (e) {
console.error(e)
}
}
setupSSE().then(() => {
setupServerSentEvents()
})
}

return setupServerSentEvents
}
40 changes: 35 additions & 5 deletions packages/web-runtime/src/components/Topbar/Notifications.vue
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ import {
formatRelativeDateFromISO,
useClientService,
useRoute,
useStore
useStore,
useServerSentEvents,
useCapabilityCoreSSE
} from 'web-pkg'
import { useGettext } from 'vue3-gettext'
import { useTask } from 'vue-concurrency'
Expand All @@ -121,6 +123,30 @@ export default {
const notificationsInterval = ref()
const dropdownOpened = ref(false)

const sseEnabled = useCapabilityCoreSSE()
let setupServerSentEvents
if (unref(sseEnabled)) {
setupServerSentEvents = useServerSentEvents({
url: 'ocs/v2.php/apps/notifications/api/v1/notifications/sse',
onOpen: (response): void => {
fetchNotificationsTask.perform()
if (!response.ok) {
console.error(`SSE notifications couldn't be set up ${response.status}`)
}
},
onMessage: (msg): void => {
if (msg.event === 'FatalError') {
console.error(`SSE notifications error: ${msg.data}`)
return
}
const data = JSON.parse(msg.data)
if (data.notification_id) {
notifications.value = [data, ...unref(notifications)]
}
}
})
}

const formatDate = (date) => {
return formatDateFromISO(date, currentLanguage)
}
Expand Down Expand Up @@ -291,11 +317,15 @@ export default {
setAdditionalData()
}

onMounted(() => {
fetchNotificationsTask.perform()
notificationsInterval.value = setInterval(() => {
onMounted(async () => {
if (unref(sseEnabled)) {
await setupServerSentEvents()
} else {
notificationsInterval.value = setInterval(() => {
fetchNotificationsTask.perform()
}, POLLING_INTERVAL)
fetchNotificationsTask.perform()
}, POLLING_INTERVAL)
}
})

onUnmounted(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ const selectors = {
notificationActions: '.oc-notifications-actions'
}

jest.mock('web-pkg/src/composables/sse/useServerSentEvents')

describe('Notification component', () => {
it('renders the notification bell and no notifications if there are none', () => {
const { wrapper } = getWrapper()
Expand All @@ -34,13 +36,15 @@ describe('Notification component', () => {
it('renders a set of notifications', async () => {
const notifications = [mock<Notification>({ messageRich: undefined })]
const { wrapper } = getWrapper({ notifications })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
expect(wrapper.find(selectors.noNewNotifications).exists()).toBeFalsy()
expect(wrapper.findAll(selectors.notificationItem).length).toBe(notifications.length)
})
it('renders the loading state', async () => {
const notifications = [mock<Notification>({ messageRich: undefined })]
const { wrapper } = getWrapper({ notifications })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
wrapper.vm.loading = true
await wrapper.vm.$nextTick()
Expand All @@ -49,10 +53,11 @@ describe('Notification component', () => {
it('marks all notifications as read', async () => {
const notifications = [mock<Notification>({ messageRich: undefined })]
const { wrapper, mocks } = getWrapper({ notifications })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
await wrapper.find(selectors.markAll).trigger('click')
expect(wrapper.find(selectors.notificationItem).exists()).toBeFalsy()
expect(mocks.$clientService.owncloudSdk.requests.ocs).toHaveBeenCalledTimes(2)
expect(mocks.$clientService.owncloudSdk.requests.ocs).toHaveBeenCalledTimes(3)
})
describe('avatar', () => {
it('loads based on the username', async () => {
Expand All @@ -61,6 +66,7 @@ describe('Notification component', () => {
user: 'einstein'
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
const avatarImageStub = wrapper.findComponent<any>(selectors.avatarImageStub)
expect(avatarImageStub.attributes('userid')).toEqual(notification.user)
Expand All @@ -74,6 +80,7 @@ describe('Notification component', () => {
messageRichParameters: { user: { displayname, name } }
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
const avatarImageStub = wrapper.findComponent<any>(selectors.avatarImageStub)
expect(avatarImageStub.attributes('userid')).toEqual(name)
Expand All @@ -87,6 +94,7 @@ describe('Notification component', () => {
message: undefined
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
expect(wrapper.find(selectors.notificationSubject).exists()).toBeTruthy()
})
Expand All @@ -98,6 +106,7 @@ describe('Notification component', () => {
message: 'some message'
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
wrapper.vm.showDrop()
await wrapper.vm.$nextTick()
Expand All @@ -112,6 +121,7 @@ describe('Notification component', () => {
}
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
wrapper.vm.showDrop()
await wrapper.vm.$nextTick()
Expand All @@ -127,6 +137,7 @@ describe('Notification component', () => {
link: 'http://some-link.com'
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
expect(wrapper.find(selectors.notificationLink).exists()).toBeTruthy()
})
Expand All @@ -142,6 +153,7 @@ describe('Notification component', () => {
}
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
wrapper.vm.showDrop()
await wrapper.vm.$nextTick()
Expand All @@ -168,6 +180,7 @@ describe('Notification component', () => {
}
})
const { wrapper } = getWrapper({ notifications: [notification], spaces: [spaceMock] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
wrapper.vm.showDrop()
await wrapper.vm.$nextTick()
Expand All @@ -187,6 +200,7 @@ describe('Notification component', () => {
actions: [mock<NotificationAction>()]
})
const { wrapper } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
expect(wrapper.find(selectors.notificationActions).exists()).toBeTruthy()
})
Expand All @@ -197,6 +211,7 @@ describe('Notification component', () => {
actions: [mock<NotificationAction>({ link: 'http://some-link.com' })]
})
const { wrapper, mocks } = getWrapper({ notifications: [notification] })
await wrapper.vm.fetchNotificationsTask.perform()
await wrapper.vm.fetchNotificationsTask.last
expect(wrapper.find(selectors.notificationItem).exists()).toBeTruthy()
const jsonResponse = {
Expand Down
9 changes: 8 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.