Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sourabh1007 committed Apr 26, 2024
1 parent 553f95f commit 8233fdd
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 27 deletions.
5 changes: 4 additions & 1 deletion src/Config/ObjectModel/RuntimeEntities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Text.Json.Serialization;
using Azure.DataApiBuilder.Config.Converters;
using Azure.DataApiBuilder.Service.Exceptions;
using Humanizer;

namespace Azure.DataApiBuilder.Config.ObjectModel;
Expand Down Expand Up @@ -67,7 +68,9 @@ public bool ContainsKey(string key)
}
else
{
throw new ApplicationException($"The entity '{key}' was not found in the dab-config json");
throw new DataApiBuilderException(message: $"The entity '{key}' was not found in the dab-config json",
statusCode: System.Net.HttpStatusCode.ServiceUnavailable,
subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError);
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/Core/Authorization/AuthorizationResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -430,20 +430,20 @@ public static IEnumerable<EntityActionOperation> GetAllOperationsForObjectType(D
return new List<EntityActionOperation> { EntityActionOperation.Execute };
}

IEnumerable<EntityActionOperation> validOperation =
IEnumerable<EntityActionOperation> validOperations =
operation is EntityActionOperation.All ? EntityAction.ValidPermissionOperations : new List<EntityActionOperation> { operation };

//For CosmosDB, add Patch operation to the list of valid operations if Update operation is present.
if (databaseType is DatabaseType.CosmosDB_NoSQL &&
validOperation.Contains(EntityActionOperation.Update))
validOperations.Contains(EntityActionOperation.Update))
{
List<EntityActionOperation> tempOperationList = validOperation.ToList<EntityActionOperation>();
List<EntityActionOperation> tempOperationList = validOperations.ToList<EntityActionOperation>();
tempOperationList.Add(EntityActionOperation.Patch);

validOperation = tempOperationList;
validOperations = tempOperationList;
};

return validOperation;
return validOperations;
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/Core/Configurations/RuntimeConfigValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,8 @@ public void ValidateEntitiesDoNotGenerateDuplicateQueriesOrMutation(DatabaseType
}
else
{
// For entities (table/view) that have graphQL exposed, two queries and four mutations would be generated.
// For entities (table/view) that have graphQL exposed, two queries, three mutations for Relational databases (e.g. MYSQL, MSSQL etc.)
// and four mutations for CosmosDb_NoSQL would be generated.
// Primary Key Query: For fetching an item using its primary key.
// List Query: To fetch a paginated list of items.
// Query names for both these queries are determined.
Expand Down Expand Up @@ -1287,7 +1288,6 @@ private void HandleOrRecordException(Exception ex)
/// Valid stored procedure actions:
/// - Execute
/// </summary>
/// <param name="databaseType">Database Type</param>
/// <param name="action">Compared against valid actions to determine validity.</param>
/// <param name="entity">Used to identify entity's representative object type.</param>
/// <param name="entityName">Used to supplement error messages.</param>
Expand Down
10 changes: 4 additions & 6 deletions src/Core/Resolvers/CosmosMutationEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace Azure.DataApiBuilder.Core.Resolvers
{
public class CosmosMutationEngine : IMutationEngine
{
private const int PATCH_OPERATIONS_LIMIT = 10;

private readonly CosmosClientProvider _clientProvider;
private readonly IMetadataProviderFactory _metadataProviderFactory;
private readonly IAuthorizationResolver _authorizationResolver;
Expand Down Expand Up @@ -256,11 +258,8 @@ private static async Task<ItemResponse<JObject>> HandleUpdateAsync(IDictionary<s
}

/// <summary>
/// Refer https://learn.microsoft.com/en-us/azure/cosmos-db/partial-document-update for more details on patch operations
/// Refer to https://learn.microsoft.com/azure/cosmos-db/partial-document-update for more details on patch operations
/// </summary>
/// <param name="queryArgs"></param>
/// <param name="container"></param>
/// <returns></returns>
/// <exception cref="InvalidDataException"></exception>
/// <exception cref="DataApiBuilderException"></exception>
private static async Task<JObject> HandlePatchAsync(IDictionary<string, object?> queryArgs, Container container)
Expand Down Expand Up @@ -311,7 +310,6 @@ private static async Task<JObject> HandlePatchAsync(IDictionary<string, object?>
List<PatchOperation> patchOperations = new();
GeneratePatchOperations(input, "", patchOperations);

int patchOperationsLimit = 10;
if (patchOperations.Count <= 10)
{
return (await container.PatchItemAsync<JObject>(id, new PartitionKey(partitionKey), patchOperations)).Resource;
Expand All @@ -321,7 +319,7 @@ private static async Task<JObject> HandlePatchAsync(IDictionary<string, object?>
// Hence dividing into multiple patch request, if it is requested for more than 10 item at a time.
TransactionalBatch batch = container.CreateTransactionalBatch(new PartitionKey(partitionKey));
int numberOfBatches = -1;
for (int counter = 0; counter < patchOperations.Count; counter += patchOperationsLimit)
for (int counter = 0; counter < patchOperations.Count; counter += PATCH_OPERATIONS_LIMIT)
{
// Get next 10 patch operations from the list
List<PatchOperation> chunk = patchOperations.GetRange(counter, Math.Min(10, patchOperations.Count - counter));
Expand Down
6 changes: 4 additions & 2 deletions src/Service.Tests/Configuration/ConfigurationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2990,8 +2990,10 @@ public async Task TestErrorMessageWithoutKeyFieldsInConfig()
Content = JsonContent.Create(payload)
};

ApplicationException ex = await Assert.ThrowsExceptionAsync<ApplicationException>(async () => await client.SendAsync(graphQLRequest));
Assert.AreEqual(ex.Message, "The entity 'Planet' was not found in the dab-config json");
DataApiBuilderException ex = await Assert.ThrowsExceptionAsync<DataApiBuilderException>(async () => await client.SendAsync(graphQLRequest));
Assert.AreEqual("The entity 'Planet' was not found in the dab-config json", ex.Message);
Assert.AreEqual(HttpStatusCode.ServiceUnavailable, ex.StatusCode);
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.ConfigValidationError, ex.SubStatusCode);
}
}

Expand Down
120 changes: 109 additions & 11 deletions src/Service.Tests/CosmosTests/MutationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
using Microsoft.Azure.Cosmos;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Newtonsoft.Json;

namespace Azure.DataApiBuilder.Service.Tests.CosmosTests
{
Expand Down Expand Up @@ -744,20 +743,24 @@ public async Task CanPatchItemWithoutVariables()
}}
}}";
_ = await ExecuteGraphQLRequestAsync("createPlanet", mutation, variables: new());

// Executing path with updating "existing" field and "adding" a new field
const string newName = "new_name";
mutation = $@"
mutation {{
patchPlanet (id: ""{id}"", _partitionKeyValue: ""{id}"", item: {{name: ""{newName}"", stars: [{{ id: ""{id}"" }}] }}) {{
id
name
stars
{{
id
}}
}}
}}";
JsonElement response = await ExecuteGraphQLRequestAsync("patchPlanet", mutation, variables: new());

// Validate results
Assert.AreEqual(id, response.GetProperty("id").GetString());
Assert.AreEqual(newName, response.GetProperty("name").GetString());
Assert.AreNotEqual(name, response.GetProperty("name").GetString());
Assert.AreEqual(id, response.GetProperty("stars")[0].GetProperty("id").GetString());
}

[TestMethod]
Expand All @@ -772,12 +775,17 @@ public async Task CanPatchItemWithVariables()
};
_ = await ExecuteGraphQLRequestAsync("createPlanet", _createPlanetMutation, new() { { "item", input } });

// Executing path with updating "existing" field and "adding" a new field
const string newName = "new_name";
string mutation = @"
mutation ($id: ID!, $partitionKeyValue: String!, $item: PatchPlanetInput!) {
patchPlanet (id: $id, _partitionKeyValue: $partitionKeyValue, item: $item) {
id
name
stars
{
id
}
}
}";
var update = new
Expand All @@ -787,17 +795,108 @@ public async Task CanPatchItemWithVariables()
};

JsonElement response = await ExecuteGraphQLRequestAsync("patchPlanet", mutation, variables: new() { { "id", id }, { "partitionKeyValue", id }, { "item", update } });
// Validate results
Assert.AreEqual(id, response.GetProperty("id").GetString());
Assert.AreEqual(newName, response.GetProperty("name").GetString());
Assert.AreEqual("TestStar", response.GetProperty("stars")[0].GetProperty("id").GetString());
}

