Skip to content

Commit

Permalink
Merge pull request #626 from Esri/b/hubApiUrl-optional
Browse files Browse the repository at this point in the history
fix(hub-common): hupApiUrl, isPortal, and authentication options shou…
  • Loading branch information
tomwayson authored Sep 17, 2021
2 parents 45888ad + 9f840a7 commit 7aa7ade
Show file tree
Hide file tree
Showing 20 changed files with 183 additions and 150 deletions.
6 changes: 6 additions & 0 deletions packages/common/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,11 @@ import { _getHubUrlFromPortalHostname } from "./urls/_get-hub-url-from-portal-ho
export function getHubApiUrl(
urlOrObject: IPortal | IHubRequestOptions | IRequestOptions | string
): string {
const hubApiUrl =
urlOrObject && (urlOrObject as IHubRequestOptions).hubApiUrl;
if (hubApiUrl) {
// this is request options w/ hubApiUrl already defined
return hubApiUrl;
}
return _getHubUrlFromPortalHostname(getPortalApiUrl(urlOrObject));
}
15 changes: 8 additions & 7 deletions packages/common/src/request.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getHubApiUrl } from ".";
import { IHubRequestOptions } from "./types";
import { buildUrl } from "./urls/build-url";

Expand Down Expand Up @@ -35,16 +36,16 @@ export function hubApiRequest(
) {
// merge in default request options
const options: IHubRequestOptions = {
hubApiUrl: "https://opendata.arcgis.com/api/v3/",
// why do we default to GET w/ our API?
httpMethod: "GET",
...requestOptions
...requestOptions,
};
// use fetch override if any
const _fetch = options.fetch || fetch;
// merge in default headers
const headers = {
"Content-Type": "application/json",
...options.headers
...options.headers,
};
// build query params/body based on requestOptions.params
let query;
Expand All @@ -58,15 +59,15 @@ export function hubApiRequest(
}
// build Hub API URL
const url = buildUrl({
host: options.hubApiUrl,
host: getHubApiUrl(options),
path: `/api/v3/${route}`.replace(/\/\//g, "/"),
query
query,
});
return _fetch(url, {
method: options.httpMethod,
headers,
body
}).then(resp => {
body,
}).then((resp) => {
if (resp.ok) {
return resp.json();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* @param {string} hubApiUrl
* @private
*/
export function _getDomainServiceUrl(hubApiUrl: string) {
export function _getDomainServiceUrl(hubApiUrl?: string) {
const base = hubApiUrl || "https://hub.arcgis.com";
return `${base}/api/v3/domains`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getUniqueDomainName } from "./get-unique-domain-name";
import { _ensureSafeDomainLength } from "./_ensure-safe-domain-length";
import { IHubRequestOptions } from "../../types";
import { stripProtocol } from "../../urls";
import { getHubApiUrl } from "../..";

/**
* Given a subdomain, ensure that we have a unique hostname
Expand All @@ -19,7 +20,7 @@ export function ensureUniqueDomainName(
prms = getUniqueDomainNamePortal(subdomain, hubRequestOptions);
} else {
const baseDomain = `${hubRequestOptions.portalSelf.urlKey}.${stripProtocol(
hubRequestOptions.hubApiUrl
getHubApiUrl(hubRequestOptions)
)}`;
prms = getUniqueDomainName(subdomain, baseDomain, hubRequestOptions);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/common/src/sites/domains/is-valid-domain.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { _getAuthHeader } from "./_get-auth-header";
import { IHubRequestOptions } from "../../types";
import { _getDomainServiceUrl } from ".";

/**
* Validate a custom domain
Expand All @@ -13,7 +14,9 @@ export function isValidDomain(
if (hubRequestOptions.isPortal) {
throw new Error(`isValidDomain is not available in ArcGIS Enterprise.`);
}
const url = `${hubRequestOptions.hubApiUrl}/api/v3/domains/validate?hostname=${hostname}`;
const url = `${_getDomainServiceUrl(
hubRequestOptions.hubApiUrl
)}/validate?hostname=${hostname}`;
const headers = _getAuthHeader(hubRequestOptions);

return fetch(url, { method: "GET", headers, mode: "cors" })
Expand Down
22 changes: 15 additions & 7 deletions packages/common/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import {
ILayerDefinition,
} from "@esri/arcgis-rest-types";
import { IPortal, ISearchResult } from "@esri/arcgis-rest-portal";
import { IUserRequestOptions } from "@esri/arcgis-rest-auth";
import { UserSession } from "@esri/arcgis-rest-auth";
import { IStructuredLicense } from "./items/get-structured-license";
import { IRequestOptions } from "@esri/arcgis-rest-request";

/**
* Generic Model, used with all items that have a json
Expand Down Expand Up @@ -102,12 +103,20 @@ interface IHubRequestOptionsPortalSelf extends IPortal {
user?: IUser;
}

export interface IHubRequestOptions extends IUserRequestOptions {
isPortal: boolean;
hubApiUrl: string;
export interface IHubRequestOptions extends IRequestOptions {
authentication?: UserSession;
isPortal?: boolean;
hubApiUrl?: string;
portalSelf?: IHubRequestOptionsPortalSelf;
}

/**
* Options for requests that require an authenticated user.
*/
export interface IHubUserRequestOptions extends IHubRequestOptions {
authentication: UserSession;
}

export interface IItemResource {
type?: string;
url: string;
Expand Down Expand Up @@ -430,9 +439,8 @@ export interface ISerializedOperationStack {
* @export
* @interface UpdateSiteOptions
*/
export interface IUpdateSiteOptions extends IHubRequestOptions {
export interface IUpdateSiteOptions extends IUpdatePageOptions {
updateVersions?: boolean;
allowList?: string[];
}

/**
Expand All @@ -443,7 +451,7 @@ export interface IUpdateSiteOptions extends IHubRequestOptions {
* @export
* @interface UpdatePageOptions
*/
export interface IUpdatePageOptions extends IHubRequestOptions {
export interface IUpdatePageOptions extends IHubUserRequestOptions {
allowList?: string[];
}

Expand Down
13 changes: 11 additions & 2 deletions packages/common/test/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getHubApiUrl } from "../src/api";
import { IRequestOptions } from "@esri/arcgis-rest-request";
import { IHubRequestOptions } from "../src/types";

describe("getHubApiUrl", () => {
let ro: IRequestOptions;
Expand All @@ -9,8 +10,8 @@ describe("getHubApiUrl", () => {
portal: null,
getToken() {
return Promise.resolve("FAKE-TOKEN");
}
}
},
},
};
});

Expand Down Expand Up @@ -50,4 +51,12 @@ describe("getHubApiUrl", () => {
ro.authentication.portal = "https://devext.arcgis.com/sharing/rest";
expect(getHubApiUrl(ro)).toBe("https://hubdev.arcgis.com");
});

it("returns existing hubApiUrl on IHubRequestOptions", () => {
const hubApiUrl = "fake.url.com";
const hro: IHubRequestOptions = {
hubApiUrl,
};
expect(getHubApiUrl(hro)).toBe(hubApiUrl);
});
});
16 changes: 8 additions & 8 deletions packages/common/test/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,30 @@ import * as fetchMock from "fetch-mock";
import { hubApiRequest } from "../src/request";

describe("hubApiRequest", () => {
it("handles a server error", done => {
it("handles a server error", (done) => {
const status = 403;
const route = "badurl";
fetchMock.once("*", {
status
status,
});
hubApiRequest(route).catch(e => {
hubApiRequest(route).catch((e) => {
expect(e.message).toBe("Forbidden");
expect(e.status).toBe(status);
expect(e.url).toBe(`https://opendata.arcgis.com/api/v3/${route}`);
expect(e.url).toBe(`https://hub.arcgis.com/api/v3/${route}`);
done();
});
});
it("stringfies params in the body of POST", done => {
it("stringfies params in the body of POST", (done) => {
fetchMock.once("*", { the: "goods" });
hubApiRequest("datasets", {
isPortal: false,
hubApiUrl: "https://some.url.com/",
httpMethod: "POST",
authentication: null,
params: {
foo: "bar"
}
}).then(response => {
foo: "bar",
},
}).then((response) => {
const [url, options] = fetchMock.calls()[0];
expect(url).toEqual("https://some.url.com/api/v3/datasets");
expect(options.body).toBe('{"foo":"bar"}');
Expand Down
21 changes: 13 additions & 8 deletions packages/discussions/src/utils/request.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { IHubRequestOptions } from "../types";
import { RemoteServerError as _RemoteServerError } from "@esri/hub-common";
import {
RemoteServerError as _RemoteServerError,
buildUrl,
} from "@esri/hub-common";

export class RemoteServerError extends _RemoteServerError {
error: string;
Expand Down Expand Up @@ -62,13 +65,15 @@ export function apiRequest<T>(
method: options.httpMethod || "GET",
mode: options.mode,
cache: options.cache,
credentials: options.credentials
credentials: options.credentials,
};

// NOTE: this should default to the prod url once deployed and microservice root URLs
// are normalized: https://github.com/Esri/hub.js/pull/479#discussion_r607866561
const apiBase =
options.hubApiUrl || "https://hub.arcgis.com/api/discussions/v1";
const apiBase = buildUrl({
// TODO: we _want_ to use getHubApiUrl(),
// but have to deal w/ the fact that this package overwrites IHubRequestOptions
host: options.hubApiUrl || "https://hub.arcgis.com",
path: "/api/discussions/v1",
});

if (options.params) {
if (options.httpMethod === "GET") {
Expand All @@ -80,12 +85,12 @@ export function apiRequest<T>(
}

const url = [apiBase.replace(/\/$/, ""), route.replace(/^\//, "")].join("/");
return fetch(url, opts).then(res => {
return fetch(url, opts).then((res) => {
if (res.ok) {
return res.json();
} else {
const { statusText, status } = res;
return res.json().then(err => {
return res.json().then((err) => {
throw new RemoteServerError(
statusText,
url,
Expand Down
16 changes: 9 additions & 7 deletions packages/sites/src/_remove-site-from-index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IModel, IHubRequestOptions } from "@esri/hub-common";
import { IModel, IHubRequestOptions, getHubApiUrl } from "@esri/hub-common";

/**
* Remove a Site from the Hub Index system
Expand All @@ -13,21 +13,23 @@ export function _removeSiteFromIndex(
if (hubRequestOptions.isPortal) {
return Promise.resolve();
} else {
const url = `${hubRequestOptions.hubApiUrl}/v2/${siteModel.item.id}`;
const url = `${getHubApiUrl(hubRequestOptions)}/api/v3/${
siteModel.item.id
}`;
const opts = {
method: "DELETE",
mode: "cors",
headers: {
Authorization: hubRequestOptions.authentication.token
}
Authorization: hubRequestOptions.authentication.token,
},
} as RequestInit;
return fetch(url, opts)
.then(raw => raw.json())
.then(_ => {
.then((raw) => raw.json())
.then((_) => {
// TODO: Should we do anything here?
return { success: true };
})
.catch(err => {
.catch((err) => {
throw Error(
`_removeSiteFromIndex: Error removing site from index: ${err}`
);
Expand Down
10 changes: 4 additions & 6 deletions packages/sites/src/create-site-model-from-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
stripProtocol,
interpolate,
addSolutionResourceUrlToAssets,
getHubApiUrl,
} from "@esri/hub-common";
import { createHubTeams } from "@esri/hub-teams";
import { HubTeamType } from "@esri/hub-teams";
Expand Down Expand Up @@ -114,12 +115,9 @@ export function createSiteModelFromTemplate(
);
settings.solution.url = getPortalSiteUrl(uniqueSubdomain, portal);
} else {
settings.solution.defaultHostname = `${uniqueSubdomain}-${
portal.urlKey
}.${stripProtocol(hubRequestOptions.hubApiUrl)}`;
settings.solution.url = `https://${uniqueSubdomain}-${
portal.urlKey
}.${stripProtocol(hubRequestOptions.hubApiUrl)}`;
const base = stripProtocol(getHubApiUrl(hubRequestOptions));
settings.solution.defaultHostname = `${uniqueSubdomain}-${portal.urlKey}.${base}`;
settings.solution.url = `https://${uniqueSubdomain}-${portal.urlKey}.${base}`;
}

// create the initiative
Expand Down
14 changes: 7 additions & 7 deletions packages/sites/src/drafts/save-published-status.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IModel, IHubRequestOptions } from "@esri/hub-common";
import { IHubUserRequestOptions, IModel } from "@esri/hub-common";
import { updateSite } from "../update-site";
import { updatePage, isPage } from "../pages";
import { isSite } from "../is-site";
Expand All @@ -10,11 +10,11 @@ import { hasUnpublishedChanges } from "./has-unpublished-changes";
* leaving everything else on the model alone.
*
* @param siteOrPageModel
* @param hubRequestOptions
* @param requestOptions
*/
export function savePublishedStatus(
siteOrPageModel: IModel,
hubRequestOptions: IHubRequestOptions
requestOptions: IHubUserRequestOptions
): Promise<IUpdateItemResponse> {
const allowList = ["item.typeKeywords"]; // only want to save typeKeywords
const { item } = siteOrPageModel;
Expand All @@ -28,14 +28,14 @@ export function savePublishedStatus(
// changes
const isUnpublished = hasUnpublishedChanges(siteOrPageModel);
prms = updateSite(siteOrPageModel, {
...hubRequestOptions,
...requestOptions,
allowList,
updateVersions: !isUnpublished
updateVersions: !isUnpublished,
});
} else if (isPage(item)) {
prms = updatePage(siteOrPageModel, {
...hubRequestOptions,
allowList
...requestOptions,
allowList,
});
} else {
throw TypeError(
Expand Down
Loading

0 comments on commit 7aa7ade

Please sign in to comment.