From e52130b0121d712c0a4d761874851fc5756a11a4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 10 Jul 2022 18:12:08 +0300 Subject: [PATCH] Urlencode parameters in tagService.js Fixes https://sentry.galaxyproject.org/share/issue/ff9f4891f34e4308b8183ef604970761/: ``` TypeError: tag_autocomplete_data() got an unexpected keyword argument 'd' File "uvicorn/protocols/http/h11_impl.py", line 366, in run_asgi result = await app(self.scope, self.receive, self.send) File "uvicorn/middleware/proxy_headers.py", line 75, in __call__ return await self.app(scope, receive, send) File "fastapi/applications.py", line 269, in __call__ await super().__call__(scope, receive, send) File "starlette/applications.py", line 124, in __call__ await self.middleware_stack(scope, receive, send) File "starlette/middleware/errors.py", line 184, in __call__ raise exc File "starlette/middleware/errors.py", line 162, in __call__ await self.app(scope, receive, _send) File "starlette/middleware/base.py", line 69, in __call__ await response(scope, receive, send) File "starlette/responses.py", line 260, in __call__ await wrap(partial(self.listen_for_disconnect, receive)) File "anyio/_backends/_asyncio.py", line 662, in __aexit__ raise exceptions[0] File "starlette/responses.py", line 256, in wrap await func() File "starlette/responses.py", line 245, in stream_response async for chunk in self.body_iterator: File "starlette/middleware/base.py", line 58, in body_stream raise app_exc File "starlette/middleware/base.py", line 36, in coro await self.app(scope, request.receive, send_stream.send) File "starlette_context/middleware/raw_middleware.py", line 96, in __call__ await self.app(scope, receive, send_wrapper) File "starlette/middleware/base.py", line 69, in __call__ await response(scope, receive, send) File "starlette/responses.py", line 260, in __call__ await wrap(partial(self.listen_for_disconnect, receive)) File "anyio/_backends/_asyncio.py", line 662, in __aexit__ raise exceptions[0] File "starlette/responses.py", line 256, in wrap await func() File "starlette/responses.py", line 245, in stream_response async for chunk in self.body_iterator: File "starlette/middleware/base.py", line 58, in body_stream raise app_exc File "starlette/middleware/base.py", line 36, in coro await self.app(scope, request.receive, send_stream.send) File "starlette/middleware/base.py", line 69, in __call__ await response(scope, receive, send) File "starlette/responses.py", line 260, in __call__ await wrap(partial(self.listen_for_disconnect, receive)) File "anyio/_backends/_asyncio.py", line 662, in __aexit__ raise exceptions[0] File "starlette/responses.py", line 256, in wrap await func() File "starlette/responses.py", line 245, in stream_response async for chunk in self.body_iterator: File "starlette/middleware/base.py", line 58, in body_stream raise app_exc File "starlette/middleware/base.py", line 36, in coro await self.app(scope, request.receive, send_stream.send) File "starlette/exceptions.py", line 93, in __call__ raise exc File "starlette/exceptions.py", line 82, in __call__ await self.app(scope, receive, sender) File "fastapi/middleware/asyncexitstack.py", line 21, in __call__ raise e File "fastapi/middleware/asyncexitstack.py", line 18, in __call__ await self.app(scope, receive, send) File "starlette/routing.py", line 670, in __call__ await route.handle(scope, receive, send) File "starlette/routing.py", line 418, in handle await self.app(scope, receive, send) File "a2wsgi/wsgi.py", line 140, in __call__ return await responder(scope, receive, send) File "a2wsgi/wsgi.py", line 179, in __call__ raise self.exc_info[0].with_traceback( File "galaxy/web/framework/middleware/error.py", line 165, in __call__ app_iter = self.application(environ, sr_checker) File "/cvmfs/main.galaxyproject.org/venv/lib/python3.8/site-packages/paste/recursive.py", line 85, in __call__ return self.application(environ, start_response) File "galaxy/web/framework/middleware/statsd.py", line 29, in __call__ req = self.application(environ, start_response) File "/cvmfs/main.galaxyproject.org/venv/lib/python3.8/site-packages/paste/httpexceptions.py", line 640, in __call__ return self.application(environ, start_response) File "galaxy/web/framework/base.py", line 159, in __call__ return self.handle_request(environ, start_response) File "galaxy/web/framework/base.py", line 244, in handle_request body = method(trans, **kwargs) File "galaxy/web/framework/decorators.py", line 84, in decorator return func(self, trans, *args, **kwargs) ``` You can test this by editing a history tag in the history menu, by writing `tag;d`, there should be no 500 error on the Galaxy logs. --- client/src/components/Tags/tagService.js | 21 +++--- client/src/components/Tags/tagService.test.js | 64 +++++-------------- 2 files changed, 28 insertions(+), 57 deletions(-) diff --git a/client/src/components/Tags/tagService.js b/client/src/components/Tags/tagService.js index 304d9c1c224..dff2439d3a1 100644 --- a/client/src/components/Tags/tagService.js +++ b/client/src/components/Tags/tagService.js @@ -57,10 +57,11 @@ export class TagService { if (!tag.valid) { throw new Error("Invalid tag"); } - const url = `${getAppRoot()}tag/add_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&new_tag=${ - tag.text - }`; - const response = await axios.get(url); + const url = `${getAppRoot()}tag/add_tag_async`; + const config = { + params: { item_id: id, item_class: itemClass, context: context, new_tag: tag.text }, + }; + const response = await axios.get(url, config); if (response.status !== 200) { throw new Error(`Unable to save tag: ${tag}`); } @@ -75,10 +76,9 @@ export class TagService { async delete(rawTag) { const { id, itemClass, context } = this; const tag = createTag(rawTag); - const url = `${getAppRoot()}tag/remove_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&tag_name=${ - tag.text - }`; - const response = await axios.get(url); + const url = `${getAppRoot()}tag/remove_tag_async`; + const config = { params: { item_id: id, item_class: itemClass, context: context, tag_name: tag.text } }; + const response = await axios.get(url, config); if (response.status !== 200) { throw new Error(`Unable to delete tag: ${tag}`); } @@ -92,8 +92,9 @@ export class TagService { */ async autocomplete(searchText) { const { id, itemClass } = this; - const url = `${getAppRoot()}tag/tag_autocomplete_data?item_id=${id}&item_class=${itemClass}&q=${searchText}`; - const response = await axios.get(url); + const url = `${getAppRoot()}tag/tag_autocomplete_data`; + const config = { params: { item_id: id, item_class: itemClass, q: searchText } }; + const response = await axios.get(url, config); if (response.status !== 200) { throw new Error(`Unable to retrieve autocomplete tags for search string: ${searchText}`); } diff --git a/client/src/components/Tags/tagService.test.js b/client/src/components/Tags/tagService.test.js index 79ba1025758..c8fb78145ee 100644 --- a/client/src/components/Tags/tagService.test.js +++ b/client/src/components/Tags/tagService.test.js @@ -2,9 +2,6 @@ import axios from "axios"; import MockAdapter from "axios-mock-adapter"; import { TagService } from "./tagService"; import { createTag } from "./model"; -//import { interval } from "rxjs"; -//import { take, takeUntil } from "rxjs/operators"; -// test response import autocompleteResponse from "./testData/autocompleteResponse.txt"; describe("Tags/tagService.js", () => { @@ -15,6 +12,12 @@ describe("Tags/tagService.js", () => { context: "", debounceInterval: 50, // shorter value than default for unit tests }; + const testLabel = "fo0bar123"; + const expectedParams = { + item_id: 123, + item_class: "fooClass", + context: "", + }; const svc = new TagService(svcParams); beforeEach(() => { @@ -26,43 +29,43 @@ describe("Tags/tagService.js", () => { }); describe("save", () => { - const testLabel = "fo0bar123"; const testTag = createTag(testLabel); - const { id, itemClass, context } = svcParams; - const expectedSaveUrl = `/tag/add_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&new_tag=${testLabel}`; + const expectedSaveUrl = `/tag/add_tag_async`; it("should save a string tag", async () => { const savedTag = await svc.save(testLabel); expect(savedTag.text).toBe(testLabel); expect(axiosMock.history.get[0].url).toBe(expectedSaveUrl); + expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, new_tag: testLabel }); }); it("should save an object tag", async () => { const savedTag = await svc.save(testTag); expect(savedTag.text).toBe(testLabel); expect(axiosMock.history.get[0].url).toBe(expectedSaveUrl); + expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, new_tag: testLabel }); }); // TODO: test error conditions }); describe("delete", () => { - const testLabel = "fo0bar123"; const testTag = createTag(testLabel); - const { id, itemClass, context } = svcParams; - const expectedDeleteUrl = `/tag/remove_tag_async?item_id=${id}&item_class=${itemClass}&context=${context}&tag_name=${testLabel}`; + const expectedDeleteUrl = `/tag/remove_tag_async`; it("should delete a text tag", async () => { const result = await svc.delete(testTag); expect(result).toBeTruthy(); expect(axiosMock.history.get[0].url).toBe(expectedDeleteUrl); + expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, tag_name: testLabel }); }); it("should delete an object tag", async () => { const result = await svc.delete(testTag); expect(result).toBeTruthy(); expect(axiosMock.history.get[0].url).toBe(expectedDeleteUrl); + expect(axiosMock.history.get[0].params).toEqual({ ...expectedParams, tag_name: testLabel }); }); // TODO: test error conditions @@ -70,8 +73,7 @@ describe("Tags/tagService.js", () => { describe("autocomplete", () => { const searchString = "foo"; - const { id, itemClass } = svcParams; - const expectedSearchUrl = `/tag/tag_autocomplete_data?item_id=${id}&item_class=${itemClass}&q=${searchString}`; + const expectedSearchUrl = `/tag/tag_autocomplete_data`; const checkAutocompleteResult = (result) => { expect(result).toBeTruthy(); @@ -83,46 +85,14 @@ describe("Tags/tagService.js", () => { // straight ajax request, unused in practice, but it's easier to // test this call if we just expose it + const { ...searchParams } = expectedParams; + delete searchParams.context; + it("ajax call should return tag objects", async () => { const result = await svc.autocomplete(searchString); expect(axiosMock.history.get[0].url).toBe(expectedSearchUrl); + expect(axiosMock.history.get[0].params).toEqual({ ...searchParams, q: searchString }); checkAutocompleteResult(result); }); - - /* - - // hit the search input with multiple entries, only one ajax call - // should result because of debouncing - it("should debounce autocomplete search inputs", (done) => { - // stub ajax request to return the success response if the - // searchString is the expected input - - // take first emission, debouncing should guarantee the first - // emission is the last thing we sent, should be searchString - const timer$ = interval(100).pipe(take(1)); - const option$ = svc.autocompleteOptions.pipe(takeUntil(timer$)); - - const nextHandler = jest.fn(); - - option$.subscribe({ - next: nextHandler, - error: (err) => console.warn("error", err), - complete: () => { - expect(nextHandler).toHaveBeenCalledTimes(1); - expect(axiosMock.history.get.length).toBe(1); - done(); - }, - }); - - // spam a bunch of key inputs followed by the correct input - const spamCount = 2 + Math.floor(Math.random() * 10); - for (let i = 0; i < spamCount; i++) { - const spamVal = new String(Math.random()); - svc.autocompleteSearchText = spamVal; - } - svc.autocompleteSearchText = searchString; - }); - - */ }); });