Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/opensak/db/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def _enable_wal_and_fk(dbapi_connection, _connection_record) -> None:
# bumped to the highest migration number whenever a new migration is added
# below — _run_migrations() skips the whole block when the database already
# reports this version, so a stale constant means new migrations never run.
SCHEMA_VERSION = 24
SCHEMA_VERSION = 25


def init_db(db_path: Path | None = None) -> Engine:
Expand Down Expand Up @@ -1013,6 +1013,19 @@ def _run_migrations(engine: Engine) -> None:
conn.commit()
print(f"Migration: tilføjede caches.{', caches.'.join(added_23)}")

# ── Migration 25: partial index on caches missing a distance ─────────
logger.debug("[migrations] entering migration 25 (+%.2fs)", time.monotonic() - _mig_t0)
# distances_up_to_date() looks for a cache without a distance on
# every startup and database switch. On an up-to-date database that
# is a full table scan (~1 s at 180k caches); this index holds only
# the rows still missing one, so the lookup is instant. Rows leave it
# as recalculate_distances() fills them, so its upkeep is negligible.
conn.execute(text(
"CREATE INDEX IF NOT EXISTS ix_caches_distance_missing "
"ON caches (id) WHERE distance IS NULL"
))
conn.commit()

# ── Stamp the schema version so the next launch skips the probes ─────
# PRAGMA does not accept bind parameters; SCHEMA_VERSION is a trusted
# int constant, so inlining it is safe.
Expand Down Expand Up @@ -1385,8 +1398,8 @@ def distances_up_to_date(lat: float, lon: float, db_path: Path | None = None) ->
if abs(fresh_distance - stored_distance) > _DISTANCE_EPSILON_KM:
return False

# Stops at the first hit; only a database that is up to date pays for
# the full scan.
# Served by the partial index ix_caches_distance_missing (migration 25),
# so this stays cheap on large databases.
with _session_on(db_path) as session:
missing = session.execute(
text(
Expand Down
39 changes: 35 additions & 4 deletions src/opensak/gui/dialogs/gsak_import_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,14 @@ class GsakImportWorker(QThread):
# from inside run() itself).

def __init__(self, db3_path: Path, target_db_path: Path | None = None,
replace: bool = False):
replace: bool = False, update_distances: bool = True):
super().__init__()
self.db3_path = db3_path
self.target_db_path = target_db_path # None → use currently active DB
self.replace = replace # empty the target DB first
# False when a later job of the same run imports into the same
# target: that one recalculates, once for all of them.
self.update_distances = update_distances

def run(self) -> None:
from opensak.db.database import get_session, session_for
Expand Down Expand Up @@ -122,7 +125,12 @@ def run(self) -> None:
progress_cb=lambda done, total: self.progress.emit(done, total),
)
self.result_ready.emit(result)
self._update_distances(target_path)
if self.update_distances:
# Busy mode — the import's own progress sits at 100% meanwhile.
self.progress.emit(0, 0)
self._update_distances(target_path)
else:
self._invalidate_distances(target_path)
except Exception:
import traceback
self.error.emit(traceback.format_exc())
Expand Down Expand Up @@ -150,6 +158,26 @@ def _update_distances(db_path: Path | None) -> None:
logger.warning("GSAK import: could not update distances for %s",
db_path, exc_info=True)

@staticmethod
def _invalidate_distances(db_path: Path | None) -> None:
"""Forget the centre *db_path*'s distances were calculated for, so
distances_up_to_date() reports them stale. The target's last job
then recalculates; should it never run (skipped, failed, cancelled),
the check after the import or on switching to that database does.
"""
if db_path is None:
return
try:
from opensak.db import db_settings
db_settings.write_file(db_path, {
"dist_calc_lat": None,
"dist_calc_lon": None,
"dist_calc_method": None,
})
except Exception:
logger.warning("GSAK import: could not invalidate distances for %s",
db_path, exc_info=True)


