fix(i18n): normalize language codes before lookup, guard unknown locale

Both from the cross-family review of this PR.

Canonicalization ran *after* the locale-table lookup, so the two passes are
collapsed into one that normalizes /B -> /T before anything is looked up.
Previously a locale table that ever gained a 'ger' key would capture the code
in the first pass and store it verbatim -- a code the rest of the stack cannot
render -- and would stop de-duplicating a book that declares both 'ger' and
'deu'. The shipped tables are /T-only so this was latent, not live; the point
is not to depend on that holding forever.

Collapsing the passes surfaced that names.items() on an unrecognised locale
had always raised AttributeError, even though get_language_names() is
documented to return None for one and get_language_name() already guards it.
The validator guards it too now and falls through to the reference table, so
the book imports instead of 500-ing the request.

Tests, also from the review:

- edit_book_languages(upload_mode=True) is now under test. It is the only
  production caller and the place remainder becomes the reporter's ValueError,
  and nothing exercised it -- every helper test could stay green while a
  refactor stopped passing upload_mode or mishandled the remainder loop.
- Replaced the canonical_lang_code monkeypatch test, which only proved the
  helper was called and would pass if it mapped everything to 'eng', with a
  public-boundary test that pins the ordering invariant above.
- test_alias_targets_resolve_through_the_language_backend claimed to run on
  either backend but asserted pycountry's specific AttributeError; it now
  asserts the semantic condition and tolerates the backend's miss form.
- Added test_reference_table_is_the_superset_of_every_locale, the property
  that makes the reference table a valid oracle, and reworded its docstring,
  which implied an ISO registry rather than generated application data.

Kept the three _resolve_lang_code empty-entry tests the review called
opportunistic: they pin a contract the docstring now states, and a documented
invariant with no test is the drift risk.

Author: new-usemame

cps/isoLanguages.py

@@ -115,7 +115,11 @@ def canonical_lang_code(lang_code):
 
 
 def _reference_language_codes():
-    """Every language code the app knows, independent of UI locale.
+    """The complete checked-in reference table, independent of UI locale.
+
+    This is the application's own language data, not an ISO registry: it is
+    the one locale table that is complete, and ``test_reference_table_is_the
+    _superset_of_every_locale`` pins that it stays a superset of every other.
 
     The per-locale tables in LANGUAGE_NAMES are *translation* data and are
     legitimately incomplete — 27 of the 28 shipped locales are missing between
@@ -164,23 +168,28 @@ def get_valid_language_codes_from_code(locale, language_names, remainder=None):
     lang = list()
     if "" in language_names:
         language_names.remove("")
-    names = get_language_names(locale)
-    for k, __ in names.items():
-        if k in language_names:
-            lang.append(k)
-            language_names.remove(k)
-    # Second pass over whatever the locale table did not claim. Validity is a
-    # property of the code, not of the current locale's translation coverage,
-    # so this pass checks the reference table — which both accepts /B input by
-    # normalizing it to /T ('ger' from an OPF is a valid language, and 'deu' is
-    # the form the rest of the stack can render), and stops a valid /T code
-    # being refused just because the user's locale has no name for it.
-    # Without this, upload rejects the book with "'ger' is not a valid
-    # language".
+    names = get_language_names(locale) or {}
+    # An unrecognised locale used to crash here (AttributeError on None.items),
+    # even though get_language_names is explicitly documented to return None
+    # for one and get_language_name already guards it. Validity does not depend
+    # on the locale table anyway, so fall through to the reference and let the
+    # book import rather than 500 the request.
     reference = _reference_language_codes()
+    # Normalize /B -> /T *before* looking anything up. Doing the locale-table
+    # lookup first and only aliasing the leftovers would let a locale table
+    # that ever gained a /B key capture 'ger' unchanged — storing a code the
+    # rest of the stack cannot render, and defeating the de-duplication of a
+    # book that declares both 'ger' and 'deu'. The current tables are /T-only,
+    # so this is about not depending on that holding forever.
     for code in list(language_names):
         canonical = canonical_lang_code(code)
