Merge pull request #3486 from microsoft/feature/southworks/fix-botski…

…lls-endpoint-validation

[Botskills] Fix RegExp for validating URLs

tools/botskills/src/functionality/connectSkill.ts

@@ -51,10 +51,10 @@ Remember to use the argument '--luisFolder' for your Skill's LUIS folder.`);
         let luisFilePath = '';
         let dispatchFolderPath = '';
         let dispatchFilePath = '';
-        let useAllIntents: boolean = false;
+        let useAllIntents = false;
 
         dispatchFolderPath = join(this.configuration.dispatchFolder, culture);
-        dispatchFilePath = join(dispatchFolderPath, `${dispatchName}.dispatch`);
+        dispatchFilePath = join(dispatchFolderPath, `${ dispatchName }.dispatch`);
 
         // Validate 'dispatch add' arguments
         if (!existsSync(dispatchFolderPath)) {
@@ -65,16 +65,16 @@ Remember to use the argument '--dispatchFolder' for your Assistant's Dispatch fo
             throw new Error(`Path to the ${ dispatchName }.dispatch file leads to a nonexistent file.`);
         }
 
-        luFile = `${luisApp}.lu`;
-        luisFile = `${luisApp}.luis`;
+        luFile = `${ luisApp }.lu`;
+        luisFile = `${ luisApp }.luis`;
         luFilePath = join(this.configuration.luisFolder, culture, luFile);
         luisFilePath = join(luisFolderPath, luisFile);
 
         if (!existsSync(luFilePath)) {
             if (this.manifest !== undefined && this.manifest.entries !== undefined) {
 
                 const model: IModel = {id: '', name: '', contentType: '', url: '', description: ''};
-                const currentLocaleApps = this.manifest.entries.find((entry: [string, IModel[]]): boolean => entry[0] === culture) || [model]
+                const currentLocaleApps = this.manifest.entries.find((entry: [string, IModel[]]): boolean => entry[0] === culture) || [model];
                 const localeApps: IModel[] = currentLocaleApps[1];
                 const currentApp: IModel = localeApps.find((model: IModel): boolean => model.id === luisApp) || model;
 
@@ -140,7 +140,7 @@ Make sure your Skill's .lu file's name matches your Skill's manifest id`);
             useAllIntents = this.manifest.allowedIntents.some(e => e === '*');
 
             if (useAllIntents && this.manifest.allowedIntents.length > 1) {
-                this.logger.warning("Found intent with name '*'. Adding all intents.");
+                this.logger.warning('Found intent with name \'*\'. Adding all intents.');
             }
 
             if (!useAllIntents && this.manifest.allowedIntents.length > 0) {

tools/botskills/src/utils/manifestUtils.ts

@@ -67,7 +67,7 @@ Please make sure to provide a valid path to your Skill manifest using the '--loc
             version: '',
             schema: '',
             allowedIntents: ['*']
-        }
+        };
     }
 
     private async getManifestFromV2(manifest: ISkillManifestV2, logger: ILogger, endpointName?: string): Promise<IManifest> {
@@ -89,7 +89,7 @@ Please make sure to provide a valid path to your Skill manifest using the '--loc
             schema: manifest.$schema,
             entries: Object.entries(manifest?.dispatchModels.languages),
             allowedIntents: Object.keys(manifest?.dispatchModels.intents)
-        }
+        };
     }
 
     private async processManifestV1(manifest: ISkillManifestV1): Promise<Map<string, string[]>> {
@@ -122,7 +122,7 @@ Please make sure to provide a valid path to your Skill manifest using the '--loc
                 luisApps.push(model.id);
             });
 
-            const filteredluisApps: string[] = [...new Set(luisApps)]
+            const filteredluisApps: string[] = [...new Set(luisApps)];
             acc.set(locale, filteredluisApps);
         });
 

tools/botskills/src/utils/validationUtils.ts

