Merge pull request bartfeenstra#875 from bartfeenstra/fix-ghost-anony…

…mized-entities

Fix a bug where ghost versions of anonymized sources and citations would remain in ancestries

betty/anonymizer/__init__.py

@@ -18,10 +18,17 @@ class AnonymousSource(Source):
     def __init__(self):
         super().__init__(self._ID, _('Private'))
 
-    def replace(self, other: Source) -> None:
+    def replace(self, other: Source, ancestry: Ancestry) -> None:
+        if isinstance(other, AnonymousSource):
+            return
+
         self.citations.append(*other.citations)
+        other.citations.clear()
         self.contains.append(*other.contains)
+        other.contains.clear()
         self.files.append(*other.files)
+        other.files.clear()
+        ancestry.entities[Source].remove(other)
 
 
 class AnonymousCitation(Citation):
@@ -31,9 +38,15 @@ def __init__(self, source: Source):
         super().__init__(self._ID, source)
         self.location = _("A citation is available, but has not been published in order to protect people's privacy")
 
-    def replace(self, other: Citation) -> None:
+    def replace(self, other: Citation, ancestry: Ancestry) -> None:
+        if isinstance(other, AnonymousCitation):
+            return
+
         self.facts.append(*other.facts)
+        other.facts.clear()
         self.files.append(*other.files)
+        other.files.clear()
+        ancestry.entities[Citation].remove(other)
 
 
 def anonymize(ancestry: Ancestry, anonymous_citation: AnonymousCitation) -> None:
@@ -52,10 +65,10 @@ def anonymize(ancestry: Ancestry, anonymous_citation: AnonymousCitation) -> None
             anonymize_file(file)
     for source in ancestry.entities[Source]:
         if source.private:
-            anonymize_source(source, anonymous_source)
+            anonymize_source(source, ancestry, anonymous_source)
     for citation in ancestry.entities[Citation]:
         if citation.private:
-            anonymize_citation(citation, anonymous_citation)
+            anonymize_citation(citation, ancestry, anonymous_citation)
 
 
 def anonymize_person(person: Person) -> None:
@@ -90,19 +103,28 @@ def anonymize_file(file: File) -> None:
     del file.entities
 
 
-def anonymize_source(source: Source, anonymous_source: AnonymousSource) -> None:
-    anonymous_source.replace(source)
-    del source.citations
+def anonymize_source(source: Source, ancestry: Ancestry, anonymous_source: AnonymousSource) -> None:
+    if isinstance(source, AnonymousSource):
+        return
+
+    anonymous_source.replace(source, ancestry)
+    for citation in source.citations:
+        if not isinstance(citation, AnonymousCitation):
+            source.citations.remove(citation)
     del source.contained_by
     del source.contains
     del source.files
 
 
-def anonymize_citation(citation: Citation, anonymous_citation: AnonymousCitation) -> None:
-    anonymous_citation.replace(citation)
+def anonymize_citation(citation: Citation, ancestry: Ancestry, anonymous_citation: AnonymousCitation) -> None:
+    if isinstance(citation, AnonymousCitation):
+        return
+
+    anonymous_citation.replace(citation, ancestry)
     del citation.facts
     del citation.files
-    del citation.source
+    if not isinstance(citation.source, AnonymousSource):
+        del citation.source
 
 
 class Anonymizer(Extension, PostLoader, GuiBuilder):

betty/tests/anonymizer/test__init__.py

@@ -27,18 +27,21 @@ def test_name(self):
         self.assertIsInstance(AnonymousSource().name, str)
 
     def test_replace(self):
+        ancestry = Ancestry()
         citations = [Citation(None, Source(None))]
         contains = [Source(None)]
         files = [Mock(File)]
         sut = AnonymousSource()
-        other = AnonymousSource()
+        other = Source(None)
+        ancestry.entities.append(other)
         other.citations = citations
         other.contains = contains
         other.files = files
-        sut.replace(other)
+        sut.replace(other, ancestry)
         self.assertEqual(citations, list(sut.citations))
         self.assertEqual(contains, list(sut.contains))
         self.assertEqual(files, list(sut.files))
+        self.assertNotIn(other, ancestry.entities)
 
 
 class AnonymousCitationTest(TestCase):
@@ -56,16 +59,19 @@ def test_location(self):
     def test_replace(self):
         class _HasCitations(HasCitations, Entity):
             pass
+        ancestry = Ancestry()
         facts = [_HasCitations()]
         files = [File('F1', __file__)]
         source = Mock(Source)
         sut = AnonymousCitation(source)
-        other = AnonymousCitation(source)
+        other = Citation(None, source)
+        ancestry.entities.append(other)
         other.facts = facts
         other.files = files
-        sut.replace(other)
+        sut.replace(other, ancestry)
         self.assertEqual(facts, list(sut.facts))
         self.assertEqual(files, list(sut.files))
+        self.assertNotIn(other, ancestry.entities)
 
 
 class AnonymizeTest(TestCase):
@@ -146,7 +152,7 @@ def test_with_private_source_should_anonymize(self, m_anonymize_source) -> None:
         ancestry = Ancestry()
         ancestry.entities.append(source)
         anonymize(ancestry, AnonymousCitation(AnonymousSource()))
-        m_anonymize_source.assert_called_once_with(source, ANY)
+        m_anonymize_source.assert_called_once_with(source, ancestry, ANY)
 
     @patch('betty.anonymizer.anonymize_citation')
     def test_with_public_citation_should_not_anonymize(self, m_anonymize_citation) -> None:
@@ -166,7 +172,7 @@ def test_with_private_citation_should_anonymize(self, m_anonymize_citation) -> N
         ancestry = Ancestry()
         ancestry.entities.append(citation)
         anonymize(ancestry, AnonymousCitation(AnonymousSource()))
-        m_anonymize_citation.assert_called_once_with(citation, ANY)
+        m_anonymize_citation.assert_called_once_with(citation, ancestry, ANY)
 
 
 class AnonymizePersonTest(TestCase):
@@ -269,37 +275,45 @@ def tearDown(self) -> None:
         self._translations.uninstall()
 
     def test_should_remove_citations(self) -> None:
+        ancestry = Ancestry()
         source = Source('S0', 'The Source')
+        ancestry.entities.append(source)
         citation = Citation(None, source)
         source.citations.append(citation)
         anonymous_source = AnonymousSource()
-        anonymize_source(source, anonymous_source)
+        anonymize_source(source, ancestry, anonymous_source)
         self.assertEqual(0, len(source.citations))
         self.assertIn(citation, anonymous_source.citations)
 
     def test_should_remove_contained_by(self) -> None:
+        ancestry = Ancestry()
         source = Source('S0', 'The Source')
+        ancestry.entities.append(source)
         contained_by = Source(None, 'The Source')
         source.contained_by = contained_by
         anonymous_source = AnonymousSource()
-        anonymize_source(source, anonymous_source)
+        anonymize_source(source, ancestry, anonymous_source)
         self.assertIsNone(source.contained_by)
 
     def test_should_remove_contains(self) -> None:
+        ancestry = Ancestry()
         source = Source('S0', 'The Source')
+        ancestry.entities.append(source)
         contains = Source(None, 'The Source')
         source.contains.append(contains)
         anonymous_source = AnonymousSource()
-        anonymize_source(source, anonymous_source)
+        anonymize_source(source, ancestry, anonymous_source)
         self.assertEqual(0, len(source.contains))
         self.assertIn(contains, anonymous_source.contains)
 
     def test_should_remove_files(self) -> None:
+        ancestry = Ancestry()
         source = Source('S0', 'The Source')
+        ancestry.entities.append(source)
         file = File('F0', __file__)
         source.files.append(file)
         anonymous_source = AnonymousSource()
-        anonymize_source(source, anonymous_source)
+        anonymize_source(source, ancestry, anonymous_source)
         self.assertEqual(0, len(source.files))
         self.assertIn(file, anonymous_source.files)
 
@@ -313,33 +327,39 @@ def tearDown(self) -> None:
         self._translations.uninstall()
 
     def test_should_remove_facts(self) -> None:
+        ancestry = Ancestry()
         source = Source('The Source')
         citation = Citation('C0', source)
+        ancestry.entities.append(citation)
         fact = PersonName(Person(None), 'Jane')
         citation.facts.append(fact)
         anonymous_source = AnonymousSource()
         anonymous_citation = AnonymousCitation(anonymous_source)
-        anonymize_citation(citation, anonymous_citation)
+        anonymize_citation(citation, ancestry, anonymous_citation)
         self.assertEqual(0, len(citation.facts))
         self.assertIn(fact, anonymous_citation.facts)
 
     def test_should_remove_files(self) -> None:
+        ancestry = Ancestry()
         source = Source('The Source')
         citation = Citation('C0', source)
+        ancestry.entities.append(citation)
         file = File('F0', __file__)
         citation.files.append(file)
         anonymous_source = AnonymousSource()
         anonymous_citation = AnonymousCitation(anonymous_source)
-        anonymize_citation(citation, anonymous_citation)
+        anonymize_citation(citation, ancestry, anonymous_citation)
         self.assertEqual(0, len(citation.files))
         self.assertIn(file, anonymous_citation.files)
 
     def test_should_remove_source(self) -> None:
+        ancestry = Ancestry()
         source = Source('The Source')
         citation = Citation('C0', source)
+        ancestry.entities.append(citation)
         anonymous_source = AnonymousSource()
         anonymous_citation = AnonymousCitation(anonymous_source)
-        anonymize_citation(citation, anonymous_citation)
+        anonymize_citation(citation, ancestry, anonymous_citation)
         self.assertIsNone(citation.source)