Skip to content

Commit f88a938

Browse files
authored
Enforces default Top for CDP Connector for non delegating expression. (#2904)
This pull request introduces a new delegation parameter class to handle `$top` query parameters for paginated data retrieval, updates the `ConnectorSettings` class to support configurable maximum row limits, and integrates these changes into the existing `CdpTable` and `CdpTableValue` implementations. It also includes updates to tests to validate the new functionality. ### Enhancements for pagination and delegation: * **Added `DefaultCDPDelegationParameter` class**: This new class implements delegation parameters to generate OData query strings, including `$top` for row limits. (`src/libraries/Microsoft.PowerFx.Connectors/Tabular/DefaultCDPDelegationParameter.cs`) * **Updated `CdpTableValue.Rows` property**: Modified to use the new `DefaultCDPDelegationParameter` for setting the maximum number of rows via `$top`. (`src/libraries/Microsoft.PowerFx.Connectors/Public/CdpTableValue.cs`) ### Configuration improvements: * **Enhanced `ConnectorSettings`**: Introduced a `DefaultConnectorTop` constant and updated the `MaxRows` property to use this default value, with the option to override it when creating new connector settings. (`src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorSettings.cs`) [[1]](diffhunk://#diff-5b668106d359b647dc2d970cc4c94e742319a83eea6389adb346c39ac208f4b3L14-R26) [[2]](diffhunk://#diff-5b668106d359b647dc2d970cc4c94e742319a83eea6389adb346c39ac208f4b3L41-R47) ### Integration with `CdpTable`: * **Exposed `ConnectorSettings` in `CdpTable`**: Added a new property to provide access to `ConnectorSettings` from `CdpTable`. (`src/libraries/Microsoft.PowerFx.Connectors/Tabular/Services/CdpTable.cs`) * **Utilized delegation parameters in `Query` method**: Updated the query logic to use `DefaultCDPDelegationParameter` for constructing OData query strings with `$top`. (`src/libraries/Microsoft.PowerFx.Connectors/Tabular/Services/CdpTable.cs`) ### Test updates: * **Modified test cases**: Updated tests to validate the correct generation of `$top` query parameters and ensure the new functionality works as expected. (`src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/PowerPlatformTabularTests.cs`) [[1]](diffhunk://#diff-7b1be2cff9606912fdb9b09ff75af4e880829323c9e42a7252a1f93feb1ded67L82-R82) [[2]](diffhunk://#diff-7b1be2cff9606912fdb9b09ff75af4e880829323c9e42a7252a1f93feb1ded67R143-R146)
1 parent 5c8c823 commit f88a938

File tree

7 files changed

+78
-5
lines changed

7 files changed

+78
-5
lines changed

src/libraries/Microsoft.PowerFx.Connectors/Public/CdpTableValue.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Net.Http;
88
using System.Threading;
99
using System.Threading.Tasks;
10+
using Microsoft.PowerFx.Connectors.Tabular;
1011
using Microsoft.PowerFx.Core.Entities;
1112
using Microsoft.PowerFx.Core.IR;
1213
using Microsoft.PowerFx.Types;
@@ -43,7 +44,7 @@ internal CdpTableValue(IRContext irContext)
4344
_cachedRows = null;
4445
}
4546

46-
public override IEnumerable<DValue<RecordValue>> Rows => GetRowsAsync(null, null, CancellationToken.None).ConfigureAwait(false).GetAwaiter().GetResult();
47+
public override IEnumerable<DValue<RecordValue>> Rows => GetRowsAsync(null, new DefaultCDPDelegationParameter(RecordType.ToTable(), _tabularService.ConnectorSettings.MaxRows), CancellationToken.None).ConfigureAwait(false).GetAwaiter().GetResult();
4748

4849
public DelegationParameterFeatures SupportedFeatures => DelegationParameterFeatures.Filter |
4950
DelegationParameterFeatures.Top |

