From 60bf56fa7bc97f175107018745219fcaef884855 Mon Sep 17 00:00:00 2001 From: konstantin Date: Wed, 4 Oct 2023 16:20:38 +0200 Subject: [PATCH] Throw meaningful `ArgumentException` for inconsistent source data (#56) instead of hard to understand exception from base class --- .../TimeRangePatchChain.cs | 18 +++++++++- .../TimeRangePatchChainTests.cs | 35 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/ChronoJsonDiffPatch/ChronoJsonDiffPatch/TimeRangePatchChain.cs b/ChronoJsonDiffPatch/ChronoJsonDiffPatch/TimeRangePatchChain.cs index d737bcd..4ee4edb 100644 --- a/ChronoJsonDiffPatch/ChronoJsonDiffPatch/TimeRangePatchChain.cs +++ b/ChronoJsonDiffPatch/ChronoJsonDiffPatch/TimeRangePatchChain.cs @@ -75,7 +75,23 @@ private static IEnumerable PrepareForTimePeriodChainConstructor( return new List(); } - return timeperiods.OrderBy(tp => tp.Start); + var result = timeperiods.OrderBy(tp => tp.Start); + var ambigousStarts = result.GroupBy(tp => tp.To).Where(g => g.Count() > 1).Select(g => g.Select(x => x.Start).Distinct()); + var ambigousEnds = result.GroupBy(tp => tp.End).Where(g => g.Count() > 1).Select(g => g.Select(x => x.End).Distinct()); ; + bool baseConstructorIsLikelyToCrash = ambigousStarts.Any() || ambigousEnds.Any(); + if (baseConstructorIsLikelyToCrash) + { + try + { + _ = new TimePeriodChain(result); // a test if the base constructor will actually crash? + } + 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 ({ambigousStarts}) or ends ({ambigousEnds})", innerException: invalidOpException); + } + } + return result; } /// diff --git a/ChronoJsonDiffPatch/ChronoJsonDiffPatchTests/TimeRangePatchChainTests.cs b/ChronoJsonDiffPatch/ChronoJsonDiffPatchTests/TimeRangePatchChainTests.cs index 980662a..f2c52e0 100644 --- a/ChronoJsonDiffPatch/ChronoJsonDiffPatchTests/TimeRangePatchChainTests.cs +++ b/ChronoJsonDiffPatch/ChronoJsonDiffPatchTests/TimeRangePatchChainTests.cs @@ -719,4 +719,39 @@ public void Test_Patching_Backwards() AssertBasicSanity(myEntity, trpCollection); */ } + + [Fact] + public void Test_Patching_Backwards_Throws_Meaningful_Error_For_Inconsistent_Data() + { + var trpCollection = new TimeRangePatchChain(patchingDirection: PatchingDirection.ParallelWithTime); + var myEntity = new DummyClass + { + MyProperty = "Foo" + }; + { + var keyDate1 = new DateTimeOffset(2034, 1, 1, 0, 0, 0, TimeSpan.Zero); + var myChangedEntity = new DummyClass + { + MyProperty = "Bar" + }; + trpCollection.Add(myEntity, myChangedEntity, keyDate1); + } + { + var keyDate2 = new DateTimeOffset(2035, 1, 1, 0, 0, 0, TimeSpan.Zero); + var myChangedEntity = new DummyClass + { + MyProperty = "Baz" + }; + trpCollection.Add(myEntity, myChangedEntity, keyDate2); + } + var allPatches = trpCollection.GetAll().ToList(); + allPatches.Should().HaveCount(3); + + allPatches[1].End = DateTime.MaxValue; // let's a create chain, that is no longer self-consistent and has 2 elements with +infinity as end date + allPatches.Where(p => p.End == DateTime.MaxValue).Should().HaveCount(2); + Action creatingAChainFromInconsistentPatches = () => new TimeRangePatchChain(allPatches, PatchingDirection.ParallelWithTime); + creatingAChainFromInconsistentPatches.Should().Throw() + .Where(ae => ae.Message.Contains("The given periods contain ambiguous starts")) + .And.InnerException.Should().BeOfType(); + } }