diff --git a/src/Marten.PLv8.Testing/Patching/patching_api.cs b/src/Marten.PLv8.Testing/Patching/patching_api.cs index 4e5410dcc6..464feb425f 100644 --- a/src/Marten.PLv8.Testing/Patching/patching_api.cs +++ b/src/Marten.PLv8.Testing/Patching/patching_api.cs @@ -1,14 +1,18 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading; using System.Threading.Tasks; +using JasperFx.Core.Reflection; using Marten.Events; using Marten.Events.Projections; using Marten.PLv8.Patching; +using Marten.Services; using Marten.Storage; using Marten.Testing.Documents; using Marten.Testing.Harness; +using Npgsql; using Shouldly; using Weasel.Core; using Weasel.Postgresql; @@ -832,6 +836,139 @@ public async Task bug_611_duplicate_field_is_updated_by_set_operation_with_multi count.ShouldBe(1); } + [Fact] + public async Task duplicated_fields_only_update_when_source_is_modified() + { + // Set up duplicate field in the schema + var t = Target.Random(); + var mapping = theStore.StorageFeatures.MappingFor(typeof(Target)); + var duplicateField = mapping.DuplicateField("String"); + + // Setup a document + var target = Target.Random(); + target.Inner = Target.Random(); + target.Inner.String = "original"; + theSession.Store(target); + await theSession.SaveChangesAsync(); + + // First verify that modifying the source updates the duplicate + var newValue = "modified source"; + theSession.Patch(target.Id).Set(x => x.String, newValue); + await theSession.SaveChangesAsync(); + + // Verify both fields are updated + await using (var command = theSession.Connection.CreateCommand()) + { + command.CommandText = $"select count(*) from {mapping.TableName.QualifiedName} " + + $"where data->>'String' = '{newValue}' and {duplicateField.ToColumn().Name} = '{newValue}'"; + var count = (long)(command.ExecuteScalar() ?? 0); + count.ShouldBe(1); + } + + // Now modify an unrelated field and capture the SQL + var capturedCommands = new List(); + theSession.Logger = new TestLogger(capturedCommands).StartSession(theSession); + + theSession.Patch(target.Id).Set(x => x.Number, 42); + await theSession.SaveChangesAsync(); + + // Verify no update to the duplicate field was executed + capturedCommands.Any(sql => sql.Contains($"set {duplicateField.ColumnName}")).ShouldBeFalse(); + } + + [Fact] + public async Task duplicated_fields_only_update_when_nested_source_is_modified() + { + // Set up duplicate field in the schema + var t = Target.Random(); + var mapping = theStore.StorageFeatures.MappingFor(typeof(Target)); + MemberInfo[] props = + [ + ReflectionHelper.GetProperty(x => x.Inner), + ReflectionHelper.GetProperty(x => x.String) + ]; + var duplicateField = mapping.DuplicateField(props, columnName: "String"); + + // Setup a document + var target = Target.Random(); + target.Inner = Target.Random(); + target.Inner.String = "original"; + theSession.Store(target); + await theSession.SaveChangesAsync(); + + // First verify that modifying the source updates the duplicate + var newValue = "modified source"; + theSession.Patch(target.Id).Set(x => x.Inner.String, newValue); + await theSession.SaveChangesAsync(); + + // Verify both fields are updated + await using (var command = theSession.Connection.CreateCommand()) + { + command.CommandText = $"select count(*) from {mapping.TableName.QualifiedName} " + + $"where data->'Inner'->>'String' = '{newValue}' and {duplicateField.ToColumn().Name} = '{newValue}'"; + var count = (long)(command.ExecuteScalar() ?? 0); + count.ShouldBe(1); + } + + // Now modify an unrelated field and capture the SQL + var capturedCommands = new List(); + theSession.Logger = new TestLogger(capturedCommands).StartSession(theSession); + + theSession.Patch(target.Id).Set(x => x.Number, 42); + await theSession.SaveChangesAsync(); + + // Verify no update to the duplicate field was executed + capturedCommands.Any(sql => sql.Contains($"set {duplicateField.ColumnName}")).ShouldBeFalse(); + } + + private class TestLogger(List capturedCommands): IMartenLogger + { + public IMartenSessionLogger StartSession(IQuerySession session) => new TestSessionLogger(capturedCommands); + public void SchemaChange(string sql) + { + } + } + + private class TestSessionLogger(List capturedCommands): IMartenSessionLogger + { + public void LogFailure(NpgsqlCommand command, Exception ex) + { + } + + public void LogFailure(NpgsqlBatch batch, Exception ex) + { + } + + public void LogFailure(Exception ex, string message) + { + } + + public void LogSuccess(NpgsqlBatch batch) + { + foreach (var command in batch.BatchCommands) + { + capturedCommands.Add(command.CommandText); + } + } + + public void LogSuccess(NpgsqlCommand command) + { + capturedCommands.Add(command.CommandText); + } + + public void RecordSavedChanges(IDocumentSession session, IChangeSet commit) + { + } + + public void OnBeforeExecute(NpgsqlCommand command) + { + } + + public void OnBeforeExecute(NpgsqlBatch batch) + { + } + } + public async Task SampleSetup() { #region sample_plv8_registering_custom_projection diff --git a/src/Marten.PLv8/Patching/PatchOperation.cs b/src/Marten.PLv8/Patching/PatchOperation.cs index d1ba411933..01b4ad1969 100644 --- a/src/Marten.PLv8/Patching/PatchOperation.cs +++ b/src/Marten.PLv8/Patching/PatchOperation.cs @@ -3,11 +3,13 @@ using System.Linq; using Marten.Internal.Operations; using Marten.Internal.Storage; +using Marten.Linq.Members; using Marten.Linq.SqlGeneration; using Marten.PLv8.Transforms; using Marten.Schema; using Marten.Schema.Identity; using Marten.Services; +using Marten.Util; using NpgsqlTypes; using Weasel.Postgresql; using Weasel.Postgresql.SqlGeneration; @@ -75,12 +77,16 @@ internal class PatchOperation: StatementOperation, NoDataReturnedCall { private readonly ISqlFragment _fragment; private readonly IDocumentStorage _storage; + private readonly IDictionary _patch; + private readonly ISerializer _serializer; public PatchOperation(TransformFunction transform, IDocumentStorage storage, IDictionary patch, ISerializer serializer, bool possiblyPolymorphic): base(storage, new PatchFragment(patch, serializer, transform, storage, possiblyPolymorphic)) { + _patch = patch; _storage = storage; + _serializer = serializer; } public OperationRole Role() @@ -102,18 +108,33 @@ private void applyUpdates(ICommandBuilder builder) return; } + // Only update duplicated fields where their mapping path is affected by the patch path + var affectedFields = fields.Where(f => IsFieldAffectedByPatchPath(f, _patch["path"].ToString())).ToList(); + + if (affectedFields.Count == 0) + { + return; + } + builder.StartNewCommand(); builder.Append("update "); builder.Append(_storage.TableName.QualifiedName); builder.Append(" as d set "); - builder.Append(fields[0].UpdateSqlFragment()); - for (var i = 1; i < fields.Count; i++) + builder.Append(affectedFields[0].UpdateSqlFragment()); + for (var i = 1; i < affectedFields.Count; i++) { builder.Append(", "); - builder.Append(fields[i].UpdateSqlFragment()); + builder.Append(affectedFields[i].UpdateSqlFragment()); } writeWhereClause(builder); } + + private bool IsFieldAffectedByPatchPath(DuplicatedField field, string modifiedPath) + { + // get the dot seperated path derived from field Members info + var path = string.Join('.', field.Members.Select(x => x.Name.FormatCase(_serializer.Casing))); + return modifiedPath.StartsWith(path, StringComparison.Ordinal); + } } diff --git a/src/Marten/Patching/PatchOperation.cs b/src/Marten/Patching/PatchOperation.cs index b242be674f..541964ca17 100644 --- a/src/Marten/Patching/PatchOperation.cs +++ b/src/Marten/Patching/PatchOperation.cs @@ -1,12 +1,13 @@ +using System; using System.Collections.Generic; using System.Linq; using Marten.Internal.Operations; using Marten.Internal.Sessions; using Marten.Internal.Storage; +using Marten.Linq.Members; using Marten.Linq.SqlGeneration; -using Marten.Schema; -using Marten.Schema.Identity; using Marten.Services; +using Marten.Util; using NpgsqlTypes; using Weasel.Core; using Weasel.Postgresql; @@ -86,6 +87,7 @@ internal class PatchOperation: StatementOperation, NoDataReturnedCall private readonly ISqlFragment _fragment; private readonly IDocumentStorage _storage; private readonly List _patchSet; + private readonly ISerializer _serializer; public PatchOperation(DocumentSessionBase session, DbObjectName function, IDocumentStorage storage, List patchSet, ISerializer serializer): @@ -93,6 +95,7 @@ public PatchOperation(DocumentSessionBase session, DbObjectName function, IDocum { _storage = storage; _patchSet = patchSet; + _serializer = serializer; } public OperationRole Role() @@ -115,18 +118,38 @@ private void applyUpdates(ICommandBuilder builder) return; } + // Get the paths being modified by this patch operation + var modifiedPaths = _patchSet + .Select(patch => patch.Items["path"].ToString()) + .ToHashSet(System.StringComparer.Ordinal); + + // Only update duplicated fields where their mapping path is affected by the patch path + var affectedFields = fields.Where(f => IsFieldAffectedByPatchPath(f, modifiedPaths)).ToList(); + + if (affectedFields.Count == 0) + { + return; + } + builder.StartNewCommand(); builder.Append("update "); builder.Append(_storage.TableName.QualifiedName); builder.Append(" as d set "); - builder.Append(fields[0].UpdateSqlFragment()); - for (var i = 1; i < fields.Count; i++) + builder.Append(affectedFields[0].UpdateSqlFragment()); + for (var i = 1; i < affectedFields.Count; i++) { builder.Append(", "); - builder.Append(fields[i].UpdateSqlFragment()); + builder.Append(affectedFields[i].UpdateSqlFragment()); } writeWhereClause(builder); } + + private bool IsFieldAffectedByPatchPath(DuplicatedField field, HashSet modifiedPaths) + { + // get the dot seperated path derived from field Members info + var path = string.Join('.', field.Members.Select(x => x.Name.FormatCase(_serializer.Casing))); + return modifiedPaths.Any(p => p.StartsWith(path, StringComparison.Ordinal)); + } } diff --git a/src/PatchingTests/Patching/patching_api.cs b/src/PatchingTests/Patching/patching_api.cs index 5e034629a8..5f16b9abfb 100644 --- a/src/PatchingTests/Patching/patching_api.cs +++ b/src/PatchingTests/Patching/patching_api.cs @@ -1,15 +1,19 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading; using System.Threading.Tasks; +using JasperFx.Core.Reflection; using Marten; using Marten.Events; using Marten.Events.Projections; using Marten.Patching; +using Marten.Services; using Marten.Storage; using Marten.Testing.Documents; using Marten.Testing.Harness; +using Npgsql; using Shouldly; using Weasel.Core; using Weasel.Postgresql.SqlGeneration; @@ -822,6 +826,139 @@ public async Task bug_611_duplicate_field_is_updated_by_set_operation_with_multi count.ShouldBe(1); } + [Fact] + public async Task duplicated_fields_only_update_when_source_is_modified() + { + // Set up duplicate field in the schema + var t = Target.Random(); + var mapping = theStore.StorageFeatures.MappingFor(typeof(Target)); + var duplicateField = mapping.DuplicateField("String"); + + // Setup a document + var target = Target.Random(); + target.Inner = Target.Random(); + target.Inner.String = "original"; + theSession.Store(target); + await theSession.SaveChangesAsync(); + + // First verify that modifying the source updates the duplicate + var newValue = "modified source"; + theSession.Patch(target.Id).Set(x => x.String, newValue); + await theSession.SaveChangesAsync(); + + // Verify both fields are updated + await using (var command = theSession.Connection.CreateCommand()) + { + command.CommandText = $"select count(*) from {mapping.TableName.QualifiedName} " + + $"where data->>'String' = '{newValue}' and {duplicateField.ToColumn().Name} = '{newValue}'"; + var count = (long)(command.ExecuteScalar() ?? 0); + count.ShouldBe(1); + } + + // Now modify an unrelated field and capture the SQL + var capturedCommands = new List(); + theSession.Logger = new TestLogger(capturedCommands).StartSession(theSession); + + theSession.Patch(target.Id).Set(x => x.Number, 42); + await theSession.SaveChangesAsync(); + + // Verify no update to the duplicate field was executed + capturedCommands.Any(sql => sql.Contains($"set {duplicateField.ColumnName}")).ShouldBeFalse(); + } + + [Fact] + public async Task duplicated_fields_only_update_when_nested_source_is_modified() + { + // Set up duplicate field in the schema + var t = Target.Random(); + var mapping = theStore.StorageFeatures.MappingFor(typeof(Target)); + MemberInfo[] props = + [ + ReflectionHelper.GetProperty(x => x.Inner), + ReflectionHelper.GetProperty(x => x.String) + ]; + var duplicateField = mapping.DuplicateField(props, columnName: "String"); + + // Setup a document + var target = Target.Random(); + target.Inner = Target.Random(); + target.Inner.String = "original"; + theSession.Store(target); + await theSession.SaveChangesAsync(); + + // First verify that modifying the source updates the duplicate + var newValue = "modified source"; + theSession.Patch(target.Id).Set(x => x.Inner.String, newValue); + await theSession.SaveChangesAsync(); + + // Verify both fields are updated + await using (var command = theSession.Connection.CreateCommand()) + { + command.CommandText = $"select count(*) from {mapping.TableName.QualifiedName} " + + $"where data->'Inner'->>'String' = '{newValue}' and {duplicateField.ToColumn().Name} = '{newValue}'"; + var count = (long)(command.ExecuteScalar() ?? 0); + count.ShouldBe(1); + } + + // Now modify an unrelated field and capture the SQL + var capturedCommands = new List(); + theSession.Logger = new TestLogger(capturedCommands).StartSession(theSession); + + theSession.Patch(target.Id).Set(x => x.Number, 42); + await theSession.SaveChangesAsync(); + + // Verify no update to the duplicate field was executed + capturedCommands.Any(sql => sql.Contains($"set {duplicateField.ColumnName}")).ShouldBeFalse(); + } + + private class TestLogger(List capturedCommands): IMartenLogger + { + public IMartenSessionLogger StartSession(IQuerySession session) => new TestSessionLogger(capturedCommands); + public void SchemaChange(string sql) + { + } + } + + private class TestSessionLogger(List capturedCommands): IMartenSessionLogger + { + public void LogFailure(NpgsqlCommand command, Exception ex) + { + } + + public void LogFailure(NpgsqlBatch batch, Exception ex) + { + } + + public void LogFailure(Exception ex, string message) + { + } + + public void LogSuccess(NpgsqlBatch batch) + { + foreach (var command in batch.BatchCommands) + { + capturedCommands.Add(command.CommandText); + } + } + + public void LogSuccess(NpgsqlCommand command) + { + capturedCommands.Add(command.CommandText); + } + + public void RecordSavedChanges(IDocumentSession session, IChangeSet commit) + { + } + + public void OnBeforeExecute(NpgsqlCommand command) + { + } + + public void OnBeforeExecute(NpgsqlBatch batch) + { + } + } + public void SampleSetup() { #region sample_registering_custom_projection