From d6a059447d4583281ad1d0520534fb61e07a5be5 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 18 Apr 2019 11:41:00 -0700 Subject: [PATCH] Fix: Enables selection of azure region for custom vision export (#765) Adds ability to select azure region where your custom vision service is hosted. Filters domain list by project type Resolves #759, #770 --- src/common/localization/en-us.ts | 18 +++ src/common/localization/es-cl.ts | 18 +++ src/common/strings.ts | 18 +++ src/providers/export/azureCustomVision.json | 49 +++++++- .../export/azureCustomVision.test.ts | 19 ++- src/providers/export/azureCustomVision.ts | 26 +++- .../export/azureCustomVision.ui.json | 11 +- .../externalPicker/externalPicker.test.tsx | 112 ++++++++++++++---- .../common/externalPicker/externalPicker.tsx | 69 ++++++++--- .../common/protectedInput/protectedInput.tsx | 2 +- .../pages/export/exportForm.test.tsx | 1 + .../components/pages/export/exportForm.tsx | 10 +- tsconfig.json | 48 ++++---- typings.json | 5 - .../globals/react-jsonschema-form/index.d.ts | 33 ------ .../react-jsonschema-form/typings.json | 8 -- typings/index.d.ts | 1 - typings/react-jsonschema-form/index.d.ts | 3 + 18 files changed, 326 insertions(+), 125 deletions(-) delete mode 100644 typings/globals/react-jsonschema-form/index.d.ts delete mode 100644 typings/globals/react-jsonschema-form/typings.json delete mode 100644 typings/index.d.ts create mode 100644 typings/react-jsonschema-form/index.d.ts diff --git a/src/common/localization/en-us.ts b/src/common/localization/en-us.ts index 5f8f10b..c9074a6 100644 --- a/src/common/localization/en-us.ts +++ b/src/common/localization/en-us.ts @@ -304,10 +304,28 @@ export const english: IAppStrings = { }, azureCV: { displayName: "Azure Custom Vision Service", + regions: { + australiaEast: "Australia East", + centralIndia: "Central India", + eastUs: "East US", + eastUs2: "East US 2", + japanEast: "Japan East", + northCentralUs: "North Central US", + northEurope: "North Europe", + southCentralUs: "South Central US", + southeastAsia: "Southeast Asia", + ukSouth: "UK South", + westUs2: "West US 2", + westEurope: "West Europe", + }, properties: { apiKey: { title: "API Key", }, + region: { + title: "Region", + description: "The Azure region where your service is deployed", + }, classificationType: { title: "Classification Type", options: { diff --git a/src/common/localization/es-cl.ts b/src/common/localization/es-cl.ts index 1fc212f..1e9d3cd 100644 --- a/src/common/localization/es-cl.ts +++ b/src/common/localization/es-cl.ts @@ -306,10 +306,28 @@ export const spanish: IAppStrings = { }, azureCV: { displayName: "Servicio de Visión Personalizada Azure", + regions: { + australiaEast: "Australia este", + centralIndia: "Centro de la India", + eastUs: "Este de EE.", + eastUs2: "Este US 2", + japanEast: "Japón este", + northCentralUs: "Centro norte de EE.", + northEurope: "Europa del norte", + southCentralUs: "Centro sur de EE.", + southeastAsia: "Sudeste asiático", + ukSouth: "UK sur", + westUs2: "West US 2", + westEurope: "Europa occidental", + }, properties: { apiKey: { title: "Clave de API", }, + region: { + title: "Región", + description: "La región de Azure donde se implementa el servicio", + }, classificationType: { title: "Tipo de clasificación", options: { diff --git a/src/common/strings.ts b/src/common/strings.ts index a3c5bf8..31e0114 100644 --- a/src/common/strings.ts +++ b/src/common/strings.ts @@ -302,10 +302,28 @@ export interface IAppStrings { }, azureCV: { displayName: string, + regions: { + eastUs: string, + eastUs2: string, + northCentralUs: string, + southCentralUs: string, + westUs2: string, + westEurope: string, + northEurope: string, + southeastAsia: string, + australiaEast: string, + centralIndia: string, + ukSouth: string, + japanEast: string, + }, properties: { apiKey: { title: string, }, + region: { + title: string, + description: string, + }, newOrExisting: { title: string, options: { diff --git a/src/providers/export/azureCustomVision.json b/src/providers/export/azureCustomVision.json index a657b4f..fd130fd 100644 --- a/src/providers/export/azureCustomVision.json +++ b/src/providers/export/azureCustomVision.json @@ -3,7 +3,8 @@ "title": "${strings.export.providers.azureCV.displayName}", "required": [ "assetState", - "apiKey" + "apiKey", + "region" ], "properties": { "assetState": { @@ -22,6 +23,40 @@ "${strings.export.providers.common.properties.assetState.options.tagged}" ] }, + "region": { + "type": "string", + "title": "${strings.export.providers.azureCV.properties.region.title}", + "description": "${strings.export.providers.azureCV.properties.region.description}", + "default": "southcentralus", + "enum": [ + "australiaeast", + "centralindia", + "eastus", + "eastus2", + "japaneast", + "northcentralus", + "northeurope", + "southcentralus", + "southeastasia", + "uksouth", + "westus2", + "westeurope" + ], + "enumNames": [ + "${strings.export.providers.azureCV.regions.australiaEast}", + "${strings.export.providers.azureCV.regions.centralIndia}", + "${strings.export.providers.azureCV.regions.eastUs}", + "${strings.export.providers.azureCV.regions.eastUs2}", + "${strings.export.providers.azureCV.regions.japanEast}", + "${strings.export.providers.azureCV.regions.northCentralUs}", + "${strings.export.providers.azureCV.regions.northEurope}", + "${strings.export.providers.azureCV.regions.southCentralUs}", + "${strings.export.providers.azureCV.regions.southeastAsia}", + "${strings.export.providers.azureCV.regions.ukSouth}", + "${strings.export.providers.azureCV.regions.westUs2}", + "${strings.export.providers.azureCV.regions.westEurope}" + ] + }, "apiKey": { "type": "string", "title": "${strings.export.providers.azureCV.properties.apiKey.title}" @@ -61,19 +96,20 @@ "projectType": { "type": "string", "title": "${strings.export.providers.azureCV.properties.projectType.title}", + "default": "Classification", "enum": [ "Classification", - "Object Detection" + "ObjectDetection" ], "enumNames": [ "${strings.export.providers.azureCV.properties.projectType.options.classification}", "${strings.export.providers.azureCV.properties.projectType.options.objectDetection}" - ], - "default": "Classification" + ] }, "classificationType": { "type": "string", "title": "${strings.export.providers.azureCV.properties.classificationType.title}", + "default": "Multilabel", "enum": [ "Multilabel", "Multiclass" @@ -81,8 +117,7 @@ "enumNames": [ "${strings.export.providers.azureCV.properties.classificationType.options.multiLabel}", "${strings.export.providers.azureCV.properties.classificationType.options.multiClass}" - ], - "default": "Multilabel" + ] }, "domainId": { "type": "string", @@ -91,6 +126,8 @@ }, "required": [ "name", + "projectType", + "classificationType", "domainId" ] }, diff --git a/src/providers/export/azureCustomVision.test.ts b/src/providers/export/azureCustomVision.test.ts index 36c1f21..e83e834 100644 --- a/src/providers/export/azureCustomVision.test.ts +++ b/src/providers/export/azureCustomVision.test.ts @@ -1,6 +1,9 @@ import shortid from "shortid"; import _ from "lodash"; -import { AzureCustomVisionProvider, IAzureCustomVisionExportOptions, NewOrExisting } from "./azureCustomVision"; +import { + AzureCustomVisionProvider, IAzureCustomVisionExportOptions, + NewOrExisting, AzureRegion, +} from "./azureCustomVision"; import registerProviders from "../../registerProviders"; import { ExportProviderFactory } from "./exportProviderFactory"; import MockFactory from "../../common/mockFactory"; @@ -29,6 +32,7 @@ describe("Azure Custom Vision Export Provider", () => { let testProject: IProject = null; const defaultOptions: IAzureCustomVisionExportOptions = { apiKey: expect.any(String), + region: AzureRegion.SouthCentralUS, assetState: ExportAssetState.All, newOrExisting: NewOrExisting.New, projectId: expect.any(String), @@ -64,6 +68,7 @@ describe("Azure Custom Vision Export Provider", () => { assetState: ExportAssetState.All, projectId: "azure-custom-vision-project-1", apiKey: "ABC123", + region: AzureRegion.SouthCentralUS, }, }, }; @@ -81,6 +86,18 @@ describe("Azure Custom Vision Export Provider", () => { expect(provider).toBeInstanceOf(AzureCustomVisionProvider); }); + it("Constructs custom vision service with correct options", () => { + const customVisionMock = AzureCustomVisionService as jest.Mocked; + const providerOptions = testProject.exportFormat.providerOptions as IAzureCustomVisionExportOptions; + providerOptions.region = AzureRegion.WestEurope; + createProvider(testProject); + + expect(customVisionMock).toBeCalledWith({ + apiKey: providerOptions.apiKey, + baseUrl: `https://${providerOptions.region}.api.cognitive.microsoft.com/customvision/v2.2/Training`, + }); + }); + it("Calling save with New project creates Azure Custom Vision project", async () => { const customVisionMock = AzureCustomVisionService as jest.Mocked; customVisionMock.prototype.create = jest.fn((project) => { diff --git a/src/providers/export/azureCustomVision.ts b/src/providers/export/azureCustomVision.ts index 832e21a..99ff10f 100644 --- a/src/providers/export/azureCustomVision.ts +++ b/src/providers/export/azureCustomVision.ts @@ -17,6 +17,7 @@ import HtmlFileReader from "../../common/htmlFileReader"; export interface IAzureCustomVisionExportOptions extends IExportProviderOptions { assetState: ExportAssetState; newOrExisting: NewOrExisting; + region: AzureRegion; apiKey: string; projectId?: string; name?: string; @@ -38,6 +39,24 @@ export enum NewOrExisting { Existing = "existing", } +/** + * Azure regions + */ +export enum AzureRegion { + EastUS = "eastus", + EastUS2 = "eastus2", + NorthCentralUS = "northcentralus", + SouthCentralUS = "southcentralus", + WestUS2 = "westus2", + WestEurope = "westeurope", + NorthEurope = "northeurope", + SoutheastAsia = "southeastasia", + AustraliaEast = "australiaeast", + CentralIndia = "centralindia", + UKSouth = "uksouth", + JapanEast = "japaneast", +} + /** * @name - Azure Custom Vision Provider * @description - Exports a VoTT project into an Azure custom vision project @@ -49,9 +68,13 @@ export class AzureCustomVisionProvider extends ExportProvider { - let wrapper: ReactWrapper = null; - const onChangeHandler = jest.fn(); + const onChangeHandler = jest.fn(() => { + console.log("hi"); + }); const defaultProps = createProps({ id: "my-custom-control", value: "", @@ -16,12 +17,13 @@ describe("External Picker", () => { formContext: { providerOptions: { apiKey: "", + region: "", }, }, onChange: onChangeHandler, options: { method: "GET", - url: "https://myserver/api", + url: "https://${props.formContext.providerOptions.region}.server.com/api", keySelector: "${item.key}", valueSelector: "${item.value}", authHeaderName: "Authorization", @@ -48,32 +50,34 @@ describe("External Picker", () => { }); }); - beforeEach(() => { - wrapper = createComponent(defaultProps as IExternalPickerProps); - }); - it("Renders select element with default option", () => { + const wrapper = createComponent(defaultProps); expect(wrapper.find("select").length).toEqual(1); expect(wrapper.find("option").length).toEqual(1); }); it("Does not bind external data if authorization is missing", () => { + createComponent(defaultProps); expect(axios.request).not.toBeCalled(); }); it("Renders items bound from external data when formContext rebinds", async () => { const expectedApiKey = "ABC123"; + const expectedRegion = "southcentralus"; - await MockFactory.flushUi(() => { - wrapper.setProps({ - formContext: { - providerOptions: { - apiKey: expectedApiKey, - }, + const props = { + ...defaultProps, + formContext: { + providerOptions: { + apiKey: expectedApiKey, + region: expectedRegion, }, - }); - }); + }, + }; + const wrapper = createComponent(props); + + await MockFactory.flushUi(); wrapper.update(); const expectedHeaders = {}; @@ -81,7 +85,7 @@ describe("External Picker", () => { expect(axios.request).toBeCalledWith({ method: defaultProps.options.method, - url: defaultProps.options.url, + url: `https://${expectedRegion}.server.com/api`, headers: expectedHeaders, }); @@ -93,19 +97,81 @@ describe("External Picker", () => { }); it("Calls onChange event handler on option selection", () => { - wrapper.setProps({ - formContext: {}, - }); + const wrapper = createComponent(defaultProps); wrapper.find("select").simulate("change", { target: { value: testResponse[0].key } }); expect(onChangeHandler).toBeCalledWith(testResponse[0].key); }); - function createProps(otherProps: any): IExternalPickerProps { + it("Clears items when HTTP request fails", async () => { + const requestMock = axios.request as jest.Mock; + requestMock.mockImplementationOnce(() => Promise.reject({ status: 400 })); + + const expectedApiKey = "ABC123"; + const expectedRegion = "southcentralus"; + const props: IExternalPickerProps = { - ...otherProps, + ...defaultProps, + formContext: { + providerOptions: { + apiKey: expectedApiKey, + region: expectedRegion, + }, + }, }; - return props; + const wrapper = createComponent(props); + await MockFactory.flushUi(); + + expect(wrapper.state().items).toEqual([]); + expect(onChangeHandler).toBeCalledWith(undefined); + }); + + describe("Filters items", () => { + it("Applies a filter to the item when defined", async () => { + const requestMock = axios.request as jest.Mock; + requestMock.mockImplementationOnce(() => Promise.resolve({ + data: [ + { id: "1", name: "Object Detection 1", type: "ObjectDetection" }, + { id: "2", name: "Object Detection 2", type: "ObjectDetection" }, + { id: "3", name: "Classification 1", type: "Classification" }, + { id: "4", name: "Classification 2", type: "Classification" }, + ], + status: 200, + })); + + const props: IExternalPickerProps = { + ...defaultProps, + formContext: { + providerOptions: { + apiKey: "ABC123", + region: "southcentralus", + projectType: "Classification", + }, + }, + }; + + props.options.keySelector = "${item.id}"; + props.options.valueSelector = "${item.name}"; + props.options.filter = { + left: "${item.type}", + right: "${props.formContext.providerOptions.projectType}", + operator: FilterOperator.Equals, + }; + + const wrapper = createComponent(props); + await MockFactory.flushUi(); + + expect(wrapper.state().items).toEqual([ + { key: "3", value: "Classification 1" }, + { key: "4", value: "Classification 2" }, + ]); + }); + }); + + function createProps(otherProps: any): IExternalPickerProps { + return { + ...otherProps, + }; } }); diff --git a/src/react/components/common/externalPicker/externalPicker.tsx b/src/react/components/common/externalPicker/externalPicker.tsx index b719bc7..acad549 100644 --- a/src/react/components/common/externalPicker/externalPicker.tsx +++ b/src/react/components/common/externalPicker/externalPicker.tsx @@ -24,6 +24,19 @@ export interface IExternalPickerUiOptions { valueSelector: string; authHeaderName?: string; authHeaderValue?: string; + filter?: IExternalPickerFilter; +} + +export interface IExternalPickerFilter { + left: string; + right: string; + operator: FilterOperator; +} + +export enum FilterOperator { + Equals = "eq", + GreaterThan = "gt", + LessThan = "lt", } /** @@ -46,15 +59,9 @@ export interface IExternalPickerState { * Dropdown that provides options from an external HTTP source */ export default class ExternalPicker extends React.Component { - constructor(props, context) { - super(props, context); - - this.state = { - items: [], - }; - - this.onChange = this.onChange.bind(this); - } + public state: IExternalPickerState = { + items: [], + }; public render() { return ( @@ -78,12 +85,12 @@ export default class ExternalPicker extends React.Component { const target = e.target as HTMLSelectElement; this.props.onChange(target.value === "" ? undefined : target.value); } - private async bindExternalData() { + private bindExternalData = async (): Promise => { const uiOptions = this.props.options; const customHeaders: any = {}; const authHeaderValue = interpolate(uiOptions.authHeaderValue, { @@ -98,24 +105,52 @@ export default class ExternalPicker extends React.Component { + + let rawItems: any[] = response.data; + + // Optionally filter results if a filter has been defined + if (uiOptions.filter) { + rawItems = rawItems.filter((item) => this.filterPredicate(item, uiOptions.filter)); + } + + const items: IKeyValuePair[] = rawItems.map((item) => { return { key: interpolate(uiOptions.keySelector, { item }), value: interpolate(uiOptions.valueSelector, { item }), }; }); - this.setState({ - items, - }); + this.setState({ items }); } catch (e) { - return; + this.setState({ items: [] }); + this.props.onChange(undefined); + } + } + + /** + * Determines if the specified item will return as part of the filter + * @param item The item to evaluate + * @param filter The filter expression to evaluate against + */ + private filterPredicate(item: any, filter: IExternalPickerFilter): boolean { + const left = interpolate(filter.left, { item, props: this.props }); + const right = interpolate(filter.right, { item, props: this.props }); + + switch (filter.operator) { + case FilterOperator.Equals: + return left === right; + case FilterOperator.GreaterThan: + return left > right; + case FilterOperator.LessThan: + return left < right; + default: + throw new Error("Invalid filter operator"); } } } diff --git a/src/react/components/common/protectedInput/protectedInput.tsx b/src/react/components/common/protectedInput/protectedInput.tsx index 5143030..ff2224e 100644 --- a/src/react/components/common/protectedInput/protectedInput.tsx +++ b/src/react/components/common/protectedInput/protectedInput.tsx @@ -33,7 +33,7 @@ export class ProtectedInput extends React.Component { providerType: "vottJson", providerOptions: { assetState: ExportAssetState.Tagged, + includeImages: true, }, }, onSubmit: onSubmitHandler, diff --git a/src/react/components/pages/export/exportForm.tsx b/src/react/components/pages/export/exportForm.tsx index 7780ddb..8263ddb 100644 --- a/src/react/components/pages/export/exportForm.tsx +++ b/src/react/components/pages/export/exportForm.tsx @@ -1,6 +1,7 @@ import React from "react"; import _ from "lodash"; import Form, { Widget, FormValidation, IChangeEvent, ISubmitEvent } from "react-jsonschema-form"; +import { getDefaultFormState } from "react-jsonschema-form/lib/utils"; import { addLocValues, strings } from "../../../../common/strings"; import { IExportFormat, IExportProviderOptions } from "../../../../models/applicationState"; import { ExportProviderFactory } from "../../../../providers/export/exportProviderFactory"; @@ -115,7 +116,8 @@ export default class ExportForm extends React.Component any; - onChange?: (e: IChangeEvent) => any; - onError?: (e: any) => any; - onSubmit?: (e: any) => any; - liveValidate?: boolean; - safeRenderCompletion?: boolean; - } - - export interface IChangeEvent { - edit: boolean; - formData: any; - errors: any[]; - errorSchema: any; - idSchema: any; - status: string; - } - - export default class Form extends React.Component {} -} diff --git a/typings/globals/react-jsonschema-form/typings.json b/typings/globals/react-jsonschema-form/typings.json deleted file mode 100644 index 7a7a41b..0000000 --- a/typings/globals/react-jsonschema-form/typings.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "resolution": "main", - "tree": { - "src": "https://raw.githubusercontent.com/DefinitelyTyped/DefinitelyTyped/46d11bc6ab4a0a9b711b403662dceefb67ebb575/react-jsonschema-form/index.d.ts", - "raw": "registry:dt/react-jsonschema-form#0.43.0+20170226152137", - "typings": "https://raw.githubusercontent.com/DefinitelyTyped/DefinitelyTyped/46d11bc6ab4a0a9b711b403662dceefb67ebb575/react-jsonschema-form/index.d.ts" - } -} diff --git a/typings/index.d.ts b/typings/index.d.ts deleted file mode 100644 index bc2855e..0000000 --- a/typings/index.d.ts +++ /dev/null @@ -1 +0,0 @@ -/// diff --git a/typings/react-jsonschema-form/index.d.ts b/typings/react-jsonschema-form/index.d.ts new file mode 100644 index 0000000..4487425 --- /dev/null +++ b/typings/react-jsonschema-form/index.d.ts @@ -0,0 +1,3 @@ +declare module "react-jsonschema-form/lib/utils" { + export function getDefaultFormState(schema: any, formData: T): T; +}