From 05e42b63fe73ec04f78a338442298d186373ecff Mon Sep 17 00:00:00 2001 From: Manuel Garbin Date: Thu, 25 Jul 2024 18:12:36 +0200 Subject: [PATCH 1/4] fix: fixed getPollingInterval func that doesn't match at all waitDisallowed var --- src/store/network/utils.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/store/network/utils.ts b/src/store/network/utils.ts index 5f9cf78b3..7153fd8bf 100644 --- a/src/store/network/utils.ts +++ b/src/store/network/utils.ts @@ -47,11 +47,13 @@ export const parsePollingInterval = (settings: AccountSettings): number => { * overridden by the server response/errors * @param res */ -export const getPollingInterval = (res: RawSoapResponse<{ waitDisallowed?: number }>): number => { +export const getPollingInterval = (res: RawSoapResponse<{ + NoOpResponse?: { waitDisallowed? : boolean } +}>): number => { const { pollingInterval } = useNetworkStore.getState(); const { settings } = useAccountStore.getState(); - const waitDisallowed = res.Body && !('Fault' in res.Body) && res.Body.waitDisallowed; - const fault = res.Body && 'Fault' in res.Body && res.Body.Fault; + const waitDisallowed = res?.Body && !('Fault' in res?.Body) && res?.Body?.NoOpResponse && res.Body?.NoOpResponse?.waitDisallowed; + const fault = res?.Body && 'Fault' in res?.Body && res?.Body?.Fault; if (fault) { return POLLING_RETRY_INTERVAL; } From 040177416b292b2daeb225f3eea1aebaaa8f8639 Mon Sep 17 00:00:00 2001 From: DocGarbo <36849503+mgarbin@users.noreply.github.com> Date: Wed, 21 Aug 2024 10:54:39 +0200 Subject: [PATCH 2/4] Update src/store/network/utils.ts Co-authored-by: Beatrice Guerra --- src/store/network/utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/store/network/utils.ts b/src/store/network/utils.ts index 7153fd8bf..678f552ab 100644 --- a/src/store/network/utils.ts +++ b/src/store/network/utils.ts @@ -52,8 +52,9 @@ export const getPollingInterval = (res: RawSoapResponse<{ }>): number => { const { pollingInterval } = useNetworkStore.getState(); const { settings } = useAccountStore.getState(); - const waitDisallowed = res?.Body && !('Fault' in res?.Body) && res?.Body?.NoOpResponse && res.Body?.NoOpResponse?.waitDisallowed; - const fault = res?.Body && 'Fault' in res?.Body && res?.Body?.Fault; + const waitDisallowed = + res.Body && !('Fault' in res.Body) && res.Body.NoOpResponse?.waitDisallowed; + const fault = res.Body && 'Fault' in res.Body && res.Body.Fault; if (fault) { return POLLING_RETRY_INTERVAL; } From a16264b0de4702d659dac38dd132c685646ba2d9 Mon Sep 17 00:00:00 2001 From: Manuel Garbin Date: Wed, 21 Aug 2024 12:30:07 +0200 Subject: [PATCH 3/4] chore: fixed source code format --- src/store/network/utils.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/store/network/utils.ts b/src/store/network/utils.ts index 678f552ab..1c6422883 100644 --- a/src/store/network/utils.ts +++ b/src/store/network/utils.ts @@ -47,9 +47,11 @@ export const parsePollingInterval = (settings: AccountSettings): number => { * overridden by the server response/errors * @param res */ -export const getPollingInterval = (res: RawSoapResponse<{ - NoOpResponse?: { waitDisallowed? : boolean } -}>): number => { +export const getPollingInterval = ( + res: RawSoapResponse<{ + NoOpResponse?: { waitDisallowed?: boolean }; + }> +): number => { const { pollingInterval } = useNetworkStore.getState(); const { settings } = useAccountStore.getState(); const waitDisallowed = From 42f80915fb0e8a50f9f8cf7c708eb30744866320 Mon Sep 17 00:00:00 2001 From: Beatrice Guerra Date: Wed, 21 Aug 2024 13:35:36 +0200 Subject: [PATCH 4/4] test: add test for getPollingInterval Refs: SHELL-239 --- src/store/network/utils.test.ts | 196 ++++++++++++++++++++++++++++++++ src/store/network/utils.ts | 11 +- src/types/network/index.ts | 4 + 3 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 src/store/network/utils.test.ts diff --git a/src/store/network/utils.test.ts b/src/store/network/utils.test.ts new file mode 100644 index 000000000..dd6abe544 --- /dev/null +++ b/src/store/network/utils.test.ts @@ -0,0 +1,196 @@ +/* + * SPDX-FileCopyrightText: 2024 Zextras + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { useNetworkStore } from './store'; +import { getPollingInterval } from './utils'; +import { JSNS } from '../../constants'; +import type { Duration, DurationUnit } from '../../types/account'; +import type { NoOpResponse, RawSoapResponse } from '../../types/network'; +import { useAccountStore } from '../account'; + +describe('Utils', () => { + describe('getPollingInterval', () => { + it('should return 10000 if the response is a NoOp with waitDisallowed set to true', () => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { ...state.settings.prefs, zimbraPrefMailPollingInterval: '500' } + } + })); + const noOpResponse = { + Header: { + context: {} + }, + Body: { + NoOpResponse: { + _jsns: JSNS.mail, + waitDisallowed: true + } + } + } satisfies RawSoapResponse<{ + NoOpResponse: NoOpResponse; + }>; + const result = getPollingInterval(noOpResponse); + expect(result).toBe(10000); + }); + + it('should return 60000 if the NoOp response includes a Fault', () => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { ...state.settings.prefs, zimbraPrefMailPollingInterval: '500' } + } + })); + const noOpResponse = { + Header: { + context: {} + }, + Body: { + Fault: { + Code: { Value: '' }, + Detail: { + Error: { + Code: '', + Trace: '' + } + }, + Reason: { + Text: '' + } + }, + NoOpResponse: { + _jsns: JSNS.mail, + waitDisallowed: true + } + } + } satisfies RawSoapResponse<{ NoOpResponse: NoOpResponse }>; + const result = getPollingInterval(noOpResponse); + expect(result).toBe(60000); + }); + describe('without Fault nor waitDisallowed', () => { + it('should return 30000 if zimbraPrefMailPollingInterval is not a valid duration', () => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { + ...state.settings.prefs, + zimbraPrefMailPollingInterval: 'invalid string' as Duration + } + } + })); + const response = { + Header: { + context: {} + }, + Body: {} + } satisfies RawSoapResponse>; + const result = getPollingInterval(response); + expect(result).toBe(30000); + }); + + // Characterization test: why the 500 absolute number is treated differently from the other absolute numbers? + it('should return 500 zimbraPrefMailPollingInterval is "500"', () => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { ...state.settings.prefs, zimbraPrefMailPollingInterval: '500' } + } + })); + const response = { + Header: { + context: {} + }, + Body: {} + } satisfies RawSoapResponse>; + const result = getPollingInterval(response); + expect(result).toBe(500); + }); + + it('should return the number * 1000 if zimbraPrefMailPollingInterval is set without a duration unit', () => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { + ...state.settings.prefs, + zimbraPrefMailPollingInterval: '753' satisfies Duration + } + } + })); + const response = { + Header: { + context: {} + }, + Body: {} + } satisfies RawSoapResponse>; + const result = getPollingInterval(response); + expect(result).toBe(753000); + }); + + // Characterization test: considering that the returned value is used in a timeout (milliseconds), + // the conversion from ms is wrong + it.each(['m', 'ms'])( + 'should return the number * 60 * 1000 if zimbraPrefMailPollingInterval duration is set with the duration unit %s', + (durationUnit) => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { + ...state.settings.prefs, + zimbraPrefMailPollingInterval: `5${durationUnit}` satisfies Duration + } + } + })); + const response = { + Header: { + context: {} + }, + Body: {} + } satisfies RawSoapResponse>; + const result = getPollingInterval(response); + expect(result).toBe(5 * 60 * 1000); + } + ); + + // Characterization test: considering that the returned value is used in a timeout (milliseconds), only the + // conversion from s is right, while the conversion from h and d are wrong + it.each(['s', 'h', 'd'])( + 'should return the number * 1000 if zimbraPrefMailPollingInterval is set with the duration unit %s', + (durationUnit) => { + useNetworkStore.setState({ pollingInterval: 123456789 }); + useAccountStore.setState((state) => ({ + ...state, + settings: { + ...state.settings, + prefs: { + ...state.settings.prefs, + zimbraPrefMailPollingInterval: `753${durationUnit}` satisfies Duration + } + } + })); + const response = { + Header: { + context: {} + }, + Body: {} + } satisfies RawSoapResponse>; + const result = getPollingInterval(response); + expect(result).toBe(753000); + } + ); + }); + }); +}); diff --git a/src/store/network/utils.ts b/src/store/network/utils.ts index 1c6422883..8721e585f 100644 --- a/src/store/network/utils.ts +++ b/src/store/network/utils.ts @@ -8,7 +8,7 @@ import { forEach } from 'lodash'; import { useNetworkStore } from './store'; import type { AccountSettings } from '../../types/account'; -import type { RawSoapResponse, SoapContext } from '../../types/network'; +import type { NoOpResponse, RawSoapResponse, SoapContext } from '../../types/network'; import { folderWorker, tagWorker } from '../../workers'; import { useAccountStore } from '../account'; import { useFolderStore } from '../folder'; @@ -45,14 +45,12 @@ export const parsePollingInterval = (settings: AccountSettings): number => { * Return the polling interval for the next NoOp request. * The interval length depends on the user settings, but it can be * overridden by the server response/errors - * @param res */ export const getPollingInterval = ( res: RawSoapResponse<{ - NoOpResponse?: { waitDisallowed?: boolean }; + NoOpResponse?: NoOpResponse; }> ): number => { - const { pollingInterval } = useNetworkStore.getState(); const { settings } = useAccountStore.getState(); const waitDisallowed = res.Body && !('Fault' in res.Body) && res.Body.NoOpResponse?.waitDisallowed; @@ -63,10 +61,7 @@ export const getPollingInterval = ( if (waitDisallowed) { return POLLING_NOWAIT_INTERVAL; } - if (!fault) { - return parsePollingInterval(settings); - } - return pollingInterval; + return parsePollingInterval(settings); }; export const handleSync = ({ refresh, notify }: SoapContext): Promise => diff --git a/src/types/network/index.ts b/src/types/network/index.ts index 592a8be70..6f3dde976 100644 --- a/src/types/network/index.ts +++ b/src/types/network/index.ts @@ -203,3 +203,7 @@ export type BatchResponse< > = SoapBody; export type NameSpace = ValueOf; + +export type NoOpResponse = SoapBody<{ + waitDisallowed?: boolean; +}>;