SF-1992 Fail sync if push failed

AGENTS.md

@@ -82,6 +82,7 @@ This repository contains three interconnected applications:
 - Do put comments into the code if the intent is not clear from the code.
 - All classes and interfaces should have a comment to briefly explain why it is there and what its purpose is in the overall system, even if it seems obvious.
 - Please do not fail to add a comment to any classes or interfaces that are created. All classes and interfaces should have a comment.
+- Use good argument and variable names that explain themselves without needing a comment. Well named arguments or variables are better than unclearly named arguments or variables with a comment.
 
 # TypeScript language
 

src/SIL.XForge.Scripture/Services/HgWrapper.cs

@@ -6,7 +6,7 @@
 
 namespace SIL.XForge.Scripture.Services;
 
-/// <summary> A wrapper for the <see cref="Hg" /> class for calling Mercurial. </summary>
+/// <summary>A wrapper for the <see cref="Hg" /> class for calling Mercurial.</summary>
 public class HgWrapper : IHgWrapper
 {
     public static string RunCommand(string repository, string cmd)
@@ -16,14 +16,17 @@ public static string RunCommand(string repository, string cmd)
         return Hg.Default.RunCommand(repository, cmd).StdOut;
     }
 
-    public static byte[] Bundle(string repository, params string[] heads)
+    /// <summary>
+    /// Returns a Mercurial bundle containing changesets above the given base revisions.
+    /// </summary>
+    public byte[] Bundle(string repository, params string[] baseRevisions)
     {
         if (Hg.Default == null)
             throw new InvalidOperationException("Hg default has not been set.");
-        return Hg.Default.Bundle(repository, heads);
+        return Hg.Default.Bundle(repository, baseRevisions);
     }
 
-    public static string[] Pull(string repository, byte[] bundle)
+    public string[] Pull(string repository, byte[] bundle)
     {
         if (Hg.Default == null)
             throw new InvalidOperationException("Hg default has not been set.");
@@ -72,12 +75,12 @@ public string GetLastPublicRevision(string repository)
     /// <summary>
     /// Returns the currently checked out revision of an hg repository.
     /// </summary>
-    public string GetRepoRevision(string repositoryPath)
+    public string GetRepoRevision(string repository)
     {
-        string rev = RunCommand(repositoryPath, "log --limit 1 --rev . --template {node}");
+        string rev = RunCommand(repository, "log --limit 1 --rev . --template {node}");
         if (string.IsNullOrWhiteSpace(rev))
         {
-            throw new InvalidDataException($"Unable to determine repo revision for hg repo at {repositoryPath}");
+            throw new InvalidDataException($"Unable to determine repo revision for hg repo at {repository}");
         }
         return rev;
     }
@@ -98,9 +101,9 @@ public void RestoreRepository(string destination, string backupFile)
     }
 
     /// <summary>
-    /// Mark all changesets available on the PT server public.
+    /// Mark all changesets as public.
     /// </summary>
-    /// <param name="repository">The repository.</param>
+    /// <param name="repository">The repository path.</param>
     public void MarkSharedChangeSetsPublic(string repository) => RunCommand(repository, "phase -p -r 'tip'");
 
     /// <summary>
@@ -136,5 +139,5 @@ public void SetDefault(Hg hgDefault)
     /// `git checkout --force --detach COMMITTISH`
     /// Changes to tracked files will be discarded. Untracked files are left in place without being cleaned up.
     /// </summary>
-    public void Update(string repositoryPath, string rev) => Hg.Default.Update(repositoryPath, rev);
+    public void Update(string repository, string rev) => Hg.Default.Update(repository, rev);
 }

src/SIL.XForge.Scripture/Services/IHgWrapper.cs

@@ -2,6 +2,7 @@
 
 namespace SIL.XForge.Scripture.Services;
 
