Skip to content

Commit ed902ec

Browse files
MIchaelMainerMIchaelMainer
authored
Merge pull request #306 from microsoftgraph/JS_Issue_291_URLParsingCorrection
Issue 291 - Changed parse path function considering nested query parameters.
2 parents 870477b + 35c804d commit ed902ec

File tree

3 files changed

+185
-39
lines changed

3 files changed

+185
-39
lines changed

‎spec/core/urlGeneration.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,59 @@ cases.push({
9191
.query("$search=senior"),
9292
});
9393

94+
cases.push({
95+
url: "https://graph.microsoft.com/beta/me/people?$select=displayName,title,id&$count=false&$expand=a($expand=a,b)",
96+
request: client
97+
.api("/me/people")
98+
.version("beta")
99+
.select(["displayName", "title"])
100+
.count(true)
101+
.expand("a($expand=a,b)")
102+
.query("$select=id")
103+
.query("$count=false"),
104+
});
105+
106+
cases.push({
107+
url: "https://graph.microsoft.com/v1.0/me/people?$select=displayName,title,id&select=value",
108+
request: client
109+
.api("/me/people")
110+
.version("v1.0")
111+
.select(["displayName", "title"])
112+
.query({ select: "value" })
113+
.query({ $select: "id" }),
114+
});
115+
116+
// handling an invalid input
117+
cases.push({
118+
url: "https://graph.microsoft.com/v1.0/me/people?$select=displayName,title&select=value&test",
119+
request: client
120+
.api("/me/people")
121+
.version("v1.0")
122+
.select(["displayName", "title"])
123+
.query({ select: "value" })
124+
.query("test"),
125+
});
126+
127+
// handling an invalid input
128+
cases.push({
129+
url: "https://graph.microsoft.com/v1.0/me/people?$expand=address($select=home,$expand=city)&$select=home,displayName,title&select=value&test",
130+
request: client
131+
.api("/me/people?$expand=address($select=home,$expand=city)&$select=home")
132+
.version("v1.0")
133+
.select(["displayName", "title"])
134+
.query({ select: "value" })
135+
.query("test"),
136+
});
137+
138+
cases.push({
139+
url: "https://graph.microsoft.com/v1.0/me/people?$expand=home($select=home)&name=test",
140+
request: client.api("/me/people").query("?name=test&$expand=home($select=home)"),
141+
});
142+
cases.push({
143+
url: "https://graph.microsoft.com/v1.0/me/people?$expand=home($select=home)&name=test",
144+
request: client.api("/me/people?name=test&$expand=home($select=home)"),
145+
});
146+
94147
cases.push({
95148
url: "https://graph.microsoft.com/v1.0/me/drive/root?$expand=children($select=name),permissions",
96149
request: client

‎spec/core/urlParsing.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@ const testCases = {
2828
"me?$select=displayName": "https://graph.microsoft.com/v1.0/me?$select=displayName",
2929
"me?select=displayName": "https://graph.microsoft.com/v1.0/me?select=displayName",
3030
"https://graph.microsoft.com/beta/me?select=displayName": "https://graph.microsoft.com/beta/me?select=displayName",
31+
32+
// test for nested query parameters
33+
"https://graph.microsoft.com/beta/identityGovernance/entitlementManagement/accessPackages/?$expand=accessPackageAssignmentPolicies,accessPackageResourceRoleScopes($expand=accessPackageResourceRole,accessPackageResourceScope)": "https://graph.microsoft.com/beta/identityGovernance/entitlementManagement/accessPackages/?$expand=accessPackageAssignmentPolicies,accessPackageResourceRoleScopes($expand=accessPackageResourceRole,accessPackageResourceScope)",
34+
"me?$select=displayName&$select=id": "https://graph.microsoft.com/v1.0/me?$select=displayName,id",
35+
"/me?$filter=b&$filter=a": "https://graph.microsoft.com/v1.0/me?$filter=a",
36+
"https://graph.microsoft.com/v1.0/me?$top=4&$expand=4&$iscount=true&$top=2": "https://graph.microsoft.com/v1.0/me?$top=2&$expand=4&$iscount=true",
37+
"/items?$expand=fields($select=Title)&$expand=name($select=firstName)": "https://graph.microsoft.com/v1.0/items?$expand=fields($select=Title),name($select=firstName)",
38+
39+
// Passing invalid parameters
40+
"/me?&test&123": "https://graph.microsoft.com/v1.0/me?&test&123",
41+
"/me?$select($select=name)": "https://graph.microsoft.com/v1.0/me?$select($select=name)",
3142
};
3243

3344
describe("urlParsing.ts", () => {
@@ -42,5 +53,5 @@ describe("urlParsing.ts", () => {
4253
}
4354
}
4455
});
45-
/* tslint:enable: no-string-literal */
4656
});
57+
/* tslint:enable: no-string-literal */

