diff --git a/test/conftest.py b/test/conftest.py index 35e3ff36..4fc8797e 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -159,6 +159,9 @@ def _handle_api(self, request: Any, context: Any) -> dict: def _handle_sparql(self, request: Any, context: Any) -> dict: params = parse_qs(urlparse(request.url).query, keep_blank_values=True) + # The query is sent in the request body (form-encoded), fall back to the URL query string for robustness. + if request.method == 'POST' and request.text: + params.update(parse_qs(request.text, keep_blank_values=True)) query = params.get('query', [''])[0] self.sparql_queries.append(query) diff --git a/test/integration/README.md b/test/integration/README.md index dfc7b18c..32e60c9b 100644 --- a/test/integration/README.md +++ b/test/integration/README.md @@ -12,14 +12,40 @@ enabled with the `integration` marker plus a few environment variables. ```bash docker compose -f test/integration/docker-compose.yml up -d -# wait until http://localhost:8880 answers +# first boot runs the MediaWiki installer; wait until http://localhost:8880 answers +``` + +Then set the environment variables and run the tests. The variables must be set +**in the same shell** that launches pytest (they are read when the tests are +collected); if they are missing, every integration test is silently **skipped** +and never contacts the instance. + +Bash / zsh: +```bash WBI_INTEGRATION_MEDIAWIKI_API_URL=http://localhost:8880/w/api.php \ WBI_INTEGRATION_USER=WikibaseAdmin \ WBI_INTEGRATION_PASSWORD=WikibaseDockerAdminPass \ pytest -m integration ``` +PowerShell (Windows) — the inline `VAR=value cmd` syntax above does **not** work +here, assign `$env:` variables first: + +```powershell +$env:WBI_INTEGRATION_MEDIAWIKI_API_URL = "http://localhost:8880/w/api.php" +$env:WBI_INTEGRATION_USER = "WikibaseAdmin" +$env:WBI_INTEGRATION_PASSWORD = "WikibaseDockerAdminPass" +poetry run pytest -m integration -v -rs +``` + +Expect `PASSED` lines. If you see `SKIPPED ... is not set`, the variables did not +reach pytest (wrong shell syntax, or set in a different window). + +The generated wiki config and the database are kept in named volumes. To reset +to a clean instance, tear it down with `docker compose -f +test/integration/docker-compose.yml down -v` (the `-v` wipes both volumes). + ## Running against another instance Any instance you are allowed to write to works (for example diff --git a/test/integration/conftest.py b/test/integration/conftest.py index 1822ad29..708e36df 100644 --- a/test/integration/conftest.py +++ b/test/integration/conftest.py @@ -12,6 +12,7 @@ See test/integration/README.md for the docker-compose setup. """ import os +from copy import deepcopy import pytest @@ -31,17 +32,37 @@ def integration_api_url() -> str: return API_URL -@pytest.fixture(autouse=True) -def integration_config(integration_api_url, preserve_config): - """Point wbi_config to the instance under test.""" +@pytest.fixture(scope='session', autouse=True) +def integration_config(integration_api_url): + """ + Point wbi_config to the instance under test, for the whole test session. + + This must be session-scoped (not the more natural function-scoped autouse): + pytest sets up fixtures broadest-scope-first, so a function-scoped fixture would + run *after* session/module-scoped ones. `login` (session) and `string_property` + (module, in test_wikibase_roundtrip.py) both talk to the real instance during their + own setup, using whatever mediawiki_api_url is in wbi_config at that point. A + function-scoped fixture would still be pointing at the default Wikidata URL then, + which mismatches the login object's own URL and raises a ValueError in + mediawiki_api_call_helper ("mediawiki_api_url can't be different with the one in + the login object."). + + (The function-scoped, autouse `preserve_config` fixture from the top-level conftest + still runs per test on top of this and is harmless: since it always executes after + this session fixture, its snapshot already includes the values set here.) + """ + original = deepcopy(wbi_config) wbi_config['USER_AGENT'] = 'WikibaseIntegrator-integration-tests/1.0' wbi_config['MEDIAWIKI_API_URL'] = integration_api_url if SPARQL_URL: wbi_config['SPARQL_ENDPOINT_URL'] = SPARQL_URL + yield wbi_config + wbi_config.clear() + wbi_config.update(original) @pytest.fixture(scope='session') -def login(integration_api_url): +def login(integration_api_url, integration_config): if not USER or not PASSWORD: pytest.skip('WBI_INTEGRATION_USER / WBI_INTEGRATION_PASSWORD are not set') return wbi_login.Login(user=USER, password=PASSWORD, mediawiki_api_url=integration_api_url, user_agent='WikibaseIntegrator-integration-tests/1.0') diff --git a/test/integration/docker-compose.yml b/test/integration/docker-compose.yml index d07c77c3..1fb29ad3 100644 --- a/test/integration/docker-compose.yml +++ b/test/integration/docker-compose.yml @@ -1,17 +1,28 @@ # Minimal Wikibase instance for the WikibaseIntegrator integration tests. # # docker compose -f test/integration/docker-compose.yml up -d -# # wait for http://localhost:8880 to answer, then: +# # wait for http://localhost:8880 to answer (first boot runs the MediaWiki +# # installer and can take a minute), then: # WBI_INTEGRATION_MEDIAWIKI_API_URL=http://localhost:8880/w/api.php \ # WBI_INTEGRATION_USER=WikibaseAdmin \ # WBI_INTEGRATION_PASSWORD=WikibaseDockerAdminPass \ # pytest -m integration # -# Based on the Wikibase release pipeline images: +# # to reset to a clean instance (wipes the generated config and the database): +# docker compose -f test/integration/docker-compose.yml down -v +# +# Based on the Wikibase Suite (release pipeline) images: # https://github.com/wmde/wikibase-release-pipeline +# +# Note: the wikibase/wikibase image requires a volume mounted at /config. On +# first boot, with an empty volume, its entrypoint generates LocalSettings.php +# and the wiki secret key there from the environment variables below. Because +# that generated config is persisted, the database is persisted with the same +# lifecycle so both stay in sync (use `down -v` to reset both together). +# Elasticsearch/CirrusSearch stay disabled as long as ELASTICSEARCH_HOST is unset. services: wikibase: - image: wikibase/wikibase + image: wikibase/wikibase:7 ports: - "8880:80" environment: @@ -19,10 +30,17 @@ services: MW_ADMIN_PASS: WikibaseDockerAdminPass MW_ADMIN_EMAIL: admin@wikibase.example MW_WG_SERVER: http://localhost:8880 + # The image forces an explicit opt-in/opt-out for the WMDE metadata pingback; + # leaving it unset makes the container exit. Disabled for a local test instance. + # (quoted so compose passes the literal string "false", not a YAML boolean) + METADATA_CALLBACK: "false" DB_SERVER: mysql:3306 DB_NAME: wikibase DB_USER: wikibase DB_PASS: wikibase + volumes: + - wikibase-config:/config + - wikibase-image-data:/var/www/html/images depends_on: mysql: condition: service_healthy @@ -34,8 +52,15 @@ services: MYSQL_DATABASE: wikibase MYSQL_USER: wikibase MYSQL_PASSWORD: wikibase + volumes: + - mysql-data:/var/lib/mysql healthcheck: test: healthcheck.sh --connect --innodb_initialized start_period: 30s interval: 5s retries: 20 + +volumes: + wikibase-config: + wikibase-image-data: + mysql-data: diff --git a/test/integration/test_wikibase_roundtrip.py b/test/integration/test_wikibase_roundtrip.py index df0f49d2..1a7ca72a 100644 --- a/test/integration/test_wikibase_roundtrip.py +++ b/test/integration/test_wikibase_roundtrip.py @@ -12,7 +12,8 @@ import pytest from wikibaseintegrator.datatypes import Item, String -from wikibaseintegrator.wbi_exceptions import NonExistentEntityError +from wikibaseintegrator.wbi_enums import ActionIfExists +from wikibaseintegrator.wbi_exceptions import MissingEntityException from wikibaseintegrator.wbi_helpers import search_entities pytestmark = pytest.mark.integration @@ -53,8 +54,10 @@ def test_create_read_update_delete(self, wbi, string_property): assert fetched.lastrevid == written.lastrevid # Update: label + a second claim + # Claims.add() defaults to ActionIfExists.REPLACE_ALL, which would replace (remove) + # the existing claim for this property instead of adding a second one. fetched.labels.set(language='en', value=label + ' (updated)') - fetched.claims.add(String(prop_nr=string_property.id, value='second value')) + fetched.claims.add(String(prop_nr=string_property.id, value='second value'), action_if_exists=ActionIfExists.APPEND_OR_REPLACE) updated = fetched.write(summary='WikibaseIntegrator integration test: update') assert updated.labels.get('en') == label + ' (updated)' @@ -65,7 +68,11 @@ def test_create_read_update_delete(self, wbi, string_property): updated.delete(reason='WikibaseIntegrator integration test cleanup') def test_get_nonexistent_item(self, wbi): - with pytest.raises(NonExistentEntityError): + # A well-formed but non-existent numeric ID doesn't trigger an API-level error: wbgetentities + # replies 200 with the entity marked 'missing', which BaseEntity.from_json turns into + # MissingEntityException. NonExistentEntityError is for the separate no-such-entity/missingtitle + # API error path (e.g. an invalid site+title lookup). + with pytest.raises(MissingEntityException): wbi.item.get('Q999999999') diff --git a/test/test_datatypes.py b/test/test_datatypes.py index 9d34bf11..e2bb420f 100644 --- a/test/test_datatypes.py +++ b/test/test_datatypes.py @@ -43,6 +43,23 @@ def test_json(self): assert dt_json['mainsnak']['datavalue']['type'] == 'string' +class TestGlobeCoordinate: + def test_equality_does_not_mutate_values(self): + latitude = 1.234567891 + coordinate1 = GlobeCoordinate(latitude=latitude, longitude=2.3456789, precision=1e-9, prop_nr='P10') + coordinate2 = GlobeCoordinate(latitude=latitude, longitude=2.3456789, precision=1e-9, prop_nr='P10') + + # Equality is checked on rounded values, but the claims themselves must keep their full precision + assert coordinate1 == coordinate2 + assert coordinate1.mainsnak.datavalue['value']['latitude'] == latitude + + def test_equality_with_valueless_claim(self): + coordinate = GlobeCoordinate(latitude=1.5, longitude=2.5, prop_nr='P10') + + assert coordinate != GlobeCoordinate(prop_nr='P10') + assert coordinate != Item(value='Q123', prop_nr='P10') + + class TestTime: def test_accessors(self): time = Time(time='-2023-12-31T00:00:00Z', before=1, after=2, precision=3, timezone=4, prop_nr='P5') @@ -60,6 +77,16 @@ def test_comparisons(self): assert time <= time2 assert time != time2 + def test_large_year_parsing(self): + # Years with more than 4 digits must be parsed correctly instead of being sliced at fixed positions + time = Time(time='+10000-01-02T00:00:00Z', prop_nr='P5') + assert time.get_year() == 10000 + assert time.get_month() == 1 + assert time.get_day() == 2 + + # Ordering keeps working across the 4/5-digit boundary + assert Time(time='+9999-01-01T00:00:00Z', prop_nr='P5') < time + class TestRank: def test_rank_parsing(self): diff --git a/test/test_entity_item.py b/test/test_entity_item.py index d05ceee2..e63ec1a4 100644 --- a/test/test_entity_item.py +++ b/test/test_entity_item.py @@ -118,6 +118,8 @@ def test_write_as_new(self, wikibase, item_q582): written = item.write(allow_anonymous=True, as_new=True) assert wikibase.last_edit['params']['new'] == 'item' + # A new entity must not carry an id in its data payload (regression: 'id': null was sent) + assert 'id' not in wikibase.last_edit['data'] assert written.id != 'Q582' def test_write_limited_claims(self, wikibase, item_q582): diff --git a/test/test_models.py b/test/test_models.py index 3950984c..366af3ca 100644 --- a/test/test_models.py +++ b/test/test_models.py @@ -11,8 +11,8 @@ from wikibaseintegrator import WikibaseIntegrator, datatypes from wikibaseintegrator.datatypes import Item, MonolingualText, String from wikibaseintegrator.entities import ItemEntity -from wikibaseintegrator.models import Descriptions, Form, Qualifiers -from wikibaseintegrator.wbi_enums import ActionIfExists +from wikibaseintegrator.models import Claims, Descriptions, Form, Qualifiers +from wikibaseintegrator.wbi_enums import ActionIfExists, WikibaseSnakType from .conftest import load_fixture @@ -81,6 +81,15 @@ def test_set_label_in_new_language(self, item): item.labels.set(value='label', language='ak') assert item.labels.get('ak') == 'label' + def test_language_value_none_guards(self): + from wikibaseintegrator.models.language_values import LanguageValue + + # A LanguageValue with no value must not crash on str/len/in (regression: these raised TypeError on None) + empty = LanguageValue(language='en', value=None) + assert str(empty) == '' + assert len(empty) == 0 + assert ('anything' in empty) is False + class TestDescriptions: def test_set_and_replace(self, item): @@ -194,6 +203,48 @@ def test_claim_reset_id(self, item): claim.reset_id() assert claim.id is None + def test_len_counts_properties_while_count_counts_claims(self): + claims = Claims() + claims.add([String(prop_nr='P1', value='a'), String(prop_nr='P1', value='b'), String(prop_nr='P2', value='c')], action_if_exists=ActionIfExists.FORCE_APPEND) + + # len() is the number of distinct properties, count() the number of individual claims + assert len(claims) == 2 + assert claims.count() == 3 + + def test_claim_comparison_with_unrelated_types(self): + claim = String(prop_nr='P1', value='foo') + + # Comparing a claim with an unrelated type must return False, not raise + assert claim != 5 + assert (claim == {'mainsnak': {}}) is False + + def test_remove_unsaved_claims(self): + claims = Claims() + claims.add([String(prop_nr='P1', value='a'), String(prop_nr='P1', value='b')], action_if_exists=ActionIfExists.FORCE_APPEND) + + claims.remove('P1') + assert claims.get('P1') == [] + + def test_merge_refs_or_append_with_valueless_snak(self): + # A no-value snak has no datavalue: MERGE_REFS_OR_APPEND must not raise a KeyError on it. + claims = Claims() + claims.add(String(prop_nr='P1', snaktype=WikibaseSnakType.NO_VALUE), action_if_exists=ActionIfExists.MERGE_REFS_OR_APPEND) + claims.add(String(prop_nr='P1', snaktype=WikibaseSnakType.NO_VALUE), action_if_exists=ActionIfExists.MERGE_REFS_OR_APPEND) + + # The two identical no-value statements are recognized as equal, so only one is kept + assert len(claims.get('P1')) == 1 + + def test_merge_refs_or_append_merges_references(self): + claims = Claims() + claims.add(Item(value='Q1', prop_nr='P1', references=[datatypes.ExternalID(value='ref1', prop_nr='P352')]), + action_if_exists=ActionIfExists.MERGE_REFS_OR_APPEND) + # Same value but a new reference block: the reference is merged into the existing claim, no new claim is added + claims.add(Item(value='Q1', prop_nr='P1', references=[datatypes.ExternalID(value='ref2', prop_nr='P352')]), + action_if_exists=ActionIfExists.MERGE_REFS_OR_APPEND) + + assert len(claims.get('P1')) == 1 + assert len(claims.get('P1')[0].references) == 2 + def test_multiline_string_values_rejected(self): item = ItemEntity() @@ -217,6 +268,14 @@ def test_remove(self, item): removed = copy.deepcopy(item) assert len(removed.claims.get('P443')[0].qualifiers.remove(Item(prop_nr='P407', value='Q150'))) == 0 + def test_count(self): + claim = Item(prop_nr='P1') + claim.qualifiers.set([Item(prop_nr='P2', value='Q1'), Item(prop_nr='P2', value='Q2'), Item(prop_nr='P3', value='Q3')]) + + # len() is the number of distinct qualifier properties, count() the number of individual snaks + assert len(claim.qualifiers) == 2 + assert claim.qualifiers.count() == 3 + def test_equality(self): claim1 = Item(prop_nr='P1') claim1.qualifiers.set([Item(prop_nr='P2', value='Q1'), Item(prop_nr='P2', value='Q2')]) @@ -264,6 +323,17 @@ def test_statement_equality_with_and_without_refs(self): olditem.references.add(datatypes.ExternalID(value='99999', prop_nr='P352')) assert olditem.equals(newitem, include_ref=True) + def test_reference_removal(self): + claim = datatypes.Item(value='Q123', prop_nr='P123', references=[datatypes.ExternalID(value='P58742', prop_nr='P352')]) + + # Removing a reference that is not present returns False + assert claim.references.remove(datatypes.ExternalID(value='unknown', prop_nr='P352')) is False + assert len(claim.references) == 1 + + # An equivalent reference built from a claim is found and removed + assert claim.references.remove(datatypes.ExternalID(value='P58742', prop_nr='P352')) is True + assert len(claim.references) == 0 + class TestForms: def test_get_forms(self): @@ -281,6 +351,46 @@ def test_get_forms(self): assert lexeme.forms.get('L5-F4') and lexeme.forms.get('L5-F5') assert len(lexeme.forms) == 4 + def test_grammatical_features_setter(self): + # int is normalized to a Q-id, str/list are wrapped/kept as a list + assert Form(grammatical_features=123).grammatical_features == ['Q123'] + assert Form(grammatical_features='Q123').grammatical_features == ['Q123'] + assert Form(grammatical_features=['Q1', 'Q2']).grammatical_features == ['Q1', 'Q2'] + assert Form().grammatical_features == [] + + # The setter replaces the value instead of accumulating on each assignment + form = Form() + form.grammatical_features = 5 + form.grammatical_features = 6 + assert form.grammatical_features == ['Q6'] + + with pytest.raises(TypeError): + Form(grammatical_features=1.5) + with pytest.raises(TypeError): + Form(grammatical_features=True) + + +class TestTermsEntity: + def test_term_setters_validate_type_across_entities(self): + from wikibaseintegrator.entities import ItemEntity, MediaInfoEntity, PropertyEntity + + # The shared labels/descriptions/aliases setters (now on the TermsEntity base) still validate their type + for entity_cls in (ItemEntity, PropertyEntity, MediaInfoEntity): + with pytest.raises(TypeError): + entity_cls(labels='not a Labels object') + + def test_mediainfo_reads_aliases_from_json(self): + from wikibaseintegrator.entities import MediaInfoEntity + + media = MediaInfoEntity().from_json({ + 'id': 'M1', 'type': 'mediainfo', 'lastrevid': 1, + 'labels': {'en': {'language': 'en', 'value': 'a caption'}}, + 'descriptions': {}, + 'aliases': {'en': [{'language': 'en', 'value': 'an alias'}]}, + }) + assert media.labels.get('en') == 'a caption' + assert 'an alias' in [alias.value for alias in media.aliases.get('en')] + class TestWikibaseIntegratorApi: def test_is_bot_propagation(self): diff --git a/test/test_wbi_fastrun.py b/test/test_wbi_fastrun.py index e52d843f..54799266 100644 --- a/test/test_wbi_fastrun.py +++ b/test/test_wbi_fastrun.py @@ -120,6 +120,43 @@ def test_write_required_via_entity(self, wikibase, frc, item_q582): item.claims.add(ExternalID(value='CHANGED', prop_nr='P352')) assert item.write_required(base_filter=[BaseDataType(prop_nr='P352'), Item(prop_nr='P703', value='Q27510868')]) is True + def test_write_required_with_property_path_base_filter(self, wikibase, item_q582): + """ + A property-path base_filter (a list of BaseDataType) must still select the matching claims. + + Regression: the anchor property (here P352) was never matched, so a write was always wrongly reported. + """ + wikibase.add_property('P352', 'external-id') + wikibase.add_property('P703', 'wikibase-item') + wikibase.sparql_bindings = statement_bindings(wikibase, 'Q582', 'P352', [literal('P40095')]) + + base_filter = [[BaseDataType(prop_nr='P352'), BaseDataType(prop_nr='P703')]] + wbi_fastrun.fastrun_store.append(wbi_fastrun.FastRunContainer(base_filter=base_filter, base_data_type=BaseDataType)) + + item = ItemEntity().from_json(load_fixture('item_Q582')) + item.claims.add(ExternalID(value='P40095', prop_nr='P352')) + assert item.write_required(base_filter=base_filter) is False + + item.claims.add(ExternalID(value='CHANGED', prop_nr='P352')) + assert item.write_required(base_filter=base_filter) is True + + +class TestPropDatatype: + def test_get_prop_datatype_is_cached(self, wikibase): + wikibase.add_property('P352', 'external-id') + frc = wbi_fastrun.FastRunContainer(base_filter=[BaseDataType(prop_nr='P352')], base_data_type=BaseDataType) + + assert frc.get_prop_datatype('P352') == 'external-id' + calls = len([r for r in wikibase.requests if r.get('action') == 'wbgetentities']) + + # A second lookup must be served from the per-instance cache, without any additional API request + assert frc.get_prop_datatype('P352') == 'external-id' + assert len([r for r in wikibase.requests if r.get('action') == 'wbgetentities']) == calls + + # clear() invalidates the cache + frc.clear() + assert 'P352' not in frc.prop_dt_map + class TestLanguageData: def test_language_data_and_check(self, wikibase): diff --git a/test/test_wbi_helpers.py b/test/test_wbi_helpers.py index fbad77e0..b23db7a9 100644 --- a/test/test_wbi_helpers.py +++ b/test/test_wbi_helpers.py @@ -8,9 +8,10 @@ import requests from wikibaseintegrator.wbi_config import config as wbi_config -from wikibaseintegrator.wbi_exceptions import MaxRetriesReachedException, ModificationFailed, MWApiError, NonExistentEntityError, SaveFailed +from wikibaseintegrator.wbi_exceptions import AnonymousEditNotAllowedError, MaxRetriesReachedException, ModificationFailed, MWApiError, NonExistentEntityError, SaveFailed from wikibaseintegrator.wbi_helpers import (download_entity_ttl, execute_sparql_query, format2wbi, format_amount, fulltext_search, generate_entity_instances, get_user_agent, - lexeme_remove_form, lexeme_remove_sense, mediawiki_api_call, mediawiki_api_call_helper, merge_items, remove_claims, search_entities) + lexeme_edit_sense, lexeme_remove_form, lexeme_remove_sense, mediawiki_api_call, mediawiki_api_call_helper, merge_items, remove_claims, + search_entities) class FakeLogin: @@ -84,6 +85,26 @@ def test_format_must_be_json(self): mediawiki_api_call('POST', mediawiki_api_url='https://example.org/w/api.php', data={'format': 'xml'}) +class TestTimeout: + def test_default_timeout_is_applied(self, wikibase, requests_mock): + wbi_config['TIMEOUT'] = (3, 33) + mediawiki_api_call('POST', mediawiki_api_url=wikibase.mediawiki_api_url, data={'action': 'wbsearchentities', 'search': 'x', 'language': 'en', 'format': 'json'}) + assert requests_mock.last_request.timeout == (3, 33) + + def test_explicit_timeout_is_not_overridden(self, wikibase, requests_mock): + wbi_config['TIMEOUT'] = (3, 33) + mediawiki_api_call('POST', mediawiki_api_url=wikibase.mediawiki_api_url, data={'action': 'wbsearchentities', 'search': 'x', 'language': 'en', 'format': 'json'}, timeout=7) + assert requests_mock.last_request.timeout == 7 + + +class TestAnonymousEdit: + def test_anonymous_edit_not_allowed_raises_dedicated_error(self, wikibase): + # A login object that only yields the anonymous token must trigger the dedicated exception, not a bare Exception. + login = FakeLogin(mediawiki_api_url=wikibase.mediawiki_api_url, edit_token='+\\') + with pytest.raises(AnonymousEditNotAllowedError): + mediawiki_api_call_helper(data={'action': 'wbeditentity', 'id': 'Q1', 'format': 'json'}, login=login, mediawiki_api_url=wikibase.mediawiki_api_url) + + class TestErrorMapping: """MediaWiki error payloads must be converted to the right exceptions.""" @@ -196,6 +217,18 @@ def test_search_stops_at_max_results(self, wikibase): search_requests = [r for r in wikibase.requests if r.get('action') == 'wbsearchentities'] assert len(search_requests) == 1 + def test_search_truncates_to_max_results(self, wikibase): + # A page holds up to 50 results, so a max_results that is not a multiple of 50 would overshoot without truncation + wikibase.search_results = [{'id': f'Q{i}', 'label': f'result {i}', 'match': {}} for i in range(200)] + + results = search_entities('rivaroxaban', max_results=60) + assert len(results) == 60 + assert results[-1] == 'Q59' + + # Only the pages needed to reach 60 results are fetched (2 x 50), not the whole dataset + search_requests = [r for r in wikibase.requests if r.get('action') == 'wbsearchentities'] + assert len(search_requests) == 2 + def test_search_dict_result(self, wikibase): wikibase.search_results = [{'id': 'Q1', 'label': 'result', 'match': {}, 'description': 'a description', 'aliases': ['alias']}] @@ -212,6 +245,14 @@ def test_search_language_parameter(self, wikibase): search_entities('anything') assert wikibase.last_request['language'] == str(wbi_config['DEFAULT_LANGUAGE']) + def test_search_strict_language_and_limit_parameters(self, wikibase): + wikibase.search_results = [{'id': f'Q{i}', 'label': f'result {i}', 'match': {}} for i in range(20)] + + results = search_entities('anything', strict_language=True, max_results=10) + assert 'strictlanguage' in wikibase.last_request + assert wikibase.last_request['limit'] == '10' + assert len(results) == 10 + class TestFulltextSearch: def test_fulltext_search(self, wikibase): @@ -249,6 +290,15 @@ def test_lexeme_form_and_sense_id_validation(self): with pytest.raises(ValueError): lexeme_remove_sense('invalid-sense-id') + def test_lexeme_edit_sense_sends_sense_id(self, wikibase): + # The mock doesn't implement wbleditsenseelements, so the call fails, but the request parameters are still recorded + with pytest.raises(MWApiError): + lexeme_edit_sense('L10-S2', data={}, allow_anonymous=True) + + request = wikibase.last_request + assert request['action'] == 'wbleditsenseelements' + assert request['senseId'] == 'L10-S2' + class TestGenerateEntityInstances: def test_multiple_entities(self, wikibase, item_q582): @@ -293,6 +343,15 @@ def test_execute_sparql_query_with_prefix(self, wikibase): execute_sparql_query('SELECT * WHERE { ?a ?b ?c . }', prefix=prefix) assert prefix in wikibase.sparql_queries[-1] + def test_query_is_sent_in_request_body(self, wikibase, requests_mock): + wikibase.sparql_bindings = [] + execute_sparql_query('SELECT * WHERE { ?a ?b ?c . }') + + last = requests_mock.last_request + # The query travels in the form-encoded request body (not the URL), without the previous bogus multipart header + assert 'query=' in (last.text or '') + assert last.headers.get('Content-Type', '').startswith('application/x-www-form-urlencoded') + def test_sparql_retry_on_429(self, wikibase, requests_mock): url = 'https://throttled.example.org/sparql' requests_mock.post(url, [ @@ -322,6 +381,7 @@ def test_format_amount(self): assert format_amount(0) == '+0' +@pytest.mark.filterwarnings("ignore:format2wbi.. is experimental:UserWarning") class TestFormat2Wbi: def test_entity_types(self, wikibase): from wikibaseintegrator.entities import ItemEntity, LexemeEntity, MediaInfoEntity, PropertyEntity diff --git a/test/test_wbi_login.py b/test/test_wbi_login.py index 7ea67ad5..3e9b0c0f 100644 --- a/test/test_wbi_login.py +++ b/test/test_wbi_login.py @@ -88,3 +88,14 @@ def test_invalid_client(self, credentials, requests_mock): with pytest.raises(LoginError): wbi_login.OAuth2(consumer_token='wrong', consumer_secret='wrong') + + def test_access_token_is_refreshed_on_renewal(self, credentials, requests_mock): + token_matcher = requests_mock.post(credentials.mediawiki_rest_url + '/oauth2/access_token', + json={'access_token': 'oauth2-access-token', 'token_type': 'Bearer', 'expires_in': 14400}) + + login = wbi_login.OAuth2(consumer_token='consumer-token', consumer_secret='consumer-secret', token_renew_period=0) + calls_after_init = token_matcher.call_count + + # The short-lived access token has no refresh token, so renewing the credentials must re-fetch it (not only the CSRF token). + login.get_edit_token() + assert token_matcher.call_count > calls_after_init diff --git a/wikibaseintegrator/datatypes/globecoordinate.py b/wikibaseintegrator/datatypes/globecoordinate.py index 611ee797..d0eb0568 100644 --- a/wikibaseintegrator/datatypes/globecoordinate.py +++ b/wikibaseintegrator/datatypes/globecoordinate.py @@ -61,19 +61,17 @@ def set_value(self, latitude: float | None = None, longitude: float | None = Non } def __eq__(self, other): - if isinstance(other, Claim) and other.mainsnak.datavalue['type'] == 'globecoordinate': - tmp_datavalue_self = self.mainsnak.datavalue - tmp_datavalue_other = other.mainsnak.datavalue - - tmp_datavalue_self['value']['latitude'] = round(tmp_datavalue_self['value']['latitude'], 6) - tmp_datavalue_self['value']['longitude'] = round(tmp_datavalue_self['value']['longitude'], 6) - tmp_datavalue_self['value']['precision'] = round(tmp_datavalue_self['value']['precision'], 17) - - tmp_datavalue_other['value']['latitude'] = round(tmp_datavalue_other['value']['latitude'], 6) - tmp_datavalue_other['value']['longitude'] = round(tmp_datavalue_other['value']['longitude'], 6) - tmp_datavalue_other['value']['precision'] = round(tmp_datavalue_other['value']['precision'], 17) - - return tmp_datavalue_self == tmp_datavalue_other and self.mainsnak.property_number == other.mainsnak.property_number and self.has_equal_qualifiers(other) + if isinstance(other, Claim) and self.mainsnak.datavalue.get('type') == 'globecoordinate' and other.mainsnak.datavalue.get('type') == 'globecoordinate': + # Compare rounded copies to ignore precision noise without mutating the claims + def rounded(datavalue: dict) -> dict: + value = dict(datavalue['value']) + value['latitude'] = round(value['latitude'], 6) + value['longitude'] = round(value['longitude'], 6) + value['precision'] = round(value['precision'], 17) + return {**datavalue, 'value': value} + + return rounded(self.mainsnak.datavalue) == rounded(other.mainsnak.datavalue) and self.mainsnak.property_number == other.mainsnak.property_number \ + and self.has_equal_qualifiers(other) return super().__eq__(other) diff --git a/wikibaseintegrator/datatypes/time.py b/wikibaseintegrator/datatypes/time.py index 5b3cada0..1276d767 100644 --- a/wikibaseintegrator/datatypes/time.py +++ b/wikibaseintegrator/datatypes/time.py @@ -105,16 +105,26 @@ def set_value(self, time: str | None = None, before: int = 0, after: int = 0, pr def get_sparql_value(self) -> str: return self.mainsnak.datavalue['value']['time'] + def _time_parts(self) -> tuple[int, int, int]: + """ + Split the timestamp into (year, month, day). + + The year can be signed and hold more than 4 digits (up to 16), so it can't be sliced at fixed positions. + """ + time = self.mainsnak.datavalue['value']['time'] + matches = re.match(r'^([+-]?[0-9]+)-([0-9]{2})-([0-9]{2})T', time) + if not matches: + raise ValueError(f"Unable to parse time value '{time}'") + return int(matches.group(1)), int(matches.group(2)), int(matches.group(3)) + def get_year(self) -> int: - return int(self.mainsnak.datavalue['value']['time'][0:5]) + return self._time_parts()[0] def get_month(self) -> int: - return int(self.mainsnak.datavalue['value']['time'][6:8]) + return self._time_parts()[1] def get_day(self) -> int: - return int(self.mainsnak.datavalue['value']['time'][9:11]) + return self._time_parts()[2] def __lt__(self, other): - return (self.get_year() < other.get_year()) or \ - (self.get_year() == other.get_year() and self.get_month() < other.get_month()) or \ - (self.get_year() == other.get_year() and self.get_month() == other.get_month() and self.get_day() < other.get_day()) + return self._time_parts() < other._time_parts() diff --git a/wikibaseintegrator/entities/baseentity.py b/wikibaseintegrator/entities/baseentity.py index 95f7027a..08ca9f0a 100644 --- a/wikibaseintegrator/entities/baseentity.py +++ b/wikibaseintegrator/entities/baseentity.py @@ -6,7 +6,10 @@ from wikibaseintegrator import wbi_fastrun from wikibaseintegrator.datatypes import BaseDataType +from wikibaseintegrator.models.aliases import Aliases from wikibaseintegrator.models.claims import Claim, Claims +from wikibaseintegrator.models.descriptions import Descriptions +from wikibaseintegrator.models.labels import Labels from wikibaseintegrator.wbi_enums import ActionIfExists, EntityField from wikibaseintegrator.wbi_exceptions import MissingEntityException from wikibaseintegrator.wbi_helpers import delete_page, edit_entity, mediawiki_api_call_helper @@ -279,7 +282,8 @@ def _write(self, data: dict | None = None, summary: str | None = None, login: _L if as_new: entity_id = None - data['id'] = None + # Don't keep an id when creating a new entity: a null id in the data payload is rejected by the API. + data.pop('id', None) else: entity_id = self.id @@ -326,9 +330,18 @@ def write_required(self, base_filter: list[BaseDataType | list[BaseDataType]] | if base_filter is None: base_filter = [] + # Collect the property numbers targeted by the base_filter. It supports both the simple form (a + # BaseDataType) and the property-path form (a list of two BaseDataType), whose anchor is the first property. + base_filter_props = set() + for prop in base_filter: + if isinstance(prop, BaseDataType): + base_filter_props.add(prop.mainsnak.property_number) + elif isinstance(prop, list) and prop and isinstance(prop[0], BaseDataType): + base_filter_props.add(prop[0].mainsnak.property_number) + claims_to_check = [] for claim in self.claims: - if claim.mainsnak.property_number in base_filter: + if claim.mainsnak.property_number in base_filter_props: claims_to_check.append(claim) # TODO: Add check_language_data @@ -357,3 +370,60 @@ def __repr__(self): id=id(self) & 0xFFFFFF, attrs="\r\n\t ".join(f"{k}={v!r}" for k, v in self.__dict__.items()), ) + + +class TermsEntity(BaseEntity): + """ + Base class for the entities that share the labels/descriptions/aliases "terms": Item, Property and MediaInfo. + """ + + def __init__(self, labels: Labels | None = None, descriptions: Descriptions | None = None, aliases: Aliases | None = None, **kwargs: Any) -> None: + super().__init__(**kwargs) + + self.labels = labels or Labels() + self.descriptions = descriptions or Descriptions() + self.aliases = aliases or Aliases() + + @property + def labels(self) -> Labels: + return self.__labels + + @labels.setter + def labels(self, labels: Labels): + if not isinstance(labels, Labels): + raise TypeError + self.__labels = labels + + @property + def descriptions(self) -> Descriptions: + return self.__descriptions + + @descriptions.setter + def descriptions(self, descriptions: Descriptions): + if not isinstance(descriptions, Descriptions): + raise TypeError + self.__descriptions = descriptions + + @property + def aliases(self) -> Aliases: + return self.__aliases + + @aliases.setter + def aliases(self, aliases: Aliases): + if not isinstance(aliases, Aliases): + raise TypeError + self.__aliases = aliases + + def _terms_from_json(self, json_data: dict[str, Any]) -> None: + """ + Deserialize the labels/descriptions/aliases blocks. + + Only the terms present in ``json_data`` are set, so this is safe to call on entities that never carry some + of them (e.g. a MediaInfo entity usually has no aliases). + """ + if 'labels' in json_data: + self.labels = Labels().from_json(json_data['labels']) + if 'descriptions' in json_data: + self.descriptions = Descriptions().from_json(json_data['descriptions']) + if 'aliases' in json_data: + self.aliases = Aliases().from_json(json_data['aliases']) diff --git a/wikibaseintegrator/entities/item.py b/wikibaseintegrator/entities/item.py index dcfe8d1d..2d133adb 100644 --- a/wikibaseintegrator/entities/item.py +++ b/wikibaseintegrator/entities/item.py @@ -3,15 +3,14 @@ import re from typing import Any -from wikibaseintegrator.entities.baseentity import BaseEntity -from wikibaseintegrator.models import LanguageValues +from wikibaseintegrator.entities.baseentity import BaseEntity, TermsEntity from wikibaseintegrator.models.aliases import Aliases from wikibaseintegrator.models.descriptions import Descriptions from wikibaseintegrator.models.labels import Labels from wikibaseintegrator.models.sitelinks import Sitelinks -class ItemEntity(BaseEntity): +class ItemEntity(TermsEntity): ETYPE = 'item' def __init__(self, labels: Labels | None = None, descriptions: Descriptions | None = None, aliases: Aliases | None = None, sitelinks: Sitelinks | None = None, **kwargs: Any) -> None: @@ -24,12 +23,7 @@ def __init__(self, labels: Labels | None = None, descriptions: Descriptions | No :param sitelinks: :param kwargs: """ - super().__init__(**kwargs) - - # Item, Property and MediaInfo specific - self.labels: LanguageValues = labels or Labels() - self.descriptions: LanguageValues = descriptions or Descriptions() - self.aliases = aliases or Aliases() + super().__init__(labels=labels, descriptions=descriptions, aliases=aliases, **kwargs) # Item specific self.sitelinks = sitelinks or Sitelinks() @@ -53,36 +47,6 @@ def id(self, value: None | str | int): BaseEntity.id.fset(self, value) # type: ignore - @property - def labels(self) -> Labels: - return self.__labels - - @labels.setter - def labels(self, labels: Labels): - if not isinstance(labels, Labels): - raise TypeError - self.__labels = labels - - @property - def descriptions(self) -> Descriptions: - return self.__descriptions - - @descriptions.setter - def descriptions(self, descriptions: Descriptions): - if not isinstance(descriptions, Descriptions): - raise TypeError - self.__descriptions = descriptions - - @property - def aliases(self) -> Aliases: - return self.__aliases - - @aliases.setter - def aliases(self, aliases: Aliases): - if not isinstance(aliases, Aliases): - raise TypeError - self.__aliases = aliases - @property def sitelinks(self) -> Sitelinks: return self.__sitelinks @@ -142,13 +106,8 @@ def get_json(self) -> dict[str, str | dict]: def from_json(self, json_data: dict[str, Any]) -> ItemEntity: super().from_json(json_data=json_data) + super()._terms_from_json(json_data=json_data) - if 'labels' in json_data: - self.labels = Labels().from_json(json_data['labels']) - if 'descriptions' in json_data: - self.descriptions = Descriptions().from_json(json_data['descriptions']) - if 'aliases' in json_data: - self.aliases = Aliases().from_json(json_data['aliases']) if 'sitelinks' in json_data: self.sitelinks = Sitelinks().from_json(json_data['sitelinks']) diff --git a/wikibaseintegrator/entities/mediainfo.py b/wikibaseintegrator/entities/mediainfo.py index 5fe4bdec..98831b1d 100644 --- a/wikibaseintegrator/entities/mediainfo.py +++ b/wikibaseintegrator/entities/mediainfo.py @@ -3,15 +3,15 @@ import re from typing import Any -from wikibaseintegrator.entities.baseentity import BaseEntity -from wikibaseintegrator.models import Claims, LanguageValues +from wikibaseintegrator.entities.baseentity import BaseEntity, TermsEntity +from wikibaseintegrator.models import Claims from wikibaseintegrator.models.aliases import Aliases from wikibaseintegrator.models.descriptions import Descriptions from wikibaseintegrator.models.labels import Labels from wikibaseintegrator.wbi_helpers import mediawiki_api_call_helper -class MediaInfoEntity(BaseEntity): +class MediaInfoEntity(TermsEntity): ETYPE = 'mediainfo' def __init__(self, labels: Labels | None = None, descriptions: Descriptions | None = None, aliases: Aliases | None = None, **kwargs: Any) -> None: @@ -24,12 +24,7 @@ def __init__(self, labels: Labels | None = None, descriptions: Descriptions | No :param sitelinks: :param kwargs: """ - super().__init__(**kwargs) - - # Item, Property and MediaInfo specific - self.labels: LanguageValues = labels or Labels() - self.descriptions: LanguageValues = descriptions or Descriptions() - self.aliases = aliases or Aliases() + super().__init__(labels=labels, descriptions=descriptions, aliases=aliases, **kwargs) @BaseEntity.id.setter # type: ignore def id(self, value: None | str | int): @@ -50,36 +45,6 @@ def id(self, value: None | str | int): BaseEntity.id.fset(self, value) # type: ignore - @property - def labels(self) -> Labels: - return self.__labels - - @labels.setter - def labels(self, labels: Labels): - if not isinstance(labels, Labels): - raise TypeError - self.__labels = labels - - @property - def descriptions(self) -> Descriptions: - return self.__descriptions - - @descriptions.setter - def descriptions(self, descriptions: Descriptions): - if not isinstance(descriptions, Descriptions): - raise TypeError - self.__descriptions = descriptions - - @property - def aliases(self) -> Aliases: - return self.__aliases - - @aliases.setter - def aliases(self, aliases: Aliases): - if not isinstance(aliases, Aliases): - raise TypeError - self.__aliases = aliases - def new(self, **kwargs: Any) -> MediaInfoEntity: return MediaInfoEntity(api=self.api, **kwargs) @@ -141,11 +106,8 @@ def get_json(self) -> dict[str, str | dict]: def from_json(self, json_data: dict[str, Any]) -> MediaInfoEntity: super().from_json(json_data=json_data) + super()._terms_from_json(json_data=json_data) - if 'labels' in json_data: - self.labels = Labels().from_json(json_data['labels']) - if 'descriptions' in json_data: - self.descriptions = Descriptions().from_json(json_data['descriptions']) if 'statements' in json_data: self.claims = Claims().from_json(json_data['statements']) diff --git a/wikibaseintegrator/entities/property.py b/wikibaseintegrator/entities/property.py index c23ec656..e27868cf 100644 --- a/wikibaseintegrator/entities/property.py +++ b/wikibaseintegrator/entities/property.py @@ -3,27 +3,22 @@ import re from typing import Any -from wikibaseintegrator.entities.baseentity import BaseEntity +from wikibaseintegrator.entities.baseentity import BaseEntity, TermsEntity from wikibaseintegrator.models.aliases import Aliases from wikibaseintegrator.models.descriptions import Descriptions from wikibaseintegrator.models.labels import Labels from wikibaseintegrator.wbi_enums import WikibaseDatatype -class PropertyEntity(BaseEntity): +class PropertyEntity(TermsEntity): ETYPE = 'property' def __init__(self, datatype: str | WikibaseDatatype | None = None, labels: Labels | None = None, descriptions: Descriptions | None = None, aliases: Aliases | None = None, **kwargs: Any): - super().__init__(**kwargs) + super().__init__(labels=labels, descriptions=descriptions, aliases=aliases, **kwargs) # Property specific self.datatype = datatype - # Item, Property and MediaInfo specific - self.labels: Labels = labels or Labels() - self.descriptions: Descriptions = descriptions or Descriptions() - self.aliases = aliases or Aliases() - @BaseEntity.id.setter # type: ignore def id(self, value: None | str | int): if isinstance(value, str): @@ -54,36 +49,6 @@ def datatype(self, value: str | WikibaseDatatype | None): else: self.__datatype = value - @property - def labels(self) -> Labels: - return self.__labels - - @labels.setter - def labels(self, labels: Labels): - if not isinstance(labels, Labels): - raise TypeError - self.__labels = labels - - @property - def descriptions(self) -> Descriptions: - return self.__descriptions - - @descriptions.setter - def descriptions(self, descriptions: Descriptions): - if not isinstance(descriptions, Descriptions): - raise TypeError - self.__descriptions = descriptions - - @property - def aliases(self) -> Aliases: - return self.__aliases - - @aliases.setter - def aliases(self, aliases: Aliases): - if not isinstance(aliases, Aliases): - raise TypeError - self.__aliases = aliases - def new(self, **kwargs: Any) -> PropertyEntity: return PropertyEntity(api=self.api, **kwargs) @@ -119,15 +84,10 @@ def get_json(self) -> dict[str, str | Any]: def from_json(self, json_data: dict[str, Any]) -> PropertyEntity: super().from_json(json_data=json_data) + super()._terms_from_json(json_data=json_data) if 'datatype' in json_data: self.datatype = json_data['datatype'] - if 'labels' in json_data: - self.labels = Labels().from_json(json_data['labels']) - if 'descriptions' in json_data: - self.descriptions = Descriptions().from_json(json_data['descriptions']) - if 'aliases' in json_data: - self.aliases = Aliases().from_json(json_data['aliases']) return self diff --git a/wikibaseintegrator/models/claims.py b/wikibaseintegrator/models/claims.py index e95f9700..afff32d3 100644 --- a/wikibaseintegrator/models/claims.py +++ b/wikibaseintegrator/models/claims.py @@ -36,7 +36,7 @@ def get(self, property: str | int) -> list[Claim]: def remove(self, property: str | None = None) -> None: if property in self.claims: - for prop in self.claims[property]: + for prop in list(self.claims[property]): if prop.id: prop.remove() else: @@ -102,11 +102,9 @@ def add(self, claims: Claims | list[Claim] | Claim, action_if_exists: ActionIfEx elif action_if_exists == ActionIfExists.MERGE_REFS_OR_APPEND: claim_exists = False for existing_claim in self.claims[property]: - existing_claim_json = existing_claim.get_json() - claim_to_add_json = claim.get_json() - - # Check if the values match, including qualifiers - if (claim_to_add_json["mainsnak"]["datavalue"]["value"] == existing_claim_json["mainsnak"]["datavalue"]["value"]) and claim.quals_equal(claim, existing_claim): + # Compare the main snaks (which also handles no-value/some-value snaks that have no + # datavalue) and the qualifiers to decide if the statement already exists. + if claim.mainsnak == existing_claim.mainsnak and claim.quals_equal(claim, existing_claim): claim_exists = True # Check if current reference block is present on references @@ -145,6 +143,15 @@ def get_json(self) -> dict[str, list]: del json_data[property] return json_data + def count(self) -> int: + """ + Return the total number of individual claims, across every property. + + Note: ``len(claims)`` returns the number of distinct properties, while iterating (``for claim in claims``) + yields the individual claims. Use this method when you need the claim count. + """ + return sum(len(claim_list) for claim_list in self.claims.values()) + def __iter__(self): iterate = [] for claim in self.claims.values(): @@ -152,6 +159,7 @@ def __iter__(self): return iter(iterate) def __len__(self): + # Returns the number of distinct properties. See count() for the total number of individual claims. return len(self.claims) @@ -351,7 +359,7 @@ def __contains__(self, item): if isinstance(item, str): return self.mainsnak.datavalue == item - return super().__contains__(item) + return False def __eq__(self, other): if isinstance(other, Claim): @@ -360,7 +368,7 @@ def __eq__(self, other): if isinstance(other, str): return self.mainsnak.property_number == other - raise super().__eq__(other) + return NotImplemented def equals(self, that: Claim, include_ref: bool = False, fref: Callable | None = None) -> bool: """ diff --git a/wikibaseintegrator/models/forms.py b/wikibaseintegrator/models/forms.py index 0c62ae0b..4b68a4da 100644 --- a/wikibaseintegrator/models/forms.py +++ b/wikibaseintegrator/models/forms.py @@ -78,14 +78,16 @@ def grammatical_features(self): return self.__grammatical_features @grammatical_features.setter - def grammatical_features(self, value: str | int | list[str]): - if not hasattr(self, '__grammatical_features') or value is None: - self.__grammatical_features = [] - - if isinstance(value, int): - self.__grammatical_features.append('Q' + str(value)) + def grammatical_features(self, value: str | int | list[str] | None): + if value is None: + self.__grammatical_features: list[str] = [] + elif isinstance(value, bool): + # bool is a subclass of int, reject it explicitly + raise TypeError(f"value must be a str, an int or a list of strings, got '{type(value)}'") + elif isinstance(value, int): + self.__grammatical_features = ['Q' + str(value)] elif isinstance(value, str): - self.__grammatical_features.append(value) + self.__grammatical_features = [value] elif isinstance(value, list): self.__grammatical_features = value else: diff --git a/wikibaseintegrator/models/language_values.py b/wikibaseintegrator/models/language_values.py index 28935e94..8858fff8 100644 --- a/wikibaseintegrator/models/language_values.py +++ b/wikibaseintegrator/models/language_values.py @@ -170,7 +170,7 @@ def get_json(self) -> dict[str, str | None]: return json_data def __contains__(self, item): - return item in self.value + return item in (self.value or '') def __eq__(self, other): if isinstance(other, LanguageValue): @@ -179,7 +179,7 @@ def __eq__(self, other): return self.value == other def __len__(self): - return len(self.value) + return len(self.value or '') def __str__(self): - return self.value + return self.value or '' diff --git a/wikibaseintegrator/models/qualifiers.py b/wikibaseintegrator/models/qualifiers.py index aa4fb1f8..892ec5aa 100644 --- a/wikibaseintegrator/models/qualifiers.py +++ b/wikibaseintegrator/models/qualifiers.py @@ -103,6 +103,15 @@ def get_json(self) -> dict[str, list]: json_data[property].append(qualifier.get_json()) return json_data + def count(self) -> int: + """ + Return the total number of individual qualifier snaks, across every property. + + Note: ``len(qualifiers)`` returns the number of distinct properties, while iterating yields the individual + snaks. Use this method when you need the qualifier count. + """ + return sum(len(snak_list) for snak_list in self.qualifiers.values()) + def __iter__(self): iterate = [] for qualifier in self.qualifiers.values(): @@ -110,4 +119,5 @@ def __iter__(self): return iter(iterate) def __len__(self): + # Returns the number of distinct properties. See count() for the total number of individual snaks. return len(self.qualifiers) diff --git a/wikibaseintegrator/models/references.py b/wikibaseintegrator/models/references.py index a566fb84..da904d4e 100644 --- a/wikibaseintegrator/models/references.py +++ b/wikibaseintegrator/models/references.py @@ -151,3 +151,10 @@ def __iter__(self): def __len__(self): return len(self.snaks) + + def __eq__(self, other): + if not isinstance(other, Reference): + return NotImplemented + + # The hash is only known server-side, so two references are considered equal when they hold the same snaks + return self.snaks.get_json() == other.snaks.get_json() diff --git a/wikibaseintegrator/models/snaks.py b/wikibaseintegrator/models/snaks.py index 8add41d5..dc6f63bb 100644 --- a/wikibaseintegrator/models/snaks.py +++ b/wikibaseintegrator/models/snaks.py @@ -147,4 +147,7 @@ def get_json(self) -> dict[str, str]: return json_data def __eq__(self, other): + if not isinstance(other, Snak): + return NotImplemented + return self.snaktype == other.snaktype and self.property_number == other.property_number and self.datatype == other.datatype and self.datavalue == other.datavalue diff --git a/wikibaseintegrator/wbi_backoff.py b/wikibaseintegrator/wbi_backoff.py index ea069c14..a9c74e37 100644 --- a/wikibaseintegrator/wbi_backoff.py +++ b/wikibaseintegrator/wbi_backoff.py @@ -16,7 +16,8 @@ def wbi_backoff_backoff_hdlr(details): exc_type, exc_value, _ = sys.exc_info() - if exc_type == JSONDecodeError: + # requests.exceptions.JSONDecodeError subclasses json.JSONDecodeError, so use issubclass to catch both. + if exc_type is not None and issubclass(exc_type, JSONDecodeError): log.error(exc_value.doc) # pragma: no cover log.error("Backing off %0.1f seconds afters %s tries calling function with args %r and kwargs %r", details['wait'], details['tries'], details['args'], details['kwargs']) diff --git a/wikibaseintegrator/wbi_config.py b/wikibaseintegrator/wbi_config.py index 481abc4a..a2633023 100644 --- a/wikibaseintegrator/wbi_config.py +++ b/wikibaseintegrator/wbi_config.py @@ -10,12 +10,18 @@ Default: 3600 (one hour) USER_AGENT: Complementary user agent string used for http requests. Both to Wikibase api, query service and others. See: https://foundation.wikimedia.org/wiki/Policy:User-Agent_policy +TIMEOUT: Timeout (in seconds) passed to every HTTP request, either a single value or a (connect, read) tuple. + Prevents a silent/unresponsive server from blocking the process indefinitely. + Set to None to disable (wait forever). Default: (5, 300) """ -config: dict[str, str | int | None | bool] = { +from typing import Any + +config: dict[str, Any] = { 'BACKOFF_MAX_TRIES': 5, 'BACKOFF_MAX_VALUE': 3600, 'USER_AGENT': None, + 'TIMEOUT': (5, 300), 'PROPERTY_CONSTRAINT_PID': 'P2302', 'DISTINCT_VALUES_CONSTRAINT_QID': 'Q21502410', 'COORDINATE_GLOBE_QID': 'http://www.wikidata.org/entity/Q2', diff --git a/wikibaseintegrator/wbi_exceptions.py b/wikibaseintegrator/wbi_exceptions.py index dd57a1f0..282fa6d3 100644 --- a/wikibaseintegrator/wbi_exceptions.py +++ b/wikibaseintegrator/wbi_exceptions.py @@ -99,3 +99,12 @@ class MissingEntityException(Exception): class SearchError(Exception): pass + + +class AnonymousEditNotAllowedError(Exception): + """ + Raised when an anonymous edit is attempted while it is not explicitly allowed. + + Set ``allow_anonymous=True`` or provide a valid ``login`` object to edit the MediaWiki instance. + """ + pass diff --git a/wikibaseintegrator/wbi_fastrun.py b/wikibaseintegrator/wbi_fastrun.py index a5221a1b..bdeed124 100644 --- a/wikibaseintegrator/wbi_fastrun.py +++ b/wikibaseintegrator/wbi_fastrun.py @@ -4,7 +4,6 @@ import copy import logging from collections import defaultdict -from functools import lru_cache from itertools import chain from typing import TYPE_CHECKING @@ -32,7 +31,7 @@ def __init__(self, base_data_type: type[BaseDataType], mediawiki_api_url: str | self.loaded_langs: dict[str, dict] = {} self.base_filter: list[BaseDataType | list[BaseDataType]] = [] self.base_filter_string = '' - self.prop_dt_map: dict[str, str] = {} + self.prop_dt_map: dict[str, str | None] = {} self.base_data_type: type[BaseDataType] = base_data_type self.mediawiki_api_url: str = str(mediawiki_api_url or config['MEDIAWIKI_API_URL']) @@ -611,15 +610,18 @@ def _process_lang(result: list) -> defaultdict[str, set]: data[qid].add(r['label']['value']) return data - @lru_cache(maxsize=100000) - def get_prop_datatype(self, prop_nr: str) -> str | None: # pylint: disable=no-self-use - from wikibaseintegrator import WikibaseIntegrator - wbi = WikibaseIntegrator() - property = wbi.property.get(prop_nr) - datatype = property.datatype - if isinstance(datatype, WikibaseDatatype): - return datatype.value - return datatype + def get_prop_datatype(self, prop_nr: str) -> str | None: + # Memoize in the per-instance prop_dt_map: this is tied to the container's lifetime (no global cache keeping + # containers alive), avoids re-instantiating WikibaseIntegrator and re-querying the API on cache hits, and is + # invalidated by clear(). + if prop_nr not in self.prop_dt_map: + from wikibaseintegrator import WikibaseIntegrator + wbi = WikibaseIntegrator() + datatype = wbi.property.get(prop_nr).datatype + if isinstance(datatype, WikibaseDatatype): + datatype = datatype.value + self.prop_dt_map[prop_nr] = datatype + return self.prop_dt_map[prop_nr] def clear(self) -> None: """ diff --git a/wikibaseintegrator/wbi_helpers.py b/wikibaseintegrator/wbi_helpers.py index 18e86f7e..52f6a31b 100644 --- a/wikibaseintegrator/wbi_helpers.py +++ b/wikibaseintegrator/wbi_helpers.py @@ -7,6 +7,7 @@ import json import logging import re +import warnings from time import sleep from typing import TYPE_CHECKING, Any from urllib.parse import urlparse @@ -17,7 +18,8 @@ from wikibaseintegrator.wbi_backoff import wbi_backoff from wikibaseintegrator.wbi_config import config -from wikibaseintegrator.wbi_exceptions import MaxRetriesReachedException, ModificationFailed, MWApiError, NonExistentEntityError, SaveFailed, SearchError +from wikibaseintegrator.wbi_exceptions import (AnonymousEditNotAllowedError, MaxRetriesReachedException, ModificationFailed, MWApiError, NonExistentEntityError, SaveFailed, + SearchError) if TYPE_CHECKING: from wikibaseintegrator.datatypes import BaseDataType @@ -32,6 +34,9 @@ class BColors: """ Default colors for pretty outputs. + + .. deprecated:: + Kept for backward compatibility only. The library no longer emits ANSI color codes in its logs. """ HEADER = '\033[95m' OKBLUE = '\033[94m' @@ -71,6 +76,10 @@ def mediawiki_api_call(method: str, mediawiki_api_url: str | None = None, sessio elif kwargs['data']['format'] != 'json': raise ValueError("'format' can only be 'json' when using mediawiki_api_call()") + # Apply a default timeout to avoid an unresponsive server blocking the process indefinitely (user can override). + if 'timeout' not in kwargs: + kwargs['timeout'] = config['TIMEOUT'] + response = None session = session if session else default_session for n in range(max_retries): @@ -146,7 +155,7 @@ def mediawiki_api_call(method: str, mediawiki_api_url: str | None = None, sessio def mediawiki_api_call_helper(data: dict[str, Any], login: _Login | None = None, mediawiki_api_url: str | None = None, user_agent: str | None = None, allow_anonymous: bool = False, - max_retries: int = 1000, retry_after: int = 60, maxlag: int = 5, is_bot: bool = False, **kwargs: Any) -> dict: + max_retries: int = 100, retry_after: int = 60, maxlag: int = 5, is_bot: bool = False, **kwargs: Any) -> dict: """ A simplified function to call the MediaWiki API. Pass the data, as a dictionary, related to the action you want to call, all commons options will be automatically managed. @@ -201,8 +210,8 @@ def mediawiki_api_call_helper(data: dict[str, Any], login: _Login | None = None, data.update({'assert': 'user'}) if 'token' in data and data['token'] == '+\\': - raise Exception("Anonymous edit are not allowed by default. " - "Set allow_anonymous to True to edit mediawiki anonymously or set the login parameter with a valid Login object.") + raise AnonymousEditNotAllowedError("Anonymous edit are not allowed by default. " + "Set allow_anonymous to True to edit mediawiki anonymously or set the login parameter with a valid Login object.") else: if 'assert' not in data and login is None: # Assert anon if allow_anonymous is True and no Login instance @@ -222,7 +231,7 @@ def mediawiki_api_call_helper(data: dict[str, Any], login: _Login | None = None, @wbi_backoff() -def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str | None = None, user_agent: str | None = None, max_retries: int = 1000, retry_after: int = 60) -> dict[ +def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str | None = None, user_agent: str | None = None, max_retries: int = 100, retry_after: int = 60) -> dict[ str, dict]: """ Static method which can be used to execute any SPARQL query @@ -252,17 +261,19 @@ def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str | 'format': 'json' } + # Send the query in the request body (application/x-www-form-urlencoded, set automatically by requests for data=). + # The previous 'multipart/form-data' Content-Type was incorrect (no multipart body was ever sent) and could be + # rejected by stricter endpoints. Using the body also avoids URL length limits with large queries. headers = { 'Accept': 'application/sparql-results+json', - 'User-Agent': get_user_agent(user_agent), - 'Content-Type': 'multipart/form-data' + 'User-Agent': get_user_agent(user_agent) } - log.debug("%s%s%s", BColors.WARNING, params['query'], BColors.ENDC) + log.debug("SPARQL query:\n%s", params['query']) for _ in range(max_retries): try: - response = helpers_session.post(sparql_endpoint_url, params=params, headers=headers) + response = helpers_session.post(sparql_endpoint_url, data=params, headers=headers, timeout=config['TIMEOUT']) except requests.exceptions.ConnectionError as e: log.exception("Connection error: %s. Sleeping for %d seconds.", e, retry_after) sleep(retry_after) @@ -282,7 +293,7 @@ def execute_sparql_query(query: str, prefix: str | None = None, endpoint: str | return results - raise Exception(f"No result after {max_retries} retries.") + raise MaxRetriesReachedException(f"No result after {max_retries} retries.") def edit_entity(data: dict, id: str | None = None, type: str | None = None, baserevid: int | None = None, summary: str | None = None, clear: bool = False, is_bot: bool = False, @@ -327,7 +338,8 @@ def edit_entity(data: dict, id: str | None = None, type: str | None = None, base 'title': title }) else: - assert type + if not type: + raise ValueError("The 'type' parameter is mandatory when creating a new entity (no id, site or title given).") params.update({'new': type}) if clear: @@ -432,7 +444,8 @@ def search_entities(search_string: str, language: str | None = None, strict_lang You can see the list of languages for Wikidata at https://www.wikidata.org/wiki/Help:Wikimedia_language_codes/lists/all (Use the WMF code) :param strict_language: Whether to disable language fallback. Default is 'False'. :param search_type: Search for this type of entity. One of the following values: form, item, lexeme, property, sense, mediainfo - :param max_results: The maximum number of search results returned. The value must be between 0 and 50. Default is 50 + :param max_results: The maximum number of search results returned. Default is 50. A single API call is limited to 50 + results; higher values trigger additional paginated calls and the returned list is truncated to this length. :param dict_result: Return the results as a detailed dictionary instead of a list of IDs. :param allow_anonymous: Allow anonymous interaction with the MediaWiki API. 'True' by default. """ @@ -444,12 +457,12 @@ def search_entities(search_string: str, language: str | None = None, strict_lang 'search': search_string, 'language': language, 'type': search_type, - 'limit': 50, + 'limit': min(max_results, 50), 'format': 'json' } if strict_language: - params.update({'strict_language': ''}) + params.update({'strictlanguage': ''}) cont_count = 0 results = [] @@ -476,15 +489,14 @@ def search_entities(search_string: str, language: str | None = None, strict_lang else: results.append(i['id']) - if 'search-continue' not in search_results: + # Stop once there is no more page or we gathered enough results + if 'search-continue' not in search_results or len(results) >= max_results: break cont_count = search_results['search-continue'] - if cont_count >= max_results: - break - - return results + # A page holds up to 50 results, so the last page can overshoot max_results: truncate to the requested length + return results[:max_results] def lexeme_add_form(lexeme_id, data, baserevid: int | None = None, tags: list[str] | None = None, is_bot: bool = False, **kwargs: Any) -> dict: @@ -657,7 +669,7 @@ def lexeme_edit_sense(sense_id: str, data, baserevid: int | None = None, tags: l params = { 'action': 'wbleditsenseelements', - 'formId': sense_id, + 'senseId': sense_id, 'data': ujson.dumps(data), 'format': 'json' } @@ -676,7 +688,7 @@ def lexeme_edit_sense(sense_id: str, data, baserevid: int | None = None, tags: l def lexeme_remove_sense(sense_id: str, baserevid: int | None = None, tags: list[str] | None = None, is_bot: bool = False, **kwargs: Any) -> dict: """ - Adds Form to Lexeme + Removes Sense from Lexeme :param sense_id: ID of the Sense, e.g. L10-S20 :param baserevid: Base Revision ID of the Lexeme, if edit conflict check is wanted. @@ -742,7 +754,9 @@ def generate_entity_instances(entities: str | list[str], allow_anonymous: bool = from wikibaseintegrator import WikibaseIntegrator for qid, v in reply['entities'].items(): wbi = WikibaseIntegrator(is_bot=kwargs.get('is_bot', False), login=kwargs.get('login', None)) - f = [x for x in BaseEntity.__subclasses__() if x.ETYPE == v['type']][0] + # Use the recursive subclass registry (not __subclasses__(), which only returns direct subclasses) so that + # entities inheriting through an intermediate base (Item/Property/MediaInfo via TermsEntity) are found. + f = [x for x in BaseEntity.subclasses if x.ETYPE == v['type']][0] ii = f(api=wbi).from_json(v) entity_instances.append((qid, ii)) @@ -861,6 +875,16 @@ def get_user_agent(user_agent: str | None = None) -> str: def format2wbi(entitytype: str, json_raw: str, allow_anonymous: bool = True, wikibase_url: str | None = None, **kwargs) -> BaseEntity: + """ + Build a WikibaseIntegrator entity from a raw Wikibase JSON string. + + .. warning:: + **Experimental.** This function (and its helper :func:`_json2datatype`) is incomplete (references are not + attached, several data types are unsupported, it relies on a mutable module-level cache and has no test + coverage). Its API and behaviour may change or be removed without notice. + """ + warnings.warn("format2wbi() is experimental and may change or be removed without notice.", stacklevel=2) + wikibase_url = str(wikibase_url or config['WIKIBASE_URL']) json_decoded = json.loads(json_raw) # pprint(json_decoded) @@ -945,6 +969,9 @@ def format2wbi(entitytype: str, json_raw: str, allow_anonymous: bool = True, wik def _json2datatype(prop_nr: str, statement: dict, wikibase_url: str | None = None, allow_anonymous=True, **kwargs) -> BaseDataType: + """ + Experimental helper for :func:`format2wbi`. See its docstring for caveats. May change or be removed without notice. + """ from wikibaseintegrator.datatypes.basedatatype import BaseDataType wikibase_url = str(wikibase_url or config['WIKIBASE_URL']) @@ -1022,7 +1049,7 @@ def download_entity_ttl(entity: str, wikibase_url: str | None = None, user_agent 'User-Agent': get_user_agent(user_agent) } - response = helpers_session.get(wikibase_url + '/entity/' + entity + '.ttl', headers=headers) + response = helpers_session.get(wikibase_url + '/entity/' + entity + '.ttl', headers=headers, timeout=config['TIMEOUT']) response.raise_for_status() results = response.text diff --git a/wikibaseintegrator/wbi_login.py b/wikibaseintegrator/wbi_login.py index 58ae680b..1a109f3f 100644 --- a/wikibaseintegrator/wbi_login.py +++ b/wikibaseintegrator/wbi_login.py @@ -4,7 +4,7 @@ import logging import time import webbrowser -from typing import Any +from typing import Any, cast from mwoauth import ConsumerToken, Handshaker, OAuthException from oauthlib.oauth2 import BackendApplicationClient, InvalidClientError @@ -61,7 +61,7 @@ def generate_edit_credentials(self) -> RequestsCookieJar: 'type': 'csrf', 'format': 'json' } - response = self.session.get(url=self.mediawiki_api_url, params=params).json() + response = self.session.get(url=self.mediawiki_api_url, params=params, timeout=config['TIMEOUT']).json() if 'error' in response: raise LoginError(f"Login failed ({response['error']['code']}). Message: '{response['error']['info']}'") if response['query']['tokens']['csrftoken'] == '+\\': @@ -120,16 +120,33 @@ def __init__(self, consumer_token: str | None = None, consumer_secret: str | Non mediawiki_rest_url = str(mediawiki_rest_url or config['MEDIAWIKI_REST_URL']) - headers = { - 'User-Agent': get_user_agent(user_agent or (str(config['USER_AGENT']) if config['USER_AGENT'] is not None else None)) - } + self.consumer_token = consumer_token + self.consumer_secret = consumer_secret + self.access_token_url = mediawiki_rest_url + '/oauth2/access_token' session = OAuth2Session(client=BackendApplicationClient(client_id=consumer_token)) + # The access token is fetched (and later refreshed) by generate_edit_credentials(), invoked by the parent __init__. + super().__init__(session=session, token_renew_period=token_renew_period, user_agent=user_agent, mediawiki_api_url=mediawiki_api_url) + + def _fetch_access_token(self) -> None: + """ + (Re)fetch the OAuth2 access token. + + The client-credentials grant used here does not issue a refresh token, so the short-lived access token + (~4h on Wikimedia) is simply re-fetched. Called on every credentials renewal to keep long-running bots alive. + """ + headers = {'User-Agent': self.session.headers.get('User-Agent', get_user_agent())} try: - session.fetch_token(token_url=mediawiki_rest_url + '/oauth2/access_token', client_id=consumer_token, client_secret=consumer_secret, headers=headers) + cast(OAuth2Session, self.session).fetch_token(token_url=self.access_token_url, client_id=self.consumer_token, client_secret=self.consumer_secret, headers=headers, + timeout=config['TIMEOUT']) except InvalidClientError as err: raise LoginError(err) from err - super().__init__(session=session, token_renew_period=token_renew_period, user_agent=user_agent, mediawiki_api_url=mediawiki_api_url) + + def generate_edit_credentials(self) -> RequestsCookieJar: + # Refresh the access token before requesting the CSRF token: the parent renews credentials every + # token_renew_period seconds, so this keeps the OAuth2 session authenticated past the token expiry. + self._fetch_access_token() + return super().generate_edit_credentials() class OAuth1(_Login): @@ -234,6 +251,7 @@ def __init__(self, user: str | None = None, password: str | None = None, mediawi filtered_kwargs = {key: value for key, value in kwargs.items() if key in allowed_kwargs} if len(filtered_kwargs) < len(kwargs): log.warning("Unsupported kwargs were ignored: %s", set(kwargs) - allowed_kwargs) + filtered_kwargs.setdefault('timeout', config['TIMEOUT']) # get login token login_token = session.post(mediawiki_api_url, data=params_login, headers=headers, **filtered_kwargs).json()['query']['tokens']['logintoken'] @@ -249,8 +267,10 @@ def __init__(self, user: str | None = None, password: str | None = None, mediawi if 'login' in login_result and login_result['login']['result'] == 'Success': log.info("Successfully logged in as %s", login_result['login']['lgusername']) + elif 'login' in login_result: + raise LoginError(f"Login failed. Reason: '{login_result['login'].get('reason', login_result['login']['result'])}'") else: - raise LoginError(f"Login failed. Reason: '{login_result['login']['reason']}'") + raise LoginError(f"Login failed. Unexpected API response: {login_result.get('error', login_result)}") if 'warnings' in login_result: log.warning("MediaWiki login warnings messages:") @@ -293,6 +313,7 @@ def __init__(self, user: str | None = None, password: str | None = None, mediawi filtered_kwargs = {key: value for key, value in kwargs.items() if key in allowed_kwargs} if len(filtered_kwargs) < len(kwargs): log.warning("Unsupported kwargs were ignored: %s", set(kwargs) - allowed_kwargs) + filtered_kwargs.setdefault('timeout', config['TIMEOUT']) # get login token login_token = session.post(mediawiki_api_url, data=params_login, headers=headers, **filtered_kwargs).json()['query']['tokens']['logintoken'] @@ -317,8 +338,10 @@ def __init__(self, user: str | None = None, password: str | None = None, mediawi raise LoginError(f"Login failed ({clientlogin['messagecode']}). Message: '{clientlogin['message']}'") log.info("Successfully logged in as %s", clientlogin['username']) - else: + elif 'error' in login_result: raise LoginError(f"Login failed ({login_result['error']['code']}). Message: '{login_result['error']['info']}'") + else: + raise LoginError(f"Login failed. Unexpected API response: {login_result}") if 'warnings' in login_result: log.warning("MediaWiki login warnings messages:")