Skip to content

Commit

Permalink
Add setting for sync timeout
Browse files Browse the repository at this point in the history
The setting is a string that can be formatted as anything that
[moment.duration](https://momentjs.com/docs/#/durations/creating/)
can parse:
* 'hh:mm:ss'
* 'PT1M' (ISO 8601)

Any invalid setting or setting to less than a second
will lead to the calendar updates being disabled.

The setting is not exposed in the UI.
It's meant to be configured by server admins
to be able to adjust the timeout and prevent an overly high load.

Signed-off-by: Azul <[email protected]>
  • Loading branch information
azul committed Feb 28, 2021
1 parent a566196 commit 56fc034
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 13 deletions.
2 changes: 2 additions & 0 deletions lib/Controller/PublicViewController.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ private function publicIndex(string $token,
$defaultSkipPopover = $this->config->getAppValue($this->appName, 'skipPopover', 'yes');
$defaultTimezone = $this->config->getAppValue($this->appName, 'timezone', 'automatic');
$defaultSlotDuration = $this->config->getAppValue($this->appName, 'slotDuration', '00:30:00');
$defaultSyncTimeout = $this->config->getAppValue($this->appName, 'syncTimeout', 'PT1M');
$defaultShowTasks = $this->config->getAppValue($this->appName, 'showTasks', 'yes');

$appVersion = $this->config->getAppValue($this->appName, 'installed_version', null);
Expand All @@ -135,6 +136,7 @@ private function publicIndex(string $token,
$this->initialStateService->provideInitialState($this->appName, 'timezone', $defaultTimezone);
$this->initialStateService->provideInitialState($this->appName, 'slot_duration', $defaultSlotDuration);
$this->initialStateService->provideInitialState($this->appName, 'show_tasks', $defaultShowTasks === 'yes');
$this->initialStateService->provideInitialState($this->appName, 'sync_timeout', $defaultSyncTimeout);
$this->initialStateService->provideInitialState($this->appName, 'tasks_enabled', false);

return new TemplateResponse($this->appName, 'main', [
Expand Down
2 changes: 2 additions & 0 deletions lib/Controller/ViewController.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public function index():TemplateResponse {
$timezone = $this->config->getUserValue($this->userId, $this->appName, 'timezone', $defaultTimezone);
$slotDuration = $this->config->getUserValue($this->userId, $this->appName, 'slotDuration', $defaultSlotDuration);
$showTasks = $this->config->getUserValue($this->userId, $this->appName, 'showTasks', $defaultShowTasks) === 'yes';
$syncTimeout = $this->config->getAppValue($this->appName, 'syncTimeout', 'PT1M');

$talkEnabled = $this->appManager->isEnabledForUser('spreed');
$tasksEnabled = $this->appManager->isEnabledForUser('tasks');
Expand All @@ -121,6 +122,7 @@ public function index():TemplateResponse {
$this->initialStateService->provideInitialState($this->appName, 'timezone', $timezone);
$this->initialStateService->provideInitialState($this->appName, 'slot_duration', $slotDuration);
$this->initialStateService->provideInitialState($this->appName, 'show_tasks', $showTasks);
$this->initialStateService->provideInitialState($this->appName, 'sync_timeout', $syncTimeout);
$this->initialStateService->provideInitialState($this->appName, 'tasks_enabled', $tasksEnabled);

return new TemplateResponse($this->appName, 'main');
Expand Down
15 changes: 14 additions & 1 deletion src/store/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { detectTimezone } from '../services/timezoneDetectionService'
import { setConfig as setCalendarJsConfig } from 'calendar-js'
import { setConfig } from '../services/settings.js'
import { logInfo } from '../utils/logger.js'
import moment from '@nextcloud/moment'

const state = {
// env
Expand All @@ -38,6 +39,7 @@ const state = {
showWeekNumbers: null,
skipPopover: null,
slotDuration: null,
syncTimeout: null,
tasksEnabled: false,
timezone: 'automatic',
// user-defined Nextcloud settings
Expand Down Expand Up @@ -126,11 +128,12 @@ const mutations = {
* @param {Boolean} data.showWeekends Whether or not to display weekends
* @param {Boolean} data.skipPopover Whether or not to skip the simple event popover
* @param {String} data.slotDuration The duration of one slot in the agendaView
* @param {String} data.syncTimeout The timeout between fetching updates from the server
* @param {Boolean} data.talkEnabled Whether or not the talk app is enabled
* @param {Boolean} data.tasksEnabled Whether ot not the tasks app is enabled
* @param {String} data.timezone The timezone to view the calendar in. Either an Olsen timezone or "automatic"
*/
loadSettingsFromServer(state, { appVersion, eventLimit, firstRun, showWeekNumbers, showTasks, showWeekends, skipPopover, slotDuration, talkEnabled, tasksEnabled, timezone }) {
loadSettingsFromServer(state, { appVersion, eventLimit, firstRun, showWeekNumbers, showTasks, showWeekends, skipPopover, slotDuration, syncTimeout, talkEnabled, tasksEnabled, timezone }) {
logInfo(`
Initial settings:
- AppVersion: ${appVersion}
Expand All @@ -141,6 +144,7 @@ Initial settings:
- ShowWeekends: ${showWeekends}
- SkipPopover: ${skipPopover}
- SlotDuration: ${slotDuration}
- SyncTimeout: ${syncTimeout}
- TalkEnabled: ${talkEnabled}
- TasksEnabled: ${tasksEnabled}
- Timezone: ${timezone}
Expand All @@ -154,6 +158,7 @@ Initial settings:
state.showWeekends = showWeekends
state.skipPopover = skipPopover
state.slotDuration = slotDuration
state.syncTimeout = syncTimeout
state.talkEnabled = talkEnabled
state.tasksEnabled = tasksEnabled
state.timezone = timezone
Expand Down Expand Up @@ -186,6 +191,14 @@ const getters = {
getResolvedTimezone: (state) => state.timezone === 'automatic'
? detectTimezone()
: state.timezone,

/**
* Gets the sync timeout in milliseconds.
*
* @param {Object} state The Vuex state
* @returns {Integer}
*/
getSyncTimeout: (state) => moment.duration(state.syncTimeout).asMilliseconds(),
}

const actions = {
Expand Down
20 changes: 11 additions & 9 deletions src/views/Calendar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,6 @@ export default {
})
}
}, 1000 * 60)
this.checkForUpdatesJob = setInterval(async() => {
if (this.$route.name.startsWith('Public') || this.$route.name.startsWith('Embed')) {
const tokens = this.$route.params.tokens.split('-')
await this.$store.dispatch('syncPublicCalendars', { tokens })
} else {
await this.$store.dispatch('syncCalendars')
}
}, 1000 * 20)
},
destroy() {

Check warning on line 188 in src/views/Calendar.vue

View check run for this annotation

Codecov / codecov/patch

src/views/Calendar.vue#L188

Added line #L188 was not covered by tests
clearInterval(this.timeFrameCacheExpiryJob)
Expand All @@ -211,6 +202,7 @@ export default {
tasksEnabled: loadState('calendar', 'tasks_enabled'),
timezone: loadState('calendar', 'timezone'),
showTasks: loadState('calendar', 'show_tasks'),
syncTimeout: loadState('calendar', 'sync_timeout'),

Check warning on line 205 in src/views/Calendar.vue

View check run for this annotation

Codecov / codecov/patch

src/views/Calendar.vue#L205

Added line #L205 was not covered by tests
})
this.$store.dispatch('initializeCalendarJsConfig')
Expand Down Expand Up @@ -255,6 +247,16 @@ export default {
this.loadingCalendars = false
}
if (this.$store.getters.getSyncTimeout > 1000) {
this.checkForUpdatesJob = setInterval(async() => {

Check warning on line 251 in src/views/Calendar.vue

View check run for this annotation

Codecov / codecov/patch

src/views/Calendar.vue#L251

Added line #L251 was not covered by tests
if (this.$route.name.startsWith('Public') || this.$route.name.startsWith('Embed')) {
const tokens = this.$route.params.tokens.split('-')
await this.$store.dispatch('syncPublicCalendars', { tokens })
} else {
await this.$store.dispatch('syncCalendars')
}
}, this.$store.getters.getSyncTimeout)
}

Check warning on line 259 in src/views/Calendar.vue

View check run for this annotation

Codecov / codecov/patch

src/views/Calendar.vue#L259

Added line #L259 was not covered by tests
},
async mounted() {
if (this.timezone === 'automatic' && this.timezoneId === 'UTC') {
Expand Down
32 changes: 32 additions & 0 deletions tests/javascript/unit/store/settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('store/settings test suite', () => {
showWeekNumbers: null,
skipPopover: null,
slotDuration: null,
syncTimeout: null,
tasksEnabled: false,
timezone: 'automatic',
momentLocale: 'en',
Expand Down Expand Up @@ -151,6 +152,7 @@ describe('store/settings test suite', () => {
showWeekNumbers: null,
skipPopover: null,
slotDuration: null,
syncTimeout: null,
tasksEnabled: false,
timezone: 'automatic',
momentLocale: 'en',
Expand All @@ -166,6 +168,7 @@ describe('store/settings test suite', () => {
showWeekends: true,
skipPopover: true,
slotDuration: '00:30:00',
syncTimeout: 'PT1M',
talkEnabled: false,
tasksEnabled: true,
timezone: 'Europe/Berlin',
Expand All @@ -185,6 +188,7 @@ Initial settings:
- ShowWeekends: true
- SkipPopover: true
- SlotDuration: 00:30:00
- SyncTimeout: PT1M
- TalkEnabled: false
- TasksEnabled: true
- Timezone: Europe/Berlin
Expand All @@ -198,6 +202,7 @@ Initial settings:
showWeekends: true,
skipPopover: true,
slotDuration: '00:30:00',
syncTimeout: 'PT1M',
talkEnabled: false,
tasksEnabled: true,
timezone: 'Europe/Berlin',
Expand All @@ -217,6 +222,7 @@ Initial settings:
showWeekNumbers: null,
skipPopover: null,
slotDuration: null,
syncTimeout: null,
tasksEnabled: false,
timezone: 'automatic',
momentLocale: 'en',
Expand All @@ -237,6 +243,7 @@ Initial settings:
showWeekNumbers: null,
skipPopover: null,
slotDuration: null,
syncTimeout: null,
tasksEnabled: false,
timezone: 'automatic',
momentLocale: 'de',
Expand Down Expand Up @@ -267,6 +274,31 @@ Initial settings:
expect(detectTimezone).toHaveBeenCalledTimes(0)
})

it('should provide a getter for the sync timeout in milliseconds', () => {
const state = {
syncTimeout: '00:01:00'
}

expect(settingsStore.getters.getSyncTimeout(state)).toEqual(60 * 1000)
})

it('can parse sync timeouts in ISO 8601 format', () => {
const state = {
syncTimeout: 'PT1M'
}

expect(settingsStore.getters.getSyncTimeout(state)).toEqual(60 * 1000)
})

it('should handle invalid sync timeout gracefully', () => {
const state = {
syncTimeout: 'not a timeout at all'
}

// Check from Calendar.vue which will disable the updates:
expect(settingsStore.getters.getSyncTimeout(state) > 1000).toBeFalsy()
})

it('should provide an action to toggle the birthday calendar - enabled to disabled', async () => {
expect.assertions(3)

Expand Down
10 changes: 7 additions & 3 deletions tests/php/unit/Controller/PublicViewControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected function setUp():void {
}

public function testPublicIndexWithBranding():void {
$this->config->expects(self::exactly(9))
$this->config->expects(self::exactly(10))
->method('getAppValue')
->willReturnMap([
['calendar', 'eventLimit', 'yes', 'no'],
Expand All @@ -74,6 +74,7 @@ public function testPublicIndexWithBranding():void {
['calendar', 'timezone', 'automatic', 'defaultTimezone'],
['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
['calendar', 'showTasks', 'yes', 'yes'],
['calendar', 'syncTimeout', 'PT1M', 'defaultSyncTimeout'],
['calendar', 'installed_version', null, '1.0.0']
]);

Expand All @@ -99,7 +100,7 @@ public function testPublicIndexWithBranding():void {
->with('imagePath456')
->willReturn('absoluteImagePath456');

$this->initialStateService->expects(self::exactly(12))
$this->initialStateService->expects(self::exactly(13))
->method('provideInitialState')
->withConsecutive(
['calendar', 'app_version', '1.0.0'],
Expand All @@ -113,6 +114,7 @@ public function testPublicIndexWithBranding():void {
['calendar', 'timezone', 'defaultTimezone'],
['calendar', 'slot_duration', 'defaultSlotDuration'],
['calendar', 'show_tasks', true],
['calendar', 'sync_timeout', 'defaultSyncTimeout'],
['calendar', 'tasks_enabled', false]
);

Expand All @@ -139,6 +141,7 @@ public function testPublicIndexForEmbedding():void {
['calendar', 'timezone', 'automatic', 'defaultTimezone'],
['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
['calendar', 'showTasks', 'yes', 'defaultShowTasks'],
['calendar', 'syncTimeout', 'PT1M', 'defaultSyncTimeout'],
['calendar', 'installed_version', null, '1.0.0']
]);
$this->request->expects(self::once())
Expand All @@ -163,7 +166,7 @@ public function testPublicIndexForEmbedding():void {
->with('imagePath456')
->willReturn('absoluteImagePath456');

$this->initialStateService->expects(self::exactly(12))
$this->initialStateService->expects(self::exactly(13))
->method('provideInitialState')
->withConsecutive(
['calendar', 'app_version', '1.0.0'],
Expand All @@ -177,6 +180,7 @@ public function testPublicIndexForEmbedding():void {
['calendar', 'timezone', 'defaultTimezone'],
['calendar', 'slot_duration', 'defaultSlotDuration'],
['calendar', 'show_tasks', false],
['calendar', 'sync_timeout', 'defaultSyncTimeout'],
['calendar', 'tasks_enabled', false]
);

Expand Down
4 changes: 4 additions & 0 deletions tests/php/unit/Controller/ViewControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public function testIndex(): void {
['calendar', 'timezone', 'automatic', 'defaultTimezone'],
['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
['calendar', 'showTasks', 'yes', 'defaultShowTasks'],
['calendar', 'syncTimeout', 'PT1M', 'PT1M'],
['calendar', 'installed_version', null, '1.0.0'],
]);
$this->config
Expand Down Expand Up @@ -115,6 +116,7 @@ public function testIndex(): void {
['calendar', 'timezone', 'Europe/Berlin'],
['calendar', 'slot_duration', '00:15:00'],
['calendar', 'show_tasks', false],
['calendar', 'sync_timeout', 'PT1M'],
['calendar', 'tasks_enabled', true]
);

Expand Down Expand Up @@ -144,6 +146,7 @@ public function testIndexViewFix(string $savedView, string $expectedView): void
['calendar', 'timezone', 'automatic', 'defaultTimezone'],
['calendar', 'slotDuration', '00:30:00', 'defaultSlotDuration'],
['calendar', 'showTasks', 'yes', 'defaultShowTasks'],
['calendar', 'syncTimeout', 'PT1M', 'PT1M'],
['calendar', 'installed_version', null, '1.0.0'],
]);
$this->config
Expand Down Expand Up @@ -180,6 +183,7 @@ public function testIndexViewFix(string $savedView, string $expectedView): void
['calendar', 'timezone', 'Europe/Berlin'],
['calendar', 'slot_duration', '00:15:00'],
['calendar', 'show_tasks', false],
['calendar', 'sync_timeout', 'PT1M'],
['calendar', 'tasks_enabled', false]
);

Expand Down

0 comments on commit 56fc034

Please sign in to comment.