From 470d162fb6c1821ad06ee87b6488d7cc3f281a74 Mon Sep 17 00:00:00 2001 From: lohit Date: Thu, 30 May 2024 23:09:34 +0530 Subject: [PATCH] fix/path param (#2388) * fix(#484): minor code fixes * code fixes * fixes for generateCode * var change * pr review fixes --- .../RequestPane/QueryParams/index.js | 36 +++++------ .../CollectionItem/GenerateCodeItem/index.js | 53 ++++++++-------- .../ReduxStore/slices/collections/index.js | 62 ++++++++++--------- .../bruno-cli/src/runner/interpolate-vars.js | 4 +- .../bruno-cli/src/runner/prepare-request.js | 3 +- .../src/ipc/network/interpolate-vars.js | 4 +- packages/bruno-lang/v2/src/bruToJson.js | 8 +-- .../bruno-schema/src/collections/index.js | 4 +- .../src/collections/index.spec.js | 2 - .../src/collections/requestSchema.spec.js | 2 - 10 files changed, 88 insertions(+), 90 deletions(-) diff --git a/packages/bruno-app/src/components/RequestPane/QueryParams/index.js b/packages/bruno-app/src/components/RequestPane/QueryParams/index.js index ecb84b6a9..162d57a43 100644 --- a/packages/bruno-app/src/components/RequestPane/QueryParams/index.js +++ b/packages/bruno-app/src/components/RequestPane/QueryParams/index.js @@ -35,22 +35,10 @@ const QueryParams = ({ item, collection }) => { const onSave = () => dispatch(saveRequest(item.uid, collection.uid)); const handleRun = () => dispatch(sendRequest(item, collection.uid)); - const handleValueChange = (data, type, value) => { - const _data = cloneDeep(data); - - if (!has(_data, type)) { - return; - } - - _data[type] = value; - - return _data; - }; - - const handleQueryParamChange = (e, data, type) => { + const handleQueryParamChange = (e, data, key) => { let value; - switch (type) { + switch (key) { case 'name': { value = e.target.value; break; @@ -65,11 +53,17 @@ const QueryParams = ({ item, collection }) => { } } - const param = handleValueChange(data, type, value); + let queryParam = cloneDeep(data); + + if (queryParam[key] === value) { + return; + } + + queryParam[key] = value; dispatch( updateQueryParam({ - param, + queryParam, itemUid: item.uid, collectionUid: collection.uid }) @@ -79,11 +73,17 @@ const QueryParams = ({ item, collection }) => { const handlePathParamChange = (e, data) => { let value = e.target.value; - const path = handleValueChange(data, 'value', value); + let pathParam = cloneDeep(data); + + if (pathParam['value'] === value) { + return; + } + + pathParam['value'] = value; dispatch( updatePathParam({ - path, + pathParam, itemUid: item.uid, collectionUid: collection.uid }) diff --git a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js index a768db8ce..59cdd6281 100644 --- a/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js +++ b/packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/GenerateCodeItem/index.js @@ -27,31 +27,22 @@ const interpolateUrl = ({ url, envVars, collectionVariables, processEnvVars }) = }); }; -const joinPathUrl = (url, params) => { - const processPaths = (uri, paths) => { - return uri +// interpolate URL paths +const interpolateUrlPathParams = (url, params) => { + const getInterpolatedBasePath = (pathname, params) => { + return pathname .split('/') .map((segment) => { if (segment.startsWith(':')) { - const paramName = segment.slice(1); - const param = paths.find((p) => p.name === paramName && p.type === 'path' && p.enabled); - return param ? param.value : segment; + const pathParamName = segment.slice(1); + const pathParam = params.find((p) => p?.name === pathParamName && p?.type === 'path'); + return pathParam ? pathParam.value : segment; } return segment; }) .join('/'); }; - const processQueryParams = (search, params) => { - const queryParams = new URLSearchParams(search); - params - .filter((p) => p.type === 'query' && p.enabled) - .forEach((param) => { - queryParams.set(param.name, param.value); - }); - return queryParams.toString(); - }; - let uri; try { uri = new URL(url); @@ -59,10 +50,9 @@ const joinPathUrl = (url, params) => { uri = new URL(`http://${url}`); } - const basePath = processPaths(uri.pathname, params); - const queryString = processQueryParams(uri.search, params); + const basePath = getInterpolatedBasePath(uri.pathname, params); - return `${uri.origin}${basePath}${queryString ? `?${queryString}` : ''}`; + return `${uri.origin}${basePath}${uri?.search || ''}`; }; const languages = [ @@ -114,10 +104,6 @@ const languages = [ ]; const GenerateCodeItem = ({ collection, item, onClose }) => { - const url = joinPathUrl( - get(item, 'draft.request.url') !== undefined ? get(item, 'draft.request.url') : get(item, 'request.url'), - get(item, 'draft.request.params') !== undefined ? get(item, 'draft.request.params') : get(item, 'request.params') - ); const environment = findEnvironmentInCollection(collection, collection.activeEnvironmentUid); let envVars = {}; if (environment) { @@ -128,12 +114,23 @@ const GenerateCodeItem = ({ collection, item, onClose }) => { }, {}); } + const requestUrl = + get(item, 'draft.request.url') !== undefined ? get(item, 'draft.request.url') : get(item, 'request.url'); + + // interpolate the query params const interpolatedUrl = interpolateUrl({ - url, + url: requestUrl, envVars, collectionVariables: collection.collectionVariables, processEnvVars: collection.processEnvVariables }); + + // interpolate the path params + const finalUrl = interpolateUrlPathParams( + interpolatedUrl, + get(item, 'draft.request.params') !== undefined ? get(item, 'draft.request.params') : get(item, 'request.params') + ); + const [selectedLanguage, setSelectedLanguage] = useState(languages[0]); return ( @@ -157,7 +154,7 @@ const GenerateCodeItem = ({ collection, item, onClose }) => {
- {isValidUrl(interpolatedUrl) ? ( + {isValidUrl(finalUrl) ? ( { item.request.url !== '' ? { ...item.request, - url: interpolatedUrl + url: finalUrl } : { ...item.draft.request, - url: interpolatedUrl + url: finalUrl } }} /> ) : (
-

Invalid URL: {interpolatedUrl}

+

Invalid URL: {url}

Please check the URL and try again

diff --git a/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js b/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js index 8cba29c5a..2a851c238 100644 --- a/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js +++ b/packages/bruno-app/src/providers/ReduxStore/slices/collections/index.js @@ -367,39 +367,42 @@ export const collectionsSlice = createSlice({ } item.draft.request.url = action.payload.url; - const parts = splitOnFirst(item.draft.request.url, '?'); - const urlParams = parseQueryParams(parts[1]); - let urlPaths = []; + const parts = splitOnFirst(item?.draft?.request?.url, '?'); + const urlQueryParams = parseQueryParams(parts[1]); + let urlPathParams = []; try { - urlPaths = parsePathParams(parts[0]); + urlPathParams = parsePathParams(parts[0]); } catch (err) { console.error(err); toast.error(err.message); } - const disabledParams = filter(item.draft.request.params, (p) => !p.enabled); - let enabledParams = filter(item.draft.request.params, (p) => p.enabled && p.type === 'query'); - let oldPaths = filter(item.draft.request.params, (p) => p.enabled && p.type === 'path'); - let newPaths = []; + const disabledQueryParams = filter(item?.draft?.request?.params, (p) => !p.enabled && p.type === 'query'); + let enabledQueryParams = filter(item?.draft?.request?.params, (p) => p.enabled && p.type === 'query'); + let oldPathParams = filter(item?.draft?.request?.params, (p) => p.enabled && p.type === 'path'); + let newPathParams = []; // try and connect as much as old params uid's as possible - each(urlParams, (urlParam) => { - const existingParam = find(enabledParams, (p) => p.name === urlParam.name || p.value === urlParam.value); - urlParam.uid = existingParam ? existingParam.uid : uuid(); - urlParam.enabled = true; - urlParam.type = 'query'; + each(urlQueryParams, (urlQueryParam) => { + const existingQueryParam = find( + enabledQueryParams, + (p) => p?.name === urlQueryParam?.name || p?.value === urlQueryParam?.value + ); + urlQueryParam.uid = existingQueryParam?.uid || uuid(); + urlQueryParam.enabled = true; + urlQueryParam.type = 'query'; // once found, remove it - trying our best here to accommodate duplicate query params - if (existingParam) { - enabledParams = filter(enabledParams, (p) => p.uid !== existingParam.uid); + if (existingQueryParam) { + enabledQueryParams = filter(enabledQueryParams, (p) => p?.uid !== existingQueryParam?.uid); } }); // filter the newest path param and compare with previous data that already inserted - newPaths = filter(urlPaths, (urlPath) => { - const existingPath = find(oldPaths, (p) => p.name === urlPath.name); - if (existingPath) { + newPathParams = filter(urlPathParams, (urlPath) => { + const existingPathParam = find(oldPathParams, (p) => p.name === urlPath.name); + if (existingPathParam) { return false; } urlPath.uid = uuid(); @@ -409,14 +412,14 @@ export const collectionsSlice = createSlice({ }); // remove path param that not used or deleted when typing url - oldPaths = filter(oldPaths, (urlPath) => { - return find(urlPaths, (p) => p.name === urlPath.name); + oldPathParams = filter(oldPathParams, (urlPath) => { + return find(urlPathParams, (p) => p.name === urlPath.name); }); // ultimately params get replaced with params in url + the disabled ones that existed prior // the query params are the source of truth, the url in the queryurl input gets constructed using these params // we however are also storing the full url (with params) in the url itself - item.draft.request.params = concat(urlParams, newPaths, disabledParams, oldPaths); + item.draft.request.params = concat(urlQueryParams, newPathParams, disabledQueryParams, oldPathParams); } } }, @@ -491,12 +494,12 @@ export const collectionsSlice = createSlice({ } const queryParam = find( item.draft.request.params, - (h) => h.uid === action.payload.param.uid && h.type === 'query' + (h) => h.uid === action.payload.queryParam.uid && h.type === 'query' ); if (queryParam) { - queryParam.name = action.payload.param.name; - queryParam.value = action.payload.param.value; - queryParam.enabled = action.payload.param.enabled; + queryParam.name = action.payload.queryParam.name; + queryParam.value = action.payload.queryParam.value; + queryParam.enabled = action.payload.queryParam.enabled; // update request url const parts = splitOnFirst(item.draft.request.url, '?'); @@ -558,11 +561,14 @@ export const collectionsSlice = createSlice({ item.draft = cloneDeep(item); } - const param = find(item.draft.request.params, (p) => p.uid === action.payload.path.uid && p.type === 'path'); + const param = find( + item.draft.request.params, + (p) => p.uid === action.payload.pathParam.uid && p.type === 'path' + ); if (param) { - param.name = action.payload.path.name; - param.value = action.payload.path.value; + param.name = action.payload.pathParam.name; + param.value = action.payload.pathParam.value; } } } diff --git a/packages/bruno-cli/src/runner/interpolate-vars.js b/packages/bruno-cli/src/runner/interpolate-vars.js index e84fc7f0a..5dff0564f 100644 --- a/packages/bruno-cli/src/runner/interpolate-vars.js +++ b/packages/bruno-cli/src/runner/interpolate-vars.js @@ -99,7 +99,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces throw { message: 'Invalid URL format', originalError: e.message }; } - const urlPaths = url.pathname + const interpolatedUrlPath = url.pathname .split('/') .filter((path) => path !== '') .map((path) => { @@ -113,7 +113,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces }) .join(''); - request.url = url.origin + urlPaths + url.search; + request.url = url.origin + interpolatedUrlPath + url.search; } if (request.proxy) { diff --git a/packages/bruno-cli/src/runner/prepare-request.js b/packages/bruno-cli/src/runner/prepare-request.js index 1e7e083cd..8b1b249db 100644 --- a/packages/bruno-cli/src/runner/prepare-request.js +++ b/packages/bruno-cli/src/runner/prepare-request.js @@ -29,8 +29,7 @@ const prepareRequest = (request, collectionRoot) => { let axiosRequest = { method: request.method, url: request.url, - headers: headers, - paths: request.paths + headers: headers }; const collectionAuth = get(collectionRoot, 'request.auth'); diff --git a/packages/bruno-electron/src/ipc/network/interpolate-vars.js b/packages/bruno-electron/src/ipc/network/interpolate-vars.js index 886ead69b..b544ed7d1 100644 --- a/packages/bruno-electron/src/ipc/network/interpolate-vars.js +++ b/packages/bruno-electron/src/ipc/network/interpolate-vars.js @@ -99,7 +99,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces throw { message: 'Invalid URL format', originalError: e.message }; } - const urlPaths = url.pathname + const urlPathnameInterpolatedWithPathParams = url.pathname .split('/') .filter((path) => path !== '') .map((path) => { @@ -113,7 +113,7 @@ const interpolateVars = (request, envVars = {}, collectionVariables = {}, proces }) .join(''); - request.url = url.origin + urlPaths + url.search; + request.url = url.origin + urlPathnameInterpolatedWithPathParams + url.search; } if (request.proxy) { diff --git a/packages/bruno-lang/v2/src/bruToJson.js b/packages/bruno-lang/v2/src/bruToJson.js index d62888d2c..e78b1b8ad 100644 --- a/packages/bruno-lang/v2/src/bruToJson.js +++ b/packages/bruno-lang/v2/src/bruToJson.js @@ -136,7 +136,7 @@ const mapPairListToKeyValPairs = (pairList = [], parseEnabled = true) => { }); }; -const mapPairListToKeyValPairsWithType = (pairList = [], type) => { +const mapRequestParams = (pairList = [], type) => { if (!pairList.length) { return []; } @@ -346,17 +346,17 @@ const sem = grammar.createSemantics().addAttribute('ast', { }, query(_1, dictionary) { return { - params: mapPairListToKeyValPairsWithType(dictionary.ast, 'query') + params: mapRequestParams(dictionary.ast, 'query') }; }, paramspath(_1, dictionary) { return { - params: mapPairListToKeyValPairsWithType(dictionary.ast, 'path') + params: mapRequestParams(dictionary.ast, 'path') }; }, paramsquery(_1, dictionary) { return { - params: mapPairListToKeyValPairsWithType(dictionary.ast, 'query') + params: mapRequestParams(dictionary.ast, 'query') }; }, headers(_1, dictionary) { diff --git a/packages/bruno-schema/src/collections/index.js b/packages/bruno-schema/src/collections/index.js index b1bf21d3e..3dc1352c8 100644 --- a/packages/bruno-schema/src/collections/index.js +++ b/packages/bruno-schema/src/collections/index.js @@ -185,7 +185,7 @@ const authSchema = Yup.object({ .noUnknown(true) .strict(); -const keyValueWithTypeSchema = Yup.object({ +const requestParamsSchema = Yup.object({ uid: uidSchema, name: Yup.string().nullable(), value: Yup.string().nullable(), @@ -203,7 +203,7 @@ const requestSchema = Yup.object({ url: requestUrlSchema, method: requestMethodSchema, headers: Yup.array().of(keyValueSchema).required('headers are required'), - params: Yup.array().of(keyValueWithTypeSchema).required('params are required'), + params: Yup.array().of(requestParamsSchema).required('params are required'), auth: authSchema, body: requestBodySchema, script: Yup.object({ diff --git a/packages/bruno-schema/src/collections/index.spec.js b/packages/bruno-schema/src/collections/index.spec.js index 0c6b69156..16b683d08 100644 --- a/packages/bruno-schema/src/collections/index.spec.js +++ b/packages/bruno-schema/src/collections/index.spec.js @@ -59,7 +59,6 @@ describe('Collection Schema Validation', () => { method: 'GET', headers: [], params: [], - paths: [], body: { mode: 'none' } @@ -117,7 +116,6 @@ describe('Collection Schema Validation', () => { method: 'GET', headers: [], params: [], - paths: [], body: { mode: 'none' } diff --git a/packages/bruno-schema/src/collections/requestSchema.spec.js b/packages/bruno-schema/src/collections/requestSchema.spec.js index 2430b2862..87399c690 100644 --- a/packages/bruno-schema/src/collections/requestSchema.spec.js +++ b/packages/bruno-schema/src/collections/requestSchema.spec.js @@ -9,7 +9,6 @@ describe('Request Schema Validation', () => { method: 'GET', headers: [], params: [], - paths: [], body: { mode: 'none' } @@ -25,7 +24,6 @@ describe('Request Schema Validation', () => { method: 'GET-junk', headers: [], params: [], - paths: [], body: { mode: 'none' }