From 719f27f94013e8f1f4977eaa9184adecffe325aa Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Tue, 17 Mar 2026 22:32:57 +0000 Subject: [PATCH 1/2] feat: id validation also removes scanning through all of the current ids if no id is given. --- tests/test_app.py | 15 +++++++++++++++ tilia/app.py | 30 +++++++++++++++++------------- tilia/timelines/base/timeline.py | 4 ++-- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/tests/test_app.py b/tests/test_app.py index a1be29f6..a3834ea8 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -20,6 +20,7 @@ from tilia.settings import settings from tilia.requests import Get, Post, post, get +from tilia.timelines.component_kinds import ComponentKind from tilia.timelines.timeline_kinds import TimelineKind from tilia.ui import commands from tilia.ui.windows import WindowKind @@ -724,3 +725,17 @@ def test_are_unique_between_files_timelines_and_components( self.assert_ids_are_unique( tls, 61 ) # 10 marker timelines + 1 slider timeline + 50 components + + def test_create_component_with_higher_id_before_lower_id(self, marker_tl): + marker_tl.create_component(ComponentKind.MARKER, time=0, id=10) + marker_tl.create_component(ComponentKind.MARKER, time=1, id=1) + marker_tl.create_component(ComponentKind.MARKER, time=2) + + assert [c.id for c in marker_tl.components] == ["10", "1", "11"] + + def test_create_component_with_duplicate_ids(self, marker_tl): + marker_tl.create_component(ComponentKind.MARKER, time=0) # id=1; tl has id=0 + marker_tl.create_component(ComponentKind.MARKER, time=1, id=1) + marker_tl.create_component(ComponentKind.MARKER, time=2, id=1) + + assert [c.id for c in marker_tl.components] == ["1", "2", "3"] diff --git a/tilia/app.py b/tilia/app.py index 2792234e..7cf9d068 100644 --- a/tilia/app.py +++ b/tilia/app.py @@ -275,28 +275,32 @@ def on_record_state(self, action, no_repeat=False, repeat_identifier=""): repeat_identifier=repeat_identifier, ) - def get_id(self) -> str: + def get_id(self, id: str | None = None) -> str: """ Returns an ID string. - IDs are unique accross timeline component and timelines. + IDs are unique across timeline component and timelines. Other IDs might contain duplicates. """ - timeline_ids = {c.id for c in self.timelines} + if id is None: + return str(next(self._id_counter)) + + int_id = int(id) + timeline_ids = {int(c.id) for c in self.timelines} component_lists = [ tl.components for tl in self.timelines if tl.components is not None ] - component_ids = set() - for component_list in component_lists: - component_ids = component_ids.union({c.id for c in component_list}) + component_ids = { + int(c.id) for component_list in component_lists for c in component_list + } existing_ids = timeline_ids.union(component_ids) - next_id = next(self._id_counter) - # Find the highest existing ID and increment until counter is higher - if existing_ids: - highest_id = max(int(id_str) for id_str in existing_ids) - while next_id <= highest_id: - next_id = next(self._id_counter) - return str(next_id) + if int_id in existing_ids: + return str(next(self._id_counter)) + + if not existing_ids or int_id > max(existing_ids): + self._id_counter = itertools.count(int_id + 1) + + return str(int_id) def reset_id_generator(self): self._id_counter = itertools.count() diff --git a/tilia/timelines/base/timeline.py b/tilia/timelines/base/timeline.py index 2d4b65a8..3391f735 100644 --- a/tilia/timelines/base/timeline.py +++ b/tilia/timelines/base/timeline.py @@ -71,7 +71,7 @@ def __init__( id: int | None = None, **kwargs, # ignores components_hash ): - self.id = id if id is not None else get(Get.ID) + self.id = get(Get.ID, id) self.name = name self.is_visible = is_visible @@ -189,7 +189,7 @@ def get_data(self, attr: str): def create_component( self, kind: ComponentKind, *args, id=None, **kwargs ) -> tuple[TC | None, str | None]: - component_id = id or get(Get.ID) + component_id = get(Get.ID, id) success, component, reason = self.component_manager.create_component( kind, self, component_id, *args, **kwargs ) From 9ed9725e6b8588add8d960a9053924caa898ddce Mon Sep 17 00:00:00 2001 From: azfoo <45888544+azfoo@users.noreply.github.com> Date: Fri, 20 Mar 2026 19:02:40 +0000 Subject: [PATCH 2/2] fix: invalid ids --- tests/test_app.py | 7 +++++++ tilia/app.py | 14 +++++++++++++- tilia/errors.py | 3 +++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/test_app.py b/tests/test_app.py index a3834ea8..1acdafac 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -739,3 +739,10 @@ def test_create_component_with_duplicate_ids(self, marker_tl): marker_tl.create_component(ComponentKind.MARKER, time=2, id=1) assert [c.id for c in marker_tl.components] == ["1", "2", "3"] + + @pytest.mark.parametrize("id", ["not an int", 3.1415, False]) + def test_invalid_id(self, marker_tl, id): + with PatchPost("tilia.errors", Post.DISPLAY_ERROR) as error: + marker_tl.create_component(ComponentKind.MARKER, time=1, id=id) + error.assert_called() + assert marker_tl.components[0].id == "1" diff --git a/tilia/app.py b/tilia/app.py index 7cf9d068..214cbe41 100644 --- a/tilia/app.py +++ b/tilia/app.py @@ -281,10 +281,22 @@ def get_id(self, id: str | None = None) -> str: IDs are unique across timeline component and timelines. Other IDs might contain duplicates. """ + + def invalid_id(id): + tilia.errors.display(tilia.errors.INVALID_ID, id) + return str(next(self._id_counter)) + if id is None: return str(next(self._id_counter)) - int_id = int(id) + if type(id) not in [int, str]: + return invalid_id(id) + + try: + int_id = int(id) + except ValueError: + return invalid_id(id) + timeline_ids = {int(c.id) for c in self.timelines} component_lists = [ tl.components for tl in self.timelines if tl.components is not None diff --git a/tilia/errors.py b/tilia/errors.py index 3d04c7dd..7aecb4d5 100644 --- a/tilia/errors.py +++ b/tilia/errors.py @@ -121,6 +121,9 @@ class Error(NamedTuple): "Error creating score timeline", "Duplicate or missing staff numbers. Staff numbers found: {}.\nDeleting timeline.", ) +INVALID_ID = Error( + "Error parsing id", "'{}' is not parsable as a valid id. Using generated id." +) def display(error: Error, *args):