class GsakExtractWorker(QThread):
"""Unpacks the selected members of a GSAK backup .zip in the background.
Expand Down Expand Up @@ -727,8 +755,11 @@ def _import_current_job(self) -> None:
self._progress.setRange(0, 0)
self._append_log(tr("gsak_import_running",
name=f"{job.gsak_name} → {job.target_name}"))
worker = GsakImportWorker(job.db3_path, target_db_path=job.target_path,
replace=job.replace)
later = self._jobs[self._job_index + 1:]
worker = GsakImportWorker(
job.db3_path, target_db_path=job.target_path, replace=job.replace,
update_distances=all(j.target_name != job.target_name for j in later),
)
worker.progress.connect(self._on_progress)
worker.cleared.connect(
lambda count, name=job.target_name:
Expand Down
61 changes: 60 additions & 1 deletion tests/unit-tests/test_gsak_import_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ def fake_import(path, session, progress_cb=None):
seen = []
w.progress.connect(lambda done, total: seen.append((done, total)))
w.run()
assert seen == [(5, 10)]
# (0, 0): busy mode while the distances are recalculated
assert seen == [(5, 10), (0, 0)]

def test_run_exception_emits_error(self, monkeypatch):
self._patch_common(monkeypatch)
Expand Down Expand Up @@ -136,6 +137,22 @@ def test_run_updates_distances_of_target(self, monkeypatch):
GsakImportWorker(Path("/gsak.db3")).run() # no target → the active DB
assert updated == [Path("/other.db"), Path("/active.db")]

def test_run_without_update_distances_invalidates_them(self, monkeypatch):
updated = self._patch_common(monkeypatch)
invalidated = []
monkeypatch.setattr(GsakImportWorker, "_invalidate_distances",
staticmethod(invalidated.append))
monkeypatch.setattr("opensak.importer.gsak_importer.import_gsak_db",
lambda path, session, progress_cb=None: _result())
monkeypatch.setattr("opensak.db.database.session_for", lambda p: _fake_session())
w = GsakImportWorker(Path("/gsak.db3"), target_db_path=Path("/other.db"),
update_distances=False)
seen = []
w.progress.connect(lambda done, total: seen.append((done, total)))
w.run()
assert updated == [] and invalidated == [Path("/other.db")]
assert seen == []

def test_run_failed_import_does_not_update_distances(self, monkeypatch):
updated = self._patch_common(monkeypatch)
monkeypatch.setattr(
Expand Down Expand Up @@ -459,6 +476,48 @@ def test_distances_are_calculated_for_each_target_database(
finally:
manager.ensure_active_initialised()

def test_distances_are_calculated_once_per_target(
self, dlg, manager, tmp_path, qtbot, monkeypatch):
"""Several GSAK databases into one target: only its last job
recalculates; the earlier ones mark the distances stale instead."""
from opensak.db import db_settings
flags = []
real_init = GsakImportWorker.__init__

def spy(self, *a, **k):
real_init(self, *a, **k)
flags.append(self.update_distances)

monkeypatch.setattr(GsakImportWorker, "__init__", spy)
dlg.set_path(_backup(tmp_path, {"A": "GC1AAA", "B": "GC2BBB", "C": "GC3CCC"},
with_settings=False))
_target(dlg, 1).setCurrentText("A")
_run(dlg, qtbot)

assert flags == [False, True, True]
path = _db_by_name(manager, "A").path
assert _codes(path) == ["GC1AAA", "GC2BBB"]
conn = sqlite3.connect(path)
try:
assert conn.execute(
"SELECT COUNT(*) FROM caches WHERE distance IS NULL").fetchone()[0] == 0
finally:
conn.close()
assert db_settings.read_file(path)["dist_calc_lat"] is not None

def test_invalidated_distances_are_reported_stale(self, manager, tmp_path):
"""What a target's earlier jobs leave behind if its last job never
runs: the check after the import / on switching recalculates."""
from opensak.db import db_settings
from opensak.db.database import distances_up_to_date
path = manager.new_database("Stale").path
db_settings.write_file(path, {"dist_calc_lat": 55.0, "dist_calc_lon": 12.0,
"dist_calc_method": "haversine"})
GsakImportWorker._invalidate_distances(path)
stored = db_settings.read_file(path)
assert stored["dist_calc_lat"] is None and stored["dist_calc_method"] is None
assert db_settings.peek_value(path, "dist_calc_lat") is None

def test_only_ticked_databases_are_imported(self, dlg, manager, tmp_path, qtbot):
dlg.set_path(_backup(tmp_path, {"A": "GC1AAA", "B": "GC2BBB"}, with_settings=False))
dlg._table.item(1, gdlg.COL_GSAK).setCheckState(Qt.CheckState.Unchecked)
Expand Down
13 changes: 13 additions & 0 deletions tests/unit-tests/test_migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,16 @@ def test_found_log_count_backfill_skipped_without_username_configured(tmp_path):

assert "found_log_count" in cache_cols
assert found_log_count == 0


def test_missing_distance_lookup_uses_partial_index(tmp_path):
# Migration 25: distances_up_to_date()'s NULL check must not scan the table.
init_db(db_path=tmp_path / "idx.db")
with get_engine().connect() as c:
plan = " ".join(
str(row[-1]) for row in c.execute(text(
"EXPLAIN QUERY PLAN SELECT 1 FROM caches WHERE distance IS NULL "
"AND latitude IS NOT NULL AND longitude IS NOT NULL LIMIT 1"
))
)
assert "ix_caches_distance_missing" in plan, plan
Loading