diff --git a/sdk/tables/Azure.Data.Tables/CHANGELOG.md b/sdk/tables/Azure.Data.Tables/CHANGELOG.md index 126e935a25663..08e33625c427c 100644 --- a/sdk/tables/Azure.Data.Tables/CHANGELOG.md +++ b/sdk/tables/Azure.Data.Tables/CHANGELOG.md @@ -6,6 +6,8 @@ ### Breaking Changes +- Calling `TableClient.Query`, `TableClient.QueryAsync`, or `TableClient.CreateQueryFilter` with a filter expression that uses `string.Equals` or `string.Compare` with a `StringComparison` parameter will now throw an exception. This is because the Azure Table service does not support these methods in query filters. Previously the `StringComparison` argument was silently ignored, which can lead to subtle bugs in client code. + ### Bugs Fixed ### Other Changes diff --git a/sdk/tables/Azure.Data.Tables/src/Queryable/ExpressionNormalizer.cs b/sdk/tables/Azure.Data.Tables/src/Queryable/ExpressionNormalizer.cs index 0e0bdcc98fe3a..1bfe6237cd08b 100644 --- a/sdk/tables/Azure.Data.Tables/src/Queryable/ExpressionNormalizer.cs +++ b/sdk/tables/Azure.Data.Tables/src/Queryable/ExpressionNormalizer.cs @@ -162,11 +162,19 @@ internal Expression VisitMethodCallNoRewrite(MethodCallExpression call) if (visited.Method.IsStatic && visited.Method.Name == "Equals" && visited.Arguments.Count > 1) { + if (visited.Arguments.Count > 2) + { + throw new NotSupportedException("string.Equals method with more than two arguments is not supported."); + } return Expression.Equal(visited.Arguments[0], visited.Arguments[1], false, visited.Method); } if (!visited.Method.IsStatic && visited.Method.Name == "Equals" && visited.Arguments.Count > 0) { + if (visited.Arguments.Count > 1) + { + throw new NotSupportedException("Equals method with more than two arguments is not supported."); + } return CreateRelationalOperator(ExpressionType.Equal, visited.Object, visited.Arguments[0]); } @@ -182,6 +190,10 @@ internal Expression VisitMethodCallNoRewrite(MethodCallExpression call) if (visited.Method.IsStatic && visited.Method.Name == "Compare" && visited.Arguments.Count > 1 && visited.Method.ReturnType == typeof(int)) { + if (visited.Arguments.Count > 2) + { + throw new NotSupportedException("string.Compare method with more than two arguments is not supported."); + } return CreateCompareExpression(visited.Arguments[0], visited.Arguments[1]); } diff --git a/sdk/tables/Azure.Data.Tables/src/TableClient.cs b/sdk/tables/Azure.Data.Tables/src/TableClient.cs index 9bc1ed3defed0..8b2346e1727c1 100644 --- a/sdk/tables/Azure.Data.Tables/src/TableClient.cs +++ b/sdk/tables/Azure.Data.Tables/src/TableClient.cs @@ -1041,6 +1041,15 @@ public virtual Response UpdateEntity(T entity, ETag ifMatch, TableUpdateMode /// /// Returns only entities that satisfy the specified filter expression. /// For example, the following expression would filter entities with a PartitionKey of 'foo': e => e.PartitionKey == "foo". + /// + /// The following string comparison methods are supported as part of a filter expression: + /// + /// + /// + /// + /// + /// + /// /// /// /// The maximum number of entities that will be returned per page. If unspecified, the default value is 1000 for storage accounts and is not limited for Cosmos DB table API. @@ -1079,6 +1088,15 @@ public virtual AsyncPageable QueryAsync( /// /// Returns only entities that satisfy the specified filter expression. /// For example, the following expression would filter entities with a PartitionKey of 'foo': e => e.PartitionKey == "foo". + /// + /// The following string comparison methods are supported as part of a filter expression: + /// + /// + /// + /// + /// + /// + /// /// /// /// The maximum number of entities that will be returned per page. If unspecified, the default value is 1000 for storage accounts and is not limited for Cosmos DB table API. @@ -1494,7 +1512,18 @@ public virtual Response SetAccessPolicy(IEnumerable table /// /// The type of the entity being queried. Typically this will be derived from /// or . - /// A filter expression. + /// A filter expression. + /// for example: e => e.PartitionKey == "foo". + /// + /// The following string comparison methods are supported as part of a filter expression: + /// + /// + /// + /// + /// + /// + /// + /// /// The string representation of the filter expression. public static string CreateQueryFilter(Expression> filter) => Bind(filter); diff --git a/sdk/tables/Azure.Data.Tables/tests/TableClientQueryExpressionTests.cs b/sdk/tables/Azure.Data.Tables/tests/TableClientQueryExpressionTests.cs index 6582a0043319c..d3799dc0571ee 100644 --- a/sdk/tables/Azure.Data.Tables/tests/TableClientQueryExpressionTests.cs +++ b/sdk/tables/Azure.Data.Tables/tests/TableClientQueryExpressionTests.cs @@ -23,6 +23,7 @@ public class TableClientQueryExpressionTests private const string TableName = "someTableName"; private const string TableName2 = "otherTableName"; private const string Partition = "partition"; + private const string MixedCasePK = "PartitionKey"; private const string Row = "row"; private const int SomeInt = 10; private const double SomeDouble = 10.10; @@ -94,6 +95,13 @@ public class TableClientQueryExpressionTests private static readonly Expression> s_tableEntExpImplicitBooleanCmpCasted = ent => (bool)ent.GetBoolean("Bool"); private static readonly Expression> s_tableEntExpImplicitBooleanCmpOr = ent => ent.GetBoolean("Bool").Value || (bool)ent.GetBoolean("Bool"); private static readonly Expression> s_tableEntExpImplicitBooleanCmpNot = ent => !ent.GetBoolean("Bool").Value; + private static readonly Expression> s_tableEntExpEquals = ent => ent.PartitionKey.Equals(Partition); + private static readonly Expression> s_CEtableEntExpEquals = ent => ent.String.Equals(Partition); + private static readonly Expression> s_CEtableEntExpStaticEquals = ent => string.Equals(ent.String, Partition); + private static readonly Expression> s_TEequalsUnsupported = ent => ent.PartitionKey.Equals(Partition, StringComparison.OrdinalIgnoreCase); + private static readonly Expression> s_TEequalsToLowerUnsupported = ent => ent.PartitionKey.Equals(Partition.ToLower(), StringComparison.OrdinalIgnoreCase); + private static readonly Expression> s_TEequalsStaticUnsupported = ent => string.Equals(ent.PartitionKey, Partition, StringComparison.OrdinalIgnoreCase); + private static readonly Expression> s_TECompareStaticUnsupported = ent => string.Compare(ent.PartitionKey, Partition, StringComparison.OrdinalIgnoreCase) > 0; public static object[] TableEntityExpressionTestCases = { @@ -125,7 +133,9 @@ public class TableClientQueryExpressionTests new object[] { $"not (Bool eq true)", s_complexExpImplicitBooleanCmpNot }, new object[] { $"BoolN eq true", s_complexExpImplicitCastedNullableBooleanCmp }, new object[] { $"(Bool eq true) and (BoolN eq true)", s_complexExpImplicitBooleanCmpAnd }, - new object[] { $"(Bool eq true) or (BoolN eq true)", s_complexExpImplicitCastedBooleanCmpOr } + new object[] { $"(Bool eq true) or (BoolN eq true)", s_complexExpImplicitCastedBooleanCmpOr }, + new object[] { $"String eq '{Partition}'", s_CEtableEntExpEquals }, + new object[] { $"String eq '{Partition}'", s_CEtableEntExpStaticEquals }, }; public static object[] TableItemExpressionTestCases = @@ -162,7 +172,16 @@ public class TableClientQueryExpressionTests new object[] { $"Bool eq true", s_tableEntExpImplicitBooleanCmp }, new object[] { $"Bool eq true", s_tableEntExpImplicitBooleanCmpCasted }, new object[] { $"(Bool eq true) or (Bool eq true)", s_tableEntExpImplicitBooleanCmpOr }, - new object[] { $"not (Bool eq true)", s_tableEntExpImplicitBooleanCmpNot } + new object[] { $"not (Bool eq true)", s_tableEntExpImplicitBooleanCmpNot }, + new object[] { $"PartitionKey eq '{Partition}'", s_tableEntExpEquals }, + }; + + public static object[] UnSupportedTableItemExpressionTestCases = + { + new object[] { s_TEequalsUnsupported }, + new object[] { s_TEequalsStaticUnsupported }, + new object[] { s_TEequalsToLowerUnsupported }, + new object[] { s_TECompareStaticUnsupported }, }; [TestCaseSource(nameof(TableItemExpressionTestCases))] @@ -191,5 +210,12 @@ public void TestDictionaryTableEntityFilterExpressions(string expectedFilter, Ex Assert.That(filter, Is.EqualTo(expectedFilter)); } + + [TestCaseSource(nameof(UnSupportedTableItemExpressionTestCases))] + [Test] + public void TestTableItemFilterExpressionsUnsupported(Expression> expression) + { + Assert.Throws(() => TableClient.CreateQueryFilter(expression)); + } } } diff --git a/sdk/tables/Azure.Data.Tables/tests/samples/Sample0_Auth.cs b/sdk/tables/Azure.Data.Tables/tests/samples/Sample0_Auth.cs index a6b3711a95c75..31b9360f51785 100644 --- a/sdk/tables/Azure.Data.Tables/tests/samples/Sample0_Auth.cs +++ b/sdk/tables/Azure.Data.Tables/tests/samples/Sample0_Auth.cs @@ -108,10 +108,7 @@ public async Task TokenCredentialAuth() #if SNIPPET new DefaultAzureCredential()); #else - new ClientSecretCredential( - GetVariable("TENANT_ID"), - GetVariable("CLIENT_ID"), - GetVariable("CLIENT_SECRET"))); + Credential); #endif // Create the table if it doesn't already exist to verify we've successfully authenticated. diff --git a/sdk/tables/Azure.Data.Tables/tests/samples/Sample8_CustomizingSerialization.cs b/sdk/tables/Azure.Data.Tables/tests/samples/Sample8_CustomizingSerialization.cs index bc7972473ac9d..01a9c9a350c38 100644 --- a/sdk/tables/Azure.Data.Tables/tests/samples/Sample8_CustomizingSerialization.cs +++ b/sdk/tables/Azure.Data.Tables/tests/samples/Sample8_CustomizingSerialization.cs @@ -28,10 +28,7 @@ public async Task CustomizeSerialization() #if SNIPPET new DefaultAzureCredential()); #else - new ClientSecretCredential( - GetVariable("TENANT_ID"), - GetVariable("CLIENT_ID"), - GetVariable("CLIENT_SECRET"))); + Credential); #endif // Create the table if it doesn't already exist. diff --git a/sdk/tables/test-resources.json b/sdk/tables/test-resources.json index 7d77065dbf796..bf462a2b9e968 100644 --- a/sdk/tables/test-resources.json +++ b/sdk/tables/test-resources.json @@ -1,5 +1,5 @@ { - "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#", + "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#", "contentVersion": "1.0.0.0", "parameters": { "baseName": { @@ -12,18 +12,18 @@ } }, "cosmosEndpointSuffix": { - "type": "string", - "defaultValue": "cosmos.azure.com", - "metadata": { - "description": "The url suffix to use when accessing the cosmos data plane." - } + "type": "string", + "defaultValue": "cosmos.azure.com", + "metadata": { + "description": "The url suffix to use when accessing the cosmos data plane." + } }, "storageEndpointSuffix": { - "type": "string", - "defaultValue": "core.windows.net", - "metadata": { - "description": "The url suffix to use when accessing the storage data plane." - } + "type": "string", + "defaultValue": "core.windows.net", + "metadata": { + "description": "The url suffix to use when accessing the storage data plane." + } } }, "variables": { @@ -48,7 +48,16 @@ "virtualNetworkRules": [], "ipRules": [], "defaultAction": "Allow" - } + }, + "customCosmosRoleName": "Azure Cosmos DB SDK role for Table Data Plane", + "customCosmosRoleDescription": "Azure Cosmos DB SDK role for Table Data Plane", + "customCosmosRoleActions": [ + "Microsoft.DocumentDB/databaseAccounts/readMetadata", + "Microsoft.DocumentDB/databaseAccounts/tables/*", + "Microsoft.DocumentDB/databaseAccounts/tables/containers/*", + "Microsoft.DocumentDB/databaseAccounts/tables/containers/entities/*", + "Microsoft.DocumentDB/databaseAccounts/throughputSettings/read" + ] }, "resources": [ { @@ -118,6 +127,39 @@ ], "ipRules": [] } + }, + { + "dependsOn": [ + "[resourceId('Microsoft.DocumentDB/databaseAccounts', variables('primaryAccountName'))]" + ], + "type": "Microsoft.DocumentDB/databaseAccounts/tableRoleDefinitions", + "apiVersion": "2024-05-15", + "name": "[concat(variables('primaryAccountName'), '/', guid(variables('customCosmosRoleName')))]", + "properties": { + "roleName": "[variables('customCosmosRoleName')]", + "description": "[variables('customCosmosRoleDescription')]", + "permissions": [ + { + "dataActions": "[variables('customCosmosRoleActions')]" + } + ], + "assignableScopes": [ + "[concat('/subscriptions/', subscription().subscriptionId, '/resourceGroups/', resourceGroup().name, '/providers/Microsoft.DocumentDB/databaseAccounts/', variables('primaryAccountName'))]" + ] + } + }, + { + "dependsOn": [ + "[resourceId('Microsoft.DocumentDB/databaseAccounts/tableRoleDefinitions', variables('primaryAccountName'), guid(variables('customCosmosRoleName')))]" + ], + "type": "Microsoft.DocumentDB/databaseAccounts/tableRoleAssignments", + "apiVersion": "2024-05-15", + "name": "[concat(variables('primaryAccountName'), '/', guid(variables('customCosmosRoleName')))]", + "properties": { + "scope": "[concat('/subscriptions/', subscription().subscriptionId, '/resourceGroups/', resourceGroup().name, '/providers/Microsoft.DocumentDB/databaseAccounts/', variables('primaryAccountName'))]", + "roleDefinitionId": "[resourceId('Microsoft.DocumentDB/databaseAccounts/tableRoleDefinitions', variables('primaryAccountName'), guid(variables('customCosmosRoleName')))]", + "principalId": "[parameters('testApplicationOid')]" + } } ], "outputs": {