Merge remote-tracking branch 'origin/19.0' into fix/941-program-config-ux-polish

# Conflicts:
#	spp_programs/__manifest__.py
#	spp_programs/tests/__init__.py

Author: emjay0921

.ruff.toml

@@ -19,6 +19,9 @@ exclude = ["setup/*", "fastapi/*", "base_user_role/*", "endpoint_route_handler/*
 [lint.per-file-ignores]
 "__init__.py" = ["F401", "I001"]  # ignore unused and unsorted imports in __init__.py
 "__manifest__.py" = ["B018", "E501"]  # allow long lines and useless expression in manifests
+# Schema definitions hold SPDCI-spec-verbatim description strings; reflowing them
+# would hurt fidelity to the standard, so allow long lines in this file only.
+"spp_dci_compliance/schemas/spdci_schemas.py" = ["E501"]
 
 [lint.isort]
 section-order = ["future", "standard-library", "third-party", "odoo", "odoo-addons", "first-party", "local-folder"]

requirements.txt

@@ -8,9 +8,9 @@ faker
 fastapi>=0.110.0
 geojson
 httpx
+jsonschema
 jwcrypto
 jwcrypto>=1.5.6
-libhxl
 numpy>=1.22.2
 openpyxl
 parse-accept-language

scripts/coverage_single_module.sh

@@ -0,0 +1,63 @@
+#!/usr/bin/env bash
+# Run coverage for a single OpenSPP module against the docker stack.
+# Mirrors the CI recipe in .github/workflows/ci.yml so local numbers
+# match what gets reported in CI.
+#
+# Usage: ./scripts/coverage_single_module.sh <module_name>
+
+set -e
+
+MODULE_NAME="${1:?usage: $0 <module_name>}"
+PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+cd "$PROJECT_ROOT"
+
+DB_NAME="cov_${MODULE_NAME}_$$"
+COV_FILE="/tmp/.coverage.${MODULE_NAME}.$$"
+LOG_FILE="/tmp/openspp-test-logs/${MODULE_NAME}_coverage_$(date +%Y%m%d_%H%M%S).log"
+mkdir -p "$(dirname "$LOG_FILE")"
+
+# Ensure DB container is up.
+if ! docker compose ps db --status running --quiet >/dev/null 2>&1; then
+    docker compose up -d db >/dev/null 2>&1
+    until docker compose exec -T db pg_isready -U odoo -d postgres >/dev/null 2>&1; do
+        sleep 1
+    done
+fi
+
+# Fresh database per run.
+docker compose exec -T db psql -U odoo -d postgres -c "DROP DATABASE IF EXISTS $DB_NAME;" >/dev/null 2>&1
+docker compose exec -T db psql -U odoo -d postgres -c "CREATE DATABASE $DB_NAME;" >/dev/null 2>&1
+
+echo "Running coverage for $MODULE_NAME (log: $LOG_FILE)..."
+
+set +e
+docker compose run --rm \
+    -e DB_NAME="$DB_NAME" \
+    -e COVERAGE_FILE="$COV_FILE" \
+    --entrypoint "" \
+    test \
+    bash -c "
+        coverage run --branch --source=/mnt/extra-addons/openspp/${MODULE_NAME} \
+            /opt/odoo/odoo/odoo-bin \
+            --addons-path=/opt/odoo/odoo/addons,/opt/odoo/odoo/odoo/addons,/mnt/extra-addons/openspp,/mnt/extra-addons/server-ux,/mnt/extra-addons/server-tools,/mnt/extra-addons/odoo-job-worker,/mnt/extra-addons/server-backend,/mnt/extra-addons/rest-framework,/mnt/extra-addons/muk-it \
+            -d $DB_NAME \
+            --db_host=db --db_port=5432 --db_user=odoo --db_password=odoo \
+            --stop-after-init --no-http \
+            --data-dir /tmp/odoo-data \
+            -i ${MODULE_NAME} \
+            --test-tags /${MODULE_NAME} \
+            --log-level=test 2>&1 | tee /tmp/test_output_${MODULE_NAME}.log
+        echo '===COVERAGE REPORT==='
+        coverage report --skip-empty --skip-covered --omit='*/tests/*,*/migrations/*' || true
+        echo '===COVERAGE TOTAL==='
+        coverage report --omit='*/tests/*,*/migrations/*' | tail -2 || true
+    " > "$LOG_FILE" 2>&1
+EXIT=$?
+set -e
+
+docker compose exec -T db psql -U odoo -d postgres -c "DROP DATABASE IF EXISTS $DB_NAME;" >/dev/null 2>&1
+
+echo "=== $MODULE_NAME coverage ==="
+sed -n '/===COVERAGE TOTAL===/,$p' "$LOG_FILE" | tail -3
+echo "(full log: $LOG_FILE)"
+exit $EXIT

scripts/detect_modules.py

@@ -151,6 +151,7 @@ def main():
     # Build dependency graph
     reverse_deps = build_reverse_dependency_graph(project_root, all_modules)
 
+    changed_modules: set[str] = set()
     if args.all:
         # List all modules with tests (no limit for --all)
         modules_to_test = {m for m in all_modules if has_tests(project_root / m)}
@@ -184,10 +185,17 @@ def main():
 
     # Apply max limit if set
     if args.max_modules > 0 and len(sorted_modules) > args.max_modules:
-        # Prioritize critical modules
-        critical_first = [m for m in sorted_modules if m in CRITICAL_MODULES]
-        others = [m for m in sorted_modules if m not in CRITICAL_MODULES]
-        sorted_modules = (critical_first + others)[: args.max_modules]
+        # Priority order when the matrix is over-subscribed:
+        #   1. Critical modules (always tested).
+        #   2. Directly-changed modules (a patch must run its own tests so
+        #      codecov/patch reflects the new coverage — without this, the
+        #      changed module can fall off the alpha tail and codecov
+        #      carryforward leaves the old figures in place).
+        #   3. Transitively-impacted modules, alphabetical.
+        critical = [m for m in sorted_modules if m in CRITICAL_MODULES]
+        directly_changed = [m for m in sorted_modules if m in changed_modules and m not in CRITICAL_MODULES]
+        others = [m for m in sorted_modules if m not in CRITICAL_MODULES and m not in changed_modules]
+        sorted_modules = (critical + directly_changed + others)[: args.max_modules]
         print(
             f"Warning: Limited to {args.max_modules} modules",
             file=sys.stderr,

spp

@@ -284,6 +284,36 @@ def _find_running_odoo() -> tuple:
     return None, None
 
 
+def _is_jobworker_running(profile: str) -> bool:
+    """Return True when the jobworker container is up under this profile.
+
+    `./spp restart` only restarts the Odoo service by default, but module
+    code that runs inside delayed jobs (async batch scoring, eligibility
+    checks, anything routed through job_worker.delay) lives in the
+    jobworker process. Without restarting the worker too, the developer
+    sees stale Python after Apps -> Upgrade. See OP#986.
+    """
+    result = run(
+        docker_compose("ps", "--format", "json", profile=profile),
+        capture=True,
+        check=False,
+    )
+    if result.returncode != 0:
+        return False
+
+    for line in result.stdout.strip().split("\n"):
+        if not line:
+            continue
+        try:
+            container = json.loads(line)
+            if container.get("Service") == "jobworker" and container.get("State") == "running":
+                return True
+        except json.JSONDecodeError:
+            continue
+
+    return False
+
+
 # ============================================================================
 # Commands
 # ============================================================================
@@ -697,8 +727,12 @@ def cmd_restart(args):
         print("Error: No Odoo container running. Start with './spp start' first.")
         sys.exit(1)
 
-    print(f"Restarting {service}...")
-    run(docker_compose("restart", service, profile=profile))
+    services_to_restart = [service]
+    if _is_jobworker_running(profile):
+        services_to_restart.append("jobworker")
+
+    print(f"Restarting {', '.join(services_to_restart)}...")
+    run(docker_compose("restart", *services_to_restart, profile=profile))
     print("Done.")
 
     _show_url()

spp_api_v2/README.rst

@@ -147,6 +147,21 @@ Dependencies
 Changelog
 =========
 
+19.0.2.0.1
+~~~~~~~~~~
+
+- Fix ``SerializationFailure`` race when multiple Odoo workers rebuild
+  their routing map simultaneously (e.g. after ``-u all``) and all try
+  to sync the same ``fastapi.endpoint`` rows
+- Serialize concurrent sync attempts across workers using a
+  transaction-scoped Postgres advisory lock; workers that don't acquire
+  the lock skip the sync and pick up the freshly synced routes on the
+  next routing-map rebuild (via ``endpoint_route_version`` cache
+  invalidation)
+- Log skipped syncs at INFO and lock-primitive failures at WARNING so
+  cold-start route-availability symptoms and broken-primitive
+  regressions are diagnosable without raising the global log level
+
 19.0.2.0.0
 ~~~~~~~~~~
 

spp_api_v2/__manifest__.py

@@ -1,7 +1,7 @@
 {
     "name": "OpenSPP API V2",
     "category": "OpenSPP/Integration",
-    "version": "19.0.2.0.0",
+    "version": "19.0.2.0.1",
     "sequence": 1,
     "author": "OpenSPP.org",
     "website": "https://github.com/OpenSPP/OpenSPP2",

spp_api_v2/models/ir_http_patch.py

@@ -8,6 +8,7 @@
 This workaround should be removed when Odoo core fixes the cache bug.
 """
 
+import hashlib
 import logging
 import threading
 
@@ -22,6 +23,46 @@
 
 _logger = logging.getLogger(__name__)
 
+# Stable 64-bit signed key for the transaction-scoped advisory lock that
+# serializes FastAPI endpoint sync attempts across workers. Derived from a
+# SHA-256 of the qualified name so it is deterministic and unlikely to collide
+# with other modules' advisory locks in the same database.
+_FASTAPI_SYNC_ADVISORY_LOCK_KEY = int.from_bytes(
+    hashlib.sha256(b"spp_api_v2.fastapi_endpoint_sync").digest()[:8],
+    byteorder="big",
+    signed=True,
+)
+
+
+def _try_acquire_fastapi_sync_lock(cr):
+    """Try to acquire the cross-worker FastAPI endpoint sync advisory lock.
+
+    Returns True if this transaction got the lock, False otherwise (either
+    because another backend holds it, or because the lock SQL itself failed —
+    e.g. exhausted shared-lock memory, permissions). The lock is transaction-
+    scoped (released automatically at COMMIT/ROLLBACK), so callers do not need
+    to release it explicitly.
+
+    Failing closed (returning False) is the safe default: callers will skip
+    the sync, and the next routing_map call will retry. Logging at WARNING so
+    a persistently broken lock primitive is visible — silently degrading to
+    every-worker-races behaviour would mask the regression this patch fixes.
+    """
+    try:
+        cr.execute(
+            "SELECT pg_try_advisory_xact_lock(%s)",
+            (_FASTAPI_SYNC_ADVISORY_LOCK_KEY,),
+        )
+        (got_lock,) = cr.fetchone()
+        return got_lock
+    except Exception as e:
+        _logger.warning(
+            "FastAPI endpoint sync advisory-lock acquire failed (%s); "
+            "treating as 'lock held elsewhere' and skipping sync this round.",
+            e,
+        )
+        return False
+
 
 class IrHttp(models.AbstractModel):
     """Patch ir.http to fix routing_map cache bug"""
@@ -80,37 +121,64 @@ def routing_map(self, key=None):
                 from odoo.api import Environment
 
                 with registry.cursor() as cr:
-                    env = Environment(cr, SUPERUSER_ID, {})
-
-                    # First check for endpoints with registry_sync=False (never synced)
-                    unsynced_endpoints = env["fastapi.endpoint"].search([("registry_sync", "=", False)])
-
-                    # Also check for endpoints that claim to be synced but have no routes
-                    # This catches cases where routes were deleted or DB was reset
-                    synced_endpoints = env["fastapi.endpoint"].search([("registry_sync", "=", True)])
-                    if synced_endpoints and "endpoint.route" in env:
-                        for endpoint in synced_endpoints:
-                            route_exists = env["endpoint.route"].search_count(
-                                [("endpoint_id", "=", endpoint.id)], limit=1
-                            )
-                            if not route_exists:
-                                _logger.warning(
-                                    "Endpoint '%s' (id=%d) claims to be synced but has no routes - forcing re-sync",
-                                    endpoint.name,
-                                    endpoint.id,
-                                )
-                                # Reset flag to trigger re-sync
-                                endpoint.registry_sync = False
-                                unsynced_endpoints |= endpoint
-
-                    if unsynced_endpoints:
-                        unsynced_endpoints.action_sync_registry()
-                        cr.commit()
+                    # Serialize concurrent sync attempts across workers. After a
+                    # registry reload (e.g. -u all) every worker's routing_map()
+                    # races to update the same fastapi_endpoint rows; without
+                    # this lock all but one fail with SerializationFailure.
+                    if not _try_acquire_fastapi_sync_lock(cr):
+                        # Skipping is safe: the worker that DID get the lock will
+                        # bump endpoint_route_version when it commits action_sync_registry().
+                        # That version is part of our routing-map cache key (line ~89),
+                        # and we re-read it per call, so any degraded routing map this
+                        # worker caches now is keyed at the old version and is naturally
+                        # invalidated on the next call after the winner commits. Bad
+                        # window is bounded by winner-commit latency (seconds at most).
+                        #
+                        # INFO (not DEBUG) so it's visible at default log level —
+                        # otherwise diagnosing transient missing routes after a
+                        # cold start has nothing to go on. Fires at most once
+                        # per registry reload per worker, so not noisy.
                         _logger.info(
-                            "Synced %d FastAPI endpoints for database %s",
-                            len(unsynced_endpoints),
+                            "FastAPI endpoint sync skipped for %s — another worker is syncing",
                             registry.db_name,
                         )
+                    else:
+                        env = Environment(cr, SUPERUSER_ID, {})
+
+                        # First check for endpoints with registry_sync=False (never synced)
+                        unsynced_endpoints = env["fastapi.endpoint"].search([("registry_sync", "=", False)])
+
+                        # Also check for endpoints that claim to be synced but have no routes
+                        # This catches cases where routes were deleted or DB was reset
+                        synced_endpoints = env["fastapi.endpoint"].search([("registry_sync", "=", True)])
+                        if synced_endpoints and "endpoint.route" in env:
+                            for endpoint in synced_endpoints:
+                                route_exists = env["endpoint.route"].search_count(
+                                    [("endpoint_id", "=", endpoint.id)], limit=1
+                                )
+                                if not route_exists:
+                                    _logger.warning(
+                                        "Endpoint '%s' (id=%d) claims to be synced but has no routes - forcing re-sync",
+                                        endpoint.name,
+                                        endpoint.id,
+                                    )
+                                    # Reset flag to trigger re-sync
+                                    endpoint.registry_sync = False
+                                    unsynced_endpoints |= endpoint
+
+                        if unsynced_endpoints:
+                            unsynced_endpoints.action_sync_registry()
+                            _logger.info(
+                                "Synced %d FastAPI endpoints for database %s",
+                                len(unsynced_endpoints),
+                                registry.db_name,
+                            )
+                            # cr.commit() ends the transaction and RELEASES the
+                            # advisory lock acquired above. Do not add any sync
+                            # work below this line — it would run unlocked and
+                            # re-introduce the SerializationFailure race this
+                            # patch exists to prevent.
+                            cr.commit()
         except Exception as e:
             # If endpoint model doesn't exist or sync fails, continue anyway
             _logger.debug("Could not sync FastAPI endpoints: %s", e)

spp_api_v2/readme/HISTORY.md

@@ -1,3 +1,9 @@
+### 19.0.2.0.1
+
+- Fix `SerializationFailure` race when multiple Odoo workers rebuild their routing map simultaneously (e.g. after `-u all`) and all try to sync the same `fastapi.endpoint` rows
+- Serialize concurrent sync attempts across workers using a transaction-scoped Postgres advisory lock; workers that don't acquire the lock skip the sync and pick up the freshly synced routes on the next routing-map rebuild (via `endpoint_route_version` cache invalidation)
+- Log skipped syncs at INFO and lock-primitive failures at WARNING so cold-start route-availability symptoms and broken-primitive regressions are diagnosable without raising the global log level
+
 ### 19.0.2.0.0
 
 - Initial migration to OpenSPP2