Newtonsoft applies values in the order the document lists them, so a settings
file naming AdditionalLocaleNames before its IdentityTokens has no registered
marketplace to compare against yet, and the duplicate check on the way in does
nothing. That account would then scan one marketplace twice.
Filtering on read makes the order irrelevant.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
The accounts grid gains a Marketplaces button per row, enabled once the account
has credentials to check with, and a dialog that asks each marketplace what it
holds and lets the user tick the ones to scan. The scan picker now lists every
marketplace an account reads, since one checkbox there can scan several.
A marketplace that could not be reached is reported as unchecked rather than
empty. Calling it empty would recreate the exact silence this feature exists to
break: titles present, and nothing anywhere in the app to suggest it.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
A title bought while an Amazon address was briefly set to another country stays
in that country's library for good, and a scan of the account's own marketplace
never sees it - no error, no warning, the titles are simply absent. The only way
to reach them was to add the same login a second time under the other
marketplace, which meant knowing it was there and logging in again.
An account now records the further marketplaces it should read, and a scan reads
all of them as a unit. MarketplaceProbe finds them: one request per marketplace,
in sequence, and only when asked for.
Nothing about stored credentials moves. Encrypted tokens are bound to their
marketplace by AES-GCM associated data built from Identity.LocaleName, and the
identity is located in the file by that same name, so both keep resolving to the
registration that already exists. The extra marketplaces are names alone, and are
left out of the file entirely when there are none - so an account with one
marketplace, which is nearly all of them, writes the file it always did.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
PreLoggingCrashLog returned its LongPath straight out, so on Windows the crash
dialog named \\?\C:\Users\...\Log202608.log. The prefix is for the Win32 API,
not for a person being asked to find the file and attach it to a bug report.
Return PathWithoutPrefix instead, and use it for the LibationFiles line inside
the record for the same reason. A test now pins that the reported path carries
no prefix and is usable as-is; on Linux the two forms are identical, which is
why local runs could not see this and the Windows CI job could.
While in there: ReplaceInstallFile moved the old file aside and then wrote the
new one, so a copy that failed after the move left nothing at all where a file
used to be. Put the old one back before reporting the failure.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
The rollback restored files, announced 'Libation restored your previous install
files', and deleted the pending marker without ever reading back what it had
written. It also could not tell the difference between an overlay that had not
started, where putting the backup back returns the install to one version, and
an overlay that got partway, where the backup covers a dozen names out of the
few hundred in the folder and cannot.
Grade the outcome instead. Verify each restored file against its backup, and
use the manifest files that did match the upgrade package as the signal that
the overlay had begun. Report which files could not be restored, and keep
restoring the rest rather than abandoning them at the first failure.
Libation has to close either way, since the files underneath it just changed,
so the wording and the offer now follow the grade: back on the previous version
asks to restart, a mixed install asks but says a fresh install would be better,
and a restore that could not finish only offers OK. A relaunched process never
offers again, so a pending marker we failed to delete cannot become a loop.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Parsing HTML belongs before the data reaches the data layer, not in it. Move the
flattening to AudibleUtilities, where the rest of the Item translation lives, so
Book.Description holds plain text and DataLayer no longer references
HtmlAgilityPack.
Import now updates a description as well as creating one, which it never did
before: books imported while descriptions were stored as raw HTML are still
holding markup, and a re-scan is what clears it. A scan that reports no summary
leaves the stored one alone.
This also cleans up the CSV/Excel export, which was emitting raw HTML too.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
The rollback test holds the file it restores open with the sharing mode .NET
uses for a loaded assembly. Against the old File.Copy(overwrite: true) it fails
with 'the process cannot access the file because it is being used by another
process', which is the same refusal Windows gives for a loaded assembly and the
reason the rollback could never finish.
The rest pin the behaviour that was missing: recovery survives a log sink that
throws on every call, a displaced file is swept up later, a stale or absent
install assembly is recognised and both versions are read, a plain file path is
not mistaken for an assembly reference so the EF Core message keeps its own
wording, and the crash record appends to the newest Log*.log and reports the
path it used.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
CsvHelper was only used to write headers and flat DTO rows in the csv
exports. The new internal writer reproduces its output byte-for-byte:
the delimiter is the culture's list separator, records end with CRLF,
fields containing the delimiter, quotes, newlines, or leading/trailing
spaces are quoted per RFC 4180, and values are formatted with the
culture. The [Name]/[Ignore] attributes it replaced are now two tiny
local attributes.
Verified byte-identical output against CsvHelper 33.1.0 for both
exporters in en-US, de-DE (';' delimiter), and invariant cultures, and
for a seeded 30-book demo library export before/after.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
FillMissingTags is driven against a real MetadataItems built from a hand-rolled
ilst box: empty, as a Widevine download arrives, and pre-populated, as an .aaxc
download arrives. Reverting any of the three fixes fails a test with the exact
value from the issue report.
MockLibraryBook.CreateBook gains a copyright knob to match UpdateBookDetails.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
When Libation cannot install an upgrade itself - a portable or AppImage
install, a Linux build with no package-manager symlink, macOS outside
/Applications - the prompt is a notice with a download link, and Chardonnay
relabels its button from "Yes" to "OK" to say so. The button still closed with
DialogResult.OK, which MainWindow and the About window both read as "yes,
install it", so acknowledging the notice downloaded the release and ran the
auto-upgrader anyway. That could only fail, and it failed loudly: an "Upgrade
Failed" error box in answer to dismissing a notice.
The button now closes with Cancel when there was no upgrade on offer, so the
answer matches the question. Classic already did the equivalent, hiding its Yes
button and relabelling No.
UpgraderBase gets the matching guard. It already refused to install under
Windows Application Control on the grounds that a UI ignoring CapUpgrade must
not be able to start an upgrade that leaves Libation unable to start; that
reasoning covers every capped upgrade, not just the Windows one, so the stop is
now keyed on CapUpgrade itself via MayInstallUpgrade.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Libation asked GitHub for a newer release every time it started, with no way
to stop it. That is noise for anyone whose install is updated by something
else - a package manager, or an AppImage updater - because the prompt it
raises is one they can do nothing useful with.
Add CheckForUpgradesAtStartup, on by default so nothing changes for people
who rely on the prompt. Only the automatic check is optional: the About
window's "Check for Upgrade" button and the CLI's `version --check` ask for a
check outright, so they run either way. That is why the setting is read in a
new CheckForUpgradeAtStartupAsync rather than inside CheckForUpgradeAsync,
which the startup path and the About button share.
The new setting takes the slot of BetaOptIn, which is deleted here. It was
declared, described and logged, but no axaml or designer ever bound it and
nothing read the value: GetLatestRelease only ever asks for the stable
release, so there was no beta channel for it to select. A stale BetaOptIn key
in an existing Settings.json needs no migration, since PersistentDictionary
ignores keys with no matching property.
Closes#1999
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
The status this flag sets is now called 'Download Pending', so -p /
--download-pending is the name the help offers.
-n / --not-downloaded keeps working. It is what years of scripts, forum
answers and issue comments tell people to run, so breaking it would cost
more than the inconsistency is worth. It is hidden from --help so the new
name is the only one advertised, and both flags feed one SetPending
property that the verb acts on.
All three names stay in the 'Download Status' option group, so 'at least
one status flag is required' still holds. That error message does
enumerate the group, which is the one place the legacy name still
surfaces.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Stephen King's 11/22/63 sits either side of the line the number handling draws.
The analyzer keeps a date like this as one token, so the title is text and must
not be zero-padded, while "title:11*" is a bare number in a text field and must
not be padded either - that one fails against the code from before the numeric
fix, going looking for "title:00000011.00*".
Audible lists it with slashes and a hyphenated spelling turns up elsewhere, so
both are covered, as are books 11, 22 and 63 minutes long so that nothing can
pass by matching a number field instead of the title.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
It was a generic primer whose examples used title, body and mod_date. Only one
of those exists here, so half the guide could not be typed into the search box.
Every example is now a real field and was run against an index before being
written down, including the exclusive range and the leading-wildcard error the
old text described but never showed.
Covers the two forms that reach the new number handling and had no test: a
wildcard on a numeric title, and a curly-brace range.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
A bare number has to keep every book it already matched and add the one whose
title is that number. The library had a book 14 minutes long but none 14 hours
long, so the Hours field went untested - the same field that made an earlier
draft of these tests pass against the bug by accident.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Lucene 3 has no numeric type, so a number field is indexed zero-padded to make
a range sort: 600 minutes is stored as "00000600.00". Every other field keeps
the number as written, so the novel "1984" is stored as "1984". Padding every
number in a query regardless of the field it was being compared against made a
numeric title unfindable - "title:1984" looked for "00001984.00" among the
titles - and a bare "1984" could only ever mean "some number field is 1984".
A number now takes the spelling its field is indexed in. With no field named,
the default field holds both spellings, so both are searched. A range and a
phrase cannot hold that disjunction: a range stays padded, a phrase does not.
Expanding a bare number to a group exposed the pure-negation workaround, which
judged a query by every clause in its tree, so "-(a OR b)" was read as mixed
and returned nothing. It is now decided per query by that query's own clauses.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Search field names are ordinary words - absent, podcast, plus, series - and
a book's own text is full of them. The sanitizer read any known field name as
a field name wherever it appeared, so a value in that position picked up the
implied ":True" of a bool field: "title:absent" became "title:absent:True",
which Lucene refuses to parse, and "title:\"absent friend\"" quietly matched
nothing. A colon straight after a field name now marks the next term as that
field's value.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
A book the index does not hold cannot be found by any positive filter, and
every negated filter - which resolves to every document in the index - drops
it from the grid instead. That is why issue #1989 reads as half a working
filter: Absent found nothing while -Absent removed exactly the absent books.
The index is only ever written as a whole, and a rebuild that fails is
deliberately swallowed so a bad index cannot fail a good scan, so a short
index stayed short until something else changed the library. Count the
index against the library once per run and rebuild when it is short.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Both halves of the Add notification were wrong. The index was read after the range
had been added, so it pointed past the end by the size of the batch - queueing two
books into a list of four announced them at index 6. And the parameter is IList<T>,
which does not implement the non-generic IList, so the compiler bound the
changedItem overload and the event named the list object itself as the single item
added rather than the books in it.
Both UIs survived it: WinForms discards the event and re-reads, and Avalonia
evidently falls back to re-reading too. But this class exists to give an index-based
consumer something it can follow, and this was the one notification it could not.
The list is also copied now, so the event does not hand out a reference the caller
can still mutate.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
With several books in flight, the book whose dialog was answered showed
"Cancelled" while some unrelated book showed "Error, Abort". Tearing the queue
down is claimed by whichever book finishes first, and the books genuinely are
interchangeable for that. The status left on a row is not: it is read afterwards
by someone who remembers which book they were asked about.
The two jobs are now separate. ShowRetryDialogAsync records the answering book as
BadBookSessionContext.AbortOriginator, written before Override so that a book
reading Override and racing ahead to the queue loop cannot find the originator
still unset and conclude there was none. The loop gives the abort to that book and
Cancelled to every book that inherited the answer. ClaimAbort is unchanged and
still decides the teardown.
With no dialog in play - Bad Book set to Abort in settings - nobody answered
anything, there is no originator, and the book that claimed the teardown keeps the
abort as before.
"(4 on this machine)" sits in whatever the two spinners leave of a fixed 400px
pane. That fit on one machine and ellipsed to "(4 on this machi..." on another,
which left the hint existing only in the tooltip on exactly the machines it was
written for.
Now "(4 at a time)": six characters shorter, and short enough to fit that column
with room over rather than by a hair. Why the number is smaller than the setting
is what the tooltip is for; the number itself is what has to be legible. Classic
reads the same string into its tooltip, so both UIs move together.
Layout geometry is untouched on purpose - the spinner widths and the row structure
are the part that was reported as clean.
DispatchPending held dispatchLock across delivery. With NotificationInvoker null,
Deliver runs inline on the mutating thread, so a handler that blocks - WinForms'
RefreshDisplay does - sat inside that lock while the UI thread it was waiting on
blocked trying to enter the same lock to deliver its own mutation. Each waiting on
the other. Unreachable while both UIs set the invoker, but it should not be one
assignment away from reachable.
The claim is now a _draining flag, set under lockObject in the same critical
section that takes the batch. One thread at a time delivers, and a thread that
finds the flag set returns immediately rather than blocking, leaving its
notifications for the draining thread's next pass. Delivery order is unchanged:
the drainer re-checks _pending after clearing the flag, under the same lock that
pending is appended beneath, so a thread that left without delivering cannot lose
what it pended.
A flag is also not reentrant the way a lock is, so a handler that mutates the
queue can no longer deliver its own notification ahead of the rest of the batch it
is standing in.
Both asserted queue.QueuedCount, which is the view model's mirror of the queued
count rather than the queue itself. It arrives through the posted notification
path, and TestInitialize installs a bare SynchronizationContext that posts to the
thread pool - so the assert raced delivery and lost most of the time. Five of six
runs red locally; on CI, six of nine build jobs reached the unit test step and all
six failed here.
They now assert queue.Queue.Count, which is read under the queue's own lock and is
already true by the time the loop returns. Polling it with the Patience deadline
would have worked too, but there is nothing left to wait for once the assertion
reads the structure instead of its mirror.
No product code changes. QueuedCount is delivered through the posted path
deliberately - that is what keeps a bound list in mutation order.
LongPath rewrites '/' to '\' on Windows, so comparing against hardcoded unix
paths failed there. The test is about which entries survive a truncated walk
and in what order, not about how they are spelled.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
SaferEnumerateFiles returned a lazy sequence, so an I/O error was raised where
the sequence was walked rather than where it was created - past the try/catch
callers had wrapped around it. IgnoreInaccessible did not help either: it only
forgives permissions, not a volume that has stopped answering.
Walk the enumerator defensively instead, keeping what was read and reporting
the reason, and let a caller ask whether a directory can be read at all.
See issue #1984.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
^(a+)+$ against 22 a's is only ~2x the 100ms match timeout, so whether
CatastrophicBacktracking_AnchoredRepeated passes depends on how fast the machine
is: a fast box completes the match, nothing throws, and the row fails its own
Assert.Fail. Every added character doubles the work, so 50 a's puts it in the
same never-completes class as the other evil-regex rows, the same treatment
9ed98cbf already gave (a+a+)+b.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
The gate read a queue-wide cancelAllRequested flag, which then had to be cleared
again - and whatever clears it can un-cancel a book that is still parked. Clearing
it at the end of the cancellation drain was the window you described; scoping it to
the run moved that window into AddToQueue, which cleared it so newly queued books
would not inherit an old cancellation. Same bug, different trigger.
WaitForDailyLimitAsync already waits on behalf of one specific book, so it now asks
whether that book was cancelled. Cancel All reaches a parked book through
ProcessBookViewModel.CancelAsync like any other active book - it is on the active
list, having been dequeued before the gate - so nothing has to be set or cleared at
the queue level and the flag is gone entirely, along with both of its clear sites.
Books queued afterwards are untouched by construction rather than by timing.
CancelAsync records the request before its early return, because a book that has
not started a step is precisely the case that matters. That also makes cancelling
a single parked book work, which it previously did not.
EffectiveConcurrentDownloads clamps the setting by Environment.ProcessorCount, so
a test asking for three books at once starts only two on a small machine, waits
out its ten second patience and fails. Reproduced with DOTNET_PROCESSOR_COUNT=2:
three failures and a thirty-four second run, against five seconds and none on a
sixteen core box. The CI matrix includes runners small enough to hit it.
Machine capability is now overridable, and the dispatch tests pin it to the
concurrency they are asking about. The clamp itself is unchanged in the app.
Two tests for the seam while it is there, since neither the point-of-use clamp
nor the hint had any coverage: that a machine smaller than the setting holds the
loop down without rewriting what the user asked for, and that ConcurrencyHint
says so and falls silent when the machine can keep up.
The docs page was linked from docs/index.md but missing from the VitePress sidebar, where every sibling feature page is listed.
In the dispatch test fake, Started.Enqueue ran before the counter incremented, so WaitForStarted could return in the gap and the HighWaterMark assertion could observe one book fewer than had started.
Comments still named TrackedQueue.Current and .Active after both were deleted, the Configuration doc comment still called itself the bound on a spinner when nothing binds it any more, and the ClaimAbort comment said the winner was the book that answered the dialog when it is really whichever book gets there first.
All the risk parallel downloads added lives in the dispatch loop, and none
of it was reachable while the only way to run a book was to download one.
Driven through ProcessBookHandler, every book here finishes when the test
says so and never touches the network, the database or the disk. The loop
is the real one.
Covers the capacity cap, including lowering it mid-run while more books
than the new cap are still in flight; the enqueue signal, both for books
queued into free slots and for one arriving as the loop winds down; the
abort drain, that it clears the queue and the loop still comes back rather
than dying inside it; that only the book which answered Abort claims it
while the rest report Cancelled; that a faulted book task is observed
instead of taking the loop out through its outer catch; and Cancel All.
The abort test runs one book at a time on purpose. With more, a book
finishing in the same instant as the abort lets the loop take one more off
the queue before the queue is cleared - the narrow window noted in the
abort commit - and that is not what this test is about.
Deleted rather than documented: TrackedQueue's Current, Active,
ClearCurrent() and MoveNext(). The first three had no callers at all, and
MoveNext() had exactly one - the Avalonia design-mode preview, which now
builds its sample queue from TryDequeueNext and MarkCompleted, the same
calls the dispatch loop makes. The "legacy / kept for compatibility" labels
pointed at callers that stopped existing when the loop stopped being
sequential.
TrackedQueue.Current's doc comment cited the speed limit display as its
reason to exist, which stopped being true when the speed limit moved to
GetActive(). SpeedLimit itself now records why the sequential loop's
per-book read-back was dropped rather than leaving it unexplained: with
several books running there is no single book to read it back from, and
doing so would have whichever started last overwrite what the user typed.
Restored the UTF-8 BOMs on ProcessQueueViewModel.cs and
BadBookSessionContext.cs, and put back the eight "// " separators in the
WinForms designer that had been rewritten to "//". That file's diff is now
identical with and without whitespace, so everything left in it is a real
change.
The existing tests all assert on inline delivery, which is the path the app never
takes. Adds a fake ISynchronizeInvoke that only queues, so a test can prove
nothing was delivered inline and then run delivery itself, in order, the way the
UI thread would.
Two tests on top of it: that a mutation made on the calling thread is still
posted rather than run inline, and that four books completing on four real
threads leave a Move-following bound list identical to the queue, over 50 runs.
Plus a test that completing a book which is not active changes nothing.
All three Windows CI legs failed on master while the other six passed, and not on
an assertion: every test in FileLiberator.Tests' PDF path suite failed in
TestInitialize with an AggregateException wrapping FileNotFoundException, naming a
path none of those tests had anything to do with.
The watcher had raised Created for a folder an earlier test's cleanup then
deleted. AddPath asked whether the path existed, was told yes, asked what it was,
and got an exception - Exists and GetAttributes can disagree over a long \\?\
path, and the answer to the first can stop being true before the second is asked
anyway. That exception ended the background scanner, so nothing further reached
the cache, and it was stored on the task, so the next Stop() rethrew it as an
AggregateException at whoever had called Refresh(). In the app that caller is the
Books directory refresh after every download.
Three changes, smallest first: the attribute read is guarded and returns 'nothing
to add' where the existence check used to say it, which also removes the race
rather than narrowing it; the scanner survives an event it cannot apply; and
Stop() waits on a scanner that has already failed without handing the failure to
a caller that is about to replace it.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Commit 065118cf also deleted migrate_to_v11_6_5, which clamped
MaxSampleRate into LAME's supported [Hz_8000, Hz_48000] range on every
startup (the fix for #1116). Its replacement, ValidateEnumSettings, only
rejects values that fail to parse - but AAXClean.SampleRate defines
Hz_7350, Hz_64000, Hz_88200 and Hz_96000, so a hand-edited or
pre-v11.6.5 Settings.json could carry a valid-but-unsupported rate
straight into the encoder.
Clamp in the property getter and setter instead of a startup hook,
following the DailyDownloadLimitQuantity pattern.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Commit 065118cf deleted Migrations.migrate_to_v11_5_0, which despite its
name ran on every startup and was the only code that read
QuickFilters.json into QuickFilters.InMemoryState. Since v13.7.6, saved
quick filters were never loaded after a restart, and adding a new filter
overwrote the file.
Make QuickFilters load its state lazily from disk on first access so it
no longer depends on a startup hook. Restore the pre-v11.5.0 format
fallback (plain string filters without names) that the deleted migration
provided, and add regression tests covering load formats and
restart persistence.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
