Introduce SkipCondition: A possibility to ignore errors while patching (#88)

* Introduce `SkipCondition`: A possibility to ignore errors while patching

* format fun

Author: hf-kklein

ChronoJsonDiffPatch/ChronoJsonDiffPatch/SkipCondition.cs

@@ -0,0 +1,25 @@
+namespace ChronoJsonDiffPatch;
+
+/// <summary>
+/// models cases in which a patch should be skipped / not be applied to an entity
+/// </summary>
+/// <remarks>
+/// Usually this shouldn't be necessary to use if everything is already. But if your data are corrupted somehow, this might be a way to still apply some patches to you data and only skip some instead of all.
+/// Use this with caution (but of course we know: as soon as it's possible to ignore errors, we'll use it.
+/// The state of the entity after a patch has been skipped is not well-defined.
+/// It _might_ be somewhat like what you'd expect but all guarantees are gone.
+/// It's the pandora's box of patching you're opening here.
+/// Be sure to monitor the state of your entities after applying patches with skipped patches and <see cref="TimeRangePatchChain{TEntity}.PatchesHaveBeenSkipped"/>.
+/// </remarks>
+/// <typeparam name="TEntity">the entity to which the patches shall be applied</typeparam>
+public interface ISkipCondition<in TEntity>
+{
+    /// <summary>
+    /// If applying <paramref name="failedPatch"/> to <paramref name="initialEntity"/> fails with <paramref name="errorWhilePatching"/>, the patch should be skipped.
+    /// </summary>
+    /// <param name="initialEntity">state before applying the patch <paramref name="failedPatch"/> (but not necessarily before apply any patch in the chain)</param>
+    /// <param name="errorWhilePatching">any error that occurred</param>
+    /// <param name="failedPatch">the patch that lead to the exception <paramref name="errorWhilePatching"/></param>
+    /// <returns>true if the patch should be skipped</returns>
+    public bool ShouldSkipPatch(TEntity initialEntity, TimeRangePatch failedPatch, Exception errorWhilePatching);
+}

ChronoJsonDiffPatch/ChronoJsonDiffPatch/SkipPatchesWithUnmatchedListItems.cs

@@ -0,0 +1,44 @@
+namespace ChronoJsonDiffPatch;
+
+/// <summary>
+/// skips patches where an item inside a list is modified or removed that is not present in the initial list (because of corrupted data)
+/// </summary>
+/// <typeparam name="TEntity">the entity to be patched</typeparam>
+/// <typeparam name="TListItem">the items inside the list</typeparam>
+public class SkipPatchesWithUnmatchedListItems<TEntity, TListItem> : ISkipCondition<TEntity>
+{
+    private readonly Func<TEntity, List<TListItem>?> _listAccessor;
+
+    /// <summary>
+    /// provide a way to access the list which index is out of range
+    /// </summary>
+    /// <param name="listAccessor"></param>
+    public SkipPatchesWithUnmatchedListItems(Func<TEntity, List<TListItem>?> listAccessor)
+    {
+        _listAccessor = listAccessor;
+    }
+
+    /// <summary>
+    /// <inheritdoc cref="ISkipCondition{TEntity}"/>
+    /// </summary>
+    public bool ShouldSkipPatch(TEntity initialEntity, TimeRangePatch failedPatch, Exception errorWhilePatching)
+    {
+        if (errorWhilePatching is not ArgumentOutOfRangeException)
+        {
+            return false;
+        }
+
+        var list = _listAccessor(initialEntity);
+        if (list is null)
+        {
+            return false;
+        }
+
+        // todo: theoretically I could
+        // 1. check the json attributes of the list property,
+        // 2. then inspect the failedPatch.Patch and then
+        // 3. see if the error _really_ originates from there
+        // but for now this is a good enough solution
+        return true;
+    }
+}

ChronoJsonDiffPatch/ChronoJsonDiffPatch/TimeRangePatchChain.cs

@@ -35,6 +35,24 @@ protected static TEntity DefaultDeSerializer(string json)
 
     private readonly Func<TEntity, string> _serialize;
     private readonly Func<string, TEntity> _deserialize;
+    private readonly IEnumerable<ISkipCondition<TEntity>>? _skipConditions;
+    private List<TimeRangePatch> _skippedPatches = new();
+
+    /// <summary>
+    /// patches that have been skipped because of errors
+    /// </summary>
+    public IReadOnlyList<TimeRangePatch> SkippedPatches
+    {
+        get => _skippedPatches.AsReadOnly();
+    }
+
+    /// <summary>
+    /// set to true, if, while modifying the chain any of the skip conditions provided to the construct were used.
+    /// </summary>
+    public bool PatchesHaveBeenSkipped
+    {
+        get => SkippedPatches?.Any() == true;
+    }
 
     /// <summary>
     /// converts the given <paramref name="entity"/> to an JToken using the serializer configured in the constructor (or default) 
@@ -77,7 +95,8 @@ private static IEnumerable<TimeRangePatch> PrepareForTimePeriodChainConstructor(
 
         var result = timeperiods.OrderBy(tp => tp.Start);
         var ambigousStarts = result.GroupBy(tp => tp.Start).Where(g => g.Count() > 1).Select(g => g.Select(x => x.Start.ToString("o")).First()).Distinct();
-        var ambigousEnds = result.GroupBy(tp => tp.End).Where(g => g.Count() > 1).Select(g => g.Select(x => x.End.ToString("o")).First()).Distinct(); ;
+        var ambigousEnds = result.GroupBy(tp => tp.End).Where(g => g.Count() > 1).Select(g => g.Select(x => x.End.ToString("o")).First()).Distinct();
+        ;
         bool baseConstructorIsLikelyToCrash = ambigousStarts.Any() || ambigousEnds.Any();
         if (baseConstructorIsLikelyToCrash)
         {
@@ -88,9 +107,11 @@ private static IEnumerable<TimeRangePatch> PrepareForTimePeriodChainConstructor(
             catch (InvalidOperationException invalidOpException) when (invalidOpException.Message.EndsWith("out of range"))
             {
                 // if it would crash and we do know the reasons, then we throw a more meaningful exception here instead of waiting for the base class to crash
-                throw new ArgumentException($"The given periods contain ambiguous starts ({string.Join(", ", ambigousStarts)}) or ends ({string.Join(", ", ambigousEnds)})", innerException: invalidOpException);
+                throw new ArgumentException($"The given periods contain ambiguous starts ({string.Join(", ", ambigousStarts)}) or ends ({string.Join(", ", ambigousEnds)})",
+                    innerException: invalidOpException);
             }
         }
+
         return result;
     }
 
@@ -101,8 +122,10 @@ private static IEnumerable<TimeRangePatch> PrepareForTimePeriodChainConstructor(
     /// <param name="patchingDirection">the direction in which the patches are to be applied; <see cref="PatchingDirection"/></param>
     /// <param name="serializer">a function that is able to serialize <typeparamref name="TEntity"/></param>
     /// <param name="deserializer">a function that is able to deserialize <typeparamref name="TEntity"/></param>
+    /// <param name="skipConditions">optional conditions under which we allow a patch to fail and ignore its changes</param>
     public TimeRangePatchChain(IEnumerable<TimeRangePatch>? timeperiods = null, PatchingDirection patchingDirection = PatchingDirection.ParallelWithTime,
-        Func<TEntity, string>? serializer = null, Func<string, TEntity>? deserializer = null) : base(PrepareForTimePeriodChainConstructor(timeperiods))
+        Func<TEntity, string>? serializer = null, Func<string, TEntity>? deserializer = null, IEnumerable<ISkipCondition<TEntity>>? skipConditions = null) : base(
+        PrepareForTimePeriodChainConstructor(timeperiods))
     {
         _serialize = serializer ?? DefaultSerializer;
         _deserialize = deserializer ?? DefaultDeSerializer;
@@ -112,6 +135,8 @@ public TimeRangePatchChain(IEnumerable<TimeRangePatch>? timeperiods = null, Patc
             throw new ArgumentException($"You must not add a patch {patchWithWrongDirection} with direction {patchWithWrongDirection.PatchingDirection}!={PatchingDirection}",
                 nameof(timeperiods));
         }
+
+        _skipConditions = skipConditions;
     }
 
     /// <summary>
@@ -246,11 +271,13 @@ public void Add(TEntity initialEntity, TEntity changedEntity, DateTimeOffset mom
                 var jsonStateWithoutMomentPatch = ToJToken(stateWithoutMomentPatch);
                 var jdpDelta = new JsonDiffPatch();
                 var jsonStateWithMomentPatch = jdpDelta.Patch(jsonStateWithoutMomentPatch, JsonConvert.DeserializeObject<JToken>(patchToBeAdded.Patch!.RootElement.GetRawText()));
-                var jsonStateBeforeMomentPatch = ToJToken(artificialChainWithoutTheRecentlyAddedPatch.PatchToDate(initialEntity, infinitelyNarrowPatch.Start - TimeSpan.FromTicks(1)));
+                var jsonStateBeforeMomentPatch =
+                    ToJToken(artificialChainWithoutTheRecentlyAddedPatch.PatchToDate(initialEntity, infinitelyNarrowPatch.Start - TimeSpan.FromTicks(1)));
                 var jdpDelta2 = new JsonDiffPatch();
                 var result = jdpDelta2.Diff(jsonStateBeforeMomentPatch, jsonStateWithMomentPatch);
                 patchToBeAdded.Patch = ToJsonDocument(result);
             }
+
             Remove(infinitelyNarrowPatch);
         }
     }
