Skip to content

Conversation

@Qualizorg
Copy link

@Qualizorg Qualizorg commented Aug 16, 2024

Related to #442, #307, #297

Implementation to support replace, set, add, remove, increment patch operations, excluding move.

Both implementations follow expression + path, inspired by #307 .

Expression approach can't work with ArrayIndexExpression ( x => x.Address.Tags[0]), can make it work if it brings value, nevertheless same goal can be achieved over string path approach (/address/tags/0)

Questions

  1. InMemoryRepository needs rework to handle the other operation types, how critical is it?
  2. Decide if bulk patch operations per partition key are valuable.

Need feedback regarding approach and any major optimizations we can perform. @IEvangelist @mumby0168

Tasks

  • Rework Replace Operation + Unit Tests
  • Implemented Set Operation + Unit Tests
  • Implemented Add Operation + Unit Tests
  • Implemented Remove Operation + Unit Tests
  • Implemented Increment Operation + Unit Tests
  • Document methods with Microsoft Docs
  • Add all corresponding tests to DefaultRepositoryTests
  • Remove commented code (dependant on question 1)

@Qualizorg
Copy link
Author

Updated DefaultRepository.Update.cs to return etag as value.

@@ -1,55 +1,396 @@
// Copyright (c) David Pine. All rights reserved.
// Copyright (c) IEvangelist. All rights reserved.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright (c) IEvangelist. All rights reserved.
// Copyright (c) David Pine. All rights reserved.


internal class PatchOperationBuilder<TItem> : IPatchOperationBuilder<TItem> where TItem : IItem
[assembly: InternalsVisibleTo("Microsoft.Azure.CosmosRepositoryTests")]
namespace Microsoft.Azure.CosmosRepository.Builders
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File-scoped namespaces please.

internal readonly List<InternalPatchOperation> _rawPatchOperations = [];

public IReadOnlyList<PatchOperation> PatchOperations => _patchOperations;
internal class PatchOperationBuilder<TItem> : IPatchOperationBuilder<TItem> where TItem : IItem
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be sealed?

_patchOperations.Add(PatchOperation.Replace($"/{propertyToReplace}", value));
public PatchOperationBuilder() =>
_namingStrategy = new CamelCaseNamingStrategy();
public PatchOperationBuilder(CosmosPropertyNamingPolicy? cosmosPropertyNamingPolicy) =>
Copy link
Owner

@IEvangelist IEvangelist Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, prefer primary constructor.

Suggested change
public PatchOperationBuilder(CosmosPropertyNamingPolicy? cosmosPropertyNamingPolicy) =>
public PatchOperationBuilder(CosmosPropertyNamingPolicy? cosmosPropertyNamingPolicy) =>

return this;
}

private string NormalizePathPrefix(string path) => "/" + path.TrimStart('/').TrimEnd('/');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer string interpolation.

@Qualizorg
Copy link
Author

Qualizorg commented Aug 20, 2024

Add Operation doesn't work properly with lambda.

            var tenantId = Guid.Parse("fd048274-190c-4e50-90ff-f2bb84974523");
            var companyId = Guid.Parse("c4707548-5033-4c00-9735-9268c326c04a");

            // Create a company with the new properties
            var company = new Company
            {
                TenantId = tenantId,
                CompanyId = companyId,
                Id = companyId.ToString(),
                CompanyName = "Tech Innovations Inc.",
                Industry = "Software Development",
                Location = "Silicon Valley, CA",
                YearFounded = 2005,
                IsPublic = true,
                Revenue = 1200000000m, // 1.2 Billion USD
                Website = "https://www.techinnovations.com",
                Manager = new CompanyEmployee
                {
                    EmployeeId = 3,
                    FullName = "Johh Walker",
                    Position = "CEO",
                    Salary = 170000m,
                    EmployeeAddress = new Address
                    {
                        Street = "456 Oak St",
                        City = "aaaa",
                        State = "IL",
                        ZipCode = "62705"
                    }
                },
                Departments = new List<Department>
            {
                new Department
                {
                    DepartmentId = 101,
                    DepartmentName = "Engineering",
                    ManagerName = "Alice Johnson",
                    Employees = new List<CompanyEmployee>
                    {
                        new CompanyEmployee
                        {
                            EmployeeId = 1,
                            FullName = "John Doe",
                            Position = "Software Engineer",
                            Salary = 80000m,
                            EmployeeAddress = new Address
                            {
                                Street = "123 Main St",
                                City = "Springfield",
                                State = "IL",
                                ZipCode = "62704"
                            }
                        }
                    }
                },
                new Department
                {
                    DepartmentId = 102,
                    DepartmentName = "Marketing",
                    ManagerName = "Bob Williams",
                    Employees = new List<CompanyEmployee>
                    {
                        new CompanyEmployee
                        {
                            EmployeeId = 2,
                            FullName = "Jane Smith",
                            Position = "Marketing Specialist",
                            Salary = 70000m,
                            EmployeeAddress = new Address
                            {
                                Street = "456 Oak St",
                                City = "Springfield",
                                State = "IL",
                                ZipCode = "62705"
                            }
                        }
                    }
                }
            }
            };
  await _testRepository.PatchAsync(companyId, tenantId, builder =>
      builder
      .Remove(x => x.Industry)
      .Replace(x => x.CompanyName, "CompanyNameTest")
      .Set(x => x.Manager, new CompanyEmployee() { EmployeeId = 4, FullName = "Paul Walker", Position = "CTOO", Salary = 4560000m })
      .Add(x => x.Departments, [new Department() { DepartmentId = 1234, DepartmentName = "Tech Dep", ManagerName = "Terry Springfield" }])
  );

Cosmos exception
Cannot deserialize the current JSON array (e.g. [1,2,3]) into type 'QV.OrganisationsComponent.Domain.Department' because the type requires a JSON object (e.g. {"name":"value"}) to deserialize correctly.\r\nTo fix this error either change the JSON to a JSON object (e.g. {"name":"value"}) or change the deserialized type to an array or a type that implements a collection interface (e.g. ICollection, IList) like List<T> that can be deserialized from a JSON array. JsonArrayAttribute can also be added to the type to force it to deserialize from a JSON array.\r\nPath 'departments[2]', line 1, position 703.

This is due the fact that x => x.Departments is a list and TValue from .Add expects it to be a list.

Needs more work.

Edit: Format Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants