fix(upload): believe a server that will not take a book

A token may hold library-upload while every folder on the server is
closed. The worker gave up on that, quietly, and left the capability
standing — so the app went on offering to send books to a server that
would never accept one, failing once per book with nothing said. An
emulator run showed fifteen books failing in silence.

A folder that will not have it and a refusal on the wire are the same
answer to a reader, so both now clear the capability and both take the
same way back: signing in again is what reads the server's mind afresh.

hack/e2e-upload grows -r for the refusing half of the capability, which
is how this was found. It covers both places a server can say no: the
scope, refused on sight, and the folder, which can only be learnt by
trying. Both must end with the app no longer offering, nothing linked,
nothing lost from the shelf and nothing written into the folder.
This commit is contained in:
Chmouel Boudjnah 2026-08-18 21:55:13 +02:00
parent 6b64589d1f
commit ee7d467e8e
3 changed files with 103 additions and 10 deletions

View file

@ -92,7 +92,25 @@ It asserts four things, and each one is a bug that actually happened:
the `library-upload` scope reaches the app as `can_upload`, the files
appear in the folder, no uploaded book had its `url` rewritten (that
key is what every reading position hangs off), and no book came back
from the following catalog pass as a second row.
from the following catalog pass as a second row. That third one is
worth the whole script: a book uploaded from the device was being
deleted by the next refresh, and its reading position with it.
The other half of the capability is refusing, and `-r` checks it:
```bash
hack/e2e-upload -r -u http://10.0.2.2:8686 -t <token> -d /srv/books
```
A server says no in two places. A token without the scope is refused on
sight, and the action is never offered. A token that holds the scope but
finds every folder closed can only be refused by trying, and the app has
to remember the answer — otherwise it offers, once per book, an action
that silently fails every time. Either way `-r` asserts the app ends up
not offering, no book was linked, none left the shelf and nothing was
written into the folder. Run it with the folder still closed, then
`folder-uploads <folder-id> on` and run the check above, and you have
covered both answers with one server.
Nothing about it is mocked. Start with a clean shelf — `hack/reset-books`
then `adb shell pm clear com.chmouel.liseur` — or the counts it compares

View file

