Update GeoIP

[pop4959]

This implements the fix suggested by @Bobcat00 in #2919

Users of GeoIP will now be required to register a MaxMind account and create a license to download the database required by the plugin. This license is entered into the new license-key field in the configuration.

[Bobcat00]

Are you sure this results in a usable database? We both tried this by manually changing the config file and the database extraction was not correct. (I still think the top-level directory needs to be accounted for.)

[k-jiang]

@Bobcat00 I think this should be fine. I remember what I did here is to iterate through all of the files in tar.gz. So it doesn't matter how deep the mmdb file is buried in, this code should be able to extract it.

[k-jiang]

@pop4959 Thank you for your commit! Sorry I was too busy to catch this up recently.
But Just to be safe. I think it is better to change the url here and here as well. They are for upgrade from old versions.

[Bobcat00]

@Bobcat00 I think this should be fine. I remember what I did here is to iterate through all of the files in tar.gz. So it doesn't matter how deep the mmdb file is buried in, this code should be able to extract it.

OK, as long as you guys tested that the database is usable.

I see that if the code doesn't find the mmdb file, it still extracts something. I would suggest checking if the file was actually found and bailing out with an error if it wasn't. That would be more robust.

[pop4959]

@Bobcat00 The issue was mostly the URL format changed and GeoIP didn't like it. I had someone join a test server with a freshly downloaded database and it seems to be working properly.

@k-jiang I wish I knew that sooner, as I spent some time trying to verify that the file code was working. Anyways, it looks like there's no issues with it, as you said. Also, good catch on the config upgrade stuff, I hadn't noticed that.

[pop4959]

Oh and probably worth mentioning, the translation for geoIpLicenseMissing is a work in progress. @md678685 suggested changing it so it links to an EssentialsX wiki page instead describing first time setup. Before updating it, that wiki page should be created first. If anyone wants to take initiative on writing that, feel free.

[DoNotSpamPls]

While not necessary at all, it could be nice to add a bStats metric on how many people have GeoIP actually set up

[pop4959]

While not necessary at all, it could be nice to add a bStats metric on how many people have GeoIP actually set up

Would be nice to do this for all modules. Might be best in a separate PR. Perhaps just add a custom chart for it. Although, you'd probably need to get a hold of SupaHam so he could enable that on the actual bStats page since he manages it.

[Ichbinjoe]

So I'm changing my tune on this PR - I'm not going to treat the geoip config values as user configurable, which reduces the burden on us needing to do weird checks to be sure that we are doing what the user actually intended.

[Ichbinjoe]

Conditional approval on wiki page change.

[mdcfe]

Does this definitely always result in a valid database being downloaded, or are there further changes needed? I recall there being an issue with the current tar extraction code.

The wiki page is currently done, with the exception of an image that needs replacing (I committed the wrong one).

[pop4959]

@md678685 I'm assuming the wiki page isn't published yet, as I wasn't able to find it. As soon as it's ready, all that needs to be done here is update geoIpLicenseMissing to something like "No license key found! Please visit {link} for first time setup instructions.". After that I think we can merge.

As for the database extraction code, I don't think there is an issue with it anymore. I think you might be referring to the Discord conversation we had. What might have happened is the file was not downloading completely for whatever reason (I may have been unlucky and the GeoIP site was having issues). That led me on a complete wild goose chase, because later that morning I was able to download it just fine with no code changes there at all. Fairly frustrating, but at the very least I can say that the code seems sound. We have the author of the code @k-jiang (who wrote it in 938f94e) here as well confirming that it works as expected.

Here are some testing results to be a bit more convincing (you can also pull the PR yourself and check).

Without license key installed:

[13:11:08 ERROR]: [EssentialsGeoIP] No license key found! Register an account and create one at https://www.maxmind.com/en/geolite2/signup

and database did not download.

With license key installed:

[13:13:15 INFO]: [EssentialsGeoIP] Downloading GeoIP database... this might take a while (country: 1.7 MB, city: 30MB)
[13:13:19 INFO]: [EssentialsGeoIP] This product includes GeoLite2 data created by MaxMind, available from http://www.maxmind.com/.

and database downloaded, working as expected.

In-game output is as expected:

[mdcfe]

https://essentialsx.cf/geoip now details how to set up the GeoIP module.

[pop4959]

@md678685 I reviewed your wiki entry, and it looks absolutely correct to me. Thanks for taking the time to create it! Should be fairly easy for server owners to understand how to update the the plugin.

... and with that last commit to point at the wiki, I believe all work here is done. Should be ready to merge.