From 1f8f66190bdfa0d3e93d6769aca044d06d0632d5 Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 11:23:48 -0500 Subject: [PATCH 1/8] Add env variable + some modest documentation Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/README.md | 6 ++++++ web/vtadmin/src/react-app-env.d.ts | 1 + 2 files changed, 7 insertions(+) diff --git a/web/vtadmin/README.md b/web/vtadmin/README.md index 7c7f57778c6..5fc6d2e711e 100644 --- a/web/vtadmin/README.md +++ b/web/vtadmin/README.md @@ -31,6 +31,12 @@ Scripts for common and not-so-common tasks. These are always run from the `vites - [TypeScript](http://typescriptlang.org/) - [protobufjs](https://github.com/protobufjs) +## Environment Variables + +Under the hood, we use create-react-app's environment variable set-up which is very well documented: https://create-react-app.dev/docs/adding-custom-environment-variables. + +All of our environment variables are enumerated and commented in [react-app-env.d.ts](./src/react-app-env.d.ts). This also gives us type hinting on `process.env`, for editors that support it. + ## Configuring your editor ### VS Code diff --git a/web/vtadmin/src/react-app-env.d.ts b/web/vtadmin/src/react-app-env.d.ts index ae651b93b89..0e50a7a2fb4 100644 --- a/web/vtadmin/src/react-app-env.d.ts +++ b/web/vtadmin/src/react-app-env.d.ts @@ -3,6 +3,7 @@ declare namespace NodeJS { interface ProcessEnv { NODE_ENV: 'development' | 'production' | 'test'; PUBLIC_URL: string; + REACT_APP_VTADMIN_API_ADDRESS: string; } } From a7c45cb14df8ae76251fde9f363d99908d5829ee Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 13:17:07 -0500 Subject: [PATCH 2/8] A first pass at testing fetch with msw Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/package-lock.json | 185 ++++++++++++++++++++++++++++++- web/vtadmin/package.json | 1 + web/vtadmin/src/api/http.test.ts | 129 +++++++++++++++++++++ web/vtadmin/src/api/http.ts | 54 +++++++++ 4 files changed, 364 insertions(+), 5 deletions(-) create mode 100644 web/vtadmin/src/api/http.test.ts create mode 100644 web/vtadmin/src/api/http.ts diff --git a/web/vtadmin/package-lock.json b/web/vtadmin/package-lock.json index d61392a8ecd..3cdb7125c8c 100644 --- a/web/vtadmin/package-lock.json +++ b/web/vtadmin/package-lock.json @@ -1798,6 +1798,12 @@ } } }, + "@open-draft/until": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/@open-draft/until/-/until-1.0.3.tgz", + "integrity": "sha512-Aq58f5HiWdyDlFffbbSjAlv596h/cOnt2DO1w3DOC7OJ5EHs0hd/nycJfiu9RJbT6Yk6F1knnRRXNSpxoIVZ9Q==", + "dev": true + }, "@pmmmwh/react-refresh-webpack-plugin": { "version": "0.4.2", "resolved": "https://registry.npmjs.org/@pmmmwh/react-refresh-webpack-plugin/-/react-refresh-webpack-plugin-0.4.2.tgz", @@ -2281,6 +2287,12 @@ "@babel/types": "^7.3.0" } }, + "@types/cookie": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.4.0.tgz", + "integrity": "sha512-y7mImlc/rNkvCRmg8gC3/lj87S7pTUIJ6QGjwHR9WQJcFs+ZMTOaoPrkdFA/YdbuqVEmEbb5RdhVxMkAcgOnpg==", + "dev": true + }, "@types/eslint": { "version": "7.2.6", "resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-7.2.6.tgz", @@ -3698,8 +3710,7 @@ "binary-extensions": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/binary-extensions/-/binary-extensions-2.1.0.tgz", - "integrity": "sha512-1Yj8h9Q+QDF5FzhMs/c9+6UntbD5MkRfRwac8DoEm9ZfUBZ7tZ55YcGVAzEe4bXsdQHEk+s9S5wsOKVdZrw0tQ==", - "optional": true + "integrity": "sha512-1Yj8h9Q+QDF5FzhMs/c9+6UntbD5MkRfRwac8DoEm9ZfUBZ7tZ55YcGVAzEe4bXsdQHEk+s9S5wsOKVdZrw0tQ==" }, "bindings": { "version": "1.5.0", @@ -4146,7 +4157,6 @@ "version": "3.4.3", "resolved": "https://registry.npmjs.org/chokidar/-/chokidar-3.4.3.tgz", "integrity": "sha512-DtM3g7juCXQxFVSNPNByEC2+NImtBuxQQvWlHunpJIS5Ocr0lG306cC7FCi7cEA0fzmybPUIl4txBIobk1gGOQ==", - "optional": true, "requires": { "anymatch": "~3.1.1", "braces": "~3.0.2", @@ -7293,6 +7303,12 @@ "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.4.tgz", "integrity": "sha512-WjKPNJF79dtJAVniUlGGWHYGz2jWxT6VhN/4m1NdkbZ2nOsEF+cI1Edgql5zCRhs/VsQYRvrXctxktVXZUkixw==" }, + "graphql": { + "version": "15.4.0", + "resolved": "https://registry.npmjs.org/graphql/-/graphql-15.4.0.tgz", + "integrity": "sha512-EB3zgGchcabbsU9cFe1j+yxdzKQKAbGUWRb13DsrsMN1yyfmmIq+2+L5MqVWcDCE4V89R5AyUOi7sMOGxdsYtA==", + "dev": true + }, "growly": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/growly/-/growly-1.3.0.tgz", @@ -7454,6 +7470,12 @@ "resolved": "https://registry.npmjs.org/he/-/he-1.2.0.tgz", "integrity": "sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==" }, + "headers-utils": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/headers-utils/-/headers-utils-1.2.0.tgz", + "integrity": "sha512-4/BMXcWrJErw7JpM87gF8MNEXcIMLzepYZjNRv/P9ctgupl2Ywa3u1PgHtNhSRq84bHH9Ndlkdy7bSi+bZ9I9A==", + "dev": true + }, "hex-color-regex": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/hex-color-regex/-/hex-color-regex-1.1.0.tgz", @@ -8049,7 +8071,6 @@ "version": "2.1.0", "resolved": "https://registry.npmjs.org/is-binary-path/-/is-binary-path-2.1.0.tgz", "integrity": "sha512-ZMERYes6pDydyuGidse7OsHxtbI7WVeUEozgR/g7rd0xUimYNlvZRE/K2MgZTjWy725IfelLeVcEM97mmtRGXw==", - "optional": true, "requires": { "binary-extensions": "^2.0.0" } @@ -10845,6 +10866,138 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, + "msw": { + "version": "0.24.4", + "resolved": "https://registry.npmjs.org/msw/-/msw-0.24.4.tgz", + "integrity": "sha512-RKPAyfOrEVwritMn6RL5QROYhZMeRVApGuj4JdIQh/uyV0ASx349wsARJVI4VAXGEKu4B1TBvJZf0P0iVMsheg==", + "dev": true, + "requires": { + "@open-draft/until": "^1.0.3", + "@types/cookie": "^0.4.0", + "chalk": "^4.1.0", + "chokidar": "^3.4.2", + "cookie": "^0.4.1", + "graphql": "^15.4.0", + "headers-utils": "^1.2.0", + "node-fetch": "^2.6.1", + "node-match-path": "^0.6.0", + "node-request-interceptor": "^0.5.3", + "statuses": "^2.0.0", + "yargs": "^16.2.0" + }, + "dependencies": { + "ansi-styles": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz", + "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==", + "dev": true, + "requires": { + "color-convert": "^2.0.1" + } + }, + "chalk": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-4.1.0.tgz", + "integrity": "sha512-qwx12AxXe2Q5xQ43Ac//I6v5aXTipYrSESdOgzrN+9XjgEpyjpKuvSGaN4qE93f7TQTlerQQ8S+EQ0EyDoVL1A==", + "dev": true, + "requires": { + "ansi-styles": "^4.1.0", + "supports-color": "^7.1.0" + } + }, + "cliui": { + "version": "7.0.4", + "resolved": "https://registry.npmjs.org/cliui/-/cliui-7.0.4.tgz", + "integrity": "sha512-OcRE68cOsVMXp1Yvonl/fzkQOyjLSu/8bhPDfQt0e0/Eb283TKP20Fs2MqoPsr9SwA595rRCA+QMzYc9nBP+JQ==", + "dev": true, + "requires": { + "string-width": "^4.2.0", + "strip-ansi": "^6.0.0", + "wrap-ansi": "^7.0.0" + } + }, + "color-convert": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-2.0.1.tgz", + "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==", + "dev": true, + "requires": { + "color-name": "~1.1.4" + } + }, + "color-name": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/color-name/-/color-name-1.1.4.tgz", + "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==", + "dev": true + }, + "cookie": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.4.1.tgz", + "integrity": "sha512-ZwrFkGJxUR3EIoXtO+yVE69Eb7KlixbaeAWfBQB9vVsNn/o+Yw69gBWSSDK825hQNdN+wF8zELf3dFNl/kxkUA==", + "dev": true + }, + "has-flag": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", + "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", + "dev": true + }, + "statuses": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", + "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", + "dev": true + }, + "supports-color": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", + "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", + "dev": true, + "requires": { + "has-flag": "^4.0.0" + } + }, + "wrap-ansi": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-7.0.0.tgz", + "integrity": "sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==", + "dev": true, + "requires": { + "ansi-styles": "^4.0.0", + "string-width": "^4.1.0", + "strip-ansi": "^6.0.0" + } + }, + "y18n": { + "version": "5.0.5", + "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.5.tgz", + "integrity": "sha512-hsRUr4FFrvhhRH12wOdfs38Gy7k2FFzB9qgN9v3aLykRq0dRcdcpz5C9FxdS2NuhOrI/628b/KSTJ3rwHysYSg==", + "dev": true + }, + "yargs": { + "version": "16.2.0", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-16.2.0.tgz", + "integrity": "sha512-D1mvvtDG0L5ft/jGWkLpG1+m0eQxOfaBvTNELraWj22wSVUMWxZUvYgJYcKh6jGGIkJFhH4IZPQhR4TKpc8mBw==", + "dev": true, + "requires": { + "cliui": "^7.0.2", + "escalade": "^3.1.1", + "get-caller-file": "^2.0.5", + "require-directory": "^2.1.1", + "string-width": "^4.2.0", + "y18n": "^5.0.5", + "yargs-parser": "^20.2.2" + } + }, + "yargs-parser": { + "version": "20.2.4", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.4.tgz", + "integrity": "sha512-WOkpgNhPTlE73h4VFAFsOnomJVaovO8VqLDzy5saChRBFQFBoMYirowyW+Q9HB4HFF4Z7VZTiG3iSzJJA29yRA==", + "dev": true + } + } + }, "multicast-dns": { "version": "6.2.3", "resolved": "https://registry.npmjs.org/multicast-dns/-/multicast-dns-6.2.3.tgz", @@ -10936,6 +11089,12 @@ } } }, + "node-fetch": { + "version": "2.6.1", + "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.1.tgz", + "integrity": "sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw==", + "dev": true + }, "node-forge": { "version": "0.10.0", "resolved": "https://registry.npmjs.org/node-forge/-/node-forge-0.10.0.tgz", @@ -11051,6 +11210,12 @@ } } }, + "node-match-path": { + "version": "0.6.0", + "resolved": "https://registry.npmjs.org/node-match-path/-/node-match-path-0.6.0.tgz", + "integrity": "sha512-mld1LbiLaufULAYFPAWgNEG4P0ccL49otlL/nbF5VBQLATuzfS1BGYV1rjRMsxbc0vcnasikFqGHoKDFMQylMw==", + "dev": true + }, "node-modules-regexp": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/node-modules-regexp/-/node-modules-regexp-1.0.0.tgz", @@ -11086,6 +11251,17 @@ "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-1.1.67.tgz", "integrity": "sha512-V5QF9noGFl3EymEwUYzO+3NTDpGfQB4ve6Qfnzf3UNydMhjQRVPR1DZTuvWiLzaFJYw2fmDwAfnRNEVb64hSIg==" }, + "node-request-interceptor": { + "version": "0.5.9", + "resolved": "https://registry.npmjs.org/node-request-interceptor/-/node-request-interceptor-0.5.9.tgz", + "integrity": "sha512-M1a3aulCW/kqajDn/w+qBX86G4So7utJGlrODAjQ1piz/kR8ZaDfd/wrJnsuPtUM12F0YxsnXG8qRKFkIEIxsw==", + "dev": true, + "requires": { + "@open-draft/until": "^1.0.3", + "debug": "^4.3.0", + "headers-utils": "^1.2.0" + } + }, "node-sass": { "version": "4.14.1", "resolved": "https://registry.npmjs.org/node-sass/-/node-sass-4.14.1.tgz", @@ -13551,7 +13727,6 @@ "version": "3.5.0", "resolved": "https://registry.npmjs.org/readdirp/-/readdirp-3.5.0.tgz", "integrity": "sha512-cMhu7c/8rdhkHXWsY+osBhfSy0JikwpHK/5+imo+LpeasTF8ouErHrlYkwT0++njiyuDvc7OFY5T3ukvZ8qmFQ==", - "optional": true, "requires": { "picomatch": "^2.2.1" } diff --git a/web/vtadmin/package.json b/web/vtadmin/package.json index adc2c3f2cf6..89bf0db4968 100644 --- a/web/vtadmin/package.json +++ b/web/vtadmin/package.json @@ -54,6 +54,7 @@ ] }, "devDependencies": { + "msw": "^0.24.4", "prettier": "^2.2.1", "protobufjs": "^6.10.2", "stylelint": "^13.8.0", diff --git a/web/vtadmin/src/api/http.test.ts b/web/vtadmin/src/api/http.test.ts new file mode 100644 index 00000000000..1ee3c8a9bd9 --- /dev/null +++ b/web/vtadmin/src/api/http.test.ts @@ -0,0 +1,129 @@ +/** + * Copyright 2021 The Vitess Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; + +import * as api from './http'; +import { vtadmin as pb } from '../proto/vtadmin'; + +const SERVER_ADDRESS = ''; +process.env.REACT_APP_VTADMIN_API_ADDRESS = SERVER_ADDRESS; + +const server = setupServer(); + +const mockServerJson = (endpoint: string, json: object) => { + server.use(rest.get(endpoint, (req, res, ctx) => res(ctx.json(json)))); +}; + +// Enable API mocking before tests. +beforeAll(() => server.listen()); + +// Reset any runtime request handlers we may add during the tests. +afterEach(() => server.resetHandlers()); + +// Disable API mocking after the tests are done. +afterAll(() => server.close()); + +describe('api/http', () => { + describe('vtfetch', () => { + it('parses and returns JSON, given an HttpOkResponse response', async () => { + const endpoint = `/api/tablets`; + const response = { ok: true, result: null }; + mockServerJson(endpoint, response); + + const result = await api.vtfetch(endpoint); + expect(result).toEqual(response); + }); + + it('parses and returns JSON, given an HttpErrorResponse response', async () => { + const endpoint = `/api/tablets`; + const response = { ok: false }; + mockServerJson(endpoint, response); + + const result = await api.vtfetch(endpoint); + expect(result).toEqual(response); + }); + + it('throws an error on malformed JSON', async () => { + const endpoint = `/api/tablets`; + server.use(rest.get(endpoint, (req, res, ctx) => res(ctx.body('this is fine')))); + + expect.assertions(2); + + try { + await api.vtfetch(endpoint); + } catch (e) { + /* eslint-disable jest/no-conditional-expect */ + expect(e.name).toEqual('SyntaxError'); + expect(e.message.startsWith('Unexpected token')).toBeTruthy(); + /* eslint-enable jest/no-conditional-expect */ + } + }); + + it('throws an error on malformed response envelopes', async () => { + const endpoint = `/api/tablets`; + mockServerJson(endpoint, { foo: 'bar' }); + + expect.assertions(1); + + try { + await api.vtfetch(endpoint); + } catch (e) { + /* eslint-disable jest/no-conditional-expect */ + expect(e.message).toEqual('invalid http envelope'); + /* eslint-enable jest/no-conditional-expect */ + } + }); + }); + + describe('fetchTablets', () => { + it('returns a list of Tablets, given a successful response', async () => { + const t0 = pb.Tablet.create({ tablet: { hostname: 't0' } }); + const t1 = pb.Tablet.create({ tablet: { hostname: 't1' } }); + const t2 = pb.Tablet.create({ tablet: { hostname: 't2' } }); + const tablets = [t0, t1, t2]; + + mockServerJson(`/api/tablets`, { + ok: true, + result: { + tablets: tablets.map((t) => t.toJSON()), + }, + }); + + const result = await api.fetchTablets(); + expect(result).toEqual(tablets); + }); + + it('throws an error if JSON cannot be unmarshalled into Tablet objects', async () => { + mockServerJson(`/api/tablets`, { + ok: true, + result: { + tablets: [{ cluster: 'this should be an object, not a string' }], + }, + }); + + expect.assertions(1); + + try { + await api.fetchTablets(); + } catch (e) { + /* eslint-disable jest/no-conditional-expect */ + expect(e.message).toEqual('cluster.object expected'); + /* eslint-enable jest/no-conditional-expect */ + } + }); + }); +}); diff --git a/web/vtadmin/src/api/http.ts b/web/vtadmin/src/api/http.ts new file mode 100644 index 00000000000..2bfd70b4c86 --- /dev/null +++ b/web/vtadmin/src/api/http.ts @@ -0,0 +1,54 @@ +/** + * Copyright 2021 The Vitess Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { vtadmin as pb } from '../proto/vtadmin'; + +interface HttpOkResponse { + ok: true; + result: any; +} + +interface HttpErrorResponse { + ok: false; +} + +type HttpResponse = HttpOkResponse | HttpErrorResponse; + +export const vtfetch = async (endpoint: string): Promise => { + const url = `${process.env.REACT_APP_VTADMIN_API_ADDRESS}${endpoint}`; + const response = await fetch(url); + + const json = await response.json(); + if (!('ok' in json)) throw Error('invalid http envelope'); + + return json as HttpResponse; +}; + +export const fetchTablets = async () => { + const res = await vtfetch('/api/tablets'); + if (!res.ok) throw Error('not ok'); + + const { result } = res; + if (!Array.isArray(result.tablets)) throw Error('no tablets'); + + const tablets: pb.Tablet[] = result.tablets.map((t: any) => { + const err = pb.Tablet.verify(t); + if (err) throw Error(err); + + return pb.Tablet.create(t); + }); + return tablets; +}; From 073dc1c87375aca86e27634011829d27ef7b0de0 Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 13:29:01 -0500 Subject: [PATCH 3/8] Add react-query + populate UI from API Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/package-lock.json | 23 +++++++++ web/vtadmin/package.json | 1 + web/vtadmin/src/components/App.tsx | 77 ++---------------------------- web/vtadmin/src/hooks/api.ts | 8 ++++ web/vtadmin/src/index.tsx | 8 +++- 5 files changed, 44 insertions(+), 73 deletions(-) create mode 100644 web/vtadmin/src/hooks/api.ts diff --git a/web/vtadmin/package-lock.json b/web/vtadmin/package-lock.json index 3cdb7125c8c..cb25ffa0647 100644 --- a/web/vtadmin/package-lock.json +++ b/web/vtadmin/package-lock.json @@ -10367,6 +10367,15 @@ "object-visit": "^1.0.0" } }, + "match-sorter": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/match-sorter/-/match-sorter-6.1.0.tgz", + "integrity": "sha512-sKPMf4kbF7Dm5Crx0bbfLpokK68PUJ/0STUIOPa1ZmTZEA3lCaPK3gapQR573oLmvdkTfGojzySkIwuq6Z6xRQ==", + "requires": { + "@babel/runtime": "^7.12.5", + "remove-accents": "0.4.2" + } + }, "mathml-tag-names": { "version": "2.1.3", "resolved": "https://registry.npmjs.org/mathml-tag-names/-/mathml-tag-names-2.1.3.tgz", @@ -13563,6 +13572,15 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.13.1.tgz", "integrity": "sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==" }, + "react-query": { + "version": "3.5.9", + "resolved": "https://registry.npmjs.org/react-query/-/react-query-3.5.9.tgz", + "integrity": "sha512-thlxrnl7cDg6qmk+N2ADjDVDJkoU3c7ZFJivYph0XoBDgkRIpb3A+tpqH7o6gu7JXZum9lfX1o294UfYfTiwvg==", + "requires": { + "@babel/runtime": "^7.5.5", + "match-sorter": "^6.0.2" + } + }, "react-refresh": { "version": "0.8.3", "resolved": "https://registry.npmjs.org/react-refresh/-/react-refresh-0.8.3.tgz", @@ -13889,6 +13907,11 @@ "mdast-util-to-markdown": "^0.6.0" } }, + "remove-accents": { + "version": "0.4.2", + "resolved": "https://registry.npmjs.org/remove-accents/-/remove-accents-0.4.2.tgz", + "integrity": "sha1-CkPTqq4egNuRngeuJUsoXZ4ce7U=" + }, "remove-trailing-separator": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/remove-trailing-separator/-/remove-trailing-separator-1.1.0.tgz", diff --git a/web/vtadmin/package.json b/web/vtadmin/package.json index 89bf0db4968..7766bf30968 100644 --- a/web/vtadmin/package.json +++ b/web/vtadmin/package.json @@ -17,6 +17,7 @@ "node-sass": "^4.14.1", "react": "^17.0.1", "react-dom": "^17.0.1", + "react-query": "^3.5.9", "react-scripts": "4.0.1", "typescript": "^4.1.3", "web-vitals": "^0.2.4" diff --git a/web/vtadmin/src/components/App.tsx b/web/vtadmin/src/components/App.tsx index 8c74240c390..5c8be10333d 100644 --- a/web/vtadmin/src/components/App.tsx +++ b/web/vtadmin/src/components/App.tsx @@ -18,84 +18,17 @@ import * as React from 'react'; import style from './App.module.scss'; import logo from '../img/vitess-icon-color.svg'; import { TabletList } from './TabletList'; -import { vtadmin as pb, topodata } from '../proto/vtadmin'; +import { useTablets } from '../hooks/api'; export const App = () => { + const { data, isLoading } = useTablets(); + return (
logo

VTAdmin

- + + {isLoading ?
Loading...
: }
); }; - -// Add some fake data solely for this super duper simple hello world example. -// When we do this for reals, we'll be fetching cluster JSON over HTTP -// from vtadmin-api and casting/validating the responses to Cluster objects. -// This requires merging https://github.com/vitessio/vitess/pull/7187 first. -// -// Long-term, we can use grpc in the browser with a library like https://github.com/grpc/grpc-web, -// which will obviate the casting/validation steps, too. -const fakeTablets = [ - { - cluster: { id: 'prod', name: 'prod' }, - tablet: { - hostname: 'tablet-prod-commerce-00-00-abcd', - keyspace: 'commerce', - shard: '-', - type: topodata.TabletType.MASTER, - }, - state: pb.Tablet.ServingState.SERVING, - }, - { - cluster: { id: 'prod', name: 'prod' }, - tablet: { - hostname: 'tablet-prod-commerce-00-00-efgh', - keyspace: 'commerce', - shard: '-', - type: topodata.TabletType.REPLICA, - }, - state: pb.Tablet.ServingState.SERVING, - }, - { - cluster: { id: 'prod', name: 'prod' }, - tablet: { - hostname: 'tablet-prod-commerce-00-00-ijkl', - keyspace: 'commerce', - shard: '-', - type: topodata.TabletType.REPLICA, - }, - state: pb.Tablet.ServingState.SERVING, - }, - { - cluster: { id: 'dev', name: 'dev' }, - tablet: { - hostname: 'tablet-dev-commerce-00-00-mnop', - keyspace: 'commerce', - shard: '-', - type: topodata.TabletType.MASTER, - }, - state: pb.Tablet.ServingState.SERVING, - }, - { - cluster: { id: 'dev', name: 'dev' }, - tablet: { - hostname: 'tablet-dev-commerce-00-00-qrst', - keyspace: 'commerce', - shard: '-', - type: topodata.TabletType.REPLICA, - }, - state: pb.Tablet.ServingState.SERVING, - }, - { - cluster: { id: 'dev', name: 'dev' }, - tablet: { - hostname: 'tablet-dev-commerce-00-00-uvwx', - keyspace: 'commerce', - shard: '-', - type: topodata.TabletType.REPLICA, - }, - state: pb.Tablet.ServingState.SERVING, - }, -].map((opts) => pb.Tablet.create(opts)); diff --git a/web/vtadmin/src/hooks/api.ts b/web/vtadmin/src/hooks/api.ts new file mode 100644 index 00000000000..b12216dde87 --- /dev/null +++ b/web/vtadmin/src/hooks/api.ts @@ -0,0 +1,8 @@ +import { useQuery } from 'react-query'; +import { fetchTablets } from '../api/http'; + +export const useTablets = () => { + return useQuery(['tablets'], async () => { + return await fetchTablets(); + }); +}; diff --git a/web/vtadmin/src/index.tsx b/web/vtadmin/src/index.tsx index e6ba2e48a86..fdfa229a4a5 100644 --- a/web/vtadmin/src/index.tsx +++ b/web/vtadmin/src/index.tsx @@ -15,12 +15,18 @@ */ import React from 'react'; import ReactDOM from 'react-dom'; +import { QueryClient, QueryClientProvider } from 'react-query'; + import './index.css'; import { App } from './components/App'; +const queryClient = new QueryClient(); + ReactDOM.render( - + + + , document.getElementById('root') ); From 92681cae03d37c2481f0c3d50c6ee9ef7facce1b Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 13:43:17 -0500 Subject: [PATCH 4/8] Add a bunch of commentary about MSW in http.test.ts Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/src/api/http.test.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/web/vtadmin/src/api/http.test.ts b/web/vtadmin/src/api/http.test.ts index 1ee3c8a9bd9..62439e1e334 100644 --- a/web/vtadmin/src/api/http.test.ts +++ b/web/vtadmin/src/api/http.test.ts @@ -19,9 +19,27 @@ import { setupServer } from 'msw/node'; import * as api from './http'; import { vtadmin as pb } from '../proto/vtadmin'; -const SERVER_ADDRESS = ''; -process.env.REACT_APP_VTADMIN_API_ADDRESS = SERVER_ADDRESS; - +// This test suite uses Mock Service Workers (https://github.com/mswjs/msw) +// to mock HTTP responses from vtadmin-api. +// +// MSW lets us intercept requests at the network level. This decouples the tests from +// whatever particular HTTP fetcher interface we are using, and obviates the need +// to mock `fetch` directly (by using a library like jest-fetch-mock, for example). +// +// MSW gives us full control over the response, including edge cases like errors, +// malformed payloads, and timeouts. +// +// The big downside to mocking or "faking" APIs like vtadmin is that +// we end up re-implementing some (or all) of vtadmin-api in our test environment. +// It is, unfortunately, impossible to completely avoid this kind of duplication +// unless we solely use e2e tests (which have their own trade-offs). +// +// That said, our use of protobufjs to validate and strongly type HTTP responses +// means our fake is more robust than it would be otherwise. Since we are using +// the exact same protos in our fake as in our real vtadmin-api server, we're guaranteed +// to have type parity. +// +process.env.REACT_APP_VTADMIN_API_ADDRESS = ''; const server = setupServer(); const mockServerJson = (endpoint: string, json: object) => { From b0e678cb59c2df2ee78c5b28436fdc093c3a8e36 Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 14:08:53 -0500 Subject: [PATCH 5/8] More commentary in api/http.ts Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/src/api/http.test.ts | 14 ++++++++++++++ web/vtadmin/src/api/http.ts | 14 +++++++++++--- web/vtadmin/src/react-app-env.d.ts | 3 +++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/web/vtadmin/src/api/http.test.ts b/web/vtadmin/src/api/http.test.ts index 62439e1e334..638a0a691d9 100644 --- a/web/vtadmin/src/api/http.test.ts +++ b/web/vtadmin/src/api/http.test.ts @@ -125,6 +125,20 @@ describe('api/http', () => { expect(result).toEqual(tablets); }); + it('throws an error if result.tablets is not an array', async () => { + mockServerJson('/api/tablets', { ok: true, result: { tablets: null } }); + + expect.assertions(1); + + try { + await api.fetchTablets(); + } catch (e) { + /* eslint-disable jest/no-conditional-expect */ + expect(e.message).toEqual('expected tablets to be an array, got null'); + /* eslint-enable jest/no-conditional-expect */ + } + }); + it('throws an error if JSON cannot be unmarshalled into Tablet objects', async () => { mockServerJson(`/api/tablets`, { ok: true, diff --git a/web/vtadmin/src/api/http.ts b/web/vtadmin/src/api/http.ts index 2bfd70b4c86..5d0dd085655 100644 --- a/web/vtadmin/src/api/http.ts +++ b/web/vtadmin/src/api/http.ts @@ -27,6 +27,14 @@ interface HttpErrorResponse { type HttpResponse = HttpOkResponse | HttpErrorResponse; +// vtfetch makes HTTP requests against the given vtadmin-api endpoint +// and returns the parsed response. +// +// HttpResponse envelope types are not defined in vtadmin.proto (nor should they be) +// thus we have to validate the shape of the API response with more care. +// +// Note that this only validates the HttpResponse envelope; it does not +// do any type checking or validation on the result. export const vtfetch = async (endpoint: string): Promise => { const url = `${process.env.REACT_APP_VTADMIN_API_ADDRESS}${endpoint}`; const response = await fetch(url); @@ -42,13 +50,13 @@ export const fetchTablets = async () => { if (!res.ok) throw Error('not ok'); const { result } = res; - if (!Array.isArray(result.tablets)) throw Error('no tablets'); + const tablets = res.result?.tablets; + if (!Array.isArray(tablets)) throw Error(`expected tablets to be an array, got ${result.tablets}`); - const tablets: pb.Tablet[] = result.tablets.map((t: any) => { + return tablets.map((t: any) => { const err = pb.Tablet.verify(t); if (err) throw Error(err); return pb.Tablet.create(t); }); - return tablets; }; diff --git a/web/vtadmin/src/react-app-env.d.ts b/web/vtadmin/src/react-app-env.d.ts index 0e50a7a2fb4..56282a3b44f 100644 --- a/web/vtadmin/src/react-app-env.d.ts +++ b/web/vtadmin/src/react-app-env.d.ts @@ -3,6 +3,9 @@ declare namespace NodeJS { interface ProcessEnv { NODE_ENV: 'development' | 'production' | 'test'; PUBLIC_URL: string; + + // Required. The full address of vtadmin-api's HTTP interface. + // Example: "http://127.0.0.1:12345" REACT_APP_VTADMIN_API_ADDRESS: string; } } From b46e03d41fdb2c6748c2313091fca79bc7e6fe46 Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 14:22:32 -0500 Subject: [PATCH 6/8] Add current setup instructions (ugh) to the README (ugh) Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/README.md | 62 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/web/vtadmin/README.md b/web/vtadmin/README.md index 5fc6d2e711e..0f024212d1f 100644 --- a/web/vtadmin/README.md +++ b/web/vtadmin/README.md @@ -1,12 +1,64 @@ # VTAdmin -## Getting started +## Running vtadmin-web locally -1. Install node v.14.15.3. (Using a Node version manager like [nvm](https://github.com/nvm-sh/nvm) is highly recommended.) -1. From the `vitess/web/vtadmin/` directory, install dependencies with `npm install`. -1. Run `npm start` to start vtadmin-web on [http://localhost:3000](http://localhost:3000) +In this section, we'll get vtadmin-web, vtadmin-api, and Vitess all running locally. This process is still somewhat... cumbersome, apologies. 😰 -Have fun! 🎉 +1. Run Vitess locally [with Docker](https://vitess.io/docs/get-started/local-docker/) (or another way, if you prefer): + + ```bash + make docker_local + ./docker/local/run.sh + ``` + +1. Create an empty vtgate credentials file to avoid the gRPC dialer bug mentioned in https://github.com/vitessio/vitess/pull/7187. Location and filename don't matter since you'll be passing this in as a flag; I put mine at ` /Users/sarabee/id1-grpc_vtgate_credentials.json`: + + ```json + { + "Username": "", + "Password": "" + } + ``` + +1. Create a cluster configuration file for the local Vitess you started up in step 1. Again, filename and location don't matter since we'll be passing in the path as a flag; I put mine at `/Users/sarabee/vtadmin-cluster1.json`. Here it is with default values for the [local Vitess/Docker example](https://vitess.io/docs/get-started/local-docker/) we're following: + + ```json + { + "vtgates": [ + { + "host": { + "hostname": "127.0.0.1:15991" + }, + "tags": ["pool:pool1", "cell:zone1", "extra:tag"] + }, + { + "host": { + "hostname": "127.0.0.1:15992" + }, + "tags": ["dead-dove-do-not-eat"] + } + ] + } + ``` + +1. Start up vtadmin-api but **make sure to update the filepaths** for the vtgate creds file and static service discovery file you created above! + + ```bash + make build + + ./bin/vtadmin \ + --addr ":15999" \ + --cluster-defaults "vtsql-credentials-path-tmpl=/Users/sarabee/id1-grpc_vtgate_credentials.json" \ + --cluster "name=cluster1,id=id1,discovery=staticFile,discovery-staticFile-path=/Users/sarabee/vtadmin-cluster1.json,vtsql-discovery-tags=cell:zone1" + ``` + +1. Finally! Start up vtadmin-web on [http://localhost:3000](http://localhost:3000), pointed at the vtadmin-api server you started in the last step. + + ```bash + cd web/vtadmin + npm install + REACT_APP_VTADMIN_API_ADDRESS="http://127.0.0.1:15999" npm start + ``` # Developer guide From 776dbc6e0d726ea1a52de4bbfe939b5aba4dac3d Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 17:23:56 -0500 Subject: [PATCH 7/8] Extend Error for API erorrs + add placeholder error UI to the front-end Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/src/api/http.test.ts | 23 ++++++++++++++++++--- web/vtadmin/src/api/http.ts | 32 +++++++++++++++++++++++++++--- web/vtadmin/src/components/App.tsx | 16 +++++++++++++-- web/vtadmin/src/hooks/api.ts | 3 ++- 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/web/vtadmin/src/api/http.test.ts b/web/vtadmin/src/api/http.test.ts index 638a0a691d9..cfd319156a6 100644 --- a/web/vtadmin/src/api/http.test.ts +++ b/web/vtadmin/src/api/http.test.ts @@ -18,6 +18,7 @@ import { setupServer } from 'msw/node'; import * as api from './http'; import { vtadmin as pb } from '../proto/vtadmin'; +import { HTTP_RESPONSE_NOT_OK_ERROR, MALFORMED_HTTP_RESPONSE_ERROR } from './http'; // This test suite uses Mock Service Workers (https://github.com/mswjs/msw) // to mock HTTP responses from vtadmin-api. @@ -38,7 +39,6 @@ import { vtadmin as pb } from '../proto/vtadmin'; // means our fake is more robust than it would be otherwise. Since we are using // the exact same protos in our fake as in our real vtadmin-api server, we're guaranteed // to have type parity. -// process.env.REACT_APP_VTADMIN_API_ADDRESS = ''; const server = setupServer(); @@ -101,7 +101,7 @@ describe('api/http', () => { await api.vtfetch(endpoint); } catch (e) { /* eslint-disable jest/no-conditional-expect */ - expect(e.message).toEqual('invalid http envelope'); + expect(e.name).toEqual(MALFORMED_HTTP_RESPONSE_ERROR); /* eslint-enable jest/no-conditional-expect */ } }); @@ -125,6 +125,23 @@ describe('api/http', () => { expect(result).toEqual(tablets); }); + it('throws an error if response.ok is false', async () => { + const response = { ok: false }; + mockServerJson('/api/tablets', response); + + expect.assertions(3); + + try { + await api.fetchTablets(); + } catch (e) { + /* eslint-disable jest/no-conditional-expect */ + expect(e.name).toEqual(HTTP_RESPONSE_NOT_OK_ERROR); + expect(e.message).toEqual('/api/tablets'); + expect(e.response).toEqual(response); + /* eslint-enable jest/no-conditional-expect */ + } + }); + it('throws an error if result.tablets is not an array', async () => { mockServerJson('/api/tablets', { ok: true, result: { tablets: null } }); @@ -134,7 +151,7 @@ describe('api/http', () => { await api.fetchTablets(); } catch (e) { /* eslint-disable jest/no-conditional-expect */ - expect(e.message).toEqual('expected tablets to be an array, got null'); + expect(e.message).toMatch('expected tablets to be an array'); /* eslint-enable jest/no-conditional-expect */ } }); diff --git a/web/vtadmin/src/api/http.ts b/web/vtadmin/src/api/http.ts index 5d0dd085655..1da8d8c2dc0 100644 --- a/web/vtadmin/src/api/http.ts +++ b/web/vtadmin/src/api/http.ts @@ -27,6 +27,28 @@ interface HttpErrorResponse { type HttpResponse = HttpOkResponse | HttpErrorResponse; +export const MALFORMED_HTTP_RESPONSE_ERROR = 'MalformedHttpResponseError'; +class MalformedHttpResponseError extends Error { + responseJson: object; + + constructor(message: string, responseJson: object) { + super(message); + this.name = MALFORMED_HTTP_RESPONSE_ERROR; + this.responseJson = responseJson; + } +} + +export const HTTP_RESPONSE_NOT_OK_ERROR = 'HttpResponseNotOkError'; +class HttpResponseNotOkError extends Error { + response: HttpErrorResponse | null; + + constructor(endpoint: string, response: HttpErrorResponse) { + super(endpoint); + this.name = HTTP_RESPONSE_NOT_OK_ERROR; + this.response = response; + } +} + // vtfetch makes HTTP requests against the given vtadmin-api endpoint // and returns the parsed response. // @@ -40,14 +62,18 @@ export const vtfetch = async (endpoint: string): Promise => { const response = await fetch(url); const json = await response.json(); - if (!('ok' in json)) throw Error('invalid http envelope'); + if (!('ok' in json)) throw new MalformedHttpResponseError('invalid http envelope', json); return json as HttpResponse; }; export const fetchTablets = async () => { - const res = await vtfetch('/api/tablets'); - if (!res.ok) throw Error('not ok'); + const endpoint = '/api/tablets'; + const res = await vtfetch(endpoint); + + // Throw "not ok" responses so that react-query correctly interprets them as errors. + // See https://react-query.tanstack.com/guides/query-functions#handling-and-throwing-errors + if (!res.ok) throw new HttpResponseNotOkError(endpoint, res); const { result } = res; const tablets = res.result?.tablets; diff --git a/web/vtadmin/src/components/App.tsx b/web/vtadmin/src/components/App.tsx index 5c8be10333d..e6438e67daa 100644 --- a/web/vtadmin/src/components/App.tsx +++ b/web/vtadmin/src/components/App.tsx @@ -21,14 +21,26 @@ import { TabletList } from './TabletList'; import { useTablets } from '../hooks/api'; export const App = () => { - const { data, isLoading } = useTablets(); + const { data, error, isError, isSuccess } = useTablets(); + + // Placeholder UI :D + let content =
Loading...
; + if (isError) { + content = ( +
+ {error?.name}: {error?.message} +
+ ); + } else if (isSuccess) { + content = ; + } return (
logo

VTAdmin

- {isLoading ?
Loading...
: } + {content}
); }; diff --git a/web/vtadmin/src/hooks/api.ts b/web/vtadmin/src/hooks/api.ts index b12216dde87..c8c783ff439 100644 --- a/web/vtadmin/src/hooks/api.ts +++ b/web/vtadmin/src/hooks/api.ts @@ -1,8 +1,9 @@ import { useQuery } from 'react-query'; import { fetchTablets } from '../api/http'; +import { vtadmin as pb } from '../proto/vtadmin'; export const useTablets = () => { - return useQuery(['tablets'], async () => { + return useQuery(['tablets'], async () => { return await fetchTablets(); }); }; From cb6087c242b427d5fb6a7f281a6dd1d672db65c8 Mon Sep 17 00:00:00 2001 From: Sara Bee <855595+doeg@users.noreply.github.com> Date: Sun, 3 Jan 2021 20:01:36 -0500 Subject: [PATCH 8/8] Small nit (removes vestigial variable) Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com> --- web/vtadmin/src/api/http.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/web/vtadmin/src/api/http.ts b/web/vtadmin/src/api/http.ts index 1da8d8c2dc0..dceb9ae3e98 100644 --- a/web/vtadmin/src/api/http.ts +++ b/web/vtadmin/src/api/http.ts @@ -75,9 +75,8 @@ export const fetchTablets = async () => { // See https://react-query.tanstack.com/guides/query-functions#handling-and-throwing-errors if (!res.ok) throw new HttpResponseNotOkError(endpoint, res); - const { result } = res; const tablets = res.result?.tablets; - if (!Array.isArray(tablets)) throw Error(`expected tablets to be an array, got ${result.tablets}`); + if (!Array.isArray(tablets)) throw Error(`expected tablets to be an array, got ${tablets}`); return tablets.map((t: any) => { const err = pb.Tablet.verify(t);