src/libraries/Microsoft.PowerFx.Connectors/Public/ConnectorSettings.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,19 @@ namespace Microsoft.PowerFx.Connectors
1111
[ThreadSafeImmutable]
1212
public class ConnectorSettings
1313
{
14-
public static ConnectorSettings NewCDPConnectorSettings(bool extractSensitivityLabel = false, string purviewAccountName = null)
14+
/// <summary>
15+
/// Default number of rows to return for connector, per page. I.E. $top=1000.
16+
/// </summary>
17+
internal const int DefaultConnectorTop = 1000;
18+
19+
public static ConnectorSettings NewCDPConnectorSettings(bool extractSensitivityLabel = false, string purviewAccountName = null, int maxRows = DefaultConnectorTop)
1520
{
1621
var connectorSettings = new ConnectorSettings(null)
1722
{
1823
Compatibility = ConnectorCompatibility.CdpCompatibility,
1924
SupportXMsEnumValues = true,
2025
ReturnEnumsAsPrimitive = false,
26+
MaxRows = maxRows,
2127
ExtractSensitivityLabel = extractSensitivityLabel,
2228
PurviewAccountName = purviewAccountName
2329
};
@@ -38,7 +44,7 @@ public ConnectorSettings(string @namespace)
3844
/// <summary>
3945
/// Maximum number of rows to return, per page.
4046
/// </summary>
41-
public int MaxRows { get; init; } = 1000;
47+
public int MaxRows { get; init; } = DefaultConnectorTop;
4248

4349
/// <summary>
4450
/// If this is enabled it will extract MIP Labels.
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Text;
7+
using Microsoft.PowerFx.Types;
8+
9+
namespace Microsoft.PowerFx.Connectors.Tabular
10+
{
11+
internal class DefaultCDPDelegationParameter : DelegationParameters
12+
{
13+
private readonly int _maxRows;
14+
15+
public override DelegationParameterFeatures Features => throw new NotImplementedException();
16+
17+
private readonly FormulaType _expectedReturnType;
18+
19+
public override FormulaType ExpectedReturnType => throw new NotImplementedException();
20+
21+
public DefaultCDPDelegationParameter(FormulaType expectedReturnType, int maxRows)
22+
{
23+
_expectedReturnType = expectedReturnType;
24+
_maxRows = maxRows;
25+
}
26+
27+
public override string GetODataApply()
28+
{
29+
return string.Empty;
30+
}
31+
32+
public override string GetOdataFilter()
33+
{
34+
return string.Empty;
35+
}
36+
37+
public override string GetODataQueryString()
38+
{
39+
var sb = new StringBuilder();
40+
sb.Append($"$top={_maxRows}");
41+
return sb.ToString();
42+
}
43+
44+
public override IReadOnlyCollection<(string, bool)> GetOrderBy()
45+
{
46+
return Array.Empty<(string, bool)>();
47+
}
48+
49+
public override bool ReturnTotalCount()
50+
{
51+
return false;
52+
}
53+
}
54+
}

src/libraries/Microsoft.PowerFx.Connectors/Tabular/Services/CdpService.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ public abstract class CdpService : CdpServiceBase
2626

2727
public abstract HttpClient HttpClient { get; }
2828

29+
public abstract ConnectorSettings ConnectorSettings { get; }
30+
2931
public virtual CdpTableValue GetTableValue()
3032
{
3133
return IsInitialized

src/libraries/Microsoft.PowerFx.Connectors/Tabular/Services/CdpTable.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Net.Http;
99
using System.Threading;
1010
using System.Threading.Tasks;
11+
using Microsoft.PowerFx.Connectors.Tabular;
1112
using Microsoft.PowerFx.Core.Entities;
1213
using Microsoft.PowerFx.Core.IR;
1314
using Microsoft.PowerFx.Types;
@@ -53,6 +54,8 @@ public class CdpTable : CdpService
5354

5455
private readonly ConnectorSettings _connectorSettings;
5556

57+
public override ConnectorSettings ConnectorSettings => _connectorSettings;
58+
5659
internal CdpTable(string dataset, string table, IReadOnlyCollection<RawTable> tables, ConnectorSettings connectorSettings, CDPMetadataItem fieldMetadata = null)
5760
{
5861
DatasetName = dataset ?? throw new ArgumentNullException(nameof(dataset));
@@ -138,7 +141,8 @@ private async Task<string> Query(IServiceProvider serviceProvider, DelegationPar
138141
{
139142
cancellationToken.ThrowIfCancellationRequested();
140143
ConnectorLogger executionLogger = serviceProvider?.GetService<ConnectorLogger>();
141-
string queryParams = parameters?.GetODataQueryString() ?? string.Empty;
144+
parameters ??= new DefaultCDPDelegationParameter(ConnnectorType.FormulaType, _connectorSettings.MaxRows);
145+
string queryParams = parameters.GetODataQueryString();
142146
if (!string.IsNullOrEmpty(queryParams))
143147
{
144148
queryParams = "&" + queryParams;

src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/FileTabularConnector.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ public FileTabularService(string fileName)
8282
// No need for files
8383
public override HttpClient HttpClient => null;
8484

85+
public override ConnectorSettings ConnectorSettings => ConnectorSettings.NewCDPConnectorSettings();
86+
8587
internal override IReadOnlyDictionary<string, Relationship> Relationships => null;
8688

8789
// Initialization can be synchronous

src/tests/Microsoft.PowerFx.Connectors.Tests.Shared/PowerPlatformTabularTests.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public async Task SQL_CdpTabular_GetTables()
7979
Assert.Equal("DisplayName", dm.Parameters.Skip(1).First().XMsDynamicValues.ValueTitle);
8080
Assert.Equal("Database name", dm.Parameters.Skip(1).First().XMsSummary);
8181

82-
CdpDataSource cds = new CdpDataSource("pfxdev-sql.database.windows.net,connectortest");
82+
CdpDataSource cds = new CdpDataSource("pfxdev-sql.database.windows.net,connectortest", ConnectorSettings.NewCDPConnectorSettings(maxRows: 101));
8383

8484
testConnector.SetResponseFromFiles(@"Responses\SQL GetDatasetsMetadata.json", @"Responses\SQL GetTables.json");
8585
IEnumerable<CdpTable> tables = await cds.GetTablesAsync(client, $"/apim/sql/{connectionId}", CancellationToken.None, logger);
@@ -140,6 +140,10 @@ public async Task SQL_CdpTabular_GetTables()
140140
// Rows are not cached here as the cache is stored in CdpTableValue which is created by InjectServiceProviderFunction, itself added during Engine.Check
141141
testConnector.SetResponseFromFile(@"Responses\SQL Server Get First Customers.json");
142142
result = await engine.EvalAsync("Last(Customers).Phone", CancellationToken.None, runtimeConfig: rc);
143+
144+
// Verify that we have the right URL with correct $top
145+
Assert.Contains("x-ms-request-url: /apim/sql/c1a4e9f52ec94d55bb82f319b3e33a6a/v2/datasets/pfxdev-sql.database.windows.net%2Cconnectortest/tables/%5Bdbo%5D.%5BCustomers%5D/items?api-version=2015-09-01&$top=101", testConnector._log.ToString(), StringComparison.InvariantCulture);
146+
143147
StringValue phone = Assert.IsType<StringValue>(result);
144148
Assert.Equal("+1-425-705-0000", phone.Value);
145149
}

0 commit comments

Comments
 (0)