forked from 1b-agent/opencadstudio-upstream
ocs_doc_api: fix branch-review findings (tab auth, bulge center, budget, caps, dead code)
CRITICAL: - execute_doc_api: reject tab_id != bound tab (confused deputy); test doc_api_tab_mismatch_is_rejected. - bulge_arc_segment: fix negative/CW bulge center (apothem magnitude + sign picks side; was double-sign-mirrored); test CW-vs-CCW center offset. - cold-tess budget: move mass_cache/cold_tess_used to per-tab Scene so the budget persists across DocApi dispatches (was reset per request); budget-exhausted -> approximate meshes.metrics fallback instead of error. WARNING: - Loft: add BULK_ITEM_CAP (was uncapped -> unbounded kernel call from one request). - CreateMany: pre-validate EntitySpec::Curve (ellipse ratio) in prepare phase -> all-or-nothing restored; regression test. - Partial Ellipse: offset start/end_parameter by rot_z on rotation; regression test. - resolve_body: add with_body borrow accessor (host overrides to borrow cache in place, O(1)); bounds() uses it instead of deep-cloning the B-rep per query. - add_raster_image: validate file path (non-empty, image extension, reject UNC/traversal); regression test. SUGGESTION: - deploy: bump API_VERSION 5 -> 6 so DocApi plugins are gated at handshake on old hosts (append-only wire + defaulted method keep old V2-V5 plugins loading). - dead code: wire query_name into apply_queries error labels; host uses crate's ObjectId::from_handle/to_handle (drop local duplicates); delete is_null/backend_mut. - mock drift: test mock now stores curve specs + computes curve bounds (aligns with example mock); Spline/Dimension bounds delegate to points_aabb. 623 host lib + 8 crate + 3 consistency + 34 plugin_api green.
This commit is contained in:
1 parent
ac54d6079b
commit
919128e087
10 files changed
+316
-71
No files matched your search
@@ -19,9 +19,22 @@ pub type KernelBody = cadkernel::brep::Body;
|
||||
pub trait DocApiBackend {
|
||||
// ── identity / solid lookup ────────────────────────────────────────────
|
||||
/// Resolve a live B-rep body for `id` (solid cache; re-lift from AcisData on
|
||||
/// miss). `UnknownId` if `id` is not a live solid.
|
||||
/// miss). `UnknownId` if `id` is not a live solid. NOTE: returns an OWNED body
|
||||
/// (a clone of the cache entry); use `with_body` for read-only queries.
|
||||
fn resolve_body(&mut self, id: ObjectId) -> ApiResult<KernelBody>;
|
||||
|
||||
/// Borrow the cached body read-only for a query, avoiding the `resolve_body`
|
||||
/// deep clone. Default: falls back to `resolve_body` (clone). The host overrides
|
||||
/// this to borrow the `solid_models` cache in place (O(1), no copy).
|
||||
fn with_body<R>(
|
||||
&mut self,
|
||||
id: ObjectId,
|
||||
f: &mut dyn FnMut(&KernelBody) -> ApiResult<R>,
|
||||
) -> ApiResult<R> {
|
||||
let body = self.resolve_body(id)?;
|
||||
f(&body)
|
||||
}
|
||||
|
||||
/// Store a new solid body as a `Solid3D` entity; returns the fresh `ObjectId`.
|
||||
/// Mirrors host `add_solid_model` (edge wires + `solid_to_sat` +
|
||||
/// `set_sat_document` + `register_solid_model`).
|
||||
|
||||
@@ -103,14 +103,19 @@ pub fn apply_op<B: DocApiBackend>(b: &mut B, op: Operation) -> ApiResult<Receipt
|
||||
return Err(over_cap(name, specs.len()));
|
||||
}
|
||||
// Upfront validation: compute/validate EVERY payload before any insert
|
||||
// (all-or-nothing without rollback, plan §5.3).
|
||||
// (all-or-nothing without rollback, plan §5.3). This includes validating
|
||||
// the curve spec's fallible conditions (e.g. ellipse ratio) that
|
||||
// `curve_spec_to_entity` would otherwise reject mid-apply-loop.
|
||||
let mut prepared = Vec::with_capacity(specs.len());
|
||||
for (i, spec) in specs.iter().enumerate() {
|
||||
let prep = match spec {
|
||||
EntitySpec::Solid(p) => {
|
||||
Prepared::Solid(crate::geom::make_solid(p).map_err(|e| at_index(name, i, e))?)
|
||||
}
|
||||
EntitySpec::Curve(c) => Prepared::Curve(c.clone()),
|
||||
EntitySpec::Curve(c) => {
|
||||
validate_curve_spec(c).map_err(|e| at_index(name, i, e))?;
|
||||
Prepared::Curve(c.clone())
|
||||
}
|
||||
};
|
||||
prepared.push(prep);
|
||||
}
|
||||
@@ -254,6 +259,11 @@ pub fn apply_op<B: DocApiBackend>(b: &mut B, op: Operation) -> ApiResult<Receipt
|
||||
OpOutcome::NewId(id)
|
||||
}
|
||||
Operation::Loft { profiles } => {
|
||||
// Bound the section count like the bulk ops (one envelope can carry many
|
||||
// ids; an uncapped loft would be an unbounded kernel call on the UI thread).
|
||||
if profiles.len() > BULK_ITEM_CAP {
|
||||
return Err(over_cap(name, profiles.len()));
|
||||
}
|
||||
// Resolve every profile to its curve set BEFORE push_undo (all-or-nothing).
|
||||
if profiles.len() < 2 {
|
||||
return Err(ApiError::validation(name, "loft needs >= 2 profiles"));
|
||||
@@ -336,24 +346,25 @@ pub fn apply_queries<B: DocApiBackend>(b: &mut B, queries: Vec<Query>) -> ApiRes
|
||||
}
|
||||
let mut results = Vec::with_capacity(queries.len());
|
||||
for q in &queries {
|
||||
let qname = q.query_name();
|
||||
let r = match q {
|
||||
Query::GetEntity { id } => QueryResult::Entity(b.get_entity(*id)?),
|
||||
Query::GetBounds { id } => QueryResult::Bounds(b.bounds(*id)?),
|
||||
Query::GetCentroid { id } => QueryResult::Centroid(b.centroid(*id)?),
|
||||
Query::GetVolume { id } => QueryResult::Volume(b.volume(*id)?),
|
||||
Query::GetEntity { id } => QueryResult::Entity(b.get_entity(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetBounds { id } => QueryResult::Bounds(b.bounds(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetCentroid { id } => QueryResult::Centroid(b.centroid(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetVolume { id } => QueryResult::Volume(b.volume(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetIntersects { a, b: bid } => {
|
||||
let ba = b.bounds(*a)?;
|
||||
let bb = b.bounds(*bid)?;
|
||||
let ba = b.bounds(*a).map_err(|e| label(qname, e))?;
|
||||
let bb = b.bounds(*bid).map_err(|e| label(qname, e))?;
|
||||
QueryResult::Intersects(ba.overlaps(&bb))
|
||||
}
|
||||
Query::GetGeometryRevision => QueryResult::Revision(b.revision()),
|
||||
Query::GetTextContent { id } => QueryResult::TextContent(b.text_content(*id)?),
|
||||
Query::GetHatchBoundary { id } => QueryResult::HatchBoundary(b.hatch_boundary(*id)?),
|
||||
Query::GetDimensionMeasurement { id } => QueryResult::DimensionMeasurement(b.dimension_measurement(*id)?),
|
||||
Query::GetAttributes { id } => QueryResult::Attributes(b.attributes(*id)?),
|
||||
Query::GetBlockEntities { block_name } => QueryResult::BlockEntities(b.block_entities(block_name)?),
|
||||
Query::GetTextContent { id } => QueryResult::TextContent(b.text_content(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetHatchBoundary { id } => QueryResult::HatchBoundary(b.hatch_boundary(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetDimensionMeasurement { id } => QueryResult::DimensionMeasurement(b.dimension_measurement(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetAttributes { id } => QueryResult::Attributes(b.attributes(*id).map_err(|e| label(qname, e))?),
|
||||
Query::GetBlockEntities { block_name } => QueryResult::BlockEntities(b.block_entities(block_name).map_err(|e| label(qname, e))?),
|
||||
Query::GetViewportView { id } => {
|
||||
let (target, height) = b.viewport_view(*id)?;
|
||||
let (target, height) = b.viewport_view(*id).map_err(|e| label(qname, e))?;
|
||||
QueryResult::ViewportView { target, height }
|
||||
}
|
||||
};
|
||||
@@ -366,6 +377,15 @@ pub fn apply_queries<B: DocApiBackend>(b: &mut B, queries: Vec<Query>) -> ApiRes
|
||||
})
|
||||
}
|
||||
|
||||
/// Attach the query name to an error for better diagnostics (uses `query_name`).
|
||||
fn label(query: &'static str, e: ApiError) -> ApiError {
|
||||
match e {
|
||||
ApiError::Validation { reason, .. } => ApiError::validation(query, reason),
|
||||
ApiError::UnknownId(id) => ApiError::validation(query, format!("unknown ObjectId {id:?}")),
|
||||
other => other,
|
||||
}
|
||||
}
|
||||
|
||||
fn profile_curves<B: DocApiBackend>(
|
||||
b: &B,
|
||||
profile: ObjectId,
|
||||
@@ -377,6 +397,21 @@ fn profile_curves<B: DocApiBackend>(
|
||||
b.profile_curves(profile)
|
||||
}
|
||||
|
||||
/// Validate a `Curve2Spec`'s fallible conditions BEFORE mutation (the same rules
|
||||
/// the host's `curve_spec_to_entity` enforces), so `CreateMany` stays all-or-nothing.
|
||||
/// Currently: ellipse minor/major ratio must be in (0, 1].
|
||||
fn validate_curve_spec(spec: &crate::ops::Curve2Spec) -> ApiResult<()> {
|
||||
if let crate::ops::Curve2Spec::Ellipse { ratio, .. } = spec {
|
||||
if !(*ratio > 0.0 && *ratio <= 1.0) {
|
||||
return Err(ApiError::validation(
|
||||
"CreateMany",
|
||||
format!("ellipse minor_axis_ratio must be in (0, 1], got {ratio}"),
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn require_exists<B: DocApiBackend>(b: &B, id: ObjectId, op: &'static str) -> ApiResult<()> {
|
||||
if b.entity_exists(id) {
|
||||
Ok(())
|
||||
|
||||
@@ -17,10 +17,6 @@ impl ObjectId {
|
||||
self.0
|
||||
}
|
||||
|
||||
pub const fn is_null(self) -> bool {
|
||||
self.0 == 0
|
||||
}
|
||||
|
||||
/// Build from a raw u64 (e.g. an `acadrust::Handle`'s value).
|
||||
pub const fn from_u64(v: u64) -> Self {
|
||||
Self(v)
|
||||
|
||||
@@ -19,10 +19,6 @@ impl<B: DocApiBackend> InProcess<B> {
|
||||
pub fn new(backend: B) -> Self {
|
||||
Self { backend: Mutex::new(backend) }
|
||||
}
|
||||
/// Borrow the backend mutably (test/introspection helper).
|
||||
pub fn backend_mut(&mut self) -> &mut B {
|
||||
self.backend.get_mut().expect("in-process backend poisoned")
|
||||
}
|
||||
/// Borrow the backend immutably (test/introspection helper).
|
||||
pub fn backend(&self) -> std::sync::MutexGuard<'_, B> {
|
||||
self.backend.lock().expect("in-process backend poisoned")
|
||||
|
||||
@@ -12,6 +12,38 @@ use ocs_doc_api::{
|
||||
HasId, ObjectId, PlacementSpec,
|
||||
};
|
||||
|
||||
/// Coarse bounds for a stored 2D curve spec (mirrors the host's entity_bounds and
|
||||
/// the example mock — all three must answer curve bounds identically).
|
||||
fn curve_bounds(spec: &Curve2Spec) -> Aabb {
|
||||
match spec {
|
||||
Curve2Spec::Line { start, end } => Aabb {
|
||||
min: [start[0].min(end[0]), start[1].min(end[1]), start[2].min(end[2])],
|
||||
max: [start[0].max(end[0]), start[1].max(end[1]), start[2].max(end[2])],
|
||||
},
|
||||
Curve2Spec::Circle { centre, radius } | Curve2Spec::Arc { centre, radius, .. } => Aabb {
|
||||
min: [centre[0] - radius, centre[1] - radius, centre[2]],
|
||||
max: [centre[0] + radius, centre[1] + radius, centre[2]],
|
||||
},
|
||||
Curve2Spec::Point { position } => Aabb { min: *position, max: *position },
|
||||
Curve2Spec::Polyline { points, .. } | Curve2Spec::Spline { control_points: points, .. } => {
|
||||
let mut min = [f64::INFINITY; 3];
|
||||
let mut max = [f64::NEG_INFINITY; 3];
|
||||
for p in points {
|
||||
for i in 0..3 {
|
||||
min[i] = min[i].min(p[i]);
|
||||
max[i] = max[i].max(p[i]);
|
||||
}
|
||||
}
|
||||
Aabb { min, max }
|
||||
}
|
||||
Curve2Spec::Ellipse { centre, major_axis, .. } => {
|
||||
let r = (major_axis[0] * major_axis[0] + major_axis[1] * major_axis[1] + major_axis[2] * major_axis[2]).sqrt();
|
||||
Aabb { min: [centre[0] - r, centre[1] - r, centre[2] - r], max: [centre[0] + r, centre[1] + r, centre[2] + r] }
|
||||
}
|
||||
Curve2Spec::Ray { origin, .. } | Curve2Spec::XLine { origin, .. } => Aabb { min: *origin, max: *origin },
|
||||
}
|
||||
}
|
||||
|
||||
/// In-memory mock backend: HashMap<ObjectId, Body> + a tiny entity table.
|
||||
/// Tracks undo steps + revision bumps to assert per-op semantics.
|
||||
#[derive(Default)]
|
||||
@@ -29,6 +61,9 @@ struct MockBackend {
|
||||
dimension_measurements: HashMap<ObjectId, f64>,
|
||||
/// Stored insert attributes.
|
||||
attributes_store: HashMap<ObjectId, Vec<(String, String)>>,
|
||||
/// Stored 2D curve specs so `bounds()` works on curve entities (aligned with
|
||||
/// the example mock — both mocks must answer curve bounds the same way).
|
||||
curves: HashMap<ObjectId, Curve2Spec>,
|
||||
/// Stored viewport views (target, height).
|
||||
viewport_views: HashMap<ObjectId, ([f64; 3], f64)>,
|
||||
}
|
||||
@@ -72,7 +107,9 @@ impl DocApiBackend for MockBackend {
|
||||
Curve2Spec::Ray { .. } => "Ray",
|
||||
Curve2Spec::XLine { .. } => "XLine",
|
||||
};
|
||||
Ok(self.alloc(kind))
|
||||
let id = self.alloc(kind);
|
||||
self.curves.insert(id, spec.clone());
|
||||
Ok(id)
|
||||
}
|
||||
fn add_insert(&mut self, spec: &ocs_doc_api::ops::InsertSpec) -> ApiResult<ObjectId> {
|
||||
if spec.block_name.is_empty() {
|
||||
@@ -237,6 +274,11 @@ impl DocApiBackend for MockBackend {
|
||||
Err(ApiError::Unsupported("profiles not mocked".into()))
|
||||
}
|
||||
fn bounds(&mut self, id: ObjectId) -> ApiResult<Aabb> {
|
||||
// Solids via the kernel body; 2D curves from their stored spec (aligned
|
||||
// with the example mock — both mocks answer curve bounds identically).
|
||||
if let Some(spec) = self.curves.get(&id) {
|
||||
return Ok(curve_bounds(spec));
|
||||
}
|
||||
let body = self.body(id)?;
|
||||
let bb = cadkernel::brep::body_bounds(body)
|
||||
.ok_or_else(|| ApiError::geometry(GeometryErrorKind::Empty, "no bounds"))?;
|
||||
|
||||
@@ -19,7 +19,14 @@
|
||||
/// vtable so API v2 plugins keep working. v4 adds full-duplex notifications on
|
||||
/// a multiplexed socket while leaving the V2/V3 ABI and protocol untouched. v5
|
||||
/// adds the `BuiltinPlugin::on_load` lifecycle callback.
|
||||
pub const API_VERSION: u32 = 5;
|
||||
/// Host ABI major version. Bumped to 6 when the DocApi v2 surface
|
||||
/// (`PluginRequest::DocApiRequest` / `PluginResponse::DocApiResponse` +
|
||||
/// `HostApi::doc_api_dispatch`) was added: plugins that call DocApi require an
|
||||
/// API-6 host and are rejected at handshake by older (API ≤ 5) hosts, instead of
|
||||
/// failing at runtime with a bincode-decode transport error. The DocApi wire
|
||||
/// changes are append-only and `doc_api_dispatch` is a defaulted method, so
|
||||
/// older V2–V5 plugins still load unchanged on an API-6 host.
|
||||
pub const API_VERSION: u32 = 6;
|
||||
|
||||
/// Oldest plugin API major the current host still loads. This keeps previously
|
||||
/// compiled cdylibs usable as long as their vtable layout is a prefix of the
|
||||
|
||||
+176
-17
@@ -19,8 +19,17 @@ use super::plugin_host::HostSession;
|
||||
/// Entry point called by the `HostApi::doc_api_dispatch` override (below) and by
|
||||
// the in-process path. Deserializes the envelope, runs the crate executor,
|
||||
// serializes the `Receipt` (or `ApiError`) back to bytes.
|
||||
pub fn execute_doc_api(host: &mut HostSession<'_>, _tab_id: u64, bytes: &[u8]) -> Result<Vec<u8>, String> {
|
||||
pub fn execute_doc_api(host: &mut HostSession<'_>, tab_id: u64, bytes: &[u8]) -> Result<Vec<u8>, String> {
|
||||
use ocs_doc_api::{DocApiEnvelope, EnvelopeBody};
|
||||
// Authorization: the request must name the SAME tab the HostSession is bound to.
|
||||
// The HostSession is built per-tab by the dispatch pump; a plugin naming a
|
||||
// different tab_id would otherwise reach a tab it isn't bound to (confused deputy).
|
||||
if tab_id != host.tab_id() {
|
||||
return Err(format!(
|
||||
"DocApi tab mismatch: request names tab {tab_id} but the bound tab is {}",
|
||||
host.tab_id()
|
||||
));
|
||||
}
|
||||
let envelope: DocApiEnvelope = bincode::deserialize(bytes)
|
||||
.map_err(|e| format!("DocApiEnvelope deserialize: {e}"))?;
|
||||
let result: ApiResult<ocs_doc_api::Receipt> = match envelope.body {
|
||||
@@ -30,11 +39,13 @@ pub fn execute_doc_api(host: &mut HostSession<'_>, _tab_id: u64, bytes: &[u8]) -
|
||||
bincode::serialize(&result).map_err(|e| format!("Receipt serialize: {e}"))
|
||||
}
|
||||
|
||||
// ObjectId <-> acadrust::Handle converters delegate to the crate's helpers
|
||||
// (single source of truth; no local duplicates).
|
||||
fn obj_to_handle(id: ObjectId) -> Handle {
|
||||
Handle::new(id.as_u64())
|
||||
id.to_handle()
|
||||
}
|
||||
fn handle_to_obj(h: Handle) -> ObjectId {
|
||||
ObjectId::from_u64(h.value())
|
||||
ObjectId::from_handle(h)
|
||||
}
|
||||
|
||||
impl DocApiBackend for HostSession<'_> {
|
||||
@@ -49,6 +60,18 @@ impl DocApiBackend for HostSession<'_> {
|
||||
.ok_or(ApiError::UnknownId(id))
|
||||
}
|
||||
|
||||
fn with_body<R>(
|
||||
&mut self,
|
||||
id: ObjectId,
|
||||
f: &mut dyn FnMut(&KernelBody) -> ApiResult<R>,
|
||||
) -> ApiResult<R> {
|
||||
let handle = obj_to_handle(id);
|
||||
// Lift-on-miss, then borrow the cache entry in place — O(1), no deep clone.
|
||||
self.scene_mut().restore_solid_models(&[handle]);
|
||||
let body = self.scene().solid_models.get(&handle).ok_or(ApiError::UnknownId(id))?;
|
||||
f(body)
|
||||
}
|
||||
|
||||
fn store_solid(&mut self, body: &KernelBody) -> ApiResult<ObjectId> {
|
||||
// Pre-flight the display prep BEFORE committing the entity: if the mesh
|
||||
// cannot be produced, fail now — no entity, no rollback, no revision bump
|
||||
@@ -409,6 +432,10 @@ impl DocApiBackend for HostSession<'_> {
|
||||
fn add_raster_image(&mut self, spec: &ocs_doc_api::ops::RasterImageSpec) -> ApiResult<ObjectId> {
|
||||
use acadrust::entities::RasterImage;
|
||||
use acadrust::types::{Vector2, Vector3};
|
||||
// Validate the plugin-supplied path before embedding it in the document:
|
||||
// non-empty, a known image extension, and not a UNC/network path or
|
||||
// parent-traversal (which could point the host at unintended resources).
|
||||
validate_image_path(&spec.file_path)?;
|
||||
let mut img = RasterImage::default();
|
||||
img.file_path = spec.file_path.clone();
|
||||
img.insertion_point = Vector3::new(spec.insertion_point[0], spec.insertion_point[1], spec.insertion_point[2]);
|
||||
@@ -574,15 +601,15 @@ impl DocApiBackend for HostSession<'_> {
|
||||
|
||||
fn bounds(&mut self, id: ObjectId) -> ApiResult<Aabb> {
|
||||
let handle = obj_to_handle(id);
|
||||
// Lift-on-miss for solids (consistent with volume/centroid): resolve_body
|
||||
// repopulates a cold solid_models cache from SAT. A solid whose cache entry
|
||||
// was dropped (e.g. by a host-side update_entity) no longer misreports
|
||||
// `Unsupported` (plan review).
|
||||
// Lift-on-miss for solids (consistent with volume/centroid); borrow the cache
|
||||
// read-only via with_body (O(1), no B-rep deep clone).
|
||||
if matches!(self.document().get_entity(handle), Some(EntityType::Solid3D(_))) {
|
||||
let body = self.resolve_body(id)?;
|
||||
let bb = cadkernel::brep::body_bounds(&body)
|
||||
.ok_or_else(|| ApiError::geometry(GeometryErrorKind::Empty, "no bounds"))?;
|
||||
return Ok(Aabb { min: bb.min, max: bb.max });
|
||||
let mut f = |body: &KernelBody| -> ApiResult<Aabb> {
|
||||
let bb = cadkernel::brep::body_bounds(body)
|
||||
.ok_or_else(|| ApiError::geometry(GeometryErrorKind::Empty, "no bounds"))?;
|
||||
Ok(Aabb { min: bb.min, max: bb.max })
|
||||
};
|
||||
return self.with_body(id, &mut f);
|
||||
}
|
||||
crate::app::doc_api_convert::entity_bounds(self.document().get_entity(handle), id)
|
||||
}
|
||||
@@ -626,7 +653,7 @@ impl HostSession<'_> {
|
||||
fn mass_properties(&mut self, id: ObjectId) -> ApiResult<(f64, [f64; 3])> {
|
||||
let handle = obj_to_handle(id);
|
||||
let epoch = self.scene().geometry_epoch;
|
||||
if let Some(&(cached_epoch, v, c)) = self.mass_cache.get(&handle) {
|
||||
if let Some(&(cached_epoch, v, c)) = self.scene().doc_api_mass_cache.get(&handle) {
|
||||
if cached_epoch == epoch {
|
||||
return Ok((v, c));
|
||||
}
|
||||
@@ -635,8 +662,10 @@ impl HostSession<'_> {
|
||||
// converge to near-analytic (the render-mesh metrics cache is the coarse
|
||||
// display LOD, NOT for query accuracy). Bounded by the cold-tess budget;
|
||||
// memoized per (handle, epoch) so repeated queries on the same solid are O(1).
|
||||
// The budget + cache live on the per-tab Scene so they persist across
|
||||
// DocApi dispatches (not reset per request).
|
||||
const COLD_TESS_BUDGET: usize = 256;
|
||||
if self.cold_tess_used >= COLD_TESS_BUDGET {
|
||||
if self.scene().doc_api_cold_tess_used >= COLD_TESS_BUDGET {
|
||||
// Budget exhausted: serve a documented-approximate value from the render
|
||||
// mesh metrics cache rather than failing (large distinct-solid batches
|
||||
// degrade gracefully instead of erroring). Accuracy-first up to the budget.
|
||||
@@ -646,18 +675,47 @@ impl HostSession<'_> {
|
||||
}
|
||||
}
|
||||
return Err(ApiError::Unsupported(
|
||||
"volume/centroid cold-tessellation budget exceeded for this session; query fewer distinct solids".into(),
|
||||
"volume/centroid cold-tessellation budget exceeded for this tab; query fewer distinct solids".into(),
|
||||
));
|
||||
}
|
||||
self.cold_tess_used += 1;
|
||||
self.scene_mut().doc_api_cold_tess_used += 1;
|
||||
let body = self.resolve_body(id)?;
|
||||
let mesh = cadkernel::brep::mesh_body(&body, 0.1, 1e-4);
|
||||
let (v, c) = mesh_volume_centroid(&mesh);
|
||||
self.mass_cache.insert(handle, (epoch, v, c));
|
||||
self.scene_mut().doc_api_mass_cache.insert(handle, (epoch, v, c));
|
||||
Ok((v, c))
|
||||
}
|
||||
}
|
||||
|
||||
/// Validate a raster-image file path before embedding it in a document.
|
||||
/// Rejects empty paths, UNC/network paths, parent-traversal, and non-image extensions.
|
||||
fn validate_image_path(path: &str) -> ApiResult<()> {
|
||||
let p = path.trim();
|
||||
if p.is_empty() {
|
||||
return Err(ApiError::validation("CreateRasterImage", "empty image file path"));
|
||||
}
|
||||
// UNC / network / device paths.
|
||||
if p.starts_with("\\\\") || p.starts_with("//") {
|
||||
return Err(ApiError::Unsupported("network/UNC image paths are not allowed".into()));
|
||||
}
|
||||
// Parent-traversal anywhere in the path.
|
||||
if p.split(['/', '\\']).any(|seg| seg == "..") {
|
||||
return Err(ApiError::Unsupported("parent-traversal ('..') in image path is not allowed".into()));
|
||||
}
|
||||
// Known image extension (last component).
|
||||
let ext_ok = p.rsplit('.').next().map(|ext| {
|
||||
matches!(ext.to_ascii_lowercase().as_str(),
|
||||
"png" | "jpg" | "jpeg" | "bmp" | "gif" | "tif" | "tiff" | "webp" | "tga")
|
||||
}).unwrap_or(false);
|
||||
if !ext_ok {
|
||||
return Err(ApiError::validation(
|
||||
"CreateRasterImage",
|
||||
format!("unsupported image file extension in {path:?}"),
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn kernel_placement(p: &PlacementSpec) -> cadkernel::brep::Placement {
|
||||
cadkernel::brep::Placement {
|
||||
x_axis: p.x_axis,
|
||||
@@ -687,7 +745,8 @@ mod tests {
|
||||
|
||||
fn dispatch(host: &mut HostSession<'_>, env: DocApiEnvelope) -> ApiResult<Receipt> {
|
||||
let bytes = bincode::serialize(&env).unwrap();
|
||||
let out = execute_doc_api(host, 0, &bytes).expect("dispatch failed");
|
||||
let tab_id = host.tab_id();
|
||||
let out = execute_doc_api(host, tab_id, &bytes).expect("dispatch failed");
|
||||
bincode::deserialize(&out).expect("receipt deserialize")
|
||||
}
|
||||
|
||||
@@ -1156,6 +1215,25 @@ mod tests {
|
||||
if let cadkernel::geom2d::Curve::Arc(arc) = &curves[0] {
|
||||
assert!((arc.radius - 5.0).abs() < 0.01, "arc radius {} (semicircle)", arc.radius);
|
||||
}
|
||||
|
||||
// Negative (CW) bulge: the center must be on the CW side (mirror of CCW).
|
||||
// A single 90° bulge segment start=(0,0), end=(1,0), bulge=-0.4142 (~90° CW):
|
||||
// correct center is BELOW the chord; the buggy double-sign put it above.
|
||||
let neg = crate::app::doc_api_convert::bulge_arc_segment([0.0, 0.0], [1.0, 0.0], -0.4142)
|
||||
.expect("negative bulge arc");
|
||||
if let cadkernel::geom2d::Curve::Arc(arc) = &neg {
|
||||
// 90° arc, chord 1: radius = 0.5/sin(45°) ≈ 0.7071; center y must be negative.
|
||||
assert!((arc.radius - 0.7071).abs() < 0.01, "cw arc radius {}", arc.radius);
|
||||
assert!(arc.centre[1] < 0.0, "CW arc center y must be below the chord, got {:?}", arc.centre);
|
||||
} else {
|
||||
panic!("expected an arc for negative bulge");
|
||||
}
|
||||
// And the CCW mirror: center above.
|
||||
let pos = crate::app::doc_api_convert::bulge_arc_segment([0.0, 0.0], [1.0, 0.0], 0.4142)
|
||||
.expect("positive bulge arc");
|
||||
if let cadkernel::geom2d::Curve::Arc(arc) = &pos {
|
||||
assert!(arc.centre[1] > 0.0, "CCW arc center y must be above the chord, got {:?}", arc.centre);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Phase 2c-ii: dimension sub-types (radius/diameter/angular) ──────────
|
||||
@@ -1309,6 +1387,75 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
// ── Review-fix regression tests (Loft cap, CreateMany validation, Ellipse
|
||||
// rotation, raster path validation) ─────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn fix_loft_over_cap_is_rejected() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
let mut host = HostSession::new(&mut app, 0);
|
||||
let over = ocs_doc_api::ops::BULK_ITEM_CAP + 1;
|
||||
let profiles: Vec<ObjectId> = (0..over).map(|i| ObjectId::from_u64(i as u64)).collect();
|
||||
let err = dispatch(&mut host, DocApiEnvelope::op(Operation::Loft { profiles })).unwrap_err();
|
||||
assert!(matches!(err, ApiError::Validation { .. }), "{err:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fix_create_many_invalid_curve_is_all_or_nothing() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
let mut host = HostSession::new(&mut app, 0);
|
||||
let rev_before = host.scene().geometry_epoch;
|
||||
// A valid point + an invalid ellipse (ratio > 1) in one CreateMany batch.
|
||||
let specs = vec![
|
||||
ocs_doc_api::ops::EntitySpec::Curve(Curve2Spec::Point { position: [0.0; 3] }),
|
||||
ocs_doc_api::ops::EntitySpec::Curve(Curve2Spec::Ellipse {
|
||||
centre: [0.0; 3], major_axis: [1.0; 3], ratio: 2.0, start: 0.0, end: std::f64::consts::TAU,
|
||||
}),
|
||||
];
|
||||
let err = dispatch(&mut host, DocApiEnvelope::op(Operation::CreateMany(specs))).unwrap_err();
|
||||
assert!(matches!(err, ApiError::Validation { .. }), "{err:?}");
|
||||
// All-or-nothing: NO entity was created (the point would have been added first).
|
||||
assert_eq!(host.scene().geometry_epoch, rev_before, "no mutation on rejected CreateMany");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fix_partial_ellipse_rotates_sweep() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
let mut host = HostSession::new(&mut app, 0);
|
||||
// Partial ellipse arc start=0,end=pi/2; rotate by +pi/2 -> sweep becomes pi/2..pi.
|
||||
let ell = new_id(&dispatch(&mut host, DocApiEnvelope::op(Operation::CreateCurve(
|
||||
Curve2Spec::Ellipse { centre: [0.0; 3], major_axis: [4.0, 0.0, 0.0], ratio: 0.5, start: 0.0, end: std::f64::consts::FRAC_PI_2 }))).unwrap());
|
||||
let rot90 = ocs_doc_api::PlacementSpec {
|
||||
x_axis: [0.0, 1.0, 0.0], y_axis: [-1.0, 0.0, 0.0], z_axis: [0.0, 0.0, 1.0], origin: [0.0; 3],
|
||||
};
|
||||
dispatch(&mut host, DocApiEnvelope::op(Operation::Transform { id: ell, placement: rot90 })).unwrap();
|
||||
let Some(EntityType::Ellipse(e)) = host.document().get_entity(obj_to_handle(ell)) else { panic!("ellipse not found") };
|
||||
let half_pi = std::f64::consts::FRAC_PI_2;
|
||||
assert!((e.start_parameter - half_pi).abs() < 1e-6, "start {} (expected {})", e.start_parameter, half_pi);
|
||||
assert!((e.end_parameter - std::f64::consts::PI).abs() < 1e-6, "end {} (expected pi)", e.end_parameter);
|
||||
// major_axis rotated from +X to +Y.
|
||||
assert!((e.major_axis.y - 4.0).abs() < 1e-6, "major_axis {:?}", e.major_axis);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fix_raster_image_path_validation() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
let mut host = HostSession::new(&mut app, 0);
|
||||
let mk = |path: &str| Operation::CreateRasterImage(ocs_doc_api::ops::RasterImageSpec {
|
||||
file_path: path.into(), insertion_point: [0.0; 3], u_vector: [0.5, 0.0, 0.0], v_vector: [0.0, 0.5, 0.0], size: [10.0, 10.0],
|
||||
});
|
||||
// Valid path succeeds.
|
||||
assert!(dispatch(&mut host, DocApiEnvelope::op(mk("C:/img/photo.png"))).is_ok());
|
||||
// UNC path rejected.
|
||||
assert!(dispatch(&mut host, DocApiEnvelope::op(mk("\\\\server\\share\\x.png"))).is_err());
|
||||
// Parent-traversal rejected.
|
||||
assert!(dispatch(&mut host, DocApiEnvelope::op(mk("C:/img/../../etc/passwd.png"))).is_err());
|
||||
// Non-image extension rejected.
|
||||
assert!(dispatch(&mut host, DocApiEnvelope::op(mk("C:/img/evil.exe"))).is_err());
|
||||
// Empty path rejected.
|
||||
assert!(dispatch(&mut host, DocApiEnvelope::op(mk(""))).is_err());
|
||||
}
|
||||
|
||||
// ── Read-mostly family bounds (Leader/Mesh/Face3D/MLine/Helix) ──────────
|
||||
|
||||
#[test]
|
||||
@@ -1603,6 +1750,18 @@ mod tests {
|
||||
assert_eq!(host.scene().geometry_epoch, before, "no entity on unknown block");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn doc_api_tab_mismatch_is_rejected() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
let mut host = HostSession::new(&mut app, 0);
|
||||
// A request naming a DIFFERENT tab than the bound one is rejected.
|
||||
let env = DocApiEnvelope::op(Operation::CreateCurve(Curve2Spec::Point { position: [0.0; 3] }));
|
||||
let bytes = bincode::serialize(&env).unwrap();
|
||||
let wrong_tab = host.tab_id().wrapping_add(999);
|
||||
let err = execute_doc_api(&mut host, wrong_tab, &bytes).unwrap_err();
|
||||
assert!(err.contains("tab mismatch"), "{err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn doc_api_unknown_id_surfaces_structured_error() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
|
||||
+14
-23
@@ -115,6 +115,9 @@ pub fn transform_entity_geometry(
|
||||
e.center = apply(e.center);
|
||||
// The major-axis is a direction+length vector: rotate+scale it (no origin).
|
||||
e.major_axis = apply_dir(e.major_axis);
|
||||
// A Z-rotation also shifts the partial-arc sweep (eccentric-anomaly params).
|
||||
e.start_parameter += rot_z;
|
||||
e.end_parameter += rot_z;
|
||||
EntityType::Ellipse(e)
|
||||
}
|
||||
EntityType::Spline(sp) => {
|
||||
@@ -191,7 +194,7 @@ fn v3(p: [f64; 3]) -> Vector3 {
|
||||
|
||||
/// Convert a polyline bulge segment (start → end, bulge = tan(θ/4)) to a
|
||||
/// `geom2d::Curve::Arc`. Positive bulge = counter-clockwise arc; negative = clockwise.
|
||||
fn bulge_arc_segment(start: [f64; 2], end: [f64; 2], bulge: f64) -> ApiResult<cadkernel::geom2d::Curve> {
|
||||
pub(crate) fn bulge_arc_segment(start: [f64; 2], end: [f64; 2], bulge: f64) -> ApiResult<cadkernel::geom2d::Curve> {
|
||||
use cadkernel::geom2d::{Arc as KArc, Curve as KCurve};
|
||||
let chord_x = end[0] - start[0];
|
||||
let chord_y = end[1] - start[1];
|
||||
@@ -207,11 +210,13 @@ fn bulge_arc_segment(start: [f64; 2], end: [f64; 2], bulge: f64) -> ApiResult<ca
|
||||
return Err(ApiError::validation("profile", "bulge segment with degenerate angle"));
|
||||
}
|
||||
let radius = (chord_len / 2.0) / sin_half.abs();
|
||||
// Center: chord midpoint + perpendicular offset (chord/2)/tan(θ/2).
|
||||
// Center: chord midpoint + perpendicular offset. The apothem MAGNITUDE is
|
||||
// (chord/2)/|tan(θ/2)| (always positive); the SIGN of the bulge alone picks the
|
||||
// side (CCW=+90° perp, CW=-90° perp). Using signed tan(half) AND the sign flip
|
||||
// would double-count and mirror CW arcs to the wrong side.
|
||||
let mid_x = (start[0] + end[0]) / 2.0;
|
||||
let mid_y = (start[1] + end[1]) / 2.0;
|
||||
let apothem = (chord_len / 2.0) / half.tan();
|
||||
// Perpendicular to chord (rotate chord unit vector by +90° for CCW bulge).
|
||||
let apothem = (chord_len / 2.0) / half.tan().abs();
|
||||
let sign = if bulge >= 0.0 { 1.0 } else { -1.0 };
|
||||
let perp_x = -chord_y / chord_len * sign;
|
||||
let perp_y = chord_x / chord_len * sign;
|
||||
@@ -392,15 +397,7 @@ pub fn entity_bounds(entity: Option<&EntityType>, id: ObjectId) -> ApiResult<Aab
|
||||
max: [e.center.x + r, e.center.y + r, e.center.z + r],
|
||||
}
|
||||
}
|
||||
EntityType::Spline(s) if !s.control_points.is_empty() => {
|
||||
let mut min = [f64::INFINITY; 3];
|
||||
let mut max = [f64::NEG_INFINITY; 3];
|
||||
for p in &s.control_points {
|
||||
min = [min[0].min(p.x), min[1].min(p.y), min[2].min(p.z)];
|
||||
max = [max[0].max(p.x), max[1].max(p.y), max[2].max(p.z)];
|
||||
}
|
||||
Aabb { min, max }
|
||||
}
|
||||
EntityType::Spline(s) => points_aabb(s.control_points.iter().collect(), id)?,
|
||||
EntityType::Viewport(vp) => Aabb {
|
||||
min: [
|
||||
vp.center.x - vp.width / 2.0,
|
||||
@@ -443,16 +440,10 @@ pub fn entity_bounds(entity: Option<&EntityType>, id: ObjectId) -> ApiResult<Aab
|
||||
min: [t.insertion_point.x, t.insertion_point.y - t.height, t.insertion_point.z],
|
||||
max: [t.insertion_point.x + t.height * t.value.len() as f64, t.insertion_point.y, t.insertion_point.z],
|
||||
},
|
||||
EntityType::Dimension(acadrust::entities::Dimension::Linear(d)) => {
|
||||
let pts = [d.first_point, d.second_point, d.definition_point];
|
||||
let mut min = [f64::INFINITY; 3];
|
||||
let mut max = [f64::NEG_INFINITY; 3];
|
||||
for p in pts {
|
||||
min = [min[0].min(p.x), min[1].min(p.y), min[2].min(p.z)];
|
||||
max = [max[0].max(p.x), max[1].max(p.y), max[2].max(p.z)];
|
||||
}
|
||||
Aabb { min, max }
|
||||
}
|
||||
EntityType::Dimension(acadrust::entities::Dimension::Linear(d)) => points_aabb(
|
||||
[&d.first_point, &d.second_point, &d.definition_point].into_iter().collect(),
|
||||
id,
|
||||
)?,
|
||||
EntityType::Hatch(h) => {
|
||||
let mut min = [f64::INFINITY; 3];
|
||||
let mut max = [f64::NEG_INFINITY; 3];
|
||||
|
||||
@@ -18,10 +18,6 @@ pub(crate) struct HostSession<'a> {
|
||||
doc_store: Option<DocumentSnapshotStore<DocumentViewData>>,
|
||||
/// In-flight DocApi v2 undo delta (between `push_undo` and `finalize_op`).
|
||||
doc_api_pending: Option<super::history::PendingDelta>,
|
||||
/// Memoized volume/centroid per (handle, geometry_epoch) for DocApi mass queries.
|
||||
pub(crate) mass_cache: std::collections::HashMap<Handle, (u64, f64, [f64; 3])>,
|
||||
/// Per-session count of cold-cache mass tessellations (budget for query DoS).
|
||||
pub(crate) cold_tess_used: usize,
|
||||
}
|
||||
|
||||
impl<'a> HostSession<'a> {
|
||||
@@ -31,8 +27,6 @@ impl<'a> HostSession<'a> {
|
||||
tab,
|
||||
doc_store: None,
|
||||
doc_api_pending: None,
|
||||
mass_cache: std::collections::HashMap::new(),
|
||||
cold_tess_used: 0,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1804,6 +1804,14 @@ pub struct Scene {
|
||||
pub block_meshes: HashMap<Handle, MeshLodSet>,
|
||||
/// Kernel B-reps used by solid operations and exact-geometry saves.
|
||||
pub solid_models: HashMap<Handle, cadkernel::brep::Body>,
|
||||
/// Memoized DocApi volume/centroid per (handle, geometry_epoch) so repeated
|
||||
/// mass queries on the same solid are O(1). Persists across DocApi dispatches.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub doc_api_mass_cache: std::collections::HashMap<Handle, (u64, f64, [f64; 3])>,
|
||||
/// Per-tab cold-tessellation budget for expensive DocApi volume/centroid
|
||||
/// computations (bounded DoS surface). Persists across DocApi dispatches.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
pub doc_api_cold_tess_used: usize,
|
||||
/// GPU render data for raster images (RasterImage entities), keyed by handle.
|
||||
pub images: HashMap<Handle, ImageModel>,
|
||||
/// The viewport that is currently "entered" (MSPACE mode).
|
||||
@@ -2067,6 +2075,10 @@ impl Scene {
|
||||
material_base_dir: None,
|
||||
block_meshes: HashMap::default(),
|
||||
solid_models: HashMap::default(),
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
doc_api_mass_cache: std::collections::HashMap::default(),
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
doc_api_cold_tess_used: 0,
|
||||
images: HashMap::default(),
|
||||
active_viewport: None,
|
||||
bg_color: [33.0 / 255.0, 40.0 / 255.0, 48.0 / 255.0, 1.0],
|
||||
|
||||
Reference in new issue
Block a user