diff --git a/crates/shift-font/src/changes.rs b/crates/shift-font/src/changes.rs index 98fa79b3..c52ac965 100644 --- a/crates/shift-font/src/changes.rs +++ b/crates/shift-font/src/changes.rs @@ -1,6 +1,7 @@ use crate::{ - Anchor, AnchorId, Axis, AxisId, Contour, ContourId, Glyph, GlyphId, GlyphLayer, GlyphName, - LayerId, Point, PointId, PointType, Source, SourceId, + Anchor, AnchorId, Axis, AxisId, Component, ComponentId, Contour, ContourId, + DecomposedTransform, Glyph, GlyphId, GlyphLayer, GlyphName, Guideline, LayerId, LibData, Point, + PointId, PointType, Source, SourceId, SourceRole, }; #[derive(Clone, Debug, Default)] @@ -187,11 +188,19 @@ impl From<&Axis> for AxisCreated { } } +/// Full source snapshot: replays (undo of a source delete, crash-recovery +/// change application) must restore every persisted field, not just the +/// name and location. #[derive(Clone, Debug)] pub struct SourceCreated { pub source_id: SourceId, pub name: String, pub location: Vec, + pub filename: Option, + pub color: Option, + pub role: SourceRole, + pub layer_name: Option, + pub lib: LibData, } impl From<&Source> for SourceCreated { @@ -207,6 +216,11 @@ impl From<&Source> for SourceCreated { value: *value, }) .collect(), + filename: source.filename().map(str::to_owned), + color: source.color().map(str::to_owned), + role: source.role(), + layer_name: source.layer_name().map(str::to_owned), + lib: source.lib().clone(), } } } @@ -371,12 +385,18 @@ pub struct LayerGeometryReplaced { pub layer: GlyphLayerValue, } +/// The layer's full authored content. Replays restore deleted layers from +/// this value, so it must cover everything the store persists per layer — +/// not just the editable geometry. #[derive(Clone, Debug)] pub struct GlyphLayerValue { pub width: f64, pub height: Option, pub contours: Vec, pub anchors: Vec, + pub components: Vec, + pub guidelines: Vec, + pub lib: LibData, } impl From<&GlyphLayer> for GlyphLayerValue { @@ -390,6 +410,36 @@ impl From<&GlyphLayer> for GlyphLayerValue { .enumerate() .map(|(order_index, anchor)| AnchorValue::from_anchor(order_index, anchor)) .collect(), + components: layer + .components_iter() + .enumerate() + .map(|(order_index, component)| { + ComponentValue::from_component(order_index, component) + }) + .collect(), + guidelines: layer.guidelines().to_vec(), + lib: layer.lib().clone(), + } + } +} + +#[derive(Clone, Debug)] +pub struct ComponentValue { + pub id: ComponentId, + pub order_index: usize, + pub base_glyph_id: GlyphId, + pub base_glyph_name: GlyphName, + pub transform: DecomposedTransform, +} + +impl ComponentValue { + pub fn from_component(order_index: usize, component: &Component) -> Self { + Self { + id: component.id(), + order_index, + base_glyph_id: component.base_glyph_id(), + base_glyph_name: component.base_glyph_name().clone(), + transform: *component.transform(), } } } diff --git a/crates/shift-store/src/change_set.rs b/crates/shift-store/src/change_set.rs index 6263cc2c..36844316 100644 --- a/crates/shift-store/src/change_set.rs +++ b/crates/shift-store/src/change_set.rs @@ -1,4 +1,4 @@ -use rusqlite::{Transaction, params}; +use rusqlite::{OptionalExtension, Transaction, params}; use shift_font as font; use crate::{ @@ -59,11 +59,11 @@ impl ShiftStore { replace_font_binaries(&tx, "image", font.images())?; replace_kerning(&tx, font.kerning())?; - for (order_index, axis) in font.axes().iter().enumerate() { - insert_axis_with_order(&tx, &font::AxisCreated::from(axis), order_index as i64)?; + for axis in font.axes() { + insert_axis(&tx, &font::AxisCreated::from(axis))?; } - for (order_index, source) in font.sources().iter().enumerate() { + for source in font.sources() { upsert_source( &tx, &source.id(), @@ -73,7 +73,6 @@ impl ShiftStore { color: source.color(), kind: SourceKind::from(source.role()), layer_name: source.layer_name(), - order_index: order_index as i64, }, )?; replace_lib_data( @@ -92,8 +91,8 @@ impl ShiftStore { } } - for (order_index, glyph) in font.glyphs().enumerate() { - upsert_glyph(&tx, &glyph.id(), glyph.glyph_name(), order_index as i64)?; + for glyph in font.glyphs() { + upsert_glyph(&tx, &glyph.id(), glyph.glyph_name())?; replace_glyph_unicodes(&tx, &glyph.id(), glyph.unicodes())?; replace_lib_data( &tx, @@ -113,7 +112,7 @@ impl ShiftStore { layer.width(), layer.height(), )?; - replace_full_layer_state(&tx, layer)?; + replace_layer_state(&tx, &layer.id(), &font::GlyphLayerValue::from(layer))?; } } @@ -124,15 +123,10 @@ impl ShiftStore { fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), StoreError> { match change { - font::FontChange::AxisCreated(change) => insert_axis_with_order(tx, change, 0), + font::FontChange::AxisCreated(change) => insert_axis(tx, change), font::FontChange::AxisDeleted(change) => { // source_locations cascade from the axis row. - let rows_changed = tx.execute( - "DELETE FROM axes WHERE id = ?1", - [change.axis_id.to_string()], - )?; - require_changed(rows_changed, "axis", change.axis_id.to_string())?; - Ok(()) + delete_ordered_row(tx, "axes", "axis", change.axis_id.to_string()) } font::FontChange::SourceCreated(change) => { upsert_source( @@ -140,48 +134,55 @@ fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), S &change.source_id, SourceRow { name: Some(&change.name), - filename: None, - color: None, - kind: SourceKind::Master, - layer_name: None, - order_index: 0, + filename: change.filename.as_deref(), + color: change.color.as_deref(), + kind: SourceKind::from(change.role), + layer_name: change.layer_name.as_deref(), }, )?; + replace_lib_data( + tx, + "source_lib", + "source_id", + Some(&change.source_id.to_string()), + &change.lib, + )?; for axis_value in &change.location { - upsert_source_location( - tx, - &change.source_id, - &axis_value.axis_id, - axis_value.value, - )?; + // Location entries on axes with no row have nothing to + // reference: the axis either never existed in the store + // (source formats allow locations on undefined axes) or a + // later step of the same undo replay restores it and + // re-emits this source's locations. + if axis_exists(tx, &axis_value.axis_id)? { + upsert_source_location( + tx, + &change.source_id, + &axis_value.axis_id, + axis_value.value, + )?; + } } Ok(()) } font::FontChange::SourceDeleted(change) => { // glyph_layers and source_locations cascade on the source row. - let rows_changed = tx.execute( - "DELETE FROM sources WHERE id = ?1", - [change.source_id.to_string()], - )?; - require_changed(rows_changed, "source", change.source_id.to_string())?; - Ok(()) + delete_ordered_row(tx, "sources", "source", change.source_id.to_string()) } font::FontChange::GlyphCreated(change) => { - upsert_glyph(tx, &change.glyph_id, &change.name, 0)?; + upsert_glyph(tx, &change.glyph_id, &change.name)?; replace_glyph_unicodes(tx, &change.glyph_id, &change.unicodes) } font::FontChange::GlyphDeleted(change) => { + delete_ordered_row(tx, "glyphs", "glyph", change.glyph_id.to_string()) + } + font::FontChange::GlyphIdentityChanged(change) => { let rows_changed = tx.execute( - "DELETE FROM glyphs WHERE id = ?1", - [change.glyph_id.to_string()], + "UPDATE glyphs SET name = ?2 WHERE id = ?1", + params![change.glyph_id.to_string(), change.to_name.as_str()], )?; require_changed(rows_changed, "glyph", change.glyph_id.to_string())?; - Ok(()) - } - font::FontChange::GlyphIdentityChanged(change) => { - upsert_glyph(tx, &change.glyph_id, &change.to_name, 0)?; replace_glyph_unicodes(tx, &change.glyph_id, &change.to_unicodes) } font::FontChange::GlyphLayerCreated(change) => upsert_layer( @@ -283,20 +284,16 @@ fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), S } font::FontChange::LayerGeometryReplaced(change) => { require_layer_exists(tx, &change.layer_id)?; - replace_layer_geometry(tx, &change.layer_id, &change.layer) + replace_layer_state(tx, &change.layer_id, &change.layer) } } } -fn insert_axis_with_order( - tx: &Transaction<'_>, - axis: &font::AxisCreated, - order_index: i64, -) -> Result<(), StoreError> { +fn insert_axis(tx: &Transaction<'_>, axis: &font::AxisCreated) -> Result<(), StoreError> { tx.execute( " INSERT INTO axes (id, tag, name, min_value, default_value, max_value, hidden, order_index) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, (SELECT COUNT(*) FROM axes)) ", params![ axis.axis_id.to_string(), @@ -306,12 +303,47 @@ fn insert_axis_with_order( axis.default, axis.maximum, axis.hidden, - order_index, ], )?; Ok(()) } +fn axis_exists(tx: &Transaction<'_>, axis_id: &font::AxisId) -> Result { + let exists: bool = tx.query_row( + "SELECT EXISTS(SELECT 1 FROM axes WHERE id = ?1)", + [axis_id.to_string()], + |row| row.get(0), + )?; + Ok(exists) +} + +/// Deletes an ordered row and shifts later rows down, mirroring the +/// in-memory `Vec::remove` so `order_index` stays a dense 0..n sequence +/// and creations can always append at `COUNT(*)`. +fn delete_ordered_row( + tx: &Transaction<'_>, + table: &'static str, + kind: &'static str, + id: String, +) -> Result<(), StoreError> { + let select_sql = format!("SELECT order_index FROM {table} WHERE id = ?1"); + let order_index: i64 = tx + .query_row(&select_sql, [id.as_str()], |row| row.get(0)) + .optional()? + .ok_or(StoreError::MissingEntity { + kind, + id: id.clone(), + })?; + + let delete_sql = format!("DELETE FROM {table} WHERE id = ?1"); + tx.execute(&delete_sql, [id.as_str()])?; + + let shift_sql = + format!("UPDATE {table} SET order_index = order_index - 1 WHERE order_index > ?1"); + tx.execute(&shift_sql, [order_index])?; + Ok(()) +} + fn upsert_source_location( tx: &Transaction<'_>, source_id: &font::SourceId, @@ -336,9 +368,11 @@ struct SourceRow<'a> { color: Option<&'a str>, kind: SourceKind, layer_name: Option<&'a str>, - order_index: i64, } +/// A fresh row appends at the end (order_index = current row count, +/// mirroring the in-memory `Vec::push`); a conflicting upsert keeps the +/// existing row's position. fn upsert_source( tx: &Transaction<'_>, source_id: &font::SourceId, @@ -347,14 +381,13 @@ fn upsert_source( tx.execute( " INSERT INTO sources (id, name, filename, color, kind, layer_name, order_index) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, (SELECT COUNT(*) FROM sources)) ON CONFLICT(id) DO UPDATE SET name = COALESCE(excluded.name, sources.name), filename = excluded.filename, color = excluded.color, kind = excluded.kind, - layer_name = excluded.layer_name, - order_index = excluded.order_index + layer_name = excluded.layer_name ", params![ source_id.to_string(), @@ -363,27 +396,26 @@ fn upsert_source( row.color, row.kind.as_str(), row.layer_name, - row.order_index ], )?; Ok(()) } +/// A fresh row appends at the end (order_index = current row count); a +/// conflicting upsert keeps the existing row's position. fn upsert_glyph( tx: &Transaction<'_>, glyph_id: &font::GlyphId, name: &font::GlyphName, - order_index: i64, ) -> Result<(), StoreError> { tx.execute( " INSERT INTO glyphs (id, name, order_index) - VALUES (?1, ?2, ?3) + VALUES (?1, ?2, (SELECT COUNT(*) FROM glyphs)) ON CONFLICT(id) DO UPDATE SET - name = excluded.name, - order_index = excluded.order_index + name = excluded.name ", - params![glyph_id.to_string(), name.as_str(), order_index], + params![glyph_id.to_string(), name.as_str()], )?; Ok(()) } @@ -441,7 +473,11 @@ fn upsert_layer( Ok(()) } -fn replace_layer_geometry( +/// Replaces the layer's full persisted content — metrics, contours, +/// anchors, components, guidelines, and lib — from a +/// [`font::GlyphLayerValue`] snapshot, so replays restoring a deleted +/// layer are lossless. +fn replace_layer_state( tx: &Transaction<'_>, layer_id: &font::LayerId, layer: &font::GlyphLayerValue, @@ -479,34 +515,25 @@ fn replace_layer_geometry( insert_anchor(tx, layer_id, anchor)?; } - Ok(()) -} - -fn replace_full_layer_state( - tx: &Transaction<'_>, - layer: &font::GlyphLayer, -) -> Result<(), StoreError> { - replace_layer_geometry(tx, &layer.id(), &font::GlyphLayerValue::from(layer))?; - tx.execute( " DELETE FROM glyph_components WHERE layer_id = ?1 ", - [layer_row_id(&layer.id())], + [layer_row_id(layer_id)], )?; - for (order_index, component) in layer.components_iter().enumerate() { - insert_component(tx, &layer.id(), component, order_index)?; + for component in &layer.components { + insert_component(tx, layer_id, component)?; } - replace_layer_guidelines(tx, &layer.id(), layer.guidelines())?; + replace_layer_guidelines(tx, layer_id, &layer.guidelines)?; replace_lib_data( tx, "glyph_layer_lib", "layer_id", - Some(&layer.id().to_string()), - layer.lib(), + Some(&layer_row_id(layer_id)), + &layer.lib, )?; Ok(()) @@ -711,10 +738,9 @@ fn insert_guideline( fn insert_component( tx: &Transaction<'_>, layer_id: &font::LayerId, - component: &font::Component, - order_index: usize, + component: &font::ComponentValue, ) -> Result<(), StoreError> { - let transform = component.transform(); + let transform = &component.transform; tx.execute( " INSERT INTO glyph_components ( @@ -736,10 +762,10 @@ fn insert_component( VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14) ", params![ - component.id().to_string(), + component.id.to_string(), layer_row_id(layer_id), - component.base_glyph_id().to_string(), - component.base_glyph_name().as_str(), + component.base_glyph_id.to_string(), + component.base_glyph_name.as_str(), transform.translate_x, transform.translate_y, transform.rotation, @@ -749,7 +775,7 @@ fn insert_component( transform.skew_y, transform.t_center_x, transform.t_center_y, - order_index as i64, + component.order_index as i64, ], )?; Ok(()) diff --git a/crates/shift-store/src/error.rs b/crates/shift-store/src/error.rs index 535aa97c..1ba9ce18 100644 --- a/crates/shift-store/src/error.rs +++ b/crates/shift-store/src/error.rs @@ -26,4 +26,9 @@ pub enum StoreError { #[error("store schema version {found} is newer than supported version {supported}")] UnsupportedSchemaVersion { found: i64, supported: i64 }, + + #[error( + "draft store schema version {found} is from an older build (current version {supported}); delete the draft store and re-import the font" + )] + OutdatedSchemaVersion { found: i64, supported: i64 }, } diff --git a/crates/shift-store/src/schema.rs b/crates/shift-store/src/schema.rs index 735cf780..cae607d4 100644 --- a/crates/shift-store/src/schema.rs +++ b/crates/shift-store/src/schema.rs @@ -1,6 +1,6 @@ use crate::StoreError; -pub(crate) const SCHEMA_V1: &str = r#" +pub(crate) const SCHEMA: &str = r#" CREATE TABLE IF NOT EXISTS font_info ( id INTEGER PRIMARY KEY CHECK (id = 1), family_name TEXT, @@ -292,13 +292,18 @@ CREATE TABLE IF NOT EXISTS workspace_state ( ); "#; -pub(crate) const SCHEMA_VERSION: i64 = 1; +/// Bump this whenever the baseline batch above changes shape (new tables, +/// new columns, changed constraints) so stores written by older builds are +/// refused loudly instead of failing later with "no such table". +pub(crate) const SCHEMA_VERSION: i64 = 2; /// Creates the baseline schema and stamps `user_version`. /// /// Pre-release policy: the app has not shipped, so schema changes edit the /// baseline batch in place instead of adding migration steps. A database -/// from a NEWER app version is refused rather than silently mangled. +/// from a NEWER app version is refused rather than silently mangled, and a +/// database from an OLDER build is refused with instructions to delete the +/// draft store and re-import — never migrated, never patched in place. pub(crate) fn ensure_current(conn: &rusqlite::Connection) -> Result<(), StoreError> { let version: i64 = conn.query_row("PRAGMA user_version", [], |row| row.get(0))?; @@ -309,9 +314,17 @@ pub(crate) fn ensure_current(conn: &rusqlite::Connection) -> Result<(), StoreErr }); } - if version < 1 { - conn.execute_batch(SCHEMA_V1)?; + if version == 0 { + conn.execute_batch(SCHEMA)?; conn.pragma_update(None, "user_version", SCHEMA_VERSION)?; + return Ok(()); + } + + if version < SCHEMA_VERSION { + return Err(StoreError::OutdatedSchemaVersion { + found: version, + supported: SCHEMA_VERSION, + }); } Ok(()) diff --git a/crates/shift-store/tests/fixtures/old_schema_v1.sql b/crates/shift-store/tests/fixtures/old_schema_v1.sql new file mode 100644 index 00000000..c77a4def --- /dev/null +++ b/crates/shift-store/tests/fixtures/old_schema_v1.sql @@ -0,0 +1,267 @@ +CREATE TABLE IF NOT EXISTS font_info ( + id INTEGER PRIMARY KEY CHECK (id = 1), + family_name TEXT, + style_name TEXT, + copyright TEXT, + trademark TEXT, + description TEXT, + note TEXT, + sample_text TEXT, + designer TEXT, + designer_url TEXT, + manufacturer TEXT, + manufacturer_url TEXT, + license_description TEXT, + license_info_url TEXT, + vendor_id TEXT, + version_major INTEGER CHECK (version_major IS NULL OR version_major >= 0), + version_minor INTEGER CHECK (version_minor IS NULL OR version_minor >= 0), + units_per_em REAL NOT NULL CHECK (units_per_em > 0), + ascender REAL NOT NULL, + descender REAL NOT NULL, + cap_height REAL, + x_height REAL, + line_gap REAL, + italic_angle REAL, + underline_position REAL, + underline_thickness REAL, + default_source_id TEXT +); + +CREATE TABLE IF NOT EXISTS axes ( + id TEXT PRIMARY KEY, + tag TEXT NOT NULL, + name TEXT NOT NULL, + min_value REAL NOT NULL, + default_value REAL NOT NULL, + max_value REAL NOT NULL, + hidden INTEGER NOT NULL DEFAULT 0 CHECK (hidden IN (0, 1)), + order_index INTEGER NOT NULL DEFAULT 0 +); + +CREATE UNIQUE INDEX IF NOT EXISTS axes_tag_unique +ON axes(tag); + +CREATE TABLE IF NOT EXISTS sources ( + id TEXT PRIMARY KEY, + name TEXT, + family_name TEXT, + style_name TEXT, + filename TEXT, + kind TEXT NOT NULL, + order_index INTEGER NOT NULL DEFAULT 0 +); + +CREATE TABLE IF NOT EXISTS glyphs ( + id TEXT PRIMARY KEY, + name TEXT, + order_index INTEGER NOT NULL DEFAULT 0 +); + +CREATE INDEX IF NOT EXISTS glyphs_name_idx +ON glyphs(name); + +CREATE TABLE IF NOT EXISTS glyph_unicodes ( + glyph_id TEXT NOT NULL, + unicode INTEGER NOT NULL CHECK (unicode >= 0), + order_index INTEGER NOT NULL, + PRIMARY KEY (glyph_id, unicode), + FOREIGN KEY (glyph_id) REFERENCES glyphs(id) ON DELETE CASCADE +); + +CREATE INDEX IF NOT EXISTS glyph_unicodes_glyph_id_idx +ON glyph_unicodes(glyph_id); + +CREATE TABLE IF NOT EXISTS glyph_layers ( + id TEXT PRIMARY KEY, + glyph_id TEXT NOT NULL, + source_id TEXT NOT NULL, + name TEXT, + width REAL NOT NULL DEFAULT 0, + height REAL, + FOREIGN KEY (glyph_id) REFERENCES glyphs(id) ON DELETE CASCADE, + FOREIGN KEY (source_id) REFERENCES sources(id) ON DELETE CASCADE +); + +CREATE INDEX IF NOT EXISTS glyph_layers_glyph_id_idx +ON glyph_layers(glyph_id); + +CREATE INDEX IF NOT EXISTS glyph_layers_source_id_idx +ON glyph_layers(source_id); + +CREATE TABLE IF NOT EXISTS glyph_layer_contours ( + id TEXT PRIMARY KEY, + layer_id TEXT NOT NULL, + closed INTEGER NOT NULL DEFAULT 0 CHECK (closed IN (0, 1)), + order_index INTEGER NOT NULL, + FOREIGN KEY (layer_id) REFERENCES glyph_layers(id) ON DELETE CASCADE +); + +CREATE UNIQUE INDEX IF NOT EXISTS glyph_layer_contours_layer_order_unique +ON glyph_layer_contours(layer_id, order_index); + +CREATE INDEX IF NOT EXISTS glyph_layer_contours_layer_id_idx +ON glyph_layer_contours(layer_id); + +CREATE TABLE IF NOT EXISTS glyph_layer_points ( + id TEXT PRIMARY KEY, + contour_id TEXT NOT NULL, + order_index INTEGER NOT NULL, + x REAL NOT NULL, + y REAL NOT NULL, + point_type TEXT NOT NULL, + smooth INTEGER NOT NULL DEFAULT 0 CHECK (smooth IN (0, 1)), + FOREIGN KEY (contour_id) REFERENCES glyph_layer_contours(id) ON DELETE CASCADE +); + +CREATE UNIQUE INDEX IF NOT EXISTS glyph_layer_points_contour_order_unique +ON glyph_layer_points(contour_id, order_index); + +CREATE INDEX IF NOT EXISTS glyph_layer_points_contour_id_idx +ON glyph_layer_points(contour_id); + +CREATE TABLE IF NOT EXISTS glyph_components ( + id TEXT PRIMARY KEY, + layer_id TEXT NOT NULL, + base_glyph_id TEXT NOT NULL, + base_glyph_name TEXT NOT NULL, + translate_x REAL NOT NULL DEFAULT 0, + translate_y REAL NOT NULL DEFAULT 0, + rotation REAL NOT NULL DEFAULT 0, + scale_x REAL NOT NULL DEFAULT 1, + scale_y REAL NOT NULL DEFAULT 1, + skew_x REAL NOT NULL DEFAULT 0, + skew_y REAL NOT NULL DEFAULT 0, + t_center_x REAL NOT NULL DEFAULT 0, + t_center_y REAL NOT NULL DEFAULT 0, + order_index INTEGER NOT NULL, + FOREIGN KEY (layer_id) REFERENCES glyph_layers(id) ON DELETE CASCADE +); + +CREATE UNIQUE INDEX IF NOT EXISTS glyph_components_layer_order_unique +ON glyph_components(layer_id, order_index); + +CREATE INDEX IF NOT EXISTS glyph_components_layer_id_idx +ON glyph_components(layer_id); + +CREATE INDEX IF NOT EXISTS glyph_components_base_glyph_id_idx +ON glyph_components(base_glyph_id); + +CREATE TABLE IF NOT EXISTS glyph_layer_anchors ( + id TEXT PRIMARY KEY, + layer_id TEXT NOT NULL, + name TEXT, + x REAL NOT NULL, + y REAL NOT NULL, + order_index INTEGER NOT NULL, + FOREIGN KEY (layer_id) REFERENCES glyph_layers(id) ON DELETE CASCADE +); + +CREATE UNIQUE INDEX IF NOT EXISTS glyph_layer_anchors_layer_order_unique +ON glyph_layer_anchors(layer_id, order_index); + +CREATE INDEX IF NOT EXISTS glyph_layer_anchors_layer_id_idx +ON glyph_layer_anchors(layer_id); + +CREATE TABLE IF NOT EXISTS font_guidelines ( + id TEXT PRIMARY KEY, + x REAL, + y REAL, + angle REAL, + name TEXT, + color TEXT, + order_index INTEGER NOT NULL +); + +CREATE TABLE IF NOT EXISTS glyph_layer_guidelines ( + id TEXT PRIMARY KEY, + layer_id TEXT NOT NULL, + x REAL, + y REAL, + angle REAL, + name TEXT, + color TEXT, + order_index INTEGER NOT NULL, + FOREIGN KEY (layer_id) REFERENCES glyph_layers(id) ON DELETE CASCADE +); + +CREATE INDEX IF NOT EXISTS glyph_layer_guidelines_layer_id_idx +ON glyph_layer_guidelines(layer_id); + +CREATE TABLE IF NOT EXISTS source_locations ( + source_id TEXT NOT NULL, + axis_id TEXT NOT NULL, + value REAL NOT NULL, + PRIMARY KEY (source_id, axis_id), + FOREIGN KEY (source_id) REFERENCES sources(id) ON DELETE CASCADE, + FOREIGN KEY (axis_id) REFERENCES axes(id) ON DELETE CASCADE +); + +CREATE TABLE IF NOT EXISTS feature_text ( + id INTEGER PRIMARY KEY CHECK (id = 1), + fea_source TEXT +); + +CREATE TABLE IF NOT EXISTS kerning_groups ( + side INTEGER NOT NULL CHECK (side IN (1, 2)), + name TEXT NOT NULL, + PRIMARY KEY (side, name) +); + +CREATE TABLE IF NOT EXISTS kerning_group_members ( + side INTEGER NOT NULL CHECK (side IN (1, 2)), + group_name TEXT NOT NULL, + glyph_name TEXT NOT NULL, + order_index INTEGER NOT NULL, + PRIMARY KEY (side, group_name, order_index), + FOREIGN KEY (side, group_name) REFERENCES kerning_groups(side, name) ON DELETE CASCADE +); + +CREATE TABLE IF NOT EXISTS kerning_pairs ( + order_index INTEGER PRIMARY KEY, + first_kind TEXT NOT NULL CHECK (first_kind IN ('glyph', 'group')), + first_value TEXT NOT NULL, + second_kind TEXT NOT NULL CHECK (second_kind IN ('glyph', 'group')), + second_value TEXT NOT NULL, + value REAL NOT NULL +); + +CREATE TABLE IF NOT EXISTS font_lib ( + key TEXT PRIMARY KEY, + value_json TEXT NOT NULL +); + +CREATE TABLE IF NOT EXISTS glyph_lib ( + glyph_id TEXT NOT NULL, + key TEXT NOT NULL, + value_json TEXT NOT NULL, + PRIMARY KEY (glyph_id, key), + FOREIGN KEY (glyph_id) REFERENCES glyphs(id) ON DELETE CASCADE +); + +CREATE TABLE IF NOT EXISTS glyph_layer_lib ( + layer_id TEXT NOT NULL, + key TEXT NOT NULL, + value_json TEXT NOT NULL, + PRIMARY KEY (layer_id, key), + FOREIGN KEY (layer_id) REFERENCES glyph_layers(id) ON DELETE CASCADE +); + +CREATE TABLE IF NOT EXISTS workspace_state ( + id INTEGER PRIMARY KEY CHECK (id = 1), + document_id TEXT, + source_kind TEXT NOT NULL CHECK (source_kind IN ('untitled', 'package', 'imported')), + source_path TEXT, + canonical_source_path TEXT, + original_import_path TEXT, + source_package_id TEXT, + source_file_identity_kind TEXT, + source_file_identity_value TEXT, + source_size INTEGER, + source_mtime_ms INTEGER, + source_fingerprint TEXT, + dirty INTEGER NOT NULL DEFAULT 0 CHECK (dirty IN (0, 1)), + revision INTEGER NOT NULL DEFAULT 0, + saved_revision INTEGER NOT NULL DEFAULT 0, + updated_at_ms INTEGER NOT NULL +); diff --git a/crates/shift-store/tests/store_test.rs b/crates/shift-store/tests/store_test.rs index 8766655d..33afff3c 100644 --- a/crates/shift-store/tests/store_test.rs +++ b/crates/shift-store/tests/store_test.rs @@ -2,9 +2,11 @@ use shift_font::test_support::sample_font; use shift_store::{ AxisId, ComponentId, Evidence, FileIdentity, FontInfo, GlyphId, LayerId, NewAxis, NewGlyph, NewGlyphComponent, NewGlyphLayer, NewSource, ShiftStore, SourceId, SourceIdentitySnapshot, - SourceKind, WorkspaceState, + SourceKind, StoreError, WorkspaceState, }; +const OLD_SCHEMA_V1: &str = include_str!("fixtures/old_schema_v1.sql"); + #[test] fn opens_memory_store() { ShiftStore::open_memory_for_test().expect("memory store should open"); @@ -916,7 +918,7 @@ fn file_stores_run_wal_with_verified_pragmas() { let version: i64 = conn .query_row("PRAGMA user_version", [], |row| row.get(0)) .expect("user_version"); - assert_eq!(version, 1); + assert_eq!(version, 2); std::fs::remove_dir_all(path.parent().unwrap()).ok(); } @@ -1070,6 +1072,133 @@ fn refuses_stores_from_newer_schema_versions() { std::fs::remove_dir_all(path.parent().unwrap()).ok(); } +#[test] +fn refuses_stores_from_older_schema_versions() { + let path = temp_store_path("outdated"); + + { + let conn = rusqlite::Connection::open(&path).expect("raw create"); + conn.execute_batch(OLD_SCHEMA_V1).expect("old schema"); + conn.pragma_update(None, "user_version", 1) + .expect("stamp v1"); + } + + let error = match ShiftStore::open(&path) { + Ok(_) => panic!("an older draft store must be refused"), + Err(error) => error, + }; + assert!(matches!( + error, + StoreError::OutdatedSchemaVersion { + found: 1, + supported: 2 + } + )); + assert!( + error.to_string().contains("delete the draft store"), + "refusal must tell the user what to do, got: {error}" + ); + + std::fs::remove_dir_all(path.parent().unwrap()).ok(); +} + +#[test] +fn source_created_change_persists_full_source_state() { + let mut store = ShiftStore::open_memory_for_test().expect("memory store should open"); + + let axis_id = shift_font::AxisId::from_raw("axis_weight"); + let axis = shift_font::Axis::with_id( + axis_id.clone(), + "wght".to_string(), + "Weight".to_string(), + 100.0, + 400.0, + 900.0, + ); + let mut location = shift_font::Location::new(); + location.set(axis_id, 700.0); + let mut source = + shift_font::Source::with_filename("Bold".to_string(), location, "Bold.ufo".to_string()); + source.set_color(Some("1,0,0,1".to_string())); + source.set_layer_name(Some("bold".to_string())); + source.lib_mut().set( + "com.shift.sourceNote".to_string(), + shift_font::LibValue::String("bold note".to_string()), + ); + + store + .apply_change_set(&shift_font::FontChangeSet::new(vec![ + shift_font::FontChange::axis_created(&axis), + shift_font::FontChange::source_created(&source), + ])) + .expect("apply change set"); + + let loaded = store.load_font_state().expect("load font state"); + assert_eq!(loaded.sources(), std::slice::from_ref(&source)); +} + +#[test] +fn source_created_change_skips_locations_on_missing_axes() { + let mut store = ShiftStore::open_memory_for_test().expect("memory store should open"); + + let mut location = shift_font::Location::new(); + location.set(shift_font::AxisId::from_raw("ghost"), 500.0); + let source = shift_font::Source::new("Bold".to_string(), location); + + store + .apply_change_set(&shift_font::FontChangeSet::new(vec![ + shift_font::FontChange::source_created(&source), + ])) + .expect("locations on axes without rows must not violate foreign keys"); + + let loaded = store.load_font_state().expect("load font state"); + assert_eq!(loaded.sources().len(), 1); + assert!(loaded.sources()[0].location().is_empty()); +} + +#[test] +fn font_level_creates_append_order_and_deletes_compact_it() { + let mut store = ShiftStore::open_memory_for_test().expect("memory store should open"); + + let sources: Vec = ["Light", "Regular", "Bold"] + .into_iter() + .map(|name| shift_font::Source::new(name.to_string(), shift_font::Location::new())) + .collect(); + let glyphs: Vec = ["A", "B", "C"] + .into_iter() + .map(shift_font::Glyph::new) + .collect(); + let mut changes: Vec = sources + .iter() + .map(shift_font::FontChange::source_created) + .collect(); + changes.extend(glyphs.iter().map(shift_font::FontChange::glyph_created)); + store + .apply_change_set(&shift_font::FontChangeSet::new(changes)) + .expect("apply creates"); + + let heavy = shift_font::Source::new("Heavy".to_string(), shift_font::Location::new()); + let glyph_d = shift_font::Glyph::new("D"); + store + .apply_change_set(&shift_font::FontChangeSet::new(vec![ + shift_font::FontChange::source_deleted(sources[1].id()), + shift_font::FontChange::source_created(&heavy), + shift_font::FontChange::glyph_deleted(glyphs[1].id()), + shift_font::FontChange::glyph_created(&glyph_d), + ])) + .expect("apply deletes and appends"); + + let loaded = store.load_font_state().expect("load font state"); + let source_names: Vec<&str> = loaded + .sources() + .iter() + .map(|source| source.name()) + .collect(); + assert_eq!(source_names, ["Light", "Bold", "Heavy"]); + let glyph_names: Vec<&str> = loaded.glyphs().map(|glyph| glyph.name()).collect(); + assert_eq!(glyph_names, ["A", "C", "D"]); +} + #[test] fn replace_and_load_font_state_preserves_source_roles_and_layer_names() { let mut store = ShiftStore::open_memory_for_test().expect("memory store should open"); diff --git a/crates/shift-workspace/Cargo.toml b/crates/shift-workspace/Cargo.toml index 06425016..58555ee9 100644 --- a/crates/shift-workspace/Cargo.toml +++ b/crates/shift-workspace/Cargo.toml @@ -14,4 +14,5 @@ thiserror = "2" windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem"] } [dev-dependencies] +shift-font = { workspace = true, features = ["test-support"] } tempfile = "3" diff --git a/crates/shift-workspace/src/workspace.rs b/crates/shift-workspace/src/workspace.rs index b40a0862..ea0cb593 100644 --- a/crates/shift-workspace/src/workspace.rs +++ b/crates/shift-workspace/src/workspace.rs @@ -735,10 +735,9 @@ fn replay_layer_pairs( .ok_or(CoreError::LayerNotFound(layer_id))?; *layer = replacement; - // Geometry replace persists contours only; metrics ride their - // own change so width/height restores reach SQLite too. + // The geometry-replace change carries the layer's full persisted + // content (metrics included), so one change restores everything. changes.push(FontChange::layer_geometry_replaced(layer)); - changes.push(FontChange::layer_metrics_changed(layer)); touched.push(TouchedLayer { layer: layer.clone(), structural: true, @@ -772,6 +771,10 @@ fn replay_glyph( Some(glyph.glyph_name().clone()), &layer, )); + // The created row is bare identity + metrics; the layer's + // contents ride a geometry-replace so the store restores the + // whole layer, not an empty shell. + changes.push(FontChange::layer_geometry_replaced(&layer)); touched.push(TouchedLayer { layer, structural: true, @@ -882,6 +885,10 @@ fn replay_glyph_layer( glyph_name, &layer, )); + // The created row is bare identity + metrics; the layer's + // contents ride a geometry-replace so the store restores the + // whole layer, not an empty shell. + changes.push(FontChange::layer_geometry_replaced(&layer)); font.insert_glyph_layer(glyph_id, layer.clone())?; touched.push(TouchedLayer { layer, diff --git a/crates/shift-workspace/tests/workspace_test.rs b/crates/shift-workspace/tests/workspace_test.rs index ec4bdc33..b13fee00 100644 --- a/crates/shift-workspace/tests/workspace_test.rs +++ b/crates/shift-workspace/tests/workspace_test.rs @@ -1,9 +1,9 @@ use std::{fs, path::PathBuf}; use shift_font::{ - AnchorId, AnchorSeed, Axis, AxisId, BooleanOp, ContourId, FontChange, FontIntent, - FontIntentSet, GlyphId, GlyphName, LayerId, Location, PointId, PointSeed, PointType, SourceId, - error::CoreError, + AnchorId, AnchorSeed, AppliedIntents, Axis, AxisId, BooleanOp, ContourId, FontChange, + FontIntent, FontIntentSet, GlyphId, GlyphName, LayerId, LibValue, Location, PointId, PointSeed, + PointType, SourceId, error::CoreError, test_support::sample_font, }; use shift_source::ShiftSourcePackage; use shift_workspace::{ @@ -11,6 +11,36 @@ use shift_workspace::{ WorkspaceRecoveryCandidate, WorkspaceSource, }; +/// Reloads the font from the draft store and asserts it equals the +/// in-memory workspace font. +/// +/// Apply and replay persist through the same change-set seam, so any field +/// a replay drops, wipes, or misorders shows up here — `Font` equality +/// covers every persisted field (index caches excluded). Route every +/// undo/redo in intent tests through [`undo_and_verify`] / +/// [`redo_and_verify`] so new intents inherit the store == memory +/// invariant. +fn assert_store_matches_font(workspace: &FontWorkspace) { + let store_font = workspace.store().load_font_state().unwrap(); + assert_eq!( + &store_font, + workspace.font(), + "draft store diverged from the in-memory font" + ); +} + +fn undo_and_verify(workspace: &mut FontWorkspace, expectation: &str) -> AppliedIntents { + let outcome = workspace.undo().unwrap().expect(expectation); + assert_store_matches_font(workspace); + outcome +} + +fn redo_and_verify(workspace: &mut FontWorkspace, expectation: &str) -> AppliedIntents { + let outcome = workspace.redo().unwrap().expect(expectation); + assert_store_matches_font(workspace); + outcome +} + fn create_glyph_intent(name: &str, unicodes: Vec) -> FontIntent { FontIntent::CreateGlyph { glyph_id: None, @@ -474,13 +504,13 @@ fn create_glyph_undo_redo_removes_and_restores_glyph_identity() { .is_empty() ); - let undone = workspace.undo().unwrap().expect("createGlyph should undo"); + let undone = undo_and_verify(&mut workspace, "createGlyph should undo"); assert_eq!(workspace.font().glyph_count(), 0); assert!(undone.changes.changes.iter().any( |change| matches!(change, FontChange::GlyphDeleted(change) if change.glyph_id == glyph_id) )); - let redone = workspace.redo().unwrap().expect("createGlyph should redo"); + let redone = redo_and_verify(&mut workspace, "createGlyph should redo"); assert_eq!(workspace.font().glyph_count(), 1); assert!(redone.layers.is_empty()); assert!(workspace.font().glyph(glyph_id).is_some()); @@ -516,10 +546,7 @@ fn create_glyph_layer_undo_redo_removes_and_restores_sparse_layer() { Some(layer_id.clone()) ); - let undone = workspace - .undo() - .unwrap() - .expect("createGlyphLayer should undo"); + let undone = undo_and_verify(&mut workspace, "createGlyphLayer should undo"); assert!(undone.changes.changes.iter().any( |change| matches!(change, FontChange::GlyphLayerDeleted(change) if change.layer_id == layer_id) )); @@ -530,10 +557,7 @@ fn create_glyph_layer_undo_redo_removes_and_restores_sparse_layer() { None ); - let redone = workspace - .redo() - .unwrap() - .expect("createGlyphLayer should redo"); + let redone = redo_and_verify(&mut workspace, "createGlyphLayer should redo"); assert_eq!(redone.layers.len(), 1); assert_eq!( workspace @@ -610,7 +634,7 @@ fn delete_source_undo_redo_removes_and_restores_existing_sparse_layers() { ); assert!(workspace.font().layer(layer_id.clone()).is_none()); - let undone = workspace.undo().unwrap().expect("deleteSource should undo"); + let undone = undo_and_verify(&mut workspace, "deleteSource should undo"); assert!(undone .changes .changes @@ -634,7 +658,7 @@ fn delete_source_undo_redo_removes_and_restores_existing_sparse_layers() { None ); - let redone = workspace.redo().unwrap().expect("deleteSource should redo"); + let redone = redo_and_verify(&mut workspace, "deleteSource should redo"); assert!(redone .changes .changes @@ -670,11 +694,11 @@ fn batched_create_glyphs_undo_as_one_step() { .unwrap(); assert_eq!(workspace.font().glyph_count(), 3); - workspace.undo().unwrap().expect("batch should undo"); + undo_and_verify(&mut workspace, "batch should undo"); assert_eq!(workspace.font().glyph_count(), 0); assert!(workspace.undo().unwrap().is_none()); - workspace.redo().unwrap().expect("batch should redo"); + redo_and_verify(&mut workspace, "batch should redo"); assert_eq!(workspace.font().glyph_count(), 3); assert!(workspace.font().glyph_id_by_name("B").is_some()); } @@ -724,14 +748,14 @@ fn mixed_font_level_batch_undoes_axis_source_and_glyph_together() { let glyph_b = workspace.font().glyph_by_name("B").unwrap(); assert_eq!(glyph_b.layers().len(), 0); - workspace.undo().unwrap().expect("batch should undo"); + undo_and_verify(&mut workspace, "batch should undo"); assert_eq!(workspace.font().axes().len(), 0); assert_eq!(workspace.font().sources().len(), base_sources); assert_eq!(workspace.font().glyph_count(), 1); let glyph_a = workspace.font().glyph_by_name("A").unwrap(); assert_eq!(glyph_a.layers().len(), 0); - workspace.redo().unwrap().expect("batch should redo"); + redo_and_verify(&mut workspace, "batch should redo"); assert_eq!(workspace.font().axes().len(), 1); assert_eq!(workspace.font().sources().len(), base_sources + 1); assert_eq!(workspace.font().glyph_count(), 2); @@ -850,10 +874,10 @@ fn assert_layer_undo_redo( let post = workspace.font().layer(layer_id.clone()).unwrap().clone(); assert_ne!(pre, post, "intent should change the layer"); - workspace.undo().unwrap().expect("intent should undo"); + undo_and_verify(workspace, "intent should undo"); assert_eq!(workspace.font().layer(layer_id.clone()).unwrap(), &pre); - workspace.redo().unwrap().expect("intent should redo"); + redo_and_verify(workspace, "intent should redo"); assert_eq!(workspace.font().layer(layer_id.clone()).unwrap(), &post); } @@ -1108,7 +1132,7 @@ fn update_glyph_undo_redo_restores_old_identity() { assert_eq!(glyph.glyph_name().to_string(), "A.alt"); assert_eq!(glyph.unicodes(), &[97]); - let undone = workspace.undo().unwrap().expect("updateGlyph should undo"); + let undone = undo_and_verify(&mut workspace, "updateGlyph should undo"); assert!(undone.changes.changes.iter().any(|change| matches!( change, FontChange::GlyphIdentityChanged(change) if change.glyph_id == glyph_id @@ -1121,7 +1145,7 @@ fn update_glyph_undo_redo_restores_old_identity() { Some(glyph_id.clone()) ); - let redone = workspace.redo().unwrap().expect("updateGlyph should redo"); + let redone = redo_and_verify(&mut workspace, "updateGlyph should redo"); assert!(redone.changes.changes.iter().any(|change| matches!( change, FontChange::GlyphIdentityChanged(change) if change.glyph_id == glyph_id @@ -1162,10 +1186,10 @@ fn create_axis_undo_redo_removes_and_restores_axis() { .unwrap(); let created = workspace.font().axes()[0].clone(); - workspace.undo().unwrap().expect("createAxis should undo"); + undo_and_verify(&mut workspace, "createAxis should undo"); assert!(workspace.font().axes().is_empty()); - workspace.redo().unwrap().expect("createAxis should redo"); + redo_and_verify(&mut workspace, "createAxis should redo"); assert_eq!(workspace.font().axes(), &[created]); } @@ -1210,7 +1234,7 @@ fn delete_axis_undo_redo_restores_full_axis_definition() { .unwrap(); assert!(workspace.font().axes().is_empty()); - let undone = workspace.undo().unwrap().expect("deleteAxis should undo"); + let undone = undo_and_verify(&mut workspace, "deleteAxis should undo"); assert!(undone.changes.changes.iter().any(|change| matches!( change, FontChange::AxisCreated(change) if change.axis_id == axis_id @@ -1235,17 +1259,14 @@ fn delete_axis_undo_redo_restores_full_axis_definition() { assert_eq!(locations.len(), 1); assert_eq!(locations[0].value, 700.0); - let redone = workspace.redo().unwrap().expect("deleteAxis should redo"); + let redone = redo_and_verify(&mut workspace, "deleteAxis should redo"); assert!(redone.changes.changes.iter().any(|change| matches!( change, FontChange::AxisDeleted(change) if change.axis_id == axis_id ))); assert!(workspace.font().axes().is_empty()); - workspace - .undo() - .unwrap() - .expect("deleteAxis should undo again"); + undo_and_verify(&mut workspace, "deleteAxis should undo again"); assert_eq!(workspace.font().axes(), &[axis]); } @@ -1277,7 +1298,7 @@ fn create_source_undo_redo_removes_and_restores_source() { .cloned() .unwrap(); - workspace.undo().unwrap().expect("createSource should undo"); + undo_and_verify(&mut workspace, "createSource should undo"); assert_eq!(workspace.font().sources().len(), base_sources); assert!( workspace @@ -1287,7 +1308,7 @@ fn create_source_undo_redo_removes_and_restores_source() { .all(|source| source.id() != source_id) ); - workspace.redo().unwrap().expect("createSource should redo"); + redo_and_verify(&mut workspace, "createSource should redo"); assert_eq!( workspace .font() @@ -1323,10 +1344,10 @@ fn failed_undo_replay_hands_the_entry_back_for_retry() { let font = workspace.font().clone(); workspace.store_mut().replace_font_state(&font).unwrap(); - workspace - .undo() - .unwrap() - .expect("failed undo should hand the entry back for retry"); + undo_and_verify( + &mut workspace, + "failed undo should hand the entry back for retry", + ); assert_eq!(workspace.font().glyph_count(), 0); } @@ -1359,7 +1380,7 @@ fn failed_redo_replay_hands_the_entry_back_for_retry() { None, ) .unwrap(); - workspace.undo().unwrap().expect("deleteSource should undo"); + undo_and_verify(&mut workspace, "deleteSource should undo"); // Wipe the store's font rows so the redo's SourceDeleted change has no // row to delete and the replay fails at the persistence step. @@ -1385,10 +1406,10 @@ fn failed_redo_replay_hands_the_entry_back_for_retry() { let font = workspace.font().clone(); workspace.store_mut().replace_font_state(&font).unwrap(); - workspace - .redo() - .unwrap() - .expect("failed redo should hand the entry back for retry"); + redo_and_verify( + &mut workspace, + "failed redo should hand the entry back for retry", + ); assert!( workspace .font() @@ -1411,6 +1432,7 @@ fn ledger_trims_oldest_entries_beyond_max() { let mut undone = 0; while workspace.undo().unwrap().is_some() { + assert_store_matches_font(&workspace); undone += 1; } @@ -1419,6 +1441,165 @@ fn ledger_trims_oldest_entries_beyond_max() { assert!(workspace.font().glyph_id_by_name("g0").is_some()); } +/// Opens a workspace over the kitchen-sink corpus, whose sources carry +/// filenames, colors, libs, and axis locations — the fields font-level +/// replays must not drop. +fn sample_package_workspace(temp: &tempfile::TempDir) -> FontWorkspace { + let package_path = temp.path().join("Sample.shift"); + ShiftSourcePackage::save_font(&package_path, &sample_font()).unwrap(); + FontWorkspace::open(&package_path, temp.path().join("working.sqlite")).unwrap() +} + +fn source_by_name<'a>(workspace: &'a FontWorkspace, name: &str) -> &'a shift_font::Source { + workspace + .font() + .sources() + .iter() + .find(|source| source.name() == name) + .unwrap_or_else(|| panic!("source {name} should exist")) +} + +#[test] +fn delete_axis_then_source_batch_undo_redo_round_trips() { + let temp = tempfile::tempdir().unwrap(); + let mut workspace = sample_package_workspace(&temp); + + // The undo replays this batch reversed: the source is re-created + // before the axis its location references exists again. The store + // must tolerate that ordering instead of wedging undo on a + // foreign-key violation. + workspace + .apply( + FontIntentSet { + intents: vec![ + FontIntent::DeleteAxis { + axis_id: AxisId::from_raw("weight"), + }, + FontIntent::DeleteSource { + source_id: SourceId::from_raw("bold"), + }, + ], + }, + None, + ) + .unwrap(); + + undo_and_verify(&mut workspace, "axis-then-source batch should undo"); + let bold = source_by_name(&workspace, "Bold"); + assert_eq!( + bold.location().get(&AxisId::from_raw("weight")), + Some(900.0) + ); + assert_eq!(bold.location().get(&AxisId::from_raw("width")), Some(112.5)); + + redo_and_verify(&mut workspace, "batch should redo"); + assert!( + workspace + .font() + .axes() + .iter() + .all(|axis| axis.tag() != "wght") + ); + assert!( + workspace + .font() + .sources() + .iter() + .all(|source| source.name() != "Bold") + ); + + undo_and_verify(&mut workspace, "batch should undo again after redo"); +} + +#[test] +fn delete_source_then_axis_batch_undo_restores_locations() { + let temp = tempfile::tempdir().unwrap(); + let mut workspace = sample_package_workspace(&temp); + + workspace + .apply( + FontIntentSet { + intents: vec![ + FontIntent::DeleteSource { + source_id: SourceId::from_raw("bold"), + }, + FontIntent::DeleteAxis { + axis_id: AxisId::from_raw("weight"), + }, + ], + }, + None, + ) + .unwrap(); + + undo_and_verify(&mut workspace, "source-then-axis batch should undo"); + let bold = source_by_name(&workspace, "Bold"); + assert_eq!( + bold.location().get(&AxisId::from_raw("weight")), + Some(900.0) + ); + let regular = source_by_name(&workspace, "Regular"); + assert_eq!( + regular.location().get(&AxisId::from_raw("weight")), + Some(400.0) + ); + + redo_and_verify(&mut workspace, "batch should redo"); + undo_and_verify(&mut workspace, "batch should undo again after redo"); +} + +#[test] +fn delete_source_undo_restores_full_source() { + let temp = tempfile::tempdir().unwrap(); + let mut workspace = sample_package_workspace(&temp); + + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::DeleteSource { + source_id: SourceId::from_raw("bold"), + }], + }, + None, + ) + .unwrap(); + + undo_and_verify(&mut workspace, "deleteSource should undo"); + let bold = source_by_name(&workspace, "Bold"); + assert_eq!(bold.color(), Some("1,0.75,0,0.7")); + assert_eq!(bold.filename(), Some("Bold.ufo")); + assert_eq!( + bold.lib().get("com.shift.sourceNote"), + Some(&LibValue::String("bold layer note".to_string())) + ); +} + +#[test] +fn delete_axis_undo_keeps_source_fields_intact() { + let temp = tempfile::tempdir().unwrap(); + let mut workspace = sample_package_workspace(&temp); + + // Undoing an axis delete re-emits every located source to restore the + // stripped location values; that re-emission must carry the sources' + // full state, not blank their colors, filenames, and libs. + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::DeleteAxis { + axis_id: AxisId::from_raw("width"), + }], + }, + None, + ) + .unwrap(); + + undo_and_verify(&mut workspace, "deleteAxis should undo"); + let bold = source_by_name(&workspace, "Bold"); + assert_eq!(bold.color(), Some("1,0.75,0,0.7")); + assert_eq!(bold.filename(), Some("Bold.ufo")); + assert_eq!(bold.location().get(&AxisId::from_raw("width")), Some(112.5)); +} + fn fixture(path: &str) -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")) .join("../..")