feat(ip2country): prefs panel with DB-IP / MaxMind / Custom sources - #113
Conversation
…stom sources
aMule's GeoIP / IP2Country flow used to be exclusively manual. Users had
to figure out the right URL, download the MMDB file themselves, drop it
at ~/.aMule/GeoLite2-Country.mmdb, then toggle "Show country flags for
clients" buried in the Interface tab. The URL field existed only in
amule.conf — no UI. MaxMind's mandatory account-and-license-key flow,
plus the .tar.gz wrapper they ship the file in, locked the feature behind
hand-editing two text files.
This PR adds a dedicated Preferences → IP2Country tab (compile-gated on
ENABLE_IP2COUNTRY) with three sources:
DB-IP (free, no account) <- default
MaxMind GeoLite2 (account)
Custom URL <- escape hatch / on-prem mirrors
The master "Show country flags for clients" checkbox moves into this
new tab as the enable switch. The amule.conf key for it
(/eMule/GeoIPEnabled) is unchanged.
Pipeline
- Preferences.cpp: five new persisted settings — GeoIPSource enum (str),
GeoIPMaxMindAccount, GeoIPMaxMindLicense, GeoIPCustomUrl,
GeoIPAutoUpdate (default true). A one-shot migration in
LoadPreferences() carries any pre-existing GeoLiteCountryUpdateUrl
value into GeoIPCustomUrl + flips Source to "custom" so users with
the v2.x hand-configured URL don't lose it on upgrade.
- Preferences::GetGeoIPResolvedDownloadUrl(monthOffset) computes the
download URL per-source: DB-IP substitutes YYYY-MM into a templated
URL; MaxMind inserts Account ID + License Key in the URL userinfo;
Custom returns the user's URL verbatim. monthOffset lets the caller
re-issue with the previous month — DB-IP often publishes the new
month's file a few days late, so the IP2Country update path retries
once with -1 before giving up.
- IP2Country.cpp: filename is now ~/.aMule/geoip.mmdb. One-shot
migration on construction moves any existing GeoLite2-Country.mmdb
over so upgrading users don't lose flag display silently. Update()
now drives off GetGeoIPResolvedDownloadUrl + the retry-with-prev-month
logic; DownloadFinished's failure branch retries DB-IP once before
disabling the feature.
- amuleDlg.cpp's EnableIP2Country() honours the new auto-update
toggle: on startup, if both IsGeoIPEnabled and IsGeoIPAutoUpdate
are set and the database is loaded, kick off a background Update()
to refresh from the selected source.
UI
- muuli_wdr.cpp: PreferencesIP2CountryTab builds the panel — master
enable checkbox + Database group box (multi-line status block,
source dropdown, source-specific sub-panel, Update Now button,
Auto-update on startup checkbox). The source-specific block hosts
three sibling sub-panels (DB-IP info / MaxMind credential fields +
info / Custom URL field + disclaimer); exactly one is shown at a
time based on the dropdown selection.
- PrefsUnifiedDlg.cpp: OnGeoIPSourceChange and OnGeoIPUpdateNow event
handlers; UpdateGeoIPSourcePanel hides/shows the right sub-panel;
UpdateGeoIPStatus renders the 3-state status (loaded with size +
source attribution / present-but-rejected / not-found) using the
per-source attribution suffix so the credit stays visible regardless
of which sub-panel is expanded.
- IDC_SHOW_COUNTRY_FLAGS deleted from PreferencesGuiTweaksTab; it's
the master enable in the new tab now.
Licensing / attribution
Both DB-IP CC BY 4.0 and the MaxMind GeoLite2 EULA require attribution.
The per-source sub-panel includes the required attribution string inline
in the visible panel content — selecting a source as the user does
satisfies the display-attribution obligation without a separate
About-dialog change. The status line surfaces a short attribution suffix
("Data by DB-IP.com" / "Data by MaxMind GeoLite2" / "Custom source") so
the credit stays on screen even when the dropdown is collapsed.
All visible strings are wrapped in _() for translation.
…d headless URL-helper test The old IP2Country.md was structured around the v2.x manual workflow: download the file from MaxMind, drop it in ~/.aMule, hand-edit amule.conf for an auto-update URL. The new doc is structured around the Preferences → IP2Country tab, with one section per source: * DB-IP (default, zero-config) * MaxMind GeoLite2 (Account ID + License Key) * Custom URL (escape hatch / mirrors) Plus migration notes for users upgrading from v2.x (GeoLite2-Country.mmdb auto-renamed to geoip.mmdb, legacy GeoLiteCountryUpdateUrl in amule.conf migrated to Custom URL). scripts/test-geoip-urls.sh is a small headless smoke test: it boots amuled once per source configuration, captures the "Download new ..." log line, and asserts the URL matches the expected pattern. Covers: * dbip -> current-month dbip-country-lite-YYYY-MM.mmdb.gz * maxmind -> https://<acct>:<key>@download.maxmind.com/.../...tar.gz * custom -> URL passed through verbatim * maxmind w/ empty credentials -> surfaces the missing-credentials log Run from repo root: ./scripts/test-geoip-urls.sh
CI failed on every non-Mac platform with PrefsUnifiedDlg.cpp:1269:47: error: invalid use of incomplete type 'class CIP2Country' Mac's looser transitive-include rules pulled IP2Country.h in via some amule.h header chain, so the local build passed. Linux's stricter check requires the include directly. Also need wx/filename.h for the status-line file-size lookup (wxFileName::FileExists / GetSize). Both #include lines are #ifdef'd on ENABLE_IP2COUNTRY to match the guards around the IP2Country code paths in this file. While we're here, regenerate po/amule.pot and merge into po/*.po so the pot-sync CI check is back in sync with the new translatable strings introduced by PreferencesIP2CountryTab and CIP2Country error messages.
…ching
Multiple bugs surfaced by GUI smoke testing on macOS:
1. Sub-panel switching glitched (e.g. "Download URL:" label stayed
visible after switching to DB-IP)
The three source-specific blocks were sibling widget groups parented
under a single wxStaticBoxSizer, not real wxPanel children. Hiding
individual widgets via Show(false) didn't propagate to nearby
ID-less labels and didn't collapse sizer slots, so unrelated text
from other sources stayed on screen.
Wrap each block in a discrete wxPanel (re-using
IDC_GEOIP_INFO_DBIP / MAXMIND / CUSTOM as the panel IDs). Now
Show/Hide operates on the wxPanel as a unit, and a single
Layout() call on the staticbox is safe (no cascade loop) because
the children are full wxPanels — not raw widgets the wxOSX
staticbox layout used to spin on.
2. Preferences dialog grew vertically because the multi-line status
block + multi-line attribution text dominated the tallest tab
Status block is now a single-line summary: "Status: Loaded (9.0
MB) — Data by DB-IP.com" / "Status: Failed to load — ..." /
"Status: Not found — ...". Full path moved to a tooltip. The
attribution strings inside each sub-panel are also shortened
(separator dashes instead of separate lines).
3. Status didn't refresh after a successful Update Now until the
user flipped the source dropdown
Add PrefsUnifiedDlg::RefreshIP2CountryStatusIfOpen() static hook;
CamuleDlg::IP2CountryDownloadFinished() calls it after the new
MMDB is loaded. The dialog's s_activeInstance pointer is set in
the constructor and cleared in the new destructor, so the hook is
a no-op if Preferences is closed.
4. "Status: ●" used a Unicode bullet that doesn't carry color
semantics — the dot rendered themeless (white in dark mode), not
the green/amber/red claimed in the code comments
Drop the bullet. The state word ("Loaded" / "Failed to load" /
"Not found") carries the semantics by itself and is accessible
to screen readers without relying on color/glyph rendering.
5. Initial "Status: (queried on open)" placeholder was a
user-visible string in wxTRANSLATE() that gets overwritten in
microseconds by UpdateGeoIPStatus() during TransferToWindow
Replace with wxEmptyString. Nothing to translate, nothing to see.
Pre-existing fixes in this commit (rebased from the earlier amend):
- MaxMind License-Key-only URL (drop Account ID field)
- ENABLE_IP2COUNTRY=OFF crash fix (gate Cfg bindings + remove dead
disable-callback)
- Mac wxOSX layout-cascade hang (now resolved differently via real
wxPanel children + single targeted Layout call)
- Auto-default ENABLE_IP2COUNTRY based on libmaxminddb detection
…eckbox gate * Manual "Update now" failures (no License Key, no Custom URL, HTTP error after DB-IP fallback exhausted, unpack/rename failure) now raise a wxMessageBox parented to the prefs dialog in addition to the network log line. Auto-update on startup stays silent so a transient outage doesn't pop up on every cold boot. Plumbed via a manualUpdate flag on CIP2Country::Update + a static PrefsUnifiedDlg notifier hook. * OnOk now detects when the user changed the GeoIP source (or the credential for the currently selected source) during the dialog session and kicks off Update(true) automatically — previously the new source was persisted but the on-disk file was still from the old source until the user remembered to click Update now. * "Show country flags for clients" master checkbox now greys out the whole panel below it (source dropdown, all three sub-panels, license + URL fields, Update now button, auto-update checkbox, status line) via UpdateGeoIPControlsEnabled, called from OnGeoIPMasterToggle and TransferToWindow. * One-shot LoadedSource backfill in CIP2Country::Enable: when an existing MMDB opens but GetGeoIPLoadedSource is empty (file pre-dates the source-aware prefs), attribute it to the currently configured source so the status line shows something meaningful. * Drop "escape hatch" jargon from the source-dropdown tooltip.
Pulled in the new translatable strings from this branch (manual-update failure popup title + reworded source-dropdown tooltip) by running scripts/update-po.sh. Merged the regenerated pot into every .po so Weblate stays in sync.
…nreliable Mac tooltip * Status block: drop the wxStaticText tooltip that surfaced the on-disk path. wxStaticText tooltips on wxOSX have a tracking area limited to the rendered glyphs and are flaky enough that promising "hover for path" in the UI misled Mac users. Path is still in the log line at load time, and the per-platform location is documented. * Doc audit: status table now matches the exact strings emitted by UpdateGeoIPStatus (was lying about both the "Failed to load" and the no-attribution variants). * Doc audit: ASCII mock-up moved Status below Update Now (matches the actual panel order) and lost the themeless bullet that the renderer was painting white-on-white. * Documented the master-checkbox greying behaviour, the manual / on-OK / auto-update download triggers + their distinct failure surfaces (popup vs log-only), the LoadedSource-vs-dropdown attribution rule, and the DB-IP early-month fallback retry. * Drop "escape hatch" wording from the Custom URL section to match the softened source-dropdown tooltip.
…" for Windows Mojibake on Windows: the literal UTF-8 em-dash bytes (\xE2\x80\x94) in _()-wrapped runtime strings hit the wx narrow-to-wide conversion path through the ANSI codepage when there is no catalog hit, which renders them as "â€\"". Linux/Mac happen to do UTF-8 conversion there and looked fine, hiding the issue. ASCII " - " sidesteps the encoding mess entirely and reads naturally in the affected lines: * Preferences > IP2Country status block (Loaded / Failed to load / Not found) * Source-dropdown tooltip * DB-IP / MaxMind / Custom URL sub-panel attribution + license text * DB-IP early-month fallback log line Em-dashes in source comments are left alone - they don't make it into the binary. po regen: scripts/update-po.sh; pot + all .po files updated to the new ASCII msgids.
This headless probe was used during IP2Country development to sanity-check that each source's URL helper returns a downloadable file; it has no place in the released tree. Removed.
|
Didn't wee agree that on not silently activate/deactivate features? |
|
Fair concern. Two paths I'd land: A. Keep build-time auto-detect, flip the runtime default to OFF. The auto-detect still rewards users who already have libmaxminddb (intent signal), but "Show country flags" ships unticked → no silent first-launch behavior change, no silent MMDB download. B. Unconditional I lean A. Will switch to B if you prefer strict consistency. |
|
Runtime behaviour is a different story, but for compile I prefer if an option is enabled and the dep is required. If this option defaults to ON it doesn't make any difference for users that have it installed, for the others: Well, at least they know now and either set the option to OFF or install it. |
|
Got it — Option B. The hard-fail path in |
…ep (#119) Drop the find_path/find_library probe in cmake/options.cmake that defaulted ENABLE_IP2COUNTRY based on whether libmaxminddb was discoverable. The probe-driven default was a "silent activation" that conflicted with the project's feature-gating policy (#113 review). Switch to the same shape used by ENABLE_NLS, ENABLE_BFD, and ENABLE_UPNP: default the option ON unconditionally. cmake/ip2country.cmake already FATAL_ERRORs with an actionable hint ("install libmaxminddb-dev or pass -DENABLE_IP2COUNTRY=NO") when the dep is missing, so the missing-dep UX is preserved. Docs updated to match: - INSTALL.md: ENABLE_IP2COUNTRY default flipped to YES. - IP2Country.md: build-prerequisites paragraph rewritten to describe the hard-fail behaviour and the opt-out flag.
Adds Italian translations for the 32 strings added by the IP2Country preferences-panel work in #113: - 19 short labels and log strings (source dropdown, status line, license-key field, attribution lines, download failure messages) - 7 fuzzy entries (Database / Source / Download URL / Auto-update / Custom source / Status: Loaded%s / Status: Loaded%s - %s) that msgmerge had guessed from neighbouring strings; corrected + de-fuzzed. - 3 multi-line sub-panel description blocks (DB-IP, MaxMind, Custom URL) covering attribution + licensing notices. Tone follows the existing it.po register (Stato, Aggiorna, Scaricamento, Sorgente). Proper nouns (DB-IP, MaxMind, GeoLite2, MMDB, License Key in MaxMind context) kept verbatim; "Chiave di licenza" used as a standalone field label. After this commit: 1725 translated, 0 fuzzy, 0 untranslated.
Closes amule-project#844.
Summary
Promotes the buried "Show country flags" checkbox in Preferences → Interface into a dedicated IP2Country preferences tab. The user picks one of three GeoIP sources — DB-IP (default, no account), MaxMind GeoLite2 (free license key required), or a Custom URL — and the panel handles credential entry, manual + automatic updates, status display, and on-disk database migration from the legacy
GeoLite2-Country.mmdbname.Background: the v2.x flow assumed MaxMind's open feed and broke when MaxMind moved to license-keyed downloads. Users had to hand-edit
amule.confto point at a working URL, which is what issue #844 is about. This PR puts every supported workflow behind UI and surfaces failures (missing license, bad URL, HTTP error) as modal popups instead of silent log lines.What's in the panel
wxSizer::Show()so the dialog height stays bounded..mmdb,.gz, or.tar.gzcontaining one.Loaded (X MB) — Data by <provider>with the on-disk path in a tooltip;Failed to loadandNot foundstates have actionable hints.wxMessageBox(the auto-update on startup stays silent so a transient outage doesn't pop up on every cold boot).Data-file handling
GeoLite2-Country.mmdbtogeoip.mmdb(source-neutral); the constructor migrates an existing legacy file in place so users don't lose flag display silently on first run of the new build.DownloadFinishedretries once with the previous month's URL before giving up. Logged at info level.LoadedSourcepref) so attribution stays correct after the user flips the dropdown to a provider they haven't downloaded from yet. One-shot backfill inEnable()populates this for files that pre-date the new pref.Build / feature gating
ENABLE_IP2COUNTRYauto-defaults ON whenlibmaxminddbis detected and OFF otherwise (was unconditionally OFF before, so users with the dep installed were getting the feature disabled silently).#ifdef ENABLE_IP2COUNTRY-gated; verified that-DENABLE_IP2COUNTRY=NObuilds cleanly on macOS with no dead-code warnings.Translation
scripts/update-po.shwas run;amule.potand all.pocatalogs are in sync with the new strings.Test plan
ENABLE_IP2COUNTRY=YES): full smoke test of all three sources, master-toggle greying, manual-update popups, auto-download on OK.ENABLE_IP2COUNTRY=NO: clean build, no missing-symbol errors.GeoLite2-Country.mmdbmigration verified by dropping a v2.x file into the config dir and confirming it gets renamed in place.