Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade query-string to v8.1.0 | Add Wrapper function for queryString.parse() #1794

Merged
merged 10 commits into from
Sep 16, 2023
2 changes: 1 addition & 1 deletion packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"moment": "^2.18.1",
"object-hash": "^3.0.0",
"prop-types": "^15.5.10",
"query-string": "^6.3.0",
"query-string": "^8.1.0",
"raven-js": "^3.22.1",
"react": "^18.2.0",
"react-circular-progressbar": "^2.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import queryString from 'query-string';
import * as reactRouterDom from 'react-router-dom';

import { ROUTE_PATH, matches, getUrl, getUrlState, sanitizeUrlState } from './url';
import * as parseQuery from '../../utils/parseQuery';

jest.mock('react-router-dom', () => ({
matchPath: jest.fn(),
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('DeepDependencyGraph/url', () => {
let parseSpy;

beforeAll(() => {
parseSpy = jest.spyOn(queryString, 'parse');
parseSpy = jest.spyOn(parseQuery, 'default');
warnSpy = jest.spyOn(console, 'warn').mockImplementation();
});

Expand Down
5 changes: 3 additions & 2 deletions packages/jaeger-ui/src/components/DeepDependencies/url.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@

import _isEmpty from 'lodash/isEmpty';
import memoizeOne from 'memoize-one';
import queryString from 'query-string';
import { matchPath } from 'react-router-dom';
import queryString from 'query-string';
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
import parseQuery from '../../utils/parseQuery';

import { EDdgDensity, TDdgSparseUrlState } from '../../model/ddg/types';
import prefixUrl from '../../utils/prefix-url';
Expand Down Expand Up @@ -62,7 +63,7 @@ export const getUrlState = memoizeOne(function getUrlState(search: string): TDdg
showOp,
start,
visEncoding,
} = queryString.parse(search);
} = parseQuery(search);
const rv: TDdgSparseUrlState = {
density: firstParam(density) as EDdgDensity,
};
Expand Down
3 changes: 2 additions & 1 deletion packages/jaeger-ui/src/components/SearchTracePage/url.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import prefixUrl from '../../utils/prefix-url';
import { MAX_LENGTH } from '../DeepDependencies/Graph/DdgNodeContent/constants';

import { SearchQuery } from '../../types/search';
import parseQuery from '../../utils/parseQuery';

function eqEq(a: string | number | null | undefined, b: string | number | null | undefined) {
return (a == null && b == null) || String(a) === String(b);
Expand Down Expand Up @@ -69,7 +70,7 @@ export function getUrl(query?: TUrlState) {
export const getUrlState: (search: string) => TUrlState = memoizeOne(function getUrlState(
search: string
): TUrlState {
const { traceID, span, ...rest } = queryString.parse(search);
const { traceID, span, ...rest } = parseQuery(search);
const rv: TUrlState = { ...rest };
const traceIDs = new Set(!traceID || Array.isArray(traceID) ? traceID : [traceID]);
const spanLinks: Record<string, string> = {};
Expand Down
3 changes: 2 additions & 1 deletion packages/jaeger-ui/src/components/TraceDiff/TraceDiff.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import TTraceDiffState from '../../types/TTraceDiffState';
import pluckTruthy from '../../utils/ts/pluckTruthy';

import './TraceDiff.css';
import parseQuery from '../../utils/parseQuery';

type TStateProps = {
a: string | undefined;
Expand Down Expand Up @@ -165,7 +166,7 @@ export class TraceDiffImpl extends React.PureComponent<TStateProps & TDispatchPr
// TODO(joe): simplify but do not invalidate the URL
export function mapStateToProps(state: ReduxState, ownProps: { match: match<TDiffRouteParams> }) {
const { a, b } = ownProps.match.params;
const { cohort: origCohort = [] } = queryString.parse(state.router.location.search);
const { cohort: origCohort = [] } = parseQuery(state.router.location.search);
const fullCohortSet: Set<string> = new Set(pluckTruthy([a, b].concat(origCohort)));
const cohort: string[] = Array.from(fullCohortSet);
const { traces } = state.trace;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ import { shallow } from 'enzyme';
import { Icon, Input } from 'antd';
import { CloseOutlined } from '@ant-design/icons';
import debounceMock from 'lodash/debounce';
import queryString from 'query-string';

import { UnconnectedUiFindInput, extractUiFindFromState } from './UiFindInput';
import updateUiFindSpy from '../../utils/update-ui-find';
import * as parseQuery from '../../utils/parseQuery';

jest.mock('lodash/debounce');

jest.mock('../../utils/update-ui-find');

describe('UiFind', () => {
const flushMock = jest.fn();
const queryStringParseSpy = jest.spyOn(queryString, 'parse');
const queryStringParseSpy = jest.spyOn(parseQuery, 'default');

const uiFind = 'uiFind';
const ownInputValue = 'ownInputValue';
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/common/UiFindInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ import { CloseOutlined } from '@ant-design/icons';
import { History as RouterHistory, Location } from 'history';
import _debounce from 'lodash/debounce';
import _isString from 'lodash/isString';
import queryString from 'query-string';
import { connect } from 'react-redux';
import { RouteComponentProps, withRouter } from 'react-router-dom';

import updateUiFind from '../../utils/update-ui-find';
import { TNil, ReduxState } from '../../types/index';
import parseQuery from '../../utils/parseQuery';

type TOwnProps = RouteComponentProps<any> & {
allowClear?: boolean;
Expand Down Expand Up @@ -110,7 +110,7 @@ export class UnconnectedUiFindInput extends React.PureComponent<TProps, StateTyp
}

export function extractUiFindFromState(state: ReduxState): TExtractUiFindFromStateReturn {
const { uiFind: uiFindFromUrl } = queryString.parse(state.router.location.search);
const { uiFind: uiFindFromUrl } = parseQuery(state.router.location.search);
const uiFind = Array.isArray(uiFindFromUrl) ? uiFindFromUrl.join(' ') : uiFindFromUrl;
return { uiFind };
}
Expand Down
48 changes: 48 additions & 0 deletions packages/jaeger-ui/src/utils/parseQuery.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) 2019 Uber Technologies, Inc.
//
// 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 parseQuery from './parseQuery';

describe('parseQuery', () => {
it('should parse a query string with no options', () => {
const queryString = 'foo=bar&baz=qux';
const expected = { foo: 'bar', baz: 'qux' };
expect(parseQuery(queryString)).toEqual(expected);
});

it('should parse a query string with options', () => {
const queryString = 'foo[]=bar&foo[]';
const options = { arrayFormat: 'bracket' };
const expected = { foo: ['bar', ''] };
expect(parseQuery(queryString, options)).toEqual(expected);
});

it('should handle null values in the query string', () => {
const queryString = 'bar=&empty&baz';
const expected = { bar: '', empty: null, baz: null };
expect(parseQuery(queryString)).toEqual(expected);
});

it('should handle multiple values for the same key', () => {
const queryString = 'colors=red&colors=blue&colors=green';
const expected = { colors: ['red', 'blue', 'green'] };
expect(parseQuery(queryString)).toEqual(expected);
});

it('should return an empty object for an empty query string', () => {
const queryString = '';
const expected = {};
expect(parseQuery(queryString)).toEqual(expected);
});
});
37 changes: 37 additions & 0 deletions packages/jaeger-ui/src/utils/parseQuery.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) 2019 Uber Technologies, Inc.
//
// 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 queryString, { ParseOptions } from 'query-string';

interface IParsedQuery {
[key: string]: string | null | (string | null)[];
}

function parseQuery(query: string, options?: ParseOptions): { [key: string]: string | string[] } {
Copy link
Member

@yurishkuro yurishkuro Sep 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. can we throw in a unit test to maintain coverage?
  2. under what conditions does this return a plain array? An HTTP query string is fundamentally a dictionary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can have a unit test for it. Adding it in.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 🚀

const parsed: IParsedQuery = queryString.parse(query, options);

const result: { [key: string]: string | string[] } = {};

Object.keys(parsed).forEach(key => {
if (Array.isArray(parsed[key])) {
result[key] = (parsed[key] as string[]).map((item: string | null) => item || '');
} else {
result[key] = parsed[key] as string | string[];
}
});

return result;
}

export default parseQuery;
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/utils/tracking/ga.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import _get from 'lodash/get';
import queryString from 'query-string';
import ReactGA from 'react-ga';
import Raven, { RavenOptions, RavenTransportOptions } from 'raven-js';

Expand All @@ -23,6 +22,7 @@ import { Config } from '../../types/config';
import { IWebAnalyticsFunc } from '../../types/tracking';
import { logTrackingCalls } from './utils';
import { getAppEnvironment, shouldDebugGoogleAnalytics } from '../constants';
import parseQuery from '../parseQuery';

const isTruish = (value?: string | string[]) => {
return Boolean(value) && value !== '0' && value !== 'false';
Expand All @@ -35,7 +35,7 @@ const GA: IWebAnalyticsFunc = (config: Config, versionShort: string, versionLong
const isTest = appEnv === 'test';
const isDebugMode =
(isDev && isTruish(shouldDebugGoogleAnalytics())) ||
isTruish(queryString.parse(_get(window, 'location.search'))['ga-debug']);
isTruish(parseQuery(_get(window, 'location.search'))['ga-debug']);
const gaID = _get(config, 'tracking.gaID');
const isErrorsEnabled = isDebugMode || Boolean(_get(config, 'tracking.trackErrors'));
const cookiesToDimensions = _get(config, 'tracking.cookiesToDimensions');
Expand Down
68 changes: 48 additions & 20 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2609,12 +2609,19 @@
resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.4.tgz#cd667bcfdd025213aafb7ca5915a932590acdcdc"
integrity sha512-EEhsLsD6UsDM1yFhAvy0Cjr6VwmpMWqFBCb9w07wVugF7w9nfajxLuVmngTIpgS6svCnm6Vaw+MZhoDCKnOfsw==

"@types/react-dom@16.9.18", "@types/react-dom@18.2.5", "@types/react-dom@^18.0.0":
version "16.9.18"
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-16.9.18.tgz#1fda8b84370b1339d639a797a84c16d5a195b419"
integrity sha512-lmNARUX3+rNF/nmoAFqasG0jAA7q6MeGZK/fdeLwY3kAA4NPgHHrG5bNQe2B5xmD4B+x6Z6h0rEJQ7MEEgQxsw==
"@types/react-dom@18.2.5":
version "18.2.5"
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-18.2.5.tgz#5c5f13548bda23cd98f50ca4a59107238bfe18f3"
integrity sha512-sRQsOS/sCLnpQhR4DSKGTtWFE3FZjpQa86KPVbhUqdYMRZ9FEFcfAytKhR/vUG2rH1oFbOOej6cuD7MFSobDRQ==
dependencies:
"@types/react" "^16"
"@types/react" "*"

"@types/react-dom@^18.0.0":
version "18.2.7"
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-18.2.7.tgz#67222a08c0a6ae0a0da33c3532348277c70abb63"
integrity sha512-GRaAEriuT4zp9N4p1i8BDBYmEyfo+xQ3yHjJU4eiK5NDa1RmUZG+unZABUTK4/Ox/M+GaHwb6Ow8rUITrtjszA==
dependencies:
"@types/react" "*"

"@types/react-helmet@^6.1.5":
version "6.1.5"
Expand Down Expand Up @@ -2652,7 +2659,7 @@
dependencies:
"@types/react" "*"

"@types/react@*", "@types/[email protected]", "@types/[email protected]", "@types/react@^16":
"@types/react@*", "@types/[email protected]":
version "16.14.35"
resolved "https://registry.yarnpkg.com/@types/react/-/react-16.14.35.tgz#9d3cf047d85aca8006c4776693124a5be90ee429"
integrity sha512-NUEiwmSS1XXtmBcsm1NyRRPYjoZF2YTE89/5QiLt5mlGffYK9FQqOKuOLuXNrjPQV04oQgaZG+Yq02ZfHoFyyg==
Expand All @@ -2661,6 +2668,14 @@
"@types/scheduler" "*"
csstype "^3.0.2"

"@types/[email protected]":
version "16.8.7"
resolved "https://registry.yarnpkg.com/@types/react/-/react-16.8.7.tgz#7b1c0223dd5494f9b4501ad2a69aa6acb350a29b"
integrity sha512-0xbkIyrDNKUn4IJVf8JaCn+ucao/cq6ZB8O6kSzhrJub1cVSqgTArtG0qCfdERWKMEIvUbrwLXeQMqWEsyr9dA==
dependencies:
"@types/prop-types" "*"
csstype "^2.2.0"

"@types/[email protected]":
version "2.2.1"
resolved "https://registry.yarnpkg.com/@types/redux-actions/-/redux-actions-2.2.1.tgz#c1f4a7283ecd3cd696291550361e441bf9389370"
Expand Down Expand Up @@ -4520,6 +4535,11 @@ cssstyle@^3.0.0:
dependencies:
rrweb-cssom "^0.6.0"

csstype@^2.2.0:
version "2.6.21"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.21.tgz#2efb85b7cc55c80017c66a5ad7cbd931fda3a90e"
integrity sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w==

csstype@^3.0.2:
version "3.1.2"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.1.2.tgz#1d4bf9d572f11c14031f0436e1c10bc1f571f50b"
Expand Down Expand Up @@ -4799,10 +4819,10 @@ decimal.js@^10.4.2, decimal.js@^10.4.3:
resolved "https://registry.yarnpkg.com/decimal.js/-/decimal.js-10.4.3.tgz#1044092884d245d1b7f65725fa4ad4c6f781cc23"
integrity sha512-VBBaLc1MgL5XpzgIP7ny5Z6Nx3UrRkIViUkPUdtl9aya5amy3De1gsUUSB1g3+3sExYNjCAsAznmukyxCb1GRA==

decode-uri-component@^0.2.0:
version "0.2.2"
resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.2.2.tgz#e69dbe25d37941171dd540e024c444cd5188e1e9"
integrity sha512-FqUYQ+8o158GyGTrMFJms9qh3CqTKvAqgqsTnkLI8sKu0028orqBhxNMFkFen0zGyg6epACD32pjVk58ngIErQ==
decode-uri-component@^0.4.1:
version "0.4.1"
resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.4.1.tgz#2ac4859663c704be22bf7db760a1494a49ab2cc5"
integrity sha512-+8VxcR21HhTy8nOt6jf20w0c9CADrw1O8d+VZ/YzzCt4bJ3uBjw+D1q2osAB8RnpwwaeYBxy0HyKQxD5JBMuuQ==

[email protected]:
version "0.7.0"
Expand Down Expand Up @@ -5997,6 +6017,11 @@ fill-range@^7.0.1:
dependencies:
to-regex-range "^5.0.1"

filter-obj@^5.1.0:
version "5.1.0"
resolved "https://registry.yarnpkg.com/filter-obj/-/filter-obj-5.1.0.tgz#5bd89676000a713d7db2e197f660274428e524ed"
integrity sha512-qWeTREPoT7I0bifpPUXtxkZJ1XJzxWtfoWWkdVGqa+eCr3SHW/Ocp89o8vLvbUuQnadybJpjOKu4V+RwO6sGng==

[email protected]:
version "1.2.0"
resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.2.0.tgz#7d23fe5731b207b4640e4fcd00aec1f9207a7b32"
Expand Down Expand Up @@ -8334,7 +8359,7 @@ lodash.upperfirst@^4.3.1:
resolved "https://registry.yarnpkg.com/lodash.upperfirst/-/lodash.upperfirst-4.3.1.tgz#1365edf431480481ef0d1c68957a5ed99d49f7ce"
integrity sha512-sReKOYJIJf74dhJONhU4e0/shzi1trVbSWDOhKYE5XV2O+H7Sb2Dihwuc7xWxVl+DgFPyTqIN3zMfT9cq5iWDg==

lodash@4.17.21, lodash@^4.16.5, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.2.1:
lodash@^4.16.5, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.2.1:
version "4.17.21"
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
Expand Down Expand Up @@ -9934,12 +9959,14 @@ [email protected]:
dependencies:
side-channel "^1.0.4"

query-string@^6.3.0:
version "6.3.0"
resolved "https://registry.yarnpkg.com/query-string/-/query-string-6.3.0.tgz#41ae8a61e1213c80b182d5db6cf129e05af89fc5"
query-string@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/query-string/-/query-string-8.1.0.tgz#e7f95367737219544cd360a11a4f4ca03836e115"
integrity sha512-BFQeWxJOZxZGix7y+SByG3F36dA0AbTy9o6pSmKFcFz7DAj0re9Frkty3saBn3nHo3D0oZJ/+rx3r8H8r8Jbpw==
dependencies:
decode-uri-component "^0.2.0"
strict-uri-encode "^2.0.0"
decode-uri-component "^0.4.1"
filter-obj "^5.1.0"
split-on-first "^3.0.0"

querystringify@^2.1.1:
version "2.2.0"
Expand Down Expand Up @@ -11490,6 +11517,11 @@ spdy@^4.0.2:
select-hose "^2.0.0"
spdy-transport "^3.0.0"

split-on-first@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/split-on-first/-/split-on-first-3.0.0.tgz#f04959c9ea8101b9b0bbf35a61b9ebea784a23e7"
integrity sha512-qxQJTx2ryR0Dw0ITYyekNQWpz6f8dGd7vffGNflQQ3Iqj9NJ6qiZ7ELpZsJ/QBhIVAiDfXdag3+Gp8RvWa62AA==

split2@^3.2.2:
version "3.2.2"
resolved "https://registry.yarnpkg.com/split2/-/split2-3.2.2.tgz#bf2cf2a37d838312c249c89206fd7a17dd12365f"
Expand Down Expand Up @@ -11560,10 +11592,6 @@ store@^2.0.12:
version "2.0.12"
resolved "https://registry.yarnpkg.com/store/-/store-2.0.12.tgz#8c534e2a0b831f72b75fc5f1119857c44ef5d593"

strict-uri-encode@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/strict-uri-encode/-/strict-uri-encode-2.0.0.tgz#b9c7330c7042862f6b142dc274bbcc5866ce3546"

string-convert@^0.2.0:
version "0.2.1"
resolved "https://registry.yarnpkg.com/string-convert/-/string-convert-0.2.1.tgz#6982cc3049fbb4cd85f8b24568b9d9bf39eeff97"
Expand Down