@@ -46,19 +46,19 @@ export function isValidCultures(availableCultures: string[], targetedCultures: s
 }
 
 export function manifestV1Validation(skillManifest: ISkillManifestV1, logger: ILogger): void {
-    if (skillManifest.name === undefined || skillManifest.name === "") {
+    if (skillManifest.name === undefined || skillManifest.name === '') {
         logger.error(`Missing property 'name' of the manifest`);
     }
 
-    if (skillManifest.id === undefined || skillManifest.id === "") {
+    if (skillManifest.id === undefined || skillManifest.id === '') {
         logger.error(`Missing property 'id' of the manifest`);
     } else if (skillManifest.id.match(/^\d|[^\w]/g) !== null) {
         logger.error(`The 'id' of the manifest contains some characters not allowed. Make sure the 'id' contains only letters, numbers and underscores, but doesn't start with number.`);
     }
 
-    if (skillManifest.endpoint === undefined || skillManifest.endpoint === "") {
+    if (skillManifest.endpoint === undefined || skillManifest.endpoint === '') {
         logger.error(`Missing property 'endpoint' of the manifest`);
-    } else if (skillManifest.endpoint.match(/^(https?:\/\/)?((([a-zA-F\d]([a-zA-F\d-]*[a-zA-F\d])*)\.)+[a-zA-F]{2,}|(((\d{1,3}\.){3}\d{1,3})|(localhost))(\:\d+)?)(\/[-a-zA-F\d%_.~+]*)*(\?[;&a-zA-F\d%_.~+=-]*)?(\#[-a-zA-F\d_]*)?$/g) === null) {
+    } else if (skillManifest.endpoint.match(/^(ht|f)tp(s?)\:\/\/[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\.\?\,\'\/\\\+&%\$#_]*)?$/g) === null) {
         logger.error(`The 'endpoint' property contains some characters not allowed.`);
     }
 
@@ -73,11 +73,11 @@ export function manifestV1Validation(skillManifest: ISkillManifestV1, logger: IL
 }
 
 export function manifestV2Validation(skillManifest: ISkillManifestV2, logger: ILogger, endpointName?: string): void {
-    if (skillManifest.$schema === undefined || skillManifest.$schema === "") {
+    if (skillManifest.$schema === undefined || skillManifest.$schema === '') {
         logger.error(`Missing property '$schema' of the manifest`);
     }
 
-    if (skillManifest.$id === undefined || skillManifest.$id === "") {
+    if (skillManifest.$id === undefined || skillManifest.$id === '') {
         logger.error(`Missing property '$id' of the manifest`);
     } else if (skillManifest.$id.match(/^\d|[^\w]/g) !== null) {
         logger.error(`The '$id' of the manifest contains some characters not allowed. Make sure the '$id' contains only letters, numbers and underscores, but doesn't start with number.`);
@@ -87,19 +87,19 @@ export function manifestV2Validation(skillManifest: ISkillManifestV2, logger: IL
         logger.error(`Missing property 'endpoints' of the manifest`);
     } else {
         let currentEndpoint = skillManifest.endpoints.find((endpoint): boolean =>  endpoint.name == endpointName) || skillManifest.endpoints[0];
-        if (currentEndpoint.name === undefined || currentEndpoint.name === ""){
+        if (currentEndpoint.name === undefined || currentEndpoint.name === ''){
             logger.error(`Missing property 'name' at the selected endpoint. If you didn't select any endpoint, the first one is taken by default`);
         }
 
-        if (currentEndpoint.msAppId === undefined || currentEndpoint.msAppId === ""){
+        if (currentEndpoint.msAppId === undefined || currentEndpoint.msAppId === ''){
             logger.error(`Missing property 'msAppId' at the selected endpoint. If you didn't select any endpoint, the first one is taken by default`);
-        } else if (currentEndpoint.msAppId.match(/^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$/g) === null) {
+        } else if (currentEndpoint.msAppId.match(/^[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}$/g) === null) {
             logger.error(`The 'msAppId' property of the selected endpoint contains invalid characters or does not comply with the GUID format. If you didn't select any endpoint, the first one is taken by default.`);
         }
 
-        if (currentEndpoint.endpointUrl === undefined || currentEndpoint.endpointUrl === ""){
+        if (currentEndpoint.endpointUrl === undefined || currentEndpoint.endpointUrl === ''){
             logger.error(`Missing property 'endpointUrl' at the selected endpoint. If you didn't select any endpoint, the first one is taken by default`);
-        } else if (currentEndpoint.endpointUrl.match(/^(https?:\/\/)?((([a-zA-F\d]([a-zA-F\d-]*[a-zA-F\d])*)\.)+[a-zA-F]{2,}|(((\d{1,3}\.){3}\d{1,3})|(localhost))(\:\d+)?)(\/[-a-zA-F\d%_.~+]*)*(\?[;&a-zA-F\d%_.~+=-]*)?(\#[-a-zA-F\d_]*)?$/g) === null) {
+        } else if (currentEndpoint.endpointUrl.match(/^(ht|f)tp(s?)\:\/\/[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\.\?\,\'\/\\\+&%\$#_]*)?$/g) === null) {
             logger.error(`The 'endpointUrl' property of the selected endpoint contains invalid characters or does not comply with the URL format. If you didn't select any endpoint, the first one is taken by default.`);
         }
     }
@@ -132,6 +132,6 @@ interface libraryCommand {
     package: string;
 }
 
-const commands: { [key: number]: libraryCommand; } = {
+const commands: { [key: number]: libraryCommand } = {
     0: { cmd: 'bf', args: ['-v'], package: 'https://www.npmjs.com/package/@microsoft/botframework-cli' }
 };

tools/botskills/test/connect.test.js

@@ -289,7 +289,7 @@ Error: Path to the nonExistenceen-usDispatch.dispatch file leads to a nonexisten
             strictEqual(errorList[errorList.length - 1], `There was an error while connecting the Skill to the Assistant:
 Error: An error ocurred while updating the Dispatch model:
 Error: There was an error in the bf luis:convert command:
-Command: bf luis:convert --in "${join(configuration.luisFolder, configuration.languages[0], "testSkill.lu")}" --culture ${configuration.languages[0]} --out ${join(configuration.luisFolder, configuration.languages[0], 'testskill.luis')} --name testSkill
+Command: bf luis:convert --in "${join(configuration.luisFolder, configuration.languages[0], "testSkill.lu")}" --culture ${configuration.languages[0]} --out ${join(configuration.luisFolder, configuration.languages[0], 'testskill.luis')} --name testSkill --force
 Error: Path to testskill.luis (${join(configuration.luisFolder, configuration.languages[0], "testskill.luis")}) leads to a nonexistent file.`);
         });
 
@@ -380,7 +380,7 @@ Make sure you have a Dispatch for the cultures you are trying to connect, and th
             strictEqual(errorList[errorList.length - 1], `There was an error while connecting the Skill to the Assistant:
 Error: An error ocurred while updating the Dispatch model:
 Error: There was an error in the bf luis:convert command:
-Command: bf luis:convert --in "${join(configuration.luisFolder, configuration.languages[0], "testSkill.lu")}" --culture ${configuration.languages[0]} --out ${join(configuration.luisFolder, configuration.languages[0], 'testskill.luis')} --name testSkill
+Command: bf luis:convert --in "${join(configuration.luisFolder, configuration.languages[0], "testSkill.lu")}" --culture ${configuration.languages[0]} --out ${join(configuration.luisFolder, configuration.languages[0], 'testskill.luis')} --name testSkill --force
 Error: The execution of the bf command failed with the following error:
 Error: Mocked function throws an Error`);
 		});
@@ -420,7 +420,7 @@ Error: Mocked function throws an Error`);
         it("The localManifest V1 points to a nonexisting Endpoint URL", async function() {
             const configuration = {
                 botName: "",
-                localManifest: resolve(__dirname, join("mocks", "skills", "invalidEndpoint.json")),
+                localManifest: resolve(__dirname, join("mocks", "manifests", "v1", "invalidEndpoint.json")),
                 remoteManifest: "",
                 languages: "",
                 luisFolder: "",
@@ -436,7 +436,7 @@ Error: Mocked function throws an Error`);
 
             const errorMessages = [
                 `Missing property 'endpoint' of the manifest`,
-                `There was an error while connecting the Skill to the Assistant:\nError: Your Skill Manifest is not compatible. Please note that the minimum supported manifest version is 2.1.`
+                `There was an error while connecting the Skill to the Assistant:\nError: One or more properties are missing from your Skill Manifest`
             ]
 
             this.connector.configuration = configuration;
@@ -447,42 +447,11 @@ Error: Mocked function throws an Error`);
                 strictEqual(errorMessage, errorMessages[index]);
             });
         });
-
-        it("The localManifest V1 points to a Endpoint URL with uppercase scenario", async function() {
-            const configuration = {
-                botName: "",
-                localManifest: resolve(__dirname, join("mocks", "skills", "invalidEndpointManifest.json")),
-                remoteManifest: "",
-                languages: "",
-                luisFolder: "",
-                dispatchFolder: "",
-                outFolder: "",
-                lgOutFolder: "",
-                resourceGroup: "",
-                appSettingsFile: "",
-                cognitiveModelsFile : resolve(__dirname, "mocks", "cognitivemodels", "cognitivemodelsWithTwoDispatch.json"),
-                lgLanguage: "",
-                logger: this.logger
-            };
-
-            const errorMessages = [
-                `The 'endpoint' property contains some characters not allowed.`,
-                `There was an error while connecting the Skill to the Assistant:\nError: Your Skill Manifest is not compatible. Please note that the minimum supported manifest version is 2.1.`
-            ]
-
-            this.connector.configuration = configuration;
-            await this.connector.connectSkill();
-            const errorList = this.logger.getError();
-
-            errorList.forEach((errorMessage, index) => {
-                strictEqual(errorMessage, errorMessages[index]);
-            });
-        });
-
+
         it("The localManifest V2 points to a nonexisting Endpoint URL", async function() {
             const configuration = {
                 botName: "",
-                localManifest: resolve(__dirname, join("mocks", "skills", "invalidEndpointV2.json")),
+                localManifest: resolve(__dirname, join("mocks", "manifests", "v2", "invalidEndpoint.json")),
                 remoteManifest: "",
                 languages: "",
                 luisFolder: "",
@@ -498,38 +467,7 @@ Error: Mocked function throws an Error`);
 
             const errorMessages = [
                 `Missing property 'endpointUrl' at the selected endpoint. If you didn't select any endpoint, the first one is taken by default`,
-                `There was an error while connecting the Skill to the Assistant:\nError: Your Skill Manifest is not compatible. Please note that the minimum supported manifest version is 2.1.`
-            ]
-
-            this.connector.configuration = configuration;
-            await this.connector.connectSkill();
-            const errorList = this.logger.getError();
-
-            errorList.forEach((errorMessage, index) => {
-                strictEqual(errorMessage, errorMessages[index]);
-            });
-        });
-
-        it("The localManifest V2 points to a Endpoint URL with uppercase scenario", async function() {
-            const configuration = {
-                botName: "",
-                localManifest: resolve(__dirname, join("mocks", "skills", "invalidEndpointManifestV2.json")),
-                remoteManifest: "",
-                languages: "",
-                luisFolder: "",
-                dispatchFolder: "",
-                outFolder: "",
-                lgOutFolder: "",
-                resourceGroup: "",
-                appSettingsFile: "",
-                cognitiveModelsFile : resolve(__dirname, "mocks", "cognitivemodels", "cognitivemodelsWithTwoDispatch.json"),
-                lgLanguage: "",
-                logger: this.logger
-            };
-
-            const errorMessages = [
-                `The 'endpointUrl' property contains some characters not allowed at the selected endpoint. If you didn't select any endpoint, the first one is taken by default.`,
-                `There was an error while connecting the Skill to the Assistant:\nError: Your Skill Manifest is not compatible. Please note that the minimum supported manifest version is 2.1.`
+                `There was an error while connecting the Skill to the Assistant:\nError: One or more properties are missing from your Skill Manifest`
             ]
 
             this.connector.configuration = configuration;

tools/botskills/test/refresh.test.js

@@ -144,7 +144,7 @@ Error: Mocked function throws an Error`);
 
             strictEqual(errorList[errorList.length - 1], `There was an error while refreshing any Skill from the Assistant:
 Error: There was an error in the bf luis:generate:${configuration.lgLanguage} command:
-Command: bf luis:generate:${configuration.lgLanguage} --in "${configuration.dispatchFolder}\\en-us\\filleden-usDispatch.json" --out "${configuration.lgOutFolder}" --force
+Command: bf luis:generate:${configuration.lgLanguage} --in "${configuration.dispatchFolder}\\en-us\\filleden-usDispatch.json" --out "${configuration.lgOutFolder}" --className DispatchLuis --force
 Error: Mocked function throws an Error`);
         });
     });