@@ -269,6 +296,7 @@ private void AddAndKeepTheFuture(TEntity initialEntity, TimeRangePatch patchToBe
                 var patchAtKeyDate = ToJsonDocument(new JsonDiffPatch().Diff(stateJustBeforeThePatch, changedToken));
                 patchToBeAdded.Patch = patchAtKeyDate;
             }
+
             entryWhosePatchShouldBeReplaced.Patch = patchToBeAdded.Patch;
 
             // we also need to modify the following entry.
@@ -385,10 +413,12 @@ private void AddAndOverrideTheFuture(TimeRangePatch patchToBeAdded)
         {
             RemoveAt(futureIndex);
         }
+
         if (GetAll().LastOrDefault(p => p.OverlapsWith(patchToBeAdded)) is { } lastOverlappingPatchWhichIsNotDeleted)
         {
             lastOverlappingPatchWhichIsNotDeleted.ShrinkEndTo(patchToBeAdded.Start);
         }
+
         Add(patchToBeAdded);
     }
 
@@ -405,6 +435,7 @@ public TEntity PatchToDate(TEntity initialEntity, DateTimeOffset keyDate)
     {
         var jdp = new JsonDiffPatch();
         var left = ToJToken(initialEntity);
+        _skippedPatches = new();
         switch (PatchingDirection)
         {
             case PatchingDirection.ParallelWithTime:
@@ -414,7 +445,21 @@ public TEntity PatchToDate(TEntity initialEntity, DateTimeOffset keyDate)
                                  .Where(p => p.Patch!.RootElement.ValueKind != System.Text.Json.JsonValueKind.Null))
                     {
                         var jtokenPatch = JsonConvert.DeserializeObject<JToken>(existingPatch.Patch!.RootElement.GetRawText());
-                        left = jdp.Patch(left, jtokenPatch);
+                        try
+                        {
+                            left = jdp.Patch(left, jtokenPatch);
+                        }
+                        catch (Exception exc) when (_skipConditions?.Any() == true)
+                        {
+                            var entityBeforePatch = _deserialize(left.ToString());
+                            if (_skipConditions?.Any(sc => sc.ShouldSkipPatch(entityBeforePatch, existingPatch, exc)) == true)
+                            {
+                                _skippedPatches.Add(existingPatch);
+                                continue;
+                            }
+
+                            throw; // re-throw
+                        }
                     }
 
                     return _deserialize(JsonConvert.SerializeObject(left));
@@ -426,7 +471,21 @@ public TEntity PatchToDate(TEntity initialEntity, DateTimeOffset keyDate)
                                  .Where(p => p.Patch != null && p.Patch!.RootElement.ValueKind != System.Text.Json.JsonValueKind.Null))
                     {
                         var jtokenPatch = JsonConvert.DeserializeObject<JToken>(existingPatch.Patch!.RootElement.GetRawText());
-                        left = jdp.Unpatch(left, jtokenPatch);
+                        try
+                        {
+                            left = jdp.Unpatch(left, jtokenPatch);
+                        }
+                        catch (Exception exc) when (_skipConditions?.Any() == true)
+                        {
+                            var entityBeforePatch = _deserialize(left.ToString());
+                            if (_skipConditions?.Any(sc => sc.ShouldSkipPatch(entityBeforePatch, existingPatch, exc)) == true)
+                            {
+                                _skippedPatches.Add(existingPatch);
+                                continue;
+                            }
+
+                            throw; // re-throw
+                        }
                     }
 
                     return _deserialize(JsonConvert.SerializeObject(left));