[TestMethod]
public async Task CanPatchNestedItemWithVariables()
{
// Run mutation Add planet;
string id = Guid.NewGuid().ToString();
var input = new
{
id,
name = "test_name",
character = new
{
id = "characterId",
name = "characterName",
homePlanet = 1,
star = new
{
id = "starId",
name = "starName",
tag = new
{
id = "tagId",
name = "tagName"
}
}
}
};
_ = await ExecuteGraphQLRequestAsync("createPlanet", _createPlanetMutation, new() { { "item", input } });

// Executing path with updating "existing" field and "adding" a new field
const string newName = "new_name";
string mutation = @"
mutation ($id: ID!, $partitionKeyValue: String!, $item: PatchPlanetInput!) {
patchPlanet (id: $id, _partitionKeyValue: $partitionKeyValue, item: $item) {
id
name
character
{
id
name
type
homePlanet
star
{
id
name
tag
{
id
name
}
}
}
stars
{
id
}
}
}";
var update = new
{
name = "new_name",
character = new
{
id = "characterId",
name = "characterName",
type = "characterType",
homePlanet = 2,
star = new
{
id = "starId",
name = "starName1",
tag = new
{
id = "tagId",
name = "tagName"
}
}
},
stars = new[] { new { id = "TestStar" } }
};

JsonElement response = await ExecuteGraphQLRequestAsync("patchPlanet", mutation, variables: new() { { "id", id }, { "partitionKeyValue", id }, { "item", update } });
// Validate results
Assert.AreEqual(id, response.GetProperty("id").GetString());
Assert.AreEqual(newName, response.GetProperty("name").GetString());
Assert.AreNotEqual(input.name, response.GetProperty("name").GetString());
Assert.AreEqual("characterType", response.GetProperty("character").GetProperty("type").GetString());
Assert.AreEqual("characterName", response.GetProperty("character").GetProperty("name").GetString());
Assert.AreEqual(2, response.GetProperty("character").GetProperty("homePlanet").GetInt32());
Assert.AreEqual("starName1", response.GetProperty("character").GetProperty("star").GetProperty("name").GetString());
Assert.AreEqual("TestStar", response.GetProperty("stars")[0].GetProperty("id").GetString());
}

/// <summary>
/// Patch Operation has limitation of executing/patching only 10 attributes at a time, internally which is 10 patch operation.
/// In DAB, we are supporting multiple patch operations in a single patch operation by executing them in a Transactional Batch.
/// </summary>
/// <returns></returns>
[TestMethod]
public async Task CanPatchMoreThan10AttributesInAnItemWithVariables()
{
Expand Down Expand Up @@ -967,13 +1066,12 @@ public async Task CanPatchMoreThan10AttributesInAnItemWithVariables()
Assert.AreEqual(patchResponse.GetProperty("id").GetString(), input.id);
Assert.AreEqual(patchResponse.GetProperty("age").GetInt32(), input.age);
Assert.AreEqual(patchResponse.GetProperty("dimension").GetString(), input.dimension);
Assert.AreEqual(patchResponse.GetProperty("suns").ToString(), JsonConvert.SerializeObject(input.suns));
// Asserting updated information
Assert.AreEqual(patchResponse.GetProperty("name").GetString(), update.name);
Assert.AreEqual(patchResponse.GetProperty("character").ToString(), JsonConvert.SerializeObject(update.character));
Assert.AreEqual(patchResponse.GetProperty("tags").ToString(), JsonConvert.SerializeObject(update.tags));
Assert.AreEqual(patchResponse.GetProperty("stars").ToString(), JsonConvert.SerializeObject(update.stars));
Assert.AreEqual(patchResponse.GetProperty("moons").ToString(), JsonConvert.SerializeObject(update.moons));
Assert.AreEqual(patchResponse.GetProperty("character").ToString(), JsonSerializer.Serialize(update.character));
Assert.AreEqual(patchResponse.GetProperty("tags").ToString(), JsonSerializer.Serialize(update.tags));
Assert.AreEqual(patchResponse.GetProperty("stars").ToString(), JsonSerializer.Serialize(update.stars));
Assert.AreEqual(patchResponse.GetProperty("moons").ToString(), JsonSerializer.Serialize(update.moons));
}

/// <summary>
Expand Down

0 comments on commit 8233fdd

Please sign in to comment.