mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-22 12:11:31 +02:00
fix(queue): an id of 0 no longer slips past the checks when queueing, batching or creating a project
Queue create, batch create and project create tested archive_id, library_file_id and parent_id for truthiness: 0 skipped the existence check and was then stored as the id. On PostgreSQL that failed the foreign key with a server error; on SQLite it saved a link to nothing. Batch create did not check its archive or library file at all. 0 now means none on all three, as it already did when editing a project's parent. Batch create also checks that a given archive exists and a given library file is active, and refuses an unknown one with 400.
This commit is contained in:
@@ -162,6 +162,7 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
|
||||
|
||||
### Fixed
|
||||
- **An id of 0 no longer slips past the checks when queueing, batching or creating a project** — Adding to the queue with an archive id of `0` next to a library file, creating a batch with an archive or library file id of `0`, and creating a project with parent `0` skipped the check that the archive, file or project exists and saved `0` as its id: a server error on PostgreSQL, a link to nothing on SQLite. `0` now means "none" there, as it already did when editing a project. Creating a batch also checks that its archive or library file exists, and refuses an unknown one.
|
||||
- **Archiving a virtual-printer upload into a project that does not exist is refused up front** — The project chosen when archiving a pending upload was saved without checking it. On PostgreSQL an unknown project failed with a server error after the file had already been archived; on SQLite the archive was saved with a broken project link. The project is now checked first and an unknown one refused with "Project not found", before anything is archived.
|
||||
- **A project's bill of materials no longer shows archives from printers outside the user's groups** — A BOM line can be linked to an archive through the API, and the BOM answered with that archive's name whichever printer it came from. A member of a group limited to some printers could read the name of any archive by linking a line to it, and everyone reading the project saw the names of archives from printers they have no access to. A link to an archive that does not exist was saved too (a server error on PostgreSQL). A BOM line now links only to an archive the user can see, and shows a linked archive only to users who can see it.
|
||||
- **Adding archives or queue jobs to a project keeps to the user's own printers and prints** — Adding archives to a project (**Assign to project**), removing them, and adding queue jobs took any archive or job by its id. A member of a group limited to some printers could link another team's archives and jobs to a project or unlink them, and the reply's count told them which hidden archives exist; any project editor could also relink other users' prints. These now change only the archives and jobs the user could edit directly: on their printers, and only their own unless they may edit everyone's (Operators: their own). Others are skipped like missing ones, and **Assign to project** now reports how many were actually added. An API key with project access still needs no archive or queue access for this, but changes only the archives and jobs of its printers that its owner could change.
|
||||
|
||||
@@ -836,6 +836,9 @@ async def add_to_queue(
|
||||
actor: User | ApiKeyActor | None = RequestActor,
|
||||
):
|
||||
"""Add an item to the print queue."""
|
||||
# 0 is no file, as on the checks below; stored, it would point at nothing.
|
||||
data.archive_id = data.archive_id or None
|
||||
data.library_file_id = data.library_file_id or None
|
||||
# Normalize target_model (e.g., "Bambu Lab X1E" / "C13" -> "X1E").
|
||||
# normalize_model_name resolves internal codes first: the previous
|
||||
# `normalize_printer_model(x) or normalize_printer_model_id(x)` chain never
|
||||
@@ -1492,6 +1495,17 @@ async def create_batch(
|
||||
|
||||
plate_targets = _validate_plate_targets(data.plates)
|
||||
await _validate_batch_project(db, data.project_id, actor)
|
||||
# The file the batch prints, for reference: 0 is none, any other id must exist.
|
||||
data.archive_id = data.archive_id or None
|
||||
data.library_file_id = data.library_file_id or None
|
||||
if data.archive_id is not None and await db.get(PrintArchive, data.archive_id) is None:
|
||||
raise HTTPException(400, "Archive not found")
|
||||
if (
|
||||
data.library_file_id is not None
|
||||
and (await db.execute(LibraryFile.active().where(LibraryFile.id == data.library_file_id))).scalar_one_or_none()
|
||||
is None
|
||||
):
|
||||
raise HTTPException(400, "Library file not found")
|
||||
if data.external_source is not None:
|
||||
existing = await db.execute(
|
||||
select(PrintBatch.id).where(
|
||||
|
||||
@@ -554,7 +554,8 @@ async def create_project(
|
||||
"""Create a new project."""
|
||||
if data.priority not in PROJECT_PRIORITIES:
|
||||
raise HTTPException(status_code=400, detail="Invalid priority")
|
||||
# Verify parent exists if specified
|
||||
# Verify parent exists if specified; 0 is no parent, as on update
|
||||
data.parent_id = data.parent_id or None
|
||||
parent_name = None
|
||||
if data.parent_id:
|
||||
parent_result = await db.execute(select(Project).where(Project.id == data.parent_id))
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
"""An id of 0 on create means "not given", as on the matching edits.
|
||||
|
||||
Queue create, batch create and project create tested these ids for
|
||||
truthiness: 0 skipped the existence check and was then stored as an id (a
|
||||
server error on PostgreSQL, a link to nothing on SQLite). Batch create did not
|
||||
check its archive or library file at all.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
from sqlalchemy import select
|
||||
|
||||
from backend.app.models.library import LibraryFile
|
||||
from backend.app.models.print_batch import PrintBatch
|
||||
from backend.app.models.print_queue import PrintQueueItem
|
||||
from backend.app.models.project import Project
|
||||
|
||||
pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
|
||||
|
||||
|
||||
async def _library_file(db_session) -> int:
|
||||
row = LibraryFile(filename="part.gcode.3mf", file_path="library/part.gcode.3mf", file_size=5, file_type="3mf")
|
||||
db_session.add(row)
|
||||
await db_session.commit()
|
||||
return row.id
|
||||
|
||||
|
||||
async def test_queueing_a_library_file_with_archive_0(async_client: AsyncClient, db_session, printer_factory):
|
||||
printer_id = (await printer_factory()).id
|
||||
file_id = await _library_file(db_session)
|
||||
|
||||
response = await async_client.post(
|
||||
"/api/v1/queue/", json={"archive_id": 0, "library_file_id": file_id, "printer_id": printer_id}
|
||||
)
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
db_session.expire_all()
|
||||
stored = (await db_session.execute(select(PrintQueueItem.archive_id, PrintQueueItem.library_file_id))).one()
|
||||
assert tuple(stored) == (None, file_id)
|
||||
|
||||
|
||||
async def test_queueing_with_both_ids_0_is_refused(async_client: AsyncClient):
|
||||
response = await async_client.post("/api/v1/queue/", json={"archive_id": 0, "library_file_id": 0})
|
||||
|
||||
assert response.status_code == 400, response.text
|
||||
assert response.json()["detail"] == "Either archive_id or library_file_id must be provided"
|
||||
|
||||
|
||||
async def test_a_batch_with_archive_0_has_no_archive(async_client: AsyncClient, db_session):
|
||||
file_id = await _library_file(db_session)
|
||||
|
||||
response = await async_client.post(
|
||||
"/api/v1/queue/batches", json={"name": "B", "archive_id": 0, "library_file_id": file_id}
|
||||
)
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
assert (response.json()["archive_id"], response.json()["library_file_id"]) == (None, file_id)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("field", "detail"), [("archive_id", "Archive not found"), ("library_file_id", "Library file not found")]
|
||||
)
|
||||
async def test_a_batch_for_an_unknown_file_is_refused(async_client: AsyncClient, db_session, field, detail):
|
||||
response = await async_client.post("/api/v1/queue/batches", json={"name": "B", field: 9999})
|
||||
|
||||
assert response.status_code == 400, response.text
|
||||
assert response.json()["detail"] == detail
|
||||
assert (await db_session.execute(select(PrintBatch.id))).first() is None
|
||||
|
||||
|
||||
async def test_a_batch_for_a_known_archive_keeps_it(async_client: AsyncClient, printer_factory, archive_factory):
|
||||
archive = await archive_factory((await printer_factory()).id)
|
||||
|
||||
response = await async_client.post("/api/v1/queue/batches", json={"name": "B", "archive_id": archive.id})
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["archive_id"] == archive.id
|
||||
|
||||
|
||||
async def test_a_project_with_parent_0_has_no_parent(async_client: AsyncClient, db_session):
|
||||
response = await async_client.post("/api/v1/projects/", json={"name": "P", "parent_id": 0})
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["parent_id"] is None
|
||||
db_session.expire_all()
|
||||
assert (await db_session.execute(select(Project.parent_id))).scalar() is None
|
||||
Reference in New Issue
Block a user