-        if canonical in reference:
+        # Validity is a property of the code, not of the current locale's
+        # translation coverage, so the reference table is what decides it —
+        # otherwise a valid /T code is refused just because the user's locale
+        # has no name for it, and upload rejects the book with "'ell' is not
+        # a valid language". The locale table is still consulted so a code it
+        # somehow carries that the reference lacks stays accepted.
+        if canonical in names or canonical in reference:
             if canonical not in lang:
                 lang.append(canonical)
             language_names.remove(code)

tests/unit/test_1109_iso6392b_language_codes.py

@@ -55,13 +55,24 @@ def test_alias_targets_resolve_through_the_language_backend():
     This one runs on either backend: every /T target must resolve through the
     module's own ``get()``, and no /B code may — if a /B code resolved
     directly, it would not need aliasing and the table entry would be wrong.
+
+    The *miss* is asserted semantically rather than as a specific exception.
+    pycountry's wrapper happens to raise AttributeError (it calls
+    _copy_fields(None)), but iso-639 is free to return None or raise
+    KeyError, and pinning one backend's failure mode would make this the
+    pycountry-only test it claims not to be.
     """
+    def resolves(code):
+        try:
+            return bool(getattr(isoLanguages.get(part3=code), "name", None))
+        except Exception:
+            return False
+
     for b_code, t_code in isoLanguages.ISO6392B_TO_T.items():
-        record = isoLanguages.get(part3=t_code)
-        assert getattr(record, "name", None), \
+        assert resolves(t_code), \
             f"/T target {t_code!r} does not resolve through the backend"
-        with pytest.raises(AttributeError):
-            isoLanguages.get(part3=b_code)
+        assert not resolves(b_code), \
+            f"/B code {b_code!r} resolves directly, so it does not need aliasing"
 
 
 def test_iso6392b_table_is_not_self_mapping():
@@ -258,20 +269,26 @@ def test_canonical_lang_code_maps_bibliographic():
     assert isoLanguages.canonical_lang_code("ger") == "deu"
 
 
-def test_validator_normalizes_through_canonical_lang_code(monkeypatch):
-    """The normalizer must be the one the validator actually uses.
+def test_normalization_precedes_the_locale_table_lookup(monkeypatch):
+    """A /B key in a locale table must not capture the code before aliasing.
 
-    A public helper that duplicates a lookup the production path does inline
-    is dead code that can silently drift from it. Patching the helper must
-    therefore change what the validator stores.
+    The shipped tables are /T-only, so this is about not silently depending on
+    that. If the locale lookup ran first and only the leftovers were aliased,
+    a table that ever gained a "ger" key would store 'ger' verbatim — a code
+    the rest of the stack cannot render — and a book declaring both forms
+    would store two rows for one language instead of de-duplicating.
     """
     monkeypatch.setattr(
-        isoLanguages, "canonical_lang_code", lambda code: "eng"
+        isoLanguages, "get_language_names",
+        lambda locale: {"ger": "German (B)", "deu": "German (T)"},
     )
-    remainder = []
-    out = isoLanguages.get_valid_language_codes_from_code("en", ["ger"], remainder)
-    assert out == ["eng"], "validator does not route through canonical_lang_code"
-    assert remainder == []
+    for order in (["ger", "deu"], ["deu", "ger"], ["ger"]):
+        remainder = []
+        out = isoLanguages.get_valid_language_codes_from_code(
+            "en", list(order), remainder
+        )
+        assert out == ["deu"], f"{order} stored {out!r}, expected ['deu']"
+        assert remainder == []
 
 
 # --------------------------------------------------------------------------
@@ -298,3 +315,106 @@ def test_present_and_non_empty_entry_resolves():
 def test_canonical_lang_code_passes_through_everything_else():
     for code in ("deu", "eng", "", "totallymadeupcode"):
         assert isoLanguages.canonical_lang_code(code) == code
