From c3dd95a4ca259aee5a04b70cbf40ef871f03194f Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Thu, 11 Apr 2019 10:16:51 -0700 Subject: [PATCH 01/11] inital prototype of param builder --- .../arcgis-rest-items/src/SearchBuilder.ts | 112 ++++++++++++++++++ packages/arcgis-rest-items/src/search.ts | 31 ++++- packages/arcgis-rest-request/src/index.ts | 1 + .../src/utils/IParamBuilder.ts | 3 + .../src/utils/process-params.ts | 13 +- 5 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 packages/arcgis-rest-items/src/SearchBuilder.ts create mode 100644 packages/arcgis-rest-request/src/utils/IParamBuilder.ts diff --git a/packages/arcgis-rest-items/src/SearchBuilder.ts b/packages/arcgis-rest-items/src/SearchBuilder.ts new file mode 100644 index 0000000000..de3cecf8c2 --- /dev/null +++ b/packages/arcgis-rest-items/src/SearchBuilder.ts @@ -0,0 +1,112 @@ +import { IParamBuilder } from "@esri/arcgis-rest-request"; + +export class SearchQueryBuilder implements IParamBuilder { + private termStack: any[] = []; + private rangeStack: any[] = []; + private q: string; + + constructor(q: string = "") { + this.q = q; + } + + public match(...terms: any[]) { + this.termStack = this.termStack.concat(terms); + return this; + } + + public in(field: string) { + if (field && field !== "*") { + this.q += `${field}: `; + } + this.commit(); + return this; + } + + public startGroup() { + this.commit(); + this.q += "("; + return this; + } + + public endGroup() { + this.q += ")"; + return this; + } + + public and() { + this.commit(); + this.q += " AND "; + return this; + } + + public or() { + this.commit(); + this.q += " OR "; + return this; + } + + public not() { + this.commit(); + this.q += " NOT "; + return this; + } + + public from(term: any) { + this.rangeStack[0] = term; + return this; + } + + public to(term: any) { + this.rangeStack[1] = term; + return this; + } + + public boost(num: number) { + this.commit(); + this.q += "^${num}"; + return this; + } + + public toParam() { + return this.q; + } + + public clone() { + return new SearchQueryBuilder(this.q + ""); + } + + private hasWhiteSpace(s: string) { + return /\s/g.test(s); + } + + private formatTerm(term: any) { + if (term instanceof Date) { + return term.getTime(); + } + + if (typeof term === "string" && this.hasWhiteSpace(term)) { + return `"${term}"`; + } + + return term; + } + + private commit() { + if (this.rangeStack.length && this.rangeStack[0] && this.rangeStack[1]) { + this.q += `[${this.formatTerm(this.rangeStack[0])} TO ${this.formatTerm( + this.rangeStack[1] + )}]`; + this.rangeStack = [undefined, undefined]; + } + + if (this.termStack.length) { + this.q += this.termStack + .map(term => { + return term; + }) + .join(" "); + } + + this.termStack = []; + } +} diff --git a/packages/arcgis-rest-items/src/search.ts b/packages/arcgis-rest-items/src/search.ts index aa4c0dacd3..77dead8528 100644 --- a/packages/arcgis-rest-items/src/search.ts +++ b/packages/arcgis-rest-items/src/search.ts @@ -8,10 +8,11 @@ import { } from "@esri/arcgis-rest-request"; import { IPagingParams, IItem } from "@esri/arcgis-rest-common-types"; +import { SearchQueryBuilder } from "./SearchBuilder"; // this interface still needs to be docced export interface ISearchRequest extends IPagingParams { - q: string; + q: string | SearchQueryBuilder; [key: string]: any; } @@ -29,6 +30,7 @@ export interface ISearchResult { num: number; nextStart: number; results: IItem[]; + nextPage?: () => Promise; } /** @@ -44,14 +46,14 @@ export interface ISearchResult { * @returns A Promise that will resolve with the data from the response. */ export function searchItems( - search: string | ISearchRequestOptions + search: string | ISearchRequestOptions | SearchQueryBuilder ): Promise { let options: ISearchRequestOptions = { httpMethod: "GET", params: {} }; - if (typeof search === "string") { + if (typeof search === "string" || search instanceof SearchQueryBuilder) { options.params.q = search; } else { // mixin user supplied requestOptions with defaults @@ -71,5 +73,26 @@ export function searchItems( const url = `${getPortalUrl(options)}/search`; // send the request - return request(url, options); + return request(url, options).then(r => { + if (options.rawResponse) { + return r; + } + + if (r.nextStart === -1) { + r.nextPage = function() { + const newOptions = { + ...options, + ...{ + params: { + ...options.params, + ...{ start: r.nextStart } + } + } + }; + return request(url, newOptions); + }; + } + + return r; + }); } diff --git a/packages/arcgis-rest-request/src/index.ts b/packages/arcgis-rest-request/src/index.ts index 336d613097..993deb5d12 100644 --- a/packages/arcgis-rest-request/src/index.ts +++ b/packages/arcgis-rest-request/src/index.ts @@ -9,6 +9,7 @@ export * from "./utils/ArcGISRequestError"; export * from "./utils/retryAuthError"; export * from "./utils/ErrorTypes"; export * from "./utils/params"; +export * from "./utils/IParamBuilder"; export * from "./utils/process-params"; export * from "./utils/get-portal"; export * from "./utils/get-portal-url"; diff --git a/packages/arcgis-rest-request/src/utils/IParamBuilder.ts b/packages/arcgis-rest-request/src/utils/IParamBuilder.ts new file mode 100644 index 0000000000..8a9ad7daf7 --- /dev/null +++ b/packages/arcgis-rest-request/src/utils/IParamBuilder.ts @@ -0,0 +1,3 @@ +export interface IParamBuilder { + toParam(): any; +} diff --git a/packages/arcgis-rest-request/src/utils/process-params.ts b/packages/arcgis-rest-request/src/utils/process-params.ts index 6dc378d777..376dd40324 100644 --- a/packages/arcgis-rest-request/src/utils/process-params.ts +++ b/packages/arcgis-rest-request/src/utils/process-params.ts @@ -8,12 +8,16 @@ */ export function requiresFormData(params: any) { return Object.keys(params).some(key => { - const value = params[key]; + let value = params[key]; if (!value) { return false; } + if (value.toParam) { + value = value.toParam(); + } + const type = value.constructor.name; switch (type) { @@ -46,7 +50,12 @@ export function processParams(params: any): any { const newParams: any = {}; Object.keys(params).forEach(key => { - const param = params[key]; + let param = params[key]; + + if (param.toParams) { + param = param.toParam(); + } + if ( !param && param !== 0 && From 7bf9fa3739fc0f26e462c125eb6cfc104e6f59d4 Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Sat, 13 Apr 2019 07:27:14 -0700 Subject: [PATCH 02/11] passing tests for SearchQueryBuilder --- package-lock.json | 83 ++++++------- package.json | 2 +- .../arcgis-rest-items/src/SearchBuilder.ts | 112 ------------------ packages/arcgis-rest-items/src/search.ts | 10 +- .../arcgis-rest-items/test/mocks/search.ts | 58 +++++++++ .../arcgis-rest-items/test/search.test.ts | 94 ++++++++++++++- packages/arcgis-rest-request/src/index.ts | 1 + packages/arcgis-rest-request/src/request.ts | 4 +- .../src/utils/process-params.ts | 5 +- .../arcgis-rest-request/test/request.test.ts | 77 +++++++++++- umd-base-profile.js | 2 +- 11 files changed, 282 insertions(+), 166 deletions(-) delete mode 100644 packages/arcgis-rest-items/src/SearchBuilder.ts diff --git a/package-lock.json b/package-lock.json index c44834fc46..8c3eae6bbf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7380,15 +7380,6 @@ "integrity": "sha512-6uHUhOPEBgQ24HM+r6b/QwWfZq+yiFcipKFrOFiBEnWdy5sdzYoi+pJeQaPI5qOLRFqWmAXUPQNsielzdLoecA==", "dev": true }, - "graphlib": { - "version": "2.1.7", - "resolved": "https://registry.npmjs.org/graphlib/-/graphlib-2.1.7.tgz", - "integrity": "sha512-TyI9jIy2J4j0qgPmOOrHTCtpPqJGN/aurBwc6ZT+bRii+di1I+Wv3obRhVrmBEXet+qkMaEX67dXrwsd3QQM6w==", - "dev": true, - "requires": { - "lodash": "^4.17.5" - } - }, "gzip-size": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/gzip-size/-/gzip-size-5.0.0.tgz", @@ -11668,12 +11659,6 @@ } } }, - "object-hash": { - "version": "1.3.1", - "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-1.3.1.tgz", - "integrity": "sha512-OSuu/pU4ENM9kmREg0BdNrUDIl1heYa4mBZacJc+vVWz4GtAwu7jO8s4AIt2aGRUTqxykpWzI3Oqnsm13tTMDA==", - "dev": true - }, "object-path": { "version": "0.9.2", "resolved": "https://registry.npmjs.org/object-path/-/object-path-0.9.2.tgz", @@ -13499,34 +13484,52 @@ "resolve": "^1.1.6" } }, - "rollup-plugin-typescript2": { - "version": "0.4.6", - "resolved": "https://registry.npmjs.org/rollup-plugin-typescript2/-/rollup-plugin-typescript2-0.4.6.tgz", - "integrity": "sha1-nPnqTK5LYdUNq2U/8wjAzqtjjiM=", + "rollup-plugin-typescript": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/rollup-plugin-typescript/-/rollup-plugin-typescript-1.0.1.tgz", + "integrity": "sha512-rwJDNn9jv/NsKZuyBb/h0jsclP4CJ58qbvZt2Q9zDIGILF2LtdtvCqMOL+Gq9IVq5MTrTlHZNrn8h7VjQgd8tw==", "dev": true, "requires": { - "colors": "^1.1.2", - "fs-extra": "^3.0.1", - "graphlib": "^2.1.1", - "lodash": "^4.17.4", - "object-hash": "^1.1.8", - "resolve": "^1.3.3", - "rollup-pluginutils": "^2.0.1", - "tslib": "^1.7.1", - "typescript": "^2.3.4" + "resolve": "^1.10.0", + "rollup-pluginutils": "^2.5.0" }, "dependencies": { - "colors": { - "version": "1.3.3", - "resolved": "https://registry.npmjs.org/colors/-/colors-1.3.3.tgz", - "integrity": "sha512-mmGt/1pZqYRjMxB1axhTo16/snVZ5krrKkcmMeVKxzECMMXoCgnvTPp10QgHfcbQZw8Dq2jMNG6je4JlWU0gWg==", + "estree-walker": { + "version": "0.6.0", + "resolved": "https://registry.npmjs.org/estree-walker/-/estree-walker-0.6.0.tgz", + "integrity": "sha512-peq1RfVAVzr3PU/jL31RaOjUKLoZJpObQWJJ+LgfcxDUifyLZ1RjPQZTl0pzj2uJ45b7A7XpyppXvxdEqzo4rw==", "dev": true }, - "typescript": { - "version": "2.9.2", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-2.9.2.tgz", - "integrity": "sha512-Gr4p6nFNaoufRIY4NMdpQRNmgxVIGMs4Fcu/ujdYk3nAZqk7supzBE9idmvfZIlH/Cuj//dvi+019qEue9lV0w==", - "dev": true + "micromatch": { + "version": "3.1.10", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-3.1.10.tgz", + "integrity": "sha512-MWikgl9n9M3w+bpsY3He8L+w9eF9338xRl8IAO5viDizwSzziFEyUzo2xrrloB64ADbTf8uA8vRqqttDTOmccg==", + "dev": true, + "requires": { + "arr-diff": "^4.0.0", + "array-unique": "^0.3.2", + "braces": "^2.3.1", + "define-property": "^2.0.2", + "extend-shallow": "^3.0.2", + "extglob": "^2.0.4", + "fragment-cache": "^0.2.1", + "kind-of": "^6.0.2", + "nanomatch": "^1.2.9", + "object.pick": "^1.3.0", + "regex-not": "^1.0.0", + "snapdragon": "^0.8.1", + "to-regex": "^3.0.2" + } + }, + "rollup-pluginutils": { + "version": "2.6.0", + "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-2.6.0.tgz", + "integrity": "sha512-aGQwspEF8oPKvg37u3p7h0cYNwmJR1sCBMZGZ5b9qy8HGtETknqjzcxrDRrcAnJNXN18lBH4Q9vZYth/p4n8jQ==", + "dev": true, + "requires": { + "estree-walker": "^0.6.0", + "micromatch": "^3.1.10" + } } } }, @@ -15534,9 +15537,9 @@ "dev": true }, "typescript": { - "version": "3.3.3333", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.3.3333.tgz", - "integrity": "sha512-JjSKsAfuHBE/fB2oZ8NxtRTk5iGcg6hkYXMnZ3Wc+b2RSqejEqTaem11mHASMnFilHrax3sLK0GDzcJrekZYLw==", + "version": "3.4.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.4.3.tgz", + "integrity": "sha512-FFgHdPt4T/duxx6Ndf7hwgMZZjZpB+U0nMNGVCYPq0rEzWKjEDobm4J6yb3CS7naZ0yURFqdw9Gwc7UOh/P9oQ==", "dev": true }, "ua-parser-js": { diff --git a/package.json b/package.json index 891b6ac0a2..013963ca0f 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "rollup-plugin-filesize": "^5.0.1", "rollup-plugin-json": "^2.3.0", "rollup-plugin-node-resolve": "^3.0.3", - "rollup-plugin-typescript2": "^0.4.6", + "rollup-plugin-typescript": "^1.0.1", "rollup-plugin-uglify": "^3.0.0", "shelljs": "^0.7.8", "slug": "^0.9.1", diff --git a/packages/arcgis-rest-items/src/SearchBuilder.ts b/packages/arcgis-rest-items/src/SearchBuilder.ts deleted file mode 100644 index de3cecf8c2..0000000000 --- a/packages/arcgis-rest-items/src/SearchBuilder.ts +++ /dev/null @@ -1,112 +0,0 @@ -import { IParamBuilder } from "@esri/arcgis-rest-request"; - -export class SearchQueryBuilder implements IParamBuilder { - private termStack: any[] = []; - private rangeStack: any[] = []; - private q: string; - - constructor(q: string = "") { - this.q = q; - } - - public match(...terms: any[]) { - this.termStack = this.termStack.concat(terms); - return this; - } - - public in(field: string) { - if (field && field !== "*") { - this.q += `${field}: `; - } - this.commit(); - return this; - } - - public startGroup() { - this.commit(); - this.q += "("; - return this; - } - - public endGroup() { - this.q += ")"; - return this; - } - - public and() { - this.commit(); - this.q += " AND "; - return this; - } - - public or() { - this.commit(); - this.q += " OR "; - return this; - } - - public not() { - this.commit(); - this.q += " NOT "; - return this; - } - - public from(term: any) { - this.rangeStack[0] = term; - return this; - } - - public to(term: any) { - this.rangeStack[1] = term; - return this; - } - - public boost(num: number) { - this.commit(); - this.q += "^${num}"; - return this; - } - - public toParam() { - return this.q; - } - - public clone() { - return new SearchQueryBuilder(this.q + ""); - } - - private hasWhiteSpace(s: string) { - return /\s/g.test(s); - } - - private formatTerm(term: any) { - if (term instanceof Date) { - return term.getTime(); - } - - if (typeof term === "string" && this.hasWhiteSpace(term)) { - return `"${term}"`; - } - - return term; - } - - private commit() { - if (this.rangeStack.length && this.rangeStack[0] && this.rangeStack[1]) { - this.q += `[${this.formatTerm(this.rangeStack[0])} TO ${this.formatTerm( - this.rangeStack[1] - )}]`; - this.rangeStack = [undefined, undefined]; - } - - if (this.termStack.length) { - this.q += this.termStack - .map(term => { - return term; - }) - .join(" "); - } - - this.termStack = []; - } -} diff --git a/packages/arcgis-rest-items/src/search.ts b/packages/arcgis-rest-items/src/search.ts index 77dead8528..c76f1c685f 100644 --- a/packages/arcgis-rest-items/src/search.ts +++ b/packages/arcgis-rest-items/src/search.ts @@ -8,7 +8,7 @@ import { } from "@esri/arcgis-rest-request"; import { IPagingParams, IItem } from "@esri/arcgis-rest-common-types"; -import { SearchQueryBuilder } from "./SearchBuilder"; +import { SearchQueryBuilder } from "./SearchQueryBuilder"; // this interface still needs to be docced export interface ISearchRequest extends IPagingParams { @@ -74,11 +74,7 @@ export function searchItems( // send the request return request(url, options).then(r => { - if (options.rawResponse) { - return r; - } - - if (r.nextStart === -1) { + if (r.nextStart && r.nextStart !== -1) { r.nextPage = function() { const newOptions = { ...options, @@ -89,7 +85,7 @@ export function searchItems( } } }; - return request(url, newOptions); + return searchItems(newOptions); }; } diff --git a/packages/arcgis-rest-items/test/mocks/search.ts b/packages/arcgis-rest-items/test/mocks/search.ts index 5b20a892b9..cfca37437c 100644 --- a/packages/arcgis-rest-items/test/mocks/search.ts +++ b/packages/arcgis-rest-items/test/mocks/search.ts @@ -4,6 +4,64 @@ import { ISearchResult } from "../../src/search"; export const SearchResponse: ISearchResult = { + query: "", + total: 10795, + start: 1, + num: 1, + nextStart: -1, + results: [ + { + id: "a5b", + owner: "dcadminqa", + created: 1496748288000, + modified: 1508856526000, + name: "survey123_7d7a9fabcb0c44bcaf1d6473cd088a07", + title: " Drug Activity Reporter", + type: "Feature Service", + typeKeywords: [ + "ArcGIS Server", + "Data", + "Feature Access", + "Feature Service", + "OwnerView", + "Service", + "Survey123", + "Survey123 Hub", + "Hosted Service" + ], + description: "Some Description", + tags: [], + snippet: "Some Snippet", + thumbnail: "thumbnail/ago_downloaded.png", + documentation: "WAT docs", + extent: [[-180, -90], [180, 90]], + categories: [], + spatialReference: null, + accessInformation: null, + licenseInfo: null, + culture: null, + properties: {}, + url: + "https://servicesqa.arcgis.com/97KLIFOSt5CxbiRI/arcgis/rest/services/survey123_7d7a9fabcb0c44bcaf1d6473cd088a07/FeatureServer", + proxyFilter: null, + access: "shared", + size: -1, + appCategories: [], + industries: [], + languages: [], + largeThumbnail: null, + banner: null, + screenshots: [], + listed: false, + numComments: 0, + numRatings: 0, + avgRating: 0, + numViews: 4 + } + ] +}; + +export const BigSearchResponse: ISearchResult = { query: "", total: 10795, start: 1, diff --git a/packages/arcgis-rest-items/test/search.test.ts b/packages/arcgis-rest-items/test/search.test.ts index 5f94e6856c..14fc99d5b9 100644 --- a/packages/arcgis-rest-items/test/search.test.ts +++ b/packages/arcgis-rest-items/test/search.test.ts @@ -5,9 +5,10 @@ import * as fetchMock from "fetch-mock"; import { searchItems, ISearchRequestOptions } from "../src/search"; -import { SearchResponse } from "./mocks/search"; +import { SearchResponse, BigSearchResponse } from "./mocks/search"; import { UserSession } from "@esri/arcgis-rest-auth"; import { TOMORROW } from "@esri/arcgis-rest-auth/test/utils"; +import { SearchQueryBuilder } from "../src/SearchQueryBuilder"; describe("search", () => { afterEach(fetchMock.restore); @@ -30,6 +31,31 @@ describe("search", () => { }); }); + it("should make a simple, single search request with a builder", done => { + fetchMock.once("*", SearchResponse); + const expectedParam = "DC AND typekeywords: hubSiteApplication"; + const q = new SearchQueryBuilder() + .match("DC") + .and() + .match("hubSiteApplication") + .in("typekeywords"); + searchItems(q) + .then(() => { + expect(fetchMock.called()).toEqual(true); + const [url, options]: [string, RequestInit] = fetchMock.lastCall("*"); + expect(url).toEqual( + `https://www.arcgis.com/sharing/rest/search?f=json&q=${encodeURIComponent( + expectedParam + )}` + ); + expect(options.method).toBe("GET"); + done(); + }) + .catch(e => { + fail(e); + }); + }); + it("should take num, start, sortField, sortDir and construct the request", done => { fetchMock.once("*", SearchResponse); @@ -84,6 +110,37 @@ describe("search", () => { }); }); + it("should pass through other requestOptions at the same time with a builder", done => { + fetchMock.once("*", SearchResponse); + const expectedParam = "DC AND typekeywords: hubSiteApplication"; + const q = new SearchQueryBuilder() + .match("DC") + .and() + .match("hubSiteApplication") + .in("typekeywords"); + searchItems({ + searchForm: { + q, + num: 12, + start: 22, + sortField: "title", + sortDir: "desc" + }, + httpMethod: "POST" + }) + .then(() => { + expect(fetchMock.called()).toEqual(true); + const [url, options]: [string, RequestInit] = fetchMock.lastCall("*"); + expect(url).toEqual("https://www.arcgis.com/sharing/rest/search"); + expect(options.body).toContain(encodeURIComponent(expectedParam)); + expect(options.method).toBe("POST"); + done(); + }) + .catch(e => { + fail(e); + }); + }); + it("should mixin generic params with the search form", done => { fetchMock.once("*", SearchResponse); @@ -113,6 +170,41 @@ describe("search", () => { }); }); + it("should provide a nextSearch() method to fetch the next page", done => { + fetchMock.once("*", BigSearchResponse); + + searchItems("DC AND typekeywords:hubSiteApplication") + .then(r => { + expect(fetchMock.called()).toEqual(true); + const [url]: [string, RequestInit] = fetchMock.lastCall("*"); + expect(url).toEqual( + "https://www.arcgis.com/sharing/rest/search?f=json&q=DC%20AND%20typekeywords%3AhubSiteApplication" + ); + if (r.nextPage) { + fetchMock.once("*", BigSearchResponse); + + r.nextPage() + .then(() => { + const [nextUrl]: [string, RequestInit] = fetchMock.lastCall("*"); + + expect(nextUrl).toEqual( + "https://www.arcgis.com/sharing/rest/search?f=json&q=DC%20AND%20typekeywords%3AhubSiteApplication&start=2" + ); + + done(); + }) + .catch(e => { + fail(e); + }); + } else { + fail("search result did not have a nextPage() function"); + } + }) + .catch(e => { + fail(e); + }); + }); + describe("Authenticated methods", () => { // setup a UserSession to use in all these tests const MOCK_USER_SESSION = new UserSession({ diff --git a/packages/arcgis-rest-request/src/index.ts b/packages/arcgis-rest-request/src/index.ts index 993deb5d12..c0a42cff05 100644 --- a/packages/arcgis-rest-request/src/index.ts +++ b/packages/arcgis-rest-request/src/index.ts @@ -10,6 +10,7 @@ export * from "./utils/retryAuthError"; export * from "./utils/ErrorTypes"; export * from "./utils/params"; export * from "./utils/IParamBuilder"; +export * from "./utils/IParamsBuilder"; export * from "./utils/process-params"; export * from "./utils/get-portal"; export * from "./utils/get-portal-url"; diff --git a/packages/arcgis-rest-request/src/request.ts b/packages/arcgis-rest-request/src/request.ts index f32ef8aec8..ff749b9ae0 100644 --- a/packages/arcgis-rest-request/src/request.ts +++ b/packages/arcgis-rest-request/src/request.ts @@ -146,7 +146,9 @@ export function request( const params: IParams = { ...{ f: "json" }, - ...requestOptions.params + ...(requestOptions.params && requestOptions.params.toParams + ? requestOptions.params.toParams() + : requestOptions.params) }; const fetchOptions: RequestInit = { diff --git a/packages/arcgis-rest-request/src/utils/process-params.ts b/packages/arcgis-rest-request/src/utils/process-params.ts index 376dd40324..8d7052249c 100644 --- a/packages/arcgis-rest-request/src/utils/process-params.ts +++ b/packages/arcgis-rest-request/src/utils/process-params.ts @@ -14,7 +14,7 @@ export function requiresFormData(params: any) { return false; } - if (value.toParam) { + if (value && value.toParam) { value = value.toParam(); } @@ -52,7 +52,7 @@ export function processParams(params: any): any { Object.keys(params).forEach(key => { let param = params[key]; - if (param.toParams) { + if (param && param.toParam) { param = param.toParam(); } @@ -64,6 +64,7 @@ export function processParams(params: any): any { ) { return; } + const type = param.constructor.name; let value: any; diff --git a/packages/arcgis-rest-request/test/request.test.ts b/packages/arcgis-rest-request/test/request.test.ts index 489d269340..548740788a 100644 --- a/packages/arcgis-rest-request/test/request.test.ts +++ b/packages/arcgis-rest-request/test/request.test.ts @@ -1,15 +1,20 @@ /* Copyright (c) 2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { request, ErrorTypes } from "../src/index"; +import { request, ErrorTypes, IParamsBuilder } from "../src/index"; import * as fetchMock from "fetch-mock"; import { SharingRestInfo, SharingRestInfoHTML } from "./mocks/sharing-rest-info"; +import { MockParamBuilder } from "./mocks/param-builder"; +import { MockParamsBuilder } from "./mocks/params-builder"; +import { MockNestedBuilder } from "./mocks/params-builder-nested"; + import { ArcGISOnlineError } from "./mocks/errors"; import { WebMapAsText, WebMapAsJSON } from "./mocks/webmap"; import { GeoJSONFeatureCollection } from "./mocks/geojson-feature-collection"; +import { IParams } from "../dist/esm/utils/IParams"; describe("request()", () => { afterEach(fetchMock.restore); @@ -260,6 +265,76 @@ describe("request()", () => { }); }); + describe("should impliment the IParamBuilder and IParamsBuilder interfaces builder", () => { + it("should encode a param that impliments IParamBuilder", done => { + fetchMock.once("*", GeoJSONFeatureCollection); + + const builder = new MockParamBuilder(); + + request( + "https://services1.arcgis.com/ORG/arcgis/rest/services/FEATURE_SERVICE/FeatureServer/0/query", + { + params: { where: builder, f: "geojson" } + } + ) + .then(response => { + const options: RequestInit = fetchMock.lastCall("*")[1]; + expect(options.body).toContain(`where=${encodeURIComponent("1=1")}`); + expect(response).toEqual(GeoJSONFeatureCollection); + done(); + }) + .catch(e => { + fail(e); + }); + }); + + it("should encode a param that impliments IParamsBuilder", done => { + fetchMock.once("*", GeoJSONFeatureCollection); + + const builder = new MockParamsBuilder(); + + request( + "https://services1.arcgis.com/ORG/arcgis/rest/services/FEATURE_SERVICE/FeatureServer/0/query", + { + params: builder + } + ) + .then(response => { + const options: RequestInit = fetchMock.lastCall("*")[1]; + expect(options.body).toContain(`where=${encodeURIComponent("1=1")}`); + expect(options.body).toContain(`f=geojson`); + expect(response).toEqual(GeoJSONFeatureCollection); + done(); + }) + .catch(e => { + fail(e); + }); + }); + + it("should chain IParamBuilder and IParamsBuilder", done => { + fetchMock.once("*", GeoJSONFeatureCollection); + + const builder = new MockNestedBuilder(); + + request( + "https://services1.arcgis.com/ORG/arcgis/rest/services/FEATURE_SERVICE/FeatureServer/0/query", + { + params: builder + } + ) + .then(response => { + const options: RequestInit = fetchMock.lastCall("*")[1]; + expect(options.body).toContain(`where=${encodeURIComponent("1=1")}`); + expect(options.body).toContain(`f=geojson`); + expect(response).toEqual(GeoJSONFeatureCollection); + done(); + }) + .catch(e => { + fail(e); + }); + }); + }); + describe("should throw errors when required dependencies are missing", () => { const oldPromise = Promise; const oldFetch = fetch; diff --git a/umd-base-profile.js b/umd-base-profile.js index eefdcd53bb..bc93d40801 100644 --- a/umd-base-profile.js +++ b/umd-base-profile.js @@ -1,4 +1,4 @@ -import typescript from "rollup-plugin-typescript2"; +import typescript from "rollup-plugin-typescript"; import resolve from "rollup-plugin-node-resolve"; import commonjs from "rollup-plugin-commonjs"; import json from "rollup-plugin-json"; From 2b040fbbcc422d73df98806b26c07258dfffb08a Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Sat, 13 Apr 2019 07:28:47 -0700 Subject: [PATCH 03/11] passing tests for SearchQueryBuilder, IParamBuilder and IParamsBuilder --- .../src/SearchQueryBuilder.ts | 207 ++++++++++++ .../test/SearchQueryBuilder.test.ts | 314 ++++++++++++++++++ .../src/utils/IParamsBuilder.ts | 5 + .../test/mocks/param-builder.ts | 7 + .../test/mocks/params-builder-nested.ts | 8 + .../test/mocks/params-builder.ts | 7 + 6 files changed, 548 insertions(+) create mode 100644 packages/arcgis-rest-items/src/SearchQueryBuilder.ts create mode 100644 packages/arcgis-rest-items/test/SearchQueryBuilder.test.ts create mode 100644 packages/arcgis-rest-request/src/utils/IParamsBuilder.ts create mode 100644 packages/arcgis-rest-request/test/mocks/param-builder.ts create mode 100644 packages/arcgis-rest-request/test/mocks/params-builder-nested.ts create mode 100644 packages/arcgis-rest-request/test/mocks/params-builder.ts diff --git a/packages/arcgis-rest-items/src/SearchQueryBuilder.ts b/packages/arcgis-rest-items/src/SearchQueryBuilder.ts new file mode 100644 index 0000000000..278fe210f6 --- /dev/null +++ b/packages/arcgis-rest-items/src/SearchQueryBuilder.ts @@ -0,0 +1,207 @@ +import { IParamBuilder, warn } from "@esri/arcgis-rest-request"; + +export class SearchQueryBuilder implements IParamBuilder { + private termStack: any[] = []; + private rangeStack: any[] = []; + private q: string; + private openGroups = 0; + private currentModifer: string; + + constructor(q: string = "") { + this.q = q; + } + + public match(...terms: any[]) { + this.termStack = this.termStack.concat(terms); + return this; + } + + public in(field?: string) { + const fn = `\`in(${field ? `"${field}"` : ""})\``; + + if (!this.hasRange && !this.hasTerms) { + warn( + // prettier-ignore + `${fn} was called with no call to \`match(...)\` or \`from(...)\`/\`to(...)\`. Your query was not modified.` + ); + return this; + } + + if (field && field !== "*") { + this.q += `${field}: `; + } + + return this.commit(); + } + + public startGroup() { + this.commit(); + if (this.openGroups > 0) { + this.q += " "; + } + this.openGroups++; + this.q += "("; + return this; + } + + public endGroup() { + if (this.openGroups <= 0) { + warn( + `\`endGroup(...)\` was called without calling \`startGroup(...)\` first. Your query was not modified.` + ); + return this; + } + this.openGroups--; + this.q += ")"; + return this.commit(); + } + + public and() { + return this.addModifier("and"); + } + + public or() { + return this.addModifier("or"); + } + + public not() { + return this.addModifier("not"); + } + + public from(term: any) { + if (this.hasTerms) { + warn( + // prettier-ignore + `\`from(...)\` is not allowed after \`match(...)\` try using \`.from(...).to(...).in(...)\`. Your query was not modified.` + ); + return this; + } + this.rangeStack[0] = term; + return this; + } + + public to(term: any) { + if (this.hasTerms) { + warn( + // prettier-ignore + `\`to(...)\` is not allowed after \`match(...)\` try using \`.from(...).to(...).in(...)\`. Your query was not modified.` + ); + return this; + } + this.rangeStack[1] = term; + return this; + } + + public boost(num: number) { + this.commit(); + this.q += `^${num}`; + return this; + } + + public toParam() { + this.commit(); + this.cleanup(); + return this.q; + } + + public clone() { + this.commit(); + this.cleanup(); + return new SearchQueryBuilder(this.q + ""); + } + + private addModifier(modifier: string) { + if (this.currentModifer) { + warn( + // prettier-ignore + `You have called \`${this.currentModifer}()\` after \`${modifier}()\`. Your current query was not modified.` + ); + return this; + } + + this.commit(); + + if (this.q === "") { + warn( + `You have called \`${modifier}()\` without calling another method to modify your query first. Try calling \`match()\` first.` + ); + return this; + } + + this.currentModifer = modifier; + this.q += ` ${modifier.toUpperCase()} `; + return this; + } + + private hasWhiteSpace(s: string) { + return /\s/g.test(s); + } + + private formatTerm(term: any) { + if (term instanceof Date) { + return term.getTime(); + } + + if (typeof term === "string" && this.hasWhiteSpace(term)) { + return `"${term}"`; + } + + return term; + } + + private commit() { + this.currentModifer = undefined; + if (this.hasRange) { + this.q += `[${this.formatTerm(this.rangeStack[0])} TO ${this.formatTerm( + this.rangeStack[1] + )}]`; + this.rangeStack = [undefined, undefined]; + } + + if (this.hasTerms) { + this.q += this.termStack + .map(term => { + return this.formatTerm(term); + }) + .join(" "); + this.termStack = []; + } + + return this; + } + + private get hasTerms() { + return this.termStack.length > 0; + } + + private get hasRange() { + return this.rangeStack.length && this.rangeStack[0] && this.rangeStack[1]; + } + + private cleanup() { + // end a group if we have started one + if (this.openGroups > 0) { + warn( + // prettier-ignore + `Automatically closing ${this.openGroups} group(s). You can use \`endGroup(...)\` to remove this warning.` + ); + + while (this.openGroups > 0) { + this.q += ")"; + this.openGroups--; + } + } + + const oldQ = this.q; + this.q = oldQ.replace(/( AND ?| NOT ?| OR ?)*$/, ""); + + if (oldQ !== this.q) { + warn( + `\`startGroup(...)\` was called without calling \`endGroup(...)\` first. Your query was not modified.` + ); + } + + // clear empty groups + this.q = this.q.replace(/(\(\))*/, ""); + } +} diff --git a/packages/arcgis-rest-items/test/SearchQueryBuilder.test.ts b/packages/arcgis-rest-items/test/SearchQueryBuilder.test.ts new file mode 100644 index 0000000000..08940fb8ed --- /dev/null +++ b/packages/arcgis-rest-items/test/SearchQueryBuilder.test.ts @@ -0,0 +1,314 @@ +import { SearchQueryBuilder } from "../src/SearchQueryBuilder"; + +describe("SearchQueryBuilder", () => { + const originalWarn = console.warn; + + beforeAll(function() { + console.warn = jasmine.createSpy().and.callFake(() => { + return; + }); + }); + + afterAll(function() { + console.warn = originalWarn; + }); + + it("should return and empty string when called with no other functions", () => { + const query = new SearchQueryBuilder().toParam(); + expect(query).toEqual(""); + }); + + it("should format a simple search query", () => { + const query = new SearchQueryBuilder().match("test").toParam(); + expect(query).toEqual("test"); + }); + + it("should format a simple search query in a field", () => { + const query = new SearchQueryBuilder() + .match("test") + .in("tags") + .toParam(); + expect(query).toEqual("tags: test"); + }); + + it("should warp multi word search terms in quotes", () => { + const query = new SearchQueryBuilder().match("foo bar").toParam(); + expect(query).toEqual(`"foo bar"`); + }); + + it("should accept .in() without a parameter", () => { + const query = new SearchQueryBuilder() + .match("test") + .in() + .toParam(); + expect(query).toEqual(`test`); + }); + + it("should accept `*` as a value for .in()", () => { + const query = new SearchQueryBuilder() + .match("test") + .in("*") + .toParam(); + expect(query).toEqual(`test`); + }); + + it("should chain calls with .and()", () => { + const query = new SearchQueryBuilder() + .match("bar") + .and() + .match("foo") + .in("tags") + .toParam(); + + expect(query).toEqual("bar AND tags: foo"); + }); + + it("should format a simple range", () => { + const query = new SearchQueryBuilder() + .from("a") + .to("z") + .in("title") + .toParam(); + expect(query).toEqual("title: [a TO z]"); + }); + + it("should format a simple group", () => { + const query = new SearchQueryBuilder() + .startGroup() + .from("a") + .to("z") + .in("title") + .endGroup() + .toParam(); + expect(query).toEqual("(title: [a TO z])"); + }); + + it("should boost the previous search", () => { + const query = new SearchQueryBuilder() + .match("test") + .boost(5) + .toParam(); + expect(query).toEqual("test^5"); + }); + + it("should convert dates into timestamps", () => { + const date1 = new Date("January 1 2019"); + const date2 = new Date("January 7 2019"); + const expectedDate1 = date1.getTime(); + const expectedDate2 = date2.getTime(); + + const query = new SearchQueryBuilder() + .from(date1) + .to(date2) + .in("created") + .and() + .match("test") + .in("tags") + .toParam(); + + expect(query).toEqual( + `created: [${expectedDate1} TO ${expectedDate2}] AND tags: test` + ); + }); + + it("should format a complex group properly", () => { + const query = new SearchQueryBuilder() + .match("fred") + .in("owner") + .and() + .startGroup() + .match("Web Mapping Application") + .in("type") + .or() + .match("Mobile Application") + .in("type") + .or() + .match("Application") + .in("type") + .endGroup() + .and() + .match("test") + .in("*") + .toParam(); + + expect(query).toEqual( + `owner: fred AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application) AND test` + ); + }); + + it("should clone searches for modification", () => { + const myAppsQuery = new SearchQueryBuilder() + .match("fred") + .in("owner") + .and() + .startGroup() + .match("Web Mapping Application") + .in("type") + .or() + .match("Mobile Application") + .in("type") + .or() + .match("Application") + .in("type") + .endGroup(); + + const myTestAppsQuery = myAppsQuery + .clone() + .and() + .match("test") + .in("*"); + + expect(myAppsQuery.toParam()).toEqual( + `owner: fred AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application)` + ); + + expect(myTestAppsQuery.toParam()).toEqual( + `owner: fred AND (type: "Web Mapping Application" OR type: "Mobile Application" OR type: Application) AND test` + ); + }); + + it("should not allow trailing modifiers, and warn user", () => { + const query = new SearchQueryBuilder() + .match("test") + .not() + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("test"); + }); + + it("should not allow chains of logic modifiers, and warn user", () => { + const query = new SearchQueryBuilder() + .not() + .and() + .or() + .not() + .or() + .or() + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual(""); + }); + + it("should not allow chains of logic modifiers after a .match, and warn user", () => { + const query = new SearchQueryBuilder() + .match("test") + .not() + .and() + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("test"); + }); + + it("should close groups on toParam(), and warn user", () => { + const query = new SearchQueryBuilder() + .startGroup() + .match("test") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("(test)"); + }); + + it("should close groups automatically on clone(), and warn user", () => { + const query = new SearchQueryBuilder() + .startGroup() + .match("test") + .clone() + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("(test)"); + }); + + it("should close more then 1 group, and warn user", () => { + const query = new SearchQueryBuilder() + .startGroup() + .match("foo") + .startGroup() + .match("bar") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("(foo (bar))"); + }); + + it("should not allow .in() without valid term or range and warn user", () => { + const query = new SearchQueryBuilder() + .in("tags") + .in("title") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual(""); + }); + + it("should not allow .in() with only .from() and warn user", () => { + const query = new SearchQueryBuilder() + .from("a") + .in("title") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual(""); + }); + + it("should not allow .in() with only .to() and warn user", () => { + const query = new SearchQueryBuilder() + .to("a") + .in("title") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual(""); + }); + + it("should not allow .to() after a .match() and warn user", () => { + const query = new SearchQueryBuilder() + .match("test") + .to("a") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("test"); + }); + + it("should not allow .to() after a .match() and warn user", () => { + const query = new SearchQueryBuilder() + .match("test") + .from("a") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("test"); + }); + + it("should not allow .endGroup() without .startGroup()", () => { + const query = new SearchQueryBuilder() + .match("a") + .in("title") + .endGroup() + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("title: a"); + }); + + it("should not allow .match().from().in(), and warn user", () => { + const query = new SearchQueryBuilder() + .match("test") + .from("a") + .in("title") + .toParam(); + + expect(console.warn).toHaveBeenCalled(); + expect(query).toEqual("title: test"); + }); + + it("should", () => { + const query = new SearchQueryBuilder().toParam(); + expect(query).toEqual(""); + }); +}); diff --git a/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts b/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts new file mode 100644 index 0000000000..edc1eb60c9 --- /dev/null +++ b/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts @@ -0,0 +1,5 @@ +import { IParams } from "./params"; + +export interface IParamsBuilder { + toParams(additionalParmas?: IParams): IParams; +} diff --git a/packages/arcgis-rest-request/test/mocks/param-builder.ts b/packages/arcgis-rest-request/test/mocks/param-builder.ts new file mode 100644 index 0000000000..4c42dc4f7e --- /dev/null +++ b/packages/arcgis-rest-request/test/mocks/param-builder.ts @@ -0,0 +1,7 @@ +import { IParamBuilder } from "../../src/index"; + +export class MockParamBuilder implements IParamBuilder { + public toParam() { + return "1=1"; + } +} diff --git a/packages/arcgis-rest-request/test/mocks/params-builder-nested.ts b/packages/arcgis-rest-request/test/mocks/params-builder-nested.ts new file mode 100644 index 0000000000..49df3d7296 --- /dev/null +++ b/packages/arcgis-rest-request/test/mocks/params-builder-nested.ts @@ -0,0 +1,8 @@ +import { IParamsBuilder, IParams } from "../../src/index"; +import { MockParamBuilder } from "./param-builder"; + +export class MockNestedBuilder implements IParamsBuilder { + public toParams(): IParams { + return { where: new MockParamBuilder(), f: "geojson" }; + } +} diff --git a/packages/arcgis-rest-request/test/mocks/params-builder.ts b/packages/arcgis-rest-request/test/mocks/params-builder.ts new file mode 100644 index 0000000000..3cef72e384 --- /dev/null +++ b/packages/arcgis-rest-request/test/mocks/params-builder.ts @@ -0,0 +1,7 @@ +import { IParamsBuilder, ResponseFormats } from "../../src/index"; + +export class MockParamsBuilder implements IParamsBuilder { + public toParams() { + return { where: "1=1", f: "geojson" as ResponseFormats }; + } +} From 8091c022b0d15be8e806ca37f914919e8d5daab1 Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Mon, 15 Apr 2019 12:31:02 -0700 Subject: [PATCH 04/11] export SearchQueryBuilder --- packages/arcgis-rest-items/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/arcgis-rest-items/src/index.ts b/packages/arcgis-rest-items/src/index.ts index a55f011762..ad134f0d39 100644 --- a/packages/arcgis-rest-items/src/index.ts +++ b/packages/arcgis-rest-items/src/index.ts @@ -9,3 +9,4 @@ export * from "./remove"; export * from "./search"; export * from "./update"; export * from "./helpers"; +export * from "./SearchQueryBuilder"; From aaa2cbc87a6ecd7d80445cd42774f99a2d2a6816 Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Mon, 15 Apr 2019 16:09:05 -0700 Subject: [PATCH 05/11] refactor searchItems to drop searchForm option --- packages/arcgis-rest-common/package.json | 3 +- packages/arcgis-rest-common/src/index.ts | 1 + packages/arcgis-rest-items/package.json | 3 +- packages/arcgis-rest-items/src/search.ts | 65 ++++++------ .../arcgis-rest-items/test/search.test.ts | 99 +++++++++++-------- packages/arcgis-rest-request/src/request.ts | 2 + 6 files changed, 100 insertions(+), 73 deletions(-) diff --git a/packages/arcgis-rest-common/package.json b/packages/arcgis-rest-common/package.json index 192ab9499e..f88780d789 100644 --- a/packages/arcgis-rest-common/package.json +++ b/packages/arcgis-rest-common/package.json @@ -13,7 +13,8 @@ "dist/**" ], "dependencies": { - "tslib": "^1.9.3" + "tslib": "^1.9.3", + "@esri/arcgis-rest-request": "^1.19.2" }, "scripts": { "prepare": "npm run build", diff --git a/packages/arcgis-rest-common/src/index.ts b/packages/arcgis-rest-common/src/index.ts index 6b6ec02ea2..4627e42ce9 100644 --- a/packages/arcgis-rest-common/src/index.ts +++ b/packages/arcgis-rest-common/src/index.ts @@ -3,6 +3,7 @@ // utility methods export * from "./util/location"; +export * from "./util/append-custom-params"; // types export * from "./types/feature"; diff --git a/packages/arcgis-rest-items/package.json b/packages/arcgis-rest-items/package.json index 9ba72e5d3f..d0d2097315 100644 --- a/packages/arcgis-rest-items/package.json +++ b/packages/arcgis-rest-items/package.json @@ -17,11 +17,12 @@ }, "devDependencies": { "@esri/arcgis-rest-auth": "^1.19.2", - "@esri/arcgis-rest-common-types": "^1.19.2", + "@esri/arcgis-rest-common": "^1.19.2", "@esri/arcgis-rest-request": "^1.19.2" }, "peerDependencies": { "@esri/arcgis-rest-auth": "^1.19.2", + "@esri/arcgis-rest-common": "^1.19.2", "@esri/arcgis-rest-common-types": "^1.19.2", "@esri/arcgis-rest-request": "^1.19.2" }, diff --git a/packages/arcgis-rest-items/src/search.ts b/packages/arcgis-rest-items/src/search.ts index c76f1c685f..c4a3f85939 100644 --- a/packages/arcgis-rest-items/src/search.ts +++ b/packages/arcgis-rest-items/src/search.ts @@ -6,20 +6,17 @@ import { IRequestOptions, getPortalUrl } from "@esri/arcgis-rest-request"; - +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { IPagingParams, IItem } from "@esri/arcgis-rest-common-types"; import { SearchQueryBuilder } from "./SearchQueryBuilder"; -// this interface still needs to be docced -export interface ISearchRequest extends IPagingParams { +export interface ISearchRequestOptions extends IRequestOptions, IPagingParams { q: string | SearchQueryBuilder; + sortField?: string; + sortDir?: string; [key: string]: any; } -export interface ISearchRequestOptions extends IRequestOptions { - searchForm?: ISearchRequest; -} - /** * Options to pass through when searching for items. */ @@ -30,7 +27,7 @@ export interface ISearchResult { num: number; nextStart: number; results: IItem[]; - nextPage?: () => Promise; + nextPage?: () => Promise; } /** @@ -48,25 +45,23 @@ export interface ISearchResult { export function searchItems( search: string | ISearchRequestOptions | SearchQueryBuilder ): Promise { - let options: ISearchRequestOptions = { - httpMethod: "GET", - params: {} - }; + let options: IRequestOptions; if (typeof search === "string" || search instanceof SearchQueryBuilder) { - options.params.q = search; - } else { - // mixin user supplied requestOptions with defaults options = { - ...options, - ...search - }; - - // mixin arbitrary request parameters with search form - options.params = { - ...search.params, - ...search.searchForm + httpMethod: "GET", + params: { + q: search + } }; + } else { + options = appendCustomParams( + search, + ["q", "num", "start", "sortField", "sortDir"], + { + httpMethod: "GET" + } + ); } // construct the search url @@ -76,15 +71,21 @@ export function searchItems( return request(url, options).then(r => { if (r.nextStart && r.nextStart !== -1) { r.nextPage = function() { - const newOptions = { - ...options, - ...{ - params: { - ...options.params, - ...{ start: r.nextStart } - } - } - }; + let newOptions: ISearchRequestOptions; + + if ( + typeof search === "string" || + search instanceof SearchQueryBuilder + ) { + newOptions = { + q: search, + start: r.nextStart + }; + } else { + newOptions = search; + newOptions.start = r.nextStart; + } + return searchItems(newOptions); }; } diff --git a/packages/arcgis-rest-items/test/search.test.ts b/packages/arcgis-rest-items/test/search.test.ts index 14fc99d5b9..c3b88e01db 100644 --- a/packages/arcgis-rest-items/test/search.test.ts +++ b/packages/arcgis-rest-items/test/search.test.ts @@ -60,13 +60,11 @@ describe("search", () => { fetchMock.once("*", SearchResponse); searchItems({ - searchForm: { - q: "DC AND typekeywords:hubSiteApplication", - num: 12, - start: 22, - sortField: "title", - sortDir: "desc" - } + q: "DC AND typekeywords:hubSiteApplication", + num: 12, + start: 22, + sortField: "title", + sortDir: "desc" }) .then(() => { expect(fetchMock.called()).toEqual(true); @@ -86,13 +84,11 @@ describe("search", () => { fetchMock.once("*", SearchResponse); searchItems({ - searchForm: { - q: "DC AND typekeywords:hubSiteApplication", - num: 12, - start: 22, - sortField: "title", - sortDir: "desc" - }, + q: "DC AND typekeywords:hubSiteApplication", + num: 12, + start: 22, + sortField: "title", + sortDir: "desc", httpMethod: "POST" }) .then(() => { @@ -119,13 +115,12 @@ describe("search", () => { .match("hubSiteApplication") .in("typekeywords"); searchItems({ - searchForm: { - q, - num: 12, - start: 22, - sortField: "title", - sortDir: "desc" - }, + q, + num: 12, + start: 22, + sortField: "title", + sortDir: "desc", + httpMethod: "POST" }) .then(() => { @@ -145,13 +140,11 @@ describe("search", () => { fetchMock.once("*", SearchResponse); searchItems({ - searchForm: { - q: "DC AND typekeywords:hubSiteApplication", - num: 12, - start: 22, - sortField: "title", - sortDir: "desc" - }, + q: "DC AND typekeywords:hubSiteApplication", + num: 12, + start: 22, + sortField: "title", + sortDir: "desc", params: { foo: "bar" } @@ -205,6 +198,41 @@ describe("search", () => { }); }); + it("should provide a nextSearch() method to fetch the next page when using options", done => { + fetchMock.once("*", BigSearchResponse); + + searchItems({ q: "DC AND typekeywords:hubSiteApplication" }) + .then(r => { + expect(fetchMock.called()).toEqual(true); + const [url]: [string, RequestInit] = fetchMock.lastCall("*"); + expect(url).toEqual( + "https://www.arcgis.com/sharing/rest/search?f=json&q=DC%20AND%20typekeywords%3AhubSiteApplication" + ); + if (r.nextPage) { + fetchMock.once("*", BigSearchResponse); + + r.nextPage() + .then(() => { + const [nextUrl]: [string, RequestInit] = fetchMock.lastCall("*"); + + expect(nextUrl).toEqual( + "https://www.arcgis.com/sharing/rest/search?f=json&q=DC%20AND%20typekeywords%3AhubSiteApplication&start=2" + ); + + done(); + }) + .catch(e => { + fail(e); + }); + } else { + fail("search result did not have a nextPage() function"); + } + }) + .catch(e => { + fail(e); + }); + }); + describe("Authenticated methods", () => { // setup a UserSession to use in all these tests const MOCK_USER_SESSION = new UserSession({ @@ -220,20 +248,13 @@ describe("search", () => { portal: "https://myorg.maps.arcgis.com/sharing/rest" }); - const MOCK_USER_REQOPTS = { - authentication: MOCK_USER_SESSION - }; - it("search should use the portal and token from Auth Manager", done => { fetchMock.once("*", SearchResponse); - const MOCK_USER_REQOPTS_SEARCH = MOCK_USER_REQOPTS as ISearchRequestOptions; - - MOCK_USER_REQOPTS_SEARCH.searchForm = { - q: "DC AND typekeywords:hubSiteApplication" - }; - - searchItems(MOCK_USER_REQOPTS_SEARCH) + searchItems({ + q: "DC AND typekeywords:hubSiteApplication", + authentication: MOCK_USER_SESSION + }) .then(() => { expect(fetchMock.called()).toEqual(true); const [url, options]: [string, RequestInit] = fetchMock.lastCall("*"); diff --git a/packages/arcgis-rest-request/src/request.ts b/packages/arcgis-rest-request/src/request.ts index ff749b9ae0..997da798dd 100644 --- a/packages/arcgis-rest-request/src/request.ts +++ b/packages/arcgis-rest-request/src/request.ts @@ -72,6 +72,8 @@ export interface IRequestOptions { * Additional [Headers](https://developer.mozilla.org/en-US/docs/Web/API/Headers) to pass into the request. */ headers?: { [key: string]: any }; + + [key: string]: any; } export const NODEJS_DEFAULT_REFERER_HEADER = `@esri/arcgis-rest-js`; From f9225db445378814c76656fc75221d9a916c6936 Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Mon, 15 Apr 2019 16:12:11 -0700 Subject: [PATCH 06/11] refactor searchItems to drop searchForm option --- .../src/util/append-custom-params.ts | 47 +++++++++++++++++++ .../test/append-custom-params.test.ts | 24 ++++++++++ 2 files changed, 71 insertions(+) create mode 100644 packages/arcgis-rest-common/src/util/append-custom-params.ts create mode 100644 packages/arcgis-rest-common/test/append-custom-params.test.ts diff --git a/packages/arcgis-rest-common/src/util/append-custom-params.ts b/packages/arcgis-rest-common/src/util/append-custom-params.ts new file mode 100644 index 0000000000..0f83bebdc6 --- /dev/null +++ b/packages/arcgis-rest-common/src/util/append-custom-params.ts @@ -0,0 +1,47 @@ +/* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc. + * Apache-2.0 */ + +import { IRequestOptions } from "@esri/arcgis-rest-request"; + +/** + * Helper for methods with lots of first order request options to pass through as request parameters. + */ +export function appendCustomParams( + customOptions: T, + keys: Array, + baseOptions?: Partial +): IRequestOptions { + const requestOptionsKeys = [ + "params", + "httpMethod", + "rawResponse", + "authentication", + "portal", + "fetch", + "maxUrlLength", + "headers" + ]; + + const options: T = { + ...{ params: {} }, + ...baseOptions, + ...customOptions + }; + + // merge all keys in customOptions into options.params + options.params = keys.reduce((value, key) => { + value[key as any] = customOptions[key]; + return value; + }, options.params); + + // now remove all properties in options that don't exist in IRequestOptions + return requestOptionsKeys.reduce( + (value, key) => { + if (options[key]) { + value[key] = options[key]; + } + return value; + }, + {} as IRequestOptions + ); +} diff --git a/packages/arcgis-rest-common/test/append-custom-params.test.ts b/packages/arcgis-rest-common/test/append-custom-params.test.ts new file mode 100644 index 0000000000..76529ce98f --- /dev/null +++ b/packages/arcgis-rest-common/test/append-custom-params.test.ts @@ -0,0 +1,24 @@ +/* Copyright (c) 2018 Environmental Systems Research Institute, Inc. + * Apache-2.0 */ + +import { appendCustomParams } from "../src/util/append-custom-params"; +import { IRequestOptions } from "@esri/arcgis-rest-request/src"; + +describe("appendCustomParams() helper", () => { + it("should assign custom params from a sub class of IRequestOptions", () => { + interface ICustomOptions extends IRequestOptions { + foo: string; + } + + expect( + appendCustomParams({ foo: "bar" }, ["foo"], { + httpMethod: "GET" + }) + ).toEqual({ + httpMethod: "GET", + params: { + foo: "bar" + } + }); + }); +}); From eb297cbdbca0acbf3a857afd3d6b3b4f03da27a0 Mon Sep 17 00:00:00 2001 From: Patrick Arlt Date: Mon, 15 Apr 2019 17:01:49 -0700 Subject: [PATCH 07/11] remove params builder support --- packages/arcgis-rest-items/src/search.ts | 3 +- packages/arcgis-rest-request/src/request.ts | 4 +- .../test/mocks/params-builder-nested.ts | 8 ---- .../test/mocks/params-builder.ts | 7 --- .../arcgis-rest-request/test/request.test.ts | 46 ------------------- 5 files changed, 2 insertions(+), 66 deletions(-) delete mode 100644 packages/arcgis-rest-request/test/mocks/params-builder-nested.ts delete mode 100644 packages/arcgis-rest-request/test/mocks/params-builder.ts diff --git a/packages/arcgis-rest-items/src/search.ts b/packages/arcgis-rest-items/src/search.ts index c4a3f85939..7e0293de2d 100644 --- a/packages/arcgis-rest-items/src/search.ts +++ b/packages/arcgis-rest-items/src/search.ts @@ -7,7 +7,7 @@ import { getPortalUrl } from "@esri/arcgis-rest-request"; import { appendCustomParams } from "@esri/arcgis-rest-common"; -import { IPagingParams, IItem } from "@esri/arcgis-rest-common-types"; +import { IPagingParams, IItem, ISearch } from "@esri/arcgis-rest-common-types"; import { SearchQueryBuilder } from "./SearchQueryBuilder"; export interface ISearchRequestOptions extends IRequestOptions, IPagingParams { @@ -46,7 +46,6 @@ export function searchItems( search: string | ISearchRequestOptions | SearchQueryBuilder ): Promise { let options: IRequestOptions; - if (typeof search === "string" || search instanceof SearchQueryBuilder) { options = { httpMethod: "GET", diff --git a/packages/arcgis-rest-request/src/request.ts b/packages/arcgis-rest-request/src/request.ts index 997da798dd..f9f5a63549 100644 --- a/packages/arcgis-rest-request/src/request.ts +++ b/packages/arcgis-rest-request/src/request.ts @@ -148,9 +148,7 @@ export function request( const params: IParams = { ...{ f: "json" }, - ...(requestOptions.params && requestOptions.params.toParams - ? requestOptions.params.toParams() - : requestOptions.params) + ...requestOptions.params }; const fetchOptions: RequestInit = { diff --git a/packages/arcgis-rest-request/test/mocks/params-builder-nested.ts b/packages/arcgis-rest-request/test/mocks/params-builder-nested.ts deleted file mode 100644 index 49df3d7296..0000000000 --- a/packages/arcgis-rest-request/test/mocks/params-builder-nested.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { IParamsBuilder, IParams } from "../../src/index"; -import { MockParamBuilder } from "./param-builder"; - -export class MockNestedBuilder implements IParamsBuilder { - public toParams(): IParams { - return { where: new MockParamBuilder(), f: "geojson" }; - } -} diff --git a/packages/arcgis-rest-request/test/mocks/params-builder.ts b/packages/arcgis-rest-request/test/mocks/params-builder.ts deleted file mode 100644 index 3cef72e384..0000000000 --- a/packages/arcgis-rest-request/test/mocks/params-builder.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { IParamsBuilder, ResponseFormats } from "../../src/index"; - -export class MockParamsBuilder implements IParamsBuilder { - public toParams() { - return { where: "1=1", f: "geojson" as ResponseFormats }; - } -} diff --git a/packages/arcgis-rest-request/test/request.test.ts b/packages/arcgis-rest-request/test/request.test.ts index 548740788a..fdc722bf3d 100644 --- a/packages/arcgis-rest-request/test/request.test.ts +++ b/packages/arcgis-rest-request/test/request.test.ts @@ -287,52 +287,6 @@ describe("request()", () => { fail(e); }); }); - - it("should encode a param that impliments IParamsBuilder", done => { - fetchMock.once("*", GeoJSONFeatureCollection); - - const builder = new MockParamsBuilder(); - - request( - "https://services1.arcgis.com/ORG/arcgis/rest/services/FEATURE_SERVICE/FeatureServer/0/query", - { - params: builder - } - ) - .then(response => { - const options: RequestInit = fetchMock.lastCall("*")[1]; - expect(options.body).toContain(`where=${encodeURIComponent("1=1")}`); - expect(options.body).toContain(`f=geojson`); - expect(response).toEqual(GeoJSONFeatureCollection); - done(); - }) - .catch(e => { - fail(e); - }); - }); - - it("should chain IParamBuilder and IParamsBuilder", done => { - fetchMock.once("*", GeoJSONFeatureCollection); - - const builder = new MockNestedBuilder(); - - request( - "https://services1.arcgis.com/ORG/arcgis/rest/services/FEATURE_SERVICE/FeatureServer/0/query", - { - params: builder - } - ) - .then(response => { - const options: RequestInit = fetchMock.lastCall("*")[1]; - expect(options.body).toContain(`where=${encodeURIComponent("1=1")}`); - expect(options.body).toContain(`f=geojson`); - expect(response).toEqual(GeoJSONFeatureCollection); - done(); - }) - .catch(e => { - fail(e); - }); - }); }); describe("should throw errors when required dependencies are missing", () => { From 99300a8c1a73fa9c9a77a592b672e3cc53b4ff65 Mon Sep 17 00:00:00 2001 From: john gravois Date: Mon, 15 Apr 2019 21:35:35 -0700 Subject: [PATCH 08/11] use new appendCustomParams consistently --- packages/arcgis-rest-common/package.json | 8 ++ .../src/util/append-custom-params.ts | 4 +- .../arcgis-rest-feature-service/package.json | 6 +- .../arcgis-rest-feature-service/src/add.ts | 23 ++---- .../arcgis-rest-feature-service/src/delete.ts | 28 +++---- .../src/helpers.ts | 5 -- .../arcgis-rest-feature-service/src/query.ts | 71 ++++++++++------ .../src/queryRelated.ts | 47 +++++------ .../arcgis-rest-feature-service/src/update.ts | 22 ++--- .../test/crud.test.ts | 13 +-- packages/arcgis-rest-geocoder/package.json | 2 + packages/arcgis-rest-geocoder/src/geocode.ts | 80 +++++++++---------- packages/arcgis-rest-items/package.json | 1 + packages/arcgis-rest-items/src/add.ts | 15 ++-- packages/arcgis-rest-items/src/remove.ts | 17 ++-- packages/arcgis-rest-request/src/index.ts | 1 - .../src/utils/IParamsBuilder.ts | 2 +- .../src/utils/append-custom-params.ts | 32 -------- .../arcgis-rest-request/test/request.test.ts | 4 - .../test/utils/append-params.test.ts | 32 -------- 20 files changed, 172 insertions(+), 241 deletions(-) delete mode 100644 packages/arcgis-rest-request/src/utils/append-custom-params.ts delete mode 100644 packages/arcgis-rest-request/test/utils/append-params.test.ts diff --git a/packages/arcgis-rest-common/package.json b/packages/arcgis-rest-common/package.json index f88780d789..5e6e8cd5b0 100644 --- a/packages/arcgis-rest-common/package.json +++ b/packages/arcgis-rest-common/package.json @@ -13,6 +13,14 @@ "dist/**" ], "dependencies": { + "tslib": "^1.9.3" + }, + + "devDependencies": { + "tslib": "^1.9.3", + "@esri/arcgis-rest-request": "^1.19.2" + }, + "peerDependencies": { "tslib": "^1.9.3", "@esri/arcgis-rest-request": "^1.19.2" }, diff --git a/packages/arcgis-rest-common/src/util/append-custom-params.ts b/packages/arcgis-rest-common/src/util/append-custom-params.ts index 0f83bebdc6..16720961d8 100644 --- a/packages/arcgis-rest-common/src/util/append-custom-params.ts +++ b/packages/arcgis-rest-common/src/util/append-custom-params.ts @@ -30,7 +30,9 @@ export function appendCustomParams( // merge all keys in customOptions into options.params options.params = keys.reduce((value, key) => { - value[key as any] = customOptions[key]; + if (customOptions[key] || typeof customOptions[key] === "boolean") { + value[key as any] = customOptions[key]; + } return value; }, options.params); diff --git a/packages/arcgis-rest-feature-service/package.json b/packages/arcgis-rest-feature-service/package.json index 9bd9da6f3d..fc2d941384 100644 --- a/packages/arcgis-rest-feature-service/package.json +++ b/packages/arcgis-rest-feature-service/package.json @@ -17,11 +17,13 @@ }, "devDependencies": { "@esri/arcgis-rest-common-types": "^1.19.2", - "@esri/arcgis-rest-request": "^1.19.2" + "@esri/arcgis-rest-request": "^1.19.2", + "@esri/arcgis-rest-common": "^1.19.2" }, "peerDependencies": { "@esri/arcgis-rest-common-types": "^1.19.2", - "@esri/arcgis-rest-request": "^1.19.2" + "@esri/arcgis-rest-request": "^1.19.2", + "@esri/arcgis-rest-common": "^1.19.2" }, "scripts": { "prepare": "npm run build", diff --git a/packages/arcgis-rest-feature-service/src/add.ts b/packages/arcgis-rest-feature-service/src/add.ts index cf875fbb88..296a7a9390 100644 --- a/packages/arcgis-rest-feature-service/src/add.ts +++ b/packages/arcgis-rest-feature-service/src/add.ts @@ -5,10 +5,11 @@ import { IFeature } from "@esri/arcgis-rest-common-types"; import { request, IRequestOptions, - appendCustomParams, cleanUrl, warn } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; + import { IEditFeaturesParams, IEditFeatureResult } from "./helpers"; /** @@ -69,21 +70,11 @@ export function addFeatures( const url = `${cleanUrl(requestOptions.url)}/addFeatures`; // edit operations are POST only - const options: IAddFeaturesRequestOptions = { - params: {}, - ...requestOptions - }; - - appendCustomParams(requestOptions, options); - - if (options.params.adds && options.params.adds.length) { - // mixin, don't overwrite - options.params.features = requestOptions.adds; - delete options.params.adds; - warn( - "The `adds` parameter is deprecated and will be removed in a future release. Please use `features` instead." - ); - } + const options = appendCustomParams( + requestOptions, + ["features", "gdbVersion", "returnEditMoment", "rollbackOnFailure"], + { params: { ...requestOptions.params } } + ); return request(url, options); } diff --git a/packages/arcgis-rest-feature-service/src/delete.ts b/packages/arcgis-rest-feature-service/src/delete.ts index f9cc71fc41..91aa918d9b 100644 --- a/packages/arcgis-rest-feature-service/src/delete.ts +++ b/packages/arcgis-rest-feature-service/src/delete.ts @@ -4,10 +4,10 @@ import { request, IRequestOptions, - appendCustomParams, cleanUrl, warn } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { IEditFeaturesParams, IEditFeatureResult, @@ -75,21 +75,17 @@ export function deleteFeatures( const url = `${cleanUrl(requestOptions.url)}/deleteFeatures`; // edit operations POST only - const options: IDeleteFeaturesRequestOptions = { - params: {}, - ...requestOptions - }; - - appendCustomParams(requestOptions, options); - - if (options.params.deletes && options.params.deletes.length) { - // mixin, don't overwrite - options.params.objectIds = requestOptions.deletes; - delete options.params.deletes; - warn( - "The `deletes` parameter is deprecated and will be removed in a future release. Please use `objectIds` instead." - ); - } + const options = appendCustomParams( + requestOptions, + [ + "where", + "objectIds", + "gdbVersion", + "returnEditMoment", + "rollbackOnFailure" + ], + { params: { ...requestOptions.params } } + ); return request(url, options); } diff --git a/packages/arcgis-rest-feature-service/src/helpers.ts b/packages/arcgis-rest-feature-service/src/helpers.ts index 62886da51c..4c386f56a8 100644 --- a/packages/arcgis-rest-feature-service/src/helpers.ts +++ b/packages/arcgis-rest-feature-service/src/helpers.ts @@ -1,8 +1,6 @@ /* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { appendCustomParams } from "@esri/arcgis-rest-request"; - import { esriGeometryType, SpatialRelationship, @@ -45,6 +43,3 @@ export interface IEditFeaturesParams { */ rollbackOnFailure?: boolean; } - -// this function has been moved into @esri/request. it is re-exported here for backwards compatibility -export { appendCustomParams }; diff --git a/packages/arcgis-rest-feature-service/src/query.ts b/packages/arcgis-rest-feature-service/src/query.ts index 5876af4d2b..1687ec9cc8 100644 --- a/packages/arcgis-rest-feature-service/src/query.ts +++ b/packages/arcgis-rest-feature-service/src/query.ts @@ -1,12 +1,8 @@ /* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { - request, - IRequestOptions, - appendCustomParams, - cleanUrl -} from "@esri/arcgis-rest-request"; +import { request, IRequestOptions, cleanUrl } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { ISpatialReference, IFeatureSet, @@ -163,22 +159,51 @@ export function getFeature( export function queryFeatures( requestOptions: IQueryFeaturesRequestOptions ): Promise { - const queryOptions: IQueryFeaturesRequestOptions = { - params: {}, - httpMethod: "GET", - url: requestOptions.url, - ...requestOptions - }; - - appendCustomParams(requestOptions, queryOptions); - - // set default query parameters - if (!queryOptions.params.where) { - queryOptions.params.where = "1=1"; - } - if (!queryOptions.params.outFields) { - queryOptions.params.outFields = "*"; - } + const queryOptions = appendCustomParams( + requestOptions, + [ + "where", + "objectIds", + "relationParam", + "time", + "distance", + "units", + "outFields", + "returnGeometry", + "maxAllowableOffset", + "geometryPrecision", + "outSR", + "gdbVersion", + "returnDistinctValues", + "returnIdsOnly", + "returnCountOnly", + "returnExtentOnly", + "orderByFields", + "groupByFieldsForStatistics", + "outStatistics", + "returnZ", + "returnM", + "multipatchOption", + "resultOffset", + "resultRecordCount", + "quantizationParameters", + "returnCentroid", + "resultType", + "historicMoment", + "returnTrueCurves", + "sqlFormat", + "returnExceededLimitFeatures" + ], + { + httpMethod: "GET", + params: { + // set default query parameters + where: "1=1", + outFields: "*", + ...requestOptions.params + } + } + ); - return request(`${cleanUrl(queryOptions.url)}/query`, queryOptions); + return request(`${cleanUrl(requestOptions.url)}/query`, queryOptions); } diff --git a/packages/arcgis-rest-feature-service/src/queryRelated.ts b/packages/arcgis-rest-feature-service/src/queryRelated.ts index a14b43edc6..a61bae3020 100644 --- a/packages/arcgis-rest-feature-service/src/queryRelated.ts +++ b/packages/arcgis-rest-feature-service/src/queryRelated.ts @@ -1,12 +1,8 @@ /* Copyright (c) 2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { - request, - IRequestOptions, - appendCustomParams, - cleanUrl -} from "@esri/arcgis-rest-request"; +import { request, IRequestOptions, cleanUrl } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { ISpatialReference, IFeature, @@ -66,26 +62,23 @@ export interface IQueryRelatedResponse extends IHasZM { export function queryRelated( requestOptions: IQueryRelatedRequestOptions ): Promise { - const options: IQueryRelatedRequestOptions = { - params: {}, - httpMethod: "GET", - url: requestOptions.url, - ...requestOptions - }; - - appendCustomParams(requestOptions, options); - - if (!options.params.definitionExpression) { - options.params.definitionExpression = "1=1"; - } - - if (!options.params.outFields) { - options.params.outFields = "*"; - } - - if (!options.params.relationshipId) { - options.params.relationshipId = 0; - } + const options = appendCustomParams( + requestOptions, + ["objectIds", "relationshipId", "definitionExpression", "outFields"], + { + httpMethod: "GET", + params: { + // set default query parameters + definitionExpression: "1=1", + outFields: "*", + relationshipId: 0, + ...requestOptions.params + } + } + ); - return request(`${cleanUrl(options.url)}/queryRelatedRecords`, options); + return request( + `${cleanUrl(requestOptions.url)}/queryRelatedRecords`, + options + ); } diff --git a/packages/arcgis-rest-feature-service/src/update.ts b/packages/arcgis-rest-feature-service/src/update.ts index b755f06575..a0d22d2b0c 100644 --- a/packages/arcgis-rest-feature-service/src/update.ts +++ b/packages/arcgis-rest-feature-service/src/update.ts @@ -5,10 +5,10 @@ import { IFeature } from "@esri/arcgis-rest-common-types"; import { request, IRequestOptions, - appendCustomParams, cleanUrl, warn } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { IEditFeaturesParams, IEditFeatureResult } from "./helpers"; /** @@ -69,21 +69,11 @@ export function updateFeatures( const url = `${cleanUrl(requestOptions.url)}/updateFeatures`; // edit operations are POST only - const options: IUpdateFeaturesRequestOptions = { - params: {}, - ...requestOptions - }; - - appendCustomParams(requestOptions, options); - - if (options.params.updates && options.params.updates.length) { - // mixin, don't overwrite - options.params.features = requestOptions.updates; - delete options.params.updates; - warn( - "The `updates` parameter is deprecated and will be removed in a future release. Please use `features` instead." - ); - } + const options = appendCustomParams( + requestOptions, + ["features", "gdbVersion", "returnEditMoment", "rollbackOnFailure"], + { params: { ...requestOptions.params } } + ); return request(url, options); } diff --git a/packages/arcgis-rest-feature-service/test/crud.test.ts b/packages/arcgis-rest-feature-service/test/crud.test.ts index 9f7a1eb7e7..e9e0c11ce4 100644 --- a/packages/arcgis-rest-feature-service/test/crud.test.ts +++ b/packages/arcgis-rest-feature-service/test/crud.test.ts @@ -201,15 +201,6 @@ describe("feature", () => { Native: "YES" } } - ], // for linting - updates: [ - { - attributes: { - OBJECTID: 1001, - Street: "NO", - Native: "YES" - } - } ], rollbackOnFailure: false } as IUpdateFeaturesRequestOptions; @@ -238,7 +229,7 @@ describe("feature", () => { it("should return objectId of the deleted feature and a truthy success", done => { const requestOptions = { url: serviceUrl, - deletes: [1001], + objectIds: [1001], where: "1=1" } as IDeleteFeaturesRequestOptions; fetchMock.once("*", deleteFeaturesResponse); @@ -251,7 +242,7 @@ describe("feature", () => { expect(options.body).toContain("where=1%3D1"); expect(options.method).toBe("POST"); expect(response.deleteResults[0].objectId).toEqual( - requestOptions.deletes[0] + requestOptions.objectIds[0] ); expect(response.deleteResults[0].success).toEqual(true); done(); diff --git a/packages/arcgis-rest-geocoder/package.json b/packages/arcgis-rest-geocoder/package.json index 8595997758..1948f870d0 100644 --- a/packages/arcgis-rest-geocoder/package.json +++ b/packages/arcgis-rest-geocoder/package.json @@ -18,11 +18,13 @@ "devDependencies": { "@esri/arcgis-rest-auth": "^1.19.2", "@esri/arcgis-rest-common-types": "^1.19.2", + "@esri/arcgis-rest-common": "^1.19.2", "@esri/arcgis-rest-request": "^1.19.2" }, "peerDependencies": { "@esri/arcgis-rest-auth": "^1.19.2", "@esri/arcgis-rest-common-types": "^1.19.2", + "@esri/arcgis-rest-common": "^1.19.2", "@esri/arcgis-rest-request": "^1.19.2" }, "scripts": { diff --git a/packages/arcgis-rest-geocoder/src/geocode.ts b/packages/arcgis-rest-geocoder/src/geocode.ts index 993d50fab7..462f48812f 100644 --- a/packages/arcgis-rest-geocoder/src/geocode.ts +++ b/packages/arcgis-rest-geocoder/src/geocode.ts @@ -1,18 +1,13 @@ /* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { - request, - appendCustomParams, - cleanUrl, - IParams -} from "@esri/arcgis-rest-request"; - +import { request, cleanUrl, IParams } from "@esri/arcgis-rest-request"; import { IExtent, ISpatialReference, IPoint } from "@esri/arcgis-rest-common-types"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { worldGeocoder, IEndpointRequestOptions } from "./helpers"; @@ -104,45 +99,50 @@ export interface IGeocodeResponse { export function geocode( address: string | IGeocodeRequestOptions ): Promise { - let options: IGeocodeRequestOptions = { - endpoint: worldGeocoder, - params: {} - }; + let options: IGeocodeRequestOptions = {}; + let endpoint: string; if (typeof address === "string") { - options.params.singleLine = address; + options.params = { singleLine: address }; + endpoint = worldGeocoder; } else { - options.endpoint = address.endpoint || worldGeocoder; - options = { - ...options, - ...address - }; - - appendCustomParams(address, options); + endpoint = address.endpoint || worldGeocoder; + options = appendCustomParams( + address, + [ + "singleLine", + "address", + "address2", + "address3", + "neighborhood", + "city", + "subregion", + "region", + "postal", + "postalExt", + "countryCode" + ], + { params: { ...address.params } } + ); } // add spatialReference property to individual matches - return request( - `${cleanUrl(options.endpoint)}/findAddressCandidates`, - options - ).then(response => { - if (options.rawResponse) { + return request(`${cleanUrl(endpoint)}/findAddressCandidates`, options).then( + response => { + if (typeof address !== "string" && address.rawResponse) { + return response; + } + const sr = response.spatialReference; + response.candidates.forEach(function(candidate: { + location: IPoint; + extent?: IExtent; + }) { + candidate.location.spatialReference = sr; + if (candidate.extent) { + candidate.extent.spatialReference = sr; + } + }); return response; } - const sr = response.spatialReference; - response.candidates.forEach(function(candidate: { - location: IPoint; - extent?: IExtent; - }) { - candidate.location.spatialReference = sr; - if (candidate.extent) { - candidate.extent.spatialReference = sr; - } - }); - return response; - }); + ); } - -export default { - geocode -}; diff --git a/packages/arcgis-rest-items/package.json b/packages/arcgis-rest-items/package.json index d0d2097315..881042f791 100644 --- a/packages/arcgis-rest-items/package.json +++ b/packages/arcgis-rest-items/package.json @@ -18,6 +18,7 @@ "devDependencies": { "@esri/arcgis-rest-auth": "^1.19.2", "@esri/arcgis-rest-common": "^1.19.2", + "@esri/arcgis-rest-common-types": "^1.19.2", "@esri/arcgis-rest-request": "^1.19.2" }, "peerDependencies": { diff --git a/packages/arcgis-rest-items/src/add.ts b/packages/arcgis-rest-items/src/add.ts index 98cbc97bf1..b0f8d0ec6f 100644 --- a/packages/arcgis-rest-items/src/add.ts +++ b/packages/arcgis-rest-items/src/add.ts @@ -1,12 +1,8 @@ /* Copyright (c) 2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { - request, - getPortalUrl, - appendCustomParams -} from "@esri/arcgis-rest-request"; - +import { request, getPortalUrl } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { IItemIdRequestOptions, IItemResourceRequestOptions, @@ -127,8 +123,11 @@ export function addItemRelationship( requestOptions )}/content/users/${owner}/addRelationship`; - const options = { params: {}, ...requestOptions }; - appendCustomParams(requestOptions, options); + const options = appendCustomParams( + requestOptions, + ["originItemId", "destinationItemId", "relationshipType"], + {} + ); return request(url, options); } diff --git a/packages/arcgis-rest-items/src/remove.ts b/packages/arcgis-rest-items/src/remove.ts index ae69e0b111..5c510ff0da 100644 --- a/packages/arcgis-rest-items/src/remove.ts +++ b/packages/arcgis-rest-items/src/remove.ts @@ -1,8 +1,8 @@ /* Copyright (c) 2018 Environmental Systems Research Institute, Inc. * Apache-2.0 */ -import { request, getPortalUrl, appendCustomParams } from "@esri/arcgis-rest-request"; - +import { request, getPortalUrl } from "@esri/arcgis-rest-request"; +import { appendCustomParams } from "@esri/arcgis-rest-common"; import { IItemIdRequestOptions, IItemResourceRequestOptions, @@ -54,12 +54,17 @@ export function removeItem( */ export function removeItemRelationship( requestOptions: IManageItemRelationshipRequestOptions -): Promise<{ "success": boolean }> { +): Promise<{ success: boolean }> { const owner = determineOwner(requestOptions); - const url = `${getPortalUrl(requestOptions)}/content/users/${owner}/removeRelationship`; + const url = `${getPortalUrl( + requestOptions + )}/content/users/${owner}/removeRelationship`; - const options = { params: {}, ...requestOptions } - appendCustomParams(requestOptions, options); + const options = appendCustomParams( + requestOptions, + ["originItemId", "destinationItemId", "relationshipType"], + {} + ); return request(url, options); } diff --git a/packages/arcgis-rest-request/src/index.ts b/packages/arcgis-rest-request/src/index.ts index c0a42cff05..716f46baba 100644 --- a/packages/arcgis-rest-request/src/index.ts +++ b/packages/arcgis-rest-request/src/index.ts @@ -14,5 +14,4 @@ export * from "./utils/IParamsBuilder"; export * from "./utils/process-params"; export * from "./utils/get-portal"; export * from "./utils/get-portal-url"; -export * from "./utils/append-custom-params"; export * from "./utils/clean-url"; diff --git a/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts b/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts index edc1eb60c9..97a6ff8f28 100644 --- a/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts +++ b/packages/arcgis-rest-request/src/utils/IParamsBuilder.ts @@ -1,5 +1,5 @@ import { IParams } from "./params"; export interface IParamsBuilder { - toParams(additionalParmas?: IParams): IParams; + toParams(additionalParams?: IParams): IParams; } diff --git a/packages/arcgis-rest-request/src/utils/append-custom-params.ts b/packages/arcgis-rest-request/src/utils/append-custom-params.ts deleted file mode 100644 index 9d02be0c51..0000000000 --- a/packages/arcgis-rest-request/src/utils/append-custom-params.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc. - * Apache-2.0 */ - -import { IRequestOptions } from "../request"; - -/** - * Helper for methods with lots of first order request options to pass through as request parameters. - */ -export function appendCustomParams( - oldOptions: IRequestOptions, - newOptions: IRequestOptions -) { - // at v2.0.0, this should be refactored as a nonmutating method that takes a single argument, mixes in everything and returns a new instance of IRequestOptions - - // only pass query parameters through in the request, not generic IRequestOptions props - Object.keys(oldOptions).forEach(function(key: string) { - if ( - key !== "url" && - key !== "params" && - key !== "authentication" && - key !== "httpMethod" && - key !== "fetch" && - key !== "portal" && - key !== "maxUrlLength" && - key !== "headers" && - key !== "endpoint" && - key !== "decodeValues" - ) { - newOptions.params[key] = (oldOptions as { [key: string]: any })[key]; - } - }); -} diff --git a/packages/arcgis-rest-request/test/request.test.ts b/packages/arcgis-rest-request/test/request.test.ts index fdc722bf3d..ca5db28eae 100644 --- a/packages/arcgis-rest-request/test/request.test.ts +++ b/packages/arcgis-rest-request/test/request.test.ts @@ -8,13 +8,9 @@ import { SharingRestInfoHTML } from "./mocks/sharing-rest-info"; import { MockParamBuilder } from "./mocks/param-builder"; -import { MockParamsBuilder } from "./mocks/params-builder"; -import { MockNestedBuilder } from "./mocks/params-builder-nested"; - import { ArcGISOnlineError } from "./mocks/errors"; import { WebMapAsText, WebMapAsJSON } from "./mocks/webmap"; import { GeoJSONFeatureCollection } from "./mocks/geojson-feature-collection"; -import { IParams } from "../dist/esm/utils/IParams"; describe("request()", () => { afterEach(fetchMock.restore); diff --git a/packages/arcgis-rest-request/test/utils/append-params.test.ts b/packages/arcgis-rest-request/test/utils/append-params.test.ts deleted file mode 100644 index 19bd6455b0..0000000000 --- a/packages/arcgis-rest-request/test/utils/append-params.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -/* Copyright (c) 2018-2019 Environmental Systems Research Institute, Inc. - * Apache-2.0 */ - -import { appendCustomParams, IRequestOptions } from "../../src/index"; - -describe("appendCustomParams", () => { - it("should not mis-identify standard request options as parameters.", () => { - const oldOptions: any = { - foo: "bar", - headers: { - Cookie: "monster" - }, - httpMethod: "GET", - params: { baz: "luhrman" }, - maxUrlLength: 1064 - }; - - const newOptions: IRequestOptions = { - params: {} - }; - - appendCustomParams(oldOptions, newOptions); - - // all other request options should be mixed in outside this helper method - expect(typeof newOptions.headers).toEqual("undefined"); - expect(typeof newOptions.httpMethod).toEqual("undefined"); - expect(typeof newOptions.maxUrlLength).toEqual("undefined"); - expect(typeof newOptions.params.baz).toEqual("undefined"); - - expect(newOptions.params.foo).toEqual("bar"); - }); -}); From 82fddd5f190b439f99f21bb2d2c26a680df4efd7 Mon Sep 17 00:00:00 2001 From: john gravois Date: Mon, 15 Apr 2019 21:45:22 -0700 Subject: [PATCH 09/11] more tests --- .../test/append-custom-params.test.ts | 37 +++++++++++++++++++ packages/arcgis-rest-items/src/add.ts | 2 +- packages/arcgis-rest-items/src/remove.ts | 2 +- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/arcgis-rest-common/test/append-custom-params.test.ts b/packages/arcgis-rest-common/test/append-custom-params.test.ts index 76529ce98f..34d6d7f1be 100644 --- a/packages/arcgis-rest-common/test/append-custom-params.test.ts +++ b/packages/arcgis-rest-common/test/append-custom-params.test.ts @@ -21,4 +21,41 @@ describe("appendCustomParams() helper", () => { } }); }); + + it("should assign custom params even if theyre falsey", () => { + interface IFalseyCustomOptions extends IRequestOptions { + foo: boolean; + } + + expect( + appendCustomParams({ foo: false }, ["foo"], { + httpMethod: "GET" + }) + ).toEqual({ + httpMethod: "GET", + params: { + foo: false + } + }); + }); + + it("should pass through manual params if nothing is present to overwrite them", () => { + interface IEmptyCustomOptions extends IRequestOptions { + foo?: boolean; + } + + expect( + appendCustomParams({}, ["foo"], { + httpMethod: "GET", + params: { + foo: "bar" + } + }) + ).toEqual({ + httpMethod: "GET", + params: { + foo: "bar" + } + }); + }); }); diff --git a/packages/arcgis-rest-items/src/add.ts b/packages/arcgis-rest-items/src/add.ts index b0f8d0ec6f..de2c0d53eb 100644 --- a/packages/arcgis-rest-items/src/add.ts +++ b/packages/arcgis-rest-items/src/add.ts @@ -126,7 +126,7 @@ export function addItemRelationship( const options = appendCustomParams( requestOptions, ["originItemId", "destinationItemId", "relationshipType"], - {} + { params: { ...requestOptions.params } } ); return request(url, options); diff --git a/packages/arcgis-rest-items/src/remove.ts b/packages/arcgis-rest-items/src/remove.ts index 5c510ff0da..887a0abede 100644 --- a/packages/arcgis-rest-items/src/remove.ts +++ b/packages/arcgis-rest-items/src/remove.ts @@ -63,7 +63,7 @@ export function removeItemRelationship( const options = appendCustomParams( requestOptions, ["originItemId", "destinationItemId", "relationshipType"], - {} + { params: { ...requestOptions.params } } ); return request(url, options); From 63a326e527342c4b12b18e6c957191d74b2e635c Mon Sep 17 00:00:00 2001 From: john gravois Date: Tue, 16 Apr 2019 08:41:58 -0700 Subject: [PATCH 10/11] tslib isnt a peerDep --- packages/arcgis-rest-common/package.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/arcgis-rest-common/package.json b/packages/arcgis-rest-common/package.json index 5e6e8cd5b0..2f8559d050 100644 --- a/packages/arcgis-rest-common/package.json +++ b/packages/arcgis-rest-common/package.json @@ -15,13 +15,10 @@ "dependencies": { "tslib": "^1.9.3" }, - "devDependencies": { - "tslib": "^1.9.3", "@esri/arcgis-rest-request": "^1.19.2" }, "peerDependencies": { - "tslib": "^1.9.3", "@esri/arcgis-rest-request": "^1.19.2" }, "scripts": { From 3a56a2312b0382d72496f6cc8a859bfc9789f993 Mon Sep 17 00:00:00 2001 From: john gravois Date: Tue, 16 Apr 2019 09:20:40 -0700 Subject: [PATCH 11/11] readd auth as peerDep of feature-service --- packages/arcgis-rest-feature-service/package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/arcgis-rest-feature-service/package.json b/packages/arcgis-rest-feature-service/package.json index d66a2b57d7..b76710229b 100644 --- a/packages/arcgis-rest-feature-service/package.json +++ b/packages/arcgis-rest-feature-service/package.json @@ -17,10 +17,12 @@ }, "devDependencies": { "@esri/arcgis-rest-request": "^1.19.2", + "@esri/arcgis-rest-auth": "^1.19.2", "@esri/arcgis-rest-common": "^1.19.2" }, "peerDependencies": { "@esri/arcgis-rest-request": "^1.19.2", + "@esri/arcgis-rest-auth": "^1.19.2", "@esri/arcgis-rest-common": "^1.19.2" }, "scripts": {