@ -40,7 +40,7 @@ class BookUploadWorker(
?: runCatching { uploader.targets(server.baseUrl, credentials) }
.getOrElse { return retryAfter("could not list upload folders: ${it.message}") }
.firstOrNull()?.folderId
?: return giveUp("no folder accepts uploads")
?: return refuse(container, "no folder accepts uploads")
val (file, temporary) = localFile(book.localUri ?: book.url)
?: return giveUp("no readable file for $bookUrl")
@ -62,10 +62,7 @@ class BookUploadWorker(
// learnt: the server keys on the book's digest, so the
// second ask is recognised rather than stored twice.
ServerUploadResult.Pending -> retryAfter("$bookUrl uploaded, not catalogued yet")
ServerUploadResult.NotAllowed -> {
container.database.remoteServerDao().setCanUpload(false)
giveUp("server refused the upload")
}
ServerUploadResult.NotAllowed -> refuse(container, "server refused the upload")
ServerUploadResult.TooLarge -> giveUp("$bookUrl is larger than the server takes")
ServerUploadResult.Rejected -> giveUp("the server would not read $bookUrl")
is ServerUploadResult.Failed -> retryAfter("upload failed: ${result.message}")
@ -150,6 +147,21 @@ class BookUploadWorker(
return Result.failure()
}
/**
* Gives up, and stops the app offering to send this server a book.
*
* A token that may upload and a server where no folder will have
* one are the same answer to a reader. The capability is what every
* entry point is drawn from, so leaving it standing offers an
* action that can only fail — silently, once per book, for as long
* as they keep asking. Signing in again is what reads the server's
* mind afresh, exactly as it is for a refusal on the wire.
*/
private suspend fun refuse(container: com.chmouel.liseur.AppContainer, why: String): Result {
container.database.remoteServerDao().setCanUpload(false)
return giveUp(why)
}
private fun retryAfter(why: String): Result {
Log.w(TAG, why)
return Result.retry()

View file

@ -14,8 +14,12 @@
# -d the watched folder's root on this machine, so the script can see
# what landed in it
# -s pick a device when several are attached
# -r expect the server to refuse: assert the app never offers to
# upload, rather than that it succeeds. Use with a token without
# the library-upload scope, or a server whose folders are all
# closed.
#
# The folder must already accept uploads:
# Without -r the folder must already accept uploads:
#
# liseur-sync admin folder-uploads <folder-id> on
#
@ -31,9 +35,10 @@ readonly MAIN_ACTIVITY="$APP_ID/.MainActivity"
# shellcheck source=hack/lib/demo-books.sh
source "$(dirname "${BASH_SOURCE[0]}")/lib/demo-books.sh"
serial="" url="" token="" root=""
serial="" url="" token="" root="" refused=""
while [[ $# -gt 0 ]]; do
case $1 in
-r | --refused) refused=yes ;;
-s | --serial) serial=${2:-}; shift ;;
-u | --url) url=${2:-}; shift ;;
-t | --token) token=${2:-}; shift ;;
@ -49,7 +54,9 @@ done
[[ -n "$url" ]] || die "need -u, the server address as the device sees it"
[[ -n "$token" ]] || die "need -t, a device token with the library-upload scope"
[[ -d "$root" ]] || die "need -d, the watched folder's root on this machine"
[[ -n "$root" || -n "$refused" ]] ||
die "need -d, the watched folder's root on this machine"
[[ -z "$root" || -d "$root" ]] || die "$root is not a directory"
command -v adb >/dev/null || die "adb is not installed"
command -v python3 >/dev/null || die "python3 is not installed"
@ -113,7 +120,8 @@ tap_button() {
tap_on "$@"
}
before=$(find "$root" -name '*.epub' | wc -l | tr -d ' ')
before=0
[[ -n "$root" ]] && before=$(find "$root" -name '*.epub' | wc -l | tr -d ' ')
step "Connecting to $url"
open_library
@ -151,6 +159,61 @@ done
note "connected"
can_upload=$(query "select can_upload from remote_server;")
# A server that will not take a book has to be believed, and it can say
# so in two places: the token may lack the scope, which the app learns
# on sight, or every folder may be closed, which it can only learn by
# trying. Both have to end with the app no longer offering, because the
# capability is what every entry point is drawn from and an offer that
# can only fail is one the reader is left to repeat.
if [[ -n "$refused" ]]; then
step "Checking the app takes no for an answer"
shelved=$(query "select count(*) from books;")
hide_keyboard
back
sleep 2
back
sleep 3
if [[ "$can_upload" == 1 ]]; then
note "the token holds the scope, so only an attempt can settle it"
tap_if_there "Send" exact || true
waited=0
while ((waited < 120)); do
[[ $(query "select can_upload from remote_server;") == 0 ]] && break
sleep 5
waited=$((waited + 5))
done
else
note "the app read the token as not allowed to upload"
! on_screen "Send" ||
die "the app offered to send books to a server that refuses them"
fi
can_upload=$(query "select can_upload from remote_server;")
[[ "$can_upload" == 0 ]] ||
die "the app still thinks it may upload (can_upload=$can_upload)"
note "the app stopped offering"
linked=$(query "select count(*) from books where remote_uuid is not null;")
[[ "$linked" == 0 ]] ||
die "$linked books were linked to a server that takes no uploads"
now=$(query "select count(*) from books;")
[[ "$now" == "$shelved" ]] ||
die "the shelf went from $shelved books to $now; a refusal must cost nothing"
note "all $now books are still on the shelf"
if [[ -n "$root" ]]; then
after=$(find "$root" -name '*.epub' | wc -l | tr -d ' ')
[[ "$after" == "$before" ]] ||
die "$((after - before)) books were written into a folder that refuses them"
note "nothing was written into $root"
fi
printf '\nDone. The server refused, and the app stopped offering.\n'
exit 0
fi
[[ "$can_upload" == 1 ]] ||
die "the app read the token as not allowed to upload (can_upload=$can_upload)"
note "the library-upload scope reached the app"