Merge pull request #328 from sillsdev/remove-binary-formatter

remove usage of BinaryFormatter

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
 

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.");
 		}
 

src/LibChorus/VcsDrivers/Mercurial/HgRepository.cs

@@ -1148,12 +1148,12 @@ public List<Revision> 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":

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;
 
 		///<summary>
@@ -65,59 +65,53 @@ private string RepoIdentifier
 		///</summary>
 		private List<Revision> LastKnownCommonBases
 		{
-			get
+			get => GetLastKnownCommonBases(PathToLocalStorage, _apiServer.Host);
+			set => SetLastKnownCommonBases(value, PathToLocalStorage, _apiServer.Host);
+		}
+
+		public static List<Revision> 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<ServerRevision> 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<Revision>(result.Count());
-							foreach (var rev in result)
-							{
-								results.Add(rev.Revision);
-							}
-							return results;
-						}
-					}
-				}
-				return new List<Revision>();
+				Directory.CreateDirectory(storagePath);
 			}
-			set
+
+			string filePath = Path.Combine(storagePath, RevisionCacheFilename);
+			if (File.Exists(filePath))
 			{
-				string remoteId = _apiServer.Host;
-				var serverRevs = new List<ServerRevision>();
-				foreach (var revision in value)
+				List<ServerRevision> 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<Revision>();
+		}
+
+		public static void SetLastKnownCommonBases(List<Revision> revisions, string storagePath, string remoteId)
+		{
+			var serverRevs = new List<ServerRevision>();
+			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<ServerRevision> 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<ServerRevision>;
-					stream.Close();
+					var revisions = jsonSerializer.Deserialize<List<ServerRevision>>(jsonReader);
 					return revisions;
 				}
 			}
@@ -143,16 +137,16 @@ internal static List<ServerRevision> 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;
 			}
 		}
 

src/LibChorus/VcsDrivers/Mercurial/Revision.cs

@@ -5,11 +5,11 @@
 
 namespace Chorus.VcsDrivers.Mercurial
 {
-	[Serializable]
+	/// <summary>
+	/// 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
+	/// </summary>
 	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<RevisionNumber>();
 			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<RevisionNumber> 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);
 		}
 
-	 /// <summary>
+		/// <summary>
 		/// 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.
 		/// </summary>
-		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()

src/LibChorus/retrieval/RevisionInspector.cs

@@ -31,7 +31,7 @@ public RevisionInspector(HgRepository repository, ChorusFileTypeHandlerCollectio
 		public IEnumerable<IChangeReport> GetChangeRecords(Revision revision)
 		{
 			var changes = new List<IChangeReport>();
-			revision.EnsureParentRevisionInfo();
+			revision.EnsureParentRevisionInfo(Repository);
 
 			if (!revision.HasAtLeastOneParent)
 			{

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);

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<Revision>()
+			{
+				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
 		{