Skip to content

Commit

Permalink
fix(🐛): ensure createFeatureService doesnt swallow errors
Browse files Browse the repository at this point in the history 
AFFECTS PACKAGES:
@esri/arcgis-rest-feature-service-admin
  • Loading branch information
@MikeTschudi
MikeTschudi authored and john gravois committed Jan 29, 2019
1 parent 9017f09 commit 8173aa5
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 156 deletions.
4 changes: 2 additions & 2 deletions karma.conf.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ module.exports = function(config) {
}
},
reports: {
"text": "",
"json": {
"directory": "coverage",
"filename": "coverage.json"
},
},
"html": "coverage"
},
compilerOptions: {
module: "commonjs"
Expand Down
45 changes: 20 additions & 25 deletions packages/arcgis-rest-feature-service-admin/src/create.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ export function createFeatureService(
requestOptions: ICreateServiceRequestOptions
): Promise<ICreateServiceResult> {
const owner = determineOwner(requestOptions);

const baseUrl = `${getPortalUrl(requestOptions)}/content/users/${owner}`;
const url = `${baseUrl}/createService`;

Expand All @@ -179,34 +178,30 @@ export function createFeatureService(
};

if (!requestOptions.folderId || requestOptions.folderId === "/") {
// If the service is destined for the root folder, just send the request and return the promise;
// services are created in the root folder
// If the service is destined for the root folder, just send the request
return request(url, requestOptions);
} else {
// If the service is destined for a subfolder, catch the results of the request and then move
// the item to the desired folder
return new Promise((resolve, reject) => {
request(url, requestOptions)
.then(createResults => {
if (createResults.success) {
moveItem({
itemId: createResults.itemId,
folderId: requestOptions.folderId,
authentication: requestOptions.authentication
})
.then(moveResults => {
if (moveResults.success) {
resolve(createResults);
} else {
reject({ success: false });
}
})
.catch();
// If the service is destined for a subfolder, move it (via another call)
return request(url, requestOptions).then(createResponse => {
if (createResponse.success) {
return moveItem({
itemId: createResponse.itemId,
folderId: requestOptions.folderId,
authentication: requestOptions.authentication
}).then(moveResponse => {
if (moveResponse.success) {
return createResponse;
} else {
reject({ success: false });
throw Error(
`A problem was encountered when trying to move the service to a different folder.`
);
}
})
.catch();
});
} else {
throw Error(
`A problem was encountered when trying to create the service.`
);
}
});
}
}
207 changes: 86 additions & 121 deletions packages/arcgis-rest-feature-service-admin/test/create.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,8 @@ import * as fetchMock from "fetch-mock";

import { createFeatureService } from "../src/create";

import {
FeatureServiceSuccessResponse,
FeatureServiceFailResponse
} from "./mocks/service";
import {
MoveToFolderSuccessResponse,
MoveToFolderFailResponse
} from "./mocks/move";
import { FeatureServiceResponse } from "./mocks/service";
import { MoveToFolderResponse } from "./mocks/move";

import { UserSession } from "@esri/arcgis-rest-auth";
import { TOMORROW } from "@esri/arcgis-rest-auth/test/utils";
Expand Down Expand Up @@ -72,9 +66,7 @@ describe("create feature service", () => {
};

it("should create a feature service defaulting to the root folder", done => {
fetchMock
.mock("end:createService", FeatureServiceSuccessResponse, {})
.mock("end:move", MoveToFolderFailResponse, {});
fetchMock.mock("end:createService", FeatureServiceResponse, {});

createFeatureService({
item: serviceDescription,
Expand Down Expand Up @@ -107,7 +99,7 @@ describe("create feature service", () => {
);

// Check response
expect(response).toEqual(FeatureServiceSuccessResponse);
expect(response).toEqual(FeatureServiceResponse);

done();
},
Expand All @@ -121,9 +113,7 @@ describe("create feature service", () => {
});

it("should create a feature service specified for the root folder 1", done => {
fetchMock
.mock("end:createService", FeatureServiceSuccessResponse, {})
.mock("end:move", MoveToFolderFailResponse, {});
fetchMock.mock("end:createService", FeatureServiceResponse, {});
const folderId = "";

createFeatureService({
Expand Down Expand Up @@ -158,7 +148,7 @@ describe("create feature service", () => {
);

// Check response
expect(response).toEqual(FeatureServiceSuccessResponse);
expect(response).toEqual(FeatureServiceResponse);

done();
},
Expand All @@ -172,9 +162,7 @@ describe("create feature service", () => {
});

it("should create a feature service specified for the root folder 2", done => {
fetchMock
.mock("end:createService", FeatureServiceSuccessResponse, {})
.mock("end:move", MoveToFolderFailResponse, {});
fetchMock.mock("end:createService", FeatureServiceResponse, {});
const folderId = "/";

createFeatureService({
Expand Down Expand Up @@ -209,7 +197,7 @@ describe("create feature service", () => {
);

// Check response
expect(response).toEqual(FeatureServiceSuccessResponse);
expect(response).toEqual(FeatureServiceResponse);

done();
},
Expand All @@ -224,8 +212,8 @@ describe("create feature service", () => {

it("should create a feature service in a particular folder", done => {
fetchMock
.mock("end:createService", FeatureServiceSuccessResponse, {})
.mock("end:move", MoveToFolderSuccessResponse, {});
.mock("end:createService", FeatureServiceResponse, {})
.mock("end:move", MoveToFolderResponse, {});
const folderId = "83216cba44bf4357bf06687ec88a847b";

createFeatureService({
Expand Down Expand Up @@ -277,7 +265,7 @@ describe("create feature service", () => {
);

// Check response
expect(response).toEqual(FeatureServiceSuccessResponse);
expect(response).toEqual(FeatureServiceResponse);

done();
},
Expand All @@ -290,123 +278,100 @@ describe("create feature service", () => {
});
});

it("should fail to create a feature service defaulting to the root folder", done => {
fetchMock
.mock("end:createService", FeatureServiceFailResponse, {})
.mock("end:move", MoveToFolderFailResponse, {});
it("should fail to create a feature service destined for a particular folder with success=false", done => {
fetchMock.mock("end:createService", { success: false });

const folderId = "83216cba44bf4357bf06687ec88a847b";

createFeatureService({
item: serviceDescription,
folderId,
...MOCK_USER_REQOPTS
})
.then(
() => {
fail(); // call is supposed to fail
},
response => {
expect(fetchMock.called("end:createService")).toEqual(true);
expect(fetchMock.called("end:move")).toEqual(false);

// Check create service call
const [urlCreate, optionsCreate]: [
string,
RequestInit
] = fetchMock.lastCall("end:createService");
expect(urlCreate).toEqual(
"https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/createService"
);
expect(optionsCreate.method).toBe("POST");
expect(optionsCreate.body).toContain("f=json");
expect(optionsCreate.body).toContain(
encodeParam(
"createParameters",
JSON.stringify(serviceDescription)
)
);
expect(optionsCreate.body).toContain("outputType=featureService");
expect(optionsCreate.body).toContain(
encodeParam("token", "fake-token")
);

// Check response
expect(response).toEqual(FeatureServiceFailResponse);

done();
}
)
.then(() => fail())
.catch(e => {
fail(e);
expect(fetchMock.called("end:createService")).toEqual(true);
expect(fetchMock.called("end:move")).toEqual(false);

// Check create service call
const [urlCreate, optionsCreate]: [
string,
RequestInit
] = fetchMock.lastCall("end:createService");
expect(urlCreate).toEqual(
"https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/createService"
);
expect(optionsCreate.method).toBe("POST");
expect(optionsCreate.body).toContain("f=json");
expect(optionsCreate.body).toContain(
encodeParam("createParameters", JSON.stringify(serviceDescription))
);
expect(optionsCreate.body).toContain("outputType=featureService");
expect(optionsCreate.body).toContain(
encodeParam("token", "fake-token")
);
expect(e.message).toEqual(
`A problem was encountered when trying to create the service.`
);
done();
});
});

it("should create a feature service, but fail to move it to a particular folder", done => {
fetchMock
.mock("end:createService", FeatureServiceSuccessResponse, {})
.mock("end:move", MoveToFolderFailResponse, {});
it("should fail to create a feature service destined for a particular folder with success=false (for the move)", done => {
fetchMock.mock("end:createService", FeatureServiceResponse, {});
fetchMock.mock("end:move", { success: false });
const folderId = "83216cba44bf4357bf06687ec88a847b";

createFeatureService({
item: serviceDescription,
folderId,
...MOCK_USER_REQOPTS
})
.then(
() => {
fail(); // call is supposed to fail
},
response => {
expect(fetchMock.called("end:createService")).toEqual(true);
expect(fetchMock.called("end:move")).toEqual(true);

// Check create service call
const [urlCreate, optionsCreate]: [
string,
RequestInit
] = fetchMock.lastCall("end:createService");
expect(urlCreate).toEqual(
"https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/createService"
);
expect(optionsCreate.method).toBe("POST");
expect(optionsCreate.body).toContain("f=json");
expect(optionsCreate.body).toContain(
encodeParam(
"createParameters",
JSON.stringify(serviceDescription)
)
);
expect(optionsCreate.body).toContain("outputType=featureService");
expect(optionsCreate.body).toContain(
encodeParam("token", "fake-token")
);

// Because the service is created in the root folder, the API follows it with a move call
const [urlMove, optionsMove]: [
string,
RequestInit
] = fetchMock.lastCall("end:move");
expect(urlMove).toEqual(
"https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/items/" +
FeatureServiceSuccessResponse.serviceItemId +
"/move"
);
expect(optionsMove.method).toBe("POST");
expect(optionsMove.body).toContain("folder=" + folderId);
expect(optionsMove.body).toContain("f=json");
expect(optionsMove.body).toContain(
encodeParam("token", "fake-token")
);

// Check response
expect(response).toEqual(FeatureServiceFailResponse);

done();
}
)
.then(() => fail())
.catch(e => {
fail(e);
expect(fetchMock.called("end:createService")).toEqual(true);
expect(fetchMock.called("end:move")).toEqual(true);

// Check create service call
const [urlCreate, optionsCreate]: [
string,
RequestInit
] = fetchMock.lastCall("end:createService");
expect(urlCreate).toEqual(
"https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/createService"
);
expect(optionsCreate.method).toBe("POST");
expect(optionsCreate.body).toContain("f=json");
expect(optionsCreate.body).toContain(
encodeParam("createParameters", JSON.stringify(serviceDescription))
);
expect(optionsCreate.body).toContain("outputType=featureService");
expect(optionsCreate.body).toContain(
encodeParam("token", "fake-token")
);

// Because the service is created in the root folder, the API follows it with a move call
const [urlMove, optionsMove]: [
string,
RequestInit
] = fetchMock.lastCall("end:move");
expect(urlMove).toEqual(
`https://myorg.maps.arcgis.com/sharing/rest/content/users/casey/items/${
FeatureServiceResponse.serviceItemId
}/move`
);
expect(optionsMove.method).toBe("POST");
expect(optionsMove.body).toContain("folder=" + folderId);
expect(optionsMove.body).toContain("f=json");
expect(optionsMove.body).toContain(
encodeParam("token", "fake-token")
);

expect(e.message).toEqual(
`A problem was encountered when trying to move the service to a different folder.`
);
done();
});
});
}); // auth requests
});
});
5 changes: 1 addition & 4 deletions packages/arcgis-rest-feature-service-admin/test/mocks/move.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@

import { IItemMoveResponse } from "@esri/arcgis-rest-items";

export const MoveToFolderSuccessResponse: IItemMoveResponse = {
export const MoveToFolderResponse: IItemMoveResponse = {
folder: "83216cba44bf4357bf06687ec88a847b",
itemId: "1b1a3c914ef94c49ae55ce223cac5754",
owner: "casey",
success: true
};
export const MoveToFolderFailResponse: any = {
success: false
};
Loading

0 comments on commit 8173aa5

Please sign in to comment.