Fix dynamic connector extensions & update internal testing (#2146)

Fix dynamic connector extensions management in making 'parameters' optional despite the spec which says it's required Update internal tests Add Guardian files Update .gitignore file Fix BuildLocalPackages.cmd script
microsoft · Jan 16, 2024 · cdcef30 · cdcef30
1 parent 382540a
commit cdcef30
Show file tree

Hide file tree

Showing 19 changed files with 369 additions and 289 deletions.
diff --git a/.gdn/.gdnsettings b/.gdn/.gdnsettings
@@ -0,0 +1,5 @@
+{
+  "jobs": [],
+  "commands": [],
+  "tools": []
+}
diff --git a/.gdn/.gitignore b/.gdn/.gitignore
@@ -0,0 +1,11 @@
+## Ignore Guardian internal files
+.r/
+rc/
+rs/
+i/
+p/
+c/
+o/
+
+## Ignore Guardian Local settings
+LocalSettings.gdn.json
diff --git a/.gitignore b/.gitignore
@@ -364,3 +364,11 @@ suppress.*.xml
 # global.json
 /src/global.json
 /global.json
+
+# Guardian results
+.gdn/r
+.gdn/internal.gdnhistory
+
+# Yaml output
+YamlOutput/
+
diff --git a/src/buildLocalPackages.cmd b/src/buildLocalPackages.cmd
@@ -2,11 +2,10 @@
 setlocal enabledelayedexpansion
 
 set MSBUILDARGS=-p:PublishRepositoryUrl=true -p:GeneratePackages=true -p:IncludeSymbols=true -p:SymbolPackageFormat=snupkg -p:InternalBuild=true -p:Configuration=Debug -p:Platform="Any CPU"
+set MSBUILD=C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Current\Bin\amd64\MSBuild.exe
 
-for /f "usebackq tokens=*" %%i in (`"%programfiles(x86)%\Microsoft Visual Studio\Installer\vswhere" -latest -requires Microsoft.Component.MSBuild -find MSBuild\**\Bin\MSBuild.exe`) do (
-    @REM Restore dependencies first if needed
-    "%%i" -t:restore
-    @REM Run build and generate nuget packages 
-    "%%i" %MSBUILDARGS%
-    exit /b !errorlevel!
-)
+@REM Restore dependencies first if needed
+"%MSBUILD%" -t:restore
+
+@REM Run build and generate nuget packages 
+"%MSBUILD%" %MSBUILDARGS%
diff --git a/src/libraries/Microsoft.PowerFx.Connectors/OpenApiExtensions.cs b/src/libraries/Microsoft.PowerFx.Connectors/OpenApiExtensions.cs
@@ -773,23 +773,22 @@ internal static ConnectorDynamicValue GetDynamicValue(this IOpenApiExtensible pa
             // https://learn.microsoft.com/en-us/connectors/custom-connectors/openapi-extensions#use-dynamic-values
             if (param?.Extensions != null && param.Extensions.TryGetValue(XMsDynamicValues, out IOpenApiExtension ext) && ext is OpenApiObject apiObj)
             {
-                if (apiObj.TryGetValue("parameters", out IOpenApiAny op_prms) && op_prms is OpenApiObject opPrms)
-                {
-                    ConnectorDynamicValue cdv = new (opPrms);
-
-                    // Mandatory operationId for connectors, except when capibility or builtInOperation are defined
-                    apiObj.WhenPresent("operationId", (opId) => cdv.OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId));                    
-                    apiObj.WhenPresent("value-title", (opValTitle) => cdv.ValueTitle = opValTitle);
-                    apiObj.WhenPresent("value-path", (opValPath) => cdv.ValuePath = opValPath);
-                    apiObj.WhenPresent("value-collection", (opValCollection) => cdv.ValueCollection = opValCollection);
+                // Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
+                OpenApiObject op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is OpenApiObject apiString ? apiString : null;
+                ConnectorDynamicValue cdv = new (op_prms);
+
+                // Mandatory operationId for connectors, except when capibility or builtInOperation are defined
+                apiObj.WhenPresent("operationId", (opId) => cdv.OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId));                    
+                apiObj.WhenPresent("value-title", (opValTitle) => cdv.ValueTitle = opValTitle);
+                apiObj.WhenPresent("value-path", (opValPath) => cdv.ValuePath = opValPath);
+                apiObj.WhenPresent("value-collection", (opValCollection) => cdv.ValueCollection = opValCollection);
 
-                    // we don't support BuiltInOperations or capabilities right now
-                    // return null to indicate that the call to get suggestions is not needed for this parameter
-                    apiObj.WhenPresent("capability", (string op_capStr) => cdv = null);
-                    apiObj.WhenPresent("builtInOperation", (string op_bioStr) => cdv = null);
+                // we don't support BuiltInOperations or capabilities right now
+                // return null to indicate that the call to get suggestions is not needed for this parameter
+                apiObj.WhenPresent("capability", (string op_capStr) => cdv = null);
+                apiObj.WhenPresent("builtInOperation", (string op_bioStr) => cdv = null);
 
-                    return cdv;
-                }
+                return cdv;                
             }
 
             return null;
