From 7a2a18a1d4bffedaf77ce4f610ed10ffe62587d5 Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Fri, 19 Jun 2020 12:42:29 +0200 Subject: [PATCH 1/7] Add background task manager --- .../BackgroundTasksProvider.test.tsx | 109 ++++++++++++++++++ .../BackgroundTasksProvider.tsx | 58 ++++++++++ src/containers/BackgroundTasks/context.ts | 7 ++ src/containers/BackgroundTasks/index.ts | 2 + src/containers/BackgroundTasks/tasks.ts | 42 +++++++ src/containers/BackgroundTasks/types.ts | 21 ++++ 6 files changed, 239 insertions(+) create mode 100644 src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx create mode 100644 src/containers/BackgroundTasks/BackgroundTasksProvider.tsx create mode 100644 src/containers/BackgroundTasks/context.ts create mode 100644 src/containers/BackgroundTasks/index.ts create mode 100644 src/containers/BackgroundTasks/tasks.ts create mode 100644 src/containers/BackgroundTasks/types.ts diff --git a/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx b/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx new file mode 100644 index 000000000..72c57cbae --- /dev/null +++ b/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx @@ -0,0 +1,109 @@ +import { renderHook } from "@testing-library/react-hooks"; + +import { + backgroundTasksRefreshTime, + useBackgroundTasks +} from "./BackgroundTasksProvider"; +import { Task, TaskData } from "./types"; + +jest.useFakeTimers(); + +describe("Background task provider", () => { + it("can queue a task", () => { + const onCompleted = jest.fn(); + const onError = jest.fn(); + + const { result } = renderHook(useBackgroundTasks); + + const taskId = result.current.queue(Task.CUSTOM, { + handle: () => true, + onCompleted, + onError + }); + expect(taskId).toBe(1); + expect(onCompleted).not.toHaveBeenCalled(); + expect(onError).not.toHaveBeenCalled(); + + jest.runOnlyPendingTimers(); + + expect(onCompleted).toHaveBeenCalled(); + expect(onError).not.toHaveBeenCalled(); + }); + + it("can handle task error", () => { + const onCompleted = jest.fn(); + const onError = jest.fn(); + + const { result } = renderHook(useBackgroundTasks); + + result.current.queue(Task.CUSTOM, { + handle: () => { + throw new Error("dummy error"); + }, + onCompleted, + onError + }); + + jest.runOnlyPendingTimers(); + + expect(onCompleted).not.toHaveBeenCalled(); + expect(onError).toHaveBeenCalled(); + }); + + it("can cancel task", () => { + const onCompleted = jest.fn(); + + const { result } = renderHook(useBackgroundTasks); + + const taskId = result.current.queue(Task.CUSTOM, { + handle: () => true, + onCompleted + }); + + jest.advanceTimersByTime(backgroundTasksRefreshTime - 1000); + result.current.cancel(taskId); + + jest.runOnlyPendingTimers(); + + expect(onCompleted).not.toHaveBeenCalled(); + }); + + it("can queue multiple tasks", () => { + const responses = [ + { + finished: false + }, + { + finished: false + } + ]; + + const tasks: TaskData[] = responses.map(response => ({ + handle: () => response.finished, + onCompleted: jest.fn() + })); + + const { result } = renderHook(useBackgroundTasks); + + tasks.forEach(task => result.current.queue(Task.CUSTOM, task)); + + jest.runOnlyPendingTimers(); + + expect(tasks[0].onCompleted).not.toHaveBeenCalled(); + expect(tasks[1].onCompleted).not.toHaveBeenCalled(); + + responses[0].finished = true; + + jest.runOnlyPendingTimers(); + + expect(tasks[0].onCompleted).toHaveBeenCalled(); + expect(tasks[1].onCompleted).not.toHaveBeenCalled(); + + responses[1].finished = true; + + jest.runOnlyPendingTimers(); + + expect(tasks[1].onCompleted).toHaveBeenCalled(); + expect(tasks[1].onCompleted).toHaveBeenCalled(); + }); +}); diff --git a/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx b/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx new file mode 100644 index 000000000..7a1b34007 --- /dev/null +++ b/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx @@ -0,0 +1,58 @@ +import React from "react"; + +import BackgroundTasksContext from "./context"; +import { handleTask, queueCustom } from "./tasks"; +import { QueuedTask, Task, TaskData } from "./types"; + +export const backgroundTasksRefreshTime = 15 * 1000; + +export function useBackgroundTasks() { + const idCounter = React.useRef(0); + const tasks = React.useRef([]); + + React.useEffect(() => { + const intervalId = setInterval(() => { + tasks.current = tasks.current.filter(task => !handleTask(task)); + }, backgroundTasksRefreshTime); + + return () => clearInterval(intervalId); + }); + + function cancel(id: number) { + tasks.current = tasks.current.filter(task => task.id !== id); + } + + function queue(type: Task, data?: TaskData) { + idCounter.current += 1; + + switch (type) { + case Task.CUSTOM: + queueCustom(idCounter.current, tasks, data); + } + + return idCounter.current; + } + + return { + cancel, + queue + }; +} + +const BackgroundTasksProvider: React.FC = ({ children }) => { + const { cancel, queue } = useBackgroundTasks(); + + return ( + + {children} + + ); +}; + +BackgroundTasksProvider.displayName = "BackgroundTasksProvider"; +export default BackgroundTasksProvider; diff --git a/src/containers/BackgroundTasks/context.ts b/src/containers/BackgroundTasks/context.ts new file mode 100644 index 000000000..985da8963 --- /dev/null +++ b/src/containers/BackgroundTasks/context.ts @@ -0,0 +1,7 @@ +import { createContext } from "React"; + +import { BackgroundTasksContextType } from "./types"; + +const BackgroundTasksContext = createContext(null); + +export default BackgroundTasksContext; diff --git a/src/containers/BackgroundTasks/index.ts b/src/containers/BackgroundTasks/index.ts new file mode 100644 index 000000000..3525dcf82 --- /dev/null +++ b/src/containers/BackgroundTasks/index.ts @@ -0,0 +1,2 @@ +export * from "./BackgroundTasksProvider"; +export { default } from "./BackgroundTasksProvider"; diff --git a/src/containers/BackgroundTasks/tasks.ts b/src/containers/BackgroundTasks/tasks.ts new file mode 100644 index 000000000..61a488e94 --- /dev/null +++ b/src/containers/BackgroundTasks/tasks.ts @@ -0,0 +1,42 @@ +import { QueuedTask, TaskData } from "./types"; + +export function handleTask(task: QueuedTask) { + let ok: boolean; + try { + ok = task.handle(); + } catch (error) { + task.onError(error); + } + + if (ok) { + task.onCompleted(); + } + + return ok; +} + +export function handleError(error: Error) { + throw error; +} + +export function queueCustom( + id: number, + tasks: React.MutableRefObject, + data: TaskData +) { + (["handle", "onCompleted"] as Array) + .filter(field => !data[field]) + .forEach(field => { + throw new Error(`${field} is required when creating custom task`); + }); + + tasks.current = [ + ...tasks.current, + { + handle: data.handle, + id, + onCompleted: data.onCompleted, + onError: data.onError || handleError + } + ]; +} diff --git a/src/containers/BackgroundTasks/types.ts b/src/containers/BackgroundTasks/types.ts new file mode 100644 index 000000000..719a6df2c --- /dev/null +++ b/src/containers/BackgroundTasks/types.ts @@ -0,0 +1,21 @@ +export enum Task { + CUSTOM +} + +export interface QueuedTask { + id: number; + handle: () => boolean; + onCompleted: () => void; + onError: (error: Error) => void; +} + +export interface TaskData { + handle?: () => boolean; + onCompleted?: () => void; + onError?: () => void; +} + +export interface BackgroundTasksContextType { + cancel: (id: number) => void; + queue: (type: Task, data?: TaskData) => void; +} From bdf90039b503da9ac6f21ce3c5c7269a92f489f7 Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Sun, 21 Jun 2020 17:31:33 +0200 Subject: [PATCH 2/7] Handle asynchronous tasks --- .../BackgroundTasksProvider.test.tsx | 100 ++++++++++++------ .../BackgroundTasksProvider.tsx | 31 +++++- src/containers/BackgroundTasks/tasks.ts | 18 ++-- src/containers/BackgroundTasks/types.ts | 9 +- 4 files changed, 110 insertions(+), 48 deletions(-) diff --git a/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx b/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx index 72c57cbae..23f29b4d5 100644 --- a/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx +++ b/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx @@ -9,77 +9,100 @@ import { Task, TaskData } from "./types"; jest.useFakeTimers(); describe("Background task provider", () => { - it("can queue a task", () => { + it("can queue a task", done => { + const handle = jest.fn, []>( + () => new Promise(resolve => resolve(true)) + ); const onCompleted = jest.fn(); const onError = jest.fn(); const { result } = renderHook(useBackgroundTasks); const taskId = result.current.queue(Task.CUSTOM, { - handle: () => true, + handle, onCompleted, onError }); expect(taskId).toBe(1); + expect(handle).not.toHaveBeenCalled(); expect(onCompleted).not.toHaveBeenCalled(); expect(onError).not.toHaveBeenCalled(); jest.runOnlyPendingTimers(); - expect(onCompleted).toHaveBeenCalled(); - expect(onError).not.toHaveBeenCalled(); + setImmediate(() => { + expect(handle).toHaveBeenCalled(); + expect(onCompleted).toHaveBeenCalled(); + expect(onError).not.toHaveBeenCalled(); + + done(); + }); }); - it("can handle task error", () => { + it("can handle task error", done => { + const handle = jest.fn, []>( + () => + new Promise(() => { + throw new Error("dummy error"); + }) + ); const onCompleted = jest.fn(); const onError = jest.fn(); const { result } = renderHook(useBackgroundTasks); result.current.queue(Task.CUSTOM, { - handle: () => { - throw new Error("dummy error"); - }, + handle, onCompleted, onError }); jest.runOnlyPendingTimers(); - expect(onCompleted).not.toHaveBeenCalled(); - expect(onError).toHaveBeenCalled(); + setImmediate(() => { + expect(handle).toHaveBeenCalled(); + expect(onCompleted).not.toHaveBeenCalled(); + expect(onError).toHaveBeenCalled(); + + done(); + }); }); - it("can cancel task", () => { + it("can cancel task", done => { const onCompleted = jest.fn(); const { result } = renderHook(useBackgroundTasks); const taskId = result.current.queue(Task.CUSTOM, { - handle: () => true, + handle: () => new Promise(resolve => resolve(true)), onCompleted }); - jest.advanceTimersByTime(backgroundTasksRefreshTime - 1000); + // Cancel task before executing it + jest.advanceTimersByTime(backgroundTasksRefreshTime * 0.9); result.current.cancel(taskId); jest.runOnlyPendingTimers(); - expect(onCompleted).not.toHaveBeenCalled(); + setImmediate(() => { + expect(onCompleted).not.toHaveBeenCalled(); + + done(); + }); }); - it("can queue multiple tasks", () => { - const responses = [ - { - finished: false - }, - { - finished: false - } + it("can queue multiple tasks", done => { + const responses: Array> = [ + new Promise(resolve => + setTimeout(() => resolve(true), backgroundTasksRefreshTime * 1.4) + ), + new Promise(resolve => + setTimeout(() => resolve(true), backgroundTasksRefreshTime * 2.1) + ) ]; const tasks: TaskData[] = responses.map(response => ({ - handle: () => response.finished, + handle: () => response, onCompleted: jest.fn() })); @@ -87,23 +110,30 @@ describe("Background task provider", () => { tasks.forEach(task => result.current.queue(Task.CUSTOM, task)); - jest.runOnlyPendingTimers(); + // Set time to backgroundTasksRefreshTime + jest.advanceTimersByTime(backgroundTasksRefreshTime + 100); - expect(tasks[0].onCompleted).not.toHaveBeenCalled(); - expect(tasks[1].onCompleted).not.toHaveBeenCalled(); + setImmediate(() => { + expect(tasks[0].onCompleted).not.toHaveBeenCalled(); + expect(tasks[1].onCompleted).not.toHaveBeenCalled(); - responses[0].finished = true; + // Set time to backgroundTasksRefreshTime * 2 + jest.advanceTimersByTime(backgroundTasksRefreshTime); - jest.runOnlyPendingTimers(); + setImmediate(() => { + expect(tasks[0].onCompleted).toHaveBeenCalled(); + expect(tasks[1].onCompleted).not.toHaveBeenCalled(); - expect(tasks[0].onCompleted).toHaveBeenCalled(); - expect(tasks[1].onCompleted).not.toHaveBeenCalled(); + // Set time to backgroundTasksRefreshTime * 3 + jest.advanceTimersByTime(backgroundTasksRefreshTime); - responses[1].finished = true; + setImmediate(() => { + expect(tasks[1].onCompleted).toHaveBeenCalled(); + expect(tasks[1].onCompleted).toHaveBeenCalled(); - jest.runOnlyPendingTimers(); - - expect(tasks[1].onCompleted).toHaveBeenCalled(); - expect(tasks[1].onCompleted).toHaveBeenCalled(); + done(); + }); + }); + }); }); }); diff --git a/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx b/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx index 7a1b34007..7199956d9 100644 --- a/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx +++ b/src/containers/BackgroundTasks/BackgroundTasksProvider.tsx @@ -2,7 +2,7 @@ import React from "react"; import BackgroundTasksContext from "./context"; import { handleTask, queueCustom } from "./tasks"; -import { QueuedTask, Task, TaskData } from "./types"; +import { QueuedTask, Task, TaskData, TaskStatus } from "./types"; export const backgroundTasksRefreshTime = 15 * 1000; @@ -12,7 +12,34 @@ export function useBackgroundTasks() { React.useEffect(() => { const intervalId = setInterval(() => { - tasks.current = tasks.current.filter(task => !handleTask(task)); + const queue = async () => { + tasks.current = tasks.current.filter( + task => task.status !== TaskStatus.ENDED + ); + try { + await Promise.all( + tasks.current.map(async task => { + let hasFinished: boolean; + + try { + hasFinished = await handleTask(task); + } catch (error) { + throw error; + } + if (hasFinished) { + const taskIndex = tasks.current.findIndex( + t => t.id === task.id + ); + tasks.current[taskIndex].status = TaskStatus.ENDED; + } + }) + ); + } catch (error) { + throw error; + } + }; + + queue(); }, backgroundTasksRefreshTime); return () => clearInterval(intervalId); diff --git a/src/containers/BackgroundTasks/tasks.ts b/src/containers/BackgroundTasks/tasks.ts index 61a488e94..5fea73b44 100644 --- a/src/containers/BackgroundTasks/tasks.ts +++ b/src/containers/BackgroundTasks/tasks.ts @@ -1,17 +1,16 @@ -import { QueuedTask, TaskData } from "./types"; +import { QueuedTask, TaskData, TaskStatus } from "./types"; -export function handleTask(task: QueuedTask) { - let ok: boolean; +export async function handleTask(task: QueuedTask): Promise { + let ok = false; try { - ok = task.handle(); + ok = await task.handle(); + if (ok) { + task.onCompleted(); + } } catch (error) { task.onError(error); } - if (ok) { - task.onCompleted(); - } - return ok; } @@ -36,7 +35,8 @@ export function queueCustom( handle: data.handle, id, onCompleted: data.onCompleted, - onError: data.onError || handleError + onError: data.onError || handleError, + status: TaskStatus.PENDING } ]; } diff --git a/src/containers/BackgroundTasks/types.ts b/src/containers/BackgroundTasks/types.ts index 719a6df2c..a4e328e0e 100644 --- a/src/containers/BackgroundTasks/types.ts +++ b/src/containers/BackgroundTasks/types.ts @@ -1,16 +1,21 @@ export enum Task { CUSTOM } +export enum TaskStatus { + PENDING, + ENDED +} export interface QueuedTask { id: number; - handle: () => boolean; + handle: () => Promise; + status: TaskStatus; onCompleted: () => void; onError: (error: Error) => void; } export interface TaskData { - handle?: () => boolean; + handle?: () => Promise; onCompleted?: () => void; onError?: () => void; } From e80ef22f0d78c8aa58d2a4a2f3a248921db94c86 Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Tue, 23 Jun 2020 14:31:27 +0200 Subject: [PATCH 3/7] Add context to project root --- src/hooks/useBackgroundTask.ts | 6 ++++++ src/index.tsx | 13 ++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 src/hooks/useBackgroundTask.ts diff --git a/src/hooks/useBackgroundTask.ts b/src/hooks/useBackgroundTask.ts new file mode 100644 index 000000000..eb477706f --- /dev/null +++ b/src/hooks/useBackgroundTask.ts @@ -0,0 +1,6 @@ +import BackgroundTasksContext from "@saleor/containers/BackgroundTasks/context"; +import { useContext } from "react"; + +const useBackgroundTask = useContext(BackgroundTasksContext); + +export default useBackgroundTask; diff --git a/src/index.tsx b/src/index.tsx index 45625ac19..2c30b3571 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -35,6 +35,7 @@ import { WindowTitle } from "./components/WindowTitle"; import { API_URI, APP_MOUNT_URI, GTM_ID } from "./config"; import ConfigurationSection, { createConfigurationMenu } from "./configuration"; import AppStateProvider from "./containers/AppState"; +import BackgroundTasksProvider from "./containers/BackgroundTasks"; import { CustomerSection } from "./customers"; import DiscountSection from "./discounts"; import HomePage from "./home"; @@ -135,11 +136,13 @@ const App: React.FC = () => { - - - - - + + + + + + + From c57598fb09c3351f508327e56ee39a1a5de436d4 Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Tue, 23 Jun 2020 14:33:11 +0200 Subject: [PATCH 4/7] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0982e9674..7598ac2d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ All notable, unreleased changes to this project will be documented in this file. - Update product stock management to newest design - #515 by @dominik-zeglen - Handle untracked products - #523 by @dominik-zeglen - Display correct error if there were no graphql errors - #525 by @dominik-zeglen +- Add background task manager - #574 by @dominik-zeglen ## 2.0.0 From 25e7ca8aa7328f92dc972df9c75007d1406d2d1c Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Tue, 23 Jun 2020 15:24:45 +0200 Subject: [PATCH 5/7] Fix useBackgroundTask hook --- src/hooks/useBackgroundTask.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hooks/useBackgroundTask.ts b/src/hooks/useBackgroundTask.ts index eb477706f..9ec45f163 100644 --- a/src/hooks/useBackgroundTask.ts +++ b/src/hooks/useBackgroundTask.ts @@ -1,6 +1,8 @@ import BackgroundTasksContext from "@saleor/containers/BackgroundTasks/context"; import { useContext } from "react"; -const useBackgroundTask = useContext(BackgroundTasksContext); +function useBackgroundTask() { + return useContext(BackgroundTasksContext); +} export default useBackgroundTask; From 6aa53c67759518eb74fce0d12c184526fe65fdff Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Tue, 23 Jun 2020 17:36:47 +0200 Subject: [PATCH 6/7] Fix case --- src/containers/BackgroundTasks/context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/containers/BackgroundTasks/context.ts b/src/containers/BackgroundTasks/context.ts index 985da8963..13f49e3ba 100644 --- a/src/containers/BackgroundTasks/context.ts +++ b/src/containers/BackgroundTasks/context.ts @@ -1,4 +1,4 @@ -import { createContext } from "React"; +import { createContext } from "react"; import { BackgroundTasksContextType } from "./types"; From 278be8a0bb3485e5e85293c7072e3c3333520bf1 Mon Sep 17 00:00:00 2001 From: dominik-zeglen Date: Fri, 26 Jun 2020 12:47:37 +0200 Subject: [PATCH 7/7] Fix index --- src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx b/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx index 23f29b4d5..51e20fb1f 100644 --- a/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx +++ b/src/containers/BackgroundTasks/BackgroundTasksProvider.test.tsx @@ -128,7 +128,7 @@ describe("Background task provider", () => { jest.advanceTimersByTime(backgroundTasksRefreshTime); setImmediate(() => { - expect(tasks[1].onCompleted).toHaveBeenCalled(); + expect(tasks[0].onCompleted).toHaveBeenCalled(); expect(tasks[1].onCompleted).toHaveBeenCalled(); done();