Files
bambuddy/backend/tests/unit/test_print_storage_2780.py
maziggy 5dd7bd213f Read a print's destination from the report topic, not just the request one (issue #1820)
current_project_url was assigned in exactly one place, _handle_request_message,
and _on_message calls that only for the request topic. A print started from the
printer's own screen publishes nothing there, so the field stayed None for the
one case the storage verdict exists for: the file is already in the printer's
model library under /userdata/model/history/, which port 990 does not serve.

The verdict then fell through to the sdcard flag, and @ojimpo's H2S reports that
flag true -- its "card" is the internal eMMC -- so every such print ran the full
sweep before giving up. He measured one: 16 filename-and-directory attempts over
22 FTPS connections, 18 of them refused, 6.4 seconds, then a fallback archive
holding a name and nothing else.

The printer does announce where the file lives, as an unsolicited project_file
*response* on the report topic about two seconds before gcode_state reaches
PREPARE. _process_message now reads the url off it, gated on result SUCCESS and
a non-empty value so a refused dispatch cannot name a file that was never
written. Reading it there rather than only at the request topic also covers an
install neither of us had in view: some brokers refuse the request-topic
subscription, and on those no print of any kind had ever populated the field.

The new branch captures state and nothing else. The "External project_file
payload" diagnostic stays with the request-topic handler: our own dispatch is
echoed on both topics, the request-topic echo lands first and clears
_own_project_file_key, so reusing the diagnostic here would have logged every
Bambuddy-started print as somebody else's. A test pins that.

What the print names is now what gets tried -- the five directories a copy could
be in, rather than the ~110 connections that cannot succeed. The probe is still
worth running: an H2S keeps recently used jobs under /cache and archives them in
full while they last, which is why the reporter's two prints on the same day
behaved differently. Slicer-sent prints are unchanged.

The banner no longer describes a step that never happened. With no reason
recorded, a blank archive fell back to the original wording -- "Store sent files
on external storage" is off in your slicer -- which on that printer is on, and
which the internal-storage wording from #2780 already explains would not help on
an H2. The archive that most needed that explanation was the only one that could
not be given it.

So file:///userdata/ now earns its own reason, internal_history, separate from
the brtc://emmc dispatch case. A dispatch chose internal storage and can be
aimed elsewhere; a print of a file that was already there had no dispatch at
all, and telling that operator to pick External in Send names a dialog they
never opened. The banner and the connection diagnostic both read the verdict's
reason rather than a fixed one, so the two surfaces cannot give the same printer
different advice. Thirteen locales, and a wiki section the banner links to.

-----

Read the K-profile selection when the mutation runs, not when it is captured

Configure Slot sends cali_idx from selectedKProfile, and the mutation read it
through its own closure. React Query hands a mutation its options from an
effect, so a click landing between a commit and that effect flushing runs the
previous render's mutationFn -- one that captured the selection as it was before
the K-profile query resolved. The payload then carries cali_idx -1 and the
printer binds the default 0.020 instead of the calibrated K, while the dialog
shows the right profile selected throughout.

It surfaced as an intermittent failure of the per-nozzle K-profile test, about
one full-suite run in six. Reproducing it with staggered query resolution showed
the divergence directly: the select element held the correct profile immediately
before and after the click, and the payload still carried -1. That test's slot is
the most exposed case in the file -- a right-hotend slot carrying the left
hotend's index, where the "keep showing the active profile" safety net cannot
repair an empty recompute.

The selection now goes through a ref written during render, so the mutation
resolves it at execute time. An effect would have inherited the same flush
ordering this exists to escape. The K value and the profile's ids travel in the
same payload and had the same exposure, so they move with it.

Measured over a staggered-resolution grid: 2 failures in 15 runs before, 0 in 12
after. api.getSlicerPrinterModels was also missing from the test file's mock, so
that query ran with no query function and rejected in all 37 tests -- mocked now,
though on its own it changed nothing, which is how the ref was confirmed as the
fix rather than assumed.
2026-08-25 11:19:46 +02:00

383 lines
16 KiB
Python