+/// <summary>Mercurial operations</summary>
 public interface IHgWrapper
 {
     void SetDefault(Hg hgDefault);
@@ -11,8 +12,10 @@ public interface IHgWrapper
     void BackupRepository(string repository, string backupFile);
     void RestoreRepository(string destination, string backupFile);
     string GetLastPublicRevision(string repository);
-    string GetRepoRevision(string repositoryPath);
+    string GetRepoRevision(string repository);
     void MarkSharedChangeSetsPublic(string repository);
+    string[] Pull(string repository, byte[] bundle);
+    byte[] Bundle(string repository, params string[] baseRevisions);
     string RecentLogGraph(string repositoryPath);
     string[] GetDraftRevisions(string repositoryPath);
 }

src/SIL.XForge.Scripture/Services/JwtInternetSharedRepositorySource.cs

@@ -13,12 +13,15 @@
 
 namespace SIL.XForge.Scripture.Services;
 
-/// <summary> An internet shared repository source that networks using JWT authenticated REST clients. </summary>
+/// <summary>
+/// An internet shared repository source that networks using JWT authenticated REST clients.
+/// </summary>
 public class JwtInternetSharedRepositorySource : InternetSharedRepositorySource, IInternetSharedRepositorySource
 {
     private readonly JwtRestClient _registryClient;
     private readonly IHgWrapper _hgWrapper;
     private readonly ILogger _logger;
+    private readonly int _maxJsonLogChars = 200;
 
     public JwtInternetSharedRepositorySource(
         string accessToken,
@@ -62,9 +65,9 @@ public bool CanUserAuthenticateToPTArchives()
     /// Uses the a REST client to pull from the Paratext send/receive server. This overrides the base implementation
     /// to avoid needing the current user's Paratext registration code to get the base revision.
     /// </summary>
-    public override string[] Pull(string repository, SharedRepository pullRepo)
+    public override string[] Pull(string repositoryPath, SharedRepository pullRepo)
     {
-        string baseRev = _hgWrapper.GetLastPublicRevision(repository);
+        string baseRev = _hgWrapper.GetLastPublicRevision(repositoryPath);
 
         // Get bundle
         string guid = Guid.NewGuid().ToString();
@@ -92,24 +95,33 @@ public override string[] Pull(string repository, SharedRepository pullRepo)
             return [];
 
         // Use bundle
-        string[] changeSets = HgWrapper.Pull(repository, bundle);
+        string[] changeSets = _hgWrapper.Pull(repositoryPath, bundle);
 
-        _hgWrapper.MarkSharedChangeSetsPublic(repository);
+        _hgWrapper.MarkSharedChangeSetsPublic(repositoryPath);
         return changeSets;
     }
 
     /// <summary>
     /// Uses the a REST client to push to the Paratext send/receive server. This overrides the base implementation
     /// to avoid needing the current user's Paratext registration code to get the base revision.
     /// </summary>
-    public override void Push(string repository, SharedRepository pushRepo)
+    public override void Push(string repositoryPath, SharedRepository pushRepo)
     {
-        string baseRev = _hgWrapper.GetLastPublicRevision(repository);
+        string baseRev = _hgWrapper.GetLastPublicRevision(repositoryPath);
 
         // Create bundle
-        byte[] bundle = HgWrapper.Bundle(repository, baseRev);
+        byte[] bundle = _hgWrapper.Bundle(repositoryPath, baseRev);
         if (bundle.Length == 0)
+        {
+            _logger.LogInformation($"Not pushing a 0 Byte bundle for project PT ID {pushRepo.SendReceiveId.Id}.");
             return;
+        }
+
+        string localTip = _hgWrapper.GetRepoRevision(repositoryPath);
+        _logger.LogInformation(
+            $"Pushing bundle of {bundle.Length} Bytes to S/R server for project PT ID "
+                + $"{pushRepo.SendReceiveId.Id}. Base revision {baseRev ?? "(null)"}. Local tip {localTip}."
+        );
 
         // Send bundle
         string guid = Guid.NewGuid().ToString();
@@ -128,7 +140,78 @@ public override void Push(string repository, SharedRepository pushRepo)
             "no"
         );
 
-        _hgWrapper.MarkSharedChangeSetsPublic(repository);
+        (bool isRevOnServer, int serverRevCount, string? serverLastRev) = CheckIfRevisionIsOnServer(pushRepo, localTip);
+        if (!isRevOnServer)
+        {
+            throw new InvalidOperationException(
+                $"Push verification failed for project PT ID {pushRepo.SendReceiveId.Id}. "
+                    + $"Expected revision {localTip} was not found in the server's revision history. "
+                    + $"Server has {serverRevCount} revisions. Last server revision: {serverLastRev ?? "(null)"}."
+            );
+        }
+
+        _hgWrapper.MarkSharedChangeSetsPublic(repositoryPath);
+    }
+
+    /// <summary>
+    /// Returns whether the expected revision is present on the Paratext Send/Receive server.
+    /// </summary>
+    internal (bool isRevOnServer, int serverRevCount, string? serverLastRev) CheckIfRevisionIsOnServer(
+        SharedRepository serverRepo,
+        string expectedRevision
+    )
+    {
+        string projRevHistResponse = GetClient()
+            .Get("projrevhist", "proj", serverRepo.ScrTextName, "projid", serverRepo.SendReceiveId.Id, "all", "1");
+
+        JObject jsonResult = JObject.Parse(projRevHistResponse);
+        if (jsonResult["project"]?["revision_history"]?["revisions"] is not JArray revisions)
+        {
+            string truncatedResult = FormatAndTruncate(jsonResult, _maxJsonLogChars);
+            _logger.LogWarning(
+                $"Getting projrevhist unexpectedly received null revisions for PT project ID {serverRepo.SendReceiveId.Id}. The JSON result is: {truncatedResult}"
+            );
+            return (false, 0, null);
+        }
+
+        bool isRevOnServer = revisions.Any(r =>
+            string.Equals(r["id"]?.ToString(), expectedRevision, StringComparison.Ordinal)
+        );
+        int serverRevCount = revisions.Count;
+        string? serverLastRev = GetFirstElementId(revisions);
+
+        return (isRevOnServer, serverRevCount, serverLastRev);
+    }
+
+    /// <summary>
+    /// Returns the first element's "id" value, if possible.
+    /// </summary>
+    private static string? GetFirstElementId(JArray revisions)
+    {
+        if (revisions.Count > 0)
+            return revisions[0]["id"]?.ToString();
+        return null;
+    }
+
+    /// <summary>
+    /// Formats JSON and truncates if too long.
+    /// </summary>
+    private static string FormatAndTruncate(JObject jsonResult, int maxChars)
+    {
+        string prettyJson = jsonResult.ToString(Newtonsoft.Json.Formatting.Indented);
+        return TruncateLogString(prettyJson, maxChars);
+    }
+
+    /// <summary>
+    /// Truncates a string to the configured character count if needed and appends truncation details.
+    /// </summary>
+    private static string TruncateLogString(string value, int maxChars)
+    {
+        if (value.Length <= maxChars)
+            return value;
+
+        int truncatedChars = value.Length - maxChars;
+        return value[..maxChars] + Environment.NewLine + $"... (truncated {truncatedChars} more characters)";
     }
 
     /// <summary>

src/SIL.XForge.Scripture/Services/LazyScrTextCollection.cs

@@ -32,7 +32,8 @@ public void Initialize(string projectsPath)
     }
 
     /// <summary>
-    /// Get a ScrText for a given user from the data for a paratext project with the target project ID and type.
+    /// Get a ScrText for a given user from the data for a paratext project with the target project ID and type. Not to
+    /// be confused with ParatextData ScrTextCollection.FindById, which this is not an override of.
     /// </summary>
     /// <param name="ptUsername"> The username of the user retrieving the ScrText. </param>
     /// <param name="projectId"> The ID of the target project. </param>