Update import API to use key/remote_ids instead of ol_id/identifiers to match type schema

Author: cdrini

openlibrary/catalog/add_book/load_book.py

@@ -161,20 +161,16 @@ def get_redirected_authors(authors: list["Author"]):
         return authors
 
     # Look for OL ID first.
-    if (key := author.get("ol_id")) and (
-        reply := list(
-            web.ctx.site.things({"type": "/type/author", "key~": f'/authors/{key}'})
-        )
-    ):
+    if (key := author.get("key")) and (record := web.ctx.site.get(key)):
         # Always match on OL ID, even if remote identifiers don't match.
-        return get_redirected_authors(list(web.ctx.site.get_many(reply)))
+        return get_redirected_authors([record])
 
     # Try other identifiers next.
-    if identifiers := author.get("identifiers"):
+    if remote_ids := author.get("remote_ids"):
         queries = []
         matched_authors = []
         # Get all the authors that match any incoming identifier.
-        for identifier, val in identifiers.items():
+        for identifier, val in remote_ids.items():
             queries.append({"type": "/type/author", f"remote_ids.{identifier}": val})
         for query in queries:
             if reply := list(web.ctx.site.things(query)):
@@ -186,7 +182,7 @@ def get_redirected_authors(authors: list["Author"]):
         highest_matches = 0
         selected_match = None
         for a in matched_authors:
-            _, matches = a.merge_remote_ids(identifiers)
+            _, matches = a.merge_remote_ids(remote_ids)
             if matches > highest_matches:
                 selected_match = a
                 highest_matches = matches
@@ -253,9 +249,9 @@ def find_entity(author: dict[str, Any]) -> "Author | None":
     """
     assert isinstance(author, dict)
     things = find_author(author)
-    if "identifiers" in author:
+    if "remote_ids" in author:
         for index, t in enumerate(things):
-            t.remote_ids, _ = t.merge_remote_ids(author["identifiers"])
+            t.remote_ids, _ = t.merge_remote_ids(author["remote_ids"])
             things[index] = t
     return things[0] if things else None
 
@@ -316,9 +312,6 @@ def import_author(author: dict[str, Any], eastern=False) -> "Author | dict[str,
     ):
         if f in author:
             a[f] = author[f]
-    # Import record hitting endpoint should list external IDs under "identifiers", but needs to be "remote_ids" when going into the DB.
-    if "identifiers" in author:
-        a["remote_ids"] = author["identifiers"]
     return a
 
 

openlibrary/catalog/add_book/tests/test_load_book.py

@@ -165,8 +165,8 @@ def test_first_match_ol_key(self, mock_site):
         # We ideally shouldn't ever have a case where different authors have the same VIAF, but this demonstrates priority.
         searched_author = {
             "name": "William H. Brewer",
-            "ol_id": "OL4A",
-            "identifiers": {"viaf": "12345678"},
+            "key": "/authors/OL4A",
+            "remote_ids": {"viaf": "12345678"},
         }
         found = import_author(searched_author)
         assert found.key == author_different_key["key"]
@@ -198,7 +198,7 @@ def test_second_match_remote_identifier(self, mock_site):
         # Look for exact match on VIAF, regardless of name field.
         searched_author = {
             "name": "William Brewer",
-            "identifiers": {"viaf": "12345678"},
+            "remote_ids": {"viaf": "12345678"},
         }
         found = import_author(searched_author)
         assert found.key == author["key"]

scripts/providers/import_wikisource.py

@@ -221,8 +221,8 @@ def format_contributor(raw_name: str) -> str:
 @dataclass
 class Author:
     friendly_name: str | None = None
-    ol_id: str | None = None
-    identifiers: dict[str, str] = field(default_factory=dict[str, str])
+    key: str | None = None
+    remote_ids: dict[str, str] = field(default_factory=dict[str, str])
     birth_date: str | None = None
     death_date: str | None = None
 
@@ -307,14 +307,8 @@ def to_dict(self):
                     "name": author.friendly_name,
                     **({"birth_date": author.birth_date} if author.birth_date else {}),
                     **({"death_date": author.death_date} if author.death_date else {}),
-                    **(
-                        {
-                            "identifiers": author.identifiers,
-                        }
-                        if author.identifiers
-                        else {}
-                    ),
-                    **({"ol_id": author.ol_id} if author.ol_id else {}),
+                    **({"remote_ids": author.remote_ids} if author.remote_ids else {}),
+                    **({"key": author.key} if author.key else {}),
                 }
                 for author in self.authors
             ]
@@ -829,7 +823,7 @@ def fix_contributor_data(
                 contributor.death_date = extract_year(obj["deathDate"]["value"])
 
             if "olId" in obj and "value" in obj["olId"]:
-                contributor.ol_id = obj["olId"]["value"]
+                contributor.key = f"/authors/{obj["olId"]["value"]}"
 
             # Couldn't find inventaire
             for id in [
@@ -852,7 +846,7 @@ def fix_contributor_data(
                     val = obj[id]["value"]
                     if id == "youtube" and val[0] != "@":
                         val = f'@{val}'
-                    contributor.identifiers[id] = val
+                    contributor.remote_ids[id] = val
 
             if contributor_id in map:
                 book_ids = map[contributor_id]