"""Which prints are worth an FTPS sweep, and which are not (#2780).
The gate this module guards is one-sided on purpose, and both sides matter:
* Missing it costs ~110 doomed FTP connections per print and an archive card
that is blank with no stated reason -- the reported bug.
* Over-applying it costs archives that work today. A printer that never
publishes ``sdcard`` and never had a ``project_file`` reach us must sweep
exactly as before, or the fix is a regression for everyone else.
So the tests below spend most of their weight on the second failure mode.
"""
import pytest
from backend.app.services.print_storage import (
REASON_INTERNAL_HISTORY,
REASON_INTERNAL_STORAGE,
REASON_NO_EXTERNAL_STORAGE,
external_storage_present,
last_print_storage_verdict,
print_file_reachable_over_ftp,
url_is_external_storage,
)
pytestmark = pytest.mark.unit
class FakeState:
"""Stand-in for PrinterState with only the fields the helper reads."""
def __init__(self, current_project_url=None, sdcard=False, sdcard_reported=False, last_project_url=None):
self.current_project_url = current_project_url
# Defaults to the per-print value: for every test that does not care
# about the distinction, the two readings agree.
self.last_project_url = current_project_url if last_project_url is None else last_project_url
self.sdcard = sdcard
self.sdcard_reported = sdcard_reported
class TestUrlScheme:
@pytest.mark.parametrize(
"url",
[
"ftp://Benchy.gcode.3mf",
# Real dispatches carry names with spaces and non-ASCII; the scheme
# is all that is being read and none of that should disturb it.
"ftp://Halterung Kühlschrank V2.gcode.3mf",
"FTP://Benchy.gcode.3mf",
],
)
def test_ftp_means_external_storage(self, url):
assert url_is_external_storage(url) is True
@pytest.mark.parametrize(
"url",
[
# The scheme every H2C and P2S dispatch in #2780's bundle carried,
# 35 out of 35.
"brtc://emmc/169356_204314.STEP.gcode.3mf",
"brtc://emmc/Benchy.gcode.3mf",
],
)
def test_brtc_means_internal_storage(self, url):
assert url_is_external_storage(url) is False
def test_an_unknown_scheme_is_not_assumed_reachable(self):
"""Matching the reachable value, not the unreachable one.
If Bambu ships a third scheme, the safe reading is "somewhere we can't
see", not "fine" -- an unrecognised scheme that read as reachable would
put the storm straight back.
"""
assert url_is_external_storage("sftp://Benchy.3mf") is False
@pytest.mark.parametrize("url", [None, "", "Benchy.gcode.3mf"])
def test_no_usable_url_declines_to_answer(self, url):
"""None is a third answer and must not collapse into False."""
assert url_is_external_storage(url) is None
class TestFileScheme:
"""A print of a file that was already on the printer.
``file://`` is what the printer reports for a reprint from its own screen,
from Handy, or after a slicer sends to storage and then prints. Measured on
an H2D 2026-08-17: ``file:///media/usb0/foobar.gcode.3mf`` while that exact
file was listable and downloadable over FTPS. Reading it as internal storage
skipped the sweep and produced an archive with no 3MF, for a file sitting
right there -- and it did so on every model, not just the H2 series.
"""
@pytest.mark.parametrize(
"url",
[
"file:///media/usb0/foobar.gcode.3mf",
"file:///media/sdcard/Benchy.gcode.3mf",
"file:///media/usb0/timelapse/video.mp4",
],
)
def test_an_external_mount_is_not_evidence_of_internal_storage(self, url):
"""None, not True: the path is good reason to look, and looking is what
the caller's default already does."""
assert url_is_external_storage(url) is None
def test_the_model_cache_is_internal(self):
"""``/userdata/model/history/<name>`` is where the printer's own file
listing puts cached models, and port 990 does not serve it."""
assert url_is_external_storage("file:///userdata/model/history/Cube.gcode.3mf") is False
def test_an_unrecognised_path_sweeps_rather_than_skips(self):
"""Skip only on positive evidence -- a path we do not know is not that."""
assert url_is_external_storage("file:///somewhere/new/Cube.3mf") is None
def test_the_sweep_runs_for_a_file_on_the_card(self):
"""The regression in one assertion."""
state = FakeState(
current_project_url="file:///media/usb0/foobar.gcode.3mf",
sdcard=True,
sdcard_reported=True,
)
assert print_file_reachable_over_ftp(state).reachable is True
def test_an_empty_slot_still_wins(self):
"""With nothing in the slot the file cannot be on it, whatever the path
says -- and the operator gets the reason they can act on."""
state = FakeState(
current_project_url="file:///media/usb0/foobar.gcode.3mf",
sdcard=False,
sdcard_reported=True,
)
verdict = print_file_reachable_over_ftp(state)
assert verdict.reachable is False
assert verdict.reason == REASON_NO_EXTERNAL_STORAGE
def test_the_model_cache_still_skips(self):
state = FakeState(
current_project_url="file:///userdata/model/history/Cube.gcode.3mf",
sdcard=True,
sdcard_reported=True,
)
verdict = print_file_reachable_over_ftp(state)
assert verdict.reachable is False
# Its own reason, not the dispatch one: nothing was sent for this print,
# so the advice attached to REASON_INTERNAL_STORAGE -- pick External in
# the slicer's Send dialog -- describes a step that never happened
# (#1820).
assert verdict.reason == REASON_INTERNAL_HISTORY
@pytest.mark.parametrize("url", [12345, [], {}, object()])
def test_a_non_string_url_declines_too(self, url):
"""The value arrives straight off the wire, so it is whatever the
sender put there. Truth-testing alone would let a non-string fall
through to the scheme comparison and read as internal storage --
which is a silent skip of a sweep that should have run.
"""
assert url_is_external_storage(url) is None
class TestSweepIsSkipped:
def test_a_print_kept_on_internal_storage(self):
verdict = print_file_reachable_over_ftp(FakeState(current_project_url="brtc://emmc/Benchy.gcode.3mf"))
assert verdict.reachable is False
assert verdict.reason == REASON_INTERNAL_STORAGE
def test_a_printer_that_says_its_slot_is_empty(self):
"""#2780's H2C: `sdcard` False for three weeks, 800 clean FTPS
connections, and a 550 on every single path it asked for."""
verdict = print_file_reachable_over_ftp(FakeState(sdcard=False, sdcard_reported=True))
assert verdict.reachable is False
assert verdict.reason == REASON_NO_EXTERNAL_STORAGE
class TestSweepStillRuns:
"""The regression guard. Every case here worked before the gate existed."""
def test_a_print_on_external_storage(self):
assert print_file_reachable_over_ftp(FakeState(current_project_url="ftp://Benchy.gcode.3mf")).reachable
def test_a_printer_that_never_mentioned_its_card(self):
"""Silence is not evidence.
`sdcard` defaults to False, so a printer whose firmware simply never
publishes the field looks identical to an empty slot unless the
"did it ever say so" flag is honoured. Reading the default as an
answer would skip the sweep for every one of them.
"""
assert print_file_reachable_over_ftp(FakeState(sdcard=False, sdcard_reported=False)).reachable
def test_a_printer_with_a_card_and_no_dispatch_seen(self):
"""Some brokers refuse the request-topic subscription, so no URL ever
arrives. That install must behave exactly as it did before."""
assert print_file_reachable_over_ftp(FakeState(sdcard=True, sdcard_reported=True)).reachable
def test_an_explicit_ftp_url_outranks_a_disagreeing_card_flag(self):
"""A false skip is a regression; a needless sweep is only slow.
When the dispatcher says the file went to external storage, believe
the specific claim over the general one.
"""
state = FakeState(current_project_url="ftp://Benchy.gcode.3mf", sdcard=False, sdcard_reported=True)
assert print_file_reachable_over_ftp(state).reachable
def test_no_state_at_all(self):
"""Printer not connected, or status not yet populated."""
assert print_file_reachable_over_ftp(None).reachable
def test_a_state_missing_the_fields_entirely(self):
"""The helper is duck-typed, and a PrinterState from a pickled or
partially-constructed source may predate these fields."""
class Bare:
pass
assert print_file_reachable_over_ftp(Bare()).reachable
class TestReasonIsAlwaysPresentWhenUnreachable:
@pytest.mark.parametrize(
"state",
[
FakeState(current_project_url="brtc://emmc/x.3mf"),
FakeState(sdcard=False, sdcard_reported=True),
FakeState(current_project_url="file:///userdata/model/history/x.3mf"),
],
)
def test_unreachable_carries_a_reason(self, state):
"""The reason crosses into the API and picks the banner text. An
unreachable verdict without one would render the generic advice --
which is the wrong advice, and the whole point of the change."""
verdict = print_file_reachable_over_ftp(state)
assert verdict.reachable is False
assert verdict.reason
def test_reachable_carries_no_reason(self):
assert print_file_reachable_over_ftp(FakeState(sdcard=True, sdcard_reported=True)).reason is None
class TestTheGateUsesThePerPrintUrlOnly:
"""A stale URL must never gate a sweep.
``current_project_url`` is cleared when a print ends; ``last_project_url``
is sticky for reporting. The gate reads only the first, and that is
load-bearing rather than tidiness: 18% of the print starts in #2780's
support bundle (14 of 79) had no ``project_file`` on the request topic at
all -- touchscreen reprints, restart recovery, anything Bambuddy did not
see dispatched. If those inherited the previous job's destination, a
printer that ran one Studio print to internal storage would skip the FTPS
sweep for every subsequent screen-started print, losing archives that work
today.
The asymmetry is what makes it worth pinning: a stale ``ftp://`` costs
only a pointless sweep, while a stale ``brtc://`` costs an archive.
"""
def test_a_print_with_no_dispatch_of_its_own_still_sweeps(self):
"""The previous print went to internal storage; this one Bambuddy
never saw dispatched. Unknown, so sweep."""
state = FakeState(
current_project_url=None,
last_project_url="brtc://emmc/previous.gcode.3mf",
sdcard=True,
sdcard_reported=True,
)
assert print_file_reachable_over_ftp(state).reachable
def test_the_sticky_reading_still_reports_it(self):
"""The diagnostic is normally run after the print that prompted it, so
it needs the answer the gate has rightly forgotten."""
state = FakeState(
current_project_url=None,
last_project_url="brtc://emmc/previous.gcode.3mf",
sdcard=True,
sdcard_reported=True,
)
verdict = last_print_storage_verdict(state)
assert verdict.reachable is False
assert verdict.reason == REASON_INTERNAL_STORAGE
def test_an_empty_slot_is_reported_by_both(self):
"""Not URL-derived, so clearing the per-print value changes nothing."""
state = FakeState(sdcard=False, sdcard_reported=True)
assert print_file_reachable_over_ftp(state).reason == REASON_NO_EXTERNAL_STORAGE
assert last_print_storage_verdict(state).reason == REASON_NO_EXTERNAL_STORAGE
def test_the_sticky_reading_never_gates_a_sweep(self):
"""Guard against someone swapping the two back: if the gate ever reads
the sticky field, the case above starts failing -- and so does this."""
import inspect
from backend.app.services import print_storage
source = inspect.getsource(print_storage.print_file_reachable_over_ftp)
assert "current_project_url" in source
assert "last_project_url" not in source.split('"""')[-1]
class TestTheTwoInternalReasonsAreToldApart:
"""Same verdict, different advice (#1820).
Both URLs mean "port 990 cannot serve this", and until the report topic was
read there was only ever one of them to see. A screen-started print names
the other, and giving it the dispatch reason puts a banner in front of the
operator telling them to pick External in a Send dialog they never opened.
"""
def test_a_dispatch_that_chose_internal_storage(self):
verdict = print_file_reachable_over_ftp(
FakeState(current_project_url="brtc://emmc/Benchy.gcode.3mf", sdcard=True, sdcard_reported=True)
)
assert verdict.reason == REASON_INTERNAL_STORAGE
@pytest.mark.parametrize(
"url",
[
# Both forms measured on the H2S in #1820: the printer's own file
# library, reached from its screen and from Handy.
"file:///userdata/model/history/JOB_A.gcode.3mf",
"file:///userdata/model/history/Halterung Kuehlschrank V2.gcode.3mf",
],
)
def test_a_print_of_a_file_that_was_already_there(self, url):
verdict = print_file_reachable_over_ftp(FakeState(current_project_url=url, sdcard=True, sdcard_reported=True))
assert verdict.reason == REASON_INTERNAL_HISTORY
def test_both_still_earn_a_probe(self):
"""The reason split changes what the banner says, not what is tried.
An H2S keeps a copy of screen-started jobs under /cache for a while, and
that copy is what archived #1820's reporter's print 231."""
for url in ("brtc://emmc/Cube.gcode.3mf", "file:///userdata/model/history/Cube.gcode.3mf"):
verdict = print_file_reachable_over_ftp(
FakeState(current_project_url=url, sdcard=True, sdcard_reported=True)
)
assert verdict.probe_filename == "Cube.gcode.3mf"
def test_the_sticky_reading_splits_them_too(self):
"""The diagnostic reads the same helper, so a divergence here would
surface as one wording in the banner and another in Settings."""
state = FakeState(
current_project_url=None,
last_project_url="file:///userdata/model/history/Cube.gcode.3mf",
sdcard=True,
sdcard_reported=True,
)
assert last_print_storage_verdict(state).reason == REASON_INTERNAL_HISTORY
class TestTimelapseUsesTheNarrowerRule:
"""The printer writes its timelapse to the card itself.
Where the *sliced file* went says nothing about whether a video exists, so
gating the timelapse scan on the URL would silently stop finding videos on
every H2C and P2S that has a card in -- a new bug, introduced by the fix
for this one.
"""
def test_internal_storage_does_not_suppress_the_timelapse_scan(self):
state = FakeState(current_project_url="brtc://emmc/x.3mf", sdcard=True, sdcard_reported=True)
assert print_file_reachable_over_ftp(state).reachable is False
assert external_storage_present(state) is True
def test_an_empty_slot_does_suppress_it(self):
assert external_storage_present(FakeState(sdcard=False, sdcard_reported=True)) is False
def test_silence_does_not(self):
assert external_storage_present(FakeState(sdcard=False, sdcard_reported=False)) is True
def test_no_state_does_not(self):
assert external_storage_present(None) is True