diff --git a/src/BloomExe/Book/AppearanceSettings.cs b/src/BloomExe/Book/AppearanceSettings.cs index 07a92c40fb47..ac59b5cd2ef7 100644 --- a/src/BloomExe/Book/AppearanceSettings.cs +++ b/src/BloomExe/Book/AppearanceSettings.cs @@ -294,6 +294,14 @@ public string GetThemeAndSubstituteCss(Tuple[] 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))) { diff --git a/src/BloomExe/Book/BookServer.cs b/src/BloomExe/Book/BookServer.cs index 2a839adbc688..d2face4c6bf2 100644 --- a/src/BloomExe/Book/BookServer.cs +++ b/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; } diff --git a/src/BloomExe/Book/BookStorage.cs b/src/BloomExe/Book/BookStorage.cs index c174934ee019..1723738f12d8 100644 --- a/src/BloomExe/Book/BookStorage.cs +++ b/src/BloomExe/Book/BookStorage.cs @@ -124,7 +124,7 @@ Tuple[] 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 /// /// History of these numbers: @@ -3684,6 +3684,12 @@ public string[] getMinimalCssFilesFromInstallThatDoNotChangeAtRuntime() /// public bool LinkToLocalCollectionStyles { get; set; } + public class CannotHarvestException : ApplicationException + { + public CannotHarvestException(string message) + : base(message) { } + } + /// /// Migrate to the new appearance system if we haven't already tried to do so. /// @@ -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) diff --git a/src/BloomExe/CLI/CreateArtifactsCommand.cs b/src/BloomExe/CLI/CreateArtifactsCommand.cs index 09d277fcb839..f6ac52388723 100644 --- a/src/BloomExe/CLI/CreateArtifactsCommand.cs +++ b/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 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 GetErrorsFromExitCode(int exitCode) } public static Task Handle(CreateArtifactsParameters options) + { + return Task.FromResult((int)HandleInternal(options)); + } + + internal static CreateArtifactsExitCode HandleInternal(CreateArtifactsParameters options) { try { @@ -103,15 +115,18 @@ public static Task 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; } } diff --git a/src/BloomExe/CollectionTab/CollectionModel.cs b/src/BloomExe/CollectionTab/CollectionModel.cs index a91e1d9f7f29..b0d2d442cd45 100644 --- a/src/BloomExe/CollectionTab/CollectionModel.cs +++ b/src/BloomExe/CollectionTab/CollectionModel.cs @@ -111,12 +111,17 @@ public string LanguageName private object _bookCollectionLock = new object(); // Locks creation of _bookCollections - public IReadOnlyList GetBookCollections() + public IReadOnlyList 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(); + } _bookCollections = new List(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); } /// diff --git a/src/BloomExe/CollectionTab/CollectionTabView.Designer.cs b/src/BloomExe/CollectionTab/CollectionTabView.Designer.cs index 443b0a83d1f9..3d498bf32498 100644 --- a/src/BloomExe/CollectionTab/CollectionTabView.Designer.cs +++ b/src/BloomExe/CollectionTab/CollectionTabView.Designer.cs @@ -23,7 +23,7 @@ protected override void Dispose(bool disposing) try { - var collections = _model?.GetBookCollections() ?? System.Linq.Enumerable.Empty(); + var collections = _model?.GetBookCollections(true) ?? System.Linq.Enumerable.Empty(); foreach (var collection in collections) collection?.StopWatchingDirectory(); } diff --git a/src/BloomExe/CollectionTab/CollectionTabView.cs b/src/BloomExe/CollectionTab/CollectionTabView.cs index ff36f7deca8d..f4870e61998a 100644 --- a/src/BloomExe/CollectionTab/CollectionTabView.cs +++ b/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(); diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index 53b21d87ee7b..c03b37a75efd 100644 --- a/src/BloomExe/Program.cs +++ b/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( diff --git a/src/BloomExe/WebLibraryIntegration/BulkUploader.cs b/src/BloomExe/WebLibraryIntegration/BulkUploader.cs index 30db5b4759a6..9e4384143645 100644 --- a/src/BloomExe/WebLibraryIntegration/BulkUploader.cs +++ b/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. diff --git a/src/BloomExe/Workspace/WorkspaceView.cs b/src/BloomExe/Workspace/WorkspaceView.cs index 9d52e82949db..c6132254db23 100644 --- a/src/BloomExe/Workspace/WorkspaceView.cs +++ b/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); } } diff --git a/src/BloomExe/web/controllers/CollectionApi.cs b/src/BloomExe/web/controllers/CollectionApi.cs index 46f0912205d9..01f9e9e51a31 100644 --- a/src/BloomExe/web/controllers/CollectionApi.cs +++ b/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); } } } diff --git a/src/BloomTests/Book/BookStarterTests.cs b/src/BloomTests/Book/BookStarterTests.cs index 6771ced41bec..0a6afd051dee 100644 --- a/src/BloomTests/Book/BookStarterTests.cs +++ b/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() { diff --git a/src/BloomTests/Book/BookTestsBase.cs b/src/BloomTests/Book/BookTestsBase.cs index 35621292a996..f341cbbf293e 100644 --- a/src/BloomTests/Book/BookTestsBase.cs +++ b/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, diff --git a/src/BloomTests/CLI/CreateArtifactsCommandTests.cs b/src/BloomTests/CLI/CreateArtifactsCommandTests.cs index 65185f41260e..6f24a0311222 100644 --- a/src/BloomTests/CLI/CreateArtifactsCommandTests.cs +++ b/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, + @" + +
+
+
+
+
+
+
+
+ + " + ); + 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)); + } + } } } diff --git a/src/BloomTests/Edit/ConfiguratorTest.cs b/src/BloomTests/Edit/ConfiguratorTest.cs index 36ea3f5097b3..15919b751e04 100644 --- a/src/BloomTests/Edit/ConfiguratorTest.cs +++ b/src/BloomTests/Edit/ConfiguratorTest.cs @@ -56,7 +56,7 @@ public void Setup() _starter = new BookStarter( _fileLocator, - (dir, fullyUpdateBookFiles) => + (dir) => new BookStorage(dir, _fileLocator, new BookRenamedEvent(), collectionSettings), collection ); diff --git a/src/BloomTests/TestDoubles/Book/FakeBookServer.cs b/src/BloomTests/TestDoubles/Book/FakeBookServer.cs index 3e3800abeb38..00b8fb8a429b 100644 --- a/src/BloomTests/TestDoubles/Book/FakeBookServer.cs +++ b/src/BloomTests/TestDoubles/Book/FakeBookServer.cs @@ -17,10 +17,7 @@ public FakeBookServer() /// A dumbed down implementation that is able to return a book with the BookInfo and set the book's FolderPath. /// /// - public override Bloom.Book.Book GetBookFromBookInfo( - BookInfo bookInfo, - bool fullyUpdateBookFiles = false - ) + public override Bloom.Book.Book GetBookFromBookInfo(BookInfo bookInfo) { var collectionSettings = new Bloom.Collection.CollectionSettings(); var fileLocator = new Bloom.BloomFileLocator(