jenkins-bot has submitted this change. (
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/965243 )
Change subject: [IMPR] Improve handling of uninitialized MediaInfo
......................................................................
[IMPR] Improve handling of uninitialized MediaInfo
Make it possible to retrieve just the id for non-existing
mediainfo's of existing files, and use this as an internal
assertion. It will also throw an error for invalid
mediainfo references.
Drop MediaInfo.get_data_for_new_entity because it will
not be called anymore.
Change-Id: I7ad7396708e9fc7fcd98274c3668f07e25a1be30
---
M pywikibot/page/_wikibase.py
M tests/file_tests.py
2 files changed, 101 insertions(+), 38 deletions(-)
Approvals:
Zache-tool: Looks good to me, but someone else must approve
Xqt: Looks good to me, approved
jenkins-bot: Verified
diff --git a/pywikibot/page/_wikibase.py b/pywikibot/page/_wikibase.py
index 753dc3f..bbfd71f 100644
--- a/pywikibot/page/_wikibase.py
+++ b/pywikibot/page/_wikibase.py
@@ -130,14 +130,17 @@
def __getattr__(self, name):
if name in self.DATA_ATTRIBUTES:
if self.getID() == '-1':
- for key, cls in self.DATA_ATTRIBUTES.items():
- setattr(self, key, cls.new_empty(self.repo))
+ self._initialize_empty()
return getattr(self, name)
return self.get()[name]
raise AttributeError("'{}' object has no attribute
'{}'"
.format(self.__class__.__name__, name))
+ def _initialize_empty(self):
+ for key, cls in self.DATA_ATTRIBUTES.items():
+ setattr(self, key, cls.new_empty(self.repo))
+
def _defined_by(self, singular: bool = False) -> dict:
"""
Internal function to provide the API parameters to identify the entity.
@@ -355,7 +358,7 @@
"""
Return the full concept URI.
- :raise NoWikibaseEntityError: if this entity doesn't exist
+ :raise NoWikibaseEntityError: if this entity's id is not known
"""
entity_id = self.getID()
if entity_id == '-1':
@@ -370,6 +373,7 @@
.. versionadded:: 6.5
"""
+ entity_type = 'mediainfo'
title_pattern = r'M[1-9]\d*'
DATA_ATTRIBUTES = {
'labels': LanguageDict,
@@ -378,12 +382,39 @@
def __getattr__(self, name):
if name == 'claims': # T149410
- name = 'statements'
- if hasattr(self, name):
- return getattr(self, name)
+ return self.statements
+
+ if name in self.DATA_ATTRIBUTES:
+ if not self.exists():
+ self._assert_has_id()
+ self._initialize_empty()
+ return getattr(self, name)
return super().__getattr__(name)
+ def _assert_has_id(self):
+ if self.id != '-1':
+ return
+
+ if not self.file.exists():
+ exc = NoPageError(self.file)
+ raise NoWikibaseEntityError(self) from exc
+
+ self.id = 'M' + str(self.file.pageid)
+
+ def _defined_by(self, singular: bool = False) -> dict:
+ """
+ Internal function to provide the API parameters to identify the entity.
+
+ :param singular: Whether the parameter names should use the singular
+ form
+ :raise NoWikibaseEntityError: if this entity is associated with
+ a non-existing file
+ :return: API parameters
+ """
+ self._assert_has_id()
+ return super()._defined_by(singular)
+
@property
def file(self) -> FilePage:
"""Get the file associated with the mediainfo."""
@@ -397,7 +428,8 @@
pywikibot.error(msg)
raise Error(msg)
- page_id = self.getID(numeric=True)
+ # avoid recursion with self.getID()
+ page_id = int(self.id[1:])
result = list(self.repo.load_pages_from_pageids([page_id]))
if not result:
raise Error(f'There is no existing page with id
"{page_id}"')
@@ -410,27 +442,24 @@
return self._file
- def get_data_for_new_entity(self) -> dict:
- """Return data required for creation of a new
mediainfo."""
- self.id = 'M' + str(self.file.pageid)
- self._content = {}
- return super().get()
-
def get(self, force: bool = False) -> dict:
"""Fetch all MediaInfo entity data and cache it.
+ .. note:: This method may raise exception even if the associated file
+ exists because the mediainfo may not have been initialized yet.
+ :attr:`labels` and :attr:`statements` can still be accessed and
+ modified. :meth:`exists` suppresses the exception.
+
+ .. note:: dicts returned by this method are references to content
+ of this entity and their modifying may indirectly cause
+ unwanted change to the live content
+
:param force: override caching
:raise NoWikibaseEntityError: if this entity doesn't exist
:return: actual data which entity holds
"""
if self.id == '-1':
- if force:
- if not self.file.exists():
- exc = NoPageError(self.file)
- raise NoWikibaseEntityError(self) from exc
- # get just the id for Wikibase API call
- self.id = 'M' + str(self.file.pageid)
- else:
+ if not force:
try:
data =
self.file.latest_revision.slots['mediainfo']['*']
except NoPageError as exc:
@@ -443,6 +472,8 @@
self._content = jsonlib.loads(data)
self.id = self._content['id']
+ self._assert_has_id()
+
return super().get(force=force)
def getID(self, numeric: bool = False):
@@ -450,9 +481,10 @@
Get the entity identifier.
:param numeric: Strip the first letter and return an int
+ :raise NoWikibaseEntityError: if this entity is associated with
+ a non-existing file
"""
- if self.id == '-1':
- self.get()
+ self._assert_has_id()
return super().getID(numeric=numeric)
diff --git a/tests/file_tests.py b/tests/file_tests.py
index 911c595..8a252c4 100755
--- a/tests/file_tests.py
+++ b/tests/file_tests.py
@@ -13,6 +13,7 @@
import pywikibot
from pywikibot.exceptions import (
+ Error,
NoPageError,
NoWikibaseEntityError,
PageRelatedError,
@@ -351,7 +352,7 @@
self.assertTrue(item.file is page)
self.assertEqual('-1', item.id)
item.get()
- self.assertEqual('M14634781', item.getID())
+ self.assertEqual('M14634781', item.id)
self.assertIsInstance(
item.labels, pywikibot.page._collections.LanguageDict)
self.assertIsInstance(
@@ -371,6 +372,15 @@
del item._file
self.assertEqual(page, item.file)
+ def test_data_item_not_file(self):
+ """Test data item with invalid pageid."""
+ item = pywikibot.MediaInfo(self.site, 'M1') # Main Page
+ with self.assertRaises(Error):
+ item.file
+ with self.assertRaises(NoWikibaseEntityError):
+ item.get()
+ self.assertFalse(item.exists())
+
def test_data_item_when_no_file_or_data_item(self):
"""Test data item associated to file that does not
exist."""
page = pywikibot.FilePage(self.site,
@@ -381,6 +391,8 @@
with self.assertRaises(NoWikibaseEntityError):
item.get()
+ with self.assertRaises(NoWikibaseEntityError):
+ item.labels
def test_data_item_when_file_exist_but_without_item(self):
"""Test if data item is missing from file."""
@@ -394,24 +406,26 @@
# Seek to first page without mediainfo.
for page in gen:
- if 'mediainfo' not in page.latest_revision.slots:
- item = page.data_item()
- self.assertIsInstance(item, pywikibot.MediaInfo)
+ if 'mediainfo' in page.latest_revision.slots:
+ continue
- # Get fails as there is no mediainfo.
- with self.assertRaises(NoWikibaseEntityError):
- item.get()
+ item = page.data_item()
+ self.assertIsInstance(item, pywikibot.MediaInfo)
- # Create new empty mediainfo.
- item.get_data_for_new_entity()
- self.assertIsInstance(
- item.labels, pywikibot.page._collections.LanguageDict)
- self.assertIsInstance(
- item.statements,
- pywikibot.page._collections.ClaimCollection)
+ # Get fails as there is no mediainfo.
+ with self.assertRaises(NoWikibaseEntityError):
+ item.get()
- # break the loop after checking first file
- break
+ self.assertFalse(item.exists())
+ self.assertEqual(f'M{page.pageid}', item.id)
+ self.assertIsInstance(
+ item.labels, pywikibot.page._collections.LanguageDict)
+ self.assertIsInstance(
+ item.statements,
+ pywikibot.page._collections.ClaimCollection)
+
+ # break the loop after checking first file
+ break
def test_data_list_to_dict_workaround(self):
"""Test that T222159 workaround converts [] to
{}."""
--
To view, visit
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/965243
To unsubscribe, or for help writing mail filters, visit
https://gerrit.wikimedia.org/r/settings
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: I7ad7396708e9fc7fcd98274c3668f07e25a1be30
Gerrit-Change-Number: 965243
Gerrit-PatchSet: 7
Gerrit-Owner: Matěj Suchánek <matejsuchanek97(a)gmail.com>
Gerrit-Reviewer: Xqt <info(a)gno.de>
Gerrit-Reviewer: Zache-tool <kimmo.virtanen(a)gmail.com>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged