Merge pull request #1251 from microsoft/vishwac/1250

[Ludown] Inline and explicit role definitions are handled correctly

packages/Ludown/lib/parseFileContents.js

@@ -570,7 +570,7 @@ const parseAndHandleEntity = function (parsedContent, chunkSplitByLine, locale,
         try {
             let rolesImport = VerifyAndUpdateSimpleEntityCollection(parsedContent, entityType, entityName);
             if (rolesImport.length !== 0) {
-                rolesImport.forEach(role => entityRoles.push(role));
+                rolesImport.forEach(role => !entityRoles.includes(role) ? entityRoles.push(role) : undefined);
             }
         } catch (err) { 
             throw (err);
@@ -600,7 +600,7 @@ const parseAndHandleEntity = function (parsedContent, chunkSplitByLine, locale,
             try {
                 let rolesImport = VerifyAndUpdateSimpleEntityCollection(parsedContent, entityName, 'RegEx');
                 if (rolesImport.length !== 0) {
-                    rolesImport.forEach(role => entityRoles.push(role));
+                    rolesImport.forEach(role => !entityRoles.includes(role) ? entityRoles.push(role) : undefined);
                 }
             } catch (err) { 
                 throw (err);
@@ -651,7 +651,7 @@ const parseAndHandleEntity = function (parsedContent, chunkSplitByLine, locale,
         try {
             let rolesImport = VerifyAndUpdateSimpleEntityCollection(parsedContent, entityName, 'Phrase List');
             if (rolesImport.length !== 0) {
-                rolesImport.forEach(role => entityRoles.push(role));
+                rolesImport.forEach(role => !entityRoles.includes(role) ? entityRoles.push(role) : undefined);
             }
         } catch (err) { 
             throw (err);
@@ -755,9 +755,7 @@ const VerifyAndUpdateSimpleEntityCollection = function (parsedContent, entityNam
     let simpleEntityExists = (parsedContent.LUISJsonStructure.entities || []).find(item => item.name == entityName);
     if (simpleEntityExists !== undefined) { 
         // take and add any roles into the roles list
-        (simpleEntityExists.roles || []).forEach(role => {
-            if (!entityRoles.includes(role)) entityRoles.push(role)
-        });
+        (simpleEntityExists.roles || []).forEach(role => !entityRoles.includes(role) ? entityRoles.push(role) : undefined);
         // remove this simple entity definition
         // Fix for #1137.
         // Current behavior does not allow for simple and phrase list entities to have the same name. 
@@ -830,11 +828,7 @@ const parseAndHandleListEntity = function (parsedContent, chunkSplitByLine, enti
     let entityName = entityDef.split(':')[0].trim();
     let parsedEntityTypeAndRole = helpers.getRolesAndType(entityType);
     entityType = parsedEntityTypeAndRole.entityType;
-    (parsedEntityTypeAndRole.roles || []).forEach(role => {
-        if (!entityRoles.includes(role)) {
-            entityRoles.push(role)
-        }
-     });
+    (parsedEntityTypeAndRole.roles || []).forEach(role => !entityRoles.includes(role) ? entityRoles.push(role) : undefined);
     // check if this list entity is already labelled in an utterance and or added as a simple entity. if so, throw an error.
     try {
         let rolesImport = VerifyAndUpdateSimpleEntityCollection(parsedContent, entityName, 'List');

packages/Ludown/test/ludown.roleTests.test.suite.js

@@ -909,4 +909,81 @@ $test:[fromTime]`;
 
     })
 
+    it ('prebuilt entities with inline as well as explicit role definition is handled correctly', function(done){
+        let testLU = `> # Intent definitions
+
+        ## Intent
+        - holiday request to {datetimeV2:to=next day}
+        - holiday request vacation from {datetimeV2:from=today}
+        - i want vacation from {datetimeV2:from} until {datetimeV2:to}
+        
+        > # Entity definitions
+        
+        > # PREBUILT Entity definitions
+        
+        $PREBUILT:datetimeV2 Roles=to, from`;
+        parser.parseFile(testLU, false, null) 
+            .then (res => {
+                let LUISJSon = res.LUISJsonStructure;
+                assert(LUISJSon.prebuiltEntities.length, 1);
+                assert(LUISJSon.prebuiltEntities[0].roles.length, 2);
+                assert.deepEqual(LUISJSon.prebuiltEntities[0].roles, ['from', 'to']);
+                done();
+            })
+            .catch (err => done(`Test failed - ${JSON.stringify(err)}`))
+
+    })
+
+    it ('regex entities with inline as well as explicit role definition is handled correctly', function(done){
+        let testLU = `> # Intent definitions
+
+        ## Intent
+        - holiday request to {regex1:to=32}
+        - holiday request vacation from {regex1:from=today}
+        - i want vacation from {regex1:from} until {regex1:to}
+        
+        > # Entity definitions
+        
+        > # PREBUILT Entity definitions
+        
+        $regex1:/[0-9]/ Roles=to, from`;
+        parser.parseFile(testLU, false, null) 
+            .then (res => {
+                let LUISJSon = res.LUISJsonStructure;
+                assert(LUISJSon.regex_entities.length, 1);
+                assert(LUISJSon.regex_entities[0].roles.length, 2);
+                assert.deepEqual(LUISJSon.regex_entities[0].roles, ['from', 'to']);
+                done();
+            })
+            .catch (err => done(`Test failed - ${JSON.stringify(err)}`))
+
+    })
+
+    it ('closed list entities with inline as well as explicit role definition is handled correctly', function(done){
+        let testLU = `> # Intent definitions
+
+        ## Intent
+        - holiday request to {list1:to=32}
+        - holiday request vacation from {list1:from=today}
+        - i want vacation from {list1:from} until {list1:to}
+        
+        > # Entity definitions
+        
+        > # PREBUILT Entity definitions
+        
+        $list1: a = Roles = to, from
+            - 32
+        `;
+        parser.parseFile(testLU, false, null) 
+            .then (res => {
+                let LUISJSon = res.LUISJsonStructure;
+                assert(LUISJSon.closedLists.length, 1);
+                assert(LUISJSon.closedLists[0].roles.length, 2);
+                assert.deepEqual(LUISJSon.closedLists[0].roles, ['from', 'to']);
+                done();
+            })
+            .catch (err => done(`Test failed - ${JSON.stringify(err)}`))
+
+    })
+
 });