Harvester uses legacy theme (or fails if that is not allowed) (BL-13174)

src/BloomExe/Book/AppearanceSettings.cs

@@ -294,6 +294,14 @@ public string GetThemeAndSubstituteCss(Tuple<string, string>[] cssFilesToCheck)
         // if we don't know of a substitute theme and its associated customBookStyles2.css file.
         CssThemeName = "default";
         string substituteAppearance = null;
+        if (Program.RunningHarvesterMode)
+        {
+            // We'll preserve the current appearance of older books we are harvesting.
+            // This should only apply to temporary copies on their way to becoming artifacts,
+            // so it doesn't interfere with getting books migrated when they are to be edited.
+            CssThemeName = "legacy-5-6";
+            return null;
+        }
 
         foreach (var css in cssFilesToCheck.Where(css => !string.IsNullOrWhiteSpace(css.Item2)))
         {

src/BloomExe/Book/BookServer.cs

@@ -30,10 +30,7 @@ public BookServer(
             _tcManager = tcManager;
         }
 
-        public virtual Book GetBookFromBookInfo(
-            BookInfo bookInfo,
-            bool fullyUpdateBookFiles = false
-        )
+        public virtual Book GetBookFromBookInfo(BookInfo bookInfo)
         {
             //Review: Note that this isn't doing any caching yet... worried that caching will just eat up memory, but if anybody is holding onto these, then the memory won't be freed anyhow
             if (bookInfo is ErrorBookInfo)
@@ -45,7 +42,7 @@ public virtual Book GetBookFromBookInfo(
                 );
             }
 
-            var book = _bookFactory(bookInfo, _storageFactory(bookInfo, fullyUpdateBookFiles));
+            var book = _bookFactory(bookInfo, _storageFactory(bookInfo));
             return book;
         }
 

src/BloomExe/Book/BookStorage.cs

@@ -124,7 +124,7 @@ Tuple<string, string>[] GetCssFilesToCheckForAppearanceCompatibility(
 
     public class BookStorage : IBookStorage
     {
-        public delegate BookStorage Factory(BookInfo bookInfo, bool fullyUpdateBookFiles = false); //autofac uses this
+        public delegate BookStorage Factory(BookInfo bookInfo); //autofac uses this
 
         /// <summary>
         /// History of these numbers:
@@ -3684,6 +3684,12 @@ public string[] getMinimalCssFilesFromInstallThatDoNotChangeAtRuntime()
         /// </summary>
         public bool LinkToLocalCollectionStyles { get; set; }
 
+        public class CannotHarvestException : ApplicationException
+        {
+            public CannotHarvestException(string message)
+                : base(message) { }
+        }
+
         /// <summary>
         /// Migrate to the new appearance system if we haven't already tried to do so.
         /// </summary>
@@ -3696,6 +3702,13 @@ public void MigrateToLevel4UseAppearanceSystem()
             if (GetMaintenanceLevel() >= 4)
                 return;
 
+            if (Program.RunningHarvesterMode && !LegacyThemeCanBeUsed)
+            {
+                throw new CannotHarvestException(
+                    "This book cannot currently be harvested, since it is not migrated to the appearance system and cannot use legacy theme."
+                );
+            }
+
             var cssFiles = GetCssFilesToCheckForAppearanceCompatibility(true);
             var substituteCssPath = BookInfo.AppearanceSettings.GetThemeAndSubstituteCss(cssFiles);
             if (substituteCssPath != null)

src/BloomExe/CLI/CreateArtifactsCommand.cs

@@ -27,7 +27,8 @@ enum CreateArtifactsExitCode
         Success = 0,
         UnhandledException = 1,
         BookHtmlNotFound = 2,
-        EpubException = 4
+        EpubException = 4,
+        LegacyBookCannotHarvest = 8
     }
 
     class CreateArtifactsCommand
@@ -61,6 +62,12 @@ public static List<string> GetErrorsFromExitCode(int exitCode)
                 exitCode &= ~(int)CreateArtifactsExitCode.EpubException;
             }
 
+            if ((exitCode & (int)CreateArtifactsExitCode.LegacyBookCannotHarvest) != 0)
+            {
+                errors.Add(CreateArtifactsExitCode.LegacyBookCannotHarvest.ToString());
+                exitCode &= ~(int)CreateArtifactsExitCode.LegacyBookCannotHarvest;
+            }
+
             // Check if:
             // 1) Some error code was found
             // 2) No unknown flags remain
