From 13722eaf9ccaff716045a505b1d9489fe6829835 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Tue, 9 Apr 2019 20:05:51 -0700 Subject: [PATCH 01/16] Add tests for Result classes Signed-off-by: Mcat12 --- src/util/__tests__/result.test.tsx | 47 ++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/util/__tests__/result.test.tsx diff --git a/src/util/__tests__/result.test.tsx b/src/util/__tests__/result.test.tsx new file mode 100644 index 0000000..eb7cd71 --- /dev/null +++ b/src/util/__tests__/result.test.tsx @@ -0,0 +1,47 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Result class tests + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +import { Ok, Err } from "../result"; + +describe("Ok", () => { + const testValue = "test"; + const ok = new Ok(testValue); + + it("knows it's Ok", () => { + expect(ok.isOk()).toBe(true); + expect(ok.isErr()).toBe(false); + }); + + it("unwraps without error", () => { + expect(ok.unwrap()).toEqual(testValue); + }); + + it("throws an error on unwrapErr", () => { + expect(() => ok.unwrapErr()).toThrow("unwrapErr on a Result.Ok"); + }); +}); + +describe("Err", () => { + const testValue = "test"; + const err = new Err(testValue); + + it("knows it's Err", () => { + expect(err.isOk()).toBe(false); + expect(err.isErr()).toBe(true); + }); + + it("throws an error on unwrap", () => { + expect(() => err.unwrap()).toThrow("unwrap on a Result.Err"); + }); + + it("does not throw an error on unwrapErr", () => { + expect(err.unwrapErr()).toEqual(testValue); + }); +}); From cf4affc230fcde28bc5ec4f458965a05cb5522e6 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Tue, 9 Apr 2019 20:06:12 -0700 Subject: [PATCH 02/16] Fix description in license of API service tests Signed-off-by: Mcat12 --- src/util/__tests__/api.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/__tests__/api.test.tsx b/src/util/__tests__/api.test.tsx index 72b6ae3..01d7215 100644 --- a/src/util/__tests__/api.test.tsx +++ b/src/util/__tests__/api.test.tsx @@ -3,7 +3,7 @@ * Network-wide ad blocking via your own hardware. * * Web Interface - * Utility function tests + * API service tests * * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ From fde34be4a33b1594ed3f3053ec83b5a7dcbfa193 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Tue, 9 Apr 2019 20:55:02 -0700 Subject: [PATCH 03/16] Move cancelable promises to new file and add tests Signed-off-by: Mcat12 --- src/components/common/EnableDisable.tsx | 5 +- src/components/common/WithAPIData.tsx | 6 +- src/components/list/ListPage.tsx | 6 +- src/components/log/QueryLog.tsx | 12 +-- src/components/settings/DHCPInfo.tsx | 6 +- src/components/settings/DNSInfo.tsx | 6 +- .../settings/PreferenceSettings.tsx | 6 +- src/util/CancelablePromise.tsx | 95 ++++++++++++++++ src/util/__tests__/CancelablePromise.test.tsx | 102 ++++++++++++++++++ src/util/index.tsx | 74 ------------- 10 files changed, 232 insertions(+), 86 deletions(-) create mode 100644 src/util/CancelablePromise.tsx create mode 100644 src/util/__tests__/CancelablePromise.test.tsx diff --git a/src/components/common/EnableDisable.tsx b/src/components/common/EnableDisable.tsx index 2ceeb7b..bd1017d 100644 --- a/src/components/common/EnableDisable.tsx +++ b/src/components/common/EnableDisable.tsx @@ -15,7 +15,10 @@ import { WithNamespaces, withNamespaces } from "react-i18next"; import NavButton from "./NavButton"; import NavDropdown from "./NavDropdown"; import { StatusContext } from "./context/StatusContext"; -import { CancelablePromise, makeCancelable } from "../../util"; +import { + CancelablePromise, + makeCancelable +} from "../../util/CancelablePromise"; import api from "../../util/api"; import { Button, diff --git a/src/components/common/WithAPIData.tsx b/src/components/common/WithAPIData.tsx index 3e72e0a..2a165b3 100644 --- a/src/components/common/WithAPIData.tsx +++ b/src/components/common/WithAPIData.tsx @@ -9,7 +9,11 @@ * Please see LICENSE file for your rights under this license. */ import { Component, ReactNode } from "react"; -import { CancelablePromise, ignoreCancel, makeCancelable } from "../../util"; +import { + CancelablePromise, + ignoreCancel, + makeCancelable +} from "../../util/CancelablePromise"; import { Err, Ok, Result } from "../../util/result"; export interface WithAPIDataProps { diff --git a/src/components/list/ListPage.tsx b/src/components/list/ListPage.tsx index 512b838..336beca 100644 --- a/src/components/list/ListPage.tsx +++ b/src/components/list/ListPage.tsx @@ -13,7 +13,11 @@ import { WithNamespaces, withNamespaces } from "react-i18next"; import DomainInput from "./DomainInput"; import Alert, { AlertType } from "../common/Alert"; import DomainList from "./DomainList"; -import { CancelablePromise, ignoreCancel, makeCancelable } from "../../util"; +import { + CancelablePromise, + ignoreCancel, + makeCancelable +} from "../../util/CancelablePromise"; export interface ListPageProps extends WithNamespaces { title: string; diff --git a/src/components/log/QueryLog.tsx b/src/components/log/QueryLog.tsx index a2b9421..b746ca1 100644 --- a/src/components/log/QueryLog.tsx +++ b/src/components/log/QueryLog.tsx @@ -20,18 +20,18 @@ import i18next from "i18next"; import { WithNamespaces, withNamespaces } from "react-i18next"; import debounce from "lodash.debounce"; import moment from "moment"; -import { - CancelablePromise, - ignoreCancel, - makeCancelable, - padNumber -} from "../../util"; +import { padNumber } from "../../util"; import api from "../../util/api"; import { dateRanges } from "../../util/dateRanges"; import { TranslatedTimeRangeSelector } from "../dashboard/TimeRangeSelector"; import { TimeRange } from "../common/context/TimeRangeContext"; import "react-table/react-table.css"; import "bootstrap-daterangepicker/daterangepicker.css"; +import { + CancelablePromise, + ignoreCancel, + makeCancelable +} from "../../util/CancelablePromise"; export interface QueryLogState { history: Array; diff --git a/src/components/settings/DHCPInfo.tsx b/src/components/settings/DHCPInfo.tsx index 85aeece..5d00a17 100644 --- a/src/components/settings/DHCPInfo.tsx +++ b/src/components/settings/DHCPInfo.tsx @@ -10,7 +10,11 @@ import React, { ChangeEvent, Component, FormEvent } from "react"; import { WithNamespaces, withNamespaces } from "react-i18next"; -import { CancelablePromise, ignoreCancel, makeCancelable } from "../../util"; +import { + CancelablePromise, + ignoreCancel, + makeCancelable +} from "../../util/CancelablePromise"; import api from "../../util/api"; import { Button, diff --git a/src/components/settings/DNSInfo.tsx b/src/components/settings/DNSInfo.tsx index f2a4d15..0d32967 100644 --- a/src/components/settings/DNSInfo.tsx +++ b/src/components/settings/DNSInfo.tsx @@ -10,7 +10,11 @@ import React, { Component, FormEvent } from "react"; import { WithNamespaces, withNamespaces } from "react-i18next"; -import { CancelablePromise, ignoreCancel, makeCancelable } from "../../util"; +import { + CancelablePromise, + ignoreCancel, + makeCancelable +} from "../../util/CancelablePromise"; import api from "../../util/api"; import DnsList from "./DnsList"; import { Button, Col, Form, FormGroup } from "reactstrap"; diff --git a/src/components/settings/PreferenceSettings.tsx b/src/components/settings/PreferenceSettings.tsx index 22ec7c3..3fbc6c8 100644 --- a/src/components/settings/PreferenceSettings.tsx +++ b/src/components/settings/PreferenceSettings.tsx @@ -10,7 +10,11 @@ import React, { ChangeEvent, Component, FormEvent } from "react"; import { WithNamespaces, withNamespaces } from "react-i18next"; -import { CancelablePromise, ignoreCancel, makeCancelable } from "../../util"; +import { + CancelablePromise, + ignoreCancel, + makeCancelable +} from "../../util/CancelablePromise"; import api from "../../util/api"; import Alert, { AlertType } from "../common/Alert"; import { Button, Col, Form, FormGroup, Input, Label } from "reactstrap"; diff --git a/src/util/CancelablePromise.tsx b/src/util/CancelablePromise.tsx new file mode 100644 index 0000000..1fa2d12 --- /dev/null +++ b/src/util/CancelablePromise.tsx @@ -0,0 +1,95 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Wrap promises to make them cancelable + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +/** + * A promise which can be canceled + */ +export interface CancelablePromise { + promise: Promise; + cancel: () => void; +} + +/** + * The options given to {@link makeCancelable} + */ +export interface CancelableOptions { + /** + * The function to call to repeat the promise + */ + repeat: () => void; + + /** + * The amount of time to wait until repeating + */ + interval: number; +} + +/** + * Make a promise cancelable and repeatable + * + * @param promise the promise + * @param options the interval repeat options + * @returns a cancelable promise + */ +export function makeCancelable( + promise: Promise, + options?: CancelableOptions +): CancelablePromise { + let hasCanceled = false; + let repeatId: NodeJS.Timeout | null = null; + + const handle = ( + resolve: (value: T) => void, + reject: (error: any) => void, + val: T, + isError: boolean + ) => { + if (hasCanceled) { + reject({ isCanceled: true }); + return; + } + + if (isError) { + reject(val); + } else { + resolve(val); + } + + if (options) { + repeatId = setTimeout(options.repeat, options.interval); + } + }; + + const wrappedPromise: Promise = new Promise((resolve, reject) => { + promise.then( + val => handle(resolve, reject, val, false), + error => handle(resolve, reject, error, true) + ); + }); + + return { + promise: wrappedPromise, + cancel() { + if (repeatId !== null) { + clearTimeout(repeatId); + } + hasCanceled = true; + } + }; +} + +/** + * Ignore canceled promises (pass into a promise's catch function) + * + * @param err the error from catching the promise + */ +export const ignoreCancel = (err: any) => { + if (!err.isCanceled) throw err; +}; diff --git a/src/util/__tests__/CancelablePromise.test.tsx b/src/util/__tests__/CancelablePromise.test.tsx new file mode 100644 index 0000000..77ef425 --- /dev/null +++ b/src/util/__tests__/CancelablePromise.test.tsx @@ -0,0 +1,102 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Tests for canceling promises + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +import { makeCancelable, ignoreCancel } from "../CancelablePromise"; + +describe("makeCancelable", () => { + const testValue = "test"; + const testError = "testError"; + const promise = Promise.resolve(testValue); + const promiseErr = Promise.reject(testError); + + it("passes through promises in the default case", async () => { + const cancelablePromise = makeCancelable(promise); + + await expect(cancelablePromise.promise).resolves.toEqual(testValue); + }); + + it("passes through errors in the default case", async () => { + const cancelablePromise = makeCancelable(promiseErr); + + await expect(cancelablePromise.promise).rejects.toEqual(testError); + }); + + it("rejects with cancel error if canceled", async () => { + const cancelablePromise = makeCancelable(promise); + + cancelablePromise.cancel(); + + await expect(cancelablePromise.promise).rejects.toEqual({ + isCanceled: true + }); + }); + + it("calls the repeat function after resolving", async () => { + jest.useFakeTimers(); + + const mockFunction = jest.fn(); + const interval = 1000; + const cancelablePromise = makeCancelable(promise, { + interval, + repeat: mockFunction + }); + + await expect(cancelablePromise.promise).resolves; + + expect(setTimeout).toHaveBeenCalledWith(mockFunction, interval); + }); + + it("calls the repeat function after rejecting", async () => { + jest.useFakeTimers(); + + const mockFunction = jest.fn(); + const interval = 1000; + const cancelablePromise = makeCancelable(promiseErr, { + interval, + repeat: mockFunction + }); + + await expect(cancelablePromise.promise).rejects.toEqual(testError); + + expect(setTimeout).toHaveBeenCalledWith(mockFunction, interval); + }); + + it("clears the timeout if canceled after resolving", async () => { + jest.useFakeTimers(); + + const mockFunction = jest.fn(); + const interval = 1000; + const cancelablePromise = makeCancelable(promise, { + interval, + repeat: mockFunction + }); + + await cancelablePromise.promise; + + cancelablePromise.cancel(); + + expect(clearTimeout).toHaveBeenCalled(); + }); +}); + +describe("ignoreCancel", () => { + it("passes through non-canceled errors", async () => { + const testError = "test"; + const promise = Promise.reject(testError); + + await expect(promise.catch(ignoreCancel)).rejects.toEqual(testError); + }); + + it("does not pass through canceled errors", async () => { + const canceledPromise = Promise.reject({ isCanceled: true }); + + await expect(canceledPromise.catch(ignoreCancel)).resolves; + }); +}); diff --git a/src/util/index.tsx b/src/util/index.tsx index b16364d..d339154 100644 --- a/src/util/index.tsx +++ b/src/util/index.tsx @@ -29,77 +29,3 @@ export const padNumber = (num: number) => { export const getIntervalForRange = (range: TimeRange): number => { return Math.ceil((range.until.unix() - range.from.unix()) / 144); }; - -export interface CancelablePromise { - promise: Promise; - cancel: () => void; -} - -export interface CancelableOptions { - /** - * The function to call to repeat the promise - */ - repeat: () => void; - - /** - * The amount of time to wait until repeating - */ - interval: number; -} - -/** - * Make a promise cancelable and repeatable - * - * @param promise the promise - * @param options the interval repeat options - * @returns {{promise: Promise, cancel(): void}} a handle on the cancelable - * promise - */ -export function makeCancelable( - promise: Promise, - options?: CancelableOptions -): CancelablePromise { - let hasCanceled = false; - let repeatId: NodeJS.Timeout | null = null; - - const handle = ( - resolve: (value: any) => void, - reject: (error: any) => void, - val: T, - isError: boolean - ) => { - if (hasCanceled) reject({ isCanceled: true }); - else { - if (isError) reject(val); - else resolve(val); - - if (options) repeatId = setTimeout(options.repeat, options.interval); - } - }; - - const wrappedPromise: Promise = new Promise((resolve, reject) => { - promise.then( - val => handle(resolve, reject, val, false), - error => handle(resolve, reject, error, true) - ); - }); - - return { - promise: wrappedPromise, - cancel() { - if (repeatId !== null) { - clearTimeout(repeatId); - } - hasCanceled = true; - } - }; -} - -/** - * Ignore canceled promises (pass into a promise's catch function) - * - * @param err the error from catching the promise - */ -export const ignoreCancel = (err: any) => { - if (!err.isCanceled) throw err; -}; From e31ed2f1af8b09a52f853bbd4c9446c6ddb24037 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Tue, 9 Apr 2019 21:08:15 -0700 Subject: [PATCH 04/16] Move graph utils to new file and add tests Signed-off-by: Mcat12 --- src/components/dashboard/ClientsGraph.tsx | 2 +- src/components/dashboard/QueriesGraph.tsx | 2 +- src/components/log/QueryLog.tsx | 2 +- src/util/__tests__/graphUtils.test.tsx | 51 +++++++++++++++++++++++ src/util/{index.tsx => graphUtils.tsx} | 2 +- 5 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 src/util/__tests__/graphUtils.test.tsx rename src/util/{index.tsx => graphUtils.tsx} (97%) diff --git a/src/components/dashboard/ClientsGraph.tsx b/src/components/dashboard/ClientsGraph.tsx index 17837ae..3ad42f2 100644 --- a/src/components/dashboard/ClientsGraph.tsx +++ b/src/components/dashboard/ClientsGraph.tsx @@ -12,7 +12,7 @@ import React, { Component, RefObject } from "react"; import ReactDOM from "react-dom"; import { Line } from "react-chartjs-2"; import { WithNamespaces, withNamespaces } from "react-i18next"; -import { getIntervalForRange, padNumber } from "../../util"; +import { getIntervalForRange, padNumber } from "../../util/graphUtils"; import api from "../../util/api"; import ChartTooltip from "./ChartTooltip"; import { WithAPIData } from "../common/WithAPIData"; diff --git a/src/components/dashboard/QueriesGraph.tsx b/src/components/dashboard/QueriesGraph.tsx index e321bb2..759bfb9 100644 --- a/src/components/dashboard/QueriesGraph.tsx +++ b/src/components/dashboard/QueriesGraph.tsx @@ -10,7 +10,7 @@ import React, { Component } from "react"; import { WithNamespaces, withNamespaces } from "react-i18next"; -import { getIntervalForRange, padNumber } from "../../util"; +import { getIntervalForRange, padNumber } from "../../util/graphUtils"; import api from "../../util/api"; import { WithAPIData } from "../common/WithAPIData"; import { ChartData, ChartOptions, TimeUnit } from "chart.js"; diff --git a/src/components/log/QueryLog.tsx b/src/components/log/QueryLog.tsx index b746ca1..16aa3fb 100644 --- a/src/components/log/QueryLog.tsx +++ b/src/components/log/QueryLog.tsx @@ -20,7 +20,7 @@ import i18next from "i18next"; import { WithNamespaces, withNamespaces } from "react-i18next"; import debounce from "lodash.debounce"; import moment from "moment"; -import { padNumber } from "../../util"; +import { padNumber } from "../../util/graphUtils"; import api from "../../util/api"; import { dateRanges } from "../../util/dateRanges"; import { TranslatedTimeRangeSelector } from "../dashboard/TimeRangeSelector"; diff --git a/src/util/__tests__/graphUtils.test.tsx b/src/util/__tests__/graphUtils.test.tsx new file mode 100644 index 0000000..3258910 --- /dev/null +++ b/src/util/__tests__/graphUtils.test.tsx @@ -0,0 +1,51 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Graph utility tests + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +import { padNumber, getIntervalForRange } from "../graphUtils"; +import { TimeRange } from "../../components/common/context/TimeRangeContext"; +import moment from "moment"; + +describe("padNumber", () => { + it("pads 0 to 00", () => { + expect(padNumber(0)).toEqual("00"); + }); + + it("pads 1 to 01", () => { + expect(padNumber(1)).toEqual("01"); + }); + + it("pads 12 to 12", () => { + expect(padNumber(12)).toEqual("12"); + }); +}); + +describe("getIntervalForRange", () => { + it("returns 10 minutes for 24 hours", () => { + const range: TimeRange = { + name: "24 Hours", + from: moment().subtract(1, "day"), + until: moment() + }; + + expect(getIntervalForRange(range)).toEqual(10 * 60); + }); + + it("returns 1 day for 144 days", () => { + const range: TimeRange = { + name: "144 days", + // Use seconds instead of days to ensure the difference in epoch time is + // equal to 144 days + from: moment().subtract(144 * 24 * 60 * 60, "seconds"), + until: moment() + }; + + expect(getIntervalForRange(range)).toEqual(24 * 60 * 60); + }); +}); diff --git a/src/util/index.tsx b/src/util/graphUtils.tsx similarity index 97% rename from src/util/index.tsx rename to src/util/graphUtils.tsx index d339154..b49c4e8 100644 --- a/src/util/index.tsx +++ b/src/util/graphUtils.tsx @@ -3,7 +3,7 @@ * Network-wide ad blocking via your own hardware. * * Web Interface - * Various utilities + * Graph utility functions * * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ From f79316e235de0ae9b3482a408368f8b105f28203 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Wed, 10 Apr 2019 20:46:44 -0700 Subject: [PATCH 05/16] Add tests for checkIfLoggedOut Signed-off-by: Mcat12 --- src/util/__tests__/http.test.tsx | 52 ++++++++++++++++++++++++++++++++ src/util/__tests__/mockWindow.ts | 33 ++++++++++++++++++++ src/util/http.tsx | 2 +- 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 src/util/__tests__/http.test.tsx create mode 100644 src/util/__tests__/mockWindow.ts diff --git a/src/util/__tests__/http.test.tsx b/src/util/__tests__/http.test.tsx new file mode 100644 index 0000000..e6acd5c --- /dev/null +++ b/src/util/__tests__/http.test.tsx @@ -0,0 +1,52 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Test basic HTTP functions + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +import { checkIfLoggedOut } from "../http"; +import api from "../api"; +import { mockLocationReload, restoreLocationReload } from "./mockWindow"; + +describe("checkIfLoggedOut", () => { + it("passes the response through if logged in and not a 401", async () => { + const response = { status: 200 } as Response; + api.loggedIn = true; + + await expect(checkIfLoggedOut(response)).resolves.toEqual(response); + }); + + it("passes the response through if not logged in and not a 401", async () => { + const response = { status: 200 } as Response; + api.loggedIn = false; + + await expect(checkIfLoggedOut(response)).resolves.toEqual(response); + }); + + it("passes the response through if not logged in and is a 401", async () => { + const response = { status: 401 } as Response; + api.loggedIn = false; + + await expect(checkIfLoggedOut(response)).resolves.toEqual(response); + }); + + it("clears the session cookie and reloads if logged in and response is a 401", async () => { + const response = { status: 401 } as Response; + api.loggedIn = true; + document.cookie = "user_id=test"; + mockLocationReload(); + + await expect(checkIfLoggedOut(response)).rejects.toEqual({ + isCanceled: true + }); + + expect(window.location.reload).toHaveBeenCalled(); + expect(document.cookie).toHaveLength(0); + + restoreLocationReload(); + }); +}); diff --git a/src/util/__tests__/mockWindow.ts b/src/util/__tests__/mockWindow.ts new file mode 100644 index 0000000..c4bf557 --- /dev/null +++ b/src/util/__tests__/mockWindow.ts @@ -0,0 +1,33 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Functions for mocking window attributes + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +const originalReload = window.location.reload; + +/** + * Mock window.location.reload + * https://remarkablemark.org/blog/2018/11/17/mock-window-location/ + * + * Restore it with {@link restoreLocationReload} + */ +export const mockLocationReload = () => { + Object.defineProperty(window.location, "reload", { + configurable: true + }); + + window.location.reload = jest.fn(); +}; + +/** + * Restore window.location.reload after being mocked by + * {@link mockLocationReload} + */ +export const restoreLocationReload = () => { + window.location.reload = originalReload; +}; diff --git a/src/util/http.tsx b/src/util/http.tsx index 6c3234f..787a3e8 100644 --- a/src/util/http.tsx +++ b/src/util/http.tsx @@ -100,7 +100,7 @@ export default { * @param response the Response from fetch * @return {Promise} if logged in, the response, otherwise a canceled promise */ -const checkIfLoggedOut = (response: Response) => { +export const checkIfLoggedOut = (response: Response) => { if (api.loggedIn && response.status === 401) { // Clear the user's old session and refresh the page document.cookie = From 3f54f321a89f56591ce31ff56151740c4c9d79a2 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Thu, 11 Apr 2019 21:43:46 -0700 Subject: [PATCH 06/16] Convert http methods and api methods into classes This will make it easier to test them. Some other included changes: - Added/fixed return types on some API methods and their usages. - Added tests for convertJSON, checkForErrors, and urlFor. - Added apiPath to the config. This stores the API's base path. Signed-off-by: Mcat12 --- src/components/common/EnableDisable.tsx | 2 +- src/components/settings/DHCPInfo.tsx | 2 +- src/components/settings/DNSInfo.tsx | 2 +- .../settings/PreferenceSettings.tsx | 2 +- src/config.development.tsx | 3 +- src/config.production.tsx | 3 +- src/config.tsx | 1 + src/util/__tests__/http.test.tsx | 104 +++++- src/util/__tests__/mockWindow.ts | 33 -- src/util/api.tsx | 299 +++++++++++------- src/util/http.tsx | 121 ++++--- src/views/Login.tsx | 9 +- 12 files changed, 345 insertions(+), 236 deletions(-) delete mode 100644 src/util/__tests__/mockWindow.ts diff --git a/src/components/common/EnableDisable.tsx b/src/components/common/EnableDisable.tsx index bd1017d..266aba7 100644 --- a/src/components/common/EnableDisable.tsx +++ b/src/components/common/EnableDisable.tsx @@ -52,7 +52,7 @@ class EnableDisable extends Component { customMultiplier: 60 }; - private updateHandler: CancelablePromise | undefined; + private updateHandler: CancelablePromise | undefined; /** * Convert a status action into a status. ex. "enable" -> "enabled" diff --git a/src/components/settings/DHCPInfo.tsx b/src/components/settings/DHCPInfo.tsx index 5d00a17..d65d124 100644 --- a/src/components/settings/DHCPInfo.tsx +++ b/src/components/settings/DHCPInfo.tsx @@ -55,7 +55,7 @@ class DHCPInfo extends Component { }; private loadHandler: undefined | CancelablePromise; - private updateHandler: undefined | CancelablePromise; + private updateHandler: undefined | CancelablePromise; loadDHCPInfo = () => { this.loadHandler = makeCancelable(api.getDHCPInfo()); diff --git a/src/components/settings/DNSInfo.tsx b/src/components/settings/DNSInfo.tsx index 0d32967..a6ca83e 100644 --- a/src/components/settings/DNSInfo.tsx +++ b/src/components/settings/DNSInfo.tsx @@ -56,7 +56,7 @@ class DNSInfo extends Component { }; private loadHandler: undefined | CancelablePromise; - private updateHandler: undefined | CancelablePromise; + private updateHandler: undefined | CancelablePromise; loadDNSInfo = () => { this.loadHandler = makeCancelable(api.getDNSInfo()); diff --git a/src/components/settings/PreferenceSettings.tsx b/src/components/settings/PreferenceSettings.tsx index 3fbc6c8..4a6d242 100644 --- a/src/components/settings/PreferenceSettings.tsx +++ b/src/components/settings/PreferenceSettings.tsx @@ -52,7 +52,7 @@ class PreferenceSettings extends Component< }; private loadHandler: undefined | CancelablePromise; - private updateHandler: undefined | CancelablePromise; + private updateHandler: undefined | CancelablePromise; loadPreferences = () => { this.loadHandler = makeCancelable(api.getPreferences()); diff --git a/src/config.development.tsx b/src/config.development.tsx index 3bfa9cf..4d08a3f 100644 --- a/src/config.development.tsx +++ b/src/config.development.tsx @@ -12,5 +12,6 @@ import { Config } from "./config"; export default { developmentMode: true, - fakeAPI: false + fakeAPI: false, + apiPath: process.env.PUBLIC_URL + "/fakeAPI" } as Config; diff --git a/src/config.production.tsx b/src/config.production.tsx index 54fcb54..97660fd 100644 --- a/src/config.production.tsx +++ b/src/config.production.tsx @@ -12,5 +12,6 @@ import { Config } from "./config"; export default { developmentMode: false, - fakeAPI: false + fakeAPI: false, + apiPath: "/admin/api" } as Config; diff --git a/src/config.tsx b/src/config.tsx index 5606de8..0635aba 100644 --- a/src/config.tsx +++ b/src/config.tsx @@ -14,6 +14,7 @@ import productionConfig from "./config.production"; export interface Config { developmentMode: boolean; fakeAPI: boolean; + apiPath: string; } let config: Config; diff --git a/src/util/__tests__/http.test.tsx b/src/util/__tests__/http.test.tsx index e6acd5c..da78b12 100644 --- a/src/util/__tests__/http.test.tsx +++ b/src/util/__tests__/http.test.tsx @@ -8,33 +8,87 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -import { checkIfLoggedOut } from "../http"; +import HttpClient, { + checkForErrors, + checkIfLoggedOut, + convertJSON +} from "../http"; import api from "../api"; -import { mockLocationReload, restoreLocationReload } from "./mockWindow"; +import { Config } from "../../config"; + +const originalReload = window.location.reload; + +/** + * Mock window.location.reload + * https://remarkablemark.org/blog/2018/11/17/mock-window-location/ + * + * Restore it with {@link restoreLocationReload} + */ +const mockLocationReload = () => { + Object.defineProperty(window.location, "reload", { + configurable: true + }); + + window.location.reload = jest.fn(); +}; + +/** + * Restore window.location.reload after being mocked by + * {@link mockLocationReload} + */ +const restoreLocationReload = () => { + window.location.reload = originalReload; +}; + +describe("HttpClient", () => { + describe("urlFor", () => { + it("uses the fakeAPI route if configured", () => { + const config: Config = { + developmentMode: false, + fakeAPI: true, + apiPath: "/fakeAPI" + }; + const httpClient = new HttpClient(config); + + expect(httpClient.urlFor("test")).toEqual("/fakeAPI/test"); + }); + + it("uses the production route if configured", () => { + const config: Config = { + developmentMode: false, + fakeAPI: false, + apiPath: "/admin/api" + }; + const httpClient = new HttpClient(config); + + expect(httpClient.urlFor("test")).toEqual("/admin/api/test"); + }); + }); +}); describe("checkIfLoggedOut", () => { - it("passes the response through if logged in and not a 401", async () => { + it("should pass the response through if logged in and not a 401", async () => { const response = { status: 200 } as Response; api.loggedIn = true; await expect(checkIfLoggedOut(response)).resolves.toEqual(response); }); - it("passes the response through if not logged in and not a 401", async () => { + it("should pass the response through if not logged in and not a 401", async () => { const response = { status: 200 } as Response; api.loggedIn = false; await expect(checkIfLoggedOut(response)).resolves.toEqual(response); }); - it("passes the response through if not logged in and is a 401", async () => { + it("should pass the response through if not logged in and is a 401", async () => { const response = { status: 401 } as Response; api.loggedIn = false; await expect(checkIfLoggedOut(response)).resolves.toEqual(response); }); - it("clears the session cookie and reloads if logged in and response is a 401", async () => { + it("should clear the session cookie and reload if logged in and response is a 401", async () => { const response = { status: 401 } as Response; api.loggedIn = true; document.cookie = "user_id=test"; @@ -50,3 +104,41 @@ describe("checkIfLoggedOut", () => { restoreLocationReload(); }); }); + +describe("convertJSON", () => { + it("should convert to JSON if it is not canceled or an error", async () => { + const body = { test: true }; + const response = new Response(JSON.stringify(body)); + + await expect(convertJSON(response)).resolves.toEqual(body); + }); + + it("should reject with input if canceled", async () => { + const cancelError = { + isCanceled: true, + test: true + }; + + await expect(convertJSON(cancelError)).rejects.toEqual(cancelError); + }); + + it("should reject with input if error", async () => { + const error = new Error("test"); + + await expect(convertJSON(error)).rejects.toEqual(error); + }); +}); + +describe("checkForErrors", () => { + it("should pass through the data if there is no error", async () => { + const data = { test: true }; + + await expect(checkForErrors(data)).resolves.toEqual(data); + }); + + it("should reject with the error if there is an error", async () => { + const data = { error: { test: true } }; + + await expect(checkForErrors(data)).rejects.toEqual(data.error); + }); +}); diff --git a/src/util/__tests__/mockWindow.ts b/src/util/__tests__/mockWindow.ts deleted file mode 100644 index c4bf557..0000000 --- a/src/util/__tests__/mockWindow.ts +++ /dev/null @@ -1,33 +0,0 @@ -/* Pi-hole: A black hole for Internet advertisements - * (c) 2019 Pi-hole, LLC (https://pi-hole.net) - * Network-wide ad blocking via your own hardware. - * - * Web Interface - * Functions for mocking window attributes - * - * This file is copyright under the latest version of the EUPL. - * Please see LICENSE file for your rights under this license. */ - -const originalReload = window.location.reload; - -/** - * Mock window.location.reload - * https://remarkablemark.org/blog/2018/11/17/mock-window-location/ - * - * Restore it with {@link restoreLocationReload} - */ -export const mockLocationReload = () => { - Object.defineProperty(window.location, "reload", { - configurable: true - }); - - window.location.reload = jest.fn(); -}; - -/** - * Restore window.location.reload after being mocked by - * {@link mockLocationReload} - */ -export const restoreLocationReload = () => { - window.location.reload = originalReload; -}; diff --git a/src/util/api.tsx b/src/util/api.tsx index af51404..0342ac1 100644 --- a/src/util/api.tsx +++ b/src/util/api.tsx @@ -8,72 +8,95 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -import http, { paramsToString, timeRangeToParams } from "./http"; +import HttpClient, { paramsToString, timeRangeToParams } from "./http"; import config from "../config"; import { TimeRange } from "../components/common/context/TimeRangeContext"; -export default { - loggedIn: false, - authenticate(key: string) { - return http.get("auth", { +export class ApiClient { + public loggedIn = false; + + constructor(private http: HttpClient) {} + + authenticate = (key: string): Promise => { + return this.http.get("auth", { headers: new Headers({ "X-Pi-hole-Authenticate": key }) }); - }, - logout() { - return http.delete("auth"); - }, - getSummary(): Promise { - return http.get("stats/summary"); - }, - getSummaryDb(range: TimeRange): Promise { - return http.get("stats/database/summary?" + timeRangeToParams(range)); - }, - getHistoryGraph(): Promise> { - return http.get("stats/overTime/history"); - }, - getHistoryGraphDb( + }; + + logout = (): Promise => { + return this.http.delete("auth"); + }; + + getSummary = (): Promise => { + return this.http.get("stats/summary"); + }; + + getSummaryDb = (range: TimeRange): Promise => { + return this.http.get("stats/database/summary?" + timeRangeToParams(range)); + }; + + getHistoryGraph = (): Promise> => { + return this.http.get("stats/overTime/history"); + }; + + getHistoryGraphDb = ( range: TimeRange, interval: number - ): Promise> { - return http.get( + ): Promise> => { + return this.http.get( "stats/database/overTime/history?interval=" + interval + "&" + timeRangeToParams(range) ); - }, - getClientsGraph(): Promise { - return http.get("stats/overTime/clients"); - }, - getClientsGraphDb( + }; + + getClientsGraph = (): Promise => { + return this.http.get("stats/overTime/clients"); + }; + + getClientsGraphDb = ( range: TimeRange, interval: number - ): Promise { - return http.get( + ): Promise => { + return this.http.get( "stats/database/overTime/clients?interval=" + interval + "&" + timeRangeToParams(range) ); - }, - getQueryTypes(): Promise> { - return http.get("stats/query_types"); - }, - getQueryTypesDb(range: TimeRange): Promise> { - return http.get("stats/database/query_types?" + timeRangeToParams(range)); - }, - getUpstreams(): Promise { - return http.get("stats/upstreams"); - }, - getUpstreamsDb(range: TimeRange): Promise { - return http.get("stats/database/upstreams?" + timeRangeToParams(range)); - }, - getTopDomains(): Promise { - return http.get("stats/top_domains"); - }, - getTopDomainsDb(range: TimeRange): Promise { - return http.get("stats/database/top_domains?" + timeRangeToParams(range)); - }, + }; + + getQueryTypes = (): Promise> => { + return this.http.get("stats/query_types"); + }; + + getQueryTypesDb = (range: TimeRange): Promise> => { + return this.http.get( + "stats/database/query_types?" + timeRangeToParams(range) + ); + }; + + getUpstreams = (): Promise => { + return this.http.get("stats/upstreams"); + }; + + getUpstreamsDb = (range: TimeRange): Promise => { + return this.http.get( + "stats/database/upstreams?" + timeRangeToParams(range) + ); + }; + + getTopDomains = (): Promise => { + return this.http.get("stats/top_domains"); + }; + + getTopDomainsDb = (range: TimeRange): Promise => { + return this.http.get( + "stats/database/top_domains?" + timeRangeToParams(range) + ); + }; + getTopBlocked(): Promise { // The API uses a GET parameter to differentiate top domains from top // blocked, but the fake API is not able to handle GET parameters right now. @@ -81,8 +104,9 @@ export default { ? "stats/top_blocked" : "stats/top_domains?blocked=true"; - return http.get(url); - }, + return this.http.get(url); + } + getTopBlockedDb(range: TimeRange): Promise { // The API uses a GET parameter to differentiate top domains from top // blocked, but the fake API is not able to handle GET parameters right now. @@ -90,75 +114,110 @@ export default { ? "stats/database/top_blocked?" : "stats/database/top_domains?blocked=true&"; - return http.get(url + timeRangeToParams(range)); - }, - getTopClients(): Promise { - return http.get("stats/top_clients"); - }, - getTopClientsDb(range: TimeRange): Promise { - return http.get("stats/database/top_clients?" + timeRangeToParams(range)); - }, - getHistory(params: any): Promise { - return http.get("stats/history?" + paramsToString(params)); - }, - getWhitelist() { - return http.get("dns/whitelist"); - }, - getBlacklist() { - return http.get("dns/blacklist"); - }, - getRegexlist() { - return http.get("dns/regexlist"); - }, - addWhitelist(domain: string) { - return http.post("dns/whitelist", { domain: domain }); - }, - addBlacklist(domain: string) { - return http.post("dns/blacklist", { domain: domain }); - }, - addRegexlist(domain: string) { - return http.post("dns/regexlist", { domain: domain }); - }, - removeWhitelist(domain: string) { - return http.delete("dns/whitelist/" + domain); - }, - removeBlacklist(domain: string) { - return http.delete("dns/blacklist/" + domain); - }, - removeRegexlist(domain: string) { - return http.delete("dns/regexlist/" + encodeURIComponent(domain)); - }, - getStatus(): Promise { - return http.get("dns/status"); - }, - setStatus(action: StatusAction, time?: number) { - return http.post("dns/status", { action, time }); - }, - getNetworkInfo(): Promise { - return http.get("settings/network"); - }, - getVersion(): Promise { - return http.get("version"); - }, - getFTLdb(): Promise { - return http.get("settings/ftldb"); - }, - getDNSInfo(): Promise { - return http.get("settings/dns"); - }, - getDHCPInfo(): Promise { - return http.get("settings/dhcp"); - }, - updateDHCPInfo(settings: ApiDhcpSettings) { - return http.put("settings/dhcp", settings); - }, - updateDNSInfo(settings: ApiDnsSettings) { - return http.put("settings/dns", settings); - }, - getPreferences(): Promise { - return http.get("settings/web"); - }, - updatePreferences(settings: ApiPreferences) { - return http.put("settings/web", settings); + return this.http.get(url + timeRangeToParams(range)); } -}; + + getTopClients = (): Promise => { + return this.http.get("stats/top_clients"); + }; + + getTopClientsDb = (range: TimeRange): Promise => { + return this.http.get( + "stats/database/top_clients?" + timeRangeToParams(range) + ); + }; + + getHistory = (params: any): Promise => { + return this.http.get("stats/history?" + paramsToString(params)); + }; + + getWhitelist = (): Promise> => { + return this.http.get("dns/whitelist"); + }; + + getBlacklist = (): Promise> => { + return this.http.get("dns/blacklist"); + }; + + getRegexlist = (): Promise> => { + return this.http.get("dns/regexlist"); + }; + + addWhitelist = (domain: string): Promise => { + return this.http.post("dns/whitelist", { domain: domain }); + }; + + addBlacklist = (domain: string): Promise => { + return this.http.post("dns/blacklist", { domain: domain }); + }; + + addRegexlist = (domain: string): Promise => { + return this.http.post("dns/regexlist", { domain: domain }); + }; + + removeWhitelist = (domain: string): Promise => { + return this.http.delete("dns/whitelist/" + domain); + }; + + removeBlacklist = (domain: string): Promise => { + return this.http.delete("dns/blacklist/" + domain); + }; + + removeRegexlist = (domain: string): Promise => { + return this.http.delete("dns/regexlist/" + encodeURIComponent(domain)); + }; + + getStatus = (): Promise => { + return this.http.get("dns/status"); + }; + + setStatus = ( + action: StatusAction, + time?: number + ): Promise => { + return this.http.post("dns/status", { + action, + time + }); + }; + + getNetworkInfo = (): Promise => { + return this.http.get("settings/network"); + }; + + getVersion = (): Promise => { + return this.http.get("version"); + }; + + getFTLdb = (): Promise => { + return this.http.get("settings/ftldb"); + }; + + getDNSInfo = (): Promise => { + return this.http.get("settings/dns"); + }; + + getDHCPInfo = (): Promise => { + return this.http.get("settings/dhcp"); + }; + + updateDHCPInfo = (settings: ApiDhcpSettings): Promise => { + return this.http.put("settings/dhcp", settings); + }; + + updateDNSInfo = (settings: ApiDnsSettings): Promise => { + return this.http.put("settings/dns", settings); + }; + + getPreferences = (): Promise => { + return this.http.get("settings/web"); + }; + + updatePreferences = ( + settings: ApiPreferences + ): Promise => { + return this.http.put("settings/web", settings); + }; +} + +export default new ApiClient(new HttpClient(config)); diff --git a/src/util/http.tsx b/src/util/http.tsx index 787a3e8..c601505 100644 --- a/src/util/http.tsx +++ b/src/util/http.tsx @@ -9,89 +9,113 @@ * Please see LICENSE file for your rights under this license. */ import api from "./api"; -import config from "../config"; +import { Config } from "../config"; import { TimeRange } from "../components/common/context/TimeRangeContext"; /** - * A group of HTTP functions. Each function parses the response checks for - * errors + * A class which provides HTTP functions. Each function parses the response and + * checks for errors */ -export default { +export default class HttpClient { + constructor(private config: Config) {} + /** * Perform a GET request * - * @param url the URL to access - * @param options optional fetch configuration - * @returns {Promise} a promise with the data or error returned + * @param url The URL to access + * @param options Optional fetch configuration + * @returns A promise with the data or error returned */ - get(url: string, options = {}) { - return fetch(urlFor(url), { - credentials: credentialType(), + get = (url: string, options: RequestInit = {}): Promise => { + return fetch(this.urlFor(url), { + credentials: this.credentialType(), ...options }) .then(checkIfLoggedOut) .then(convertJSON) .catch(convertJSON) .then(checkForErrors); - }, + }; /** * Perform a POST request * - * @param url the URL to access - * @param data the data to send - * @returns {Promise} a promise with the data or error returned + * @param url The URL to access + * @param data The data to send + * @returns A promise with the data or error returned */ - post(url: string, data: {}) { - return fetch(urlFor(url), { + post = (url: string, data: object): Promise => { + return fetch(this.urlFor(url), { method: "POST", body: JSON.stringify(data), headers: new Headers({ "Content-Type": "application/json" }), - credentials: credentialType() + credentials: this.credentialType() }) .then(checkIfLoggedOut) .then(convertJSON) .catch(convertJSON) .then(checkForErrors); - }, + }; /** * Perform a PUT request * - * @param url the URL to access - * @param data the data to send - * @returns {Promise} a promise with the data or error returned + * @param url The URL to access + * @param data The data to send + * @returns A promise with the data or error returned */ - put(url: string, data: {}) { - return fetch(urlFor(url), { + put = (url: string, data: object): Promise => { + return fetch(this.urlFor(url), { method: "PUT", body: JSON.stringify(data), headers: new Headers({ "Content-Type": "application/json" }), - credentials: credentialType() + credentials: this.credentialType() }) .then(checkIfLoggedOut) .then(convertJSON) .catch(convertJSON) .then(checkForErrors); - }, + }; /** * Perform a DELETE request * - * @param url the URL to access - * @returns {Promise} a promise with the data or error returned + * @param url The URL to access + * @returns A promise with the data or error returned */ - delete(url: string) { - return fetch(urlFor(url), { + delete = (url: string): Promise => { + return fetch(this.urlFor(url), { method: "DELETE", - credentials: credentialType() + credentials: this.credentialType() }) .then(checkIfLoggedOut) .then(convertJSON) .catch(convertJSON) .then(checkForErrors); - } -}; + }; + + /** + * Get the URL for an endpoint + * + * @param endpoint The endpoint + * @returns The URL for the endpoint + */ + urlFor = (endpoint: string): string => { + return this.config.apiPath + "/" + endpoint; + }; + + /** + * Get the credential type for requests + * + * @returns The credential type + */ + credentialType = (): RequestCredentials => { + // Development API requests may use a different origin (pi.hole) since it is + // running off of the developer's machine. Therefore, allow credentials to + // be used across origins when in development mode. + return this.config.developmentMode ? "include" : "same-origin"; + }; +} /** * If the user is logged in, check if the user's session has lapsed. @@ -120,7 +144,7 @@ export const checkIfLoggedOut = (response: Response) => { * @param data a Response or Error * @returns {*} a promise with the parsed JSON, or the error */ -const convertJSON = (data: any): Promise => { +export const convertJSON = (data: any): Promise => { if (data.isCanceled || data instanceof Error) { return Promise.reject(data); } @@ -135,7 +159,7 @@ const convertJSON = (data: any): Promise => { * @returns {*} a resolving promise with the data if no error, otherwise a * rejecting promise with the error */ -const checkForErrors = (data: any): Promise => { +export const checkForErrors = (data: any): Promise => { if (data.error) { return Promise.reject(data.error); } @@ -143,35 +167,6 @@ const checkForErrors = (data: any): Promise => { return Promise.resolve(data); }; -/** - * Get the URL for an endpoint - * - * @param endpoint the endpoint - * @returns {string} the URL for the endpoint - */ -const urlFor = (endpoint: string): string => { - let apiLocation; - - if (config.fakeAPI) { - apiLocation = process.env.PUBLIC_URL + "/fakeAPI"; - } else { - apiLocation = "/admin/api"; - } - - return apiLocation + "/" + endpoint; -}; - -/** - * Get the credential type for requests - * - * @returns {string} the credential type - */ -const credentialType = () => { - // Development API requests use a different origin (pi.hole) since it is running off of the developer's machine. - // Therefore, allow credentials to be used across origins when in development mode. - return config.developmentMode ? "include" : "same-origin"; -}; - /** * Convert an object into GET parameters. The object must be flat (only * key-value pairs). diff --git a/src/views/Login.tsx b/src/views/Login.tsx index 6ea0f82..3fde2ee 100644 --- a/src/views/Login.tsx +++ b/src/views/Login.tsx @@ -78,14 +78,7 @@ class Login extends Component { // Send the password to the API to authenticate the user api .authenticate(hashedPassword) - .then(data => { - // Verify status - if (data.status !== "success") { - console.log("Failed to log in:"); - console.log(data); - return; - } - + .then(() => { api.loggedIn = true; if (config.fakeAPI) { From 4e81f16ecec36ae58e3427ad7c085ecffae03186 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Thu, 11 Apr 2019 22:12:43 -0700 Subject: [PATCH 07/16] Improve type-safety in HttpClient and CancelablePromise Signed-off-by: Mcat12 --- src/util/CancelablePromise.tsx | 7 ++++++ src/util/__tests__/http.test.tsx | 6 ++--- src/util/http.tsx | 39 ++++++++++++++++++++------------ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/util/CancelablePromise.tsx b/src/util/CancelablePromise.tsx index 1fa2d12..2bda5bf 100644 --- a/src/util/CancelablePromise.tsx +++ b/src/util/CancelablePromise.tsx @@ -31,6 +31,13 @@ export interface CancelableOptions { interval: number; } +/** + * The error thrown when the {@link CancelablePromise} is canceled + */ +export interface CanceledError { + isCanceled: true; +} + /** * Make a promise cancelable and repeatable * diff --git a/src/util/__tests__/http.test.tsx b/src/util/__tests__/http.test.tsx index da78b12..d1469dd 100644 --- a/src/util/__tests__/http.test.tsx +++ b/src/util/__tests__/http.test.tsx @@ -15,6 +15,7 @@ import HttpClient, { } from "../http"; import api from "../api"; import { Config } from "../../config"; +import { CanceledError } from "../CancelablePromise"; const originalReload = window.location.reload; @@ -114,9 +115,8 @@ describe("convertJSON", () => { }); it("should reject with input if canceled", async () => { - const cancelError = { - isCanceled: true, - test: true + const cancelError: CanceledError = { + isCanceled: true }; await expect(convertJSON(cancelError)).rejects.toEqual(cancelError); diff --git a/src/util/http.tsx b/src/util/http.tsx index c601505..3c0df75 100644 --- a/src/util/http.tsx +++ b/src/util/http.tsx @@ -11,6 +11,7 @@ import api from "./api"; import { Config } from "../config"; import { TimeRange } from "../components/common/context/TimeRangeContext"; +import { CanceledError } from "./CancelablePromise"; /** * A class which provides HTTP functions. Each function parses the response and @@ -27,6 +28,7 @@ export default class HttpClient { * @returns A promise with the data or error returned */ get = (url: string, options: RequestInit = {}): Promise => { + // @ts-ignore return fetch(this.urlFor(url), { credentials: this.credentialType(), ...options @@ -45,6 +47,7 @@ export default class HttpClient { * @returns A promise with the data or error returned */ post = (url: string, data: object): Promise => { + // @ts-ignore return fetch(this.urlFor(url), { method: "POST", body: JSON.stringify(data), @@ -65,6 +68,7 @@ export default class HttpClient { * @returns A promise with the data or error returned */ put = (url: string, data: object): Promise => { + // @ts-ignore return fetch(this.urlFor(url), { method: "PUT", body: JSON.stringify(data), @@ -84,6 +88,7 @@ export default class HttpClient { * @returns A promise with the data or error returned */ delete = (url: string): Promise => { + // @ts-ignore return fetch(this.urlFor(url), { method: "DELETE", credentials: this.credentialType() @@ -121,10 +126,10 @@ export default class HttpClient { * If the user is logged in, check if the user's session has lapsed. * If so, log them out and refresh the page. * - * @param response the Response from fetch - * @return {Promise} if logged in, the response, otherwise a canceled promise + * @param response The Response from fetch + * @return If logged in, the response, otherwise a canceled promise */ -export const checkIfLoggedOut = (response: Response) => { +export const checkIfLoggedOut = (response: Response): Promise => { if (api.loggedIn && response.status === 401) { // Clear the user's old session and refresh the page document.cookie = @@ -144,22 +149,24 @@ export const checkIfLoggedOut = (response: Response) => { * @param data a Response or Error * @returns {*} a promise with the parsed JSON, or the error */ -export const convertJSON = (data: any): Promise => { - if (data.isCanceled || data instanceof Error) { +export const convertJSON = ( + data: Response | Error | CanceledError +): Promise => { + if ((data as CanceledError).isCanceled || data instanceof Error) { return Promise.reject(data); } - return data.json(); + return (data as Response).json(); }; /** * Check for an error returned by the API * * @param data the parsed JSON body of the response - * @returns {*} a resolving promise with the data if no error, otherwise a + * @returns A resolving promise with the data if no error, otherwise a * rejecting promise with the error */ -export const checkForErrors = (data: any): Promise => { +export const checkForErrors = (data: T): Promise => { if (data.error) { return Promise.reject(data.error); } @@ -171,13 +178,16 @@ export const checkForErrors = (data: any): Promise => { * Convert an object into GET parameters. The object must be flat (only * key-value pairs). * - * @param params the parameters object - * @returns {string} the parameters converted into GET parameter form + * @param params The parameters object + * @returns The parameters converted into GET parameter form */ -export const paramsToString = (params: any) => - Object.keys(params) +export const paramsToString = (params: { + [key: string]: string | number; +}): string => { + return Object.keys(params) .map(key => key + "=" + params[key]) .join("&"); +}; /** * Convert a time range into GET parameters @@ -185,8 +195,9 @@ export const paramsToString = (params: any) => * @param range The time range to convert * @return The time range as GET parameters */ -export const timeRangeToParams = (range: TimeRange) => - paramsToString({ +export const timeRangeToParams = (range: TimeRange) => { + return paramsToString({ from: range.from.unix(), until: range.until.unix() }); +}; From 2f72c9a2b8045bb445b76e88a60306ce8e3cef7b Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Fri, 12 Apr 2019 18:06:40 -0700 Subject: [PATCH 08/16] Add tests for paramsToString and timeRangeToParams Signed-off-by: Mcat12 --- src/util/__tests__/http.test.tsx | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/util/__tests__/http.test.tsx b/src/util/__tests__/http.test.tsx index d1469dd..a1f9621 100644 --- a/src/util/__tests__/http.test.tsx +++ b/src/util/__tests__/http.test.tsx @@ -11,11 +11,15 @@ import HttpClient, { checkForErrors, checkIfLoggedOut, - convertJSON + convertJSON, + paramsToString, + timeRangeToParams } from "../http"; import api from "../api"; import { Config } from "../../config"; import { CanceledError } from "../CancelablePromise"; +import { TimeRange } from "../../components/common/context/TimeRangeContext"; +import moment from "moment"; const originalReload = window.location.reload; @@ -142,3 +146,29 @@ describe("checkForErrors", () => { await expect(checkForErrors(data)).rejects.toEqual(data.error); }); }); + +describe("paramsToString", () => { + it("converts an object into parameters", () => { + const object = { + test1: "1", + test2: "two", + test3: 3 + }; + const expectedParams = "test1=1&test2=two&test3=3"; + + expect(paramsToString(object)).toEqual(expectedParams); + }); +}); + +describe("timeRangeToParams", () => { + it("converts a time range into parameters", () => { + const range: TimeRange = { + name: "Test time range", + from: moment("2019-04-12T01:03:17+00:00"), + until: moment("2019-04-13T01:03:17+00:00") + }; + const expectedParams = "from=1555030997&until=1555117397"; + + expect(timeRangeToParams(range)).toEqual(expectedParams); + }); +}); From 8253c68586c88f8f21889cc9db14633de62f90bf Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sat, 13 Apr 2019 16:45:22 -0700 Subject: [PATCH 09/16] Fix development API path always using fakeAPI Signed-off-by: Mcat12 --- src/config.development.tsx | 2 +- src/config.tsx | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/config.development.tsx b/src/config.development.tsx index 4d08a3f..37b7cd7 100644 --- a/src/config.development.tsx +++ b/src/config.development.tsx @@ -13,5 +13,5 @@ import { Config } from "./config"; export default { developmentMode: true, fakeAPI: false, - apiPath: process.env.PUBLIC_URL + "/fakeAPI" + apiPath: process.env.PUBLIC_URL } as Config; diff --git a/src/config.tsx b/src/config.tsx index 0635aba..e021d03 100644 --- a/src/config.tsx +++ b/src/config.tsx @@ -27,6 +27,7 @@ if (process.env.NODE_ENV === "development") { if (process.env.REACT_APP_FAKE_API) { config.fakeAPI = true; + config.apiPath += "/fakeAPI"; } export default config; From 209d0bab62bedd6e99b86f1660e1f75f8e7cbc99 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sat, 13 Apr 2019 16:47:11 -0700 Subject: [PATCH 10/16] Consolidate and improve response handling in HttpClient The `catch(convertJSON)` call was in practice just rethrowing the error. Signed-off-by: Mcat12 --- src/util/http.tsx | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/util/http.tsx b/src/util/http.tsx index 3c0df75..d76ba3a 100644 --- a/src/util/http.tsx +++ b/src/util/http.tsx @@ -20,6 +20,18 @@ import { CanceledError } from "./CancelablePromise"; export default class HttpClient { constructor(private config: Config) {} + /** + * Check if the user is logged out, convert to JSON, and check for API errors + * + * @param response The HTTP response + */ + handleResponse = (response: Response): Promise => { + // @ts-ignore + return checkIfLoggedOut(response) + .then(convertJSON) + .then(checkForErrors); + }; + /** * Perform a GET request * @@ -30,13 +42,10 @@ export default class HttpClient { get = (url: string, options: RequestInit = {}): Promise => { // @ts-ignore return fetch(this.urlFor(url), { + method: "GET", credentials: this.credentialType(), ...options - }) - .then(checkIfLoggedOut) - .then(convertJSON) - .catch(convertJSON) - .then(checkForErrors); + }).then(this.handleResponse); }; /** @@ -53,11 +62,7 @@ export default class HttpClient { body: JSON.stringify(data), headers: new Headers({ "Content-Type": "application/json" }), credentials: this.credentialType() - }) - .then(checkIfLoggedOut) - .then(convertJSON) - .catch(convertJSON) - .then(checkForErrors); + }).then(this.handleResponse); }; /** @@ -74,11 +79,7 @@ export default class HttpClient { body: JSON.stringify(data), headers: new Headers({ "Content-Type": "application/json" }), credentials: this.credentialType() - }) - .then(checkIfLoggedOut) - .then(convertJSON) - .catch(convertJSON) - .then(checkForErrors); + }).then(this.handleResponse); }; /** @@ -92,11 +93,7 @@ export default class HttpClient { return fetch(this.urlFor(url), { method: "DELETE", credentials: this.credentialType() - }) - .then(checkIfLoggedOut) - .then(convertJSON) - .catch(convertJSON) - .then(checkForErrors); + }).then(this.handleResponse); }; /** From 2189c1410faaf4667f7d6c040b084937b437c6b2 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sat, 13 Apr 2019 16:47:47 -0700 Subject: [PATCH 11/16] Add more tests for HttpClient This brings the http.tsx file up to 100% coverage. Signed-off-by: Mcat12 --- src/util/__tests__/http.test.tsx | 110 +++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/src/util/__tests__/http.test.tsx b/src/util/__tests__/http.test.tsx index a1f9621..8bddd18 100644 --- a/src/util/__tests__/http.test.tsx +++ b/src/util/__tests__/http.test.tsx @@ -20,6 +20,7 @@ import { Config } from "../../config"; import { CanceledError } from "../CancelablePromise"; import { TimeRange } from "../../components/common/context/TimeRangeContext"; import moment from "moment"; +import fetchMock from "fetch-mock"; const originalReload = window.location.reload; @@ -46,6 +47,115 @@ const restoreLocationReload = () => { }; describe("HttpClient", () => { + const testEndpoint = "test"; + const testEndpointFull = "/api/test"; + const data = { test: true }; + const config: Config = { + developmentMode: true, + apiPath: "/api", + fakeAPI: false + }; + const canceledError: CanceledError = { isCanceled: true }; + let httpClient: HttpClient; + + beforeEach(() => { + httpClient = new HttpClient(config); + }); + + describe("handleResponse", () => { + it("should make a GET request and return the parsed data", async () => { + const response = { + status: 200, + json: () => Promise.resolve(data) + } as Response; + + await expect(httpClient.handleResponse(response)).resolves.toEqual(data); + }); + + it("should cancel if logged out by API", async () => { + const error: ApiError = { + key: "unauthorized", + message: "Unauthorized", + data: null + }; + const response = { + status: 401, + json: () => Promise.resolve({ error }) + } as Response; + + api.loggedIn = true; + mockLocationReload(); + + await expect(httpClient.handleResponse(response)).rejects.toEqual( + canceledError + ); + expect(window.location.reload).toHaveBeenCalled(); + restoreLocationReload(); + }); + + it("should reject with the API error if set", async () => { + const error: ApiError = { + key: "test_key", + message: "Test message", + data: null + }; + const response = { + status: 500, + json: () => Promise.resolve({ error }) + } as Response; + + await expect(httpClient.handleResponse(response)).rejects.toEqual(error); + }); + }); + + describe("HTTP functions", () => { + it("should make a GET request and call handleResponse", async () => { + fetchMock.get(testEndpointFull, { body: data }); + httpClient.handleResponse = jest.fn(() => Promise.resolve(data)); + + await expect(httpClient.get(testEndpoint)).resolves.toEqual(data); + const request = fetchMock.lastCall(testEndpointFull)![1]!; + + expect(httpClient.handleResponse).toHaveBeenCalled(); + expect(request.method).toEqual("GET"); + }); + + it("should make a POST request and call handleResponse", async () => { + fetchMock.post(testEndpointFull, { body: data }); + httpClient.handleResponse = jest.fn(() => Promise.resolve(data)); + + await expect(httpClient.post(testEndpoint, data)).resolves.toEqual(data); + const request = fetchMock.lastCall(testEndpointFull)![1]!; + + expect(httpClient.handleResponse).toHaveBeenCalled(); + expect(request.method).toEqual("POST"); + expect(request.body).toEqual(JSON.stringify(data)); + }); + + it("should make a PUT request and call handleResponse", async () => { + fetchMock.put(testEndpointFull, { body: data }); + httpClient.handleResponse = jest.fn(() => Promise.resolve(data)); + + await expect(httpClient.put(testEndpoint, data)).resolves.toEqual(data); + const request = fetchMock.lastCall(testEndpointFull)![1]!; + + expect(httpClient.handleResponse).toHaveBeenCalled(); + expect(request.method).toEqual("PUT"); + expect(request.body).toEqual(JSON.stringify(data)); + }); + + it("should make a DELETE request and call handleResponse", async () => { + fetchMock.delete(testEndpointFull, { body: data }); + httpClient.handleResponse = jest.fn(() => Promise.resolve(data)); + + await expect(httpClient.delete(testEndpoint)).resolves.toEqual(data); + const request = fetchMock.lastCall(testEndpointFull)![1]!; + + expect(httpClient.handleResponse).toHaveBeenCalled(); + expect(request.method).toEqual("DELETE"); + }); + }); + describe("urlFor", () => { it("uses the fakeAPI route if configured", () => { const config: Config = { From c9f69815fd491507f6c24053e635fb3a2e7e9655 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sat, 13 Apr 2019 18:28:14 -0700 Subject: [PATCH 12/16] Add tests for ApiClient Signed-off-by: Mcat12 --- src/util/__tests__/api.test.tsx | 319 +++++++++++++++++++++++++++++++- src/util/api.tsx | 6 +- src/util/http.tsx | 2 +- 3 files changed, 314 insertions(+), 13 deletions(-) diff --git a/src/util/__tests__/api.test.tsx b/src/util/__tests__/api.test.tsx index 01d7215..dafd93e 100644 --- a/src/util/__tests__/api.test.tsx +++ b/src/util/__tests__/api.test.tsx @@ -8,16 +8,317 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -import api from "../api"; +import { ApiClient } from "../api"; +import HttpClient from "../http"; +import { TimeRange } from "../../components/common/context/TimeRangeContext"; +import moment from "moment"; +import { Config } from "../../config"; -// This is a dumb test used to set up the next test, -// which checks that the logged in state is reset before each test -it("sets logged in to true", () => { - api.loggedIn = true; +// Test each endpoint function to make sure it is calling the right endpoint +// with the right data. +describe("ApiClient", () => { + // Services + let httpClient: HttpClient; + let api: ApiClient; - expect(api.loggedIn).toBeTruthy(); -}); + // Test response data + const getData = { test: "GET" }; + const postData = { test: "POST" }; + const putData = { test: "PUT" }; + const deleteData = { test: "DELETE" }; + const getPromise = Promise.resolve(getData); + const postPromise = Promise.resolve(postData); + const putPromise = Promise.resolve(putData); + const deletePromise = Promise.resolve(deleteData); -it("resets the logged in state for each test", () => { - expect(api.loggedIn).toBeFalsy(); + // Test data + const range: TimeRange = { + name: "Test time range", + from: moment("2019-04-12T01:03:17+00:00"), + until: moment("2019-04-13T01:03:17+00:00") + }; + const rangeParams = "from=1555030997&until=1555117397"; + const config: Config = { + developmentMode: true, + fakeAPI: true, + apiPath: "/admin/api" + }; + + beforeEach(() => { + // Create fresh services + httpClient = ({ + get: jest.fn(() => getPromise), + post: jest.fn(() => postPromise), + put: jest.fn(() => putPromise), + delete: jest.fn(() => deletePromise), + config + } as any) as HttpClient; + api = new ApiClient(httpClient); + }); + + describe("authentication calls", () => { + it("should call login endpoint with auth headers", async () => { + const key = "test"; + await expect(api.authenticate(key)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("auth", { + headers: { "X-Pi-hole-Authenticate": key } + }); + }); + + it("should call logout endpoint", async () => { + await expect(api.logout()).resolves.toEqual(deleteData); + expect(httpClient.delete).toHaveBeenCalledWith("auth"); + }); + }); + + describe("statistics calls", () => { + it("should call summary endpoint", async () => { + await expect(api.getSummary()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/summary"); + }); + + it("should call history graph endpoint", async () => { + await expect(api.getHistoryGraph()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/overTime/history"); + }); + + it("should call clients graph endpoint", async () => { + await expect(api.getClientsGraph()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/overTime/clients"); + }); + + it("should call query types endpoint", async () => { + await expect(api.getQueryTypes()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/query_types"); + }); + + it("should call upstreams endpoint", async () => { + await expect(api.getUpstreams()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/upstreams"); + }); + + it("should call top domains endpoint", async () => { + await expect(api.getTopDomains()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/top_domains"); + }); + + it("should call top blocked endpoint (top_domains?blocked=true)", async () => { + config.fakeAPI = false; + await expect(api.getTopBlocked()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/top_domains?blocked=true" + ); + }); + + it("should call top blocked endpoint (top_blocked)", async () => { + config.fakeAPI = true; + await expect(api.getTopBlocked()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/top_blocked"); + }); + + it("should call top clients endpoint", async () => { + await expect(api.getTopClients()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/top_clients"); + }); + + it("should call top history endpoint with params", async () => { + const params = { test: "params" }; + await expect(api.getHistory(params)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("stats/history?test=params"); + }); + }); + + describe("database statistic calls", () => { + it("should call summary DB endpoint with time range", async () => { + await expect(api.getSummaryDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/summary?" + rangeParams + ); + }); + + it("should call history graph DB endpoint with interval and time range", async () => { + await expect(api.getHistoryGraphDb(range, 100)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/overTime/history?interval=100&" + rangeParams + ); + }); + + it("should call client graph DB endpoint with interval and time range", async () => { + await expect(api.getClientsGraphDb(range, 100)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/overTime/clients?interval=100&" + rangeParams + ); + }); + + it("should call query types DB endpoint with time range", async () => { + await expect(api.getQueryTypesDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/query_types?" + rangeParams + ); + }); + + it("should call query types DB endpoint with time range", async () => { + await expect(api.getQueryTypesDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/query_types?" + rangeParams + ); + }); + + it("should call upstreams DB endpoint with time range", async () => { + await expect(api.getUpstreamsDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/upstreams?" + rangeParams + ); + }); + + it("should call top domains DB endpoint with time range", async () => { + await expect(api.getTopDomainsDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/top_domains?" + rangeParams + ); + }); + + it("should call top blocked DB endpoint (top_domains?blocked=true) with time range", async () => { + config.fakeAPI = false; + await expect(api.getTopBlockedDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/top_domains?blocked=true&" + rangeParams + ); + }); + + it("should call top blocked DB endpoint (top_blocked) with time range", async () => { + config.fakeAPI = true; + await expect(api.getTopBlockedDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/top_blocked?" + rangeParams + ); + }); + + it("should call top clients DB endpoint with time range", async () => { + await expect(api.getTopClientsDb(range)).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith( + "stats/database/top_clients?" + rangeParams + ); + }); + }); + + describe("dns calls", () => { + it("should call get whitelist endpoint", async () => { + await expect(api.getWhitelist()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("dns/whitelist"); + }); + + it("should call get blacklist endpoint", async () => { + await expect(api.getBlacklist()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("dns/blacklist"); + }); + + it("should call get regexlist endpoint", async () => { + await expect(api.getRegexlist()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("dns/regexlist"); + }); + + it("should call add whitelist endpoint with domain", async () => { + const domain = "test.com"; + await expect(api.addWhitelist(domain)).resolves.toEqual(postData); + expect(httpClient.post).toHaveBeenCalledWith("dns/whitelist", { domain }); + }); + + it("should call add blacklist endpoint with domain", async () => { + const domain = "test.com"; + await expect(api.addBlacklist(domain)).resolves.toEqual(postData); + expect(httpClient.post).toHaveBeenCalledWith("dns/blacklist", { domain }); + }); + + it("should call add regexlist endpoint with domain", async () => { + const domain = "test.com"; + await expect(api.addRegexlist(domain)).resolves.toEqual(postData); + expect(httpClient.post).toHaveBeenCalledWith("dns/regexlist", { domain }); + }); + + it("should call remove whitelist endpoint with domain", async () => { + const domain = "test.com"; + await expect(api.removeWhitelist(domain)).resolves.toEqual(deleteData); + expect(httpClient.delete).toHaveBeenCalledWith("dns/whitelist/" + domain); + }); + + it("should call remove blacklist endpoint with domain", async () => { + const domain = "test.com"; + await expect(api.removeBlacklist(domain)).resolves.toEqual(deleteData); + expect(httpClient.delete).toHaveBeenCalledWith("dns/blacklist/" + domain); + }); + + it("should call remove regexlist endpoint with encoded domain", async () => { + const regex = "^test\\.com$"; + await expect(api.removeRegexlist(regex)).resolves.toEqual(deleteData); + expect(httpClient.delete).toHaveBeenCalledWith( + "dns/regexlist/%5Etest%5C.com%24" + ); + }); + + it("should call get status endpoint", async () => { + await expect(api.getStatus()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("dns/status"); + }); + + it("should call set status endpoint with action data", async () => { + const action: StatusAction = "enable"; + const time = 100; + await expect(api.setStatus(action, time)).resolves.toEqual(postData); + expect(httpClient.post).toHaveBeenCalledWith("dns/status", { + action, + time + }); + }); + }); + + describe("settings calls", () => { + it("should call get network settings endpoint", async () => { + await expect(api.getNetworkInfo()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("settings/network"); + }); + + it("should call version endpoint", async () => { + await expect(api.getVersion()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("version"); + }); + + it("should call FTL DB settings endpoint", async () => { + await expect(api.getFTLdb()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("settings/ftldb"); + }); + + it("should call get DNS settings endpoint", async () => { + await expect(api.getDNSInfo()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("settings/dns"); + }); + + it("should call get DHCP settings endpoint", async () => { + await expect(api.getDHCPInfo()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("settings/dhcp"); + }); + + it("should call get web preferences settings endpoint", async () => { + await expect(api.getPreferences()).resolves.toEqual(getData); + expect(httpClient.get).toHaveBeenCalledWith("settings/web"); + }); + + it("should call set DNS settings endpoint", async () => { + const settings = ({ test: true } as any) as ApiDnsSettings; + await expect(api.updateDNSInfo(settings)).resolves.toEqual(putData); + expect(httpClient.put).toHaveBeenCalledWith("settings/dns", settings); + }); + + it("should call set DHCP settings endpoint", async () => { + const settings = ({ test: true } as any) as ApiDhcpSettings; + await expect(api.updateDHCPInfo(settings)).resolves.toEqual(putData); + expect(httpClient.put).toHaveBeenCalledWith("settings/dhcp", settings); + }); + + it("should call set web preferences settings endpoint", async () => { + const settings = ({ test: true } as any) as ApiPreferences; + await expect(api.updatePreferences(settings)).resolves.toEqual(putData); + expect(httpClient.put).toHaveBeenCalledWith("settings/web", settings); + }); + }); }); diff --git a/src/util/api.tsx b/src/util/api.tsx index 0342ac1..ba95804 100644 --- a/src/util/api.tsx +++ b/src/util/api.tsx @@ -19,7 +19,7 @@ export class ApiClient { authenticate = (key: string): Promise => { return this.http.get("auth", { - headers: new Headers({ "X-Pi-hole-Authenticate": key }) + headers: { "X-Pi-hole-Authenticate": key } }); }; @@ -100,7 +100,7 @@ export class ApiClient { getTopBlocked(): Promise { // The API uses a GET parameter to differentiate top domains from top // blocked, but the fake API is not able to handle GET parameters right now. - const url = config.fakeAPI + const url = this.http.config.fakeAPI ? "stats/top_blocked" : "stats/top_domains?blocked=true"; @@ -110,7 +110,7 @@ export class ApiClient { getTopBlockedDb(range: TimeRange): Promise { // The API uses a GET parameter to differentiate top domains from top // blocked, but the fake API is not able to handle GET parameters right now. - const url = config.fakeAPI + const url = this.http.config.fakeAPI ? "stats/database/top_blocked?" : "stats/database/top_domains?blocked=true&"; diff --git a/src/util/http.tsx b/src/util/http.tsx index d76ba3a..e8cde6a 100644 --- a/src/util/http.tsx +++ b/src/util/http.tsx @@ -18,7 +18,7 @@ import { CanceledError } from "./CancelablePromise"; * checks for errors */ export default class HttpClient { - constructor(private config: Config) {} + constructor(public config: Config) {} /** * Check if the user is logged out, convert to JSON, and check for API errors From cc044e9a645bfb6b4708fe30b9bc0492dd5bf60a Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sat, 13 Apr 2019 20:07:31 -0700 Subject: [PATCH 13/16] Add i18n test Now the util folder has 100% coverage. Signed-off-by: Mcat12 --- src/util/__tests__/i18n.test.tsx | 29 +++++++++++++++++++++++++++++ src/util/i18n.tsx | 12 +++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 src/util/__tests__/i18n.test.tsx diff --git a/src/util/__tests__/i18n.test.tsx b/src/util/__tests__/i18n.test.tsx new file mode 100644 index 0000000..64539a1 --- /dev/null +++ b/src/util/__tests__/i18n.test.tsx @@ -0,0 +1,29 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Test internationalization setup + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +import { setupI18n } from "../i18n"; + +// Mock react-i18next, as it does not properly export the i18next module during +// testing. +// https://github.com/i18next/react-i18next/issues/434 +jest.mock("react-i18next", () => ({ + reactI18nextModule: { + type: "3rdParty", + init: () => {} + } +})); + +it("configures i18n successfully", async () => { + // Provide a mock ajax function to the XHR backend + const fakeAjax = (url: any, options: any, callback: any) => callback("", {}); + + // Make sure i18n initializes without error + await setupI18n(fakeAjax); +}); diff --git a/src/util/i18n.tsx b/src/util/i18n.tsx index 16e0ffa..89cf31c 100644 --- a/src/util/i18n.tsx +++ b/src/util/i18n.tsx @@ -15,8 +15,13 @@ import { reactI18nextModule } from "react-i18next"; import config from "../config"; import languages from "../languages.json"; -export function setupI18n() { - i18n +/** + * Set up the internationalization service + * + * @param ajax An optional ajax function to use when fetching translations + */ +export function setupI18n(ajax?: any) { + return i18n .use(XHR) .use(LanguageDetector) .use(reactI18nextModule) @@ -46,7 +51,8 @@ export function setupI18n() { escapeValue: false }, backend: { - loadPath: process.env.PUBLIC_URL + "/i18n/{{lng}}/{{ns}}.json" + loadPath: process.env.PUBLIC_URL + "/i18n/{{lng}}/{{ns}}.json", + ajax }, react: { // Wait until translations are loaded before rendering From a942e831322b546649790f12535a7656ded149a9 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sun, 14 Apr 2019 15:26:49 -0700 Subject: [PATCH 14/16] Fix issues caused by merge conflicts Signed-off-by: Mcat12 --- src/config.development.tsx | 2 +- src/config.production.tsx | 2 +- src/config.tsx | 2 +- src/index.tsx | 2 +- src/util/CancelablePromise.tsx | 17 ----------------- src/util/basePath.ts | 26 ++++++++++++++++++++++++++ 6 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 src/util/basePath.ts diff --git a/src/config.development.tsx b/src/config.development.tsx index 37b7cd7..d9e4d4f 100644 --- a/src/config.development.tsx +++ b/src/config.development.tsx @@ -13,5 +13,5 @@ import { Config } from "./config"; export default { developmentMode: true, fakeAPI: false, - apiPath: process.env.PUBLIC_URL + apiPath: process.env.PUBLIC_URL + "/api" } as Config; diff --git a/src/config.production.tsx b/src/config.production.tsx index 97660fd..5bfc186 100644 --- a/src/config.production.tsx +++ b/src/config.production.tsx @@ -13,5 +13,5 @@ import { Config } from "./config"; export default { developmentMode: false, fakeAPI: false, - apiPath: "/admin/api" + apiPath: process.env.PUBLIC_URL + "/api" } as Config; diff --git a/src/config.tsx b/src/config.tsx index e021d03..db0dccd 100644 --- a/src/config.tsx +++ b/src/config.tsx @@ -27,7 +27,7 @@ if (process.env.NODE_ENV === "development") { if (process.env.REACT_APP_FAKE_API) { config.fakeAPI = true; - config.apiPath += "/fakeAPI"; + config.apiPath = process.env.PUBLIC_URL + "/fakeAPI"; } export default config; diff --git a/src/index.tsx b/src/index.tsx index 81b85c6..fcb1712 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -19,7 +19,7 @@ import "./scss/style.css"; import Full from "./containers/Full"; import api from "./util/api"; import { setupI18n } from "./util/i18n"; -import { getBasePath } from "./util"; +import { getBasePath } from "./util/basePath"; // Before rendering anything, check if there is a session cookie. // Note: the user could have an old session, so the first API call diff --git a/src/util/CancelablePromise.tsx b/src/util/CancelablePromise.tsx index 622358e..2bda5bf 100644 --- a/src/util/CancelablePromise.tsx +++ b/src/util/CancelablePromise.tsx @@ -8,23 +8,6 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -/** - * Get the base path of the web interface. The API will inject a base element - * for this purpose, but if the web interface is not hosted by the API, it will - * fall back to the public URL set by Create React App. - * - * @returns The base path to use - */ -export const getBasePath = (): string => { - const baseElement = document.getElementsByTagName("base")[0]; - - if (baseElement) { - return new URL(baseElement.href).pathname; - } else { - return process.env.PUBLIC_URL; - } -}; - /** * A promise which can be canceled */ diff --git a/src/util/basePath.ts b/src/util/basePath.ts new file mode 100644 index 0000000..8caa571 --- /dev/null +++ b/src/util/basePath.ts @@ -0,0 +1,26 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Provide the base path for relative paths + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +/** + * Get the base path of the web interface. The API will inject a base element + * for this purpose, but if the web interface is not hosted by the API, it will + * fall back to the public URL set by Create React App. + * + * @returns The base path to use + */ +export const getBasePath = (): string => { + const baseElement = document.getElementsByTagName("base")[0]; + + if (baseElement) { + return new URL(baseElement.href).pathname; + } else { + return process.env.PUBLIC_URL; + } +}; From 9326988168b478e6ab80db1b304d91a99515047d Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sun, 14 Apr 2019 15:33:54 -0700 Subject: [PATCH 15/16] Add tests for getBasePath Signed-off-by: Mcat12 --- src/util/__tests__/basePath.test.tsx | 31 ++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 src/util/__tests__/basePath.test.tsx diff --git a/src/util/__tests__/basePath.test.tsx b/src/util/__tests__/basePath.test.tsx new file mode 100644 index 0000000..5ec92be --- /dev/null +++ b/src/util/__tests__/basePath.test.tsx @@ -0,0 +1,31 @@ +/* Pi-hole: A black hole for Internet advertisements + * (c) 2019 Pi-hole, LLC (https://pi-hole.net) + * Network-wide ad blocking via your own hardware. + * + * Web Interface + * Tests for base path function + * + * This file is copyright under the latest version of the EUPL. + * Please see LICENSE file for your rights under this license. */ + +import { getBasePath } from "../basePath"; + +it("should return the public URL when there is no base", () => { + const publicUrl = process.env.PUBLIC_URL; + + const basePath = getBasePath(); + + expect(basePath).toEqual(publicUrl); +}); + +it("should return the path from the base element when it exists", () => { + const expectedBasePath = "/admin"; + const baseElement = document.createElement("base"); + baseElement.href = expectedBasePath; + document.head.appendChild(baseElement); + + const actualBasePath = getBasePath(); + + document.head.removeChild(baseElement); + expect(actualBasePath).toEqual(expectedBasePath); +}); From 0a565b9259be320a8ac1ebaf37fd630e209edaab Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sun, 14 Apr 2019 15:50:47 -0700 Subject: [PATCH 16/16] Fix languages.json import error when running tests Signed-off-by: Mcat12 --- src/util/__tests__/i18n.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/__tests__/i18n.test.tsx b/src/util/__tests__/i18n.test.tsx index 64539a1..ab58f4f 100644 --- a/src/util/__tests__/i18n.test.tsx +++ b/src/util/__tests__/i18n.test.tsx @@ -20,6 +20,9 @@ jest.mock("react-i18next", () => ({ } })); +// languages.json is generated during a build or run, so it may not exist yet +jest.mock("../../languages.json", () => [], { virtual: true }); + it("configures i18n successfully", async () => { // Provide a mock ajax function to the XHR backend const fakeAjax = (url: any, options: any, callback: any) => callback("", {});