From a51e777d0bceb0c44d77494673b0967d2d824e95 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 25 Oct 2023 15:11:38 +0700 Subject: [PATCH] remove usage of BinaryFormatter, transition to json serialization. Refactor Revision.cs to support serialization +semver:major --- CHANGELOG.md | 1 + .../RepositorySetup.cs | 2 +- .../VcsDrivers/Mercurial/HgRepository.cs | 6 +- .../VcsDrivers/Mercurial/HgResumeTransport.cs | 106 +++++++++--------- .../VcsDrivers/Mercurial/Revision.cs | 31 +++-- src/LibChorus/retrieval/RevisionInspector.cs | 2 +- src/LibChorus/sync/Synchronizer.cs | 2 +- .../Mercurial/HgResumeTransportTests.cs | 81 ++++++++++++- 8 files changed, 151 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d1f9fbeb..e1b705765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Use UTF-8 in conflict details view - [SIL.Chorus.LibChorus] Add ChorusStorage (the bundle cache) as an Excluded folder +- [SIL.Chorus.LibChorus] Changed HgResumeTransport LastKnownCommonBases to use Json serialization instead of BinaryFormatter ### Fixed diff --git a/src/LibChorus.TestUtilities/RepositorySetup.cs b/src/LibChorus.TestUtilities/RepositorySetup.cs index 404e49083..4b1a55efb 100644 --- a/src/LibChorus.TestUtilities/RepositorySetup.cs +++ b/src/LibChorus.TestUtilities/RepositorySetup.cs @@ -297,7 +297,7 @@ public void CreateRejectForkAndComeBack() Synchronizer.SyncNow(options); Revision revision = Repository.GetRevisionWorkingSetIsBasedOn(); - revision.EnsureParentRevisionInfo(); + revision.EnsureParentRevisionInfo(Repository); Assert.AreEqual(originalTip.Number.LocalRevisionNumber, revision.Parents[0].LocalRevisionNumber, "Should have moved back to original tip."); } diff --git a/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs b/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs index d17047048..5a75eada3 100644 --- a/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs +++ b/src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs @@ -1148,12 +1148,12 @@ public List GetRevisionsFromQueryResultText(string queryResultText) infiniteLoopChecker++; break; case "changeset": - item = new Revision(this); + item = new Revision(); items.Add(item); - item.SetRevisionAndHashFromCombinedDescriptor(value); + item.SetRevisionAndHashFromCombinedDescriptor(value, this); break; case "parent": - item.AddParentFromCombinedNumberAndHash(value); + item.AddParentFromCombinedNumberAndHash(value, this); break; case "branch": diff --git a/src/LibChorus/VcsDrivers/Mercurial/HgResumeTransport.cs b/src/LibChorus/VcsDrivers/Mercurial/HgResumeTransport.cs index b382d2b78..b2e506c6a 100644 --- a/src/LibChorus/VcsDrivers/Mercurial/HgResumeTransport.cs +++ b/src/LibChorus/VcsDrivers/Mercurial/HgResumeTransport.cs @@ -3,10 +3,10 @@ using System.IO; using System.Linq; using System.Net; -using System.Runtime.Serialization.Formatters.Binary; using System.Text; using Chorus.Properties; using Chorus.Utilities; +using Newtonsoft.Json; using SIL.Progress; namespace Chorus.VcsDrivers.Mercurial @@ -32,7 +32,7 @@ public class HgResumeTransport : IHgTransport private const int MaximumChunkSize = 20000000; // 20MB private const int TimeoutInSeconds = 30; private const int TargetTimeInSeconds = 7; - internal const string RevisionCacheFilename = "revisioncache.db"; + internal const string RevisionCacheFilename = "revisioncache.json"; internal int RevisionRequestQuantity = 200; /// @@ -65,59 +65,53 @@ private string RepoIdentifier /// private List LastKnownCommonBases { - get + get => GetLastKnownCommonBases(PathToLocalStorage, _apiServer.Host); + set => SetLastKnownCommonBases(value, PathToLocalStorage, _apiServer.Host); + } + + public static List GetLastKnownCommonBases(string storagePath, string remoteId) + { + if (!Directory.Exists(storagePath)) { - string storagePath = PathToLocalStorage; - if (!Directory.Exists(storagePath)) - { - Directory.CreateDirectory(storagePath); - } - string filePath = Path.Combine(storagePath, RevisionCacheFilename); - if (File.Exists(filePath)) - { - List revisions = ReadServerRevisionCache(filePath); - if(revisions != null) - { - var remoteId = _apiServer.Host; - var result = revisions.Where(x => x.RemoteId == remoteId); - if (result.Count() > 0) - { - var results = new List(result.Count()); - foreach (var rev in result) - { - results.Add(rev.Revision); - } - return results; - } - } - } - return new List(); + Directory.CreateDirectory(storagePath); } - set + + string filePath = Path.Combine(storagePath, RevisionCacheFilename); + if (File.Exists(filePath)) { - string remoteId = _apiServer.Host; - var serverRevs = new List(); - foreach (var revision in value) + List revisions = ReadServerRevisionCache(filePath); + if (revisions != null) { - serverRevs.Add(new ServerRevision(remoteId, revision)); - } - var storagePath = PathToLocalStorage; - if (!Directory.Exists(storagePath)) - { - Directory.CreateDirectory(storagePath); + return revisions.Where(x => x.RemoteId == remoteId).Select(r => r.Revision).ToList(); } + } - var filePath = Path.Combine(storagePath, RevisionCacheFilename); - var fileContents = ReadServerRevisionCache(filePath); - fileContents.RemoveAll(x => x.RemoteId == remoteId); - serverRevs.AddRange(fileContents); - using(Stream stream = File.Open(filePath, FileMode.Create)) - { - var bFormatter = new BinaryFormatter(); - bFormatter.Serialize(stream, serverRevs); - stream.Close(); - } - return; + return new List(); + } + + public static void SetLastKnownCommonBases(List revisions, string storagePath, string remoteId) + { + var serverRevs = new List(); + foreach (var revision in revisions) + { + serverRevs.Add(new ServerRevision(remoteId, revision)); + } + + if (!Directory.Exists(storagePath)) + { + Directory.CreateDirectory(storagePath); + } + + var filePath = Path.Combine(storagePath, RevisionCacheFilename); + var fileContents = ReadServerRevisionCache(filePath); + fileContents.RemoveAll(x => x.RemoteId == remoteId); + serverRevs.AddRange(fileContents); + var jsonSerializer = JsonSerializer.CreateDefault(); + using (var stream = File.Open(filePath, FileMode.Create)) + using (var jsonWriter = new JsonTextWriter(new StreamWriter(stream))) + { + jsonSerializer.Serialize(jsonWriter, serverRevs); + jsonWriter.Flush(); } } @@ -129,11 +123,11 @@ internal static List ReadServerRevisionCache(string filePath) { if (File.Exists(filePath)) { + var jsonSerializer = JsonSerializer.CreateDefault(); using (Stream stream = File.Open(filePath, FileMode.Open)) + using (var jsonReader = new JsonTextReader(new StreamReader(stream))) { - var bFormatter = new BinaryFormatter(); - var revisions = bFormatter.Deserialize(stream) as List; - stream.Close(); + var revisions = jsonSerializer.Deserialize>(jsonReader); return revisions; } } @@ -143,16 +137,16 @@ internal static List ReadServerRevisionCache(string filePath) } } - [Serializable] internal class ServerRevision { public readonly string RemoteId; public readonly Revision Revision; - public ServerRevision(string id, Revision rev) + //parameter names must be the same as the field names for json serialization to work + public ServerRevision(string remoteId, Revision revision) { - RemoteId = id; - Revision = rev; + RemoteId = remoteId; + Revision = revision; } } diff --git a/src/LibChorus/VcsDrivers/Mercurial/Revision.cs b/src/LibChorus/VcsDrivers/Mercurial/Revision.cs index e13f2ac43..cc3f5dfe3 100644 --- a/src/LibChorus/VcsDrivers/Mercurial/Revision.cs +++ b/src/LibChorus/VcsDrivers/Mercurial/Revision.cs @@ -5,11 +5,11 @@ namespace Chorus.VcsDrivers.Mercurial { - [Serializable] + /// + /// this class is json serialized, so don't change the names of the properties and don't store service objects in it that can't be serialized + /// public class Revision { - [NonSerialized] - private readonly HgRepository _repository; public string UserId { get; set; } public RevisionNumber Number; public string Summary { get; set; } @@ -27,9 +27,8 @@ public bool HasAtLeastOneParent } - public Revision(HgRepository repository) + public Revision() { - _repository = repository; Parents = new List(); Tag = string.Empty; Branch = string.Empty; @@ -38,7 +37,7 @@ public Revision(HgRepository repository) } public Revision(HgRepository repository, string name, string localRevisionNumber, string hash, string comment) - :this(repository) + :this() { UserId = name; Number = new RevisionNumber(repository, localRevisionNumber, hash); @@ -51,9 +50,9 @@ public Revision(HgRepository repository, string branchName, string userName, str Branch = branchName; } - public void SetRevisionAndHashFromCombinedDescriptor(string descriptor) + public void SetRevisionAndHashFromCombinedDescriptor(string descriptor, HgRepository repository) { - Number = new RevisionNumber(_repository, descriptor); + Number = new RevisionNumber(repository, descriptor); } public bool IsMatchingStub(Revision stub) { @@ -66,30 +65,29 @@ public IEnumerable GetLocalNumbersOfParents() // return Repository.GetParentsOfRevision(this.Number.LocalRevisionNumber); } - public void AddParentFromCombinedNumberAndHash(string descriptor) + public void AddParentFromCombinedNumberAndHash(string descriptor, HgRepository repository) { - Parents.Add(new RevisionNumber(_repository, descriptor)); - + Parents.Add(new RevisionNumber(repository, descriptor)); } - public bool IsDirectDescendantOf(Revision revision) + public bool IsDirectDescendantOf(Revision revision, HgRepository repository) { - EnsureParentRevisionInfo(); + EnsureParentRevisionInfo(repository); //TODO: this is only checking direct descendant return Parents.Any(p => p.Hash == revision.Number.Hash); } - /// + /// /// I can't for the life of me get hg to indicate parentage in the "hg log" (even with templates /// asking for parents), if the revision is not the result of a merge. And yet, it's expensive /// to ask again for every single one. So as a hack, for now, this can be called on a revision /// where we really need to know the parent. /// - public void EnsureParentRevisionInfo() + public void EnsureParentRevisionInfo(HgRepository repository) { if (this.Parents.Count == 0) { - Parents.AddRange(_repository.GetParentsRevisionNumbers(this.Number.LocalRevisionNumber)); + Parents.AddRange(repository.GetParentsRevisionNumbers(this.Number.LocalRevisionNumber)); } } @@ -99,7 +97,6 @@ public bool GetMatchesLocalOrHash(string localOrHash) } } - [Serializable] public class RevisionNumber { internal RevisionNumber() diff --git a/src/LibChorus/retrieval/RevisionInspector.cs b/src/LibChorus/retrieval/RevisionInspector.cs index fdae8ce08..218419627 100644 --- a/src/LibChorus/retrieval/RevisionInspector.cs +++ b/src/LibChorus/retrieval/RevisionInspector.cs @@ -31,7 +31,7 @@ public RevisionInspector(HgRepository repository, ChorusFileTypeHandlerCollectio public IEnumerable GetChangeRecords(Revision revision) { var changes = new List(); - revision.EnsureParentRevisionInfo(); + revision.EnsureParentRevisionInfo(Repository); if (!revision.HasAtLeastOneParent) { diff --git a/src/LibChorus/sync/Synchronizer.cs b/src/LibChorus/sync/Synchronizer.cs index 9075ea8c1..01786db21 100644 --- a/src/LibChorus/sync/Synchronizer.cs +++ b/src/LibChorus/sync/Synchronizer.cs @@ -562,7 +562,7 @@ private void UpdateToTheDescendantRevision(HgRepository repository, Revision par // when there are more than 2 people merging and there's a failure or a no-op merge happened foreach (var head in heads) { - if (parent.Number.Hash == head.Number.Hash || (head.Branch == parent.Branch && head.IsDirectDescendantOf(parent))) + if (parent.Number.Hash == head.Number.Hash || (head.Branch == parent.Branch && head.IsDirectDescendantOf(parent, repository))) { repository.RollbackWorkingDirectoryToRevision(head.Number.LocalRevisionNumber); _sychronizerAdjunct.SimpleUpdate(_progress, true); diff --git a/src/LibChorusTests/VcsDrivers/Mercurial/HgResumeTransportTests.cs b/src/LibChorusTests/VcsDrivers/Mercurial/HgResumeTransportTests.cs index 1b57e9a7e..b18d20f2b 100644 --- a/src/LibChorusTests/VcsDrivers/Mercurial/HgResumeTransportTests.cs +++ b/src/LibChorusTests/VcsDrivers/Mercurial/HgResumeTransportTests.cs @@ -813,7 +813,86 @@ public void CalculateEstimatedTimeRemaining_ExpectedTimeRemainingString() Assert.That(HgResumeTransport.CalculateEstimatedTimeRemaining(bundleSize, chunkSize, startOfWindow), Is.EqualTo("(about 5 minutes)")); } - + [Test] + public void RoundTripLastKnownCommonBases_ResultsAreTheSame() + { + string storagePath = Path.Combine(Path.GetTempPath(), "HgResumeTransportTest"); + var expectedRevisions = new List() + { + new Revision() + { + Branch = "test1", + Number = new RevisionNumber() + { + Hash = "hash1", + LocalRevisionNumber = "abc", + LongHash = "long hash" + }, + Summary = "summary", + UserId = "123", + Tag = "hello", + DateString = "a date?", + Parents = + { + new RevisionNumber() + { + LocalRevisionNumber = "parent1", + Hash = "parent hash", + LongHash = "parent long hash" + } + } + }, + new Revision() + { + Branch = "test2", + Number = new RevisionNumber() + { + Hash = "hash2", + LocalRevisionNumber = "def", + LongHash = "long hash 2" + }, + Summary = "summary 2", + UserId = "456", + Tag = "hello 2", + DateString = "a date 2?", + Parents = + { + new RevisionNumber() + { + LocalRevisionNumber = "parent2", + Hash = "parent hash 2", + LongHash = "parent long hash 2" + } + } + } + }; + + HgResumeTransport.SetLastKnownCommonBases(expectedRevisions, storagePath, "id1"); + var actualRevisions = HgResumeTransport.GetLastKnownCommonBases(storagePath, "id1"); + Assert.That(actualRevisions.Count, Is.EqualTo(expectedRevisions.Count)); + for (int i = 0; i < expectedRevisions.Count; i++) + { + var actualRevision = actualRevisions[i]; + var expectedRevision = expectedRevisions[i]; + Assert.That(actualRevision.Branch, Is.EqualTo(expectedRevision.Branch)); + Assert.That(actualRevision.Number.Hash, Is.EqualTo(expectedRevision.Number.Hash)); + Assert.That(actualRevision.Number.LocalRevisionNumber, Is.EqualTo(expectedRevision.Number.LocalRevisionNumber)); + Assert.That(actualRevision.Number.LongHash, Is.EqualTo(expectedRevision.Number.LongHash)); + Assert.That(actualRevision.Summary, Is.EqualTo(expectedRevision.Summary)); + Assert.That(actualRevision.UserId, Is.EqualTo(expectedRevision.UserId)); + Assert.That(actualRevision.Tag, Is.EqualTo(expectedRevision.Tag)); + Assert.That(actualRevision.DateString, Is.EqualTo(expectedRevision.DateString)); + Assert.That(actualRevision.Parents.Count, Is.EqualTo(expectedRevision.Parents.Count)); + for (int j = 0; j < expectedRevision.Parents.Count; j++) + { + var actualRevisionParent = actualRevision.Parents[j]; + var expectedRevisionParent = expectedRevision.Parents[j]; + Assert.That(actualRevisionParent.LocalRevisionNumber, Is.EqualTo(expectedRevisionParent.LocalRevisionNumber)); + Assert.That(actualRevisionParent.Hash, Is.EqualTo(expectedRevisionParent.Hash)); + Assert.That(actualRevisionParent.LongHash, Is.EqualTo(expectedRevisionParent.LongHash)); + } + } + } private class BranchTestAdjunct : ISychronizerAdjunct {