@@ -71,6 +78,11 @@ public static List<string> GetErrorsFromExitCode(int exitCode)
         }
 
         public static Task<int> Handle(CreateArtifactsParameters options)
+        {
+            return Task.FromResult((int)HandleInternal(options));
+        }
+
+        internal static CreateArtifactsExitCode HandleInternal(CreateArtifactsParameters options)
         {
             try
             {
@@ -103,15 +115,18 @@ public static Task<int> Handle(CreateArtifactsParameters options)
                         Bloom.Program.SetProjectContext(_projectContext);
 
                         // Make the .bloompub and /bloomdigital outputs
-                        var exitCode = CreateArtifacts(options);
-                        return Task.FromResult((int)exitCode);
+                        return CreateArtifacts(options);
                     }
                 }
             }
+            catch (BookStorage.CannotHarvestException)
+            {
+                return CreateArtifactsExitCode.LegacyBookCannotHarvest;
+            }
             catch (Exception ex)
             {
                 Console.WriteLine(ex);
-                return Task.FromResult((int)CreateArtifactsExitCode.UnhandledException);
+                return CreateArtifactsExitCode.UnhandledException;
             }
         }
 

src/BloomExe/CollectionTab/CollectionModel.cs

@@ -111,12 +111,17 @@ public string LanguageName
 
         private object _bookCollectionLock = new object(); // Locks creation of _bookCollections
 