+
+
+# --------------------------------------------------------------------------
+# The invariant that makes _reference_language_codes() a valid oracle.
+# --------------------------------------------------------------------------
+
+def test_reference_table_is_the_superset_of_every_locale():
+    """Validation resolves against the reference table, so it must not be
+    possible for a locale to know a code the reference does not.
+
+    If a future translation adds a code to, say, `de` but not `en`, the
+    reference stops being an oracle and this fails rather than silently
+    narrowing what uploads accept.
+    """
+    from cps.iso_language_names import LANGUAGE_NAMES
+
+    reference = set(isoLanguages._reference_language_codes())
+    for locale, table in LANGUAGE_NAMES.items():
+        extra = set(table) - reference
+        assert not extra, \
+            f"locale {locale!r} carries codes absent from the reference: {sorted(extra)}"
+
+
+# --------------------------------------------------------------------------
+# The real caller. get_valid_language_codes_from_code() is only reachable in
+# production through edit_book_languages(upload_mode=True), which is where
+# remainder becomes the ValueError the reporter saw. Pinning the helper alone
+# would let a refactor stop passing upload_mode, swap to the name parser, or
+# mishandle the remainder loop with every helper test still green.
+# --------------------------------------------------------------------------
+
+def _import_editbooks():
+    import cps.editbooks as editbooks
+    return editbooks
+
+
+def _upload_languages(locale, languages):
+    """Drive edit_book_languages(upload_mode=True); return stored lang codes.
+
+    Everything past the validation branch (the filter-language fixup, the
+    session write) is mocked out — this pins the accept/reject boundary, not
+    the DB layer.
+    """
+    from types import SimpleNamespace
+    from unittest.mock import MagicMock, patch
+
+    editbooks = _import_editbooks()
+    book = SimpleNamespace(id=1, languages=[])
+    captured = {}
+
+    def fake_modify(input_l, db_field, db_type, session, db_name):
+        captured["langs"] = list(input_l)
+        return True
+
+    user = MagicMock()
+    user.filter_language.return_value = "all"
+
+    with patch.object(editbooks, "get_locale", lambda: locale), \
+            patch.object(editbooks, "current_user", user), \
+            patch.object(editbooks, "calibre_db", MagicMock()), \
+            patch.object(editbooks, "modify_database_object", fake_modify), \
+            patch.object(editbooks, "log", MagicMock()):
+        editbooks.edit_book_languages(languages, book, upload_mode=True)
+    return captured.get("langs", [])
+
+
+def test_upload_accepts_greek_under_a_locale_missing_the_name():
+    """#1109's upload half, at the boundary that actually raised.
+
+    'ell' is absent from the pt_BR name table, so validating against that
+    table dropped it into remainder and edit_book_languages turned it into
+    ValueError("'ell' is not a valid language") — a Greek book refused for a
+    Brazilian-Portuguese user and imported fine for an English one.
+    """
+    assert _upload_languages("pt_BR", "ell") == ["ell"]
+
+
+def test_upload_accepts_bibliographic_code_and_stores_terminological():
+    assert _upload_languages("en", "ger") == ["deu"]
+
+
+def test_upload_dedupes_a_book_declaring_both_forms():
+    assert _upload_languages("en", "ger,deu") == ["deu"]
+
+
+def test_upload_still_rejects_a_genuinely_invalid_code():
+    """The permissive change must not turn the validator into a rubber stamp."""
+    with pytest.raises(ValueError):
+        _upload_languages("pt_BR", "zzz")
+
+
+def test_unknown_locale_does_not_crash_the_validator():
+    """get_language_names() is documented to return None for an unrecognised
+    locale, and get_language_name() has always guarded that. The validator did
+    not, so the same input raised AttributeError on None.items() and 500'd the
+    request. Validity does not depend on the locale table, so the book imports.
+    """
+    remainder = []
+    out = isoLanguages.get_valid_language_codes_from_code(
+        "xx_YY", ["ger", "eng", "zzz"], remainder
+    )
+    assert out == ["deu", "eng"]
+    assert remainder == ["zzz"]