diff --git a/DistFiles/localization/en/Bloom.xlf b/DistFiles/localization/en/Bloom.xlf index d0a8ac8a5e1e..685d3c1d1088 100644 --- a/DistFiles/localization/en/Bloom.xlf +++ b/DistFiles/localization/en/Bloom.xlf @@ -314,6 +314,28 @@ There was a problem while saving. Please return to the previous page and make sure it looks correct. ID: Browser.ProblemSaving + + The collection "{0}" requires Bloom {1} or greater. You are running Bloom {2}. + ID: Collection.CollectionRequiresNewerVersion + {0} is the name of the collection, {1} is the version it requires, {2} is the version of Bloom that is running. + + + This collection needs a newer version of Bloom. + ID: Collection.NewerVersionNeededHeader + + + This collection needs Bloom {0}, but you already have the newest Bloom available to you. + ID: Collection.NothingNewerAvailable + {0} is the version the collection requires. + + + Open a Different Collection + ID: Collection.OpenDifferentCollection + + + Upgrade Bloom + ID: Collection.UpgradeBloom + About Bloom Subscriptions ID: CollectionSettingsDialog.AboutBloomSubscriptions diff --git a/src/BloomExe/ApplicationContainer.cs b/src/BloomExe/ApplicationContainer.cs index d8d55cf72191..d0331ded4687 100644 --- a/src/BloomExe/ApplicationContainer.cs +++ b/src/BloomExe/ApplicationContainer.cs @@ -70,7 +70,12 @@ public ApplicationContainer() // containers, which is what we want for all the application singletons. .SingleInstance() .Where(t => - new[] { typeof(CommonApi), typeof(NewCollectionWizardApi) }.Contains(t) + new[] + { + typeof(CommonApi), + typeof(NewCollectionWizardApi), + typeof(ProgressDialogApi), + }.Contains(t) ); _container = builder.Build(); @@ -87,6 +92,17 @@ public ApplicationContainer() var server = _container.Resolve(); _container.Resolve().RegisterWithApiHandler(server.ApiHandler); _container.Resolve().RegisterWithApiHandler(server.ApiHandler); + // A progress dialog has to be possible before any collection is open: the "this + // collection needs a newer Bloom" dialog upgrades Bloom right there, and the dialog it + // shows while doing so talks over these endpoints. This belongs here rather than in + // ProjectContext (where it used to be) because all of ProgressDialogApi's handlers are + // static and know nothing about a project -- and because it can only be registered + // ONCE: RegisterEndpointHandler does a Dictionary.Add, which throws on a duplicate + // key, and application-level registrations are deliberately not cleared between + // collections, so a second registration would never go away. + _container + .Resolve() + .RegisterWithApiHandler(server.ApiHandler); server.ApiHandler.RecordApplicationLevelHandlers(); } diff --git a/src/BloomExe/ApplicationUpdateSupport.cs b/src/BloomExe/ApplicationUpdateSupport.cs index 42e72963c9e4..869a07cd8b92 100644 --- a/src/BloomExe/ApplicationUpdateSupport.cs +++ b/src/BloomExe/ApplicationUpdateSupport.cs @@ -75,6 +75,44 @@ enum UploadStatus static UploadStatus _status = UploadStatus.NothingKnown; private static Exception _updateException; + /// + /// True while a check for updates, or the download one led to, is still running. Bloom + /// starts a check of its own a minute after a collection opens, so someone can ask for an + /// upgrade while one is already under way; the minimum-version dialog waits this out + /// rather than being told Bloom is busy (BL-16690). + /// + internal static bool IsBusyCheckingOrDownloading => + _status == UploadStatus.LookingForUpdates || _status == UploadStatus.Downloading; + + /// + /// How far along the current download is, for the same dialog to show real progress while + /// it waits: the wait can be twenty minutes on the connections where it happens at all, and + /// a bar sitting at 0% that whole time reads as a hang. Gated on the status so a finished + /// or cancelled download's last value cannot show up in a later attempt. + /// + internal static int CurrentDownloadPercent => + _status == UploadStatus.Downloading ? _lastDownloadPercent : 0; + + private static int _lastDownloadPercent; + + /// + /// True once we have arranged to install a downloaded update as Bloom exits. We must only do + /// that once: applying the same update twice is how an upgrade fails, or restarts Bloom when + /// nobody asked it to. + /// + private static bool _willInstallUpdateOnExit; + + // These three are all "this should not happen" cases, so we have never localized them. + // They are constants only because each is said twice: once to the user, and once to the + // caller through UpdateReporter.Finished, so that a caller which has to repeat it says + // exactly what we said. + private const string kRestartToTryAgainMessage = + "Restart Bloom to try checking for updates again"; + private const string kUnableToCheckMessage = + "Bloom was unable to check for updates. Restart to try again."; + private const string kUnableToDownloadMessage = + "Bloom was unable to download and install updates. Restart to try again."; + /// /// See if any updates are available and if approved, download them. Once they are ready a notification /// pops up and the user can restart Bloom to run the new version. (Or if you don't, they will get installed @@ -84,11 +122,18 @@ enum UploadStatus /// An action that is executed if the user clicks the toast that suggests /// a restart. This is the responsibility of the caller (the workspace view). It /// just shuts down Bloom; the update and restart are managed automatically by Velopack. + /// Where to say what is happening, and where to report the outcome. + /// Toasts, if not given, which is the normal case. + /// Skip the "updates are available, do you want + /// them?" step: the user has said yes somewhere else, so asking again would be odd. internal static async void CheckForAVelopackUpdate( BloomUpdateMessageVerbosity verbosity, - Action restartBloom + Action restartBloom, + UpdateReporter reporter = null, + bool userHasAlreadyAgreedToUpdate = false ) { + reporter = reporter ?? new ToastUpdateReporter(); // In Bloom 6.3, we updated to DotNet 8. So at this point, there's no reason to check OS versions; // This Velopack-based update code only runs in 6.3, and 6.3 (at least by the time we release a beta) // only runs on an OS that is at least 10; in fact, it has to be quite a recent 10. But that check @@ -128,29 +173,50 @@ Action restartBloom // The rest of this method looks for them and deals with the results break; case UploadStatus.Failed: - // Hopefully we don't get into this state. Don't think it's worth localizing. - ShowToastForError("Restart Bloom to try checking for updates again"); + // Hopefully we don't get into this state. + ReportFailure(reporter, kRestartToTryAgainMessage); return; case UploadStatus.LookingForUpdates: // We don't need this message if the caller is the timer (presumably AFTER the user already // asked us to check). if (verbosity == BloomUpdateMessageVerbosity.Verbose) { - ShowToastForLookingForUpdates(); + reporter.Say(AlreadyCheckingMessage()); } + reporter.Finished(UpdateAttemptOutcome.Failed, null, AlreadyCheckingMessage()); return; // Conceivably the appropriate toast is still up. Very likely in the last case, since that // one doesn't go away. But it's harmless to show it again, and maybe the new animation will // help the user notice it. case UploadStatus.FoundUpdates: - ShowToastForFoundUpdates(verbosity, restartBloom); + // Unless the user has already said yes elsewhere, in which case asking again by + // toast would be strange -- go straight to downloading what we found. + if (userHasAlreadyAgreedToUpdate) + { + DownloadAndApplyUpdates(restartBloom, reporter); + return; + } + OfferFoundUpdates(reporter, restartBloom); + reporter.Finished(UpdateAttemptOutcome.Offered, null, null); return; case UploadStatus.Downloading: - ShowToastForDownloading(); + reporter.Say(DownloadingMessage()); + reporter.Finished(UpdateAttemptOutcome.Failed, null, AlreadyCheckingMessage()); return; case UploadStatus.DownloadedWaitingForRestart: - ShowToastForDownloadedWaitingForRestart(restartBloom); + OfferRestartToApplyDownload( + reporter, + _newVersion.TargetFullRelease.Version.ToString(), + restartBloom + ); + // Already downloaded and already arranged to install on exit, so as far as the + // caller is concerned this is a success: quitting will install it. + reporter.Finished( + UpdateAttemptOutcome.Downloaded, + _newVersion?.TargetFullRelease?.Version?.ToString(), + null + ); return; } @@ -164,8 +230,13 @@ Action restartBloom // bit of code gets even more so. I decided that if we've detected a new version, // we won't actually look again during this run.) - if (!GetUpdateUrl(verbosity, out var updateUrl)) + if (!GetUpdateUrl(verbosity, reporter, out var updateUrl)) + { + // Overwhelmingly the reason we can't work out where to look is that we can't + // reach the server. + reporter.Finished(UpdateAttemptOutcome.Failed, null, CannotConnectMessage()); return; // we can stay in NothingKnown state, allow user to try again. + } // Now we're starting stuff we don't want to overlap with other update efforts. // Thus the other states all display a message and return above. @@ -180,54 +251,123 @@ Action restartBloom _newVersion = await _bloomUpdateManager.CheckForUpdatesAsync(); if (_newVersion == null) { - ShowToastForUpToDate(verbosity); + if (verbosity == BloomUpdateMessageVerbosity.Verbose) + { + // Only say this if the user manually initiated the check. + reporter.Say(UpToDateMessage()); + } _bloomUpdateManager = null; // no updates, so no need to keep this object around _status = UploadStatus.NothingKnown; // allows user to try again + reporter.Finished(UpdateAttemptOutcome.NothingNewer, null, null); return; } // There are updates available. If the user is not installing updates automatically, - // ask whether to download them. - if (!Settings.Default.AutoUpdate) + // ask whether to download them -- unless they have already said yes somewhere else. + if (!Settings.Default.AutoUpdate && !userHasAlreadyAgreedToUpdate) { _status = UploadStatus.FoundUpdates; - ShowToastForFoundUpdates(verbosity, restartBloom); + OfferFoundUpdates(reporter, restartBloom); + reporter.Finished(UpdateAttemptOutcome.Offered, null, null); return; } } catch (Exception e) { - // Hopefully this is very rare. Don't think it's worth localizing. - // But we do want some indication of a problem if we can't get updates. + // Hopefully this is very rare. But we do want some indication of a problem if we + // can't get updates. // Review: should we go straight to "NotifyUserOfProblem" if verbosity // is verbose (i.e., called by Check for Updates user action)? - ShowToastForError( - "Bloom was unable to check for updates. Restart to try again.", - e - ); + ReportFailure(reporter, kUnableToCheckMessage, e); return; } // If autoupdate is true, we just go ahead and download the updates. - DownloadAndApplyUpdates(restartBloom); + DownloadAndApplyUpdates(restartBloom, reporter); #endif } - private static async void DownloadAndApplyUpdates(Action restartBloom) + private static async void DownloadAndApplyUpdates( + Action restartBloom, + UpdateReporter reporter + ) { #if !__MonoCS__ + // One download at a time. The switch in CheckForAVelopackUpdate guards the way in, but + // not this method, which the "Update Now" toast also calls straight from its click. A + // toast left on screen from an earlier check plus the upgrade dialog -- which skips the + // asking step -- can now both arrive here, and two DownloadUpdatesAsync calls on one + // UpdateManager is not something we want to find out about in the field. + if (_status == UploadStatus.DownloadedWaitingForRestart) + { + // Not a failure: it is already downloaded and already arranged to install when + // Bloom exits. Telling the caller otherwise would send someone who asked to be + // upgraded away to pick another collection, when the new Bloom is sitting ready. + reporter.Finished( + UpdateAttemptOutcome.Downloaded, + _newVersion?.TargetFullRelease?.Version?.ToString(), + null + ); + return; + } + if (_status == UploadStatus.Downloading) + { + reporter.Say(DownloadingMessage()); + reporter.Finished(UpdateAttemptOutcome.Failed, null, AlreadyCheckingMessage()); + return; + } + try { _status = UploadStatus.Downloading; - ShowToastForDownloading(); + _lastDownloadPercent = 0; + reporter.Say(DownloadingMessage()); + + await _bloomUpdateManager.DownloadUpdatesAsync( + _newVersion, + percent => + { + _lastDownloadPercent = percent; + reporter.Percent(percent); + }, + reporter.CancellationToken + ); + + // The transfer can finish in the very instant the user cancels, in which case the + // await returns normally rather than throwing, and everything below would go on to + // arrange an install they had just said no to. Checking here is what makes Cancel + // mean it even in that sliver: the bits may be on disk, but no exit handler is + // registered, so nothing installs. + if (reporter.CancellationToken.IsCancellationRequested) + { + _status = UploadStatus.NothingKnown; + reporter.Finished(UpdateAttemptOutcome.Cancelled, null, null); + return; + } - await _bloomUpdateManager.DownloadUpdatesAsync(_newVersion); _status = UploadStatus.DownloadedWaitingForRestart; - ShowToastForDownloadedWaitingForRestart(restartBloom); + OfferRestartToApplyDownload( + reporter, + _newVersion.TargetFullRelease.Version.ToString(), + restartBloom + ); // When we exit, apply the updates. (If autoupdate is false, this is still appropriate, // because the user responded to the message about updates available by clicking "Update Now", // so we're just completing something already approved). + // Only ever register one exit handler, however many times we come through here: + // applying the same update twice is how an upgrade fails or restarts Bloom when + // nobody asked it to. + if (_willInstallUpdateOnExit) + { + reporter.Finished( + UpdateAttemptOutcome.Downloaded, + _newVersion?.TargetFullRelease?.Version?.ToString(), + null + ); + return; + } + _willInstallUpdateOnExit = true; Application.ApplicationExit += (sender, args) => { // Write a file so that if the update fails (e.g., a running process prevents it), @@ -242,161 +382,186 @@ private static async void DownloadAndApplyUpdates(Action restartBloom) _bloomUpdateManager.WaitExitThenApplyUpdates(null, true, false); } }; + + reporter.Finished( + UpdateAttemptOutcome.Downloaded, + _newVersion?.TargetFullRelease?.Version?.ToString(), + null + ); + } + catch (OperationCanceledException) + { + // The user pressed Cancel, so Velopack abandoned the transfer. Leave no trace: no + // exit handler was registered (we never got that far), nothing is downloaded, and + // putting the status back to NothingKnown means a later attempt this session starts + // cleanly rather than being told an update is already in progress. + _status = UploadStatus.NothingKnown; + reporter.Finished(UpdateAttemptOutcome.Cancelled, null, null); } catch (Exception e) { - // Hopefully this is very rare. Don't think it's worth localizing. But it's dangerous not - // to catch all exceptions in an async void method, according to a VS popup. - ShowToastForError( - "Bloom was unable to download and install updates. Restart to try again.", - e - ); + // Hopefully this is very rare. But it's dangerous not to catch all exceptions in an + // async void method, according to a VS popup. + ReportFailure(reporter, kUnableToDownloadMessage, e); } #endif } - private static void ShowToastForUpToDate(BloomUpdateMessageVerbosity verbosity) + /// + /// Say that the attempt has failed, and remember that it has: having got here we are not + /// confident of being in a state where it is safe to try again this session. + /// + private static void ReportFailure( + UpdateReporter reporter, + string message, + Exception e = null + ) { - if (verbosity == BloomUpdateMessageVerbosity.Verbose) - { - // Only show this if the user manually initiated the check. - var message = LocalizationManager.GetString( - "CollectionTab.UpToDate", - "Your Bloom is up to date." - ); - ToastService.ShowToast(type: ToastType.Update, text: message, durationSeconds: 5); - } + _status = UploadStatus.Failed; + if (e != null) + _updateException = e; + // _updateException rather than e, deliberately. The one caller that passes no exception + // is the "restart Bloom to try again" case, which only happens BECAUSE an earlier + // attempt failed -- so the exception already on file is exactly the one a problem + // report should carry. + reporter.SayProblem(message, _updateException); + reporter.Finished(UpdateAttemptOutcome.Failed, null, message); } - private static void ShowToastForLookingForUpdates() - { - var message = LocalizationManager.GetString( + // ------------------------------------------------------------------------------------ + // The words. Each of these is worked out once, here, and then given to whichever + // UpdateReporter is in use, so that the toast route and the progress-dialog route say the + // same thing without either knowing about the other. + // ------------------------------------------------------------------------------------ + + private static string UpToDateMessage() => + LocalizationManager.GetString("CollectionTab.UpToDate", "Your Bloom is up to date."); + + // Internal so the minimum-version dialog can say the same (already translated) thing + // while it waits out a check another caller started. + internal static string AlreadyCheckingMessage() => + LocalizationManager.GetString( "CollectionTab.UpdateCheckInProgress", "Bloom is already working on checking for updates." ); - ToastService.ShowToast(type: ToastType.Update, text: message, durationSeconds: 5); - } - private static void ShowToastForFoundUpdates( - BloomUpdateMessageVerbosity verbosity, - Action restartBloom - ) - { - var msgAvail = LocalizationManager.GetString( - "CollectionTab.UpdatesAvailable", - "A new version of Bloom is available." - ); - var actionInstall = LocalizationManager.GetString( - "CollectionTab.UpdateNow", - "Update Now" + private static string CannotConnectMessage() => + LocalizationManager.GetString( + "CollectionTab.UnableToCheckForUpdate", + "Could not connect to the server to check for an update. Are you connected to the internet?", + "Shown when Bloom tries to check for an update but can't, for example because it can't connect to the internet, or a problems with our server, etc." ); - ToastService.ShowToast( - type: ToastType.Update, - text: msgAvail, - durationSeconds: 10, - action: new ToastAction - { - Label = actionInstall, - Callback = () => DownloadAndApplyUpdates(restartBloom), - } - ); - } - private static void ShowToastForError(string msg, Exception e = null) - { - // I'm not confident of getting back to a state where it's safe to try again. - _status = UploadStatus.Failed; - if (e != null) - _updateException = e; - ToastService.ShowToast( - ToastType.Error, - text: msg, - durationSeconds: 10, - action: new ToastAction - { - Callback = () => ErrorReport.NotifyUserOfProblem(_updateException, msg), - } + private static string UpdatesAvailableMessage() => + LocalizationManager.GetString( + "CollectionTab.UpdatesAvailable", + "A new version of Bloom is available." ); - } - private static bool _restartingAfterToastClicked = false; - - private static void ShowToastForDownloading() + private static string DownloadingMessage() { - // Show a notification that we're downloading the update. - // We could show a progress bar, but it would be hard to get it right. - // Velopack may use a more sophisticated algorithm to decide which to download, // but this should be good enough to give the user an idea. var fullSize = _newVersion.TargetFullRelease.Size; var deltasSize = _newVersion.DeltasToTarget.Sum(d => d.Size); - var downloadSize = deltasSize; - if (_newVersion.DeltasToTarget.Length > 0 && fullSize < deltasSize) - downloadSize = fullSize; - var updatingMsg = String.Format( + // With no deltas to add up we have to quote the full release, or we claim the download + // is 0K. That is what happened for every full download, and Bloom asks for a full one + // whenever the user is more than MaximumDeltasBeforeFallback builds behind -- so an + // ordinary two-releases-behind user saw "(0K)". It only ever flashed past in a + // five-second toast before; now it is what they read while they wait. + var downloadSize = + _newVersion.DeltasToTarget.Length == 0 ? fullSize : Math.Min(deltasSize, fullSize); + return DownloadingMessage( + _newVersion.TargetFullRelease.Version.ToString(), + downloadSize / 1024 + ); + } + + private static string DownloadingMessage(string version, long sizeInK) => + String.Format( LocalizationManager.GetString( "CollectionTab.Updating", "Downloading update to {0} ({1}K)" ), - _newVersion.TargetFullRelease.Version.ToString(), - downloadSize / 1024 + version, + sizeInK ); - ShowToastForDownloadingMessage(updatingMsg); - } - private static void ShowToastForDownloadingMessage(string updatingMsg) - { - ToastService.ShowToast(type: ToastType.Update, text: updatingMsg, durationSeconds: 5); - } + private static string DownloadedMessage(string version) => + String.Format( + LocalizationManager.GetString( + "CollectionTab.UpdateInstalled", + "Update for {0} is ready", + "Appears after Bloom has downloaded a program update in the background and is ready to switch the user to it the next time they run Bloom." + ), + version + ); + + // ------------------------------------------------------------------------------------ + // The two things we say that come with something for the user to click. + // ------------------------------------------------------------------------------------ - private static void ShowToastForDownloadedWaitingForRestart(Action restartBloom) + private static void OfferFoundUpdates(UpdateReporter reporter, Action restartBloom) { - ShowToastForDownloadedWaitingForRestart( - _newVersion.TargetFullRelease.Version.ToString(), - restartBloom + reporter.OfferToDownload( + UpdatesAvailableMessage(), + LocalizationManager.GetString("CollectionTab.UpdateNow", "Update Now"), + () => DownloadAndApplyUpdates(restartBloom, reporter) ); } - private static void ShowToastForDownloadedWaitingForRestart( + private static bool _restartingAfterToastClicked = false; + + private static void OfferRestartToApplyDownload( + UpdateReporter reporter, string version, Action restartBloom ) { - var msg = String.Format( - LocalizationManager.GetString( - "CollectionTab.UpdateInstalled", - "Update for {0} is ready", - "Appears after Bloom has downloaded a program update in the background and is ready to switch the user to it the next time they run Bloom." - ), - version - ); - var action = String.Format( + reporter.OfferToRestart( + DownloadedMessage(version), LocalizationManager.GetString( "CollectionTab.RestartToUpdate", "Restart Bloom to Update", "Restart the Bloom program, not Windows" - ) - ); - ToastService.ShowToast( - type: ToastType.Update, - text: msg, - action: new ToastAction + ), + () => { - Label = action, - Callback = () => - { - _restartingAfterToastClicked = true; - _bloomUpdateManager?.WaitExitThenApplyUpdates(null); - Logger.WriteMinorEvent("shutting Bloom down in order to apply updates"); - restartBloom(); - }, + ArrangeToApplyUpdateAndRestart(); + restartBloom(); } ); } + /// + /// Hand the downloaded update to Velopack the way the "Restart Bloom to Update" toast does: + /// with the arguments that show Velopack's own progress bar while it installs and then bring + /// Bloom back by itself. The caller shuts Bloom down straight afterwards. + /// + /// The alternative, which the exit handler uses, applies the update quietly and does NOT + /// relaunch. That is right when the user was quitting anyway and wrong when they have just + /// asked to be upgraded, because it leaves them looking at a closed program having to start + /// it again themselves. + /// + internal static void ArrangeToApplyUpdateAndRestart() + { +#if !__MonoCS__ + // Only once. On the mid-session path both routes to this exist at the same time -- the + // restart toast the workspace can show, and the upgrade dialog -- and handing the same + // update to Velopack twice is how an install fails or Bloom relaunches when nobody + // asked it to. + if (_restartingAfterToastClicked) + return; + _restartingAfterToastClicked = true; + _bloomUpdateManager?.WaitExitThenApplyUpdates(null); + Logger.WriteMinorEvent("shutting Bloom down in order to apply updates"); +#endif + } + // returns true if we should proceed with the update check. private static bool GetUpdateUrl( BloomUpdateMessageVerbosity verbosity, + UpdateReporter reporter, out string updateUrl ) { @@ -434,12 +599,7 @@ out string updateUrl // but if they did, try and give them a hint about what went wrong if (result.IsConnectivityError) { - var failMsg = LocalizationManager.GetString( - "CollectionTab.UnableToCheckForUpdate", - "Could not connect to the server to check for an update. Are you connected to the internet?", - "Shown when Bloom tries to check for an update but can't, for example because it can't connect to the internet, or a problems with our server, etc." - ); - ShowFailureNotification(failMsg); + reporter.SayWarning(CannotConnectMessage()); } else if ( result.Error == null @@ -452,7 +612,7 @@ out string updateUrl } else { - ShowFailureNotification(result.Error.Message); + reporter.SayWarning(result.Error.Message); } return false; @@ -463,11 +623,6 @@ out string updateUrl return true; } - private static void ShowFailureNotification(string failMsg) - { - ToastService.ShowToast(ToastType.Warning, text: failMsg, durationSeconds: 5); - } - /// /// Gets the path to the file we write when Bloom is about to exit for a Velopack update. /// The channel name is included so that parallel installations on different channels @@ -583,34 +738,33 @@ internal static void CheckForFailedUpdate() internal static void DebugShowToastScenario(string scenario, Action restartBloom = null) { restartBloom ??= () => { }; + var reporter = new ToastUpdateReporter(); switch (scenario) { case "looking": - ShowToastForLookingForUpdates(); + reporter.Say(AlreadyCheckingMessage()); return; case "upToDate": - ShowToastForUpToDate(BloomUpdateMessageVerbosity.Verbose); + reporter.Say(UpToDateMessage()); return; case "foundUpdates": - ShowToastForFoundUpdates(BloomUpdateMessageVerbosity.Verbose, restartBloom); + OfferFoundUpdates(reporter, restartBloom); return; case "downloading": - ShowToastForDownloadingMessage("Downloading update to 9.9.9 (123K)"); + reporter.Say(DownloadingMessage("9.9.9", 123)); return; case "downloadedWaitingForRestart": - ShowToastForDownloadedWaitingForRestart("9.9.9", restartBloom); + OfferRestartToApplyDownload(reporter, "9.9.9", restartBloom); return; case "error": - ShowToastForError( - "Bloom was unable to download and install updates. Restart to try again.", + reporter.SayProblem( + kUnableToDownloadMessage, new ApplicationException("Debug update error") ); return; case "failure": - ShowFailureNotification( - "Could not connect to the server to check for an update. Are you connected to the internet?" - ); + reporter.SayWarning(CannotConnectMessage()); return; default: throw new ArgumentException( @@ -682,18 +836,5 @@ public static string ChannelName return "Release"; } } - - internal enum UpdateOutcome - { - GotNewVersion, - AlreadyUpToDate, - InstallFailed, - } - - internal class UpdateResult - { - public string NewInstallDirectory; - public UpdateOutcome Outcome; - } } } diff --git a/src/BloomExe/Collection/CollectionSettings.cs b/src/BloomExe/Collection/CollectionSettings.cs index 8548884c00dd..18d9d87b0cf5 100644 --- a/src/BloomExe/Collection/CollectionSettings.cs +++ b/src/BloomExe/Collection/CollectionSettings.cs @@ -72,6 +72,20 @@ public class CollectionSettings // if this is null, relevant code uses the default, so we don't have to initialize it here public string BadgeQrCodeLabel; + /// + /// The oldest version of Bloom that is allowed to open this collection, e.g. "6.5". + /// Empty means any version may open it. At this point there is no UI for setting this; + /// it has to be added by editing the .bloomCollection file by hand. See BL-16690. + /// The gate that actually enforces it is MinimumBloomVersionCheck. + /// + public string MinimumBloomVersion = ""; + + /// + /// The name of the element in the .bloomCollection file that holds MinimumBloomVersion. + /// MinimumBloomVersionCheck reads it without loading the whole CollectionSettings. + /// + public const string kMinimumBloomVersionElementName = "MinimumBloomVersion"; + public static readonly Dictionary CssNumberStylesToCultureOrDigits = new Dictionary() { @@ -419,6 +433,10 @@ public void Save() xml.Add(BulkPublishBloomPubSettings.ToXElement()); xml.Add(new XElement("ShowBlorgLanguageQrCode", ShowBlorgLanguageQrCode)); xml.Add(new XElement("BadgeQrCodeLabel", BadgeQrCodeLabel)); + // Only write this if it is actually in use. Save() builds the file from scratch, so if we + // didn't write it back, the first save after someone hand-added it would silently lose it. + if (!string.IsNullOrWhiteSpace(MinimumBloomVersion)) + xml.Add(new XElement(kMinimumBloomVersionElementName, MinimumBloomVersion)); RobustIO.SaveXElement(xml, SettingsFilePath); // Color palette settings are stored in a separate Json file @@ -705,6 +723,7 @@ public void Load() ShowBlorgLanguageQrCode = ReadBoolean(xml, "ShowBlorgLanguageQrCode", true); BadgeQrCodeLabel = ReadString(xml, "BadgeQrCodeLabel", ""); + MinimumBloomVersion = ReadString(xml, kMinimumBloomVersionElementName, ""); LoadDictionary(xml, "Palette", ColorPalettes); } diff --git a/src/BloomExe/Collection/MinimumBloomVersionCheck.cs b/src/BloomExe/Collection/MinimumBloomVersionCheck.cs new file mode 100644 index 000000000000..46ff97aeabab --- /dev/null +++ b/src/BloomExe/Collection/MinimumBloomVersionCheck.cs @@ -0,0 +1,802 @@ +using System; +using System.ComponentModel; +using System.Diagnostics; +using System.Globalization; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using System.Windows.Forms; +using System.Xml.Linq; +using Bloom.Api; +using Bloom.MiscUI; +using L10NSharp; +using SIL.IO; +using SIL.Reporting; + +namespace Bloom.Collection +{ + /// + /// A collection can declare, via the MinimumBloomVersion element of its .bloomCollection file, + /// the oldest version of Bloom that is allowed to open it. This is in preparation for Cloud + /// syncing: once a collection has been touched by a version of Bloom that knows how to sync it, + /// letting an older Bloom loose on it could do real damage. + /// + /// At this point there is no UI for setting the flag; it has to be added to the file by hand. + /// See BL-16690. + /// + public static class MinimumBloomVersionCheck + { + /// + /// Decide whether this Bloom is allowed to open the given collection. + /// + /// Path to the .bloomCollection file + /// What the collection asked for, as major.minor, when we say no + /// true if the collection demands a newer Bloom than we are + public static bool IsThisBloomTooOld(string settingsFilePath, out string minimumVersion) + { + // When this is a Team Collection and we can actually read the repository, the repository + // decides -- including when it says there is no requirement at all. It has to be able to + // LIFT a requirement, not just impose one: a member who is being refused never opens the + // collection, so the startup sync that would refresh their own copy never runs, and an + // administrator's mistaken requirement would shut them out on every launch for ever, + // fixable only by hand-editing the file on each machine. + // + // Otherwise -- an ordinary collection, or a shared folder we cannot reach right now -- + // we fall back to what we have: the file on this computer and anything the repository + // told us earlier this session, taking whichever demands more. Not being able to see the + // repository is not the same as the repository saying "no requirement", so in that case + // we keep the protection rather than quietly dropping it. + var declaredVersion = + MinimumVersionInTeamCollectionRepo(settingsFilePath) + ?? MoreDemandingOf( + ReadMinimumBloomVersion(settingsFilePath), + MinimumVersionLearnedFromRepo(settingsFilePath) + ); + if (IsVersionSufficient(declaredVersion, RunningBloomVersion)) + { + minimumVersion = ""; + return false; + } + // We can only get here if it parsed, since anything we can't parse counts as satisfied. + minimumVersion = ToMajorMinor(Version.Parse(declaredVersion.Trim())); + return true; + } + + /// + /// Compare a collection's declared minimum version against the version we are running. + /// We compare only major and minor, like the other version gates in Bloom + /// (BookStorage's feature requirements and BookDownload's minVersion), so a minimum of + /// "6.5" is satisfied by any 6.5.x or later, and a build number in the minimum is ignored. + /// + /// The channel (Alpha/Beta/Release) deliberately plays no part: 6.5 means the same thing on + /// every channel, so an alpha tester who is running ahead of the release is correctly let in. + /// + /// Separated out from the file reading so it can be unit tested. + internal static bool IsVersionSufficient(string minimumVersion, Version runningVersion) + { + if (string.IsNullOrWhiteSpace(minimumVersion)) + return true; // the normal case: the collection doesn't care + + // Version.TryParse needs at least "major.minor", so a bare "6" lands here too. + if (!Version.TryParse(minimumVersion.Trim(), out var requiredVersion)) + { + // Someone hand-edited the file and mistyped. Locking them out of their own + // collection over a typo would be worse than ignoring it, so just complain to the log. + Logger.WriteEvent( + $"Ignoring unparseable {CollectionSettings.kMinimumBloomVersionElementName} '{minimumVersion}' in collection settings." + ); + return true; + } + + if (runningVersion.Major != requiredVersion.Major) + return runningVersion.Major > requiredVersion.Major; + return runningVersion.Minor >= requiredVersion.Minor; + } + + /// + /// What a Team Collection's repository says, read straight from the shared folder, or null + /// for an ordinary collection or a repository we cannot reach. + /// + /// This is what closes the first-launch gap. The local copy of a Team Collection's settings + /// is only refreshed from the repository later in startup, well after this gate, so a member + /// whose administrator set a minimum version while they were closed would otherwise be + /// judged on yesterday's file, let in, and only stopped the launch after that. Asking the + /// repository directly costs one small read, and only for collections that are actually in + /// a Team Collection. See BL-16690. + /// + private static string MinimumVersionInTeamCollectionRepo(string settingsFilePath) + { + var collectionFolder = System.IO.Path.GetDirectoryName(settingsFilePath); + string repoSettings; + try + { + // The shared folder may be on a network share that has gone away, where even asking + // whether a folder exists can block for a long time -- and we are at the one moment + // in startup with no window and nothing to show a user who is wondering why nothing + // is happening. So give it a few seconds and no more. Not answering in time means + // "we don't know", which falls back to the local copy, and the next launch will + // very likely get a straight answer. + var read = Task.Run(() => + Bloom.TeamCollection.FolderTeamCollection.GetRepoCollectionSettingsForCollectionFolder( + collectionFolder + ) + ); + if (!read.Wait(TimeSpan.FromSeconds(5))) + { + Logger.WriteEvent( + "Timed out reading the Team Collection repo settings while checking the minimum Bloom version; using the local copy instead." + ); + return null; + } + repoSettings = read.Result; + } + catch (Exception ex) + { + Logger.WriteEvent( + $"Could not read the Team Collection repo settings while checking the minimum Bloom version: {ex.Message}" + ); + return null; + } + + if (string.IsNullOrWhiteSpace(repoSettings)) + return null; + try + { + return ParseMinimumBloomVersion(repoSettings); + } + catch (Exception ex) + { + // Settings we can't parse tell us nothing; let the local file speak. + Logger.WriteEvent( + $"Could not read {CollectionSettings.kMinimumBloomVersionElementName} from the Team Collection repo settings: {ex.Message}" + ); + return null; + } + } + + /// + /// Of two declared minimums, whichever demands more. Anything empty or unparseable counts as + /// no requirement at all, the same rule IsVersionSufficient applies, so a typo in one source + /// simply leaves the other to speak. + /// + private static string MoreDemandingOf(string one, string other) + { + var oneVersion = ParseOrNull(one?.Trim()); + var otherVersion = ParseOrNull(other?.Trim()); + if (oneVersion == null) + return other; + if (otherVersion == null) + return one; + return oneVersion >= otherVersion ? one : other; + } + + /// + /// Minimum versions we have learned from a Team Collection repository during this run, keyed + /// by the collection's settings file path. + /// + /// Bloom deliberately does not rewrite local collection settings mid-session, so once an + /// administrator sets a minimum version the file on this computer goes on saying nothing + /// about it until the next startup sync. Without this record, someone we had just shut out + /// could walk straight back in by choosing the same collection from the chooser: the gate + /// would read the stale file, see no requirement, and open it. See BL-16690. + /// + /// Concurrent because the two writers are on different threads: the Team Collection startup + /// sync runs on the progress dialog's background worker, while a repository change + /// notification is handled on the UI thread. + /// + private static readonly System.Collections.Concurrent.ConcurrentDictionary< + string, + string + > _minimumVersionsFromRepo = new System.Collections.Concurrent.ConcurrentDictionary< + string, + string + >(StringComparer.OrdinalIgnoreCase); + + /// + /// Record what the repository says this collection's minimum version is, which may be newer + /// than what its file on this computer says. + /// + /// Pass the empty string when the repository declares no requirement. This record only + /// matters when we cannot reach the shared folder at the moment of the check: if we can, + /// IsThisBloomTooOld reads the repository afresh and that answer governs. When we cannot, + /// this and the local file are all we have, and we take whichever demands more, so a + /// withdrawal recorded here does not by itself cancel a requirement the local file still + /// carries. + /// + public static void RememberMinimumVersionFromRepo( + string settingsFilePath, + string minimumVersion + ) + { + if (string.IsNullOrEmpty(settingsFilePath)) + return; + _minimumVersionsFromRepo[NormalizePath(settingsFilePath)] = minimumVersion ?? ""; + } + + /// + /// What the repository told us about this collection during this run, or null if it never + /// did. Both null and "" end up meaning "no requirement from this source" once + /// MoreDemandingOf has had them, so the distinction is only for the reader: null is silence, + /// "" is the repository actively declaring no minimum. + /// + private static string MinimumVersionLearnedFromRepo(string settingsFilePath) + { + if (string.IsNullOrEmpty(settingsFilePath)) + return null; + return _minimumVersionsFromRepo.TryGetValue( + NormalizePath(settingsFilePath), + out var version + ) + ? version + : null; + } + + /// + /// So that the same collection reached by two spellings of its path lands on one entry. + /// The 8.3 short form matters here and GetFullPath does not expand it: the startup gate is + /// handed a path that has already been through LongPathAware.GetLongPath, while what we + /// record comes straight from the Team Collection's own folder, so the two could otherwise + /// disagree and the remembered requirement would simply not be found. + /// + private static string NormalizePath(string path) + { + try + { + return Utils.LongPathAware.GetLongPath(System.IO.Path.GetFullPath(path)); + } + catch (Exception) + { + // A path we can't even canonicalize is not going to match anything either way. + return path; + } + } + + /// + /// Read just the minimum version out of the settings file. We can't use CollectionSettings + /// for this, because we have to answer the question before we commit to building a + /// ProjectContext around the collection. + /// + internal static string ReadMinimumBloomVersion(string settingsFilePath) + { + try + { + if (!RobustFile.Exists(settingsFilePath)) + return ""; + return ParseMinimumBloomVersion( + RobustFile.ReadAllText(settingsFilePath, Encoding.UTF8) + ); + } + catch (Exception ex) + { + // A settings file we can't even parse is a real problem, but this is not the place to + // report it. Let the normal open fail and give the user its much better error report. + Logger.WriteEvent( + $"Could not read {CollectionSettings.kMinimumBloomVersionElementName} from {settingsFilePath}: {ex.Message}" + ); + return ""; + } + } + + /// + /// Pull the minimum version out of the text of a collection settings file. Separate from + /// reading the file so that a Team Collection can ask about the copy in the repository, which + /// lives inside a zip and has never been written to this computer. + /// + internal static string ParseMinimumBloomVersion(string settingsXml) + { + if (string.IsNullOrEmpty(settingsXml)) + return ""; + return CollectionSettings.ReadString( + XElement.Parse(settingsXml), + CollectionSettings.kMinimumBloomVersionElementName, + "" + ); + } + + /// + /// Like IsThisBloomTooOld, but for settings we have in hand rather than in a file. + /// + public static bool IsThisBloomTooOldForSettings( + string settingsXml, + out string minimumVersion + ) + { + minimumVersion = ""; + string declaredVersion; + try + { + declaredVersion = ParseMinimumBloomVersion(settingsXml); + } + catch (Exception ex) + { + Logger.WriteEvent( + $"Could not read {CollectionSettings.kMinimumBloomVersionElementName} from settings content: {ex.Message}" + ); + return false; + } + if (IsVersionSufficient(declaredVersion, RunningBloomVersion)) + return false; + minimumVersion = ToMajorMinor(Version.Parse(declaredVersion.Trim())); + return true; + } + + /// + /// Tell the user that this collection needs a newer Bloom, and give them the two ways forward: + /// upgrade, or open some other collection. Upgrading happens in place; if we can't manage it + /// we say why and they are left to choose a different collection instead. We never send them + /// off to the website. + /// + /// true if we downloaded an upgrade, in which case we have already asked Bloom to + /// quit so it can be installed, and the caller must not start any more UI. + public static bool ReportCollectionNeedsNewerBloom( + string collectionName, + string minimumVersion + ) + { + var header = LocalizationManager.GetString( + "Collection.NewerVersionNeededHeader", + "This collection needs a newer version of Bloom." + ); + var explanation = string.Format( + LocalizationManager.GetString( + "Collection.CollectionRequiresNewerVersion", + "The collection \"{0}\" requires Bloom {1} or greater. You are running Bloom {2}.", + "{0} is the name of the collection, {1} is the version it requires, {2} is the version of Bloom that is running." + ), + // The message is HTML, and a collection name can perfectly well contain an ampersand. + System.Net.WebUtility.HtmlEncode(collectionName), + minimumVersion, + ToMajorMinor(RunningBloomVersion) + ); + var upgradeButtonText = LocalizationManager.GetString( + "Collection.UpgradeBloom", + "Upgrade Bloom" + ); + var chooseOtherButtonText = LocalizationManager.GetString( + "Collection.OpenDifferentCollection", + "Open a Different Collection" + ); + + // Order matters: the buttons appear left to right in this order, and the default one is + // drawn filled and takes the initial focus. Upgrading is what we actually want the user + // to do, so it goes last (the rightmost, primary position) and is the default. + // No external-link icon on this button: this upgrades Bloom in place and never opens a + // browser, so marking it as leaving Bloom would simply be wrong. + var buttons = new[] + { + new MessageBoxButton() { Text = chooseOtherButtonText, Id = "chooseOther" }, + new MessageBoxButton() + { + Text = upgradeButtonText, + Id = kUpgradeButtonId, + Default = true, + }, + }; + + var result = BloomMessageBox.Show( + null, + $"{header}