-        public IReadOnlyList<BookCollection> GetBookCollections()
+        public IReadOnlyList<BookCollection> GetBookCollections(bool disposing = false)
         {
             lock (_bookCollectionLock)
             {
                 if (_bookCollections == null)
                 {
+                    if (disposing)
+                    {
+                        // We don't want to create new collections when we're disposing of the model.
+                        return new List<BookCollection>();
+                    }
                     _bookCollections = new List<BookCollection>(GetBookCollectionsOnce());
 
                     //we want the templates to be second (after the editable collection) regardless of alphabetical sorting
@@ -999,14 +1004,14 @@ public BookInfo GetBookInfoByFolderPath(string path)
             return collection.GetBookInfoByFolderPath(path);
         }
 
-        public Book.Book GetBookFromBookInfo(BookInfo bookInfo, bool fullyUpdateBookFiles = false)
+        public Book.Book GetBookFromBookInfo(BookInfo bookInfo)
         {
             // If we're looking for the current book it's important to return the actual book object,
             // because it could end up modified in ways that make our one out-of-date if we modify another
             // instance based on the same folder. For example, Rename could make our FolderPath wrong.
             if (bookInfo.FolderPath == _bookSelection.CurrentSelection?.FolderPath)
                 return _bookSelection.CurrentSelection;
-            return _bookServer.GetBookFromBookInfo(bookInfo, fullyUpdateBookFiles);
+            return _bookServer.GetBookFromBookInfo(bookInfo);
         }
 
         /// <summary>

src/BloomExe/CollectionTab/CollectionTabView.cs

@@ -206,7 +206,7 @@ private void OnBookCollectionCreated(object collection, EventArgs args)
                             // One day we may enhance it so that we switch tabs and show it,
                             // but there are states where that would be dangerous.
                             var newBook = new BookInfo(eventArgs.Path, false);
-                            var book = _model.GetBookFromBookInfo(newBook, true);
+                            var book = _model.GetBookFromBookInfo(newBook);
                             if (string.IsNullOrEmpty(book.Storage.ErrorMessagesHtml))
                             {
                                 // Happy path. Usually we can make a book object out of a downloaded book folder.
@@ -223,7 +223,7 @@ private void OnBookCollectionCreated(object collection, EventArgs args)
                                 t.Tick += (o, args1) =>
                                 {
                                     retries++;
-                                    book = _model.GetBookFromBookInfo(newBook, true);
+                                    book = _model.GetBookFromBookInfo(newBook);
                                     if (string.IsNullOrEmpty(book.Storage.ErrorMessagesHtml))
                                     {
                                         t.Stop();

src/BloomExe/Program.cs

@@ -1734,7 +1734,7 @@ internal static void SetUpErrorHandling()
             if (_errorHandlingHasBeenSetUp)
                 return;
 
-            if (!ApplicationUpdateSupport.IsDev)
+            if (!ApplicationUpdateSupport.IsDev && !Program.RunningUnitTests)
             {
                 try
                 {
@@ -1803,8 +1803,11 @@ internal static void SetUpErrorHandling()
             SIL.Reporting.ErrorReport.AddStandardProperties();
             // with squirrel, the file's dates only reflect when they were installed, so we override this version thing which
             // normally would include a bogus "Apparently Built On" date:
+            var versionNumber = Program.RunningUnitTests
+                ? "Current build" // for some reason VersionNumberString throws when running unit tests, so just use this.
+                : ErrorReport.VersionNumberString;
             ErrorReport.Properties["Version"] =
-                ErrorReport.VersionNumberString + " " + ApplicationUpdateSupport.ChannelName;
+                versionNumber + " " + ApplicationUpdateSupport.ChannelName;
             SIL.Reporting.ExceptionHandler.Init(new FatalExceptionHandler());
 
             ExceptionHandler.AddDelegate(

src/BloomExe/WebLibraryIntegration/BulkUploader.cs

@@ -368,7 +368,7 @@ ref ProjectContext context
                 context.TeamCollectionManager.CurrentCollectionEvenIfDisconnected
                     ?? new AlwaysEditSaveContext() as ISaveContext
             );
-            var book = server.GetBookFromBookInfo(bookInfo, fullyUpdateBookFiles: true);
+            var book = server.GetBookFromBookInfo(bookInfo);
             book.BringBookUpToDate(new NullProgress());
             uploadParams.Folder = book.FolderPath; // BringBookUpToDate can change the title and folder (see BL-10330)
             book.Storage.CleanupUnusedSupportFiles(isForPublish: false); // we are publishing, but this is the real folder not a copy, so play safe.

src/BloomExe/Workspace/WorkspaceView.cs

@@ -319,7 +319,7 @@ private void SelectBookAtStartup()
                     //var info = new BookInfo(selBookPath, inCurrentCollection, _tcManager.CurrentCollectionEvenIfDisconnected ?? new AlwaysEditSaveContext() as ISaveContext);
                     // Fully updating book files ensures that the proper branding files are found for
                     // previewing when the collection settings change but the book selection does not.
-                    var book = _bookServer.GetBookFromBookInfo(info, fullyUpdateBookFiles: true);
+                    var book = _bookServer.GetBookFromBookInfo(info);
                     _bookSelection.SelectBook(book);
                 }
             }

src/BloomExe/web/controllers/CollectionApi.cs

@@ -218,7 +218,7 @@ public void RegisterWithApiHandler(BloomApiHandler apiHandler)
                 kApiUrlPart + "selectAndEditBook",
                 request =>
                 {
-                    var book = GetBookObjectFromPost(request, true);
+                    var book = GetBookObjectFromPost(request);
                     if (book.FolderPath != _bookSelection?.CurrentSelection?.FolderPath)
                     {
                         _collectionModel.SelectBook(book);
@@ -781,18 +781,15 @@ private BookInfo GetBookInfoFromPost(ApiRequest request)
             return collection.GetBookInfos().FirstOrDefault(predicate);
         }
 
-        private Book.Book GetBookObjectFromPost(
-            ApiRequest request,
-            bool fullyUpdateBookFiles = false
-        )
+        private Book.Book GetBookObjectFromPost(ApiRequest request)
         {
             var info = GetBookInfoFromPost(request);
-            return _collectionModel.GetBookFromBookInfo(info, fullyUpdateBookFiles);
+            return _collectionModel.GetBookFromBookInfo(info);
         }
 
         private Book.Book GetUpdatedBookObjectFromBookInfo(BookInfo info)
         {
-            return _collectionModel.GetBookFromBookInfo(info, true);
+            return _collectionModel.GetBookFromBookInfo(info);
         }
     }
 }

src/BloomTests/Book/BookStarterTests.cs

@@ -58,7 +58,7 @@ public void Setup()
 
             _starter = new BookStarter(
                 _fileLocator,
-                (dir, fullyUpdateBookFiles) =>
+                (dir) =>
                     new BookStorage(dir, _fileLocator, new BookRenamedEvent(), collectionSettings),
                 _defaultCollectionSettings
             );
@@ -233,8 +233,7 @@ private BookServer CreateBookServer()
                         new BookRefreshEvent()
                     );
                 },
-                (path, fullyUpdateBookFiles) =>
-                    new BookStorage(path, _fileLocator, null, collectionSettings),
+                (path) => new BookStorage(path, _fileLocator, null, collectionSettings),
                 () => _starter,
                 null
             );
@@ -697,7 +696,7 @@ public void CreateBookOnDiskFromTemplate_HasEnglishTextArea_VernacularTextAreaAd
                     1
                 );
         }
