From fc94fd9a3909d0d1634dc222180e1b0b21767843 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Mon, 18 Dec 2023 16:28:52 -0500 Subject: [PATCH] Replace postForm() with postFormData() Because the Calculator module now supports multiple String Calculator implementations, there's no need for the main form to contain an action attribute. The default './add' URL is now defined as calculators.DEFAULT_ENDPOINT, and is referenced in the new calculators.defaultPost(). Calculator now ends up calling defaultPost(), with no need to know the POST URL. As mentioned in the previous change, this will make it easy to add an implementation that talks to a separately running Tomcat backend eventually. The core change wasn't very large and invasive, despite the apparent size of the commit. Several of the additional changes were basic housekeeping and refactorings that made sense to include at once. Some of the additional changes and important details of the core change include: - Moves DocumentFragment + form comments from the deleted postForm() test into a new "Notes" section of strcalc/src/main/frontend/README.md. - Removes top-level `apiUrl` parameter, as the POST URL is now encapsulated in the calculator implementation function. - Deletes the 'impl' field from the FormData produced by the input form, since it's irrelevant to the implementation. It will actually cause the Tomcat backend to return a server error if it's included. - Deletes the now unused resolvedUrl() test helper. - Moved test/helpers.js to test/page-loader.js, since that's the only entity left in that module. - Moved setupFetchStub() from request.test.js to test/fetch-stub.js to reuse it in the new calculators.test.js. --- strcalc/src/main/frontend/README.md | 28 +++++++++++++ strcalc/src/main/frontend/components/app.js | 1 - .../main/frontend/components/calculator.hbs | 2 +- .../main/frontend/components/calculator.js | 12 ++++-- .../frontend/components/calculator.test.js | 30 ++++++++------ .../main/frontend/components/calculators.js | 10 +++-- .../frontend/components/calculators.test.js | 25 +++++++++++ .../src/main/frontend/components/request.js | 7 ++-- .../main/frontend/components/request.test.js | 41 ++++--------------- strcalc/src/main/frontend/main.js | 2 +- strcalc/src/main/frontend/main.test.js | 2 +- strcalc/src/main/frontend/test/fetch-stub.js | 30 ++++++++++++++ .../test/{helpers.js => page-loader.js} | 9 ---- 13 files changed, 129 insertions(+), 70 deletions(-) create mode 100644 strcalc/src/main/frontend/components/calculators.test.js create mode 100644 strcalc/src/main/frontend/test/fetch-stub.js rename strcalc/src/main/frontend/test/{helpers.js => page-loader.js} (97%) diff --git a/strcalc/src/main/frontend/README.md b/strcalc/src/main/frontend/README.md index a7b0abc..0706913 100644 --- a/strcalc/src/main/frontend/README.md +++ b/strcalc/src/main/frontend/README.md @@ -178,6 +178,30 @@ You'll need to be careful not to commit these changes. You can `git stash` them or run `git restore package.json`, followed by `pnpm i` to match the CI build again. +## Notes + +Some implementation notes I may want to work into documentation or blog +posts one day. + +### Document vs. DocumentFragment and <form> behavior + +Originally I tried using [DocumentFragment][] objects in the tests as much +as I could to avoid polluting the main [Document][]. However, differences in +[<form>][] behavior led me to implement a different scheme, adding and +removing unique [<div>][]s to and from the main Document instead. + +Specifically, `form.action` resolves differently depending on how it's created. +Elements in a DocumentFragment are in a separate DOM, causing the <form +action="/fetch"> attribute to be: + +- '/fetch' in jsdom +- '' in Chrome +- `${document.location.href}/fetch` in Firefox + +Creating a <form> element via `document.createElement()` causes +`form.action` to become `${document.location.href}/fetch` in every +environment. + [mbland/tomcat-servlet-testing-example]: https://github.com/mbland/tomcat-servlet-testing-example [top level README.md]: ../../../../README.md [Node.js]: https://nodejs.org/ @@ -204,3 +228,7 @@ again. [PowerShell]: https://learn.microsoft.com/powershell/ [`start`]: https://learn.microsoft.com/windows-server/administration/windows-commands/start [eslint-plugin-jsdoc]: https://www.npmjs.com/package/eslint-plugin-jsdoc +[DocumentFragment]: https://developer.mozilla.org/docs/Web/API/DocumentFragment +[Document]: https://developer.mozilla.org/docs/Web/API/Document +[<form>]: https://developer.mozilla.org/docs/Web/HTML/Element/form +[<div>]: https://developer.mozilla.org/docs/Web/HTML/Element/div diff --git a/strcalc/src/main/frontend/components/app.js b/strcalc/src/main/frontend/components/app.js index 94aa6bf..d955841 100644 --- a/strcalc/src/main/frontend/components/app.js +++ b/strcalc/src/main/frontend/components/app.js @@ -21,7 +21,6 @@ export default class App { * demonstrate how to design much larger applications for testability. * @param {object} params - parameters made available to all initializers * @param {Element} params.appElem - parent Element containing all components - * @param {string} params.apiUrl - API backend server URL * @param {object} params.calculators - calculator implementations */ init(params) { diff --git a/strcalc/src/main/frontend/components/calculator.hbs b/strcalc/src/main/frontend/components/calculator.hbs index 2406654..832956b 100644 --- a/strcalc/src/main/frontend/components/calculator.hbs +++ b/strcalc/src/main/frontend/components/calculator.hbs @@ -3,7 +3,7 @@ License, v. 2.0. If a copy of the MPL was not distributed with this file, You can obtain one at https://mozilla.org/MPL/2.0/. --}} -
+
diff --git a/strcalc/src/main/frontend/components/calculator.js b/strcalc/src/main/frontend/components/calculator.js index cd391f1..0f26f71 100644 --- a/strcalc/src/main/frontend/components/calculator.js +++ b/strcalc/src/main/frontend/components/calculator.js @@ -11,13 +11,12 @@ export default class Calculator { * Initializes the Calculator within the document. * @param {object} params - parameters made available to all initializers * @param {Element} params.appElem - parent Element containing all components - * @param {string} params.apiUrl - API backend server URL * @param {object} params.calculators - calculator implementations */ - init({ appElem, apiUrl, calculators }) { + init({ appElem, calculators }) { const calcOptions = Object.entries(calculators) .map(([k, v]) => ({ value: k, label: v.label })) - const t = Template({ apiUrl, calcOptions }) + const t = Template({ calcOptions }) const [ form, resultElem ] = t.children appElem.appendChild(t) @@ -32,11 +31,16 @@ export default class Calculator { event.preventDefault() const form = event.currentTarget + const data = new FormData(form) const selected = form.querySelector('input[name="impl"]:checked').value const result = resultElem.querySelector('p') + // None of the backends need the 'impl' parameter, and the Java backend + // will return a 500 if we send it. + data.delete('impl') + try { - const response = await calculators[selected].impl(form) + const response = await calculators[selected].impl(data) result.textContent = `Result: ${response.result}` } catch (err) { result.textContent = err diff --git a/strcalc/src/main/frontend/components/calculator.test.js b/strcalc/src/main/frontend/components/calculator.test.js index c7dfac7..e2921bb 100644 --- a/strcalc/src/main/frontend/components/calculator.test.js +++ b/strcalc/src/main/frontend/components/calculator.test.js @@ -7,7 +7,6 @@ import Calculator from './calculator' import { afterAll, afterEach, describe, expect, test, vi } from 'vitest' -import { resolvedUrl } from '../test/helpers' import StringCalculatorPage from '../test/page' // @vitest-environment jsdom @@ -15,16 +14,20 @@ describe('Calculator', () => { const page = StringCalculatorPage.new() const setup = () => { - const postForm = vi.fn() + const postFormData = vi.fn() const calculators = { - 'api': { label: 'API', impl: postForm }, + 'api': { label: 'API', impl: postFormData }, 'browser': { label: 'Browser', impl: () => {} } } - new Calculator().init({ - appElem: page.appElem, apiUrl: './add', calculators - }) - return { page, postForm } + new Calculator().init({ appElem: page.appElem, calculators }) + return { page, postFormData } + } + + const expectedFormData = (numbersString) => { + const data = new FormData() + data.append('numbers', numbersString) + return data } afterEach(() => page.clear()) @@ -34,25 +37,26 @@ describe('Calculator', () => { const { page } = setup() expect(page.form()).not.toBeNull() - expect(page.form().action).toBe(resolvedUrl('./add')) + expect(page.result()).not.toBeNull() }) test('updates result placeholder with successful result', async () => { - const { page, postForm } = setup() - postForm.mockResolvedValueOnce({ result: 5 }) + const { page, postFormData } = setup() + postFormData.mockResolvedValueOnce({ result: 5 }) const result = vi.waitFor(page.enterValueAndSubmit('2,2')) await expect(result).resolves.toBe('Result: 5') - expect(postForm).toHaveBeenCalledWith(page.form()) + expect(postFormData).toHaveBeenCalledWith(expectedFormData('2,2')) }) test('updates result placeholder with error message', async () => { - const { page, postForm } = setup() - postForm.mockRejectedValueOnce(new Error('D\'oh!')) + const { page, postFormData } = setup() + postFormData.mockRejectedValueOnce(new Error('D\'oh!')) const result = vi.waitFor(page.enterValueAndSubmit('2,2')) await expect(result).resolves.toBe('Error: D\'oh!') + expect(postFormData).toHaveBeenCalledWith(expectedFormData('2,2')) }) }) diff --git a/strcalc/src/main/frontend/components/calculators.js b/strcalc/src/main/frontend/components/calculators.js index 0b63352..6cd6d93 100644 --- a/strcalc/src/main/frontend/components/calculators.js +++ b/strcalc/src/main/frontend/components/calculators.js @@ -4,12 +4,16 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -import { postForm } from './request' +import { postFormData } from './request' + +export const DEFAULT_ENDPOINT = './add' + +const defaultPost = async (data)=> postFormData(DEFAULT_ENDPOINT, data) /** * Collection of production String Calculator implementations */ export default { - 'api': { label: 'Tomcat backend API (Java)', impl: postForm }, - 'browser': { label: 'In-browser (JavaScript)', impl: postForm } + 'api': { label: 'Tomcat backend API (Java)', impl: defaultPost }, + 'browser': { label: 'In-browser (JavaScript)', impl: defaultPost } } diff --git a/strcalc/src/main/frontend/components/calculators.test.js b/strcalc/src/main/frontend/components/calculators.test.js new file mode 100644 index 0000000..2082374 --- /dev/null +++ b/strcalc/src/main/frontend/components/calculators.test.js @@ -0,0 +1,25 @@ +/* eslint-env browser, node, jest, vitest */ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +import { default as calculators, DEFAULT_ENDPOINT } from './calculators' +import { afterEach, describe, expect, test, vi } from 'vitest' +import setupFetchStub from '../test/fetch-stub' +import { postOptions } from './request' + +describe('calculators', () => { + afterEach(() => { vi.unstubAllGlobals() }) + + test('defaultPost requests expected backend', async () => { + const data = new FormData() + const fetchStub = setupFetchStub(JSON.stringify({ ok: true })) + data.append('want', 'status') + + await expect(calculators.api.impl(data)).resolves.toEqual({ ok: true }) + expect(fetchStub).toHaveBeenCalledWith( + DEFAULT_ENDPOINT, postOptions({ want: 'status' })) + }) +}) diff --git a/strcalc/src/main/frontend/components/request.js b/strcalc/src/main/frontend/components/request.js index c82ed3f..5000e19 100644 --- a/strcalc/src/main/frontend/components/request.js +++ b/strcalc/src/main/frontend/components/request.js @@ -7,11 +7,12 @@ /** * Posts the data from a via fetch() and returns the response object * @see https://simonplend.com/how-to-use-fetch-to-post-form-data-as-json-to-your-api/ - * @param {FormData} form - form containing data to POST + * @param {string} url - address of server request + * @param {FormData} formData - data to include in the POST request * @returns {Promise} - response from the server */ -export async function postForm(form) { - return post(form.action, Object.fromEntries(new FormData(form).entries())) +export async function postFormData(url, formData) { + return post(url, Object.fromEntries(formData.entries())) } /** diff --git a/strcalc/src/main/frontend/components/request.test.js b/strcalc/src/main/frontend/components/request.test.js index 5685d68..a5532fd 100644 --- a/strcalc/src/main/frontend/components/request.test.js +++ b/strcalc/src/main/frontend/components/request.test.js @@ -4,22 +4,14 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -import { post, postForm, postOptions } from './request' +import { post, postFormData, postOptions } from './request' import { afterEach, describe, expect, test, vi } from 'vitest' -import { resolvedUrl } from '../test/helpers' +import setupFetchStub from '../test/fetch-stub' // @vitest-environment jsdom describe('Request', () => { const req = { want: 'foo' } - const setupFetchStub = (body, options) => { - const fetchStub = vi.fn() - - fetchStub.mockReturnValueOnce(Promise.resolve(new Response(body, options))) - vi.stubGlobal('fetch', fetchStub) - return fetchStub - } - afterEach(() => { vi.unstubAllGlobals() }) describe('post', () => { @@ -53,34 +45,15 @@ describe('Request', () => { }) }) - describe('postForm', () => { + describe('postFormData', () => { test('succeeds', async () => { - // We have to be careful creating the , because form.action resolves - // differently depending on how we created it. - // - // Originally I tried creating it using fragment() from '../test/helpers', - // which creates elements using a new