Skip to content

feat(ip2country): prefs panel with DB-IP / MaxMind / Custom sources - #113

Merged
got3nks merged 8 commits into
amule-org:masterfrom
got3nks:feat/ip2country-prefs-panel
Jun 11, 2026
Merged

feat(ip2country): prefs panel with DB-IP / MaxMind / Custom sources#113
got3nks merged 8 commits into
amule-org:masterfrom
got3nks:feat/ip2country-prefs-panel

Conversation

@got3nks

@got3nks got3nks commented Jun 11, 2026

Copy link
Copy Markdown

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.mmdb name.

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.conf to 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

  • Source dropdown (DB-IP / MaxMind / Custom URL) with provider-specific sub-panels that show/hide cleanly via wxSizer::Show() so the dialog height stays bounded.
  • MaxMind sub-panel: License Key field (Account ID isn't needed for the GeoLite2 download endpoint).
  • Custom URL sub-panel: free-form URL pointing at an .mmdb, .gz, or .tar.gz containing one.
  • Per-source auto-update on startup, plus an explicit Update now button.
  • Status line: Loaded (X MB) — Data by <provider> with the on-disk path in a tooltip; Failed to load and Not found states have actionable hints.
  • The whole panel greys out when the master "Show country flags for clients" checkbox is off.
  • Failed manual updates raise a wxMessageBox (the auto-update on startup stays silent so a transient outage doesn't pop up on every cold boot).
  • If the user changes the source (or the credential for the active source) and clicks OK without clicking Update now, the new database is fetched automatically — previously the new source was persisted but the on-disk file was from the old source until the user remembered to refresh manually.

Data-file handling

  • Renames the on-disk file from GeoLite2-Country.mmdb to geoip.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.
  • DB-IP early-month fallback: if the current month's URL 404s (DB-IP publishes a few days into each month), DownloadFinished retries once with the previous month's URL before giving up. Logged at info level.
  • Persists which source actually wrote the file (LoadedSource pref) so attribution stays correct after the user flips the dropdown to a provider they haven't downloaded from yet. One-shot backfill in Enable() populates this for files that pre-date the new pref.

Build / feature gating

  • ENABLE_IP2COUNTRY auto-defaults ON when libmaxminddb is detected and OFF otherwise (was unconditionally OFF before, so users with the dep installed were getting the feature disabled silently).
  • All new symbols are #ifdef ENABLE_IP2COUNTRY-gated; verified that -DENABLE_IP2COUNTRY=NO builds cleanly on macOS with no dead-code warnings.

Translation

  • scripts/update-po.sh was run; amule.pot and all .po catalogs are in sync with the new strings.

Test plan

  • macOS arm64 (Homebrew, monolithic, ENABLE_IP2COUNTRY=YES): full smoke test of all three sources, master-toggle greying, manual-update popups, auto-download on OK.
  • macOS arm64 with ENABLE_IP2COUNTRY=NO: clean build, no missing-symbol errors.
  • Linux + Windows CI on the fork branch — both green at the tip.
  • Legacy GeoLite2-Country.mmdb migration verified by dropping a v2.x file into the config dir and confirming it gets renamed in place.

got3nks added 6 commits June 11, 2026 22:39
…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.
got3nks added 2 commits June 11, 2026 22:58
…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.
@got3nks
got3nks merged commit c5ada9e into amule-org:master Jun 11, 2026
9 checks passed
@got3nks
got3nks deleted the feat/ip2country-prefs-panel branch June 11, 2026 21:12
got3nks added a commit that referenced this pull request Jun 11, 2026
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.
@Vollstrecker

Copy link
Copy Markdown

Didn't wee agree that on not silently activate/deactivate features?

@got3nks

got3nks commented Jun 12, 2026

Copy link
Copy Markdown
Author

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 ENABLE_IP2COUNTRY=ON, hard-fail at configure if libmaxminddb is missing. Matches NLS/BFD/UPnP exactly. Users without libmaxminddb pass -DENABLE_IP2COUNTRY=NO.

I lean A. Will switch to B if you prefer strict consistency.

@Vollstrecker

Copy link
Copy Markdown

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.

@got3nks

got3nks commented Jun 12, 2026

Copy link
Copy Markdown
Author

Got it — Option B. The hard-fail path in cmake/ip2country.cmake is already wired; I'll open a small PR with the cmake change + the matching docs updates (INSTALL.md table, IP2Country.md prerequisites paragraph).

got3nks added a commit that referenced this pull request Jun 12, 2026
…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.
got3nks added a commit that referenced this pull request Jun 12, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

enhancement: geoIP

2 participants