-        
+
         [Test]
         public void CreateBookOnDiskFromTemplate_ConflictingCss_UsesLegacy()
         {

src/BloomTests/Book/BookTestsBase.cs

@@ -424,7 +424,7 @@ public BookServer CreateBookServer()
             );
             var starter = new BookStarter(
                 fileLocator,
-                (dir, fullyUpdateBookFiles) =>
+                (dir) =>
                     new BookStorage(dir, fileLocator, new BookRenamedEvent(), _collectionSettings),
                 _collectionSettings
             );
@@ -444,7 +444,7 @@ public BookServer CreateBookServer()
                     );
                 },
                 // storage factory
-                (info, fullyUpdateBookFiles) =>
+                (info) =>
                 {
                     var storage = new BookStorage(
                         info,

src/BloomTests/CLI/CreateArtifactsCommandTests.cs

@@ -1,7 +1,11 @@
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
+using Bloom.Book;
 using Bloom.CLI;
+using Bloom.Collection;
+using BloomTemp;
 using NUnit.Framework;
 
 namespace BloomTests.CLI
@@ -36,6 +40,15 @@ public void CreateArtifactsExitCode_GetErrorsFromExitCode_BookHtmlNotFound_Retur
             CollectionAssert.AreEquivalent(new string[] { "BookHtmlNotFound" }, errors);
         }
 
+        [Test]
+        public void CreateArtifactsExitCode_GetErrorsFromExitCode_LegacyBookCannotHarvest_Returns1Error()
+        {
+            int exitCode = 8;
+            var errors = CreateArtifactsCommand.GetErrorsFromExitCode(exitCode);
+
+            CollectionAssert.AreEquivalent(new string[] { "LegacyBookCannotHarvest" }, errors);
+        }
+
         [Test]
         public void CreateArtifactsExitCode_GetErrorsFromExitCode_EpubError_Returns1Error()
         {
@@ -82,5 +95,60 @@ public void CreateArtifactsExitCode_GetErrorsFromExitCode_BigNumber_AddsUnknown(
 
             Assert.That(errors.Contains("Unknown"), Is.True);
         }
+
+        [Test]
+        public void CreateArtifacts_LegacyBookWithInvalidXmatter_ReportsLegacyBookCannotHarvest()
+        {
+            using (
+                var testFolder = new TemporaryFolder(
+                    "CreateArtifacts_LegacyBookWithInvalidXmatter_ReportsLegacyBookCannotHarvest"
+                )
+            )
+            {
+                var collectionFolderPath = testFolder.Combine("collection");
+
+                var bookFolderPath = Path.Combine(collectionFolderPath, "book");
+                System.IO.Directory.CreateDirectory(bookFolderPath);
+                var collectionFilePath = Path.Combine(
+                    collectionFolderPath,
+                    "collection.bloomCollection"
+                );
+                var settings = new CollectionSettings(collectionFilePath);
+                settings.XMatterPackName = "ABC-Reader";
+                settings.Save();
+                var metaData = new BookMetaData();
+                metaData.WriteToFolder(bookFolderPath);
+                var bookPath = System.IO.Path.Combine(bookFolderPath, "book.htm");
+                System.IO.File.WriteAllText(
+                    bookPath,
+                    @"<html>
+					<body>
+						<div class='bloom-page'>
+							<div class='marginBox'>
+								<div class='bloom-translationGroup normal-style'>
+									<div class='bloom-editable normal-style bloom-content1 bloom-contentNational1 bloom-visibility-code-on' lang='en'>
+									</div>
+								</div>
+							</div>
+						</div>
+					</body>
+				</html>"
+                );
+                System.IO.File.WriteAllText(
+                    System.IO.Path.Combine(bookFolderPath, "book.xmatter"),
+                    "invalid"
+                );
+                var result = CreateArtifactsCommand.HandleInternal(
+                    new CreateArtifactsParameters()
+                    {
+                        BookPath = bookFolderPath,
+                        CollectionPath = collectionFilePath,
+                        BloomPubOutputPath = Path.Combine(testFolder.FolderPath, "output")
+                    }
+                );
+
+                Assert.That(result, Is.EqualTo(CreateArtifactsExitCode.LegacyBookCannotHarvest));
+            }
+        }
     }
 }