Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .beads/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ daemon.lock
daemon.log
daemon.pid
bd.sock
.jsonl.lock
.migration-hint-ts

# Local version tracking (prevents upgrade notification spam after git ops)
.local_version
Expand Down
47 changes: 44 additions & 3 deletions openforge/db/fixtures/incremental.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ def __init__(self, conn: connection, verbose: bool = False):
self.current_fixture_files = (
set()
) # Will be populated when processing fixture data
# Blueprint IDs that were renamed in this load — used to skip the
# subsequent deprecation step so a renamed row isn't tombstoned and
# stripped of tags/images. Per-apply state: populated in
# `_handle_addition`'s rename branch and reset at the top of
# `_apply_changes_with_cursor` so it doesn't leak between fixtures
# when one loader instance processes a directory.
self._renamed_blueprint_ids = set()

def _load_existing_blueprints(self, curs: cursor = None) -> Dict[str, Dict]:
"""Load existing blueprints from database for comparison.
Expand Down Expand Up @@ -752,6 +759,9 @@ def _apply_changes_with_cursor(self, curs: cursor, changes: ComparisonResult):
# Load existing blueprints for modification handling
existing_blueprints = self._load_existing_blueprints(curs)

# Reset rename tracking for this load.
self._renamed_blueprint_ids = set()

# Track new blueprint IDs for version change linking
new_blueprint_ids = {}
new_blueprint_ids_by_md5 = {}
Expand Down Expand Up @@ -798,6 +808,17 @@ def _handle_deprecation(
deprecated_bp: Blueprint to deprecate
successor_id: Optional ID of the successor blueprint (for version changes)
"""
# If this blueprint was renamed in this same load, the row was
# already updated in place. Deprecating it now would tombstone the
# live row and strip its freshly-inserted tags/images.
if deprecated_bp.get("id") in self._renamed_blueprint_ids:
if self.verbose:
write_output(
f"Skipping deprecation of blueprint {deprecated_bp['id']} "
f"(renamed in this load)\n"
)
return

# Check if a deprecated blueprint with this MD5 already exists
if deprecated_bp.get("file_md5"):
existing_deprecated = self._find_deprecated_blueprint_by_md5(
Expand Down Expand Up @@ -897,12 +918,26 @@ def _handle_addition(self, curs: cursor, new_item: Dict):
"adding to consolidated_paths\n"
)

# Update the blueprint's full_name to the new path
# Update the blueprint's full_name to the new path.
# Also resync blueprint_name + search_text and clear
# deprecated, since the underlying file is the same
# bytes under a new name.
new_file_name = os.path.basename(new_full_name)
words = self._get_words(new_item)
search_text = blueprint_sql.blueprint_search_text(
{"blueprint_name": new_file_name}, words
)
update_data = {
"full_name": new_full_name,
"file_name": os.path.basename(new_full_name),
"file_name": new_file_name,
"blueprint_name": new_file_name,
"search_text": search_text,
"deprecated": False,
}
blueprint_sql.update_blueprint(curs, bp["id"], update_data)
# Mark this id as renamed so a later deprecation step
# in the same load won't tombstone it.
self._renamed_blueprint_ids.add(bp["id"])