‎src/GraphRequest.ts‎

Lines changed: 120 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export interface URLComponents {
4949
path?: string;
5050
oDataQueryParams: KeyValuePairObjectStringNumber;
5151
otherURLQueryParams: KeyValuePairObjectStringNumber;
52+
otherURLQueryOptions: any[];
5253
}
5354

5455
/**
@@ -118,6 +119,7 @@ export class GraphRequest {
118119
version: this.config.defaultVersion,
119120
oDataQueryParams: {},
120121
otherURLQueryParams: {},
122+
otherURLQueryOptions: [],
121123
};
122124
this._headers = {};
123125
this._options = {};
@@ -170,14 +172,7 @@ export class GraphRequest {
170172
// Capture query string into oDataQueryParams and otherURLQueryParams
171173
const queryParams = path.substring(queryStrPos + 1, path.length).split("&");
172174
for (const queryParam of queryParams) {
173-
const qParams = queryParam.split("=");
174-
const key = qParams[0];
175-
const value = qParams[1];
176-
if (oDataQueryNames.indexOf(key) !== -1) {
177-
this.urlComponents.oDataQueryParams[key] = value;
178-
} else {
179-
this.urlComponents.otherURLQueryParams[key] = value;
180-
}
175+
this.parseQueryParameter(queryParam);
181176
}
182177
}
183178
};
@@ -244,9 +239,105 @@ export class GraphRequest {
244239
}
245240
}
246241
}
242+
243+
if (urlComponents.otherURLQueryOptions.length !== 0) {
244+
for (const str of urlComponents.otherURLQueryOptions) {
245+
query.push(str);
246+
}
247+
}
247248
return query.length > 0 ? "?" + query.join("&") : "";
248249
}
249250

251+
/**
252+
* @private
253+
* Parses the query parameters to set the urlComponents property of the GraphRequest object
254+
* @param {string|KeyValuePairObjectStringNumber} queryDictionaryOrString - The query parameter
255+
* @returns The same GraphRequest instance that is being called with
256+
*/
257+
private parseQueryParameter(queryDictionaryOrString: string | KeyValuePairObjectStringNumber): GraphRequest {
258+
if (typeof queryDictionaryOrString === "string") {
259+
if (queryDictionaryOrString.charAt(0) === "?") {
260+
queryDictionaryOrString = queryDictionaryOrString.substring(1, queryDictionaryOrString.length);
261+
}
262+
263+
if (queryDictionaryOrString.indexOf("&") !== -1) {
264+
const queryParams = queryDictionaryOrString.split("&");
265+
for (const str of queryParams) {
266+
this.parseQueryParamenterString(str);
267+
}
268+
} else {
269+
this.parseQueryParamenterString(queryDictionaryOrString);
270+
}
271+
} else if (queryDictionaryOrString.constructor === Object) {
272+
for (const key in queryDictionaryOrString) {
273+
if (queryDictionaryOrString.hasOwnProperty(key)) {
274+
this.setURLComponentsQueryParamater(key, queryDictionaryOrString[key]);
275+
}
276+
}
277+
} else {
278+
/*Push values which are not of key-value structure.
279+
Example-> Handle an invalid input->.query(123) and let the Graph API respond with the error in the URL*/ this.urlComponents.otherURLQueryOptions.push(queryDictionaryOrString);
280+
}
281+
282+
return this;
283+
}
284+
285+
/**
286+
* @private
287+
* Parses the query parameter of string type to set the urlComponents property of the GraphRequest object
288+
* @param {string} queryParameter - the query parameters
289+
* returns nothing
290+
*/
291+
private parseQueryParamenterString(queryParameter: string): void {
292+
/* The query key-value pair must be split on the first equals sign to avoid errors in parsing nested query parameters.
293+
Example-> "/me?$expand=home($select=city)" */
294+
if (this.isValidQueryKeyValuePair(queryParameter)) {
295+
const indexOfFirstEquals = queryParameter.indexOf("=");
296+
const paramKey = queryParameter.substring(0, indexOfFirstEquals);
297+
const paramValue = queryParameter.substring(indexOfFirstEquals + 1, queryParameter.length);
298+
this.setURLComponentsQueryParamater(paramKey, paramValue);
299+
} else {
300+
/* Push values which are not of key-value structure.
301+
Example-> Handle an invalid input->.query(test), .query($select($select=name)) and let the Graph API respond with the error in the URL*/
302+
this.urlComponents.otherURLQueryOptions.push(queryParameter);
303+
}
304+
}
305+
306+
/**
307+
* @private
308+
* Sets values into the urlComponents property of GraphRequest object.
309+
* @param {string} paramKey - the query parameter key
310+
* @param {string} paramValue - the query paramter value
311+
* @returns nothing
312+
*/
313+
private setURLComponentsQueryParamater(paramKey: string, paramValue: string | number): void {
314+
if (oDataQueryNames.indexOf(paramKey) !== -1) {
315+
const currentValue = this.urlComponents.oDataQueryParams[paramKey];
316+
const isValueAppendable = currentValue && (paramKey === "$expand" || paramKey === "$select" || paramKey === "$orderby");
317+
this.urlComponents.oDataQueryParams[paramKey] = isValueAppendable ? currentValue + "," + paramValue : paramValue;
318+
} else {
319+
this.urlComponents.otherURLQueryParams[paramKey] = paramValue;
320+
}
321+
}
322+
/**
323+
* @private
324+
* Check if the query parameter string has a valid key-value structure
325+
* @param {string} queryString - the query parameter string. Example -> "name=value"
326+
* #returns true if the query string has a valid key-value structure else false
327+
*/
328+
private isValidQueryKeyValuePair(queryString: string): boolean {
329+
const indexofFirstEquals = queryString.indexOf("=");
330+
if (indexofFirstEquals === -1) {
331+
return false;
332+
}
333+
const indexofOpeningParanthesis = queryString.indexOf("(");
334+
if (indexofOpeningParanthesis !== -1 && queryString.indexOf("(") < indexofFirstEquals) {
335+
// Example -> .query($select($expand=true));
336+
return false;
337+
}
338+
return true;
339+
}
340+
250341
/**
251342
* @private
252343
* Updates the custom headers and options for a request
@@ -393,7 +484,7 @@ export class GraphRequest {
393484
* @public
394485
* To add properties for select OData Query param
395486
* @param {string|string[]} properties - The Properties value
396-
* @returns The same GraphRequest instance that is being called with
487+
* @returns The same GraphRequest instance that is being called with, after adding the properties for $select query
397488
*/
398489
/*
399490
* Accepts .select("displayName,birthday")
@@ -410,7 +501,7 @@ export class GraphRequest {
410501
* @public
411502
* To add properties for expand OData Query param
412503
* @param {string|string[]} properties - The Properties value
413-
* @returns The same GraphRequest instance that is being called with
504+
* @returns The same GraphRequest instance that is being called with, after adding the properties for $expand query
414505
*/
415506
public expand(properties: string | string[]): GraphRequest {
416507
this.addCsvQueryParameter("$expand", properties, arguments);
@@ -421,7 +512,7 @@ export class GraphRequest {
421512
* @public
422513
* To add properties for orderby OData Query param
423514
* @param {string|string[]} properties - The Properties value
424-
* @returns The same GraphRequest instance that is being called with
515+
* @returns The same GraphRequest instance that is being called with, after adding the properties for $orderby query
425516
*/
426517
public orderby(properties: string | string[]): GraphRequest {
427518
this.addCsvQueryParameter("$orderby", properties, arguments);
@@ -430,9 +521,9 @@ export class GraphRequest {
430521

431522
/**
432523
* @public
433-
* To add query string for filter OData Query param
524+
* To add query string for filter OData Query param. The request URL accepts only one $filter Odata Query option and its value is set to the most recently passed filter query string.
434525
* @param {string} filterStr - The filter query string
435-
* @returns The same GraphRequest instance that is being called with
526+
* @returns The same GraphRequest instance that is being called with, after adding the $filter query
436527
*/
437528
public filter(filterStr: string): GraphRequest {
438529
this.urlComponents.oDataQueryParams.$filter = filterStr;
@@ -441,9 +532,9 @@ export class GraphRequest {
441532

442533
/**
443534
* @public
444-
* To add criterion for search OData Query param
535+
* To add criterion for search OData Query param. The request URL accepts only one $search Odata Query option and its value is set to the most recently passed search criterion string.
445536
* @param {string} searchStr - The search criterion string
446-
* @returns The same GraphRequest instance that is being called with
537+
* @returns The same GraphRequest instance that is being called with, after adding the $search query criteria
447538
*/
448539
public search(searchStr: string): GraphRequest {
449540
this.urlComponents.oDataQueryParams.$search = searchStr;
@@ -452,9 +543,9 @@ export class GraphRequest {
452543

453544
/**
454545
* @public
455-
* To add number for top OData Query param
546+
* To add number for top OData Query param. The request URL accepts only one $top Odata Query option and its value is set to the most recently passed number value.
456547
* @param {number} n - The number value
457-
* @returns The same GraphRequest instance that is being called with
548+
* @returns The same GraphRequest instance that is being called with, after adding the number for $top query
458549
*/
459550
public top(n: number): GraphRequest {
460551
this.urlComponents.oDataQueryParams.$top = n;
@@ -463,9 +554,9 @@ export class GraphRequest {
463554

464555
/**
465556
* @public
466-
* To add number for skip OData Query param
557+
* To add number for skip OData Query param. The request URL accepts only one $skip Odata Query option and its value is set to the most recently passed number value.
467558
* @param {number} n - The number value
468-
* @returns The same GraphRequest instance that is being called with
559+
* @returns The same GraphRequest instance that is being called with, after adding the number for the $skip query
469560
*/
470561
public skip(n: number): GraphRequest {
471562
this.urlComponents.oDataQueryParams.$skip = n;
@@ -474,9 +565,9 @@ export class GraphRequest {
474565

475566
/**
476567
* @public
477-
* To add token string for skipToken OData Query param
568+
* To add token string for skipToken OData Query param. The request URL accepts only one $skipToken Odata Query option and its value is set to the most recently passed token value.
478569
* @param {string} token - The token value
479-
* @returns The same GraphRequest instance that is being called with
570+
* @returns The same GraphRequest instance that is being called with, after adding the token string for $skipToken query option
480571
*/
481572
public skipToken(token: string): GraphRequest {
482573
this.urlComponents.oDataQueryParams.$skipToken = token;
@@ -485,9 +576,9 @@ export class GraphRequest {
485576

486577
/**
487578
* @public
488-
* To add boolean for count OData Query param
579+
* To add boolean for count OData Query param. The URL accepts only one $count Odata Query option and its value is set to the most recently passed boolean value.
489580
* @param {boolean} isCount - The count boolean
490-
* @returns The same GraphRequest instance that is being called with
581+
* @returns The same GraphRequest instance that is being called with, after adding the boolean value for the $count query option
491582
*/
492583
public count(isCount: boolean = false): GraphRequest {
493584
this.urlComponents.oDataQueryParams.$count = isCount.toString();
@@ -498,23 +589,14 @@ export class GraphRequest {
498589
* @public
499590
* Appends query string to the urlComponent
500591
* @param {string|KeyValuePairObjectStringNumber} queryDictionaryOrString - The query value
501-
* @returns The same GraphRequest instance that is being called with
592+
* @returns The same GraphRequest instance that is being called with, after appending the query string to the url component
593+
*/
594+
/*
595+
* Accepts .query("displayName=xyz")
596+
* and .select({ name: "value" })
502597
*/
503598
public query(queryDictionaryOrString: string | KeyValuePairObjectStringNumber): GraphRequest {
504-
const otherURLQueryParams = this.urlComponents.otherURLQueryParams;
505-
if (typeof queryDictionaryOrString === "string") {
506-
const querySplit = queryDictionaryOrString.split("=");
507-
const queryKey = querySplit[0];
508-
const queryValue = querySplit[1];
509-
otherURLQueryParams[queryKey] = queryValue;
510-
} else {
511-
for (const key in queryDictionaryOrString) {
512-
if (queryDictionaryOrString.hasOwnProperty(key)) {
513-
otherURLQueryParams[key] = queryDictionaryOrString[key];
514-
}
515-
}
516-
}
517-
return this;
599+
return this.parseQueryParameter(queryDictionaryOrString);
518600
}
519601

520602
/**

0 commit comments

Comments
 (0)