mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 11:56:01 +02:00
fix(slicer): strip zero-valued filament-index sentinels, and sanitise the preview slice too (issue #3030)
Bambu Studio writes 0 into wall_filament, sparse_infill_filament and
solid_infill_filament to mean "use whichever filament the object is set
to". Bambu Studio and OrcaSlicer 2.4 define these min 0 and accept it;
OrcaSlicer 2.3 and earlier used the 1-based scheme (min 1, default 1)
and reject it with "0 not in range [1.000000,...]". Sidecar images are
version-tagged, so an install can be pinned to one of those builds.
Same shape as the -1 inherit markers from #1201 with a different marker,
so the allowlist becomes a key-to-marker map rather than one global
constant. The buckets must not bleed: a -1 on a filament index is a real
value, and a 0 on a raft field is a setting the user chose.
The key is removed rather than rewritten, which is what makes it safe on
every build. The CLI then uses its own default: 0 where 0 was legal
(unchanged), 1 on the older builds, which is what "the active filament"
means under that scheme.
The preview slice never ran the sanitiser at all, so a file that sliced
fine could still fail its automatic plate preview and fall back to the
painted-face heuristic. It matters more there than in a real slice: the
preview runs on the file's own embedded settings, so there is no
--load-settings pass that could supply a replacement for a field the
range validator has already rejected. That also explains the reported
"same error on a later attempt of an unchanged file" without any second
copy of the keys -- the validator that emits it reads the merged global
config, which per-object model_settings.config overrides never reach.
The sanitiser moves to utils/threemf_tools so the service can use it
without importing a route module, and both preview callers pick it up
from one place. Drops _strip_3mf_embedded_settings and its constant,
which have had no callers since the strip-everything experiment was
reverted.
This commit is contained in:
@@ -94,6 +94,7 @@ from backend.app.utils.threemf_tools import (
|
||||
extract_embedded_presets_from_3mf,
|
||||
extract_nozzle_mapping_from_3mf,
|
||||
extract_project_filaments_from_3mf,
|
||||
sanitize_project_settings_sentinels,
|
||||
select_plate_gcode_name,
|
||||
supports_enabled_in_config,
|
||||
)
|
||||
@@ -3492,142 +3493,6 @@ async def get_library_file_filament_requirements(
|
||||
}
|
||||
|
||||
|
||||
_STRIPPABLE_3MF_CONFIGS = frozenset(
|
||||
{
|
||||
# Settings dump used by --load-settings validation; the CLI tries to
|
||||
# match its sentinel values (`prime_tower_brim_width: -1`, empty
|
||||
# arrays) against the supplied profile and rejects out-of-range.
|
||||
"Metadata/project_settings.config",
|
||||
# Per-object settings overrides referencing the source plate's
|
||||
# filament IDs / printer IDs. When the user picks a different
|
||||
# printer / filament triplet, the IDs no longer resolve and the
|
||||
# CLI exits non-zero on input validation.
|
||||
"Metadata/model_settings.config",
|
||||
# Slicer-version + plate-config + filament-mapping snapshot from
|
||||
# the original slice. Includes the original printer model and
|
||||
# filament references; mismatches against `--load-settings`
|
||||
# consistently surfaced as `Slicer CLI failed (500)` for every
|
||||
# 3MF in production. Removing it lets the CLI build a fresh slice
|
||||
# plan from the supplied profile triplet.
|
||||
"Metadata/slice_info.config",
|
||||
# Multi-part / split-mesh metadata referencing object IDs from the
|
||||
# original slice. Strip for the same reason — preserves the geometry
|
||||
# in `3D/3dmodel.model` while dropping the orphan references.
|
||||
"Metadata/cut_information.xml",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _strip_3mf_embedded_settings(zip_bytes: bytes) -> bytes:
|
||||
"""Remove embedded slicer-config metadata from a 3MF.
|
||||
|
||||
Bambuddy supplies the slicer profile triplet via the sidecar's
|
||||
``--load-settings`` path; the 3MF's embedded settings would otherwise be
|
||||
validated by the CLI first and can fail with sentinel-value range
|
||||
checks (`prime_tower_brim_width: -1 not in range`, etc.) regardless of
|
||||
what we pass via ``--load-settings``. Stripping the embedded configs
|
||||
forces the CLI to use the supplied profiles only. Geometry
|
||||
(``3D/3dmodel.model``), thumbnails, color, and multi-part data inside
|
||||
the 3MF are preserved.
|
||||
|
||||
The set of strippable filenames is centralised in
|
||||
``_STRIPPABLE_3MF_CONFIGS`` — see that constant for the per-file
|
||||
rationale. Project-settings alone wasn't enough: real-world Bambu
|
||||
Studio 3MFs cross-reference printer / filament IDs from the other
|
||||
metadata configs, and any single leftover triggered the validation
|
||||
failure that made every profile-driven slice fall back to embedded
|
||||
settings.
|
||||
"""
|
||||
from io import BytesIO
|
||||
|
||||
src = BytesIO(zip_bytes)
|
||||
dst = BytesIO()
|
||||
with zipfile.ZipFile(src, "r") as zin, zipfile.ZipFile(dst, "w", zipfile.ZIP_DEFLATED) as zout:
|
||||
for item in zin.infolist():
|
||||
if item.filename in _STRIPPABLE_3MF_CONFIGS:
|
||||
continue
|
||||
zout.writestr(item, zin.read(item.filename))
|
||||
return dst.getvalue()
|
||||
|
||||
|
||||
# Keys in ``Metadata/project_settings.config`` that BambuStudio writes ``"-1"``
|
||||
# to when the user wants the value inherited from the parent process preset.
|
||||
# The CLI's ``StaticPrintConfig`` validator runs against the embedded settings
|
||||
# *before* ``--load-settings`` overrides apply, so a sentinel ``"-1"`` trips
|
||||
# the field's lower-bound range check and the CLI exits non-zero before our
|
||||
# profile triplet is ever consulted (#1201 — MakerWorld P2S models).
|
||||
#
|
||||
# Allowlisted (rather than "strip every '-1' value") because some fields
|
||||
# legitimately accept negative numbers (z_offset, translation values, etc.)
|
||||
# and a blanket strip would silently corrupt those.
|
||||
#
|
||||
# Add new entries here as more reports surface — the slicer's error message
|
||||
# names the offending field directly (`<field>: -1 not in range [...]`).
|
||||
_PROJECT_SETTINGS_SENTINEL_KEYS = frozenset(
|
||||
{
|
||||
# Reported in #1201 (MakerWorld P2S 3MFs).
|
||||
"raft_first_layer_expansion",
|
||||
"tree_support_wall_count",
|
||||
# Cited in the strip-experiment comment block above as a known sentinel
|
||||
# case from earlier reports.
|
||||
"prime_tower_brim_width",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _sanitize_project_settings_sentinels(zip_bytes: bytes) -> bytes:
|
||||
"""Strip ``"-1"`` inherit-from-parent sentinels from the 3MF's
|
||||
``Metadata/project_settings.config`` so the slicer CLI's range validator
|
||||
accepts the file (#1201).
|
||||
|
||||
Removes only allowlisted keys (see ``_PROJECT_SETTINGS_SENTINEL_KEYS``)
|
||||
when their value is exactly ``"-1"``. The rest of the config — and every
|
||||
other entry in the zip — is preserved byte-for-byte. Unlike the earlier
|
||||
full-strip experiment (see ``_strip_3mf_embedded_settings`` and the
|
||||
cautionary comment in ``_run_slicer_with_fallback``) this leaves
|
||||
``StaticPrintConfig`` initialisation intact: the file is still present,
|
||||
still parses, and the slicer falls back to the supplied
|
||||
``--load-settings`` value for the removed key.
|
||||
|
||||
Returns the original bytes unchanged when no sanitisation is needed
|
||||
(input isn't a valid zip, no ``project_settings.config``, no allowlisted
|
||||
sentinels present, or any other parse failure) so the caller can pass
|
||||
the result on without further checks.
|
||||
"""
|
||||
from io import BytesIO
|
||||
|
||||
try:
|
||||
with zipfile.ZipFile(BytesIO(zip_bytes), "r") as zin:
|
||||
if "Metadata/project_settings.config" not in zin.namelist():
|
||||
return zip_bytes
|
||||
try:
|
||||
config = json.loads(zin.read("Metadata/project_settings.config").decode("utf-8"))
|
||||
except (json.JSONDecodeError, UnicodeDecodeError):
|
||||
return zip_bytes
|
||||
if not isinstance(config, dict):
|
||||
return zip_bytes
|
||||
removed = [key for key in _PROJECT_SETTINGS_SENTINEL_KEYS if config.get(key) == "-1"]
|
||||
if not removed:
|
||||
return zip_bytes
|
||||
for key in removed:
|
||||
config.pop(key, None)
|
||||
patched = json.dumps(config)
|
||||
logger.info(
|
||||
"3MF sanitiser: removed sentinel '-1' for keys %s — slicer will use --load-settings defaults",
|
||||
sorted(removed),
|
||||
)
|
||||
dst = BytesIO()
|
||||
with zipfile.ZipFile(dst, "w", zipfile.ZIP_DEFLATED) as zout:
|
||||
for item in zin.infolist():
|
||||
if item.filename == "Metadata/project_settings.config":
|
||||
zout.writestr(item, patched)
|
||||
else:
|
||||
zout.writestr(item, zin.read(item.filename))
|
||||
return dst.getvalue()
|
||||
except (zipfile.BadZipFile, OSError):
|
||||
return zip_bytes
|
||||
|
||||
|
||||
def _patch_process_bed_type(process_json: str, bed_type: str) -> str:
|
||||
"""Overwrite ``curr_bed_type`` in a process-profile JSON before forwarding
|
||||
to the slicer sidecar.
|
||||
@@ -4022,13 +3887,14 @@ async def _run_slicer_with_fallback(
|
||||
is_3mf = model_filename.lower().endswith(".3mf")
|
||||
primary_bytes = model_bytes
|
||||
if is_3mf:
|
||||
# Strip "-1" inherit-from-parent sentinels from
|
||||
# Metadata/project_settings.config so the CLI's StaticPrintConfig
|
||||
# range validator accepts the file (#1201). Surgical — keeps the
|
||||
# config present, just removes the offending keys; the supplied
|
||||
# --load-settings (and the fallback's embedded values for keys we
|
||||
# didn't touch) still drive the slice.
|
||||
primary_bytes = _sanitize_project_settings_sentinels(primary_bytes)
|
||||
# Strip inherit/unset sentinels from Metadata/project_settings.config
|
||||
# so the CLI's StaticPrintConfig range validator accepts the file
|
||||
# (#1201, #3030). Surgical — keeps the config present, just removes
|
||||
# the offending keys; the supplied --load-settings (and the fallback's
|
||||
# embedded values for keys we didn't touch) still drive the slice.
|
||||
# The preview-slice path applies the same sanitiser in
|
||||
# ``slice_preview.get_preview_filaments``.
|
||||
primary_bytes = sanitize_project_settings_sentinels(primary_bytes)
|
||||
|
||||
# #2622: the process settings the file's designer moved off the stock
|
||||
# preset. Read once — the support patch below needs to know which of
|
||||
|
||||
@@ -14,10 +14,13 @@ we don't need to thread the user's profile triplet through here. That choice
|
||||
also protects the numbers — overriding the process preset drops the project's
|
||||
own support configuration, which loses whole slots from the answer.
|
||||
|
||||
The one thing that can defeat those embedded settings is a custom G-code
|
||||
template written by a Studio newer than the sidecar, which fails to parse
|
||||
before any slice_info exists. That case gets one retry with the offending
|
||||
template blanked; see ``_blank_custom_gcode``.
|
||||
Two things can defeat those embedded settings. A custom G-code template
|
||||
written by a Studio newer than the sidecar fails to parse before any
|
||||
slice_info exists; that case gets one retry with the offending template
|
||||
blanked, see ``_blank_custom_gcode``. And Bambu Studio writes inherit/unset
|
||||
markers into ``project_settings.config`` that some slicer builds' range
|
||||
validator rejects outright, so the same sanitiser the real slice runs is
|
||||
applied here too, see ``sanitize_project_settings_sentinels``.
|
||||
|
||||
Results are cached by ``(kind, source_id, plate_id, content_hash)`` so
|
||||
repeat opens on the same plate are instant. LRU eviction keeps the cache
|
||||
@@ -42,6 +45,7 @@ from backend.app.services.slicer_api import (
|
||||
SlicerApiError,
|
||||
SlicerApiService,
|
||||
)
|
||||
from backend.app.utils.threemf_tools import sanitize_project_settings_sentinels
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -203,13 +207,17 @@ async def get_preview_filaments(
|
||||
|
||||
Uses the file's embedded settings (``slice_without_profiles``) since the
|
||||
slot mapping is a model property, independent of any user-picked profile
|
||||
triplet. A slice killed by an unparsable custom G-code template is retried
|
||||
once with that template blanked, still on the file's own settings.
|
||||
triplet. Those settings are sentinel-sanitised first (#1201, #3030). A
|
||||
slice killed by an unparsable custom G-code template is retried once with
|
||||
that template blanked, still on the file's own settings.
|
||||
|
||||
Returns ``None`` when the preview slice fails — the caller should fall
|
||||
back to whatever heuristic it has (typically the project_filaments +
|
||||
painted-face approach in ``threemf_tools``).
|
||||
"""
|
||||
# Hash the file as it was given to us, not as it is sent: the key
|
||||
# identifies the source file, and sanitising is deterministic, so folding
|
||||
# it in would only make two names for one thing.
|
||||
h = _content_hash(file_bytes)
|
||||
key: _PreviewCacheKey = (kind, source_id, plate_id, h)
|
||||
cached = _preview_cache.get(key)
|
||||
@@ -231,6 +239,17 @@ async def get_preview_filaments(
|
||||
# while the slicer is visibly working.
|
||||
svc_kwargs = {} if timeout_seconds is None else {"timeout_seconds": timeout_seconds}
|
||||
|
||||
# Same sanitiser the real slice runs (#1201, #3030). It matters more
|
||||
# here, not less: this path slices on the file's own embedded
|
||||
# settings, so there is no --load-settings pass that could supply a
|
||||
# replacement for a field the CLI's range validator has already
|
||||
# rejected. Without it a MakerWorld 3MF carrying Bambu's inherit
|
||||
# markers fails before producing any slice_info, and the modal falls
|
||||
# back to its painted-face heuristic for a file the slicer could have
|
||||
# answered exactly. Applied before the G-code retry below so that
|
||||
# retry inherits it rather than reintroducing the markers.
|
||||
slice_bytes = sanitize_project_settings_sentinels(file_bytes)
|
||||
|
||||
async def _slice(model_bytes: bytes):
|
||||
async with SlicerApiService(base_url=api_url, **svc_kwargs) as svc:
|
||||
return await svc.slice_without_profiles(
|
||||
@@ -242,7 +261,7 @@ async def get_preview_filaments(
|
||||
)
|
||||
|
||||
try:
|
||||
result = await _slice(file_bytes)
|
||||
result = await _slice(slice_bytes)
|
||||
except SlicerApiError as e:
|
||||
# One retry, and only for a custom-G-code template the sidecar
|
||||
# cannot parse — a file from a Studio newer than the sidecar. The
|
||||
@@ -259,7 +278,7 @@ async def get_preview_filaments(
|
||||
retry_bytes = None
|
||||
option = _unparsable_gcode_option(str(e))
|
||||
if option is not None:
|
||||
retry_bytes = _blank_custom_gcode(file_bytes, option)
|
||||
retry_bytes = _blank_custom_gcode(slice_bytes, option)
|
||||
if retry_bytes is None:
|
||||
logger.warning(
|
||||
"Preview slice failed for %s/%s plate %s: %s",
|
||||
|
||||
@@ -1767,3 +1767,125 @@ def extract_plate_extruder_set_from_3mf(zf: zipfile.ZipFile, plate_id: int) -> s
|
||||
used.update(_scan_paint(path))
|
||||
break
|
||||
return used
|
||||
|
||||
|
||||
# Keys in ``Metadata/project_settings.config`` that Bambu Studio writes an
|
||||
# "inherit / unset" marker into, mapped to the marker it uses for that key.
|
||||
# The slicer CLI's ``StaticPrintConfig`` validator runs against the embedded
|
||||
# settings *before* ``--load-settings`` overrides apply, so a marker the CLI's
|
||||
# own range check rejects makes it exit non-zero before our profile triplet is
|
||||
# ever consulted.
|
||||
#
|
||||
# There are two markers because there are two conventions, and which one a
|
||||
# given CLI rejects depends on the build:
|
||||
#
|
||||
# "-1" -- inherit from the parent process preset (#1201, MakerWorld P2S
|
||||
# 3MFs). ``raft_first_layer_expansion`` and ``tree_support_wall_count`` are
|
||||
# min 0 in every OrcaSlicer to date, so those still fail on the current
|
||||
# sidecar; ``prime_tower_brim_width`` gained min -1 in Orca 2.4.2 and now
|
||||
# passes there, but not on older builds.
|
||||
#
|
||||
# "0" -- "use the active object/part filament", the default Bambu Studio
|
||||
# writes for the three feature-filament indices (#3030). Bambu Studio and
|
||||
# OrcaSlicer 2.4.0+ both define these min 0, so 0 is legal there; OrcaSlicer
|
||||
# 2.3.x and earlier still used the 1-based scheme (min 1, default 1) and
|
||||
# reject it with ``0 not in range [1.000000,...]``. Sidecar images are
|
||||
# version-tagged, so an install can be pinned to one of those.
|
||||
#
|
||||
# Removing the key rather than rewriting it is what makes this safe on every
|
||||
# build: the CLI then falls back to its own compiled default, which is 0 on
|
||||
# the builds where 0 was legal (so nothing changes) and 1 on the older ones,
|
||||
# which is what "the active filament" means under that scheme.
|
||||
#
|
||||
# Allowlisted (rather than "strip every marker-shaped value") because some
|
||||
# fields legitimately take the marker value -- z_offset, translations, and any
|
||||
# feature index a user really did set to a first filament -- and a blanket
|
||||
# strip would silently corrupt those.
|
||||
#
|
||||
# Add new entries as reports surface: the slicer names the offending field
|
||||
# directly, e.g. ``<field>: <value> not in range [...]``.
|
||||
PROJECT_SETTINGS_SENTINELS: dict[str, str] = {
|
||||
# Reported in #1201 (MakerWorld P2S 3MFs).
|
||||
"raft_first_layer_expansion": "-1",
|
||||
"tree_support_wall_count": "-1",
|
||||
# Known sentinel case from earlier reports, cited in #1201.
|
||||
"prime_tower_brim_width": "-1",
|
||||
# Reported in #3030 (MakerWorld 3MF, OrcaSlicer sidecar).
|
||||
"wall_filament": "0",
|
||||
"sparse_infill_filament": "0",
|
||||
"solid_infill_filament": "0",
|
||||
}
|
||||
|
||||
PROJECT_SETTINGS_PATH = "Metadata/project_settings.config"
|
||||
|
||||
|
||||
def _is_sentinel(value: object, sentinel: str) -> bool:
|
||||
"""Does ``value`` carry ``sentinel``, whether stored as text or a number?
|
||||
|
||||
Bambu Studio writes every ``project_settings.config`` value as a string,
|
||||
but a 3MF that has been round-tripped through another tool can carry the
|
||||
same field as a JSON number. ``bool`` is excluded explicitly: it is an
|
||||
``int`` subclass in Python, and ``str(False)`` would otherwise never match
|
||||
anyway -- the exclusion is there so a future numeric sentinel like ``0``
|
||||
cannot be matched by ``False``.
|
||||
"""
|
||||
if isinstance(value, bool):
|
||||
return False
|
||||
if isinstance(value, (str, int)):
|
||||
return str(value) == sentinel
|
||||
return False
|
||||
|
||||
|
||||
def sanitize_project_settings_sentinels(zip_bytes: bytes) -> bytes:
|
||||
"""Strip inherit/unset sentinels from a 3MF's ``project_settings.config``
|
||||
so the slicer CLI's range validator accepts the file (#1201, #3030).
|
||||
|
||||
Removes only allowlisted keys (see ``PROJECT_SETTINGS_SENTINELS``) and only
|
||||
when the value is exactly that key's sentinel. The rest of the config --
|
||||
and every other entry in the zip -- is preserved byte-for-byte. Unlike a
|
||||
whole-file strip this leaves ``StaticPrintConfig`` initialisation intact:
|
||||
the file is still present, still parses, and the slicer falls back to the
|
||||
supplied ``--load-settings`` value, or to its own default, for the removed
|
||||
key.
|
||||
|
||||
Returns the original bytes unchanged when no sanitisation is needed (input
|
||||
isn't a valid zip, no ``project_settings.config``, no allowlisted sentinels
|
||||
present, or any other parse failure) so the caller can pass the result on
|
||||
without further checks.
|
||||
"""
|
||||
from io import BytesIO
|
||||
|
||||
try:
|
||||
with zipfile.ZipFile(BytesIO(zip_bytes), "r") as zin:
|
||||
if PROJECT_SETTINGS_PATH not in zin.namelist():
|
||||
return zip_bytes
|
||||
try:
|
||||
config = json.loads(zin.read(PROJECT_SETTINGS_PATH).decode("utf-8"))
|
||||
except (json.JSONDecodeError, UnicodeDecodeError):
|
||||
return zip_bytes
|
||||
if not isinstance(config, dict):
|
||||
return zip_bytes
|
||||
removed = {
|
||||
key: sentinel
|
||||
for key, sentinel in PROJECT_SETTINGS_SENTINELS.items()
|
||||
if _is_sentinel(config.get(key), sentinel)
|
||||
}
|
||||
if not removed:
|
||||
return zip_bytes
|
||||
for key in removed:
|
||||
config.pop(key, None)
|
||||
patched = json.dumps(config)
|
||||
logger.info(
|
||||
"3MF sanitiser: removed inherit sentinels %s - slicer will use its defaults for those keys",
|
||||
sorted(f"{key}={sentinel}" for key, sentinel in removed.items()),
|
||||
)
|
||||
dst = BytesIO()
|
||||
with zipfile.ZipFile(dst, "w", zipfile.ZIP_DEFLATED) as zout:
|
||||
for item in zin.infolist():
|
||||
if item.filename == PROJECT_SETTINGS_PATH:
|
||||
zout.writestr(item, patched)
|
||||
else:
|
||||
zout.writestr(item, zin.read(item.filename))
|
||||
return dst.getvalue()
|
||||
except (zipfile.BadZipFile, OSError):
|
||||
return zip_bytes
|
||||
|
||||
@@ -495,3 +495,139 @@ class TestPreviewRetriesUnparsableGcode:
|
||||
assert len(stub.calls) == 2
|
||||
# A failed retry must not be cached — the sidecar may be upgraded.
|
||||
assert not slice_preview._preview_cache
|
||||
|
||||
|
||||
class _RecordingService:
|
||||
"""Succeeds every slice, recording the bytes it was handed."""
|
||||
|
||||
def __init__(self, response_bytes: bytes) -> None:
|
||||
self.response_bytes = response_bytes
|
||||
self.calls: list[bytes] = []
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *exc):
|
||||
return False
|
||||
|
||||
async def slice_without_profiles(self, **kw):
|
||||
self.calls.append(kw["model_bytes"])
|
||||
return SliceResult(
|
||||
content=self.response_bytes,
|
||||
print_time_seconds=0,
|
||||
filament_used_g=0.0,
|
||||
filament_used_mm=0.0,
|
||||
)
|
||||
|
||||
|
||||
class TestPreviewSanitisesSentinels:
|
||||
"""The preview slices on the file's own embedded settings, so there is no
|
||||
``--load-settings`` pass to supply a replacement for a field the CLI's
|
||||
range validator has already rejected. Until #3030 this path handed the
|
||||
sidecar raw bytes, so a MakerWorld 3MF carrying Bambu's inherit markers
|
||||
failed before producing any slice_info and the modal fell back to its
|
||||
painted-face heuristic for a file the slicer could have answered exactly.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_slicer_is_handed_sanitised_bytes(self):
|
||||
original = _make_project_3mf(
|
||||
{
|
||||
"wall_filament": "0",
|
||||
"raft_first_layer_expansion": "-1",
|
||||
"layer_height": "0.2",
|
||||
}
|
||||
)
|
||||
body = _make_sliced_3mf(plate_id=1, filaments=[{"id": "1", "type": "PLA", "color": "#000"}])
|
||||
stub = _RecordingService(body)
|
||||
with patch.object(slice_preview, "SlicerApiService", lambda **kw: stub):
|
||||
result = await get_preview_filaments(
|
||||
kind="library_file",
|
||||
source_id=1,
|
||||
plate_id=1,
|
||||
file_bytes=original,
|
||||
file_name="x.3mf",
|
||||
api_url="http://sidecar",
|
||||
)
|
||||
assert result is not None
|
||||
sent = _settings_of(stub.calls[0])
|
||||
assert "wall_filament" not in sent
|
||||
assert "raft_first_layer_expansion" not in sent
|
||||
# Everything the user actually configured still reaches the slicer.
|
||||
assert sent["layer_height"] == "0.2"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_file_without_sentinels_is_forwarded_untouched(self):
|
||||
original = _make_project_3mf({"layer_height": "0.2", "wall_filament": "2"})
|
||||
body = _make_sliced_3mf(plate_id=1, filaments=[{"id": "1", "type": "PLA", "color": "#000"}])
|
||||
stub = _RecordingService(body)
|
||||
with patch.object(slice_preview, "SlicerApiService", lambda **kw: stub):
|
||||
await get_preview_filaments(
|
||||
kind="library_file",
|
||||
source_id=1,
|
||||
plate_id=1,
|
||||
file_bytes=original,
|
||||
file_name="x.3mf",
|
||||
api_url="http://sidecar",
|
||||
)
|
||||
# Not merely equal: the sanitiser returns the input object when it has
|
||||
# nothing to do, so the common case never pays for a zip rebuild.
|
||||
assert stub.calls[0] is original
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_gcode_retry_keeps_the_sanitisation(self):
|
||||
# Both faults at once. The retry derives from the sanitised bytes, so
|
||||
# it must not reintroduce the sentinel while blanking the template --
|
||||
# otherwise the retry trades one CLI rejection for the other.
|
||||
original = _make_project_3mf(
|
||||
{
|
||||
"time_lapse_gcode": "{if timelapse_inline_photo}",
|
||||
"wall_filament": "0",
|
||||
"layer_height": "0.2",
|
||||
}
|
||||
)
|
||||
body = _make_sliced_3mf(plate_id=1, filaments=[{"id": "1", "type": "PLA", "color": "#000"}])
|
||||
stub = _FailThenSucceedService(SlicerApiServerError(_TIMELAPSE_PARSE_ERROR), body)
|
||||
with patch.object(slice_preview, "SlicerApiService", lambda **kw: stub):
|
||||
result = await get_preview_filaments(
|
||||
kind="library_file",
|
||||
source_id=1,
|
||||
plate_id=1,
|
||||
file_bytes=original,
|
||||
file_name="x.3mf",
|
||||
api_url="http://sidecar",
|
||||
)
|
||||
assert result is not None
|
||||
assert len(stub.calls) == 2
|
||||
for sent in stub.calls:
|
||||
assert "wall_filament" not in _settings_of(sent)
|
||||
assert _settings_of(stub.calls[1])["time_lapse_gcode"] == ""
|
||||
assert _settings_of(stub.calls[1])["layer_height"] == "0.2"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_cache_key_still_follows_the_source_file(self):
|
||||
# Sanitising is deterministic, so the key stays the hash of what the
|
||||
# caller read off disk -- a second open of the same file is a hit, not
|
||||
# a second 30-second slice.
|
||||
original = _make_project_3mf({"wall_filament": "0"})
|
||||
body = _make_sliced_3mf(plate_id=1, filaments=[{"id": "1", "type": "PLA", "color": "#000"}])
|
||||
stub = _RecordingService(body)
|
||||
with patch.object(slice_preview, "SlicerApiService", lambda **kw: stub):
|
||||
first = await get_preview_filaments(
|
||||
kind="library_file",
|
||||
source_id=1,
|
||||
plate_id=1,
|
||||
file_bytes=original,
|
||||
file_name="x.3mf",
|
||||
api_url="http://sidecar",
|
||||
)
|
||||
second = await get_preview_filaments(
|
||||
kind="library_file",
|
||||
source_id=1,
|
||||
plate_id=1,
|
||||
file_bytes=original,
|
||||
file_name="x.3mf",
|
||||
api_url="http://sidecar",
|
||||
)
|
||||
assert second == first
|
||||
assert len(stub.calls) == 1
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Unit tests for ``_sanitize_project_settings_sentinels`` (#1201).
|
||||
"""Unit tests for ``sanitize_project_settings_sentinels`` (#1201, #3030).
|
||||
|
||||
MakerWorld 3MFs sliced for the P2S (and potentially other Bambu printers)
|
||||
ship ``Metadata/project_settings.config`` entries with ``"-1"`` values on
|
||||
@@ -22,6 +22,21 @@ re-zip. The slicer then falls back to the supplied ``--load-settings``
|
||||
default for the removed keys, while every other entry in the zip stays
|
||||
byte-identical.
|
||||
|
||||
#3030 added a second sentinel value. ``wall_filament``,
|
||||
``sparse_infill_filament`` and ``solid_infill_filament`` are filament indices
|
||||
that Bambu Studio writes as ``"0"`` meaning "use the active object/part
|
||||
filament". Bambu Studio and OrcaSlicer 2.4.0+ define these ``min 0``, so the
|
||||
value is legal there; OrcaSlicer 2.3.x and earlier used the 1-based scheme
|
||||
(``min 1``, default ``1``) and answer::
|
||||
|
||||
wall_filament: 0 not in range [1.000000,...]
|
||||
|
||||
Sidecar images are version-tagged, so an install can be pinned to one of
|
||||
those builds. So the allowlist is a key -> sentinel mapping now rather than
|
||||
one global constant, and the two buckets must not bleed into each other: a
|
||||
``"-1"`` on a filament index is not a sentinel, and a ``"0"`` on a raft field
|
||||
is a value the user chose.
|
||||
|
||||
Pinning the contract here rather than via the slicer integration tests
|
||||
because the fix is purely about the bytes we hand to the sidecar — no
|
||||
slicer mock needed.
|
||||
@@ -33,9 +48,9 @@ import zipfile
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.api.routes.library import (
|
||||
_PROJECT_SETTINGS_SENTINEL_KEYS,
|
||||
_sanitize_project_settings_sentinels,
|
||||
from backend.app.utils.threemf_tools import (
|
||||
PROJECT_SETTINGS_SENTINELS,
|
||||
sanitize_project_settings_sentinels,
|
||||
)
|
||||
|
||||
|
||||
@@ -65,10 +80,10 @@ def _zip_namelist(zip_bytes: bytes) -> list[str]:
|
||||
|
||||
|
||||
class TestRemovesSentinelValues:
|
||||
@pytest.mark.parametrize("key", sorted(_PROJECT_SETTINGS_SENTINEL_KEYS))
|
||||
def test_removes_each_allowlisted_key_when_value_is_minus_one(self, key):
|
||||
original = _make_3mf(settings={key: "-1", "layer_height": "0.2"})
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
@pytest.mark.parametrize(("key", "sentinel"), sorted(PROJECT_SETTINGS_SENTINELS.items()))
|
||||
def test_removes_each_allowlisted_key_at_its_own_sentinel(self, key, sentinel):
|
||||
original = _make_3mf(settings={key: sentinel, "layer_height": "0.2"})
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
|
||||
cfg = _read_settings(sanitised)
|
||||
assert key not in cfg, f"Sentinel key {key!r} should have been removed"
|
||||
@@ -85,7 +100,7 @@ class TestRemovesSentinelValues:
|
||||
"layer_height": "0.2",
|
||||
}
|
||||
)
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
|
||||
cfg = _read_settings(sanitised)
|
||||
assert "raft_first_layer_expansion" not in cfg
|
||||
@@ -99,7 +114,7 @@ class TestPreservesUnaffectedValues:
|
||||
# A user who deliberately configured raft_first_layer_expansion=0 must
|
||||
# see that 0 forwarded to the slicer — only literal "-1" gets stripped.
|
||||
original = _make_3mf(settings={"raft_first_layer_expansion": "0"})
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
assert _read_settings(sanitised)["raft_first_layer_expansion"] == "0"
|
||||
|
||||
def test_does_not_touch_non_allowlisted_keys_with_minus_one(self):
|
||||
@@ -107,7 +122,7 @@ class TestPreservesUnaffectedValues:
|
||||
# Some Bambu fields legitimately allow negative values (z_offset,
|
||||
# translation, etc.) and a blanket "-1" strip would corrupt those.
|
||||
original = _make_3mf(settings={"z_offset": "-1", "layer_height": "0.2"})
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
|
||||
cfg = _read_settings(sanitised)
|
||||
assert cfg["z_offset"] == "-1"
|
||||
@@ -117,7 +132,7 @@ class TestPreservesUnaffectedValues:
|
||||
# If nothing needs sanitising, return the input identity-equal so
|
||||
# the caller's downstream comparisons / hashes don't churn.
|
||||
original = _make_3mf(settings={"layer_height": "0.2", "z_offset": "0"})
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
assert sanitised is original
|
||||
|
||||
def test_does_not_strip_array_value_even_if_includes_minus_one(self):
|
||||
@@ -128,7 +143,7 @@ class TestPreservesUnaffectedValues:
|
||||
# sentinel and removed wholesale. If a future report shows the CLI
|
||||
# rejects array-form sentinels, expand this then.
|
||||
original = _make_3mf(settings={"raft_first_layer_expansion": ["-1", "0"]})
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
cfg = _read_settings(sanitised)
|
||||
assert cfg["raft_first_layer_expansion"] == ["-1", "0"]
|
||||
|
||||
@@ -143,7 +158,7 @@ class TestZipPreservation:
|
||||
"Metadata/_rels/model_settings.rels": "<rels/>",
|
||||
},
|
||||
)
|
||||
sanitised = _sanitize_project_settings_sentinels(original)
|
||||
sanitised = sanitize_project_settings_sentinels(original)
|
||||
assert sanitised is not original
|
||||
|
||||
names = _zip_namelist(sanitised)
|
||||
@@ -171,12 +186,12 @@ class TestDefensiveFallbacks:
|
||||
# routing decides whether 3MF sanitisation runs anyway, but
|
||||
# defending here means a misrouted call can't corrupt the bytes.
|
||||
garbage = b"not a zip file"
|
||||
assert _sanitize_project_settings_sentinels(garbage) is garbage
|
||||
assert sanitize_project_settings_sentinels(garbage) is garbage
|
||||
|
||||
def test_returns_original_when_settings_config_absent(self):
|
||||
# 3MF without an embedded project_settings.config — nothing to do.
|
||||
original = _make_3mf(settings=None)
|
||||
assert _sanitize_project_settings_sentinels(original) is original
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
def test_returns_original_on_malformed_json(self):
|
||||
# Settings file present but not valid JSON. We don't risk rebuilding
|
||||
@@ -187,7 +202,7 @@ class TestDefensiveFallbacks:
|
||||
zf.writestr("3D/3dmodel.model", "<model/>")
|
||||
zf.writestr("Metadata/project_settings.config", "{not valid json")
|
||||
original = buf.getvalue()
|
||||
assert _sanitize_project_settings_sentinels(original) is original
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
def test_returns_original_when_settings_root_is_not_a_dict(self):
|
||||
# Real-world configs are objects, but defend against an array root
|
||||
@@ -198,4 +213,69 @@ class TestDefensiveFallbacks:
|
||||
zf.writestr("3D/3dmodel.model", "<model/>")
|
||||
zf.writestr("Metadata/project_settings.config", "[]")
|
||||
original = buf.getvalue()
|
||||
assert _sanitize_project_settings_sentinels(original) is original
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
|
||||
class TestTheTwoSentinelBucketsDoNotBleed:
|
||||
"""Each key has exactly one sentinel, and the other bucket's value is a
|
||||
real setting on it. Getting this wrong in either direction is worse than
|
||||
the bug: strip a filament index that says ``-1`` and the slice loses a
|
||||
value nothing will put back; strip a raft field that says ``0`` and a
|
||||
user who deliberately turned the raft expansion off gets the preset's
|
||||
default instead."""
|
||||
|
||||
@pytest.mark.parametrize("key", ["wall_filament", "sparse_infill_filament", "solid_infill_filament"])
|
||||
def test_a_filament_index_of_minus_one_is_left_alone(self, key):
|
||||
original = _make_3mf(settings={key: "-1"})
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
@pytest.mark.parametrize("key", ["raft_first_layer_expansion", "tree_support_wall_count", "prime_tower_brim_width"])
|
||||
def test_a_zero_on_a_minus_one_key_is_left_alone(self, key):
|
||||
original = _make_3mf(settings={key: "0"})
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
def test_a_mixed_config_removes_exactly_the_sentinels(self):
|
||||
original = _make_3mf(
|
||||
settings={
|
||||
# sentinels, both buckets
|
||||
"raft_first_layer_expansion": "-1",
|
||||
"wall_filament": "0",
|
||||
# same keys' non-sentinel values, crossed over
|
||||
"tree_support_wall_count": "0",
|
||||
"sparse_infill_filament": "-1",
|
||||
# an explicit filament pick
|
||||
"solid_infill_filament": "2",
|
||||
"layer_height": "0.2",
|
||||
}
|
||||
)
|
||||
cfg = _read_settings(sanitize_project_settings_sentinels(original))
|
||||
assert "raft_first_layer_expansion" not in cfg
|
||||
assert "wall_filament" not in cfg
|
||||
assert cfg["tree_support_wall_count"] == "0"
|
||||
assert cfg["sparse_infill_filament"] == "-1"
|
||||
assert cfg["solid_infill_filament"] == "2"
|
||||
assert cfg["layer_height"] == "0.2"
|
||||
|
||||
|
||||
class TestNumericValuesAreRecognised:
|
||||
"""Bambu Studio writes every value as a string, but a 3MF round-tripped
|
||||
through another tool can carry the same field as a JSON number. The
|
||||
slicer's validator reads the deserialised int either way, so the
|
||||
sanitiser has to as well."""
|
||||
|
||||
def test_an_integer_sentinel_is_removed(self):
|
||||
original = _make_3mf(settings={"wall_filament": 0, "layer_height": "0.2"})
|
||||
cfg = _read_settings(sanitize_project_settings_sentinels(original))
|
||||
assert "wall_filament" not in cfg
|
||||
assert cfg["layer_height"] == "0.2"
|
||||
|
||||
def test_an_integer_non_sentinel_survives(self):
|
||||
original = _make_3mf(settings={"wall_filament": 2})
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
def test_false_is_not_a_zero_sentinel(self):
|
||||
# bool is an int subclass in Python. A config that stores a flag as
|
||||
# JSON ``false`` under one of these names must not be mistaken for
|
||||
# the numeric 0 sentinel.
|
||||
original = _make_3mf(settings={"wall_filament": False})
|
||||
assert sanitize_project_settings_sentinels(original) is original
|
||||
|
||||
Reference in New Issue
Block a user