Fix bugs and refactor entity architecture

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)
 

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

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')

test/integration/docker-compose.yml

@@ -1,28 +1,46 @@
 # 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:
       MW_ADMIN_NAME: WikibaseAdmin
       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:

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')
 
 

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):

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):