jenkins-bot has submitted this change. (
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/1358297?usp=email )
Change subject: lexeme: Serialize nested form and sense edits
......................................................................
lexeme: Serialize nested form and sense edits
Match saved children by ID and emit explicit additions and removals.
Keep unsaved children distinct, including when deleting them, and reject
duplicate saved IDs that would make a diff ambiguous. Normalize explicit
operation lists without inferring removals.
Preserve fields assigned after a save while reloading the baseline for
the next diff. Full serialization remains API edit data rather than a
replacement of all server content.
Bug: T189321
Change-Id: I30730f0d0447d4a8606fb7372dbd7670b6e4e110
---
M pywikibot/page/_collections.py
M pywikibot/page/_wikibase.py
M tests/collections_tests.py
M tests/wikibase_tests.py
4 files changed, 184 insertions(+), 13 deletions(-)
Approvals:
Xqt: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/page/_collections.py b/pywikibot/page/_collections.py
index 7566a17..3e07c30 100644
--- a/pywikibot/page/_collections.py
+++ b/pywikibot/page/_collections.py
@@ -541,7 +541,8 @@
index += len(self._data)
del self._data[index]
del self._by_key[obj.id]
- self._reindex(index)
+ # Other unsaved children can still share the temporary ID.
+ self._reindex(0 if obj.id == '-1' else index)
def __len__(self) -> int:
return len(self._data)
@@ -575,20 +576,65 @@
return this
@classmethod
- def normalizeData(cls, data: list) -> dict:
- """Helper function to expand data into the Wikibase API structure.
+ def normalizeData(cls, data: list[dict]) -> list[dict]:
+ """Normalize explicit API operations without inferring removals.
- :param data: Data to normalize
- :return: The altered dict from parameter data.
+ Each operation must identify an existing child by id, add a child
+ with ``add``, or delete one with both ``id`` and ``remove``.
+
+ :param data: List of child operation dictionaries
+ :return: Normalized operations in their original order
+ :raises TypeError: Input is not a list of dictionaries
+ :raises ValueError: Operation markers are missing or conflicting
"""
- raise NotImplementedError # TODO
+ if not isinstance(data, list):
+ raise TypeError('Subentity operations must be a list')
+ result = []
+ for entity in data:
+ if not isinstance(entity, dict):
+ raise TypeError('Subentity operations must be dictionaries')
+ markers = entity.keys() & {'id', 'add', 'remove'}
+ if markers not in ({'id'}, {'add'}, {'id', 'remove'}):
+ raise ValueError(
+ 'Subentities require id, add, or id with remove')
+ normalized = cls.type_class._normalizeData(entity)
+ normalized.update((key, entity[key]) for key in markers)
+ result.append(normalized)
+ return result
- def toJSON(self, diffto: dict | None = None) -> dict:
- """Create JSON suitable for Wikibase API.
+ def toJSON(self, diffto: list[dict] | None = None) -> list[dict]:
+ """Serialize current children or changes against a complete baseline.
- When diffto is provided, JSON representing differences to the
- provided data is created.
+ Full serialization is API edit data, not replacement of server
+ content. Unsaved children carry an ``add`` marker. Only a supplied
+ baseline allows missing children to become explicit removals.
+ An empty result makes no changes; it does not clear the collection.
- :param diffto: JSON containing entity data
+ :param diffto: Complete original collection, or None for all fields
+ :return: Child edits followed by removals in baseline order
+ :raises ValueError: Saved child IDs occur more than once
"""
- raise NotImplementedError # TODO
+ previous = {}
+ for entity in diffto or []:
+ entity_id = entity['id']
+ if entity_id in previous:
+ raise ValueError(f'Duplicate subentity ID: {entity_id}')
+ previous[entity_id] = entity
+
+ result = []
+ retained = set()
+ for entity in self:
+ entity_id = entity.id
+ if entity_id != '-1':
+ if entity_id in retained:
+ raise ValueError(f'Duplicate subentity ID: {entity_id}')
+ retained.add(entity_id)
+ data = entity.toJSON(diffto=previous.get(entity_id))
+ if entity_id == '-1':
+ data['add'] = ''
+ if entity_id not in previous or data != {'id': entity_id}:
+ result.append(data)
+
+ result.extend({'id': entity_id, 'remove': ''}
+ for entity_id in previous if entity_id not in retained)
+ return result
diff --git a/pywikibot/page/_wikibase.py b/pywikibot/page/_wikibase.py
index e423cad..c0c1ae1 100644
--- a/pywikibot/page/_wikibase.py
+++ b/pywikibot/page/_wikibase.py
@@ -2395,6 +2395,10 @@
When diffto is provided, JSON representing differences to the
provided data is created.
+ Forms and senses use API edit operations, not replacement of the
+ server's collections. Removing children requires a complete
+ original collection in diffto.
+
:param diffto: JSON containing entity data
"""
data = super().toJSON(diffto=diffto)
@@ -2425,6 +2429,13 @@
redirect, do not raise an exception.
:raise NotImplementedError: a value in args or kwargs
"""
+ pending = {}
+ if not force and self.__dict__.get('_needs_reload', False):
+ # Fields assigned since invalidation are new local changes.
+ pending = {key: self.__dict__[key]
+ for key in (*self.DATA_ATTRIBUTES, 'language',
+ 'lexicalCategory')
+ if key in self.__dict__}
data = super().get(force, *args, **kwargs)
if self.isRedirectPage() and not get_redirect:
@@ -2444,6 +2455,9 @@
data['language'] = self.language
data['lexicalCategory'] = self.lexicalCategory
self._needs_reload = False
+ for key, value in pending.items():
+ setattr(self, key, value)
+ data[key] = value
return data
diff --git a/tests/collections_tests.py b/tests/collections_tests.py
index a1b3470..a494b81 100755
--- a/tests/collections_tests.py
+++ b/tests/collections_tests.py
@@ -7,6 +7,7 @@
"""Tests for the Wikidata parts of the page module."""
from __future__ import annotations
+import copy
import unittest
from contextlib import suppress
@@ -16,7 +17,11 @@
LanguageDict,
SiteLinkCollection,
)
-from pywikibot.page._wikibase import LexemeSense, LexemeSenseCollection
+from pywikibot.page._wikibase import (
+ LexemeFormCollection,
+ LexemeSense,
+ LexemeSenseCollection,
+)
from tests.aspects import TestCase, WikidataTestCase
@@ -258,6 +263,90 @@
self._test_new_empty()
+class TestSubEntityJSON(DataCollectionTestCase):
+
+ """Test full serialization, diffs and explicit child operations."""
+
+ dry = True
+
+ def test_collection_diff(self) -> None:
+ """Match saved IDs and keep multiple unsaved children distinct."""
+ for cls, prefix, key in (
+ (LexemeFormCollection, 'L1-F', 'representations'),
+ (LexemeSenseCollection, 'L1-S', 'glosses'),
+ ):
+ with self.subTest(cls=cls):
+ baseline = [
+ {'id': prefix + str(index), key: {
+ 'en': {'language': 'en', 'value': str(index)}}}
+ for index in (1, 2)
+ ]
+ if cls is LexemeFormCollection:
+ for entry in baseline:
+ entry['grammaticalFeatures'] = []
+ original = copy.deepcopy(baseline)
+ collection = cls.fromJSON(baseline, self.get_repo())
+ self.assertEqual(collection.toJSON(), baseline)
+ self.assertEqual(collection.toJSON(diffto=[]), baseline)
+ self.assertEqual(collection.toJSON(diffto=baseline), [])
+ getattr(collection[0], key)['en'] = 'changed'
+ del collection[1]
+ for value in ('new one', 'new two'):
+ entity = cls.type_class(self.get_repo())
+ getattr(entity, key)['en'] = value
+ collection.append(entity)
+ changes = collection.toJSON(diffto=baseline)
+ self.assertEqual(changes[0], {
+ 'id': prefix + '1', key: {
+ 'en': {'language': 'en', 'value': 'changed'}}})
+ self.assertEqual([entry[key]['en']['value']
+ for entry in changes[1:3]],
+ ['new one', 'new two'])
+ for entry in changes[1:3]:
+ self.assertEqual(entry['add'], '')
+ self.assertNotIn('id', entry)
+ self.assertEqual(changes[-1],
+ {'id': prefix + '2', 'remove': ''})
+ self.assertEqual(baseline, original)
+ pending = cls(self.get_repo(), list(collection)[1:])
+ pending.clear()
+ self.assertEqual(pending.toJSON(), [])
+ empty = cls.new_empty(self.get_repo())
+ self.assertEqual(empty.toJSON(diffto=baseline), [
+ {'id': entry['id'], 'remove': ''} for entry in baseline])
+ with self.assertRaisesRegex(ValueError, 'Duplicate'):
+ collection.toJSON(diffto=baseline + baseline)
+ collection.append(collection[0])
+ with self.assertRaisesRegex(ValueError, 'Duplicate'):
+ collection.toJSON()
+
+ def test_normalize_operations(self) -> None:
+ """Preserve operation markers and reject ambiguous operations."""
+ data = [
+ {'id': 'L1-F1', 'representations': {'en': 'changed'}},
+ {'add': '', 'representations': {'en': 'new'},
+ 'grammaticalFeatures': []},
+ {'id': 'L1-F2', 'remove': ''},
+ ]
+ original = copy.deepcopy(data)
+ normalized = LexemeFormCollection.normalizeData(data)
+ self.assertEqual(normalized, [
+ {'id': 'L1-F1', 'representations': {
+ 'en': {'language': 'en', 'value': 'changed'}}},
+ {'add': '', 'representations': {
+ 'en': {'language': 'en', 'value': 'new'}},
+ 'grammaticalFeatures': []},
+ {'id': 'L1-F2', 'remove': ''},
+ ])
+ self.assertEqual(data, original)
+ self.assertEqual(LexemeFormCollection.normalizeData(normalized),
+ normalized)
+ for invalid in ({}, {'remove': ''}, {'add': '', 'remove': ''},
+ {'id': 'L1-F1', 'add': ''}):
+ with self.subTest(invalid=invalid), self.assertRaises(ValueError):
+ LexemeFormCollection.normalizeData([invalid])
+
+
class TestClaimCollection(DataCollectionTestCase):
"""Test cases covering ClaimCollection methods."""
diff --git a/tests/wikibase_tests.py b/tests/wikibase_tests.py
index a02051a..9950340 100755
--- a/tests/wikibase_tests.py
+++ b/tests/wikibase_tests.py
@@ -1270,6 +1270,28 @@
self.assertIsNot(lexeme.forms, old_forms)
self.assertEqual(lexeme.forms[0].id, 'L1-F1')
+ def test_repeated_save(self) -> None:
+ """Reload assigned IDs so a second save does not repeat additions."""
+ form = LexemeForm(self.repo)
+ form.representations['en'] = 'colours'
+ self.lexeme.forms.append(form)
+ response = copy.deepcopy(self.content)
+ response['lastrevid'] = 2
+ response['forms'] = [dict(form.toJSON(), id='L1-F1')]
+ with patch.object(self.repo, 'editEntity', return_value={
+ 'entity': response}) as save, patch.object(
+ self.repo, 'loadcontent', side_effect=lambda *args: {
+ 'L1': copy.deepcopy(response)}):
+ self.lexeme.editEntity()
+ self.assertIn('add', save.call_args.args[1]['forms'][0])
+ self.lexeme.editEntity()
+ self.assertEqual(save.call_args.args[1], {})
+ self.assertEqual(save.call_args.kwargs['baserevid'], 2)
+ self.lexeme.lemmas = LanguageDict({'en': 'new local edit'})
+ self.lexeme.editEntity()
+ self.assertEqual(save.call_args.args[1], {'lemmas': {
+ 'en': {'language': 'en', 'value': 'new local edit'}}})
+
def test_failed_save_and_reload(self) -> None:
"""Rejected edits preserve changes; failed reloads prevent writes."""
lexeme = self.lexeme
--
To view, visit
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/1358297?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.wikimedia.org/r/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: I30730f0d0447d4a8606fb7372dbd7670b6e4e110
Gerrit-Change-Number: 1358297
Gerrit-PatchSet: 6
Gerrit-Owner: Mahveotm <[email protected]>
Gerrit-Reviewer: Xqt <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
Pywikibot-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
