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

Increase refresh rate and fix duplicate api calls #97

Merged
merged 2 commits into from
Oct 19, 2023
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ junit.xml
# IDEs, IntelliJ
/.idea

# VSCode
/.vscode

/src/excluded

.env
Expand Down
111 changes: 62 additions & 49 deletions src/common/store/response.store.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { create } from 'zustand';

import { uploadFile, deleteFromLocation, fetchFromLocation, fetchWithRetries } from 'common/api';
import { deleteFromLocation, fetchFromLocation, fetchWithRetries, uploadFile } from 'common/api';
import { HTTP_STATUS_CODE } from 'common/constants';
import i18next from 'common/translations/i18n';
import { useGeometryStore } from './geometry.store';
import { useLayersStore } from './layers.store';
import { useLevelsStore } from './levels.store';
import { useGeometryStore } from './geometry.store';
import { useProgressBarStore } from './progress-bar-steps';
import { resetStores } from './reset';
import { useReviewManifestStore } from './review-manifest.store';
Expand All @@ -28,10 +28,13 @@ export const useResponseStore = create((set, get) => ({
operationLocation: '',
lroStatus: '',
errorMessage: '',
acknowledgeError: () => set({
errorMessage: ''
}),
uploadFile: (file) => {
refreshRunning: false,
acknowledgeError: () => {
set({
errorMessage: '',
});
},
uploadFile: file => {
set(() => ({
errorMessage: '',
lroStatus: LRO_STATUS.UPLOADING,
Expand All @@ -40,40 +43,48 @@ export const useResponseStore = create((set, get) => ({
useReviewManifestStore.getState().setOriginalPackage(file);

uploadFile(file)
.then(async (r) => {
if (r.status !== HTTP_STATUS_CODE.ACCEPTED) {
const data = await r.json();
const errorMessage = data?.error?.message;
if (errorMessage) {
throw new Error(errorMessage);
.then(async r => {
if (r.status !== HTTP_STATUS_CODE.ACCEPTED) {
const data = await r.json();
const errorMessage = data?.error?.message;
if (errorMessage) {
throw new Error(errorMessage);
}
throw new Error(i18next.t('error.upload.file'));
}
throw new Error(i18next.t('error.upload.file'));
}

// Once accepted by the backend, the response will contain the location of the operation
set(() => ({
lroStatus: LRO_STATUS.UPLOADED,
operationLocation: r.headers.get(OPERATION_LOCATION),
}));
})
.catch(({ message }) => {
const errorMsg = message === 'Failed to fetch' ? i18next.t('error.network.issue.cors') : message;
set(() => ({ errorMessage: errorMsg }));
});
// Once accepted by the backend, the response will contain the location of the operation
set(() => ({
lroStatus: LRO_STATUS.UPLOADED,
operationLocation: r.headers.get(OPERATION_LOCATION),
}));
})
.catch(({ message }) => {
const errorMsg = message === 'Failed to fetch' ? i18next.t('error.network.issue.cors') : message;
set(() => ({ errorMessage: errorMsg }));
});
},

// Poll the backend for the status of the upload operation
// Uses the operationLocation set by uploadFile()
refreshStatus: () => {
refreshStatus: async () => {
const operationLocation = get().operationLocation;
const lroStatus = get().lroStatus;

if (operationLocation === '' || lroStatus === LRO_STATUS.FETCHING_DATA || lroStatus === LRO_STATUS.SUCCEEDED) {
const refreshRunning = get().refreshRunning;

if (
operationLocation === '' ||
lroStatus === LRO_STATUS.FETCHING_DATA ||
lroStatus === LRO_STATUS.SUCCEEDED ||
refreshRunning
) {
return;
}

fetchFromLocation(operationLocation)
.then(async (r) => {
set({ refreshRunning: true });

await fetchFromLocation(operationLocation)
.then(async r => {
const data = await r.json();
if (r.status !== HTTP_STATUS_CODE.OK) {
const errorMessage = data?.error?.message;
Expand All @@ -88,7 +99,7 @@ export const useResponseStore = create((set, get) => ({
fetchUrl: r.headers.get(RESOURCE_LOCATION),
};
})
.then((data) => {
.then(async data => {
if (!data || !data.status) {
throw new Error(i18next.t('error.upload.file.processing'));
}
Expand All @@ -108,7 +119,8 @@ export const useResponseStore = create((set, get) => ({
set(() => ({
lroStatus: LRO_STATUS.FETCHING_DATA,
}));
get().fetchManifestData(data.fetchUrl);

await get().fetchManifestData(data.fetchUrl);
}
})
.catch(({ message }) => {
Expand All @@ -118,11 +130,13 @@ export const useResponseStore = create((set, get) => ({
lroStatus: LRO_STATUS.FAILED,
}));
});

set({ refreshRunning: false });
},

fetchManifestData: (fetchUrl) => {
fetchWithRetries(fetchUrl)
.then(async (data) => {
fetchManifestData: async fetchUrl => {
return fetchWithRetries(fetchUrl)
.then(async data => {
resetStores();

// Compute and store useful response data
Expand Down Expand Up @@ -159,11 +173,9 @@ export const useResponseStore = create((set, get) => ({
coordinates: data.anchorPoint,
});

useLayersStore.getState().setLayerNames(
Array.from(layerNames),
Array.from(polygonLayerNames),
Array.from(textLayerNames)
);
useLayersStore
.getState()
.setLayerNames(Array.from(layerNames), Array.from(polygonLayerNames), Array.from(textLayerNames));

useLevelsStore.getState().setLevels(data.drawings.map(drawing => drawing.fileName));

Expand All @@ -183,17 +195,13 @@ export const useResponseStore = create((set, get) => ({
useLayersStore.getState().setLayerFromManifestJson(jsonData.featureClasses);
useLayersStore.getState().setVisited();
useGeometryStore.setState({
dwgLayers: jsonData.dwgLayers.filter((layer) => polygonLayerNames.has(layer)),
dwgLayers: jsonData.dwgLayers.filter(layer => polygonLayerNames.has(layer)),
});
useGeometryStore.getState().updateAnchorPoint({
coordinates: [
jsonData.georeference.lon,
jsonData.georeference.lat,
],
coordinates: [jsonData.georeference.lon, jsonData.georeference.lat],
angle: jsonData.georeference.angle,
});
}

}

set(() => ({
Expand All @@ -203,7 +211,8 @@ export const useResponseStore = create((set, get) => ({
deleteFromLocation(fetchUrl);
})
.catch(({ message }) => {
const errorMsg = message === 'Failed to fetch' ? i18next.t('error.network.issue.cors') : message || defaultErrorMessage;
const errorMsg =
message === 'Failed to fetch' ? i18next.t('error.network.issue.cors') : message || defaultErrorMessage;
set(() => ({
errorMessage: errorMsg,
lroStatus: LRO_STATUS.FAILED,
Expand All @@ -228,7 +237,11 @@ export function parseManifestJson(json) {
if (!Array.isArray(json.featureClasses)) {
return null;
}
if (typeof json.georeference?.lon !== 'number' || typeof json.georeference?.lat !== 'number' || typeof json.georeference?.angle !== 'number') {
if (
typeof json.georeference?.lon !== 'number' ||
typeof json.georeference?.lat !== 'number' ||
typeof json.georeference?.angle !== 'number'
) {
return null;
}

Expand All @@ -241,7 +254,7 @@ export function parseManifestJson(json) {
};
}

export function getFirstMeaningfulError({ error = {}}) {
export function getFirstMeaningfulError({ error = {} }) {
if (error.message) {
return error.message;
}
Expand All @@ -268,4 +281,4 @@ export function getFirstMeaningfulError({ error = {}}) {
}

return null;
}
}
40 changes: 14 additions & 26 deletions src/pages/processing/processing.js
Original file line number Diff line number Diff line change
@@ -1,50 +1,41 @@
import { ProgressIndicator } from '@fluentui/react/lib/ProgressIndicator';
import { PATHS } from 'common';
import { progressBarStepsByKey, useCompletedSteps, useResponseStore, useUserStore } from 'common/store';
import { LRO_STATUS } from 'common/store/response.store';
import { useEffect, useState } from 'react';
import { useNavigate } from 'react-router-dom';
import { useTranslation } from 'react-i18next';
import { ProgressIndicator } from '@fluentui/react/lib/ProgressIndicator';
import { useNavigate } from 'react-router-dom';
import { shallow } from 'zustand/shallow';

import { containerStyle, progressIndicatorLabel, progressIndicatorStyles } from './processing.style';
import { PATHS } from 'common';
import { useResponseStore, useUserStore, useCompletedSteps, progressBarStepsByKey } from 'common/store';
import { LRO_STATUS } from 'common/store/response.store';

export const API_REQUEST_INTERVAL = 10; // in seconds
export const PAGE_REFRESH_INTERVAL = 1;
export const REFRESH_INTERVAL = 1;

const LABEL = {
UPLOADING: 'processing.label.uploading',
RUNNING: 'processing.label.processing',
};

const responseStoreSelector = (s) => [s.errorMessage, s.lroStatus, s.refreshStatus];
const subKeySelector = (s) => s.subscriptionKey;
const responseStoreSelector = s => [s.errorMessage, s.lroStatus, s.refreshStatus];
const subKeySelector = s => s.subscriptionKey;

const ProcessingPage = () => {
const { t } = useTranslation();
const navigate = useNavigate();
const completedSteps = useCompletedSteps();

const [lastUpdated, setLastUpdated] = useState(0);
const [label, setLabel] = useState();

const subscriptionKey = useUserStore(subKeySelector);
const [errorMessage, lroStatus, refreshStatus] = useResponseStore(responseStoreSelector, shallow);

useEffect(() => {
const interval = setInterval(() => {
setLastUpdated((seconds) => (seconds + PAGE_REFRESH_INTERVAL) % API_REQUEST_INTERVAL);
}, PAGE_REFRESH_INTERVAL * 1000);
refreshStatus(subscriptionKey);
}, REFRESH_INTERVAL * 1000);
return () => clearInterval(interval);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useEffect(() => {
if (lastUpdated !== API_REQUEST_INTERVAL - 1) return;
refreshStatus(subscriptionKey);
// eslint-disable-next-line react-hooks/exhaustive-deps -- we only care about lastUpdated
}, [lastUpdated]);

useEffect(() => {
switch (lroStatus) {
case LRO_STATUS.UPLOADING:
Expand All @@ -71,25 +62,22 @@ const ProcessingPage = () => {
navigate(PATHS.INDEX);
break;
}

setLastUpdated(0);
// eslint-disable-next-line react-hooks/exhaustive-deps
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [lroStatus]);

useEffect(() => {
if (errorMessage === '') {
return;
}
navigate(PATHS.INDEX);
// eslint-disable-next-line react-hooks/exhaustive-deps
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [errorMessage]);

return (
<div className={containerStyle}>
<ProgressIndicator label={t(label)} className={progressIndicatorStyles} styles={progressIndicatorLabel}
description={t('processing.last.checked', { seconds: lastUpdated })} />
<ProgressIndicator label={t(label)} className={progressIndicatorStyles} styles={progressIndicatorLabel} />
</div>
);
};

export default ProcessingPage;
export default ProcessingPage;
32 changes: 3 additions & 29 deletions src/pages/processing/processing.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { act, render } from '@testing-library/react';

import ProcessingPage, { API_REQUEST_INTERVAL, PAGE_REFRESH_INTERVAL} from './processing';
import { render } from '@testing-library/react';
import { useResponseStore, useUserStore } from 'common/store';
import ProcessingPage from './processing';

const refreshRateMs = PAGE_REFRESH_INTERVAL * 1000;
const requestFrequency = parseInt(API_REQUEST_INTERVAL / PAGE_REFRESH_INTERVAL);
const mockT = jest.fn();
const mockNavigate = jest.fn();

Expand All @@ -31,32 +28,9 @@ describe('ProcessingPage', () => {
expect(render(<ProcessingPage />)).toMatchSnapshot();
});

it('should countdown', async () => {
const state = useResponseStore.getState();
const spy = jest.spyOn(state, 'refreshStatus');
jest.useFakeTimers();
render(<ProcessingPage />);
for (let i = 0; i < requestFrequency; i++) {
expect(mockT).toHaveBeenCalledWith('processing.last.checked', { seconds: i });
act(() => { jest.advanceTimersByTime(refreshRateMs); });
}

expect(spy).toHaveBeenCalledWith('subscriptionKey');
expect(spy).toHaveBeenCalledTimes(1);

for (let i = 0; i < requestFrequency * 10; i++) {
expect(mockT).toHaveBeenCalledWith('processing.last.checked', { seconds: i % 10 });
act(() => { jest.advanceTimersByTime(refreshRateMs); });
}

expect(spy).toHaveBeenCalledTimes(11);

jest.useRealTimers();
});

it('should navigate to create manifest page in case of error', () => {
useResponseStore.setState({ errorMessage: 'erreur!' });
render(<ProcessingPage />);
expect(mockNavigate).toHaveBeenCalledWith('/');
});
});
});