@@ -800,33 +799,32 @@ internal static ConnectorDynamicList GetDynamicList(this IOpenApiExtensible para
             // https://learn.microsoft.com/en-us/connectors/custom-connectors/openapi-extensions#use-dynamic-values
             if (param?.Extensions != null && param.Extensions.TryGetValue(XMsDynamicList, out IOpenApiExtension ext) && ext is OpenApiObject apiObj)
             {
-                // Mandatory openrationId for connectors
+                // Mandatory operationId for connectors
                 if (apiObj.TryGetValue("operationId", out IOpenApiAny op_id) && op_id is OpenApiString opId)
                 {
-                    if (apiObj.TryGetValue("parameters", out IOpenApiAny op_prms) && op_prms is OpenApiObject opPrms)
+                    // Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
+                    OpenApiObject op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is OpenApiObject apiString ? apiString : null;
+                    ConnectorDynamicList cdl = new (op_prms)
                     {
-                        ConnectorDynamicList cdl = new (opPrms)
-                        {
-                            OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),                            
-                        };
+                        OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),
+                    };
 
-                        if (apiObj.TryGetValue("itemTitlePath", out IOpenApiAny op_valtitle) && op_valtitle is OpenApiString opValTitle)
-                        {
-                            cdl.ItemTitlePath = opValTitle.Value;
-                        }
-
-                        if (apiObj.TryGetValue("itemsPath", out IOpenApiAny op_valpath) && op_valpath is OpenApiString opValPath)
-                        {
-                            cdl.ItemPath = opValPath.Value;
-                        }
+                    if (apiObj.TryGetValue("itemTitlePath", out IOpenApiAny op_valtitle) && op_valtitle is OpenApiString opValTitle)
+                    {
+                        cdl.ItemTitlePath = opValTitle.Value;
+                    }
 
-                        if (apiObj.TryGetValue("itemValuePath", out IOpenApiAny op_valcoll) && op_valcoll is OpenApiString opValCollection)
-                        {
-                            cdl.ItemValuePath = opValCollection.Value;
-                        }
+                    if (apiObj.TryGetValue("itemsPath", out IOpenApiAny op_valpath) && op_valpath is OpenApiString opValPath)
+                    {
+                        cdl.ItemPath = opValPath.Value;
+                    }
 
-                        return cdl;
+                    if (apiObj.TryGetValue("itemValuePath", out IOpenApiAny op_valcoll) && op_valcoll is OpenApiString opValCollection)
+                    {
+                        cdl.ItemValuePath = opValCollection.Value;
                     }
+
+                    return cdl;                    
                 }
                 else
                 {
@@ -839,7 +837,12 @@ internal static ConnectorDynamicList GetDynamicList(this IOpenApiExtensible para
 
         internal static Dictionary<string, IConnectorExtensionValue> GetParameterMap(this OpenApiObject opPrms, SupportsConnectorErrors errors, bool forceString = false)
         {
-            Dictionary<string, IConnectorExtensionValue> dvParams = new ();            
+            Dictionary<string, IConnectorExtensionValue> dvParams = new ();
+
+            if (opPrms == null)
+            {
+                return dvParams;
+            }
 
             foreach (KeyValuePair<string, IOpenApiAny> prm in opPrms)
             {                
@@ -907,23 +910,23 @@ internal static ConnectorDynamicSchema GetDynamicSchema(this IOpenApiExtensible
             // https://learn.microsoft.com/en-us/connectors/custom-connectors/openapi-extensions#use-dynamic-values
             if (param?.Extensions != null && param.Extensions.TryGetValue(XMsDynamicSchema, out IOpenApiExtension ext) && ext is OpenApiObject apiObj)
             {
-                // Mandatory openrationId for connectors
+                // Mandatory operationId for connectors
                 if (apiObj.TryGetValue("operationId", out IOpenApiAny op_id) && op_id is OpenApiString opId)
                 {
-                    if (apiObj.TryGetValue("parameters", out IOpenApiAny op_prms) && op_prms is OpenApiObject opPrms)
-                    {
-                        ConnectorDynamicSchema cds = new (opPrms)
-                        {
-                            OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),                            
-                        };
+                    // Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
+                    OpenApiObject op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is OpenApiObject apiString ? apiString : null;
 
-                        if (apiObj.TryGetValue("value-path", out IOpenApiAny op_valpath) && op_valpath is OpenApiString opValPath)
-                        {
-                            cds.ValuePath = opValPath.Value;
-                        }
+                    ConnectorDynamicSchema cds = new (op_prms)
+                    {
+                        OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),
+                    };
 
-                        return cds;
+                    if (apiObj.TryGetValue("value-path", out IOpenApiAny op_valpath) && op_valpath is OpenApiString opValPath)
+                    {
+                        cds.ValuePath = opValPath.Value;
                     }
+
+                    return cds;                    
                 }
                 else
                 {
@@ -939,23 +942,23 @@ internal static ConnectorDynamicProperty GetDynamicProperty(this IOpenApiExtensi
             // https://learn.microsoft.com/en-us/connectors/custom-connectors/openapi-extensions#use-dynamic-values
             if (param?.Extensions != null && param.Extensions.TryGetValue(XMsDynamicProperties, out IOpenApiExtension ext) && ext is OpenApiObject apiObj)
             {
-                // Mandatory openrationId for connectors
+                // Mandatory operationId for connectors
                 if (apiObj.TryGetValue("operationId", out IOpenApiAny op_id) && op_id is OpenApiString opId)
                 {
-                    if (apiObj.TryGetValue("parameters", out IOpenApiAny op_prms) && op_prms is OpenApiObject opPrms)
-                    {
-                        ConnectorDynamicProperty cdp = new (opPrms)
-                        {
-                            OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),                            
-                        };
+                    // Parameters is required in the spec but there are examples where it's not specified and we'll support this condition with an empty list
+                    OpenApiObject op_prms = apiObj.TryGetValue("parameters", out IOpenApiAny openApiAny) && openApiAny is OpenApiObject apiString ? apiString : null;
 
-                        if (apiObj.TryGetValue("itemValuePath", out IOpenApiAny op_valpath) && op_valpath is OpenApiString opValPath)
-                        {
-                            cdp.ItemValuePath = opValPath.Value;
-                        }
+                    ConnectorDynamicProperty cdp = new (op_prms)
+                    {
+                        OperationId = OpenApiHelperFunctions.NormalizeOperationId(opId.Value),                            
+                    };
 
-                        return cdp;
+                    if (apiObj.TryGetValue("itemValuePath", out IOpenApiAny op_valpath) && op_valpath is OpenApiString opValPath)
+                    {
+                        cdp.ItemValuePath = opValPath.Value;
                     }
+
+                    return cdp;                    
                 }
                 else
                 {

diff --git a/src/libraries/Microsoft.PowerFx.Connectors/OpenApiParser.cs b/src/libraries/Microsoft.PowerFx.Connectors/OpenApiParser.cs
@@ -84,7 +84,7 @@ internal static IEnumerable<ConnectorFunction> GetFunctionsInternal(ConnectorSet
             {
                 configurationLogger?.LogError($"{nameof(openApiDocument)} is null");
                 return functions;
-            }
+            }            
 
             if (!ValidateSupportedOpenApiDocument(openApiDocument, ref connectorIsSupported, ref connectorNotSupportedReason, connectorSettings.FailOnUnknownExtension, configurationLogger))
             {
@@ -260,7 +260,11 @@ private static bool ValidateSupportedOpenApiDocument(OpenApiDocument openApiDocu
                 // openApiDocument.Components.RequestBodies is ok
                 // openApiDocument.Components.Responses contains references from "path" definitions
                 // openApiDocument.Components.Schemas contains global "definitions"
-                // openApiDocument.Components.SecuritySchemes are critical but as we don't manage them at all, we'll ignore this parameter                
+
+                if (openApiDocument.Components.SecuritySchemes.Count > 0)
+                {                                        
+                    logger?.LogInformation($"Unsupported document: {notSupportedReason}");
+                }
             }
 
             if (isSupported && failOnUnknownExtensions)

diff --git a/src/tests/Microsoft.PowerFx.Connectors.Tests/BaseConnectorTest.cs b/src/tests/Microsoft.PowerFx.Connectors.Tests/BaseConnectorTest.cs
@@ -23,12 +23,12 @@ namespace Microsoft.PowerFx.Connectors.Tests
 {
     public abstract class BaseConnectorTest : PowerFxTest, IDisposable
     {
-        internal ITestOutputHelper _output;                
+        internal ITestOutputHelper _output;
         internal string _swaggerFile;
         private bool _disposedValue;
 
         public BaseConnectorTest(ITestOutputHelper output, string swaggerFile)
-        {            
+        {
             _swaggerFile = swaggerFile;
             _output = output;
         }
@@ -43,11 +43,11 @@ public BaseConnectorTest(ITestOutputHelper output, string swaggerFile)
 
         public virtual ConnectorSettings GetConnectorSettings(bool returnUnknownRecordFieldsAsUntypedObjects = false)
         {
-            return new ConnectorSettings(GetNamespace()) 
-            { 
-                Compatibility = returnUnknownRecordFieldsAsUntypedObjects ? ConnectorCompatibility.SwaggerCompatibility : ConnectorCompatibility.PowerAppsCompatibility, 
-                IncludeInternalFunctions = true, 
-                ReturnUnknownRecordFieldsAsUntypedObjects = returnUnknownRecordFieldsAsUntypedObjects 
+            return new ConnectorSettings(GetNamespace())
+            {
+                Compatibility = returnUnknownRecordFieldsAsUntypedObjects ? ConnectorCompatibility.SwaggerCompatibility : ConnectorCompatibility.PowerAppsCompatibility,
+                IncludeInternalFunctions = true,
+                ReturnUnknownRecordFieldsAsUntypedObjects = returnUnknownRecordFieldsAsUntypedObjects
             };
         }
 
@@ -73,10 +73,10 @@ internal IReadOnlyList<ConnectorFunction> EnumerateFunctions()
 
         internal (LoggingTestServer testConnector, OpenApiDocument apiDoc, PowerFxConfig config, HttpClient httpClient, PowerPlatformConnectorClient client, ConnectorSettings connectorSettings, RuntimeConfig runtimeConfig) GetElements(bool live = false, bool returnUO = false)
         {
-            var testConnector = new LoggingTestServer(_swaggerFile, live);                                  
+            var testConnector = new LoggingTestServer(_swaggerFile, _output, live);
             HttpClient httpClient = new HttpClient(testConnector);
             PowerPlatformConnectorClient client = new PowerPlatformConnectorClient(GetEndpoint(), GetEnvironment(), GetConnectionId(), () => GetJWTToken(), httpClient) { SessionId = "9315f316-5182-4260-b333-7a43a36ca3b0" };
-            
+
             PowerFxConfig config = new PowerFxConfig();
             OpenApiDocument apiDoc = testConnector._apiDocument;
             ConnectorSettings connectorSettings = GetConnectorSettings(returnUO);
@@ -108,7 +108,7 @@ internal async Task RunConnectorTestAsync(bool live, string expr, string expecte
                     : ($@"Responses\{ef.Substring(4)}", (HttpStatusCode)int.Parse(ef.Substring(0, 3)))).ToArray());
             }
 
-            FormulaValue fv = await engine.EvalAsync(expr, CancellationToken.None, options: new ParserOptions() { AllowsSideEffects = true }, runtimeConfig: runtimeConfig).ConfigureAwait(false);           
+            FormulaValue fv = await engine.EvalAsync(expr, CancellationToken.None, options: new ParserOptions() { AllowsSideEffects = true }, runtimeConfig: runtimeConfig).ConfigureAwait(false);
 
             string network = testConnector._log.ToString();
             string urls = string.Join("|", Regex.Matches(network, @"x-ms-request-method: (?<r>[^ \r\n]+)\s*x-ms-request-url: (?<u>[^ \r\n]+)").Select(g => $"{g.Groups["r"].Value}:{g.Groups["u"].Value}"));
@@ -150,7 +150,7 @@ internal async Task RunConnectorTestAsync(bool live, string expr, string expecte
             else if (expectedResult.StartsWith("DATETIME:"))
             {
                 Assert.True(fv is not ErrorValue, fv is ErrorValue ev ? $"EvalAsync Error: {string.Join(", ", ev.Errors.Select(er => er.Message))}" : null);
-                DateTimeValue dtv = Assert.IsType<DateTimeValue>(fv);                                
+                DateTimeValue dtv = Assert.IsType<DateTimeValue>(fv);
 
                 Assert.Equal(DateTime.Parse(expectedResult.Substring(9)).ToUniversalTime(), new ConvertToUTC(GetTimeZoneInfo()).ToUTC(dtv));
             }
@@ -171,8 +171,8 @@ internal async Task RunConnectorTestAsync(bool live, string expr, string expecte
             }
 
             Assert.Equal(xUrls, urls);
-            Assert.Equal(xBodies, bodies);     
-            
+            Assert.Equal(xBodies, bodies);
+
             if (!string.IsNullOrEmpty(extra))
             {
                 foreach (string e in extra.Split("|"))
@@ -237,7 +237,7 @@ protected virtual void Dispose(bool disposing)
             if (!_disposedValue)
             {
                 if (disposing)
-                {                    
+                {
                 }
 
                 _disposedValue = true;