{explanation}", + buttons + // No icon asked for. Nothing in BL-16690 calls for one, and asking for + // MessageBoxIcon.Warning did not produce one anyway -- see the note in + // BloomMessageBox.Show about why a warning icon cannot currently be requested at all. + ); + + if (result == kUpgradeButtonId && UpgradeBloom(minimumVersion)) + { + ProgramExit.Exit(); + return true; + } + // Either they asked for a different collection, or they wanted to upgrade and we could + // not manage it -- in which case we have already explained why. Either way the caller + // takes over, and puts the collection chooser up. + return false; + } + + private const string kUpgradeButtonId = "upgrade"; + + /// + /// The collection we are in the middle of shutting someone out of, if any. Keyed by name + /// rather than a plain flag for two reasons: the repository can report the same change more + /// than once, including while Bloom is shutting down, and we must not stack up dialogs; but + /// if the user moves on to a *different* collection in the same session, that one deserves + /// its own lock-out. + /// + private static string _collectionBeingLockedOut; + + /// + /// Shut the user out of a collection they already have open, because a minimum version they + /// don't meet has just arrived -- in practice, a Team Collection administrator set one while + /// they were working. They get the same dialog as at startup, and the same two ways out: + /// upgrade, or go to a different collection. There is deliberately no third option: the + /// dialog has no close box, and cancelling the collection chooser brings the dialog back + /// rather than dropping them into a collection this Bloom is not allowed to touch. + /// + /// false if a lock-out for this collection is already under way, so the caller + /// should carry on as usual rather than assume the collection is being torn down. + public static bool LockUserOutOfOpenCollection(string collectionName, string minimumVersion) + { + if (_collectionBeingLockedOut == collectionName) + return false; + _collectionBeingLockedOut = collectionName; + + if (ReportCollectionNeedsNewerBloom(collectionName, minimumVersion)) + return true; // upgrading; Bloom is already on its way down + + // They want a different collection -- or they wanted to upgrade and we couldn't manage + // it, and have already been told why. This is the same route as Open/Create Collection + // on the toolbar: it closes the current one and puts up the chooser. + if (Program.ChooseACollection(Shell.GetShellOrOtherOpenForm() as Shell)) + return true; + + // They cancelled the chooser. Staying in this collection is the one thing they can't + // do -- but quitting must stay possible. Someone with nothing newer to upgrade to and + // no other collection to open would otherwise have no way out of Bloom at all except + // Task Manager, since this dialog deliberately has no close box. Cancelling here means + // "none of the above", which is exactly how the startup path reads it too. + ProgramExit.Exit(); + return true; + } + + /// + /// Called when a collection has actually been opened, which ends any lock-out we were in + /// the middle of. The flag has to be cleared by something outside the lock-out itself: + /// clearing it on the way out would let a repeat notification (they arrive during shutdown) + /// put the dialog straight back up. + /// + public static void NoteCollectionOpened() + { + _collectionBeingLockedOut = null; + } + + /// + /// Get the user onto a newer Bloom, using exactly the machinery the "an update is available" + /// toast drives, and show them what it is doing while it happens. + /// + /// true if a new Bloom was downloaded and the caller should now shut Bloom down so + /// it can be installed. + private static bool UpgradeBloom(string minimumVersion) + { + var reporter = RunTheUpdateBehindAProgressDialog(minimumVersion); + if (reporter.Outcome != UpdateAttemptOutcome.Downloaded) + return false; + + // Someone who pressed Cancel does not get restarted, even if the download turned out to + // have finished in the same moment. Without this the two race: the wait gives up on a + // cancel, and by the time we look at the outcome it says Downloaded, so Bloom would + // quit and reinstall itself under a user who had just said no. + if (reporter.UserCancelled) + return false; + + // Ask for the restarting flavour of the install: the user asked to be upgraded, so + // leaving them at a closed program to start again themselves would be a poor end to it. + // This also gets Velopack's own progress bar while it installs. + // + // We install whatever version it turns out to be, and deliberately say nothing about + // whether it goes far enough: Velopack can only offer the newest build on this user's + // channel, and if that is still short of what the collection needs, the upgraded Bloom + // meets this same dialog next launch and the user decides again from there. + ApplicationUpdateSupport.ArrangeToApplyUpdateAndRestart(); + return true; + } + + /// + /// The last thing the progress dialog says before the user closes it. Everything else it + /// has to say the update code has already said through the reporter, including how any + /// failure was explained -- this is the one ending the update code has no words for, + /// because only we know what this collection was asking for. (The Downloaded ending says + /// nothing at all: the dialog closes and Bloom restarts, which is the news.) + /// + private static void SayHowTheUpgradeEnded( + ProgressUpdateReporter reporter, + string minimumVersion + ) + { + switch (reporter.Outcome) + { + case UpdateAttemptOutcome.NothingNewer: + // Being specific matters: "your Bloom is up to date", which is what the update + // code says here, would be a baffling thing to read right after being told this + // Bloom is too old. + reporter.Say( + string.Format( + LocalizationManager.GetString( + "Collection.NothingNewerAvailable", + "This collection needs Bloom {0}, but you already have the newest Bloom available to you.", + "{0} is the version the collection requires." + ), + minimumVersion + ) + ); + break; + } + } + + /// + /// The same refusals, in the same words, that WorkspaceView.CheckForUpdatesImpl applies + /// before it drives this machinery from the menu. They belong to the caller rather than + /// to the update code, so we have to repeat them, and the user is owed the same + /// explanation whichever route they came by. (WorkspaceView also refuses for an all-users + /// install -- SharedByAllUsers -- but we no longer ship one, so that is not repeated + /// here.) + /// + private static bool CanThisBloomUpdateItself(out string whyNot) + { + if (Debugger.IsAttached) + { + whyNot = "Sorry, you cannot check for updates from the debugger."; + return false; + } + if (ApplicationUpdateSupport.IsDev) + { + whyNot = + "Checking for updates is disabled on developer builds. No relevant channel."; + return false; + } + whyNot = null; + return true; + } + + /// + /// How long we are prepared to watch an update that never reports back. This is not a + /// judgement about how long an upgrade should take -- it is a ninety-megabyte download, and + /// some of our users are on links where that really does take hours, which is what the + /// Cancel button is for. It is only here so that a download that has genuinely wedged cannot + /// leave someone with a progress dialog, no collection, and no way out of Bloom. + /// + /// Two hours was reviewed and kept deliberately (BL-16690, 2026-08-19). Shortening it would + /// risk abandoning a download that is slow but perfectly healthy, which is the common case + /// this is most likely to hit; the wedged case it exists for is rare, and the user has Cancel + /// in the meantime either way. + /// + private static readonly TimeSpan kHowLongToWaitForAnUpdate = TimeSpan.FromHours(2); + + /// + /// Run the ordinary update, showing the user what it is doing in a progress dialog. + /// + /// The dialog is here because this runs before any collection is open. The update code says + /// what it is doing through toasts, and toasts are drawn by ToastHost, which is mounted only + /// in the main workspace -- so on this path they would go nowhere at all, and the user would + /// watch an empty screen for the length of a large download. Handing the update code a + /// ProgressUpdateReporter instead puts those very same sentences, plus Velopack's + /// percentage, into a dialog. + /// + /// It is also the only window we get. A second one -- a message box afterwards to say how it + /// went -- does not come to the front and has no taskbar button of its own (ReactDialog sets + /// ShowInTaskbar false, which is right for a dialog with a parent window and leaves an + /// orphan at startup with nowhere to be). By the time it opens, this dialog has closed and + /// Bloom has no foreground window left to hand the activation on from, so the message ends + /// up behind whatever the user was doing and Bloom looks like it has hung. So everything + /// gets said here, in the window that is already in front, and the user closes it when they + /// have read it. + /// + private static ProgressUpdateReporter RunTheUpdateBehindAProgressDialog( + string minimumVersion + ) + { + // Built before the dialog, not inside its worker, so that the caller always gets a + // reporter to ask about. If the dialog never runs its worker body, this one still + // answers -- as a failure with nothing to say, which lands the user back at the + // collection chooser rather than on a null reference. + var reporter = new ProgressUpdateReporter(); + + // This dialog is reached from two quite different places, and they differ in whether a + // websocket server already exists. + // + // At startup there is none: Instance is set by the ProjectContext of a collection, and + // this gate runs before we build one. So we make a server for the duration, as + // WorkspaceView does for the language chooser (BL-15230). + // + // Mid-session there is, because a Team Collection's repository has started demanding a + // newer Bloom while the member has the collection open. Standing up a second listener + // on the port the first one holds throws, and BloomWebSocketServer.Init answers a + // SocketException by telling the user Bloom "cannot start properly" and quitting -- so + // there we use the server that is already running. + // + // Instance being non-null is a trustworthy test of that only because Dispose now clears + // it; otherwise a collection the user had closed would leave it pointing at a dead + // server, and this dialog would wait forever for a reply that could never come. + if (BloomWebSocketServer.Instance != null) + { + ShowTheDialog(BloomWebSocketServer.Instance, reporter, minimumVersion); + return reporter; + } + + using (var socketServer = new BloomWebSocketServer()) + { + socketServer.Init(BloomServer.WebSocketPort.ToString(CultureInfo.InvariantCulture)); + ShowTheDialog(socketServer, reporter, minimumVersion); + } + // Disposing it clears Instance, so the next collection's ProjectContext starts clean. + + return reporter; + } + + /// + /// Put up the progress dialog and run the update inside it, reporting through + /// . Returns when the dialog has closed. + /// + private static void ShowTheDialog( + Bloom.web.IBloomWebSocketServer socketServer, + ProgressUpdateReporter reporter, + string minimumVersion + ) + { + BrowserProgressDialog.DoWorkWithProgressDialog( + socketServer, + () => + new ReactDialog( + "progressDialogBundle", + new + { + // The same words as the button they just clicked. + title = LocalizationManager.GetString( + "Collection.UpgradeBloom", + "Upgrade Bloom" + ), + titleColor = "white", + titleBackgroundColor = Palette.kBloomBlueHex, + showReportButton = "never", + showCancelButton = true, + // Velopack tells us how far along the download is, so show a + // real bar rather than a spinner that says nothing. + determinate = true, + linearProgress = true, + }, + "Upgrade Bloom" + ) + { + Width = 620, + // Only a few lines ever appear here, and 400 left half the dialog + // empty. The message area scrolls, so a failure with more to say + // still fits. + Height = 260, + }, + (progress, worker) => + { + reporter.WriteTo(progress); + if (!CanThisBloomUpdateItself(out var whyNot)) + { + reporter.SayProblem(whyNot, null); + reporter.Finished(UpdateAttemptOutcome.CannotUpdateThisBloom, null, whyNot); + return true; + } + + // Bloom starts a check for updates on its own account a minute after a + // collection opens, so on a slow connection it is quite possible to click + // Upgrade Bloom while that check, or the download it led to, is still + // running. The update code turns a second request away ("Bloom is already + // working on checking for updates"), which is a poor thing to read right + // after asking for an upgrade. So we simply wait the other attempt out and + // then proceed as usual: whatever state it ended in -- an update found, an + // update downloaded, nothing newer -- the check below picks up from that + // state and answers accordingly. If what is running is a download, we show + // its progress as we wait. Cancel still works while we wait; and if + // the other attempt has genuinely wedged, the deadline lets the check below + // deliver its refusal rather than holding this dialog forever. + if (ApplicationUpdateSupport.IsBusyCheckingOrDownloading) + { + // The same (already translated) sentence the update code itself uses for + // this state. Once the other attempt is downloading, its percentage moves + // the bar; during its brief check phase the bar sits at zero. + reporter.Say(ApplicationUpdateSupport.AlreadyCheckingMessage()); + var deadline = DateTime.UtcNow + kHowLongToWaitForAnUpdate; + while ( + ApplicationUpdateSupport.IsBusyCheckingOrDownloading + && DateTime.UtcNow < deadline + ) + { + if (worker.CancellationPending) + { + // They cancelled the wait. The other attempt was never ours to + // stop -- Bloom started it for itself -- so it carries on + // unwatched, exactly as if this dialog had never opened. + return false; + } + reporter.Percent(ApplicationUpdateSupport.CurrentDownloadPercent); + Thread.Sleep(250); + } + } + + ApplicationUpdateSupport.CheckForAVelopackUpdate( + ApplicationUpdateSupport.BloomUpdateMessageVerbosity.Quiet, + restartBloom: null, // nothing to click here, and we quit ourselves + reporter: reporter, + userHasAlreadyAgreedToUpdate: true // they clicked Upgrade Bloom + ); + if (!WaitForTheUpdate(reporter, worker)) + { + // The user clicked Cancel, or we gave up waiting. Either way they want to be + // rid of this window, not asked to close it again. + return false; + } + + if (reporter.Outcome == UpdateAttemptOutcome.Downloaded) + { + // Nothing to read here: returning false closes the dialog by itself, and + // the caller restarts Bloom, which is the news. Velopack shows its own + // progress while it installs, so the user is not left staring at nothing. + // A Close button at this point would be pure delay -- there is no "later" + // to choose -- and someone who walked away from a slow download would come + // back to a stalled dialog instead of an upgraded Bloom. + return false; + } + + SayHowTheUpgradeEnded(reporter, minimumVersion); + // Leave the dialog up, with a Close button, so the user can read how it went. It + // is the only window we have that is reliably in front. + return true; + } + ); + } + + /// + /// Block the progress dialog's worker until the update reports back, watching for a click on + /// Cancel as we go. + /// + /// Cancel really cancels: Velopack is handed the reporter's cancellation token and abandons + /// the transfer, so nothing is left downloaded and nothing is waiting to install. We then + /// wait a little longer for the update code to confirm it has stopped, so the dialog does not + /// disappear while a download is still unwinding behind it. + /// + /// true if the update reported back on its own; false if the user cancelled it or we gave up + private static bool WaitForTheUpdate( + ProgressUpdateReporter reporter, + BackgroundWorker worker + ) + { + var deadline = DateTime.UtcNow + kHowLongToWaitForAnUpdate; + while (DateTime.UtcNow < deadline) + { + if (reporter.WaitForFinish(TimeSpan.FromMilliseconds(250))) + { + // A cancel that arrived during that wait still counts: the user pressed it + // before they could possibly know the download had just landed, and taking the + // download's word for it here is what would restart Bloom under someone who had + // said no. + if (worker.CancellationPending) + { + reporter.CancelTheDownload(); + return false; + } + return true; + } + if (worker.CancellationPending) + { + reporter.CancelTheDownload(); + // Give it a moment to actually stop and say so. It answers within a second or + // so; the bound is only so a download that ignores us cannot hold the dialog. + if (!reporter.WaitForFinish(TimeSpan.FromSeconds(10))) + Logger.WriteEvent( + "The Velopack download did not confirm cancellation within ten seconds." + ); + return false; + } + } + Logger.WriteEvent( + "Gave up waiting for the Velopack update started from the minimum version dialog." + ); + return false; + } + + private static Version ParseOrNull(string version) => + Version.TryParse(version ?? "", out var v) ? v : null; + + /// + /// Format a version the way we actually compare it: major and minor only. Showing the user a + /// build number we don't look at would misrepresent the rule, and invites them to compare + /// the wrong digits ("6.5 required, but I have 6.4.900, and 900 is more than 5"). + /// + private static string ToMajorMinor(Version version) => $"{version.Major}.{version.Minor}"; + + /// + /// The version of Bloom we are running. We use the assembly version rather than + /// Application.ProductVersion because the latter can carry a non-numeric suffix. + /// A loaded assembly always has a version, so we don't guard against it being missing: + /// any fallback we picked would be a lie, and a low one would lock the user out of every + /// collection that declares a minimum -- the opposite of what this class is careful to do. + /// + private static Version RunningBloomVersion => + typeof(MinimumBloomVersionCheck).Assembly.GetName().Version; + } +} diff --git a/src/BloomExe/Edit/EditingView.cs b/src/BloomExe/Edit/EditingView.cs index c05f901542bd..dbfa8defa909 100644 --- a/src/BloomExe/Edit/EditingView.cs +++ b/src/BloomExe/Edit/EditingView.cs @@ -1893,7 +1893,11 @@ public async Task AddImageFromUrlAsync(string desiredFileNameWithoutExtension, s } catch (Exception ex) { - BloomWebSocketServer.Instance.SendEvent( + // Instance is only set while a collection is open, so this can be null while one + // is closing -- and it used to be a disposed server instead, which swallowed the + // message silently. Losing the notification is the right failure here; turning a + // handled download error into a NullReferenceException is not. + BloomWebSocketServer.Instance?.SendEvent( "makeThumbnailFile-" + desiredFileNameWithoutExtension, "error: " + ex.Message ); diff --git a/src/BloomExe/InstallerSupport.cs b/src/BloomExe/InstallerSupport.cs index ff59c84ddb67..1d38d26a247f 100644 --- a/src/BloomExe/InstallerSupport.cs +++ b/src/BloomExe/InstallerSupport.cs @@ -173,6 +173,8 @@ static bool CheckForBadInstall() /// /// True if we consider our install to be shared by all users of the computer. /// We currently detect this based on being in the Program Files folder. + /// NOTE: currently always false -- we no longer ship an all-users install -- so the + /// branches this guards are dead code (as of 6.4, 2026). /// /// public static bool SharedByAllUsers() @@ -195,7 +197,7 @@ internal static void MakeBloomRegistryEntries(string[] programArgs) if (Program.RunningUnitTests) return; // unit testing. // If we support allUsers installs, we will need to make an exception here when called during installation. - if (SharedByAllUsers()) + if (SharedByAllUsers()) // currently always false; see its comment return; _installInLocalMachine = SharedByAllUsers(); if (Platform.IsLinux) @@ -345,7 +347,7 @@ private static RegistryKey HiveToMakeRegistryKeysIn { get { - if (SharedByAllUsers()) + if (SharedByAllUsers()) // currently always false; see its comment return Registry.LocalMachine.CreateSubKey(@"Software\Classes"); else return Registry.CurrentUser.CreateSubKey(@"Software\Classes"); diff --git a/src/BloomExe/MiscUI/BloomMessageBox.cs b/src/BloomExe/MiscUI/BloomMessageBox.cs index f3e7506902a1..e29c4e624b45 100644 --- a/src/BloomExe/MiscUI/BloomMessageBox.cs +++ b/src/BloomExe/MiscUI/BloomMessageBox.cs @@ -36,6 +36,14 @@ public static string Show( { messageHtml, rightButtonDefinitions = rightButtons, + // KNOWN GAP: you cannot currently ask for a warning icon and get one. + // MessageBoxIcon.Warning and .Exclamation are the same value in .NET, so + // ToString() here yields "exclamation", while BloomMessageBox.tsx only + // handles "warning" and "asterisk" -- so passing Warning silently draws + // nothing at all. (Measured: zero icons rendered.) MessageBoxIcon.Information + // does work, because it stringifies to "Asterisk". Whoever needs a warning + // icon should either map Exclamation to "warning" here or teach the .tsx the + // name it is actually being sent; until then, do not expect one. See BL-16690. icon = icon.ToString().ToLowerInvariant(), closeWithAPICall = true, } diff --git a/src/BloomExe/MiscUI/StartupScreenManager.cs b/src/BloomExe/MiscUI/StartupScreenManager.cs index 5ae90d886746..d4519cd15b7a 100644 --- a/src/BloomExe/MiscUI/StartupScreenManager.cs +++ b/src/BloomExe/MiscUI/StartupScreenManager.cs @@ -301,6 +301,29 @@ private static void DoStartupAction(object sender, EventArgs e) ConsiderCurrentTaskDone(); } + /// + /// Get the splash screen out of the way of a dialog we are about to show, without doing the + /// things that belong to startup actually being over. In particular + /// DoLastOfAllAfterClosingSplashScreen brings the main window to the front, which is both + /// wrong here (there may not be one yet) and wasteful, since it is a one-shot: consuming it + /// now means the main window doesn't come to the front when it finally does open. + /// This does the same thing DoStartupAction does for a task with ShouldHideSplashScreen. + /// + public static void HideSplashScreenForDialog() + { + if (_splashForm != null) + { + _splashForm.Hide(); + _splashForm = null; // it's gone, not needed again + } + + if (_doWhenSplashScreenShouldClose != null) + { + _doWhenSplashScreenShouldClose(); + _doWhenSplashScreenShouldClose = null; + } + } + public static void PutSplashAbove(Form aboveThis) { if (_splashForm != null) diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index 88671ec742b5..dff742c20de0 100644 --- a/src/BloomExe/Program.cs +++ b/src/BloomExe/Program.cs @@ -1634,6 +1634,48 @@ private static bool OpenProjectWindow(string collectionPath) return false; } + // The collection may declare a minimum Bloom version. Check that before we go to the + // trouble of building a ProjectContext around it. This is the one place all the ways + // of opening a collection funnel through, and each caller responds to a false return + // by putting up the collection chooser, which is one of the two choices we offer the + // user here. See BL-16690. + // + // Deliberately, this gates only the interactive ways of opening a collection. The + // command-line commands and BulkUploader build a ProjectContext directly and so are + // not stopped by a minimum version. We decided to leave it that way: those tools have + // no user to read a dialog or choose another collection, and failing an overnight + // upload job is its own kind of damage. If the flag ever needs to protect the files + // themselves rather than warn a person, this is the decision to revisit. + if (MinimumBloomVersionCheck.IsThisBloomTooOld(path, out var minimumBloomVersion)) + { + // We're normally still showing the splash screen at this point, and it would sit on + // top of our dialog. Also, the dialog is a ReactDialog, so it needs the server. + StartupScreenManager.HideSplashScreenForDialog(); + _applicationContainer.BloomServer.EnsureListening(); + if ( + MinimumBloomVersionCheck.ReportCollectionNeedsNewerBloom( + Path.GetFileNameWithoutExtension(path), + minimumBloomVersion + ) + ) + { + // The user chose to upgrade: a newer Bloom has been downloaded and we have + // asked Bloom to quit so it can be installed. Claim success, not because we + // opened anything, but so that no caller puts up the collection chooser + // while we are shutting down -- that would start a fresh modal message loop + // and could keep Bloom alive. + // + // NOTE for anyone adding a caller: this is the one path where we return true + // with _projectContext still null. It suits today's callers, which only use + // the result to decide whether to offer the chooser (and recording this + // collection as most-recently-used is right, since the upgraded Bloom should + // reopen it). A caller that goes on to use _projectContext would need a + // three-way result -- opened / refused / quitting -- instead of this bool. + return true; + } + return false; + } + //NB: initially, you could have multiple blooms, if they were different projects. //however, then we switched to the embedded http image server, which can't share //a port. So we could fix that (get different ports), but for now, I'm just going @@ -1657,6 +1699,11 @@ private static bool OpenProjectWindow(string collectionPath) BloomThreadCancelService.Dispose(); BloomThreadCancelService = new CancellationTokenSource(); + // We have a collection open, so no lock-out is in progress any more. Without this, + // a user locked out of collection A who moved to B and later came back to A would + // find that A could never lock them out again this session. See BL-16690. + MinimumBloomVersionCheck.NoteCollectionOpened(); + return true; } catch (Exception e) @@ -1841,6 +1888,11 @@ private static bool OpenCollection(string path) { if (OpenProjectWindow(path)) { + // Note that OpenProjectWindow also reports success when the user chose to upgrade + // rather than open anything. Recording the collection is right in that case too: an + // upgrade has been downloaded and will be installed as Bloom closes, so the next + // Bloom to start really can open this collection, and landing straight back in it is + // exactly what the user was trying to do. Settings.Default.MruProjects.AddNewPath(path); Settings.Default.Save(); return true; @@ -1884,7 +1936,14 @@ static void HandleProjectWindowClosed(object sender, EventArgs e) private static void ReopenProject(object sender, EventArgs e) { Application.Idle -= ReopenProject; - OpenCollection(Settings.Default.MruProjects.Latest); + if (!OpenCollection(Settings.Default.MruProjects.Latest)) + { + // The shell is already closed by the time we get here, so if the re-open failed + // we must put the chooser up or the user is left with no window at all. This can + // happen now that a collection can refuse to open: a Team Collection member on a + // newer Bloom can add a MinimumBloomVersion that syncs down mid-session. + ChooseACollection(); + } } private static void OpenCollectionChosenInDialog(object sender, EventArgs e) diff --git a/src/BloomExe/ProjectContext.cs b/src/BloomExe/ProjectContext.cs index 4842f19e73a2..5a8f327ed3da 100644 --- a/src/BloomExe/ProjectContext.cs +++ b/src/BloomExe/ProjectContext.cs @@ -185,7 +185,6 @@ IContainer parentContainer typeof(SignLanguageApi), typeof(AudioSegmentationApi), typeof(FileIOApi), - typeof(ProgressDialogApi), typeof(EditingViewApi), typeof(ProblemReportApi), typeof(FontsApi), @@ -432,7 +431,12 @@ IContainer parentContainer _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); - _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); + // ProgressDialogApi is registered once, application-wide, by ApplicationContainer: a + // progress dialog has to be possible before any collection is open. Registering it + // here as well would be a duplicate key in the endpoint dictionary, which throws -- + // and application-level registrations are deliberately NOT cleared between + // collections, so the second one would never go away. This is the same arrangement as + // CommonApi and NewCollectionWizardApi, which likewise appear only there. _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); _scope.Resolve().RegisterWithApiHandler(server.ApiHandler); diff --git a/src/BloomExe/TeamCollection/FolderTeamCollection.cs b/src/BloomExe/TeamCollection/FolderTeamCollection.cs index b2d3b041c0d8..a7c9246499ab 100644 --- a/src/BloomExe/TeamCollection/FolderTeamCollection.cs +++ b/src/BloomExe/TeamCollection/FolderTeamCollection.cs @@ -364,6 +364,50 @@ private static string GetCollectionSettingsEntryName(string zipPath) return null; } + /// + /// The repository's copy of the collection settings for a local collection folder, or null if + /// this is not a Team Collection or we cannot reach the repository right now. + /// + /// Static, and taking a plain folder path, because the minimum-version gate has to ask this + /// before any TeamCollection object exists -- indeed before Bloom has committed to opening + /// the collection at all. Otherwise a member whose repository has just gained a minimum + /// version would be judged on their own stale copy and let in for the whole session, since + /// the repository is not copied down until later in startup. See BL-16690. + /// + internal static string GetRepoCollectionSettingsForCollectionFolder( + string localCollectionFolder + ) + { + try + { + if (string.IsNullOrEmpty(localCollectionFolder)) + return null; + var linkPath = TeamCollectionManager.GetTcLinkPathFromLcPath(localCollectionFolder); + if (!RobustFile.Exists(linkPath)) + return null; // an ordinary collection; nothing to consult + var repoFolder = TeamCollectionManager.RepoFolderPathFromLinkPath(linkPath); + if (string.IsNullOrWhiteSpace(repoFolder) || !Directory.Exists(repoFolder)) + return null; // disconnected, or the shared folder has moved + var zipPath = GetRepoProjectFilesZipPath(repoFolder); + if (!RobustFile.Exists(zipPath)) + return null; + var entryName = GetCollectionSettingsEntryName(zipPath); + if (entryName == null) + return null; + return RobustZip.GetZipEntryContent(zipPath, entryName); + } + catch (Exception e) + { + // Offline, a part-written zip, a network share that has gone away: none of that is a + // good reason to stop someone opening a collection. The local file still has its say. + Logger.WriteError( + "Could not read the Team Collection repo settings while checking the minimum Bloom version", + e + ); + return null; + } + } + /// /// Return a list of all the books currently in the repo. (It will not update as changes are made, /// either locally or remotely. Beware that conceivably a book in the list might be removed @@ -471,6 +515,26 @@ public override void PutCollectionFiles(string[] names) RobustZip.WriteFilesToZip(names, _localCollectionFolder, destPath); } + /// + /// Read the collection settings straight out of the repo's zip of collection files, without + /// unpacking anything or disturbing the local copy. Used to notice a minimum Bloom version + /// arriving from an administrator while the user has the collection open. + /// + protected override string GetRepoCollectionSettingsContent() + { + var zipPath = GetRepoProjectFilesZipPath(_repoFolderPath); + if (!RobustFile.Exists(zipPath)) + return null; + // Find the entry by its extension rather than building the name from the local + // collection path: the collection may have been renamed locally, in which case the + // name we would build is not the one in the repo. (Shared with BL-16691, which reads + // AllowCheckouts out of the same file.) + var entryName = GetCollectionSettingsEntryName(zipPath); + if (entryName == null) + return null; + return RobustZip.GetZipEntryContent(zipPath, entryName); + } + protected override DateTime LastRepoCollectionFileModifyTime { get diff --git a/src/BloomExe/TeamCollection/TeamCollection.cs b/src/BloomExe/TeamCollection/TeamCollection.cs index c5615da701d0..ea32ae30fe28 100644 --- a/src/BloomExe/TeamCollection/TeamCollection.cs +++ b/src/BloomExe/TeamCollection/TeamCollection.cs @@ -877,6 +877,21 @@ public void SyncLocalAndRepoCollectionFiles(bool atStartup = true) // allowed, and the next Save() would write that back over the change and // un-pause the whole team. See BL-16691. UpdateAllowCheckoutsFromRepo(); + // MinimumBloomVersion is hand-edited into the local file by exactly the same + // administrator workflow, and would be erased by exactly the same next Save(). + // We only take the value; we deliberately do NOT lock the administrator out + // from here. This runs while the collection is still being opened, with no + // Shell to own a dialog and nowhere sensible to send them. Their own gate + // catches it at the next launch, by which time the local file says so. See + // BL-16690. + // + // Read it from the local file rather than by reading back the zip we have + // just written. That zip can briefly refuse to open -- Dropbox mid-sync, a + // part-written file -- and a failed read would leave our in-memory copy + // empty, which is exactly how the next ordinary Save() would delete the + // administrator's requirement for the whole team. The local file is where + // the value came from a moment ago, so it cannot fail us that way. + RememberRepoMinimumBloomVersion(TryReadLocalCollectionSettingsContent()); } } } @@ -1211,7 +1226,29 @@ internal void QueuePendingBookChange(RepoChangeEventArgs args) _pendingRepoChanges.Enqueue(args); } + private bool _handlingRepoChangeOnIdle; + internal void HandleRemoteBookChangesOnIdle(object sender, EventArgs e) + { + // Handling a change can put up a modal dialog (a lock-out, for one), and while that + // dialog is up the message pump keeps delivering Idle events, so we would re-enter and + // start processing further repo changes on top of the one we are still in the middle + // of -- against a collection we may be busy closing. Whatever is left in the queue can + // wait for the next Idle after we return. + if (_handlingRepoChangeOnIdle) + return; + _handlingRepoChangeOnIdle = true; + try + { + HandleOneRemoteBookChange(); + } + finally + { + _handlingRepoChangeOnIdle = false; + } + } + + private void HandleOneRemoteBookChange() { if (_pendingRepoChanges.TryDequeue(out RepoChangeEventArgs args)) { @@ -1225,8 +1262,13 @@ internal void HandleRemoteBookChangesOnIdle(object sender, EventArgs e) HandleDeletedRepoFileAfterPause(delArgs); else if (args is BookRepoChangeEventArgs changeArgs) HandleModifiedFile(changeArgs); - else - HandleCollectionSettingsChange(args); + else if (HandleCollectionSettingsChange(args)) + { + // We just shut the user out of this collection, so Bloom is either quitting or + // closing the collection down. Don't go on to poke at the book selection of a + // collection that is being torn down. + return; + } // These "HandleX" methods above send a C# event, which is helpful for the C# end of things. // Unfortunately, a websocket message is needed to make sure that javascript-land is up-to-date // with any remote changes, for example, that the TeamCollection button updates (See BL-10270). @@ -1343,7 +1385,9 @@ internal void HandleDeletedRepoFile(string fileName) UpdateBookStatus(bookBaseName, true); } - internal void HandleCollectionSettingsChange(RepoChangeEventArgs result) + /// true if we are shutting the user out of this collection, so the caller should + /// stop working with it. + internal bool HandleCollectionSettingsChange(RepoChangeEventArgs result) { _tcLog.WriteMessage( MessageAndMilestoneType.NewStuff, @@ -1352,7 +1396,160 @@ internal void HandleCollectionSettingsChange(RepoChangeEventArgs result) null, null ); + // Check this first. It can shut the user out of the collection altogether, and it + // blocks in a modal dialog until they decide, so there would be no point updating a + // setting on a collection they are in the middle of leaving. + if (CheckWhetherRepoNowRequiresANewerBloom()) + return true; + UpdateAllowCheckoutsFromRepo(); + return false; + } + + /// + /// The change a teammate just made may have been to set a minimum Bloom version that this + /// Bloom doesn't meet. Unlike other collection settings, that one can't wait until the next + /// restart to take effect: the whole point of it is to stop this Bloom touching the + /// collection, and the user is in it right now. So we shut them out immediately. + /// + /// Note that we read the repository's copy of the settings, not the local one. Bloom + /// deliberately doesn't overwrite local collection settings mid-session, so the local file + /// still says what it said when the collection was opened. + /// + /// true if we shut the user out of the collection. + private bool CheckWhetherRepoNowRequiresANewerBloom() + { + var repoSettings = TryGetRepoCollectionSettingsContent(); + + RememberRepoMinimumBloomVersion(repoSettings); + + if ( + !MinimumBloomVersionCheck.IsThisBloomTooOldForSettings( + repoSettings, + out var minimumVersion + ) + ) + return false; + + // ErrorNoReload rather than Error: reloading is exactly what cannot help here, since the + // reloaded collection meets the same gate. This entry usually spends no time on screen, + // because the lock-out dialog takes over immediately -- but it stays visible if the + // lock-out is skipped because one is already under way for this collection. + _tcLog.WriteMessage( + MessageAndMilestoneType.ErrorNoReload, + // The same words, and so the same XLF entry, as the lock-out dialog's header. + "Collection.NewerVersionNeededHeader", + "This collection needs a newer version of Bloom.", + null, + null + ); + + return MinimumBloomVersionCheck.LockUserOutOfOpenCollection( + Path.GetFileNameWithoutExtension(CollectionPath(_localCollectionFolder)), + minimumVersion + ); + } + + /// + /// This computer's own copy of the collection settings, or null if we cannot read it. Used + /// straight after we have pushed the local files up, when the local file is by definition + /// what the repository now holds and is the more reliable of the two to read. + /// + private string TryReadLocalCollectionSettingsContent() + { + try + { + var path = CollectionPath(_localCollectionFolder); + if (!RobustFile.Exists(path)) + return null; + return RobustFile.ReadAllText(path, Encoding.UTF8); + } + catch (Exception e) + { + Logger.WriteError("TeamCollection could not read the local collection settings", e); + return null; + } + } + + /// + /// The repository's copy of the collection settings, or null if we cannot get at it just + /// now -- there is no repo copy yet, the zip is part-written, Dropbox is mid-sync. Never + /// throws: failing to read it is a normal transient condition, and no caller should + /// interrupt someone's work over it. If the collection really does require a newer Bloom, + /// we find out at the next start. + /// + private string TryGetRepoCollectionSettingsContent() + { + try + { + return GetRepoCollectionSettingsContent(); + } + catch (Exception e) + { + Logger.WriteError("TeamCollection could not read the repo collection settings", e); + return null; + } + } + + /// + /// Copy the repository's minimum version into the settings we are holding in memory. This + /// matters even when this Bloom is new enough to carry on: CollectionSettings.Save() rebuilds + /// the file from scratch and writes MinimumBloomVersion out of memory, and plenty of ordinary + /// actions save mid-session. If our copy were still the empty value we loaded before the + /// administrator's change arrived, the next save would drop the element from the local file, + /// and the Team Collection would push that up -- quietly removing the protection for the + /// whole team, from the very Bloom it was meant to keep out. + /// + private void RememberRepoMinimumBloomVersion(string repoSettings) + { + var settings = _tcManager?.Settings; + if (settings == null) + return; // no settings to update (unit tests) + + // Not knowing what the repo says is quite different from the repo saying "no minimum". + // Only the second of those should clear what we are holding. + if (string.IsNullOrWhiteSpace(repoSettings)) + return; + + string repoValue; + try + { + repoValue = MinimumBloomVersionCheck.ParseMinimumBloomVersion(repoSettings); + } + catch (Exception e) + { + Logger.WriteError( + "TeamCollection could not parse the repo collection settings to read its minimum Bloom version", + e + ); + return; + } + + // Record it against the collection as well as in the settings object. The local + // .bloomCollection is deliberately not rewritten mid-session, so without this the + // startup gate would read the stale file and let someone we had just shut out back in + // by picking the same collection from the chooser. See BL-16690. + MinimumBloomVersionCheck.RememberMinimumVersionFromRepo( + CollectionPath(_localCollectionFolder), + repoValue + ); + + if (repoValue == settings.MinimumBloomVersion) + return; + Logger.WriteEvent( + $"TeamCollection: MinimumBloomVersion changed remotely to '{repoValue}'." + ); + settings.MinimumBloomVersion = repoValue; + } + + /// + /// The text of the collection settings file as it stands in the repository right now, or + /// null if we have no way to get at it. Subclasses that keep the collection files somewhere + /// we can read should override this. + /// + protected virtual string GetRepoCollectionSettingsContent() + { + return null; } /// diff --git a/src/BloomExe/UpdateReporter.cs b/src/BloomExe/UpdateReporter.cs new file mode 100644 index 000000000000..2db146b97744 --- /dev/null +++ b/src/BloomExe/UpdateReporter.cs @@ -0,0 +1,300 @@ +using System; +using System.Threading; +using Bloom.ErrorReporter; +using Bloom.web; +using SIL.Reporting; + +namespace Bloom +{ + /// + /// What came of an update attempt. + /// + internal enum UpdateAttemptOutcome + { + /// A newer Bloom was downloaded and will be installed when Bloom exits. + Downloaded, + + /// We reached the update feed, and there is nothing newer on this channel. + NothingNewer, + + /// There IS something newer and the user has been offered it, but has not said yes yet. + /// Distinct from NothingNewer because a caller that reports the outcome to the user would + /// otherwise tell them the exact opposite of the truth. + Offered, + + /// We can't update this copy of Bloom at all: a developer build, one an administrator + /// manages, or one running under the debugger. + CannotUpdateThisBloom, + + /// We tried and something went wrong -- most likely we couldn't reach the feed. + Failed, + + /// The user stopped it. Nothing was downloaded, nothing is waiting to install, and a later + /// attempt in the same session can start from scratch. + Cancelled, + } + + /// + /// Where the update code's running commentary goes. + /// + /// Everything it has to say is worked out in one place, ApplicationUpdateSupport's message + /// methods, and arrives here already localized; this class only decides where the words are + /// shown. Normally that is a toast, which is why the update code grew up calling ToastService + /// directly. But toasts are drawn by ToastHost, which is only mounted in the main workspace, so + /// before a collection is open they go nowhere at all -- and the "this collection needs a newer + /// Bloom" dialog runs exactly there. Rather than teach that dialog to reproduce the update + /// code's wording, it hands in a reporter that puts the very same sentences into a progress + /// dialog. See BL-16690. + /// + internal abstract class UpdateReporter + { + /// + /// Ordinary news: looking, downloading, already up to date. + /// + public abstract void Say(string message); + + /// + /// Something is not right, but the attempt is not over. + /// + public abstract void SayWarning(string message); + + /// + /// Something went wrong. The exception, where we have one, is what a problem report would + /// be built from. + /// + public abstract void SayProblem(string message, Exception exception); + + /// + /// There is an update to be had, if the user wants it. + /// + public abstract void OfferToDownload(string message, string acceptLabel, Action accept); + + /// + /// It is downloaded, and takes effect when Bloom restarts. + /// + public abstract void OfferToRestart(string message, string acceptLabel, Action accept); + + /// + /// The attempt is over, one way or another, and nothing further will be said. The words in + /// are the ones that were used to explain a failure, so a caller + /// that wants to repeat them somewhere else says the same thing we did. + /// + public virtual void Finished( + UpdateAttemptOutcome outcome, + string downloadedVersion, + string message + ) { } + + /// + /// How far along the download is, 0-100. Ignored by the toasts, which have nowhere to put it. + /// + public virtual void Percent(int percent) { } + + /// + /// Cancelled when whoever is watching this attempt gives up on it, so the download really + /// stops rather than carrying on unwatched. Never cancelled for a route with no way to ask. + /// + public virtual CancellationToken CancellationToken => CancellationToken.None; + } + + /// + /// The normal route: everything the update code says becomes a toast in the workspace, exactly + /// as it did before there was any other route. + /// + internal class ToastUpdateReporter : UpdateReporter + { + public override void Say(string message) + { + ToastService.ShowToast(type: ToastType.Update, text: message, durationSeconds: 5); + } + + public override void SayWarning(string message) + { + ToastService.ShowToast(ToastType.Warning, text: message, durationSeconds: 5); + } + + public override void SayProblem(string message, Exception exception) + { + ToastService.ShowToast( + ToastType.Error, + text: message, + durationSeconds: 10, + action: new ToastAction + { + Callback = () => ErrorReport.NotifyUserOfProblem(exception, message), + } + ); + } + + public override void OfferToDownload(string message, string acceptLabel, Action accept) + { + ToastService.ShowToast( + type: ToastType.Update, + text: message, + durationSeconds: 10, + action: new ToastAction { Label = acceptLabel, Callback = () => accept() } + ); + } + + public override void OfferToRestart(string message, string acceptLabel, Action accept) + { + // Deliberately no duration: this one stays until the user deals with it. + ToastService.ShowToast( + type: ToastType.Update, + text: message, + action: new ToastAction { Label = acceptLabel, Callback = () => accept() } + ); + } + } + + /// + /// The route for an update the user asked for before any collection is open: the same sentences, + /// written into a progress dialog, plus a real percentage while the download runs. + /// + /// It also collects the outcome, because the caller has to know what to do next -- and, unlike + /// the toasts, has to do it itself. + /// + internal class ProgressUpdateReporter : UpdateReporter + { + // Until the progress dialog is up there is nowhere to write, and a reporter that exists + // before its dialog is what lets the caller always have one to ask about. + private IWebSocketProgress _progress = new NullWebSocketProgress(); + private readonly ManualResetEventSlim _finished = new ManualResetEventSlim(false); + + /// + /// Start writing to the progress dialog, once there is one. + /// + public void WriteTo(IWebSocketProgress progress) + { + _progress = progress; + } + + public UpdateAttemptOutcome Outcome { get; private set; } = UpdateAttemptOutcome.Failed; + + /// + /// The user pressed Cancel. Worth knowing separately from the outcome, because a download + /// can finish in the very moment they cancel, and a caller acting on the outcome alone would + /// restart Bloom under someone who had just said no. + /// + public bool UserCancelled { get; private set; } + + private readonly CancellationTokenSource _cancelDownload = new CancellationTokenSource(); + + public override CancellationToken CancellationToken => _cancelDownload.Token; + + /// + /// Stop the download, because the user pressed Cancel. Velopack is given this through + /// and abandons the transfer, so nothing is left downloaded + /// and nothing is waiting to install -- which is the only reading of a button labelled + /// Cancel that a user could be expected to accept. + /// + public void CancelTheDownload() + { + UserCancelled = true; + _cancelDownload.Cancel(); + } + + /// + /// The last thing we told the user, so that Finished does not repeat it. + /// + private string _lastSaid; + + public override void Say(string message) + { + _lastSaid = message; + _progress.MessageWithoutLocalizing(message); + } + + public override void SayWarning(string message) + { + _lastSaid = message; + _progress.MessageWithoutLocalizing(message, ProgressKind.Warning); + } + + public override void SayProblem(string message, Exception exception) + { + _lastSaid = message; + _progress.MessageWithoutLocalizing(message, ProgressKind.Error); + if (exception != null) + Logger.WriteError("Bloom was unable to update itself", exception); + } + + // There are no buttons here, so an offer cannot be made. Whether to take one up is the + // caller's decision, made from what Finished tells it, which is why neither of these + // quietly accepts on the user's behalf either. + public override void OfferToDownload(string message, string acceptLabel, Action accept) => + Say(message); + + /// + /// Deliberately silent. "Update for 6.4.108 is ready" is a fine thing for a toast to say, + /// because a toast is all the user gets; here the caller acts on the Downloaded outcome + /// itself -- the dialog closes and Bloom restarts -- so announcing it first would be news + /// about something the user is about to watch happen. + /// + public override void OfferToRestart(string message, string acceptLabel, Action accept) { } + + private int _lastPercentReported = -1; + + public override void Percent(int percent) + { + // Velopack calls this for every chunk it reads, which for a ninety-megabyte download is + // a great many times per percentage point. Each one would be a websocket message, so + // only pass on the ones that would actually change what the user sees. + if (percent == _lastPercentReported) + return; + _lastPercentReported = percent; + _progress.SendPercent(percent); + } + + public override void Finished( + UpdateAttemptOutcome outcome, + string downloadedVersion, + string message + ) + { + Outcome = outcome; + + // Say it if nobody has. The update code decides what to show from `verbosity`, and we + // ask for Quiet -- so on the paths where it stays quiet (it cannot reach the update + // server, or another attempt is already running) the explanation reaches us here and + // nowhere else. Without this, someone with no internet clicks Upgrade Bloom and gets + // an empty dialog. Toasts have no equivalent gap: there, staying quiet IS the answer, + // because the user did not ask for anything. + if (!string.IsNullOrEmpty(message) && message != _lastSaid) + { + _progress.MessageWithoutLocalizing( + message, + outcome == UpdateAttemptOutcome.NothingNewer + ? ProgressKind.Progress + : ProgressKind.Error + ); + } + + _finished.Set(); + } + + /// + /// Wait up to for the update attempt to report back. + /// + /// false if it has not reported back yet + public bool WaitForFinish(TimeSpan timeout) + { + return _finished.Wait(timeout); + } + + // _finished and _cancelDownload are deliberately never disposed, and this was reviewed and + // kept as it stands (BL-16690, 2026-08-19). + // + // Waiting on _finished with a timeout does allocate a kernel handle, so one leaks per + // upgrade the user asks for. Two attempts at releasing it safely each had their own race: + // the "has it reported yet" flag has to be set either before or after Set(), and both sides + // are wrong -- before, and a cancel in that instant disposes the event as the download is + // about to signal it; after, and the waiter can dispose before the flag is set. That is a + // lot of delicate reasoning to buy back one handle in a process usually about to exit and + // reinstall itself. + // + // Cancel now really cancels, so the window in which anything still touches this object + // after the caller has finished with it is much smaller than it was -- but "much smaller" + // is not "closed", and nothing here is worth another race. + } +} diff --git a/src/BloomExe/WebLibraryIntegration/BookUpload.cs b/src/BloomExe/WebLibraryIntegration/BookUpload.cs index 3068e64a4c97..71b889909b35 100644 --- a/src/BloomExe/WebLibraryIntegration/BookUpload.cs +++ b/src/BloomExe/WebLibraryIntegration/BookUpload.cs @@ -1067,7 +1067,10 @@ internal async Task FullUpload( var url = BloomLibraryUrls.BloomLibraryDetailPageUrlFromBookId(bookObjectId); book.ReportSimplisticFontAnalytics(FontAnalytics.FontEventType.PublishWeb, url); - BloomWebSocketServer.Instance.SendEvent("booksOnBlorg", "reload"); + // Instance is only set while a collection is open. An upload that finishes as the + // collection is closing has nothing left to tell, and no longer has a disposed + // server to tell it to, so say nothing rather than throw. + BloomWebSocketServer.Instance?.SendEvent("booksOnBlorg", "reload"); return bookObjectId; } finally diff --git a/src/BloomExe/Workspace/WorkspaceView.cs b/src/BloomExe/Workspace/WorkspaceView.cs index 7d1ae2c2939c..6ec31beb385e 100644 --- a/src/BloomExe/Workspace/WorkspaceView.cs +++ b/src/BloomExe/Workspace/WorkspaceView.cs @@ -921,7 +921,7 @@ private void _applicationUpdateCheckTimer_Tick(object sender, EventArgs e) if ( !Debugger.IsAttached && Platform.IsWindows - && !InstallerSupport.SharedByAllUsers() + && !InstallerSupport.SharedByAllUsers() // currently always false; see its comment && !ApplicationUpdateSupport.IsDev ) { @@ -1644,6 +1644,7 @@ private void CheckForUpdatesImpl() { MessageBox.Show(this, "Sorry, you cannot check for updates from the debugger."); } + // Currently dead: SharedByAllUsers is always false now (see its comment). else if (InstallerSupport.SharedByAllUsers()) { MessageBox.Show( diff --git a/src/BloomExe/web/BloomWebSocketServer.cs b/src/BloomExe/web/BloomWebSocketServer.cs index a3bea6c044b3..c4e841d1a03d 100644 --- a/src/BloomExe/web/BloomWebSocketServer.cs +++ b/src/BloomExe/web/BloomWebSocketServer.cs @@ -351,6 +351,15 @@ public void Dispose() _server = null; } } + + // Stop advertising ourselves once we can no longer carry a message. Without this, + // Instance goes on pointing at this disposed server after its collection closes, so + // the two places that ask "is there a server already?" -- WorkspaceView's language + // chooser and the minimum-version upgrade dialog -- are told yes and then talk to + // something that will never answer. In the dialog's case that means a progress window + // that never fills in and, having no close box, cannot be dismissed. + if (ReferenceEquals(Instance, this)) + Instance = null; } } } diff --git a/src/BloomTests/Collection/MinimumBloomVersionCheckTests.cs b/src/BloomTests/Collection/MinimumBloomVersionCheckTests.cs new file mode 100644 index 000000000000..996e8da89f48 --- /dev/null +++ b/src/BloomTests/Collection/MinimumBloomVersionCheckTests.cs @@ -0,0 +1,387 @@ +using System; +using System.IO; +using Bloom.Collection; +using NUnit.Framework; +using SIL.IO; +using SIL.TestUtilities; + +namespace BloomTests.Collection +{ + /// + /// Tests the gate that keeps an older Bloom from opening a collection that declares + /// a MinimumBloomVersion. See BL-16690. + /// + [TestFixture] + public class MinimumBloomVersionCheckTests + { + private TemporaryFolder _folder; + + [OneTimeSetUp] + public void FixtureSetup() + { + _folder = new TemporaryFolder("MinimumBloomVersionCheckTests"); + } + + [OneTimeTearDown] + public void Cleanup() + { + _folder.Dispose(); + } + + // No requirement at all: everything is allowed in. + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + // A requirement we can't make sense of is ignored rather than locking the user out. + [TestCase("banana")] + [TestCase("6")] // Version.TryParse insists on at least major.minor + [TestCase("6.5-beta")] + public void IsVersionSufficient_NoUsableRequirement_AllowsAnyVersion(string minimumVersion) + { + Assert.That( + MinimumBloomVersionCheck.IsVersionSufficient(minimumVersion, new Version(1, 0, 0)), + Is.True, + "Even an ancient Bloom should be allowed in when there is no usable requirement." + ); + } + + [TestCase("6.5", "6.5.0.0", true, Description = "exactly the required version")] + [TestCase("6.5", "6.5.132.0", true, Description = "same minor, later build")] + [TestCase("6.5", "6.6.0.0", true, Description = "later minor")] + [TestCase("6.5", "7.0.0.0", true, Description = "later major")] + [TestCase("6.5", "6.4.900.0", false, Description = "earlier minor, even with a high build")] + [TestCase("6.5", "5.9.0.0", false, Description = "earlier major")] + [TestCase("7.0", "6.9.0.0", false, Description = "earlier major, higher minor")] + // We compare major.minor only, matching Bloom's other version gates, so a build number + // in the requirement is deliberately ignored. + [TestCase("6.5.132", "6.5.10.0", true, Description = "build number in requirement ignored")] + public void IsVersionSufficient_ComparesMajorAndMinor( + string minimumVersion, + string runningVersion, + bool expected + ) + { + Assert.That( + MinimumBloomVersionCheck.IsVersionSufficient( + minimumVersion, + Version.Parse(runningVersion) + ), + Is.EqualTo(expected) + ); + } + + /// + /// We compare major.minor only, so that is what the user should be shown. Reporting a build + /// number we don't actually enforce would misrepresent the rule. Version 99 is used so these + /// stay true however far Bloom's real version advances. + /// + // Each case needs its own collection name: two of these differ only by whitespace, so + // deriving the name from the value would have them share a folder and overwrite each other. + [TestCase("99.0", "99.0", "ReportedPlain")] + [TestCase( + "99.0.132", + "99.0", + "ReportedWithBuild", + Description = "build number dropped, since we ignore it" + )] + [TestCase( + " 99.0 ", + "99.0", + "ReportedPadded", + Description = "surrounding whitespace tolerated" + )] + public void IsThisBloomTooOld_TooOld_ReportsRequirementAsMajorMinor( + string declaredVersion, + string expectedReported, + string collectionName + ) + { + var path = WriteSettingsFile( + collectionName, + $"{declaredVersion}" + ); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out var minimumVersion), + Is.True, + "No conceivable Bloom version satisfies a requirement of 99.x." + ); + Assert.That(minimumVersion, Is.EqualTo(expectedReported)); + } + + [Test] + public void IsThisBloomTooOld_RequirementSatisfied_ReturnsFalse() + { + var path = WriteSettingsFile( + "AncientRequirement", + "0.1" + ); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out var minimumVersion), + Is.False, + "Every Bloom that has ever shipped is newer than 0.1." + ); + Assert.That(minimumVersion, Is.Empty); + } + + /// + /// A Team Collection has to judge the settings sitting in the repository, which it reads out + /// of a zip and never writes to disk, so the same check has to work on content in hand. + /// + [Test] + public void IsThisBloomTooOldForSettings_RequirementNoBloomCanMeet_SaysSoAndReportsIt() + { + var xml = SettingsXml("99.0"); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOldForSettings(xml, out var minimumVersion), + Is.True, + "No conceivable Bloom version satisfies a requirement of 99.x." + ); + Assert.That(minimumVersion, Is.EqualTo("99.0")); + } + + [Test] + public void IsThisBloomTooOldForSettings_NoRequirement_LetsUsIn() + { + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOldForSettings(SettingsXml(""), out _), + Is.False + ); + } + + /// + /// Nothing to read means we know nothing, which must not be mistaken for "you are locked + /// out" -- that would shut a Team Collection user out of their work over a read failure. + /// + [TestCase(null)] + [TestCase("")] + public void IsThisBloomTooOldForSettings_NothingToRead_LetsUsIn(string xml) + { + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOldForSettings(xml, out _), + Is.False + ); + } + + [Test] + public void IsThisBloomTooOldForSettings_UnparseableXml_LetsUsInRatherThanThrowing() + { + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOldForSettings("not xml at all", out _), + Is.False + ); + } + + private static string SettingsXml(string extraElements) => + $@" + + xyz + {extraElements} +"; + + [Test] + public void ReadMinimumBloomVersion_ElementPresent_ReturnsIt() + { + var path = WriteSettingsFile( + "MinimumVersionPresent", + "6.5" + ); + + Assert.That(MinimumBloomVersionCheck.ReadMinimumBloomVersion(path), Is.EqualTo("6.5")); + } + + [Test] + public void ReadMinimumBloomVersion_ElementAbsent_ReturnsEmpty() + { + var path = WriteSettingsFile("MinimumVersionAbsent", ""); + + Assert.That(MinimumBloomVersionCheck.ReadMinimumBloomVersion(path), Is.Empty); + } + + /// + /// A settings file we can't parse is a real problem, but it is not this check's problem to + /// report; the normal open will fail and give the user a much better error report. + /// + [Test] + public void ReadMinimumBloomVersion_UnparseableFile_ReturnsEmptyRatherThanThrowing() + { + var path = Path.Combine(_folder.Path, "Garbage.bloomCollection"); + RobustFile.WriteAllText(path, "this is not xml at all"); + + Assert.That(MinimumBloomVersionCheck.ReadMinimumBloomVersion(path), Is.Empty); + } + + [Test] + public void ReadMinimumBloomVersion_NoSuchFile_ReturnsEmpty() + { + var path = Path.Combine(_folder.Path, "NotThere.bloomCollection"); + Assert.That( + RobustFile.Exists(path), + Is.False, + "Test setup problem: this file was supposed to not exist." + ); + + Assert.That(MinimumBloomVersionCheck.ReadMinimumBloomVersion(path), Is.Empty); + } + + /// + /// Save() rebuilds the settings file from scratch, so a hand-added MinimumBloomVersion would be + /// silently lost the first time the user changed anything in Collection Settings, unless we + /// write it back out. That would be a nasty way to lose the protection. + /// + [Test] + public void CollectionSettings_MinimumBloomVersion_SurvivesLoadAndSave() + { + var path = WriteSettingsFile( + "RoundTrip", + "6.5" + ); + + var settings = new CollectionSettings(path); + Assert.That( + settings.MinimumBloomVersion, + Is.EqualTo("6.5"), + "Should have read the minimum version from the file we just wrote." + ); + + settings.Save(); + + Assert.That( + MinimumBloomVersionCheck.ReadMinimumBloomVersion(path), + Is.EqualTo("6.5"), + "Save() dropped the minimum version, which would leave the collection unprotected." + ); + } + + /// + /// We don't want to add a meaningless empty element to every collection settings file in the world. + /// + [Test] + public void CollectionSettings_NoMinimumBloomVersion_NotWrittenOnSave() + { + var path = WriteSettingsFile("NoMinimum", ""); + + var settings = new CollectionSettings(path); + Assert.That( + settings.MinimumBloomVersion, + Is.Empty, + "Test setup problem: there should be no minimum version yet." + ); + + settings.Save(); + + Assert.That( + RobustFile.ReadAllText(path), + Does.Not.Contain(CollectionSettings.kMinimumBloomVersionElementName) + ); + } + + /// + /// When a Team Collection administrator sets a minimum version mid-session, the copy of the + /// settings on this computer deliberately isn't rewritten, so it goes on saying nothing about + /// it. Someone we had just shut out could otherwise walk straight back in by choosing the same + /// collection from the chooser -- the gate would read the stale file and see no requirement. + /// See BL-16690. + /// + [Test] + public void IsThisBloomTooOld_RepoSaidNewerThanTheLocalFileDoes_StillSaysTooOld() + { + var path = WriteSettingsFile("RepoBeatsStaleFile", ""); + + // Sanity check: on the file alone, this collection is wide open. + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out _), + Is.False, + "Test setup problem: the file itself should demand nothing." + ); + + MinimumBloomVersionCheck.RememberMinimumVersionFromRepo(path, "99.0"); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out var minimumVersion), + Is.True + ); + Assert.That(minimumVersion, Is.EqualTo("99.0")); + } + + /// + /// The two sources can disagree, and neither wins outright: we take whichever demands more. + /// The repository copy lives in a zip that can be mid-sync and momentarily missing the + /// element, and this is a protection, so the failure we can live with is keeping someone out + /// a little longer than necessary rather than letting them in when we should not have. + /// + [Test] + public void IsThisBloomTooOld_RepoSaysNoRequirementButFileDoes_StillSaysTooOld() + { + var path = WriteSettingsFile( + "RepoSilentFileDemands", + "99.0" + ); + + // Sanity check: the file on its own shuts us out. + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out _), + Is.True, + "Test setup problem: the file should demand a version no Bloom has." + ); + + MinimumBloomVersionCheck.RememberMinimumVersionFromRepo(path, ""); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out var minimumVersion), + Is.True + ); + Assert.That(minimumVersion, Is.EqualTo("99.0")); + } + + /// + /// The other direction of the same rule: a requirement in the file we already meet must not + /// water down a stiffer one the repository has since told us about. + /// + [Test] + public void IsThisBloomTooOld_FileAsksLessThanTheRepoDoes_TakesTheRepoRequirement() + { + var path = WriteSettingsFile( + "FileAsksLess", + "1.0" + ); + + // Sanity check: on the file alone, any Bloom is welcome. + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out _), + Is.False, + "Test setup problem: the file should demand nothing this Bloom can't meet." + ); + + MinimumBloomVersionCheck.RememberMinimumVersionFromRepo(path, "99.0"); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(path, out var minimumVersion), + Is.True + ); + Assert.That(minimumVersion, Is.EqualTo("99.0")); + } + + /// + /// Writes a minimal .bloomCollection file in its own folder, since CollectionSettings + /// expects the folder to be named after the collection. + /// + private string WriteSettingsFile(string collectionName, string extraElements) + { + var collectionFolder = Path.Combine(_folder.Path, collectionName); + Directory.CreateDirectory(collectionFolder); + var path = Path.Combine(collectionFolder, collectionName + ".bloomCollection"); + RobustFile.WriteAllText( + path, + $@" + + xyz + {extraElements} +" + ); + return path; + } + } +} diff --git a/src/BloomTests/TeamCollection/FolderTeamCollectionTests2.cs b/src/BloomTests/TeamCollection/FolderTeamCollectionTests2.cs index 91446e279be9..046c0c155b14 100644 --- a/src/BloomTests/TeamCollection/FolderTeamCollectionTests2.cs +++ b/src/BloomTests/TeamCollection/FolderTeamCollectionTests2.cs @@ -1461,6 +1461,237 @@ public void SyncLocalAndRepoCollectionFiles_LocalPausePushedUp_UpdatesLiveSettin } } + /// + /// The first-launch gap. An administrator sets a minimum version while a teammate's Bloom is + /// closed. When that teammate starts up, their own copy of the settings is still yesterday's + /// -- Bloom does not copy the repository's collection files down until later in startup, well + /// after the gate has decided whether to open the collection. So the gate has to ask the + /// repository itself, or the teammate gets the whole session inside a collection they are no + /// longer allowed in. See BL-16690. + /// + [Test] + public void IsThisBloomTooOld_OnlyTheRepoKnowsAboutTheRequirement_StillSaysTooOld() + { + using (var collectionFolder = new TemporaryFolder("RepoOnlyRequirement_Collection")) + { + using (var repoFolder = new TemporaryFolder("RepoOnlyRequirement_Repo")) + { + var mockTcManager = new Mock(); + var settings = new CollectionSettings(); + mockTcManager.Setup(m => m.Settings).Returns(settings); + var tc = new TestFolderTeamCollection( + mockTcManager.Object, + collectionFolder.FolderPath, + repoFolder.FolderPath + ); + Directory.CreateDirectory(Path.Combine(repoFolder.FolderPath, "Books")); + var settingsPath = CollectionSettings.GetSettingsFilePath( + collectionFolder.FolderPath + ); + + // The administrator's edit, pushed to the repository... + File.WriteAllText( + settingsPath, + "99.0" + ); + tc.CopyRepoCollectionFilesFromLocal(collectionFolder.FolderPath); + FolderTeamCollection.CreateTeamCollectionLinkFile( + collectionFolder.FolderPath, + repoFolder.FolderPath + ); + + // ...while this teammate's own copy still says nothing about it, exactly as it + // would on the morning after the administrator made the change. + File.WriteAllText(settingsPath, ""); + Assert.That( + MinimumBloomVersionCheck.ReadMinimumBloomVersion(settingsPath), + Is.Empty, + "setup failed: the local file should not know about the requirement" + ); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld( + settingsPath, + out var minimumVersion + ), + Is.True, + "the repository's requirement should be honoured on the very first launch" + ); + Assert.That(minimumVersion, Is.EqualTo("99.0")); + } + } + } + + /// + /// A Team Collection whose repository copy cannot be read at all. Used to prove that + /// picking up the administrator's newly pushed requirement does not depend on reading back + /// the zip we have just written -- that zip can briefly refuse to open while Dropbox is + /// syncing it, and losing the value there is what would let the next save erase the + /// administrator's protection for the whole team. See BL-16690. + /// + private class TeamCollectionWhoseRepoReadFails : TestFolderTeamCollection + { + public TeamCollectionWhoseRepoReadFails( + ITeamCollectionManager tcManager, + string localCollectionFolder, + string repoFolderPath + ) + : base(tcManager, localCollectionFolder, repoFolderPath) { } + + protected override string GetRepoCollectionSettingsContent() + { + throw new IOException("pretending the repo zip is mid-sync"); + } + } + + [Test] + public void SyncLocalAndRepoCollectionFiles_RepoReadFails_StillRemembersWhatWePushed() + { + using (var collectionFolder = new TemporaryFolder("RepoReadFails_Collection")) + { + using (var repoFolder = new TemporaryFolder("RepoReadFails_Repo")) + { + var mockTcManager = new Mock(); + var settings = new CollectionSettings(); + mockTcManager.Setup(m => m.Settings).Returns(settings); + var tc = new TeamCollectionWhoseRepoReadFails( + mockTcManager.Object, + collectionFolder.FolderPath, + repoFolder.FolderPath + ); + Directory.CreateDirectory(Path.Combine(repoFolder.FolderPath, "Books")); + var settingsPath = CollectionSettings.GetSettingsFilePath( + collectionFolder.FolderPath + ); + + File.WriteAllText( + settingsPath, + "1.0" + ); + Assert.That( + settings.MinimumBloomVersion, + Is.Empty, + "setup failed: the running Bloom should not know about it yet" + ); + + tc.SyncLocalAndRepoCollectionFiles(false); + + Assert.That( + settings.MinimumBloomVersion, + Is.EqualTo("1.0"), + "a repo read failure must not lose the requirement we just pushed, or the next save deletes it for everyone" + ); + } + } + } + + /// + /// The administrator must be able to undo a mistake. Once a member is being refused, they + /// never open the collection, so the startup sync that would refresh their own copy of the + /// settings never runs -- which means a requirement lifted in the repository has to be + /// honoured from the repository, or that member is shut out on every launch for ever, with + /// no way back in from inside Bloom. See BL-16690. + /// + [Test] + public void IsThisBloomTooOld_RepoHasWithdrawnTheRequirement_LetsThemBackIn() + { + using (var collectionFolder = new TemporaryFolder("RepoWithdrewRequirement_Collection")) + { + using (var repoFolder = new TemporaryFolder("RepoWithdrewRequirement_Repo")) + { + var mockTcManager = new Mock(); + var settings = new CollectionSettings(); + mockTcManager.Setup(m => m.Settings).Returns(settings); + var tc = new TestFolderTeamCollection( + mockTcManager.Object, + collectionFolder.FolderPath, + repoFolder.FolderPath + ); + Directory.CreateDirectory(Path.Combine(repoFolder.FolderPath, "Books")); + var settingsPath = CollectionSettings.GetSettingsFilePath( + collectionFolder.FolderPath + ); + + // The administrator has thought better of it, so the repository asks for nothing... + File.WriteAllText(settingsPath, ""); + tc.CopyRepoCollectionFilesFromLocal(collectionFolder.FolderPath); + FolderTeamCollection.CreateTeamCollectionLinkFile( + collectionFolder.FolderPath, + repoFolder.FolderPath + ); + + // ...but this member's own copy still carries the requirement they were refused + // by, and always will, because being refused is what stops it being refreshed. + File.WriteAllText( + settingsPath, + "99.0" + ); + Assert.That( + MinimumBloomVersionCheck.ReadMinimumBloomVersion(settingsPath), + Is.EqualTo("99.0"), + "setup failed: the local file should still carry the old requirement" + ); + + Assert.That( + MinimumBloomVersionCheck.IsThisBloomTooOld(settingsPath, out _), + Is.False, + "the administrator lifting the requirement must let the member back in" + ); + } + } + } + + /// + /// The same workflow, and the same trap, for MinimumBloomVersion: the administrator adds it + /// to their own local settings file while Bloom runs. Their in-memory copy is still empty, + /// so the next Save() would rewrite the file without it and push that up, erasing the + /// requirement for the whole team. We deliberately only take the value here -- locking the + /// administrator out mid-sync is not this method's job. See BL-16690. + /// + [Test] + public void SyncLocalAndRepoCollectionFiles_LocalMinimumVersionPushedUp_UpdatesLiveSettings() + { + using (var collectionFolder = new TemporaryFolder("LocalMinVersionPushedUp_Collection")) + { + using (var repoFolder = new TemporaryFolder("LocalMinVersionPushedUp_Repo")) + { + var mockTcManager = new Mock(); + var settings = new CollectionSettings(); + mockTcManager.Setup(m => m.Settings).Returns(settings); + var tc = new TestFolderTeamCollection( + mockTcManager.Object, + collectionFolder.FolderPath, + repoFolder.FolderPath + ); + Directory.CreateDirectory(Path.Combine(repoFolder.FolderPath, "Books")); + var settingsPath = CollectionSettings.GetSettingsFilePath( + collectionFolder.FolderPath + ); + + // The administrator's hand-edit. 1.0 is old enough that nothing here will try to + // shut them out, which would want a dialog we cannot show from a unit test. + File.WriteAllText( + settingsPath, + "1.0" + ); + // ...while this running Bloom still has the value it loaded at startup. + Assert.That( + settings.MinimumBloomVersion, + Is.Empty, + "setup failed: the running Bloom should not know about the minimum version yet" + ); + + tc.SyncLocalAndRepoCollectionFiles(false); + + Assert.That( + settings.MinimumBloomVersion, + Is.EqualTo("1.0"), + "the machine that made the change should know about it too, or its next save will erase it" + ); + } + } + } + [Test] public void UpdateAllowCheckoutsFromRepo_NoRepoSettings_LeavesSettingAlone() { @@ -1475,5 +1706,71 @@ public void UpdateAllowCheckoutsFromRepo_NoRepoSettings_LeavesSettingAlone() } ); } + + /// + /// Picking up the repo's minimum version matters even when this Bloom is new enough to carry + /// on working. CollectionSettings.Save() rebuilds the file from memory, so if we were still + /// holding the empty value we loaded at startup, the next ordinary save would drop the + /// element and the Team Collection would push that up -- removing the administrator's + /// protection for everybody. See BL-16690. + /// + [Test] + public void HandleCollectionSettingsChange_RepoDeclaresAMinimumWeMeet_RemembersIt() + { + WithRepoSettingsFile( + "RememberMinimumVersion", + "1.0", + (tc, settings) => + { + // Sanity check: we must start out not knowing about it, or the test proves nothing. + Assert.That( + settings.MinimumBloomVersion, + Is.Empty, + "setup failed: should have started with no minimum version" + ); + // And this Bloom really must satisfy the 1.0 below. On a build where the + // version was never stamped in (0.0.x) it would not, and the code under test + // would try to lock the user out -- which from a unit test means a dialog and a + // hang rather than a failure. Better to say so plainly here. + Assert.That( + typeof(CollectionSettings).Assembly.GetName().Version, + Is.GreaterThanOrEqualTo(new Version(1, 0)), + "setup failed: this Bloom's assembly version is not stamped, so the test cannot tell a met minimum from an unmet one" + ); + + // 1.0 is old enough that this cannot try to lock anyone out (which would want a dialog). + var lockedOut = tc.HandleCollectionSettingsChange(new RepoChangeEventArgs()); + + Assert.That( + lockedOut, + Is.False, + "should not shut anyone out over a minimum this Bloom easily meets" + ); + Assert.That(settings.MinimumBloomVersion, Is.EqualTo("1.0")); + } + ); + } + + /// + /// Not being able to read the repo copy is quite different from the repo saying there is no + /// minimum, and only the second should clear what we are holding. Getting this wrong would + /// turn a transient read failure into the loss of the setting. See BL-16690. + /// + [Test] + public void HandleCollectionSettingsChange_NoRepoSettings_LeavesMinimumVersionAlone() + { + WithRepoSettingsFile( + "RememberMinimumVersionNoRepo", + null, + (tc, settings) => + { + settings.MinimumBloomVersion = "1.0"; + + tc.HandleCollectionSettingsChange(new RepoChangeEventArgs()); + + Assert.That(settings.MinimumBloomVersion, Is.EqualTo("1.0")); + } + ); + } } }