diff --git a/mmif/serialize/view.py b/mmif/serialize/view.py index f0f057bf..6a99fa49 100644 --- a/mmif/serialize/view.py +++ b/mmif/serialize/view.py @@ -82,6 +82,25 @@ def __init__(self, view_obj: Optional[Union[bytes, str, dict]] = None, parent_mm self._required_attributes = ["id", "metadata", "annotations"] super().__init__(view_obj) self._fix_old_short_ids() + self._rebuild_id_counts() + + def _rebuild_id_counts(self): + """ + Re-seed the per-view auto-ID counters from the annotations loaded during + deserialization. + + ``_id_counts`` is not serialized, so a view read back from JSON starts + with an empty counter. Without this, the next :meth:`new_annotation` + regenerates an ID (e.g. ``an_1``) that already exists in the + deserialized view and :meth:`AnnotationsList.append` raises ``KeyError``. + See issue #393. + """ + for annotation in self.annotations: + local_id = annotation.id.split(self.id_delimiter)[-1] + prefix, _, num = local_id.rpartition('_') + if prefix and num.isdigit(): + self._id_counts[prefix] = max( + self._id_counts.get(prefix, 0), int(num)) def _fix_old_short_ids(self): """ diff --git a/tests/test_serialize.py b/tests/test_serialize.py index f2d36e68..271cf9c1 100644 --- a/tests/test_serialize.py +++ b/tests/test_serialize.py @@ -1451,7 +1451,57 @@ def test_document_added_properties_with_manual_capital_annotation(self): 'author')) self.assertEqual('you', list(mmif_roundtrip2.views.get_last_contentful_view().get_annotations(AnnotationTypes.Annotation))[ 0].get_property('author')) - + + def test_view_id_counts_rebuilt_on_deserialize(self): + # regression for issue #393: `_id_counts` is not serialized, so after + # deserialization the next generated id must not collide with an existing + # one already present in the view. + mmif = Mmif(validate=False) + doc = Document(); doc.at_type = DocumentTypes.VideoDocument + doc.id = 'd1'; doc.location = 'file:///v.mp4' + mmif.add_document(doc) + v = mmif.new_view() + v.metadata.app = tester_appname + a1 = v.new_annotation(AnnotationTypes.Annotation, document='d1', frameCount=10) + self.assertTrue(a1.id.endswith('an_1')) + # round-trip: the reloaded view still contains `an_1` but starts with an + # empty counter unless rebuilt + v2 = Mmif(mmif.serialize())[v.id] + a2 = v2.new_annotation(AnnotationTypes.Annotation, document='d1', duration=20) + self.assertTrue(a2.id.endswith('an_2')) # must not regenerate an_1 + + def test_capital_annotation_no_collision_across_apps(self): + # regression for issue #393: a downstream app re-deriving a document + # property with a *different* value must not crash on serialize; the + # distinct value is recorded as an additional capital annotation + # (value-based de-dup preserves both assertions), not a colliding one. + m = Mmif(validate=False) + d = Document() + d.at_type = DocumentTypes.VideoDocument + d.id = 'd1' + d.location = 'file:///v.mp4' + m.add_document(d) + v1 = m.new_view(); v1.metadata.app = 'http://app/producer' + v1.new_annotation(AnnotationTypes.TimeFrame, label='bars') + d.add_property('frameCount', 51248.0) + d.add_property('duration', 1709977.0) + + # downstream app loads it and re-derives a *different* duration value + m2 = Mmif(m.serialize()) + d2 = m2['d1'] + v2 = m2.new_view(); v2.metadata.app = 'http://app/consumer' + v2.new_annotation(AnnotationTypes.TimeFrame, label='caption') + d2.add_property('frameCount', 51248) # same value -> de-duped away + d2.add_property('duration', 1709975) # different value -> new annotation + out = Mmif(m2.serialize()) # must not raise KeyError + + caps = [a for view in out.views + for a in view.get_annotations(AnnotationTypes.Annotation)] + durations = {a.get_property('duration') for a in caps if 'duration' in a} + # both distinct duration assertions coexist; neither is lost or overwritten + self.assertEqual(2, len(caps)) + self.assertEqual({1709977.0, 1709975}, durations) + def test_capital_annotation_generation_viewfinder(self): mmif = Mmif(validate=False) for i in range(1, 3):