diff --git a/.github/skills/add-e2e-test/SKILL.md b/.github/skills/add-e2e-test/SKILL.md index c6f713010c4f..892c74b8c54d 100644 --- a/.github/skills/add-e2e-test/SKILL.md +++ b/.github/skills/add-e2e-test/SKILL.md @@ -191,10 +191,13 @@ The worker-scoped fixture launches Bloom on a temp copy of that collection and y - `page` — Playwright's `page`, overridden to be Bloom's shell document (the top bar and the showing tab). Most tests need nothing else. -- `bloomApp` — `{ page, httpPort, cdpPort, bloomPid, collectionDir, restart }`. Build book - paths from `collectionDir`, never from `output/testing-inputs`. `restart(callback)` +- `bloomApp` — `{ page, httpPort, cdpPort, bloomPid, collectionDir, userSettingsDir, restart }`. + Build book paths from `collectionDir`, never from `output/testing-inputs`. `restart(callback)` stops Bloom, runs the callback, and starts it again, which is how a test changes what - Bloom reads only at startup, such as the collection's languages. + Bloom reads only at startup, such as the collection's languages. `userSettingsDir` is where + this Bloom keeps its user settings (`user.config`): a folder of its own in the temp folder, + empty at launch, so a test's Bloom starts from default settings and shares none with the + developer's Bloom or the previous run. `helpers/userSettings.ts` reads what it saved there. The fixture also watches for the "Bloom had a problem" dialog and fails the test with the exception it scrapes from behind the dialog's own "Learn More" link. See diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 5829e9483f9b..13d4c87684c7 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -313,6 +313,13 @@ jobs: # also work, but asking for it here says what the run needs instead of leaving # it to whatever the runner happens to do. BLOOM_AUTOMATION_MONITOR: headless + # The password of the Bloom Library test account (e2e-tester@example.org on + # dev.bloomlibrary.org), for the tests that sign in and upload for real. The + # account exists only for this; the email is a constant in the suite + # (src/BloomE2E/helpers/bloomLibraryAccount.ts). Locally the same variable is set + # at User scope; a developer without it has those tests skipped, while a run on CI + # without it fails, so a missing secret is noticed. + BLOOM_E2E_TESTER_EMAIL_BLORG_PASSWORD: ${{ secrets.BLOOM_E2E_TESTER_EMAIL_BLORG_PASSWORD }} run: pnpm test --reporter=list,junit,html # What explains an e2e failure: Playwright's HTML report, plus the trace and failure diff --git a/src/BloomE2E/AUTOMATION-DEBT.md b/src/BloomE2E/AUTOMATION-DEBT.md index 52814806ff7f..d36d5d587351 100644 --- a/src/BloomE2E/AUTOMATION-DEBT.md +++ b/src/BloomE2E/AUTOMATION-DEBT.md @@ -23,8 +23,7 @@ and ask its author. | --- | --- | --- | | #8299 | `BL-16799-page-change` | `editView/jumpToPage` refuses a jump it cannot do, and every page-changing helper waits for the Edit tab to settle. | | #8300 | `BL-16799-collection-languages` | The `e2e/setCollectionLanguages` hook, so no test composes `.bloomCollection` XML. | -| not yet open | `e2e-private-user-settings` | Every Bloom a test launches keeps its user settings in a folder of its own, named on the command line, so a run starts from defaults and its settings die with its temp folder. | -| not yet open | `e2e-real-library-login` | A test can sign in to dev.bloomlibrary.org for real, with a test account whose credentials the run supplies, so the upload cases can run to the end. Branches off the one above. | +| #8306 | `e2e-real-library-login` | A test can sign in to dev.bloomlibrary.org for real, with a test account whose credentials the run supplies, so the upload cases can run to the end. Branches off `e2e-private-user-settings`, which gave every Bloom a test launches a settings folder of its own and removed its entry from this file. | Three of these also add entries of their own, for the debt that is left after the fix. The stack replaces PR #8276, which did all of this at once. @@ -149,29 +148,19 @@ use. it, so it was left alone -- but it is the same shape, and worth remembering if a test ever finds that api reporting a stale subscription. -## The Bloom Library login cannot be done for real in a test - -Bloom's login state lives in machine-wide settings (`Settings.Default.WebUserId`), which an e2e -Bloom shares with the developer's own Bloom, and signing in goes out to an external browser with -real credentials. So a test can drive neither half of it: posting `account/logout` would sign the -developer out of their own Bloom, and `account/login` would sit waiting for a human. The e2e hook -`e2e/loginState` therefore makes Bloom *report* a login state without touching the real one, which -is enough for the gate the upload screen enforces (Upload is offered only to a signed-in user) but -covers neither the real sign-in and sign-out buttons nor anything that needs a real account — -which is every manual case that uploads for real (#204, #205, #211-#213, #215, #217, #218, #220), -so none of those can be automated either. Fix direction: a test account plus a per-instance login -store (a login the `--e2e` instance keeps to itself), so a run can sign in for real and upload to -dev.bloomlibrary.org without touching the developer's settings. The per-instance half of that is -the same fix "Every Bloom of one build shares one user.config" asks for, below; the test account -is the rest. Note that the pretense changes only what Bloom reports, so Bloom under `--e2e` now -refuses to upload at all rather than let an automated click publish under the developer's real -account. -(Found 2026-09-02 automating Test Case ID 606, `upload-required-items.spec.ts`.) - -being fixed on `e2e-real-library-login`, once `e2e-private-user-settings` (the user.config entry -below) has landed: with a settings folder of its own, a test's Bloom can be given a real test -account's login before it starts, and can sign out for real without touching anyone else's login. -The test account, and where its credentials live, are the rest of this branch. +## The Sign in button's trip through the system browser cannot be driven + +A test can now sign a Bloom in to dev.bloomlibrary.org for real (`signBloomIntoLibraryForReal`, +`helpers/bloomLibraryAccount.ts`): it signs the test account in the way the website does and posts +the result to `external/login`, the endpoint the website posts a browser login back to, and Bloom +keeps it in the run's own settings folder. What stays out of reach is the button itself: Sign in +opens the system browser on bloomlibrary.org's login page, and Playwright drives only Bloom's +WebView2, so the click, the browser round trip, and the hand-back to Bloom are never exercised. +Sign out is a plain button and could be, but no test does yet. Cost: the manual cases about the +sign-in and sign-out buttons stay manual; every case that merely needs to be signed in (the upload +cases) can run. Fix direction: none cheap. A Bloom-hosted login page under `--e2e` would test a +different flow from the one users have. +(Found 2026-09-04 automating Test Case ID 211, `bulk-upload-quick-test.spec.ts`.) ## Which front end the e2e suite tests depends on what else is running @@ -355,28 +344,6 @@ copy is a feature we want; if it is, put the page on the real clipboard, and giv fixture a way to run a second instance. (Found 2026-09-01 while automating Test Case ID 348.) -## Every Bloom of one build shares one user.config, so a run inherits another Bloom's settings - -Bloom keeps its user settings (UI language, page zoom, and the rest of `Settings.Default`) in -`%LOCALAPPDATA%\SIL\Bloom\\user.config`, one file per build version, and `--e2e` does -nothing to change that. So the Bloom a test launches starts from whatever the last Bloom of the -same version saved, and saves its own changes for the next one. The e2e lock keeps suites from -running at once, but a developer's own Bloom from a worktree of the same version is outside the -lock and shares the file all the same, and so does the previous run of any suite. - -Seen 2026-09-02 (Test Case ID 356, `format-gear-positioning.spec.ts`): two runs found every -factory template named in Turkish, then in French, and failed in `makeBookFromTemplate`, which -matches the English title; a Bloom nobody in the suite had started was running at the time, and -the file said `en` again a moment later. The same test has to restore the zoom it changes, because -that setting is shared too. Fix direction: under `--e2e`, point the settings provider at a -per-instance folder (a sibling of the temp collection would do), so a test's Bloom starts from -defaults and its changes die with it. - -being fixed on `e2e-private-user-settings`: a command-line argument names the folder Bloom keeps -its user settings in, and the launch fixture gives every Bloom it starts a folder inside the run's -temp folder, so the settings start from defaults, or from whatever the test puts there first, and -are deleted with the rest of the run. - ## No way to run the suite at a chosen monitor resolution and scale factor Every run takes the resolution and the scale factor of whatever monitor it lands on, so a @@ -408,8 +375,8 @@ Fix direction, cheapest first, none of it tried yet: driver is told to offer. Setting that monitor's *scale factor* is the harder half: Windows exposes per-monitor scale only through display-config calls Microsoft does not document. Worth an afternoon of investigation before committing to it. -- **A virtual machine or Windows Sandbox** at a chosen resolution and scale. Heaviest, but it - is the only one that also isolates the shared `user.config` described above. +- **A virtual machine or Windows Sandbox** at a chosen resolution and scale. Heaviest, and the + only one that would also isolate everything else on the machine a run could inherit. Whatever the mechanism, the suite needs the same thing from it: a way to say "run these tests at 1920x1080 at 150%" and have the run either honour it or refuse, rather than silently using diff --git a/src/BloomE2E/README.md b/src/BloomE2E/README.md index 4786fa36df8a..32717e23a789 100644 --- a/src/BloomE2E/README.md +++ b/src/BloomE2E/README.md @@ -69,8 +69,20 @@ and whatever tab is showing. Most tests need nothing else. | `cdpPort` | The port the embedded WebView2 answers CDP on. | | `bloomPid` | The process id of the Bloom serving this collection. | | `collectionDir` | The temp copy of the collection. Build book paths from this, never from `output/testing-inputs`. | +| `userSettingsDir` | The folder this Bloom keeps its user settings in (its `user.config`), beside the collection in the temp folder. | | `restart` | Stop Bloom, run an optional callback, start it again on the same collection, and return the new page. | +Every Bloom the fixture launches keeps its user settings, the contents of `user.config` (UI +language, page zoom, the Bloom Library login, and the rest of `Settings.Default`), in +`userSettingsDir`, passed to Bloom as `--user-settings-folder`. Every Bloom of one build otherwise +shares one `user.config` in `%LOCALAPPDATA%\SIL\Bloom\`, so a run would start from +whatever the developer's Bloom, or the previous run, saved last, and leave its own changes behind +for them. Instead the folder starts empty, so Bloom starts from default settings, and it is deleted +with the rest of the run. A restart keeps it, so what one launch saved the next one reads, as on a +real machine. `helpers/userSettings.ts` reads what Bloom has saved there. The fixture checks that +the Bloom it started really is using the folder, so a stale `Bloom.exe` fails the launch rather +than quietly sharing settings. + `restart(betweenStopAndStart)` is how a test changes something Bloom only reads at startup. The collection's languages are the case that needed it: the collection Settings dialog is a WinForms surface CDP cannot reach, and `collectionSettings/changeLanguage` only answers while that dialog @@ -112,6 +124,8 @@ real bug in the code under test; read the message and fix it rather than working - `helpers/addPageDialog.ts` — open, read, scroll and close the real Add Page dialog, and add a page through it. Tests that only need a page in their book call `addPage` instead. - `helpers/files.ts` — `fingerprintFolder`, `isInsideFolder`, for checks against the disk. +- `helpers/userSettings.ts` — `getUserSettingsFolder`, `readSavedUserSetting`: where the Bloom + under test keeps its user settings, and what it has saved there. - `helpers/api.ts` — `apiGet`, `apiPost`, `apiGetJson`. These run `fetch` inside the page with a relative URL, which is not a style choice: Bloom's server rejects a `127.0.0.1` Host header, and the CDP endpoint does not answer on `localhost`. The file explains it. diff --git a/src/BloomE2E/fixtures/bloomTest.ts b/src/BloomE2E/fixtures/bloomTest.ts index 2404dcec276d..e67e030a1ad7 100644 --- a/src/BloomE2E/fixtures/bloomTest.ts +++ b/src/BloomE2E/fixtures/bloomTest.ts @@ -44,6 +44,12 @@ export interface IBloomApp { bloomPid: number; /** The collection folder Bloom has open: a temp folder, never the inputs repository itself. */ collectionDir: string; + /** + * The folder this Bloom keeps its user settings in (its user.config), beside the collection in + * the temp folder, so nothing it saves reaches the developer's own Bloom or the next run. It + * starts empty, and a restart keeps it. See helpers/userSettings.ts to read what is in it. + */ + userSettingsDir: string; /** * Quit Bloom and start it again on the same collection folder, and return the new shell page. * @@ -256,6 +262,7 @@ export const test = base.extend({ cdpPort: launched.cdpPort, bloomPid: launched.bloomPid, collectionDir: launched.collectionDir, + userSettingsDir: launched.userSettingsDir, restart: async (betweenStopAndStart) => { // Close the old CDP connection first: it holds a socket into the process // that is about to be killed. diff --git a/src/BloomE2E/fixtures/launchBloom.ts b/src/BloomE2E/fixtures/launchBloom.ts index de82593169d4..25e247769342 100644 --- a/src/BloomE2E/fixtures/launchBloom.ts +++ b/src/BloomE2E/fixtures/launchBloom.ts @@ -12,6 +12,12 @@ // 3. Discovery matches on the OPEN COLLECTION FOLDER, not on a port. Bloom takes the next free // port block, and a developer's own Bloom may already hold 8089, so the folder is the only // reliable way to tell our instance from theirs. +// 4. Every Bloom we launch keeps its user settings (user.config: UI language, page zoom, the Bloom +// Library login, and the rest of Settings.Default) in a folder of its own inside the temp +// folder, passed as --user-settings-folder. Every Bloom of one build otherwise shares one +// user.config, so a run would start from whatever the developer's Bloom, or the previous run, +// saved last, and leave its own changes behind for them. This way it starts from defaults, or +// from whatever a test puts in the folder first, and its settings die with the temp folder. // // Nothing here knows about Playwright; fixtures/bloomTest.ts adds the CDP attachment on top. @@ -31,6 +37,12 @@ export interface ILaunchedBloom { bloomPid: number; /** The temp copy of the collection folder that this Bloom has open. */ collectionDir: string; + /** + * The folder this Bloom keeps its user settings in (its user.config), a sibling of the + * collection in the temp folder. It starts empty, so Bloom starts from default settings; a + * restart keeps it, so what one launch saved the next one reads, as on a real machine. + */ + userSettingsDir: string; /** Kill the process tree, confirm the HTTP port went dark, and delete the temp copy. */ stop: () => Promise; /** @@ -66,6 +78,12 @@ export interface ICollectionSpec { * feature. */ subscriptionCode?: string; + /** + * The Bloom Library bookshelf every book of the collection is uploaded to, by its url key, e.g. + * "test-bookshelf-1" (see kTestBookshelves). Bloom honours it only under an enterprise + * subscription whose bookshelves include it, which is what the Settings dialog offers a person. + */ + bookshelf?: string; } /** Options for launchBloom. Give exactly one of collectionName and collectionSpec. */ @@ -376,11 +394,16 @@ const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); * window IS visible. */ function environmentForBloom(): NodeJS.ProcessEnv { + // Every Bloom a test launches talks to the sandbox, dev.bloomlibrary.org, never to + // bloomlibrary.org: a test signs in there with a test account and uploads for real. A Debug + // Bloom uses the sandbox anyway; a Release Bloom, which is what CI builds, uses bloomlibrary.org + // unless this variable says otherwise (see BookUpload.UseSandboxWithoutUserChoice). + const env: NodeJS.ProcessEnv = { ...process.env, BloomSandbox: "true" }; const asked = process.env.BLOOM_AUTOMATION_MONITOR?.trim().toLowerCase(); if (process.env.PWDEBUG && (asked === "headless" || asked === "0")) { - return { ...process.env, BLOOM_AUTOMATION_MONITOR: "" }; + env.BLOOM_AUTOMATION_MONITOR = ""; } - return process.env; + return env; } /** @@ -416,6 +439,8 @@ interface IInstanceInfo { editableCollectionFolder?: string; processId?: number; cdpPort?: number; + /** Where this Bloom keeps its user settings; absent from a Bloom built before it reported this. */ + userSettingsFolder?: string; } /** @@ -483,12 +508,20 @@ export function writeNewCollection( fs.mkdirSync(collectionDir, { recursive: true }); fs.writeFileSync( Path.join(collectionDir, `${spec.name}.bloomCollection`), - makeCollectionXml(spec.languages, "Factory", spec.subscriptionCode), + makeCollectionXml(spec.languages, "Factory", spec), "utf8", ); return collectionDir; } +/** The settings makeCollectionXml writes beyond the languages and the front/back matter pack. */ +export interface ICollectionXmlExtras { + /** See ICollectionSpec.subscriptionCode. */ + subscriptionCode?: string; + /** See ICollectionSpec.bookshelf. */ + bookshelf?: string; +} + /** * The XML of a .bloomCollection with these languages and this front/back matter pack. Exported * because a test changes collection settings by rewriting this file between a stop and a start @@ -498,14 +531,16 @@ export function writeNewCollection( * pack the Settings dialog calls Paper Saver), "Traditional", "SuperPaperSaver", "Device", * "SIL-PNG". The default is Factory, which is what the collections here have always had. * - * `subscriptionCode` is written only when given. It is the collection's subscription, and so its - * tier: Bloom parses the tier out of the code as it opens the collection. See - * kEnterpriseSubscriptionCode in helpers/collectionSettings.ts. + * `extras.subscriptionCode` is written only when given. It is the collection's subscription, and + * so its tier: Bloom parses the tier out of the code as it opens the collection (since Bloom 6.1 + * from SubscriptionCode alone; BrandingProjectName is written for older Blooms). See + * kEnterpriseSubscriptionCode in helpers/collectionSettings.ts. `extras.bookshelf`, also written + * only when given, becomes a "bookshelf:" tag in DefaultBookTags, which is how Bloom keeps it. */ export function makeCollectionXml( languages: string[], xmatterPack = "Factory", - subscriptionCode?: string, + extras: ICollectionXmlExtras = {}, ): string { // Bloom treats Language2 as "same as Language1" when a collection names only one language, // which is what its own new-collection code writes. @@ -526,8 +561,11 @@ export function makeCollectionXml( languageElements + `\n ${xmatterPack}\n` + ` Default\n` + - (subscriptionCode - ? ` ${subscriptionCode}\n` + (extras.subscriptionCode + ? ` ${extras.subscriptionCode}\n` + : "") + + (extras.bookshelf + ? ` bookshelf:${extras.bookshelf}\n` : "") + ` True\n` + ` Decimal\n` + @@ -606,6 +644,7 @@ function isPid(pid: number | undefined): pid is number { */ async function startBloomOn( collectionDir: string, + userSettingsDir: string, readyTimeoutMs: number, experimentalFeatures?: string[], ): Promise { @@ -637,6 +676,8 @@ async function startBloomOn( // tree rather than a stale output/browser (see getViteDevPort). const vitePort = getViteDevPort(); if (vitePort) args.push("--vite-port", vitePort); + // --user-settings-folder: keep this Bloom's user settings to itself (point 4 at the top). + args.push("--user-settings-folder", userSettingsDir); // --experimental-features: turn these on for this Bloom alone, without touching the saved // setting the developer's own Bloom shares (see ILaunchBloomOptions.experimentalFeatures). if (experimentalFeatures?.length) @@ -706,6 +747,21 @@ async function startBloomOn( ); } + // A Bloom that is not keeping its settings where we said would share them with the developer's + // own Bloom, which is the very thing the folder prevents; a Bloom.exe built before the argument + // existed reports no folder at all. Either way, no test can be trusted, so stop here. + if ( + !found.info.userSettingsFolder || + !samePath(found.info.userSettingsFolder, userSettingsDir) + ) { + killProcessTree([bloomProcess.pid, found.info.processId].filter(isPid)); + throw new Error( + `Bloom was asked to keep its user settings in ${userSettingsDir} but reports ` + + `${found.info.userSettingsFolder ?? "no user settings folder"}. ` + + `Is ${exe} built from current sources?`, + ); + } + return { httpPort: found.httpPort, cdpPort: found.info.cdpPort, @@ -756,10 +812,13 @@ export async function launchBloom( ); let collectionDir: string; + // Empty, so this Bloom starts from default settings (point 4 at the top). Deleted with tempRoot. + const userSettingsDir = Path.join(tempRoot, "user-settings"); try { collectionDir = options.collectionSpec ? writeNewCollection(tempRoot, options.collectionSpec) : copyPreparedCollection(tempRoot, options.collectionName!); + fs.mkdirSync(userSettingsDir); } catch (error) { fs.rmSync(tempRoot, { recursive: true, force: true }); throw error; @@ -782,6 +841,7 @@ export async function launchBloom( try { running = await startBloomOn( collectionDir, + userSettingsDir, readyTimeoutMs, options.experimentalFeatures, ); @@ -796,6 +856,7 @@ export async function launchBloom( cdpPort: running.cdpPort, bloomPid: running.servingPid, collectionDir, + userSettingsDir, restart: async (betweenStopAndStart) => { await killAndWaitForPortToGoDark(running!); @@ -805,6 +866,7 @@ export async function launchBloom( if (betweenStopAndStart) await betweenStopAndStart(); running = await startBloomOn( collectionDir, + userSettingsDir, readyTimeoutMs, options.experimentalFeatures, ); diff --git a/src/BloomE2E/helpers/bloomLibraryAccount.ts b/src/BloomE2E/helpers/bloomLibraryAccount.ts new file mode 100644 index 000000000000..460257172330 --- /dev/null +++ b/src/BloomE2E/helpers/bloomLibraryAccount.ts @@ -0,0 +1,176 @@ +// Sign a test's Bloom in to dev.bloomlibrary.org for real, as a dedicated test account. +// +// Why this is possible now: every Bloom a test launches keeps its user settings, the Bloom Library +// login included, in a folder of its own (bloomApp.userSettingsDir; see fixtures/launchBloom.ts). +// So a test can sign a real account in without touching the developer's own Bloom, and Bloom's +// bulk-upload child process, which reads the login back from that folder, uploads as the test +// account rather than as the developer. (Before that, the login lived in settings shared with the +// developer's Bloom, so no test could sign in for real, and the upload cases stayed manual.) +// +// The sign-in goes straight to the endpoint the website posts a browser login back to, not through +// Bloom's Sign in button: that button opens the system browser, which no test can drive. See +// AUTOMATION-DEBT.md, "The Sign in button's trip through the system browser cannot be driven". +// +// The account and its password: the email is a constant here, because it is not secret and a reader +// should see which account a test signs in as (an assertion can check it too — the Sign out button +// carries it). Only the password is kept out of the source, in the BLOOM_E2E_TESTER_EMAIL_BLORG_PASSWORD +// environment variable (User scope on a developer's machine, a repository secret in CI). A run +// without it skips the tests that need it locally, and fails in CI, so a missing secret is noticed. +// +// What this file does NOT do: reach the server to read or delete books. That is bloomLibraryServer.ts, +// which owns the sandbox back ends. This file borrows that module's login type and its parse-server +// address, so there is one description of the sandbox, not two. + +import { test, type Page } from "@playwright/test"; +import { apiPost } from "./api"; +import { + DEV_PARSE_APPLICATION_ID, + DEV_PARSE_SERVER_URL, + type IBloomLibraryLogin, +} from "./bloomLibraryServer"; + +/** + * The test account every real-login test signs in as. Not secret: it is here so a reader can see + * which account a test uses, and an assertion can check it. Only the password is kept out of the + * source (see PASSWORD_ENV_VAR). The account exists only for these tests, on dev.bloomlibrary.org. + */ +export const TEST_ACCOUNT_EMAIL = "e2e-tester@example.org"; + +/** The environment variable that carries the test account's dev.bloomlibrary.org password. */ +export const PASSWORD_ENV_VAR = "BLOOM_E2E_TESTER_EMAIL_BLORG_PASSWORD"; + +// dev.bloomlibrary.org's Firebase auth, which the website uses to sign a user in before exchanging +// the result for a parse-server session. The key ships in the website's JS bundle, so it is not a +// secret. The Firebase project is shared with production; it is the parse server (dev, from +// bloomLibraryServer.ts) that makes this a sandbox login. +const FIREBASE_API_KEY = "AIzaSyACJ7fi7_Rg_bFgTIacZef6OQckr6QKoTY"; + +/** + * Whether the test account's password is available. When it is not, a real-login test cannot run. + * See skipIfNoLibraryPassword, which is how a test acts on this. + */ +export function isLibraryPasswordConfigured(): boolean { + return !!process.env[PASSWORD_ENV_VAR]; +} + +/** + * Skip the current test when the test account's password is not set — but only off CI. On CI a + * missing password is a broken secret, not a developer who has not set one up, so there the test is + * left to fail rather than quietly skipped. Call at the top of a test that signs in for real. + */ +export function skipIfNoLibraryPassword(): void { + if (isLibraryPasswordConfigured()) return; + if (process.env.CI) + throw new Error( + `${PASSWORD_ENV_VAR} is not set, so this run cannot sign in to dev.bloomlibrary.org as ` + + `${TEST_ACCOUNT_EMAIL}. On CI this is a missing repository secret; add it.`, + ); + test.skip( + true, + `${PASSWORD_ENV_VAR} is not set. Set it (User environment scope) from the team password ` + + `manager to run the tests that sign in to dev.bloomlibrary.org as ${TEST_ACCOUNT_EMAIL}.`, + ); +} + +/** + * Sign the test account in to dev.bloomlibrary.org and return the login, doing exactly what the + * website's login-for-editor page does: authenticate to Firebase with the email and password, then + * exchange the Firebase token for a parse-server session (the bloomLink cloud function links or + * creates the parse user, then a users POST logs in). Throws with a diagnostic naming the step that + * failed. The account's email must be verified, or the parse server refuses it. + */ +export async function getDevBloomLibraryLogin(): Promise { + const password = process.env[PASSWORD_ENV_VAR]; + if (!password) + throw new Error( + `${PASSWORD_ENV_VAR} is not set; call skipIfNoLibraryPassword first.`, + ); + + const firebase = await postJson( + `https://identitytoolkit.googleapis.com/v1/accounts:signInWithPassword?key=${FIREBASE_API_KEY}`, + { email: TEST_ACCOUNT_EMAIL, password, returnSecureToken: true }, + "Firebase sign-in", + ); + + // bloomLink links the Firebase user to a parse user (creating one if needed) before the users + // POST logs in; the website calls it first for the same reason. + await postJson( + `${DEV_PARSE_SERVER_URL}/functions/bloomLink`, + { token: firebase.idToken, id: TEST_ACCOUNT_EMAIL }, + "parse-server bloomLink", + { "X-Parse-Application-Id": DEV_PARSE_APPLICATION_ID }, + ); + const user = await postJson( + `${DEV_PARSE_SERVER_URL}/users`, + { + authData: { + bloom: { token: firebase.idToken, id: TEST_ACCOUNT_EMAIL }, + }, + username: TEST_ACCOUNT_EMAIL, + email: TEST_ACCOUNT_EMAIL, + }, + "parse-server login", + { "X-Parse-Application-Id": DEV_PARSE_APPLICATION_ID }, + ); + if (!user.sessionToken || !user.objectId) + throw new Error( + `parse-server login for ${TEST_ACCOUNT_EMAIL} returned no session token or user id: ` + + JSON.stringify(user), + ); + return { + email: TEST_ACCOUNT_EMAIL, + userId: user.objectId, + sessionToken: user.sessionToken, + }; +} + +/** + * Sign the running Bloom in to Bloom Library as the test account, for real, by posting the login to + * external/login — the same endpoint the website posts to after a browser login. Bloom saves it in + * its own user-settings folder, so its bulk-upload child process signs in as this account too, and + * the top bar shows the account signed in. Returns the login, which the caller keeps so it can + * delete afterwards what it uploads (see bloomLibraryServer.ts). Uploads go only to the sandbox. + */ +export async function signBloomIntoLibraryForReal( + page: Page, +): Promise { + const login = await getDevBloomLibraryLogin(); + await apiPost( + page, + "external/login", + JSON.stringify({ + sessionToken: login.sessionToken, + email: login.email, + userId: login.userId, + }), + "application/json", + ); + return login; +} + +/** POST JSON and parse the JSON reply, throwing a diagnostic that names the step on any failure. */ +async function postJson( + url: string, + body: unknown, + what: string, + extraHeaders: Record = {}, +): Promise { + let response: Response; + try { + response = await fetch(url, { + method: "POST", + headers: { "Content-Type": "application/json", ...extraHeaders }, + body: JSON.stringify(body), + }); + } catch (error) { + throw new Error(`${what} could not reach ${url}: ${error}`); + } + const text = await response.text(); + if (!response.ok) + throw new Error(`${what} failed (${response.status}): ${text}`); + try { + return JSON.parse(text); + } catch { + throw new Error(`${what} did not return JSON: ${text}`); + } +} diff --git a/src/BloomE2E/helpers/bloomLibraryServer.ts b/src/BloomE2E/helpers/bloomLibraryServer.ts new file mode 100644 index 000000000000..98489084d2ae --- /dev/null +++ b/src/BloomE2E/helpers/bloomLibraryServer.ts @@ -0,0 +1,212 @@ +// Talk to the Bloom Library SANDBOX, dev.bloomlibrary.org, from a test: which books are there, +// and deleting the ones a test uploaded. Nothing here talks to Bloom, and nothing here can reach +// bloomlibrary.org itself: every address below is the sandbox's. +// +// The sandbox has two back ends, the same two the website talks to (see BloomLibrary2's +// src/connection): a parse-server, which holds the book records and answers anonymous reads with +// the application id below, and the Bloom Library API, which takes the writes and wants the +// signed-in user's parse session token. Both ids here are public; the website ships them. +// +// The files of an uploaded book sit in the sandbox's public S3 bucket, at the record's baseUrl; a +// test reads the uploaded HTML from there to see what Bloom actually sent. + +import { xmatterPackInBookHtml } from "./bookHtml"; + +/** The sandbox parse-server, where the book records live. */ +export const DEV_PARSE_SERVER_URL = "https://dev-server.bloomlibrary.org/parse"; +export const DEV_PARSE_APPLICATION_ID = + "yrXftBF6mbAuVu3fO6LnhCJiHxZPIdE7gl1DUVGR"; + +/** The Bloom Library API; "env=dev" on every request keeps it to the sandbox. */ +const BLOOM_LIBRARY_API_URL = "https://api.bloomlibrary.org/v1"; + +/** A signed-in Bloom Library user, as the parse-server describes one. */ +export interface IBloomLibraryLogin { + email: string; + /** The parse-server's id for the user; Bloom keeps it as LastLoginUserId. */ + userId: string; + /** The parse session token; Bloom keeps it as LastLoginSessionToken and sends it with uploads. */ + sessionToken: string; +} + +/** One book record on the sandbox. Only the fields a test reads. */ +export interface IBookOnServer { + /** The record's own id, which the API's delete route takes. */ + objectId: string; + /** The book's id inside Bloom (meta.json's bookInstanceId), which uploads are matched on. */ + bookInstanceId: string; + title: string; + /** Bloom's tags, e.g. "bookshelf:test-bookshelf-1". */ + tags: string[]; + /** The bookshelves the book sits on, by url key: the "bookshelf:" tags without their prefix. */ + bookshelves: string[]; + /** The email of the account that uploaded it. */ + uploaderEmail: string | undefined; + /** + * Where the book's files are on S3, as Bloom wrote it (BloomS3Client.GetBaseUrl): URL-encoded + * with the slashes as %2f and spaces as +. Ends with the book folder's name. + */ + baseUrl: string; +} + +const parseHeaders = { + "X-Parse-Application-Id": DEV_PARSE_APPLICATION_ID, + "Content-Type": "application/json", +}; + +/** + * Query the sandbox's book records with this parse "where" clause, as the signed-in user when a + * login is given (the website sends the session token for a user's own records, so a test does + * too). Returns the records in no particular order. + */ +async function queryBooksOnDevServer( + where: object, + describe: string, + login?: IBloomLibraryLogin, +): Promise { + const url = + `${DEV_PARSE_SERVER_URL}/classes/books?where=${encodeURIComponent(JSON.stringify(where))}` + + `&include=uploader&keys=title,bookInstanceId,tags,uploader,baseUrl&limit=1000`; + const headers = login + ? { ...parseHeaders, "X-Parse-Session-Token": login.sessionToken } + : parseHeaders; + const response = await fetch(url, { headers }); + if (!response.ok) + throw new Error( + `The sandbox parse-server answered ${response.status} to a query for ${describe}: ${await response.text()}`, + ); + const body = (await response.json()) as { + results: { + objectId: string; + bookInstanceId: string; + title: string; + tags?: string[]; + uploader?: { email?: string }; + baseUrl: string; + }[]; + }; + const bookshelfPrefix = "bookshelf:"; + return body.results.map((r) => ({ + objectId: r.objectId, + bookInstanceId: r.bookInstanceId, + title: r.title, + tags: r.tags ?? [], + bookshelves: (r.tags ?? []) + .filter((tag) => tag.startsWith(bookshelfPrefix)) + .map((tag) => tag.substring(bookshelfPrefix.length)), + uploaderEmail: r.uploader?.email, + baseUrl: r.baseUrl, + })); +} + +/** + * The book records on the sandbox for these book instance ids, in no particular order. A book that + * is not there is simply absent from the result, so a test compares lengths. + */ +export async function findBooksOnDevServer( + bookInstanceIds: string[], +): Promise { + return queryBooksOnDevServer( + { bookInstanceId: { $in: bookInstanceIds } }, + "books", + ); +} + +/** + * Every book the signed-in account has on the sandbox, whatever a test named them. Cleanup uses + * this so a run can delete not only the books it just uploaded but any a crashed earlier run left + * behind, which is safe because the account exists only for these tests. + */ +export async function findBooksUploadedBy( + login: IBloomLibraryLogin, +): Promise { + return queryBooksOnDevServer( + { + uploader: { + __type: "Pointer", + className: "_User", + objectId: login.userId, + }, + }, + "the account's books", + login, + ); +} + +/** + * The HTML of a book as it was uploaded, read from the sandbox's S3 bucket. Bloom uploads the book + * folder under the record's baseUrl and names the .htm after the folder, so this reads + * `.htm`. The bucket is public-read; that is how the website shows books. + */ +export async function fetchUploadedBookHtml( + book: IBookOnServer, +): Promise { + // Bloom writes baseUrl with HttpUtility.UrlEncode, which puts spaces as "+", so undo that + // before the general decoding turns the %2f slashes (and any %2b plus) back into themselves. + const folderPath = decodeURIComponent(book.baseUrl.replace(/\+/g, " ")); + const folderName = folderPath.split("/").filter(Boolean).pop()!; + // The URL constructor re-encodes the spaces and the rest of the path for the request. + const url = + new URL(folderPath).href + encodeURIComponent(folderName) + ".htm"; + const response = await fetch(url); + if (!response.ok) + throw new Error( + `S3 answered ${response.status} for the uploaded HTML of "${book.title}" at ${url}`, + ); + return response.text(); +} + +/** + * The front/back matter pack the uploaded copy of this book carries (see xmatterPackInBookHtml): + * how a test sees that an upload sent the book with the collection's current pack. + */ +export async function getXmatterPackOfBookOnServer( + book: IBookOnServer, +): Promise { + return xmatterPackInBookHtml(await fetchUploadedBookHtml(book)); +} + +/** + * Delete every book the signed-in account has on the sandbox, and return how many. The safety net a + * real-upload test calls in cleanup, so nothing it uploaded (or a crashed run uploaded before it) + * is left on dev.bloomlibrary.org. + */ +export async function deleteAllBooksUploadedBy( + login: IBloomLibraryLogin, +): Promise { + const books = await findBooksUploadedBy(login); + for (const book of books) await deleteBookFromDevServer(book, login); + return books.length; +} + +/** + * Delete one book from the sandbox, the way the website's Delete button does (BloomLibrary2's + * deleteBook): through the Bloom Library API, as the signed-in user, who must be its uploader. + * Returns once the parse-server no longer lists the record. + */ +export async function deleteBookFromDevServer( + book: IBookOnServer, + login: IBloomLibraryLogin, +): Promise { + const response = await fetch( + `${BLOOM_LIBRARY_API_URL}/books/${book.objectId}?env=dev`, + { + method: "DELETE", + headers: { "Authentication-Token": login.sessionToken }, + }, + ); + if (!response.ok) + throw new Error( + `The Bloom Library API answered ${response.status} to deleting book ${book.objectId} ` + + `("${book.title}") from the sandbox: ${await response.text()}`, + ); + const deadline = Date.now() + 30000; + while (Date.now() < deadline) { + const left = await findBooksOnDevServer([book.bookInstanceId]); + if (!left.some((b) => b.objectId === book.objectId)) return; + await new Promise((resolve) => setTimeout(resolve, 1000)); + } + throw new Error( + `Book ${book.objectId} ("${book.title}") is still on the sandbox 30 seconds after the API accepted its deletion.`, + ); +} diff --git a/src/BloomE2E/helpers/bookHtml.ts b/src/BloomE2E/helpers/bookHtml.ts index 197ff9ba6034..1cbd447f4918 100644 --- a/src/BloomE2E/helpers/bookHtml.ts +++ b/src/BloomE2E/helpers/bookHtml.ts @@ -153,3 +153,35 @@ export function bookFileExists( const file = relativePath.split("#")[0]; return fs.existsSync(Path.join(bookFolder, file)); } + +/** + * The front/back matter pack a book's HTML carries: the key before "-XMatter" in the one pack + * stylesheet its head links, e.g. "Factory" or "Traditional". Bloom links exactly one such + * stylesheet per book (XMatterHelper.GetStyleSheetFileName) and swaps it when it brings the book up + * to date under a collection whose pack changed, so this says which pack the book was last brought + * up to date with. It is the same answer for a book on disk and for the copy an upload sent to + * Bloom Library, which is why it takes the HTML rather than a path. Throws, naming the stylesheets + * it did find, when the HTML links no pack or more than one. + */ +export function xmatterPackInBookHtml(html: string): string { + const hrefs = [...html.matchAll(/]*\shref="([^"]+)"/g)].map( + (match) => match[1], + ); + const packs = hrefs.flatMap((href) => { + const match = /^(.+)-XMatter\.css$/.exec(href); + return match ? [match[1]] : []; + }); + if (packs.length !== 1) + throw new Error( + `Expected the book's HTML to link exactly one -XMatter.css, found ${packs.length}. ` + + `Stylesheets linked: ${hrefs.join(", ")}`, + ); + return packs[0]; +} + +/** The front/back matter pack of the book saved in `bookFolder`; see xmatterPackInBookHtml. */ +export function readXmatterPackOfBook(bookFolder: string): string { + return xmatterPackInBookHtml( + fs.readFileSync(bookHtmlPath(bookFolder), "utf8"), + ); +} diff --git a/src/BloomE2E/helpers/bulkUpload.ts b/src/BloomE2E/helpers/bulkUpload.ts new file mode 100644 index 000000000000..3eed7ca61306 --- /dev/null +++ b/src/BloomE2E/helpers/bulkUpload.ts @@ -0,0 +1,144 @@ +// Drive Publish: Web's "Upload this collection" bulk upload, and read what it did. +// +// Bulk upload is the split button beside "Upload Book" on the Publish: Web screen: its dropdown +// offers "Upload this collection" and "Upload folder of collections". Picking one sets it as the +// button's action; then the button runs it (see bloomSplitButton.tsx). The button shows only to a +// signed-in user and is enabled only when the selected book is ready to upload and the agreements +// are ticked, which is the same gate a single upload passes. +// +// The upload itself runs in a second Bloom that Bloom starts (BloomLibraryPublishModel.BulkUpload), +// so the result does not come back through the screen. That second Bloom writes BloomBulkUploadLog.txt +// into the collection folder, and its last lines say how many books were uploaded, updated and +// skipped. A test reads that, which is also how a person reads a bulk upload's result. + +import { expect, type Page } from "@playwright/test"; +import * as fs from "node:fs"; +import * as Path from "node:path"; +import { acceptAllAgreements, openPublishToWeb } from "./libraryPublish"; + +/** The name of the log the bulk-upload child process writes into the collection folder. */ +const BULK_UPLOAD_LOG = "BloomBulkUploadLog.txt"; + +/** What one bulk upload did, read from its log's final tally. */ +export interface IBulkUploadResult { + /** Books uploaded for the first time. */ + newBooks: number; + /** Books that had changed and were re-uploaded. */ + updated: number; + /** Books skipped because nothing had changed since the last upload. */ + skipped: number; + /** The whole log, for a failure message when the tally is not what a test expected. */ + log: string; +} + +/** The path of the bulk-upload log in a collection folder. */ +function logPath(collectionDir: string): string { + return Path.join(collectionDir, BULK_UPLOAD_LOG); +} + +/** + * Remove the bulk-upload log, so the next upload's result is read fresh rather than from a tally an + * earlier round left. Call before each upload; each round writes the whole log again. + */ +export function clearBulkUploadLog(collectionDir: string): void { + fs.rmSync(logPath(collectionDir), { force: true }); +} + +/** + * Start a bulk upload of the whole collection through the split button, the way a person does: + * open the dropdown beside Upload Book, pick "Upload this collection", and click the button. A book + * must already be selected, the user signed in, and the agreements ticked, or the button is + * disabled. This does not wait for the upload to finish; see waitForBulkUploadResult. + */ +export async function startCollectionUpload(page: Page): Promise { + const buttons = page.getByTestId("upload-buttons"); + // Open the dropdown. The arrow button is the split button's second button. + await buttons.getByRole("button", { name: "select upload source" }).click(); + // The menu is rendered at the document root, not inside upload-buttons. The label is + // "Upload this Collection"; match case-insensitively so a capitalization tweak does not break it. + await page + .getByRole("menuitem", { name: /upload this collection/i }) + .click(); + // Picking only selected it; the primary button runs it. + const primary = buttons.getByRole("button", { + name: /upload this collection/i, + }); + await expect(primary).toBeEnabled({ timeout: 15000 }); + await primary.click(); +} + +/** + * Wait until the bulk-upload child process has finished and return its tally. It writes the log as + * it goes and ends with the three counts, so this polls for the "Skipped ... books" line — the last + * of the three — then reads all three. Throws with the log when it does not finish in time. + */ +export async function waitForBulkUploadResult( + collectionDir: string, + timeoutMs = 180000, +): Promise { + const file = logPath(collectionDir); + const skippedLine = /Skipped (\d+) books/; + await expect + .poll( + () => (fs.existsSync(file) ? fs.readFileSync(file, "utf8") : ""), + { + timeout: timeoutMs, + message: `The bulk upload never finished: ${file} did not report a final tally.`, + }, + ) + .toMatch(skippedLine); + + const log = fs.readFileSync(file, "utf8"); + const count = (pattern: RegExp): number => { + const match = log.match(pattern); + if (!match) + throw new Error( + `The bulk-upload log did not report "${pattern.source}". Log:\n${log}`, + ); + return Number(match[1]); + }; + return { + newBooks: count(/Uploaded (\d+) new books/), + updated: count(/Updated (\d+) books/), + skipped: count(/Skipped (\d+) books/), + log, + }; +} + +/** + * Upload the whole collection and wait for its result, the way a person does from a selected book: + * go to Publish: Web, tick the agreements, run "Upload this collection", and read the tally the + * child Bloom wrote. A book must be selected and the user signed in. Coming back to Publish: Web + * and ticking already-ticked agreements is harmless, so a test calls this for every upload round, + * wherever it left Bloom in between. + */ +export async function uploadCollection( + page: Page, + collectionDir: string, +): Promise { + await openPublishToWeb(page); + await acceptAllAgreements(page); + clearBulkUploadLog(collectionDir); + await startCollectionUpload(page); + return waitForBulkUploadResult(collectionDir); +} + +/** + * Try to upload the collection with no bookshelf set, and return the message Bloom refuses with. + * Bloom will not bulk-upload a collection that has no Bloom Library bookshelf; it tells the user to + * set one. The message goes to the Upload screen's progress box, which is what this reads. + */ +export async function uploadCollectionExpectingBookshelfWarning( + page: Page, +): Promise { + await startCollectionUpload(page); + const progress = page.getByTestId("progress-box-log"); + await expect(progress).toContainText("bookshelf", { + ignoreCase: true, + timeout: 30000, + }); + return (await progress.innerText()).trim(); +} + +/** Re-export so a test opens the Web screen and drives bulk upload from one import. */ +export { openPublishToWeb }; diff --git a/src/BloomE2E/helpers/collectionSettings.ts b/src/BloomE2E/helpers/collectionSettings.ts index d10784f48332..081053652973 100644 --- a/src/BloomE2E/helpers/collectionSettings.ts +++ b/src/BloomE2E/helpers/collectionSettings.ts @@ -29,6 +29,8 @@ export interface ICollectionSettings { * Left out, the collection has no code and so is Basic. */ subscriptionCode?: string; + /** The Bloom Library bookshelf, by url key; see ICollectionSpec.bookshelf. Left out, none. */ + bookshelf?: string; } /** @@ -66,7 +68,7 @@ export async function restartWithCollectionSettings( makeCollectionXml( settings.languages, settings.xmatterPack, - settings.subscriptionCode, + settings, ), "utf8", ), @@ -113,6 +115,13 @@ export type SubscriptionTier = */ export const kEnterpriseSubscriptionCode = "Test-727011-1339"; +/** + * The bookshelves the Test subscription (kEnterpriseSubscriptionCode) owns on Bloom Library, by + * url key, which Contentful knows about. A collection under that code can name either as the + * bookshelf its books are uploaded to, as the Settings dialog offers a person. + */ +export const kTestBookshelves = ["test-bookshelf-1", "test-bookshelf-2"]; + /** * What Bloom says about one feature, as features/status reports it. It answers with more than * this, all of it about how to word the message offering an upgrade; these are the fields that say diff --git a/src/BloomE2E/helpers/copyrightAndLicense.ts b/src/BloomE2E/helpers/copyrightAndLicense.ts index 7cfb74d262e1..6796e18e9e45 100644 --- a/src/BloomE2E/helpers/copyrightAndLicense.ts +++ b/src/BloomE2E/helpers/copyrightAndLicense.ts @@ -7,6 +7,11 @@ import { expect, type Locator, type Page } from "@playwright/test"; +// There is deliberately no direct-API "set the copyright" helper. The obvious endpoint, +// copyrightAndLicense/bookCopyrightAndLicense, is handled on Bloom's UI thread and deadlocks when a +// test posts it (the fetch comes from the WebView2 whose UI thread the handler then blocks), on the +// Edit tab and off it alike. A test sets copyright the way a person does, through the dialog below. + /** The open dialog, whichever page opened it. */ function dialog(page: Page): Locator { return page diff --git a/src/BloomE2E/helpers/libraryPublish.ts b/src/BloomE2E/helpers/libraryPublish.ts index c72ebeadce66..e4bdd5febb4d 100644 --- a/src/BloomE2E/helpers/libraryPublish.ts +++ b/src/BloomE2E/helpers/libraryPublish.ts @@ -201,9 +201,10 @@ export async function expectUploadStepButtons( } /** - * Click the Upload Book button. Bloom refuses to upload at all under --e2e (see - * LibraryPublishApi.RefuseUploadWhileRunningE2eTests), so this cannot publish anything; what a - * test can see is what the screen does before that point, such as the template warning below. + * Click the Upload Book button. Under --e2e Bloom uploads only to the sandbox and refuses + * bloomlibrary.org itself (see LibraryPublishApi.RefuseUploadWhileRunningE2eTests), and a test that + * has only a pretended login (e2e/loginState) has no real session to upload with; what such a test + * sees is what the screen does before an upload would happen, such as the template warning below. */ export async function clickUploadBook(page: Page): Promise { await page @@ -226,10 +227,11 @@ export async function waitForTemplateUploadWarning( } /** - * Answer No to the template warning. There is deliberately no helper for Yes: Bloom refuses to - * upload under --e2e, so answering Yes would prove nothing beyond that refusal, and what the - * manual test is really checking — that a template is allowed through the copyright rule — is - * already settled by the warning appearing at all. + * Answer No to the template warning. There is deliberately no helper for Yes here: this helper + * serves tests that only pretend to be logged in, which have no real session to upload with, and + * what the manual test is really checking — that a template is allowed through the copyright rule — + * is already settled by the warning appearing at all. A test that signs in for real and means to + * upload drives the Yes path itself. */ export async function declineTemplateUploadWarning(page: Page): Promise { await templateUploadWarning(page) diff --git a/src/BloomE2E/helpers/userSettings.ts b/src/BloomE2E/helpers/userSettings.ts new file mode 100644 index 000000000000..41d58522d442 --- /dev/null +++ b/src/BloomE2E/helpers/userSettings.ts @@ -0,0 +1,44 @@ +// Where the Bloom under test keeps its user settings (user.config: UI language, page zoom, the +// Bloom Library login, and the rest of Settings.Default), and what it has saved there. +// +// The launch fixture gives every Bloom it starts a settings folder of its own inside the run's +// temp folder (bloomApp.userSettingsDir; see fixtures/launchBloom.ts), so a test can look at the +// settings its Bloom saved with nobody else's Bloom in the way, and can put settings there before +// a launch for Bloom to start from. + +import * as fs from "node:fs"; +import * as Path from "node:path"; +import type { Page } from "@playwright/test"; +import { apiGetJson } from "./api"; + +/** Ask Bloom which folder it is keeping its user settings in. */ +export async function getUserSettingsFolder(page: Page): Promise { + const info = await apiGetJson<{ userSettingsFolder: string }>( + page, + "common/instanceInfo", + ); + return info.userSettingsFolder; +} + +/** + * Read one user setting Bloom has saved to the user.config in `folder`, by the name it has in + * Bloom's Settings.settings (e.g. "PageZoom"), as the string in the file; undefined when there is + * no such file yet or Bloom has not saved that setting. Only settings serialized as a string are + * readable this way, which is nearly all of them. + * + * This reads the disk rather than asking Bloom, because what a test usually wants to know is what + * reached the file: that is what the next Bloom to use the folder starts from. Bloom writes some + * settings a moment after they change (the zoom two seconds after the last change), so poll. + */ +export function readSavedUserSetting( + folder: string, + name: string, +): string | undefined { + const file = Path.join(folder, "user.config"); + if (!fs.existsSync(file)) return undefined; + const xml = fs.readFileSync(file, "utf8"); + const match = new RegExp( + `]*>\\s*([^<]*)`, + ).exec(xml); + return match ? match[1] : undefined; +} diff --git a/src/BloomE2E/tests/bulk-upload-quick-test.spec.ts b/src/BloomE2E/tests/bulk-upload-quick-test.spec.ts new file mode 100644 index 000000000000..a89328b856a0 --- /dev/null +++ b/src/BloomE2E/tests/bulk-upload-quick-test.spec.ts @@ -0,0 +1,290 @@ +// Bulk upload a small collection to dev.bloomlibrary.org for real, and check what the manual "Bulk +// Upload Quick Test" watches: a collection with no bookshelf is refused, a first upload sends every +// book, an unchanged re-upload skips them all, changing one book updates only that one, and moving +// the collection to another bookshelf updates them all and puts them on the new shelf with the +// collection's current front/back matter. Automates Notion test case 211, whose last steps absorbed +// the retired "Bulk Upload Across Bookshelves Applies Xmatter" (test case 212). +// +// This is the first e2e test that signs in to Bloom Library for real and uploads for real. It can +// do so because a test's Bloom keeps its login in a settings folder of its own (bloomApp.userSettingsDir), +// so the sign-in, and the upload's child Bloom that reads it back, never touch the developer's own +// Bloom. Everything goes to the sandbox, dev.bloomlibrary.org; Bloom refuses under --e2e to upload +// to production at all. The test deletes what it uploads, and any the account had left from before, +// in an afterAll — see helpers/bloomLibraryServer.ts. +// +// What is NOT covered here, and stays on a manual portion of the card: confirming each uploaded +// book on the website against the screenshot on its front cover, which is a human visual check. +// +// The account's password comes from BLOOM_E2E_TESTER_EMAIL_BLORG_PASSWORD (see +// helpers/bloomLibraryAccount.ts): without it the test skips locally and fails on CI. + +import { expect, test } from "../fixtures/bloomTest"; +import type { Page } from "@playwright/test"; +import type { IBloomApp } from "../fixtures/bloomTest"; +import { + addPage, + findBookFolder, + getContentPages, + getPages, + goToPage, + makeBookFromTemplate, + typeInGroup, +} from "../helpers/bookMaking"; +import { readXmatterPackOfBook } from "../helpers/bookHtml"; +import { setCopyrightHolder } from "../helpers/copyrightAndLicense"; +import { selectBook } from "../helpers/collection"; +import { switchTab } from "../helpers/workspace"; +import { + acceptAllAgreements, + clickToFixMissingItem, + openPublishToWeb, +} from "../helpers/libraryPublish"; +import { + kEnterpriseSubscriptionCode, + kTestBookshelves, + restartWithCollectionSettings, + type ICollectionSettings, +} from "../helpers/collectionSettings"; +import { + signBloomIntoLibraryForReal, + skipIfNoLibraryPassword, + TEST_ACCOUNT_EMAIL, +} from "../helpers/bloomLibraryAccount"; +import { + deleteAllBooksUploadedBy, + findBooksUploadedBy, + getXmatterPackOfBookOnServer, + type IBloomLibraryLogin, +} from "../helpers/bloomLibraryServer"; +import { + uploadCollection, + uploadCollectionExpectingBookshelfWarning, +} from "../helpers/bulkUpload"; + +// A collection under the Test enterprise subscription (bulk upload needs an enterprise tier), with +// no bookshelf yet: the first thing the card checks is that a bookshelf-less collection is refused. +test.use({ + collectionSpec: { + name: "bulk-upload-quick-test", + languages: ["en"], + subscriptionCode: kEnterpriseSubscriptionCode, + }, +}); + +test.describe.configure({ mode: "serial" }); + +// Four books, as the manual test uses. Each title is distinctive so the server records are easy to +// tell apart in a failure message. +const BOOK_TITLES = [ + "Bulk Upload Quick Test Book 1", + "Bulk Upload Quick Test Book 2", + "Bulk Upload Quick Test Book 3", + "Bulk Upload Quick Test Book 4", +]; +const COPYRIGHT_HOLDER = "Bloom Automated Test"; +// The collection goes on the first shelf, then moves to the second. +const [FIRST_BOOKSHELF, SECOND_BOOKSHELF] = kTestBookshelves; +// The front/back matter pack the collection starts with (the fixture's default) and the one it +// moves to along with the bookshelf, so the last upload has to bring every book up to date. +const FIRST_XMATTER_PACK = "Factory"; +const SECOND_XMATTER_PACK = "Traditional"; + +// The login the test signs in with, kept so afterAll can delete what was uploaded. +let login: IBloomLibraryLogin | undefined; +// The folder of each book the test made, in BOOK_TITLES order. Bloom names a book's folder after +// its title, but only after a save, so the folders are looked up rather than assumed. +const bookFolders: string[] = []; + +/** + * Make one book that is ready to upload: a Basic Book with a title, a content page with a word of + * text (so it has a language to publish), and a copyright. Leaves it saved and returns its folder. + * Setup, not the behavior under test, so it takes the fast route to each piece. + */ +async function makeUploadableBook(page: Page, title: string): Promise { + await switchTab(page, "collection"); + await makeBookFromTemplate(page, "Basic Book"); // lands on the cover in the Edit tab + await typeInGroup(page, ".bookTitle", "en", title); + await addPage(page, "Just Text"); + const [contentPage] = await getContentPages(page); + await goToPage(page, contentPage.id); + await typeInGroup(page, ".bloom-translationGroup", "en", "Hello."); + // Leave the content page (back to the cover) so Bloom saves what was typed. + await goToPage(page, (await getPages(page))[0].id); + // Give it a copyright through the real dialog, reached from Publish: Web's "Click to fix" on the + // Missing Copyright warning (the direct copyright API deadlocks; see helpers/copyrightAndLicense.ts). + await openPublishToWeb(page); + await clickToFixMissingItem(page, "Copyright"); + await setCopyrightHolder(page, COPYRIGHT_HOLDER); + return findBookFolder(page, title); +} + +/** + * Give the collection these settings (on top of its language and subscription), the way a person + * does in the Settings dialog, and come back signed in with the first book selected, ready to + * upload. The restart signs Bloom out of the login it kept only in memory, so this signs in again + * for the upload's child process to read from the settings folder. Returns the new shell page. + */ +async function restartReadyToUpload( + bloomApp: IBloomApp, + settings: Pick, +): Promise { + await restartWithCollectionSettings(bloomApp, { + languages: ["en"], + subscriptionCode: kEnterpriseSubscriptionCode, + ...settings, + }); + const page = bloomApp.page; + login = await signBloomIntoLibraryForReal(page); + await selectBook(page, bookFolders[0]); + return page; +} + +/** + * Check the four books on the sandbox, which is the card's "on Blorg, the books are on the bookshelf + * you set and carry the collection's xmatter": all four are there under the test account, each on + * this bookshelf, and each uploaded with this front/back matter pack. (The record's branding is not + * checked: the server fills that field in some time after the upload finishes, so a check right + * after the tally is flaky.) + */ +async function expectBooksOnServer( + account: IBloomLibraryLogin, + bookshelf: string, + xmatterPack: string, +): Promise { + const onServer = await findBooksUploadedBy(account); + expect( + onServer.map((b) => b.title).sort(), + `dev.bloomlibrary.org should list the four uploaded books for ${TEST_ACCOUNT_EMAIL}.`, + ).toEqual([...BOOK_TITLES].sort()); + for (const book of onServer) { + // "On", not "only on": after the move below, the sandbox still lists the first shelf as + // well (seen 2026-09-04). Bloom sends only the current shelf's tag, dropping any earlier + // bookshelf tag (BookUpload.UploadBookAsync), so it is the server that keeps the old + // one when a re-upload lands. Whether that is meant is an open question for the library + // team; this checks what the card asks for, that the books sit on the new shelf. + expect( + book.bookshelves, + `${book.title} should be on the ${bookshelf} bookshelf.`, + ).toContain(bookshelf); + expect( + await getXmatterPackOfBookOnServer(book), + `${book.title} should have been uploaded with the collection's ${xmatterPack} front/back matter.`, + ).toBe(xmatterPack); + } +} + +test.describe("bulk uploading a collection to dev.bloomlibrary.org", () => { + test.afterAll(async () => { + // Delete everything the account has on the sandbox, whether this run uploaded it or a + // crashed earlier run did, so dev.bloomlibrary.org is left clean. + if (login) await deleteAllBooksUploadedBy(login); + }); + + test("bulk upload: refused without a bookshelf, then new / skipped / updated / moved to another bookshelf [Test Case ID 211]", async ({ + page, + bloomApp, + }) => { + skipIfNoLibraryPassword(); + test.setTimeout(900000); + + // Clean the account first, so a crashed earlier run's books do not turn this run's "4 new" + // into "some updated", and get the login this test signs in and cleans up with. + login = await signBloomIntoLibraryForReal(page); + await deleteAllBooksUploadedBy(login); + + // ---- Four uploadable books -------------------------------------------------------------- + for (const title of BOOK_TITLES) + bookFolders.push(await makeUploadableBook(page, title)); + + // ---- A collection with no bookshelf is refused ------------------------------------------ + await selectBook(page, bookFolders[0]); + await openPublishToWeb(page); + await acceptAllAgreements(page); + const warning = await uploadCollectionExpectingBookshelfWarning(page); + expect( + warning, + "The no-bookshelf refusal should name the bookshelf setting.", + ).toContain("bookshelf"); + + // ---- Set the bookshelf, then upload: four new books ------------------------------------- + const pageAfterRestart = await restartReadyToUpload(bloomApp, { + bookshelf: FIRST_BOOKSHELF, + }); + const firstUpload = await uploadCollection( + pageAfterRestart, + bloomApp.collectionDir, + ); + expect( + firstUpload, + `The first bulk upload should have sent all four books as new. Log:\n${firstUpload.log}`, + ).toMatchObject({ newBooks: 4, updated: 0, skipped: 0 }); + // The four books really are on the sandbox now, on the shelf and with the pack the + // collection had. + await expectBooksOnServer(login!, FIRST_BOOKSHELF, FIRST_XMATTER_PACK); + + // ---- Upload again with nothing changed: four skipped ------------------------------------ + const secondUpload = await uploadCollection( + pageAfterRestart, + bloomApp.collectionDir, + ); + expect( + secondUpload, + `An unchanged re-upload should skip all four books. Log:\n${secondUpload.log}`, + ).toMatchObject({ newBooks: 0, updated: 0, skipped: 4 }); + + // ---- Change one book, upload again: one updated, three skipped -------------------------- + await selectBook(pageAfterRestart, bookFolders[0]); + await switchTab(pageAfterRestart, "edit"); + const [firstContentPage] = await getContentPages(pageAfterRestart); + await goToPage(pageAfterRestart, firstContentPage.id); + await typeInGroup( + pageAfterRestart, + ".bloom-translationGroup", + "en", + "Hello again.", + ); + // Leave the page so Bloom saves the change before the upload reads the folder. + await switchTab(pageAfterRestart, "collection"); + const thirdUpload = await uploadCollection( + pageAfterRestart, + bloomApp.collectionDir, + ); + expect( + thirdUpload, + `After changing one book, only that book should be updated. Log:\n${thirdUpload.log}`, + ).toMatchObject({ newBooks: 0, updated: 1, skipped: 3 }); + + // ---- Move the collection to another bookshelf, with another front/back matter pack: + // all four updated, and each lands on the new shelf with the new pack -------------- + // The pack changes along with the shelf because that is what the card's xmatter check needs: + // books nobody has opened since the pack changed must still go up with the collection's + // current pack, which bulk upload gets by bringing each book up to date before it hashes it + // (BulkUploader.UploadBookInternal). The shelf alone would already make every book count as + // changed, since the hash a skip is decided on covers the collection file too. + const pageAfterMove = await restartReadyToUpload(bloomApp, { + bookshelf: SECOND_BOOKSHELF, + xmatterPack: SECOND_XMATTER_PACK, + }); + // Sanity check: the books nobody selected still carry the old pack on disk, so the new pack + // on the server can only come from the upload bringing them up to date. (Selecting the first + // book, above, may already have brought that one up to date, so it is left out.) + for (const folder of bookFolders.slice(1)) + expect( + readXmatterPackOfBook(folder), + `${folder} should still have the old front/back matter until the upload brings it up to date.`, + ).toBe(FIRST_XMATTER_PACK); + const fourthUpload = await uploadCollection( + pageAfterMove, + bloomApp.collectionDir, + ); + expect( + fourthUpload, + `Moving the collection to another bookshelf should update all four books. Log:\n${fourthUpload.log}`, + ).toMatchObject({ newBooks: 0, updated: 4, skipped: 0 }); + await expectBooksOnServer( + login!, + SECOND_BOOKSHELF, + SECOND_XMATTER_PACK, + ); + }); +}); diff --git a/src/BloomE2E/tests/user-settings-isolation.spec.ts b/src/BloomE2E/tests/user-settings-isolation.spec.ts new file mode 100644 index 000000000000..2a7b88e0cfea --- /dev/null +++ b/src/BloomE2E/tests/user-settings-isolation.spec.ts @@ -0,0 +1,103 @@ +// A test's Bloom keeps its user settings (user.config: UI language, page zoom, the Bloom Library +// login, and the rest of Settings.Default) in a folder of its own inside the run's temp folder, +// not in the %LOCALAPPDATA%\SIL\Bloom\ folder that every Bloom of that build shares. So +// a run starts from default settings, and what it saves dies with the run, instead of starting +// from whatever the developer's Bloom saved last and leaving its own changes behind for them. +// +// This spec checks that the machinery holds: Bloom is using the folder the fixture gave it, a +// setting a test changes lands there, and it is still there after a restart within the run. It is +// infrastructure, so it has no "[Test Case ID N]" tag. + +import * as Path from "node:path"; +import { expect, test } from "../fixtures/bloomTest"; +import { makeBookFromTemplate } from "../helpers/bookMaking"; +import { + getUserSettingsFolder, + readSavedUserSetting, +} from "../helpers/userSettings"; +import { getZoom, setZoom } from "../helpers/workspace"; + +test.use({ + collectionSpec: { name: "user-settings-isolation", languages: ["en"] }, +}); + +test.describe.configure({ mode: "serial" }); + +/** Compare two folder paths the way Windows does. */ +function sameFolder(a: string, b: string): boolean { + return Path.resolve(a).toLowerCase() === Path.resolve(b).toLowerCase(); +} + +test.describe("a test's Bloom keeps its user settings to itself", () => { + test("Bloom keeps its user settings in the folder the run gave it", async ({ + page, + bloomApp, + }) => { + const folder = await getUserSettingsFolder(page); + expect( + sameFolder(folder, bloomApp.userSettingsDir), + `Bloom keeps its user settings in ${folder}, not in the run's own ${bloomApp.userSettingsDir}.`, + ).toBe(true); + + // Bloom accepted the license for us at startup (there is nobody to click Accept) and saved + // that, so the folder already holds a user.config, and this is what is in it. + expect( + readSavedUserSetting(bloomApp.userSettingsDir, "LicenseAccepted"), + ).toBe("True"); + }); + + test("a setting changed in the test's Bloom is saved in that folder", async ({ + page, + bloomApp, + }) => { + test.setTimeout(300000); + // The zoom is a user setting Bloom saves on its own, a couple of seconds after it changes, + // and there is a zoom only while a book is being edited. + await makeBookFromTemplate(page, "Basic Book"); + const { zoom, minZoom, maxZoom } = await getZoom(page); + // A fresh settings folder means Bloom starts at its default zoom, whatever the developer's + // own Bloom is zoomed to. + expect(zoom).toBe(100); + const newZoom = Math.min(zoom + 20, maxZoom); + expect(newZoom).toBeGreaterThanOrEqual(minZoom); + + await setZoom(page, newZoom); + + await expect + .poll( + () => + readSavedUserSetting(bloomApp.userSettingsDir, "PageZoom"), + { + timeout: 15000, + message: `Bloom never saved the zoom of ${newZoom}% to ${bloomApp.userSettingsDir}.`, + }, + ) + .toBe(String(newZoom)); + }); + + test("the saved setting is still there after a restart within the run", async ({ + bloomApp, + }) => { + test.setTimeout(300000); + const savedZoom = readSavedUserSetting( + bloomApp.userSettingsDir, + "PageZoom", + ); + expect( + savedZoom, + "test setup: the previous test saved a zoom", + ).toBeDefined(); + + const page = await bloomApp.restart(); + + expect( + sameFolder( + await getUserSettingsFolder(page), + bloomApp.userSettingsDir, + ), + ).toBe(true); + expect(readSavedUserSetting(bloomApp.userSettingsDir, "PageZoom")).toBe( + savedZoom, + ); + }); +}); diff --git a/src/BloomExe/BloomSettingsProvider.cs b/src/BloomExe/BloomSettingsProvider.cs new file mode 100644 index 000000000000..f262e9599fbc --- /dev/null +++ b/src/BloomExe/BloomSettingsProvider.cs @@ -0,0 +1,118 @@ +using System.Configuration; +using System.IO; +using SIL.Settings; + +namespace Bloom +{ + /// + /// The provider behind Bloom's user settings (Settings.Default, the contents of user.config). + /// It is libpalaso's CrossPlatformSettingsProvider, which keeps one user.config per build + /// version in %LOCALAPPDATA%\SIL\Bloom\<version>\, with one addition: a folder named on the + /// command line (--user-settings-folder) replaces that location, for this process only. + /// + /// Every Bloom of one build otherwise shares one user.config, so a Bloom that an automated + /// test launches would start from whatever the last Bloom of that version saved (the UI + /// language, the page zoom, the Bloom Library login) and save its own changes for the next one, + /// including the developer's own Bloom from a worktree of the same version. The e2e launch + /// fixture therefore gives each Bloom it starts a folder inside the run's temp folder, so its + /// settings start from defaults, or from whatever the test put there first, and are deleted + /// with the rest of the run. Such a folder holds exactly the settings its owner put there, so + /// this provider also brings nothing in from a previous version's user.config when one is + /// named (see Upgrade below); an automated run would otherwise inherit the developer's + /// settings after all. + /// + /// Only this class knows whether a folder was named. Everything else asks it for what it + /// needs: where the settings are (GetUserSettingsFolder), or what to put on the command line + /// of another Bloom so that it uses the same ones (CommandLineArgumentsForChildBloom). + /// + public class BloomSettingsProvider : CrossPlatformSettingsProvider, IApplicationSettingsProvider + { + // The folder named on the command line, or null for the usual per-version one. + private static string _folderFromCommandLine; + + /// + /// Keep user.config in this folder instead of the usual per-version one; null restores the + /// usual one. Program's startup argument parser calls this for --user-settings-folder, + /// before anything reads Settings.Default: a provider computes its location when it is + /// constructed, and Settings.Default constructs its providers the first time any setting + /// is read. + /// + public static void SetUserSettingsFolder(string folder) + { + _folderFromCommandLine = folder; + } + + public BloomSettingsProvider() + { + if (_folderFromCommandLine != null) + { + UserLocalLocation = _folderFromCommandLine; + UserRoamingLocation = _folderFromCommandLine; + } + } + + /// + /// The folder this process keeps user.config in: the one named on the command line when + /// there was one, otherwise the usual %LOCALAPPDATA%\SIL\Bloom\<version>. Reported + /// through common/instanceInfo so an automated run can check that the Bloom it launched + /// really is keeping its settings where it was told to. + /// + public static string GetUserSettingsFolder() + { + return new BloomSettingsProvider().UserConfigLocation; + } + + /// + /// The path of the user.config file this process reads and writes. + /// + public static string GetUserConfigPath() + { + return Path.Combine(GetUserSettingsFolder(), UserConfigFileName); + } + + /// + /// What to put on the command line of another Bloom this one starts so that it reads and + /// writes the same user settings as this one: the --user-settings-folder argument when a + /// folder was named, otherwise nothing, since another Bloom of this build shares the usual + /// per-version folder anyway. + /// + public static string CommandLineArgumentsForChildBloom => + _folderFromCommandLine == null + ? "" + : $"--user-settings-folder \"{_folderFromCommandLine}\""; + + // ApplicationSettingsBase drives its providers through IApplicationSettingsProvider, so + // re-implementing the interface here (libpalaso's methods are not virtual) lets this class + // decide what "a previous version's settings" means for it: nothing, when a folder was + // named. Settings.Default.Upgrade() then does exactly what the caller intends in both + // cases, and no caller needs to know which case it is in. + + /// + /// Bring in the settings of a previous Bloom version, unless a folder was named on the + /// command line, which holds exactly the settings its owner put there. + /// + void IApplicationSettingsProvider.Upgrade( + SettingsContext context, + SettingsPropertyCollection properties + ) + { + if (_folderFromCommandLine != null) + return; + base.Upgrade(context, properties); + } + + /// + /// A setting's value from a previous Bloom version, or null when a folder was named on the + /// command line: such a folder has no previous version. + /// + SettingsPropertyValue IApplicationSettingsProvider.GetPreviousVersion( + SettingsContext context, + SettingsProperty property + ) + { + if (_folderFromCommandLine != null) + return null; + return base.GetPreviousVersion(context, property); + } + } +} diff --git a/src/BloomExe/Program.cs b/src/BloomExe/Program.cs index 55b93b4ae6af..7de80e8e4688 100644 --- a/src/BloomExe/Program.cs +++ b/src/BloomExe/Program.cs @@ -121,6 +121,11 @@ static class Program // rebuild and relaunch us. internal static int? StartupLauncherPort { get; private set; } + // The folder --user-settings-folder asked Bloom to keep its user settings in, or null + // when none was named. BloomSettingsProvider is what acts on it; this copy is only for + // reporting what was requested. + internal static string StartupUserSettingsFolder { get; private set; } + internal static string StartupRequestedPortSummary => string.Join( ", ", @@ -131,6 +136,9 @@ static class Program StartupLauncherPort.HasValue ? $"launcherPort={StartupLauncherPort.Value}" : null, + StartupUserSettingsFolder != null + ? $"userSettingsFolder={StartupUserSettingsFolder}" + : null, }.Where(value => value != null) ); @@ -146,6 +154,31 @@ static int Main(string[] args1) // final call to CleanupTempFolder. Also prevents our temp files competing with // other programs for 64K available default temp file names. TempFile.NamePrefix = "bloom"; + + // Parse our own startup arguments before anything reads Settings.Default: + // --user-settings-folder decides where the settings live (the parser hands it to + // BloomSettingsProvider), and a settings provider fixes its location when it is + // constructed, which happens the first time a setting is read. + var args = ParseStartupPortArguments(args1, out var startupPortErrorMessage); + if (startupPortErrorMessage != null) + { + // A rejected launch touches no settings, its own or anyone else's, so the error is + // reported before anything reads Settings.Default. CheckForCorruptUserConfig and + // SetUpLocalization below both do, and a rejected launch owns no settings folder + // (the parser hands one to BloomSettingsProvider only for an accepted command + // line), so they would read, and might repair, the shared profile of whoever is + // running Bloom. Only what a message box needs is set up. + Application.EnableVisualStyles(); + Application.SetCompatibleTextRenderingDefault(false); + MessageBox.Show( + startupPortErrorMessage, + "Bloom", + MessageBoxButtons.OK, + MessageBoxIcon.Error + ); + return 1; + } + CheckForCorruptUserConfig(); // Tell any Freeze Doctor already running that a Bloom has started, as early in Main as it can // go, so it adopts us at once instead of at its next five-second sweep. Everything before this @@ -194,18 +227,6 @@ static int Main(string[] args1) // every startup path calls it. SetUpLocalization(); - var args = ParseStartupPortArguments(args1, out var startupPortErrorMessage); - if (startupPortErrorMessage != null) - { - MessageBox.Show( - startupPortErrorMessage, - "Bloom", - MessageBoxButtons.OK, - MessageBoxIcon.Error - ); - return 1; - } - // Old comment: Firefox60 uses Gtk3, so we need to as well. (BL-10469) // Aug 2023, we've moved away from GeckoFx/Firefox to wv2, but I don't know if this is still needed or not... // Steve says he thinks Gtk3 is still better but that it likely only matters for Linux, @@ -359,6 +380,7 @@ static int Main(string[] args1) if (Settings.Default.NeedUpgrade) { //see http://stackoverflow.com/questions/3498561/net-applicationsettingsbase-should-i-call-upgrade-every-time-i-load + // (BloomSettingsProvider decides what, if anything, there is to bring in.) Settings.Default.Upgrade(); Settings.Default.Reload(); Settings.Default.NeedUpgrade = false; @@ -807,9 +829,15 @@ internal static string[] ParseStartupPortArguments(string[] args, out string err StartupLabel = null; StartupAutomation = false; StartupLauncherPort = null; + StartupUserSettingsFolder = null; + BloomSettingsProvider.SetUserSettingsFolder(null); RunningE2eTests = false; StartupExperimentalFeatures = null; + // Collected here and handed to BloomSettingsProvider only once the whole command line + // has been accepted, so a rejected launch never owns a settings folder: Main reports + // the error before anything reads settings, and there is nothing to undo here. + string userSettingsFolder = null; var remainingArgs = new List(); for (var i = 0; i < args.Length; i++) @@ -864,6 +892,12 @@ out errorMessage value => StartupExperimentalFeatures = value, out errorMessage ) + || TryHandleUserSettingsFolderArgument( + args, + ref i, + ref userSettingsFolder, + out errorMessage + ) ) { if (errorMessage != null) @@ -883,9 +917,62 @@ out errorMessage return Array.Empty(); } + StartupUserSettingsFolder = userSettingsFolder; + BloomSettingsProvider.SetUserSettingsFolder(userSettingsFolder); return remainingArgs.ToArray(); } + /// + /// Handle --user-settings-folder: the folder to keep user.config in, which the caller + /// hands to BloomSettingsProvider. Stored as a full path, because Bloom changes its + /// working directory during startup (NormalizeWorkingDirectory) and a relative path would + /// otherwise point somewhere else by the time the settings are saved. + /// + private static bool TryHandleUserSettingsFolderArgument( + string[] args, + ref int index, + ref string folder, + out string errorMessage + ) + { + const string optionName = "--user-settings-folder"; + if ( + !TryParseStartupStringArgument( + args, + ref index, + optionName, + out var value, + out errorMessage + ) + ) + { + return false; + } + + if (errorMessage != null) + return true; + + if (folder != null) + { + errorMessage = $"Bloom only accepts one {optionName} argument."; + return true; + } + + try + { + folder = Path.GetFullPath(value); + } + catch (Exception e) + when (e is ArgumentException + || e is NotSupportedException + || e is System.IO.PathTooLongException + ) + { + errorMessage = $"Bloom cannot use \"{value}\" as the {optionName}: {e.Message}"; + } + return true; + } + private static bool TryHandleStartupFlagArgument( string[] args, ref int index, @@ -2971,7 +3058,8 @@ public static BloomFileLocator OptimizedFileLocator private static void CheckForCorruptUserConfig() { //First check the user.config we get through using the palaso stuff. This is the one in a folder with a name like Bloom/3.5.0.0 - var palasoSettings = new SIL.Settings.CrossPlatformSettingsProvider(); + // (or the folder --user-settings-folder named; BloomSettingsProvider knows which). + var palasoSettings = new BloomSettingsProvider(); palasoSettings.Initialize(null, null); var error = palasoSettings.CheckForErrorsInSettingsFile(); if (error != null) diff --git a/src/BloomExe/Properties/Settings.Designer.cs b/src/BloomExe/Properties/Settings.Designer.cs index ff5087a7052f..f205ef0a45a2 100644 --- a/src/BloomExe/Properties/Settings.Designer.cs +++ b/src/BloomExe/Properties/Settings.Designer.cs @@ -24,7 +24,7 @@ public static Settings Default { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("True")] public bool FirstTimeRun { @@ -37,7 +37,7 @@ public bool FirstTimeRun { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] public global::Bloom.CollectionChoosing.MostRecentPathsList MruProjects { get { @@ -49,7 +49,7 @@ public bool FirstTimeRun { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("True")] public bool NeedUpgrade { @@ -62,7 +62,7 @@ public bool NeedUpgrade { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string UserInterfaceLanguage { @@ -75,7 +75,7 @@ public string UserInterfaceLanguage { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool UserInterfaceLanguageSetExplicitly { @@ -88,7 +88,7 @@ public bool UserInterfaceLanguageSetExplicitly { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string LastSourceLanguageViewed { @@ -101,7 +101,7 @@ public string LastSourceLanguageViewed { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string LastSourceLanguageViewed2 { @@ -114,7 +114,7 @@ public string LastSourceLanguageViewed2 { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("http")] public string ImageHandler { @@ -127,7 +127,7 @@ public string ImageHandler { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string ImageGalleryProviderKeys { @@ -140,7 +140,7 @@ public string ImageGalleryProviderKeys { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool ShowLocalizationControls { @@ -153,7 +153,7 @@ public bool ShowLocalizationControls { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool ShowUnapprovedLocalizations { @@ -166,7 +166,7 @@ public bool ShowUnapprovedLocalizations { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string EnabledExperimentalFeatures { @@ -179,7 +179,7 @@ public string EnabledExperimentalFeatures { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string DontShowThisAgain { @@ -192,7 +192,7 @@ public string DontShowThisAgain { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool ShowExperimentalFeatures { @@ -205,7 +205,7 @@ public bool ShowExperimentalFeatures { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string WebUserId { @@ -218,7 +218,7 @@ public string WebUserId { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string WebPassword { @@ -231,7 +231,7 @@ public string WebPassword { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("True")] public bool WebShowPassword { @@ -244,7 +244,7 @@ public bool WebShowPassword { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("True")] public bool MaximizeWindow { @@ -257,7 +257,7 @@ public bool MaximizeWindow { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] public global::SIL.Windows.Forms.PortableSettingsProvider.FormSettings WindowSizeAndLocation { get { @@ -269,7 +269,7 @@ public bool MaximizeWindow { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool LicenseAccepted { @@ -282,7 +282,7 @@ public bool LicenseAccepted { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("0, 0, 1028, 586")] public global::System.Drawing.Rectangle RestoreBounds { @@ -295,7 +295,7 @@ public bool LicenseAccepted { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string ImageSearchLanguage { @@ -308,7 +308,7 @@ public string ImageSearchLanguage { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("1.0")] public string PageZoom { @@ -321,7 +321,7 @@ public string PageZoom { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool AdobeColorProfileEula2003Accepted { @@ -334,7 +334,7 @@ public bool AdobeColorProfileEula2003Accepted { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string BloomDeviceFileExportFolder { @@ -347,7 +347,7 @@ public string BloomDeviceFileExportFolder { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string PublishAndroidMethod { @@ -360,7 +360,7 @@ public string PublishAndroidMethod { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("1")] public int CurrentStage { @@ -373,7 +373,7 @@ public int CurrentStage { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("1")] public int CurrentLevel { @@ -386,7 +386,7 @@ public int CurrentLevel { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string CurrentBookPath { @@ -399,7 +399,7 @@ public string CurrentBookPath { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("0")] public int AutoUpdateDialogShown { @@ -412,7 +412,7 @@ public int AutoUpdateDialogShown { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool AlwaysMeasurePerformance { @@ -425,7 +425,7 @@ public bool AlwaysMeasurePerformance { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool RunFreezeDoctor { @@ -438,7 +438,7 @@ public bool RunFreezeDoctor { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("True")] public bool AutoUpdate { @@ -451,7 +451,7 @@ public bool AutoUpdate { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] public global::System.DateTime ForumInvitationLastShown { get { @@ -463,7 +463,7 @@ public bool AutoUpdate { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("False")] public bool ForumInvitationAcknowledged { @@ -476,7 +476,7 @@ public bool ForumInvitationAcknowledged { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string LastLoginDest { @@ -489,7 +489,7 @@ public string LastLoginDest { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string LastLoginSessionToken { @@ -502,7 +502,7 @@ public string LastLoginSessionToken { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string LastLoginUserId { @@ -515,6 +515,7 @@ public string LastLoginUserId { } [global::System.Configuration.UserScopedSettingAttribute()] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string ExportImportFileFolder { @@ -527,7 +528,7 @@ public string ExportImportFileFolder { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string OpenRouterApiKey { @@ -540,7 +541,7 @@ public string OpenRouterApiKey { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string CollectionTabSplitterSizes { @@ -553,7 +554,7 @@ public string CollectionTabSplitterSizes { } [global::System.Configuration.UserScopedSettingAttribute()] - [global::System.Configuration.SettingsProviderAttribute(typeof(SIL.Settings.CrossPlatformSettingsProvider))] + [global::System.Configuration.SettingsProviderAttribute(typeof(Bloom.BloomSettingsProvider))] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Configuration.DefaultSettingValueAttribute("")] public string WebSiteDestinationOverride { diff --git a/src/BloomExe/Properties/Settings.settings b/src/BloomExe/Properties/Settings.settings index ede1c5758ca1..74c14135c5f7 100644 --- a/src/BloomExe/Properties/Settings.settings +++ b/src/BloomExe/Properties/Settings.settings @@ -2,136 +2,136 @@ - + True - + - + True - + - + False - + - + - + http - + - + False - + False - + False - + - + - + False - + - + - + True - + False - + True - + - + False - + 0, 0, 1028, 586 - + - + 1.0 - + False - + - + - + 1 - + 1 - + - + 0 - + False - + False - + True - + - + False - + - + - + - + - + - + - + diff --git a/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs b/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs index 87bc200bb3a6..897f5cc5b4a7 100644 --- a/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs +++ b/src/BloomExe/Publish/BloomLibrary/BloomLibraryPublishModel.cs @@ -656,15 +656,37 @@ public void BulkUpload(string rootFolderPath, IProgress progress) : UploadDestination.Production; var bloomExePath = Program.BloomExePath; - var command = - $"\"{bloomExePath}\" upload \"{rootFolderPath}\" -u {WebUserId} -d {target}"; + // The upload runs in a second Bloom, which signs in with the login this one saved in + // its user settings, so the second one has to read the same settings as this one, or + // it finds no login at all, or somebody else's. + var arguments = + $"upload \"{rootFolderPath}\" -u {WebUserId} -d {target} {BloomSettingsProvider.CommandLineArgumentsForChildBloom}"; + // An automated run wants the second Bloom to behave as this one does: no modal dialogs. + if (Program.RunningE2eTests) + arguments += " --e2e"; + var command = $"\"{bloomExePath}\" {arguments}"; if (SIL.PlatformUtilities.Platform.IsLinux) command = $"/opt/mono5-sil/bin/mono {command}"; // The Bloom process run as a command line tool for bulk upload is safe for using the current culture. // We don't wait for this to finish, so we don't use the CommandLineRunner methods. ProcessStartInfo startInfo; - if (SIL.PlatformUtilities.Platform.IsWindows) + if (Program.RunningE2eTests) + { + // An automated run has nobody to read a terminal window, and the window below stays + // open (cmd /k) until a person closes it, so it would outlive the test. Run the + // upload with no window at all; BloomBulkUploadLog.txt in the collection folder + // gets everything the window would have shown, and is what a test reads. + startInfo = new ProcessStartInfo() + { + FileName = bloomExePath, + Arguments = arguments, + WorkingDirectory = Path.GetDirectoryName(bloomExePath), + UseShellExecute = false, + CreateNoWindow = true, + }; + } + else if (SIL.PlatformUtilities.Platform.IsWindows) { startInfo = new ProcessStartInfo() { @@ -707,7 +729,15 @@ public void BulkUpload(string rootFolderPath, IProgress progress) BookUploaded = true; // Flag that an upload has occurred BookUploadedId = null; // Multiple books have been uploaded, so a single id doesn't exist: all may need to be updated - ProcessExtra.StartInFront(startInfo); + if (Program.RunningE2eTests) + { + // There is no window to bring to the front. + Process.Start(startInfo); + } + else + { + ProcessExtra.StartInFront(startInfo); + } progress.WriteMessage("Starting bulk upload in a terminal window..."); progress.WriteMessage( "This process will skip books if it can tell that nothing has changed since the last bulk upload." diff --git a/src/BloomExe/web/controllers/CommonApi.cs b/src/BloomExe/web/controllers/CommonApi.cs index 0568c22a1f5a..c7148470b9c5 100644 --- a/src/BloomExe/web/controllers/CommonApi.cs +++ b/src/BloomExe/web/controllers/CommonApi.cs @@ -297,6 +297,11 @@ private void HandleInstanceInfo(ApiRequest request) // Control port of the dev launcher that started us (null when not // launched via go.sh); see .github/skills/bloom-automation. launcherControlPort = Program.StartupLauncherPort, + // Where this instance keeps its user settings (user.config): the folder + // --user-settings-folder named, or the usual per-version one. The e2e launch + // fixture checks this to be sure the Bloom it started is not sharing settings + // with anyone; see BloomSettingsProvider. + userSettingsFolder = BloomSettingsProvider.GetUserSettingsFolder(), } ); } diff --git a/src/BloomExe/web/controllers/LibraryPublishApi.cs b/src/BloomExe/web/controllers/LibraryPublishApi.cs index 1b846f1a0158..ace30720e87e 100644 --- a/src/BloomExe/web/controllers/LibraryPublishApi.cs +++ b/src/BloomExe/web/controllers/LibraryPublishApi.cs @@ -201,19 +201,21 @@ private async Task HandleUpload(ApiRequest request, bool changeUploader) } /// - /// Uploading is refused while Bloom is running e2e tests (the --e2e flag). A test can ask - /// Bloom to REPORT a Bloom Library login (the e2e/loginState hook) so it can check what the - /// upload screen offers a signed-in user; that pretense does not touch the real - /// credentials, so without this an automated click on Upload would publish a real book to - /// a real server under whatever account the developer happens to be signed in as. + /// Uploading to bloomlibrary.org itself is refused while Bloom is running e2e tests (the + /// --e2e flag); an automated run may upload only to the sandbox, dev.bloomlibrary.org. A + /// test signs in there for real, with a test account, by posting the account's session token + /// to external/login, the route the website uses; and a test's Bloom keeps its login in a + /// settings folder of its own (see BloomSettingsProvider), so it is never the developer's + /// account that uploads. Production stays closed all the same, so that no automated click + /// can ever publish a book where the public would see it, whatever account is signed in. /// The refusal goes to the progress box, which is where this screen shows upload trouble. /// private bool RefuseUploadWhileRunningE2eTests() { - if (!Program.RunningE2eTests) + if (!Program.RunningE2eTests || BookUpload.UseSandbox) return false; _webSocketProgress.MessageWithoutLocalizing( - "Uploading is disabled while Bloom is running e2e tests (--e2e).", + "Uploading to bloomlibrary.org is disabled while Bloom is running e2e tests (--e2e); an automated run may upload only to dev.bloomlibrary.org.", ProgressKind.Error ); return true; diff --git a/src/BloomFreezeDoctor.Core/Outbox/YouTrackSubmitter.cs b/src/BloomFreezeDoctor.Core/Outbox/YouTrackSubmitter.cs index cfd6fa9ee01f..22f52d8a0277 100644 --- a/src/BloomFreezeDoctor.Core/Outbox/YouTrackSubmitter.cs +++ b/src/BloomFreezeDoctor.Core/Outbox/YouTrackSubmitter.cs @@ -916,10 +916,16 @@ private static string ReadReport(QueuedBundle bundle) } } - private static string Describe(TimeSpan age) => - age.TotalDays >= 1 ? $"{age.TotalDays:F0} day(s)" - : age.TotalHours >= 1 ? $"{age.TotalHours:F0} hour(s)" - : $"{age.TotalMinutes:F0} minute(s)"; + private static string Describe(TimeSpan age) + { + if (age.TotalDays >= 1) + return $"{age.TotalDays:F0} day(s)"; + + if (age.TotalHours >= 1) + return $"{age.TotalHours:F0} hour(s)"; + + return $"{age.TotalMinutes:F0} minute(s)"; + } private static string Trim(string value) => value.Length <= 300 ? value : value.Substring(0, 299) + "…"; diff --git a/src/BloomFreezeDoctor.Protocol/DoctorSignals.cs b/src/BloomFreezeDoctor.Protocol/DoctorSignals.cs index 3f336c7bae0e..04da78eefb6c 100644 --- a/src/BloomFreezeDoctor.Protocol/DoctorSignals.cs +++ b/src/BloomFreezeDoctor.Protocol/DoctorSignals.cs @@ -59,7 +59,7 @@ public static string QuitRequestName(int processId) => /// Polling stays as the backstop, and must: a Bloom too old to know about any of this cannot announce /// itself, and those are the Blooms most worth watching. /// - public static string BloomStartedName() => @"LocalBloomFreezeDoctor.bloomstarted"; + public static string BloomStartedName() => @"Local\BloomFreezeDoctor.bloomstarted"; /// Set by Bloom as it dies, to ask for a dump while the process still exists. public static string DumpRequestName(int processId) => diff --git a/src/BloomTests/BloomSettingsProviderTests.cs b/src/BloomTests/BloomSettingsProviderTests.cs new file mode 100644 index 000000000000..b428fde0119a --- /dev/null +++ b/src/BloomTests/BloomSettingsProviderTests.cs @@ -0,0 +1,180 @@ +using System; +using System.Configuration; +using System.IO; +using Bloom; +using Bloom.Properties; +using NUnit.Framework; +using SIL.TestUtilities; + +namespace BloomTests +{ + /// + /// BloomSettingsProvider keeps user.config where libpalaso's provider does, unless a folder is + /// named (--user-settings-folder), in which case that folder is used instead. These tests drive + /// the provider directly, the way ApplicationSettingsBase does, so they need no Bloom running + /// and touch no real settings file. + /// + [TestFixture] + public class BloomSettingsProviderTests + { + private const string kGroupName = "Bloom.Properties.Settings"; + private TemporaryFolder _folder; + + [SetUp] + public void Setup() + { + // Settings.Default constructs its providers the first time any setting is read, and a + // provider fixes its location when it is constructed. Make sure that has already + // happened, so that naming a folder below moves only the providers these tests + // construct, never the test run's own settings. + var _ = Settings.Default.ShowExperimentalFeatures; + _folder = new TemporaryFolder("BloomSettingsProviderTests"); + } + + [TearDown] + public void TearDown() + { + BloomSettingsProvider.SetUserSettingsFolder(null); + _folder.Dispose(); + } + + [Test] + public void GetUserSettingsFolder_NoFolderNamed_IsThePerVersionFolderUnderLocalAppData() + { + BloomSettingsProvider.SetUserSettingsFolder(null); + + var folder = BloomSettingsProvider.GetUserSettingsFolder(); + + Assert.That( + folder, + Does.StartWith( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) + ) + ); + // libpalaso names the last folder after the entry assembly's version. + Assert.That( + Path.GetFileName(folder), + Is.EqualTo( + System.Reflection.Assembly.GetEntryAssembly().GetName().Version.ToString() + ) + ); + } + + [Test] + public void GetUserSettingsFolder_FolderNamed_IsThatFolder() + { + BloomSettingsProvider.SetUserSettingsFolder(_folder.Path); + + Assert.That(BloomSettingsProvider.GetUserSettingsFolder(), Is.EqualTo(_folder.Path)); + Assert.That( + BloomSettingsProvider.GetUserConfigPath(), + Is.EqualTo(Path.Combine(_folder.Path, "user.config")) + ); + } + + [Test] + public void CommandLineArgumentsForChildBloom_NoFolderNamed_IsEmpty() + { + BloomSettingsProvider.SetUserSettingsFolder(null); + + Assert.That(BloomSettingsProvider.CommandLineArgumentsForChildBloom, Is.Empty); + } + + [Test] + public void CommandLineArgumentsForChildBloom_FolderNamed_NamesItForTheChild() + { + BloomSettingsProvider.SetUserSettingsFolder(_folder.Path); + + Assert.That( + BloomSettingsProvider.CommandLineArgumentsForChildBloom, + Is.EqualTo($"--user-settings-folder \"{_folder.Path}\"") + ); + } + + [Test] + public void SetPropertyValues_FolderNamed_WritesUserConfigThereAndReadsItBack() + { + BloomSettingsProvider.SetUserSettingsFolder(_folder.Path); + var userConfig = Path.Combine(_folder.Path, "user.config"); + Assert.That(File.Exists(userConfig), Is.False, "test setup: the folder starts empty"); + + var property = MakeStringProperty(); + var context = MakeContext(); + + var writer = new BloomSettingsProvider(); + writer.Initialize(null, null); + writer.SetPropertyValues( + context, + new SettingsPropertyValueCollection + { + new SettingsPropertyValue(property) { SerializedValue = "fr" }, + } + ); + + Assert.That( + File.Exists(userConfig), + Is.True, + "user.config was not written to the named folder" + ); + Assert.That( + File.ReadAllText(userConfig), + Does.Contain(" + /// The settings provider is keeping user.config in libpalaso's per-version folder under + /// %LOCALAPPDATA%, not in any folder a command line named. + /// + private static void AssertProviderIsOnTheUsualFolder() + { + Assert.That( + BloomSettingsProvider.GetUserSettingsFolder(), + Does.StartWith( + System.Environment.GetFolderPath( + System.Environment.SpecialFolder.LocalApplicationData + ) + ) + ); + } + + [Test] + public void ParseStartupPortArguments_RejectsUserSettingsFolderWithoutValue() + { + var remainingArgs = Program.ParseStartupPortArguments( + new[] { "--user-settings-folder" }, + out var errorMessage + ); + + Assert.That( + errorMessage, + Is.EqualTo("Bloom requires a value after --user-settings-folder.") + ); + Assert.That(Program.StartupUserSettingsFolder, Is.Null); + AssertProviderIsOnTheUsualFolder(); + Assert.That(remainingArgs, Is.Empty); + } + + [Test] + public void ParseStartupPortArguments_RejectsEmptyUserSettingsFolder() + { + var remainingArgs = Program.ParseStartupPortArguments( + new[] { "--user-settings-folder=" }, + out var errorMessage + ); + + Assert.That( + errorMessage, + Does.StartWith("Bloom cannot use \"\" as the --user-settings-folder") + ); + Assert.That(Program.StartupUserSettingsFolder, Is.Null); + AssertProviderIsOnTheUsualFolder(); + Assert.That(remainingArgs, Is.Empty); + } + + [Test] + public void ParseStartupPortArguments_RejectsDuplicateUserSettingsFolders() + { + var remainingArgs = Program.ParseStartupPortArguments( + new[] { @"--user-settings-folder=C:\one", @"--user-settings-folder=C:\two" }, + out var errorMessage + ); + + Assert.That( + errorMessage, + Is.EqualTo("Bloom only accepts one --user-settings-folder argument.") + ); + Assert.That(Program.StartupUserSettingsFolder, Is.Null); + AssertProviderIsOnTheUsualFolder(); + Assert.That(remainingArgs, Is.Empty); + } + + [Test] + public void ParseStartupPortArguments_GivesTheProviderNoFolderWhenALaterArgumentIsBad() + { + // A valid folder before a bad argument must not reach the settings provider: the launch + // is rejected, and startup must not open that folder's user.config before reporting the + // error. + Program.ParseStartupPortArguments( + new[] { @"--user-settings-folder=C:\valid", "--vite-port", "70000" }, + out var errorMessage + ); + + Assert.That(errorMessage, Is.Not.Null); + Assert.That(Program.StartupUserSettingsFolder, Is.Null); + AssertProviderIsOnTheUsualFolder(); + } + + [Test] + public void ParseStartupPortArguments_GivesTheProviderNoFolderWhenExperimentalFeaturesLackE2e() + { + // The experimental-features check rejects the launch after the loop, so a folder the + // loop already accepted must not reach the provider either. + Program.ParseStartupPortArguments( + new[] + { + @"--user-settings-folder=C: alid", + "--experimental-features", + "team-collections", + }, + out var errorMessage + ); + + Assert.That( + errorMessage, + Is.EqualTo("Bloom only accepts --experimental-features together with --e2e.") + ); + Assert.That(Program.StartupUserSettingsFolder, Is.Null); + AssertProviderIsOnTheUsualFolder(); + } + // --- IsBenignUnobservedTaskSocketNoise: the Sentry BeforeSend filter for // BLOOM-DESKTOP-EQ4 / -E4J / -E9K --- diff --git a/src/BloomVisualRegressionTests/index.spec.ts b/src/BloomVisualRegressionTests/index.spec.ts index 46e71a4a4b17..f97c5f02719d 100644 --- a/src/BloomVisualRegressionTests/index.spec.ts +++ b/src/BloomVisualRegressionTests/index.spec.ts @@ -913,7 +913,18 @@ async function launchDedicatedBloom() { // --e2e: skip the DEBUG "Attach debugger now" prompt and suppress modal error dialogs so a // Bloom problem fails the test instead of hanging the run. --automation: allow this instance to // run alongside a Bloom the developer already has open (bypasses the single-instance token). - bloomProcess = execFile(exe, [collection, "--e2e", "--automation"]); + // --user-settings-folder: keep this Bloom's user settings (user.config) in the temp folder, so + // the run starts from default settings, as it does on a fresh CI runner, rather than from + // whatever the developer's Bloom of the same version saved last, and leaves nothing behind. + const userSettingsDir = Path.join(tempCollectionsRoot, "user-settings"); + fs.mkdirSync(userSettingsDir); + bloomProcess = execFile(exe, [ + collection, + "--e2e", + "--automation", + "--user-settings-folder", + userSettingsDir, + ]); // Capture Bloom's output and watch for an early exit. Without this a launch failure (crash on // startup, missing WebView2 runtime, first-run dialog) is invisible: the poll below just runs // out the full 90s and reports "seen: none" with no clue why. Echo to our own stderr too so the