@@ -475,7 +534,7 @@ public Tuple<TEntity, TimeRangePatchChain<TEntity>> Reverse(TEntity initialEntit
                         return backwardsTrp;
                     }).OrderBy(trp => trp.From).ToList();
                     return new Tuple<TEntity, TimeRangePatchChain<TEntity>>(stateAtPlusInfinity,
-                        new TimeRangePatchChain<TEntity>(backwardsPatches, PatchingDirection.AntiParallelWithTime));
+                        new TimeRangePatchChain<TEntity>(backwardsPatches, PatchingDirection.AntiParallelWithTime, skipConditions: _skipConditions));
                 }
             case PatchingDirection.AntiParallelWithTime:
                 {
@@ -488,7 +547,8 @@ public Tuple<TEntity, TimeRangePatchChain<TEntity>> Reverse(TEntity initialEntit
                             patchingDirection: PatchingDirection.ParallelWithTime);
                         return forwardsTrp;
                     }).OrderBy(trp => trp.From).ToList();
-                    return new Tuple<TEntity, TimeRangePatchChain<TEntity>>(stateAtMinusInfinity, new TimeRangePatchChain<TEntity>(forwardPatches, PatchingDirection.ParallelWithTime));
+                    return new Tuple<TEntity, TimeRangePatchChain<TEntity>>(stateAtMinusInfinity,
+                        new TimeRangePatchChain<TEntity>(forwardPatches, PatchingDirection.ParallelWithTime, skipConditions: _skipConditions));
                 }
             default:
                 throw new ArgumentOutOfRangeException();

