diff --git a/pkg/ui/workspaces/cluster-ui/src/api/fetchData.spec.ts b/pkg/ui/workspaces/cluster-ui/src/api/fetchData.spec.ts deleted file mode 100644 index 1bd6eab700e9..000000000000 --- a/pkg/ui/workspaces/cluster-ui/src/api/fetchData.spec.ts +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2021 The Cockroach Authors. -// -// Use of this software is governed by the Business Source License -// included in the file licenses/BSL.txt. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -import { propsToQueryString } from "./fetchData"; - -describe("fetchData functions", () => { - describe("propsToQueryString", () => { - it("creates query string from object", () => { - const obj = { - start: 100, - end: 200, - strParam: "hello", - bool: false, - }; - const expected = "start=100&end=200&strParam=hello&bool=false"; - const res = propsToQueryString(obj); - expect(res).toEqual(expected); - }); - - it("skips entries with nullish values", () => { - const obj = { - start: 100, - end: 200, - strParam: null as any, - hello: undefined as any, - }; - const expected = "start=100&end=200"; - const res = propsToQueryString(obj); - expect(res).toEqual(expected); - }); - }); -}); diff --git a/pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts b/pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts index b0ecc8c5c2ee..4669c4782f1d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/fetchData.ts @@ -11,7 +11,6 @@ import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; import { RequestError } from "../util"; import { getBasePath } from "./basePath"; -import { stringify } from "querystring"; interface ProtoBuilder< P extends ConstructorType, @@ -30,18 +29,6 @@ export function toArrayBuffer(encodedRequest: Uint8Array): ArrayBuffer { ); } -// propsToQueryString is a helper function that converts a set of object -// properties to a query string -// - keys with null or undefined values will be skipped -// - non-string values will be toString'd -export function propsToQueryString(props: { [k: string]: any }) { - const params = new URLSearchParams(); - Object.entries(props).forEach( - ([k, v]: [string, any]) => v != null && params.set(k, v.toString()), - ); - return params.toString(); -} - /** * @param RespBuilder expects protobuf stub class to build decode response; * @param path relative URL path for requested resource; diff --git a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts index 4eff1f9acf3d..1c500dd562ba 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/statementsApi.ts @@ -9,7 +9,8 @@ // licenses/APL.txt. import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; -import { fetchData, propsToQueryString } from "src/api"; +import { fetchData } from "src/api"; +import { propsToQueryString } from "src/util"; const STATEMENTS_PATH = "/_status/statements"; @@ -28,7 +29,7 @@ export const getCombinedStatements = ( const queryStr = propsToQueryString({ start: req.start.toInt(), end: req.end.toInt(), - combined: true, + combined: req.combined, }); return fetchData( cockroach.server.serverpb.StatementsResponse, diff --git a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx index 78ced333c249..a3f5652d0cde 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx @@ -14,7 +14,7 @@ import { sessionAttr } from "src/util/constants"; import { Helmet } from "react-helmet"; import { Loading } from "../loading"; import _ from "lodash"; -import { Link, RouteComponentProps, withRouter } from "react-router-dom"; +import { Link, RouteComponentProps } from "react-router-dom"; import { SessionInfo } from "./sessionsTable"; @@ -320,7 +320,6 @@ export class SessionDetails extends React.Component { statementNoConstants: stmt.sql_no_constants, implicitTxn: session.active_txn?.implicit, app: "", - search: "", })} onClick={() => this.props.onStatementClick && this.props.onStatementClick() diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts index f57a1a6ea345..f2377829fd35 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.selectors.ts @@ -10,6 +10,7 @@ import { createSelector } from "@reduxjs/toolkit"; import { RouteComponentProps, match as Match } from "react-router-dom"; +import { Location } from "history"; import _ from "lodash"; import { AppState } from "../store"; import { @@ -24,6 +25,7 @@ import { databaseAttr, StatementStatistics, statementKey, + queryByName, } from "../util"; import { AggregateStatistics } from "../statementsTable"; import { Fraction } from "./statementDetails"; @@ -87,12 +89,13 @@ function fractionMatching( function filterByRouterParamsPredicate( match: Match, + location: Location, internalAppNamePrefix: string, ): (stat: ExecutionStatistics) => boolean { const statement = getMatchParamByName(match, statementAttr); const implicitTxn = getMatchParamByName(match, implicitTxnAttr) === "true"; - const database = getMatchParamByName(match, databaseAttr); - let app = getMatchParamByName(match, appAttr); + const database = queryByName(location, databaseAttr); + let app = queryByName(location, appAttr); const filterByKeys = (stmt: ExecutionStatistics) => stmt.statement === statement && @@ -129,7 +132,11 @@ export const selectStatement = createSelector( const flattened = flattenStatementStats(statements); const results = _.filter( flattened, - filterByRouterParamsPredicate(props.match, internalAppNamePrefix), + filterByRouterParamsPredicate( + props.match, + props.location, + internalAppNamePrefix, + ), ); const statement = getMatchParamByName(props.match, statementAttr); return { @@ -137,7 +144,7 @@ export const selectStatement = createSelector( stats: combineStatementStats(results.map(s => s.stats)), byNode: coalesceNodeStats(results), app: _.uniq(results.map(s => s.app)), - database: getMatchParamByName(props.match, databaseAttr), + database: queryByName(props.location, databaseAttr), distSQL: fractionMatching(results, s => s.distSQL), vec: fractionMatching(results, s => s.vec), opt: fractionMatching(results, s => s.opt), diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx index 66941321ae62..c3e08435e6f7 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.tsx @@ -30,10 +30,10 @@ import { NumericStat, StatementStatistics, stdDev, - getMatchParamByName, formatNumberForDisplay, calculateTotalWorkload, unique, + queryByName, } from "src/util"; import { Loading } from "src/loading"; import { Button } from "src/button"; @@ -389,7 +389,7 @@ export class StatementDetails extends React.Component< }; render(): React.ReactElement { - const app = getMatchParamByName(this.props.match, appAttr); + const app = queryByName(this.props.location, appAttr); return (
@@ -445,7 +445,7 @@ export class StatementDetails extends React.Component< } = this.props.statement; if (!stats) { - const sourceApp = getMatchParamByName(this.props.match, appAttr); + const sourceApp = queryByName(this.props.location, appAttr); const listUrl = "/statements" + (sourceApp ? "/" + sourceApp : ""); return ( diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts index c77ac67b517b..a86f4dbe7664 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts @@ -17,7 +17,7 @@ import { ExecutionStatistics, flattenStatementStats, formatDate, - getMatchParamByName, + queryByName, statementKey, StatementStatistics, TimestampToMoment, @@ -142,7 +142,7 @@ export const selectStatements = createSelector( return null; } let statements = flattenStatementStats(state.data.statements); - const app = getMatchParamByName(props.match, appAttr); + const app = queryByName(props.location, appAttr); const isInternal = (statement: ExecutionStatistics) => statement.app.startsWith(state.data.internal_app_name_prefix); diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index f35783b68924..f3dcddfb2192 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -36,6 +36,7 @@ import { calculateTotalWorkload, unique, containAny, + queryByName, } from "src/util"; import { AggregateStatistics, @@ -433,7 +434,7 @@ export class StatementsPage extends React.Component< } = this.props; const appAttrValue = getMatchParamByName(match, appAttr); const selectedApp = appAttrValue || ""; - const appOptions = [{ value: "All", label: "All" }]; + const appOptions = [{ value: "all", label: "All" }]; this.props.apps.forEach(app => appOptions.push({ value: app, label: app })); const data = this.filteredStatementsData(); const totalWorkload = calculateTotalWorkload(data); @@ -577,12 +578,12 @@ export class StatementsPage extends React.Component< render() { const { - match, + location, refreshStatementDiagnosticsRequests, onActivateStatementDiagnostics, onDiagnosticsModalOpen, } = this.props; - const app = getMatchParamByName(match, appAttr); + const app = queryByName(location, appAttr); return (
diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx index 175111ef2d32..789b233185a4 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTableContent.tsx @@ -22,7 +22,13 @@ import { Dropdown } from "src/dropdown"; import { Button } from "src/button"; import { Tooltip } from "@cockroachlabs/ui-components"; -import { summarize, TimestampToMoment } from "src/util"; +import { + appAttr, + databaseAttr, + propsToQueryString, + summarize, + TimestampToMoment, +} from "src/util"; import { shortStatement } from "./statementsTable"; import styles from "./statementsTableContent.module.scss"; import { cockroach } from "@cockroachlabs/crdb-protobuf-client"; @@ -121,39 +127,43 @@ export const StatementTableCell = { ), }; -interface StatementLinkProps { +type StatementLinkTargetProps = { statement: string; app: string; implicitTxn: boolean; - search: string; statementNoConstants?: string; database?: string; - onClick?: (statement: string) => void; -} +}; // StatementLinkTarget returns the link to the relevant statement page, given // the input statement details. -export const StatementLinkTarget = (props: StatementLinkProps) => { - let base: string; - if (props.app && props.app.length > 0) { - base = `/statements/${props.app}`; - } else { - base = `/statement`; - } - if (props.database && props.database.length > 0) { - base = base + `/${props.database}/${props.implicitTxn}`; - } else { - base = base + `/${props.implicitTxn}`; - } +export const StatementLinkTarget = ( + props: StatementLinkTargetProps, +): string => { + const base = `/statement/${props.implicitTxn}`; + const linkStatement = props.statementNoConstants || props.statement; + + const searchParams = propsToQueryString({ + [databaseAttr]: props.database, + [appAttr]: props.app, + }); - let linkStatement = props.statement; - if (props.statementNoConstants) { - linkStatement = props.statementNoConstants; - } - return `${base}/${encodeURIComponent(linkStatement)}`; + return `${base}/${encodeURIComponent(linkStatement)}?${searchParams}`; }; -export const StatementLink = (props: StatementLinkProps) => { +interface StatementLinkProps { + statement: string; + app: string; + implicitTxn: boolean; + search: string; + statementNoConstants?: string; + database?: string; + onClick?: (statement: string) => void; +} + +export const StatementLink = ( + props: StatementLinkProps, +): React.ReactElement => { const summary = summarize(props.statement); const { onClick, statement } = props; const onStatementClick = React.useCallback(() => { @@ -162,8 +172,16 @@ export const StatementLink = (props: StatementLinkProps) => { } }, [onClick, statement]); + const linkProps = { + statement: props.statement, + app: props.app, + implicitTxn: props.implicitTxn, + statementNoConstants: props.statementNoConstants, + database: props.database, + }; + return ( - +
{ - describe("queryToString", () => { - it("make query to string", () => { - assert.equal(queryToString({ a: "test" }), "a=test"); - assert.equal(queryToString({ a: "test", b: "test" }), "a=test&b=test"); - assert.equal(queryToString({ a: undefined }), "a=undefined"); + describe("propsToQueryString", () => { + it("creates query string from object", () => { + const obj = { + start: 100, + end: 200, + strParam: "hello", + bool: false, + }; + const expected = "start=100&end=200&strParam=hello&bool=false"; + const res = propsToQueryString(obj); + expect(res).toEqual(expected); + }); + + it("skips entries with nullish values", () => { + const obj = { + start: 100, + end: 200, + strParam: null as any, + hello: undefined as any, + }; + const expected = "start=100&end=200"; + const res = propsToQueryString(obj); + expect(res).toEqual(expected); }); }); describe("queryByName", () => { diff --git a/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts b/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts index 60beb417154a..3425fe0786c8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts +++ b/pkg/ui/workspaces/cluster-ui/src/util/query/query.ts @@ -15,15 +15,16 @@ interface ParamsObj { [key: string]: string; } -/* eslint-disable no-prototype-builtins,no-restricted-syntax */ -export function queryToString(obj: any) { - const str = []; - for (const p in obj) { - if (obj.hasOwnProperty(p)) { - str.push(`${encodeURIComponent(p)}=${encodeURIComponent(obj[p])}`); - } - } - return str.join("&"); +// propsToQueryString is a helper function that converts a set of object +// properties to a query string +// - keys with null or undefined values will be skipped +// - non-string values will be toString'd +export function propsToQueryString(props: { [k: string]: any }): string { + const params = new URLSearchParams(); + Object.entries(props).forEach( + ([k, v]: [string, any]) => v != null && params.set(k, v.toString()), + ); + return params.toString(); } export function queryToObj(location: Location, key: string, value: string) { @@ -44,12 +45,15 @@ export function queryToObj(location: Location, key: string, value: string) { return paramObj; } -export function queryByName(location: Location, key: string) { +export function queryByName(location: Location, key: string): string { const urlParams = new URLSearchParams(location.search); return urlParams.get(key); } -export function getMatchParamByName(match: Match, key: string) { +export function getMatchParamByName( + match: Match, + key: string, +): string | null { const param = match.params[key]; if (param) { return decodeURIComponent(param); diff --git a/pkg/ui/workspaces/db-console/src/app.spec.tsx b/pkg/ui/workspaces/db-console/src/app.spec.tsx index fb72f84bf4c2..bfc3a80c616c 100644 --- a/pkg/ui/workspaces/db-console/src/app.spec.tsx +++ b/pkg/ui/workspaces/db-console/src/app.spec.tsx @@ -346,13 +346,6 @@ describe("Routing to", () => { }); }); - describe("'/statement/:${statementAttr}' path", () => { - it("routes to component", () => { - navigateToPath("/statement/statement-attr"); - assert.lengthOf(appWrapper.find(StatementDetails), 1); - }); - }); - describe("'/statement/:${implicitTxnAttr}/:${statementAttr}' path", () => { it("routes to component", () => { navigateToPath("/statement/implicit-attr/statement-attr/"); diff --git a/pkg/ui/workspaces/db-console/src/app.tsx b/pkg/ui/workspaces/db-console/src/app.tsx index e774d5d67e52..a4a3a14177ba 100644 --- a/pkg/ui/workspaces/db-console/src/app.tsx +++ b/pkg/ui/workspaces/db-console/src/app.tsx @@ -65,6 +65,7 @@ import SessionsPage from "src/views/sessions/sessionsPage"; import SessionDetails from "src/views/sessions/sessionDetails"; import TransactionsPage from "src/views/transactions/transactionsPage"; import StatementsDiagnosticsHistoryView from "src/views/reports/containers/statementDiagnosticsHistory"; +import { RedirectToStatementDetails } from "src/routes/RedirectToStatementDetails"; import "styl/app.styl"; // NOTE: If you are adding a new path to the router, and that path contains any @@ -185,39 +186,38 @@ export const App: React.FC = (props: AppProps) => { /> - } + path={`/statements/:${appAttr}/:${databaseAttr}/:${implicitTxnAttr}/:${statementAttr}`} + render={RedirectToStatementDetails} /> } /> {/* sessions */} diff --git a/pkg/ui/workspaces/db-console/src/routes/RedirectToStatementDetails.tsx b/pkg/ui/workspaces/db-console/src/routes/RedirectToStatementDetails.tsx new file mode 100644 index 000000000000..a58057ce8e56 --- /dev/null +++ b/pkg/ui/workspaces/db-console/src/routes/RedirectToStatementDetails.tsx @@ -0,0 +1,37 @@ +// Copyright 2021 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +import React from "react"; +import { Redirect, match as Match } from "react-router-dom"; +import { StatementLinkTarget } from "@cockroachlabs/cluster-ui"; +import { getMatchParamByName } from "src/util/query"; +import { + appAttr, + databaseAttr, + implicitTxnAttr, + statementAttr, +} from "src/util/constants"; + +type Props = { + match: Match; +}; + +// RedirectToStatementDetails is designed to route old versions of StatementDetails routes +// where app and database are route params, to the new StatementDetails route. +export function RedirectToStatementDetails({ match }: Props) { + const linkProps = { + statement: getMatchParamByName(match, statementAttr), + app: getMatchParamByName(match, appAttr), + implicitTxn: getMatchParamByName(match, implicitTxnAttr) === "true", + database: getMatchParamByName(match, databaseAttr), + }; + + return ; +} diff --git a/pkg/ui/workspaces/db-console/src/util/api.spec.ts b/pkg/ui/workspaces/db-console/src/util/api.spec.ts index 8e74b92f08b7..8db994343a46 100644 --- a/pkg/ui/workspaces/db-console/src/util/api.spec.ts +++ b/pkg/ui/workspaces/db-console/src/util/api.spec.ts @@ -22,100 +22,6 @@ import { REMOTE_DEBUGGING_ERROR_TEXT } from "src/util/constants"; import Severity = cockroach.util.log.Severity; describe("rest api", function() { - describe("propsToQueryString", function() { - interface PropBag { - [k: string]: string; - } - - // helper decoding function used to doublecheck querystring generation - function decodeQueryString(qs: string): PropBag { - return _.reduce( - qs.split("&"), - (memo: PropBag, v: string) => { - const [key, value] = v.split("="); - memo[decodeURIComponent(key)] = decodeURIComponent(value); - return memo; - }, - {}, - ); - } - - it("creates an appropriate querystring", function() { - const testValues: { [k: string]: any } = { - a: "testa", - b: "testb", - }; - - const querystring = api.propsToQueryString(testValues); - - assert(/a=testa/.test(querystring)); - assert(/b=testb/.test(querystring)); - assert.lengthOf(querystring.match(/=/g), 2); - assert.lengthOf(querystring.match(/&/g), 1); - assert.deepEqual(testValues, decodeQueryString(querystring)); - }); - - it("handles falsy values correctly", function() { - const testValues: { [k: string]: any } = { - // null and undefined should be ignored - undefined: undefined, - null: null, - // other values should be added - false: false, - "": "", - 0: 0, - }; - - const querystring = api.propsToQueryString(testValues); - - assert(/false=false/.test(querystring)); - assert(/0=0/.test(querystring)); - assert(/([^A-Za-z]|^)=([^A-Za-z]|$)/.test(querystring)); - assert.lengthOf(querystring.match(/=/g), 3); - assert.lengthOf(querystring.match(/&/g), 2); - assert.notOk(/undefined/.test(querystring)); - assert.notOk(/null/.test(querystring)); - assert.deepEqual( - { false: "false", "": "", 0: "0" }, - decodeQueryString(querystring), - ); - }); - - it("handles special characters", function() { - const key = "!@#$%^&*()=+-_\\|\"`'?/<>"; - const value = key - .split("") - .reverse() - .join(""); // key reversed - const testValues: { [k: string]: any } = { - [key]: value, - }; - - const querystring = api.propsToQueryString(testValues); - - assert(querystring.match(/%/g).length > (key + value).match(/%/g).length); - assert.deepEqual(testValues, decodeQueryString(querystring)); - }); - - it("handles non-string values", function() { - const testValues: { [k: string]: any } = { - boolean: true, - number: 1, - emptyObject: {}, - emptyArray: [], - objectWithProps: { a: 1, b: 2 }, - arrayWithElts: [1, 2, 3], - long: Long.fromNumber(1), - }; - - const querystring = api.propsToQueryString(testValues); - assert.deepEqual( - _.mapValues(testValues, _.toString), - decodeQueryString(querystring), - ); - }); - }); - describe("databases request", function() { afterEach(fetchMock.restore); @@ -652,7 +558,9 @@ describe("rest api", function() { response: (_url: string, requestObj: RequestInit) => { assert.isUndefined(requestObj.body); const encodedResponse = protos.cockroach.server.serverpb.ClusterResponse.encode( - { cluster_id: clusterID }, + { + cluster_id: clusterID, + }, ).finish(); return { body: api.toArrayBuffer(encodedResponse), @@ -735,7 +643,9 @@ describe("rest api", function() { response: (_url: string, requestObj: RequestInit) => { assert.isUndefined(requestObj.body); const encodedResponse = protos.cockroach.server.serverpb.MetricMetadataResponse.encode( - { metadata }, + { + metadata, + }, ).finish(); return { body: api.toArrayBuffer(encodedResponse), @@ -829,7 +739,9 @@ describe("rest api", function() { response: (_url: string, requestObj: RequestInit) => { assert.isUndefined(requestObj.body); const logsResponse = protos.cockroach.server.serverpb.LogEntriesResponse.encode( - { entries: [logEntry] }, + { + entries: [logEntry], + }, ).finish(); return { body: api.toArrayBuffer(logsResponse), diff --git a/pkg/ui/workspaces/db-console/src/util/api.ts b/pkg/ui/workspaces/db-console/src/util/api.ts index bbdab5cdef07..f1563bc1160e 100644 --- a/pkg/ui/workspaces/db-console/src/util/api.ts +++ b/pkg/ui/workspaces/db-console/src/util/api.ts @@ -17,6 +17,7 @@ import moment from "moment"; import * as protos from "src/js/protos"; import { FixLong } from "src/util/fixLong"; +import { propsToQueryString } from "src/util/query"; export type DatabasesRequestMessage = protos.cockroach.server.serverpb.DatabasesRequest; export type DatabasesResponseMessage = protos.cockroach.server.serverpb.DatabasesResponse; @@ -254,19 +255,6 @@ export type APIRequestFn = ( timeout?: moment.Duration, ) => Promise; -// propsToQueryString is a helper function that converts a set of object -// properties to a query string -// - keys with null or undefined values will be skipped -// - non-string values will be toString'd -export function propsToQueryString(props: { [k: string]: any }) { - return _.compact( - _.map(props, (v: any, k: string) => - !_.isNull(v) && !_.isUndefined(v) - ? `${encodeURIComponent(k)}=${encodeURIComponent(v.toString())}` - : null, - ), - ).join("&"); -} /** * ENDPOINTS */ @@ -676,7 +664,7 @@ export function getStatements( timeout?: moment.Duration, ): Promise { const queryStr = propsToQueryString({ - combined: true, + combined: req.combined, start: req.start.toInt(), end: req.end.toInt(), }); diff --git a/pkg/ui/workspaces/db-console/src/util/query.spec.ts b/pkg/ui/workspaces/db-console/src/util/query.spec.ts index 9d468c897e31..4007f4e443ac 100644 --- a/pkg/ui/workspaces/db-console/src/util/query.spec.ts +++ b/pkg/ui/workspaces/db-console/src/util/query.spec.ts @@ -9,8 +9,10 @@ // licenses/APL.txt. import { assert } from "chai"; -import { queryToString, queryByName } from "./query"; +import { propsToQueryString, queryByName } from "./query"; import { Location } from "history"; +import _ from "lodash"; +import Long from "long"; const location: Location = { pathname: "/debug/chart", @@ -22,11 +24,96 @@ const location: Location = { }; describe("Query utils", () => { - describe("queryToString", () => { - it("make query to string", () => { - assert.equal(queryToString({ a: "test" }), "a=test"); - assert.equal(queryToString({ a: "test", b: "test" }), "a=test&b=test"); - assert.equal(queryToString({ a: undefined }), "a=undefined"); + describe("propsToQueryString", function() { + interface PropBag { + [k: string]: string; + } + + // helper decoding function used to doublecheck querystring generation + function decodeQueryString(qs: string): PropBag { + return qs.split("&").reduce((memo: PropBag, v: string) => { + const [key, value] = v.split("="); + memo[decodeURIComponent(key)] = decodeURIComponent(value).replace( + "%20", + "+", + ); + return memo; + }, {}); + } + + it("creates an appropriate querystring", function() { + const testValues: { [k: string]: any } = { + a: "testa", + b: "testb", + }; + + const querystring = propsToQueryString(testValues); + + assert(/a=testa/.test(querystring)); + assert(/b=testb/.test(querystring)); + assert.lengthOf(querystring.match(/=/g), 2); + assert.lengthOf(querystring.match(/&/g), 1); + assert.deepEqual(testValues, decodeQueryString(querystring)); + }); + + it("handles falsy values correctly", function() { + const testValues: { [k: string]: any } = { + // null and undefined should be ignored + undefined: undefined, + null: null, + // other values should be added + false: false, + "": "", + 0: 0, + }; + + const querystring = propsToQueryString(testValues); + + assert(/false=false/.test(querystring)); + assert(/0=0/.test(querystring)); + assert(/([^A-Za-z]|^)=([^A-Za-z]|$)/.test(querystring)); + assert.lengthOf(querystring.match(/=/g), 3); + assert.lengthOf(querystring.match(/&/g), 2); + assert.notOk(/undefined/.test(querystring)); + assert.notOk(/null/.test(querystring)); + assert.deepEqual( + { false: "false", "": "", 0: "0" }, + decodeQueryString(querystring), + ); + }); + + it("handles special characters", function() { + const key = "!@#$%^&*()=+-_\\|\"`'?/<>"; + const value = key + .split("") + .reverse() + .join(""); // key reversed + const testValues: { [k: string]: any } = { + [key]: value, + }; + + const querystring = propsToQueryString(testValues); + + assert(querystring.match(/%/g).length > (key + value).match(/%/g).length); + assert.deepEqual(testValues, decodeQueryString(querystring)); + }); + + it("handles non-string values", function() { + const testValues: { [k: string]: any } = { + boolean: true, + number: 1, + emptyObject: {}, + emptyArray: [], + objectWithProps: { a: 1, b: 2 }, + arrayWithElts: [1, 2, 3], + long: Long.fromNumber(1), + }; + + const querystring = propsToQueryString(testValues); + assert.deepEqual( + _.mapValues(testValues, _.toString), + decodeQueryString(querystring), + ); }); }); describe("queryByName", () => { diff --git a/pkg/ui/workspaces/db-console/src/util/query.ts b/pkg/ui/workspaces/db-console/src/util/query.ts index 93c891044b4b..5478bb82e75f 100644 --- a/pkg/ui/workspaces/db-console/src/util/query.ts +++ b/pkg/ui/workspaces/db-console/src/util/query.ts @@ -9,6 +9,7 @@ // licenses/APL.txt. import { Location } from "history"; +import _ from "lodash"; import { match as Match } from "react-router-dom"; interface ParamsObj { @@ -19,15 +20,18 @@ interface URLSearchParamsWithKeys extends URLSearchParams { keys: () => string; } -/* eslint-disable no-prototype-builtins,no-restricted-syntax */ -export function queryToString(obj: any) { - const str = []; - for (const p in obj) { - if (obj.hasOwnProperty(p)) { - str.push(`${encodeURIComponent(p)}=${encodeURIComponent(obj[p])}`); - } - } - return str.join("&"); +// propsToQueryString is a helper function that converts a set of object +// properties to a query string +// - keys with null or undefined values will be skipped +// - non-string values will be toString'd +export function propsToQueryString(props: { [k: string]: any }): string { + return _.compact( + _.map(props, (v: any, k: string) => + !_.isNull(v) && !_.isUndefined(v) + ? `${encodeURIComponent(k)}=${encodeURIComponent(v.toString())}` + : null, + ), + ).join("&"); } export function queryToObj(location: Location, key: string, value: string) { @@ -48,7 +52,7 @@ export function queryToObj(location: Location, key: string, value: string) { return paramObj; } -export function queryByName(location: Location, key: string) { +export function queryByName(location: Location, key: string): string | null { const urlParams = new URLSearchParams(location.search); return urlParams.get(key); } diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementDetails.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementDetails.tsx index 56c90698dcaa..19e7578af96a 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementDetails.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementDetails.tsx @@ -13,6 +13,7 @@ import { match as Match, withRouter, } from "react-router-dom"; +import { Location } from "history"; import { createSelector } from "reselect"; import _ from "lodash"; @@ -41,7 +42,7 @@ import { statementAttr, } from "src/util/constants"; import { FixLong } from "src/util/fixLong"; -import { getMatchParamByName } from "src/util/query"; +import { getMatchParamByName, queryByName } from "src/util/query"; import { selectDiagnosticsReportsByStatementFingerprint } from "src/redux/statements/statementsSelectors"; import { StatementDetails, @@ -122,12 +123,13 @@ function fractionMatching( function filterByRouterParamsPredicate( match: Match, + location: Location, internalAppNamePrefix: string, ): (stat: ExecutionStatistics) => boolean { const statement = getMatchParamByName(match, statementAttr); const implicitTxn = getMatchParamByName(match, implicitTxnAttr) === "true"; - const database = getMatchParamByName(match, databaseAttr); - let app = getMatchParamByName(match, appAttr); + const database = queryByName(location, databaseAttr); + let app = queryByName(location, appAttr); const filterByKeys = (stmt: ExecutionStatistics) => stmt.statement === statement && @@ -163,9 +165,12 @@ export const selectStatement = createSelector( const internalAppNamePrefix = statementsState.data?.internal_app_name_prefix; const flattened = flattenStatementStats(statements); - const results = _.filter( - flattened, - filterByRouterParamsPredicate(props.match, internalAppNamePrefix), + const results = flattened.filter( + filterByRouterParamsPredicate( + props.match, + props.location, + internalAppNamePrefix, + ), ); const statement = getMatchParamByName(props.match, statementAttr); return { @@ -173,7 +178,7 @@ export const selectStatement = createSelector( stats: combineStatementStats(results.map(s => s.stats)), byNode: coalesceNodeStats(results), app: _.uniq(results.map(s => s.app)), - database: getMatchParamByName(props.match, databaseAttr), + database: queryByName(props.location, databaseAttr), distSQL: fractionMatching(results, s => s.distSQL), vec: fractionMatching(results, s => s.vec), opt: fractionMatching(results, s => s.opt),