# Also update tags and images from the new fixture data
# First remove old tags and images
Expand Down Expand Up @@ -1014,8 +1049,14 @@ def _handle_modification(

blueprint_id = existing_bp["id"]

# Update blueprint data
# Update blueprint data. Recompute search_text alongside the column
# update because tag changes are a common modify trigger and tag
# words feed into search_text — without this, search_text drifts
# whenever tags are edited.
bp_data = self._munge_blueprint(modified_item)
bp_data["search_text"] = blueprint_sql.blueprint_search_text(
bp_data, self._get_words(modified_item)
)
blueprint_sql.update_blueprint(curs, blueprint_id, bp_data)

# Update tags (delete old, insert new)
Expand Down
5 changes: 3 additions & 2 deletions openforge/db/sql/blueprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def get_deprecated_blueprints(curs: cursor) -> list[dict]:
return results


def _blueprint_search_text(data: dict, words: list[str]) -> str:
def blueprint_search_text(data: dict, words: list[str]) -> str:
retwords = set()
retwords.update(words)
retwords.update(re.split(r"[^a-zA-Z0-9]", data["blueprint_name"]))
Expand All @@ -104,7 +104,7 @@ def _blueprint_search_text(data: dict, words: list[str]) -> str:
def insert_blueprint(
curs: cursor, data: dict, rescue_md5_conflict: bool = False, words: list[str] = []
) -> dict:
data["search_text"] = _blueprint_search_text(data, words)
data["search_text"] = blueprint_search_text(data, words)
query_list = [
sql.SQL(
"""
Expand Down Expand Up @@ -191,6 +191,7 @@ def update_blueprint(curs: cursor, blueprint_id: uuid.UUID, data: dict) -> dict:
"full_name",
"file_modified_at",
"storage_address",
"search_text",
# Phase 1 fields
"consolidated_paths",
"deprecated",
Expand Down
136 changes: 135 additions & 1 deletion tests/test_incremental_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Tests for incremental fixtures loading functionality.
"""

from unittest.mock import Mock
from unittest.mock import Mock, patch

import pytest

Expand Down Expand Up @@ -499,3 +499,137 @@ def test_comparison_result_summary(self):
assert "1 modified" in summary
assert "1 deprecated" in summary
assert result.has_changes()

# ------------------------------------------------------------------
# Rename-within-same-fixture regression tests
# ------------------------------------------------------------------
# These tests pin down the bug where renaming a file in Dropbox
# (same MD5, new full_name) corrupted the DB row: blueprint_name and
# search_text went stale, the row got tombstoned in the same load,
# and tags/images were deleted.

def _rename_loader(self):
"""Build a loader primed for the same-fixture rename branch."""
mock_conn = create_mock_connection()
loader = IncrementalFixturesLoader(mock_conn, verbose=False)
# Trigger the is_rename=True branch in _handle_addition: both
# paths must share the subset path, and the existing path must
# not be in the current fixture files set.
loader.fixture_subset_path = "tiles/aztlan"
loader.current_fixture_files = {
"tiles/aztlan/floors/floor/openforge/aztlan#floor.1x1.openforge.stl"
}
return loader

def test_rename_resyncs_blueprint_name_and_clears_deprecated(self):
"""Rename branch must resync blueprint_name + search_text and
clear deprecated, not just full_name/file_name."""
loader = self._rename_loader()

# Existing DB row uses the old (typo'd) name; insert returns it
# as the rescued md5 conflict.
existing_bp = {
"id": "bp-1",
"full_name": (
"tiles/aztlan/floors/floor/openforge/atzlan#floor.1x1.openforge.stl"
),
}

new_item = create_mock_fixture_item(
"tiles/aztlan/floors/floor/openforge/aztlan#floor.1x1.openforge.stl",
"md5-shared",
tags=[["texture", "aztlan"], ["shape", "floor"]],
)
# _munge_blueprint reads file_metadata["file"]; create_mock_fixture_item
# omits it, so add the basename here.
new_item["file_metadata"]["file"] = "aztlan#floor.1x1.openforge.stl"

captured = {}

def fake_update(curs, blueprint_id, data):
captured["id"] = blueprint_id
captured["data"] = data

with (
patch(
"openforge.db.fixtures.incremental.blueprint_sql.insert_blueprint",
return_value=existing_bp,
),
patch(
"openforge.db.fixtures.incremental.blueprint_sql.update_blueprint",
side_effect=fake_update,
),
patch(
"openforge.db.fixtures.incremental.tag_sql.delete_all_blueprint_tags"
),
patch("openforge.db.fixtures.incremental.tag_sql.insert_tag"),
patch(
"openforge.db.fixtures.incremental.image_sql"
".delete_images_for_blueprint"
),
patch(
"openforge.db.fixtures.incremental.image_sql.insert_image_for_blueprint"
),
):
loader._handle_addition(Mock(), new_item)

# The rename id is tracked so the deprecation step skips it.
assert "bp-1" in loader._renamed_blueprint_ids
# All four corruption-prone fields must be in the update payload.
assert captured["data"]["full_name"] == (
"tiles/aztlan/floors/floor/openforge/aztlan#floor.1x1.openforge.stl"
)
assert captured["data"]["file_name"] == "aztlan#floor.1x1.openforge.stl"
assert captured["data"]["blueprint_name"] == ("aztlan#floor.1x1.openforge.stl")
assert captured["data"]["deprecated"] is False
# search_text reflects the new name and tag words, not the old.
search_text = captured["data"]["search_text"]
assert "aztlan" in search_text.split()
assert "atzlan" not in search_text.split()
assert "floor" in search_text.split()

def test_renamed_id_skips_deprecation(self):
"""A bp id added to _renamed_blueprint_ids during a load must
not be re-deprecated by _handle_deprecation in the same load."""
loader = self._rename_loader()
loader._renamed_blueprint_ids.add("bp-1")

deprecated_bp = {"id": "bp-1", "file_md5": "md5-shared"}

with (
patch(
"openforge.db.fixtures.incremental.blueprint_sql"
".mark_blueprint_deprecated"
) as mark_dep,
patch(
"openforge.db.fixtures.incremental.tag_sql.delete_all_blueprint_tags"
) as del_tags,
patch(
"openforge.db.fixtures.incremental.image_sql"
".delete_images_for_blueprint"
) as del_imgs,
):
loader._handle_deprecation(Mock(), deprecated_bp)

# The skip path means none of the deprecation side-effects fire.
mark_dep.assert_not_called()
del_tags.assert_not_called()
del_imgs.assert_not_called()

def test_apply_changes_resets_renamed_ids_per_load(self):
"""_renamed_blueprint_ids must reset at the top of each apply so
state from one fixture doesn't leak into the next."""
loader = self._rename_loader()
loader._renamed_blueprint_ids.add("stale-id-from-prior-load")

empty_changes = ComparisonResult()

# _link_deprecated_to_successors runs at the end and calls
# cursor.fetchall(); make it return an empty list.
cursor = Mock()
cursor.fetchall = Mock(return_value=[])

with patch.object(loader, "_load_existing_blueprints", return_value={}):
loader._apply_changes_with_cursor(cursor, empty_changes)

assert loader._renamed_blueprint_ids == set()