ChronoJsonDiffPatch/ChronoJsonDiffPatchTests/ListPatchingTests.cs

@@ -118,6 +118,51 @@ public void Reproduce_ArgumentOutOfRangeException()
         corruptedInitialEntity.MyList.RemoveAt(1);
         var applyingPatchesToACorruptedInitialEntity = () => antiparallelChain.PatchToDate(corruptedInitialEntity, keyDate1 - TimeSpan.FromDays(10));
         applyingPatchesToACorruptedInitialEntity.Should().ThrowExactly<ArgumentOutOfRangeException>();
+        antiparallelChain.PatchesHaveBeenSkipped.Should().BeFalse();
+    }
+
+    /// <summary>
+    /// Shows that the error from <see cref="Reproduce_ArgumentOutOfRangeException"/> can be surpressed using a <see cref="ISkipCondition{TEntity}"/>.
+    /// </summary>
+    [Fact]
+    public void Test_ArgumentOutOfRangeException_Can_Be_Surpressed()
+    {
+        var chain = new TimeRangePatchChain<EntityWithList>(skipConditions: new List<ISkipCondition<EntityWithList>>
+            { new SkipPatchesWithUnmatchedListItems<EntityWithList, ListItem>(x => x.MyList) });
+        var initialEntity = new EntityWithList
+        {
+            MyList = new List<ListItem>
+            {
+                new() { Value = "My First Value" },
+            }
+        };
+        var keyDate1 = new DateTimeOffset(2024, 1, 1, 0, 0, 0, TimeSpan.Zero);
+        {
+            var updatedEntity1 = new EntityWithList
+            {
+                MyList = new List<ListItem>
+                {
+                    new() { Value = "My First Value" },
+                    new() { Value = "My Second Value" }
+                }
+            };
+
+            chain.Add(initialEntity, updatedEntity1, keyDate1);
+            chain.Count.Should().Be(2); // [-infinity, keyDate1); [keyDate1, +infinity)
+            ReverseAndRevert(chain, initialEntity);
+        }
+        (var antiparallelInitialEntity, var antiparallelChain) = chain.Reverse(initialEntity);
+        antiparallelInitialEntity.Should().Match<EntityWithList>(x => x.MyList.Count == 2, because: "Initially the list had 2 items");
+        var patchingACorrectInitialEntity = () => antiparallelChain.PatchToDate(antiparallelInitialEntity, keyDate1 - TimeSpan.FromDays(10));
+        patchingACorrectInitialEntity.Should().NotThrow();
+
+        var corruptedInitialEntity =
+            antiparallelInitialEntity; // we modify the reference here, but that's fine. We improve the readability but don't re-use the antiparallelInitialEntity anywhere downstream.
+        corruptedInitialEntity.MyList.RemoveAt(1);
+        var applyingPatchesToACorruptedInitialEntity = () => antiparallelChain.PatchToDate(corruptedInitialEntity, keyDate1 - TimeSpan.FromDays(10));
+        applyingPatchesToACorruptedInitialEntity.Should().NotThrow()
+            .And.Subject.Invoke().Should().BeEquivalentTo(corruptedInitialEntity);
+        antiparallelChain.PatchesHaveBeenSkipped.Should().BeTrue();
     }
 
     private static void ReverseAndRevert(TimeRangePatchChain<EntityWithList> chain, EntityWithList initialEntity)