From 2cd4461b57fed9275badb351c52dffd2dcb7b34f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 04:32:49 -0400 Subject: [PATCH 1/7] [locale] Rename JSON handler - The handler for JSON data should be called that, not named specially after the original provider it was implemented for. - Make filename and classname consistent, GeoIPJSON. --- src/modules/locale/CMakeLists.txt | 2 +- .../locale/{GeoIPFreeGeoIP.cpp => GeoIPJSON.cpp} | 4 ++-- src/modules/locale/{GeoIPFreeGeoIP.h => GeoIPJSON.h} | 11 +++++------ src/modules/locale/GeoIPTests.cpp | 6 +++--- src/modules/locale/LocaleViewStep.cpp | 6 +++--- src/modules/locale/test_geoip.cpp | 4 ++-- 6 files changed, 16 insertions(+), 17 deletions(-) rename src/modules/locale/{GeoIPFreeGeoIP.cpp => GeoIPJSON.cpp} (95%) rename src/modules/locale/{GeoIPFreeGeoIP.h => GeoIPJSON.h} (83%) diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index 3c0cc3712..384135d4c 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -6,7 +6,7 @@ endif() include_directories( ${PROJECT_BINARY_DIR}/src/libcalamaresui ) -set( geoip_src GeoIP.cpp GeoIPFreeGeoIP.cpp ) +set( geoip_src GeoIP.cpp GeoIPJSON.cpp ) set( geoip_libs ) find_package(Qt5 COMPONENTS Xml) diff --git a/src/modules/locale/GeoIPFreeGeoIP.cpp b/src/modules/locale/GeoIPJSON.cpp similarity index 95% rename from src/modules/locale/GeoIPFreeGeoIP.cpp rename to src/modules/locale/GeoIPJSON.cpp index 9ef534a5d..78f6c67a7 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.cpp +++ b/src/modules/locale/GeoIPJSON.cpp @@ -17,7 +17,7 @@ * along with Calamares. If not, see . */ -#include "GeoIPFreeGeoIP.h" +#include "GeoIPJSON.h" #include "utils/Logger.h" #include "utils/YamlUtils.h" @@ -27,7 +27,7 @@ #include GeoIP::RegionZonePair -FreeGeoIP::processReply( const QByteArray& data ) +GeoIPJSON::processReply( const QByteArray& data ) { try { diff --git a/src/modules/locale/GeoIPFreeGeoIP.h b/src/modules/locale/GeoIPJSON.h similarity index 83% rename from src/modules/locale/GeoIPFreeGeoIP.h rename to src/modules/locale/GeoIPJSON.h index e8986db3f..26a0560f6 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.h +++ b/src/modules/locale/GeoIPJSON.h @@ -16,20 +16,19 @@ * along with Calamares. If not, see . */ -#ifndef GEOIPFREEGEOIP_H -#define GEOIPFREEGEOIP_H +#ifndef GEOIPJSON_H +#define GEOIPJSON_H #include "GeoIP.h" -/** @brief GeoIP lookup via freegeoip.com +/** @brief GeoIP lookup for services that return JSON. * * This is the original implementation of GeoIP lookup, - * using the FreeGeoIP service, or similar which returns - * data in the same format. + * (e.g. using the FreeGeoIP.net service), or similar. * * The data is assumed to be in JSON format with a time_zone attribute. */ -struct FreeGeoIP : public GeoIP +struct GeoIPJSON : public GeoIP { virtual RegionZonePair processReply( const QByteArray& ); } ; diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp index 2c59aa729..4f946b8b7 100644 --- a/src/modules/locale/GeoIPTests.cpp +++ b/src/modules/locale/GeoIPTests.cpp @@ -18,7 +18,7 @@ #include "GeoIPTests.h" -#include "GeoIPFreeGeoIP.h" +#include "GeoIPJSON.h" #ifdef HAVE_XML #include "GeoIPXML.h" #endif @@ -46,7 +46,7 @@ GeoIPTests::testJSON() static const char data[] = "{\"time_zone\":\"Europe/Amsterdam\"}"; - FreeGeoIP handler; + GeoIPJSON handler; auto tz = handler.processReply( data ); QCOMPARE( tz.first, QLatin1String( "Europe" ) ); @@ -65,7 +65,7 @@ GeoIPTests::testJSONbad() { static const char data[] = "time_zone: 1"; - FreeGeoIP handler; + GeoIPJSON handler; auto tz = handler.processReply( data ); tz = handler.processReply( data ); diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 7cafd8793..243ad436c 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -20,7 +20,7 @@ #include "LocaleViewStep.h" #include "GeoIP.h" -#include "GeoIPFreeGeoIP.h" +#include "GeoIPJSON.h" #ifdef HAVE_XML #include "GeoIPXML.h" #endif @@ -124,11 +124,11 @@ LocaleViewStep::fetchGeoIpTimezone() if ( m_geoipStyle.isEmpty() || m_geoipStyle == "legacy" ) { actualUrl.append( "/json/" ); - handler = new FreeGeoIP; + handler = new GeoIPJSON; } else if ( m_geoipStyle == "json" ) { - handler = new FreeGeoIP; + handler = new GeoIPJSON; } #if defined(HAVE_XML) else if ( m_geoipStyle == "xml" ) diff --git a/src/modules/locale/test_geoip.cpp b/src/modules/locale/test_geoip.cpp index 5797ad9ff..70c205b40 100644 --- a/src/modules/locale/test_geoip.cpp +++ b/src/modules/locale/test_geoip.cpp @@ -22,7 +22,7 @@ #include -#include "GeoIPFreeGeoIP.h" +#include "GeoIPJSON.h" #ifdef HAVE_XML #include "GeoIPXML.h" #endif @@ -39,7 +39,7 @@ int main(int argc, char** argv) GeoIP* handler = nullptr; if ( QLatin1String( "json" ) == argv[1] ) - handler = new FreeGeoIP; + handler = new GeoIPJSON; #ifdef HAVE_XML else if ( QLatin1String( "xml" ) == argv[1] ) handler = new XMLGeoIP; From 79a6d7ccbdfa13e5399afc79adfc8ce8af82c346 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 04:35:32 -0400 Subject: [PATCH 2/7] [locale] Make file and class consistent GeoIPXML - Rename the class to match the filename. --- src/modules/locale/GeoIPTests.cpp | 6 +++--- src/modules/locale/GeoIPXML.cpp | 2 +- src/modules/locale/GeoIPXML.h | 2 +- src/modules/locale/LocaleViewStep.cpp | 2 +- src/modules/locale/test_geoip.cpp | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp index 4f946b8b7..d4522836c 100644 --- a/src/modules/locale/GeoIPTests.cpp +++ b/src/modules/locale/GeoIPTests.cpp @@ -100,7 +100,7 @@ GeoIPTests::testXML() )"; #ifdef HAVE_XML - XMLGeoIP handler; + GeoIPXML handler; auto tz = handler.processReply( data ); QCOMPARE( tz.first, QLatin1String( "Europe" ) ); @@ -115,7 +115,7 @@ GeoIPTests::testXML2() "America/North Dakota/Beulah"; #ifdef HAVE_XML - XMLGeoIP handler; + GeoIPXML handler; auto tz = handler.processReply( data ); QCOMPARE( tz.first, QLatin1String( "America" ) ); @@ -127,7 +127,7 @@ void GeoIPTests::testXMLbad() { #ifdef HAVE_XML - XMLGeoIP handler; + GeoIPXML handler; auto tz = handler.processReply( "{time_zone: \"Europe/Paris\"}" ); QCOMPARE( tz.first, QString() ); diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp index a0f9b7a2d..28761e163 100644 --- a/src/modules/locale/GeoIPXML.cpp +++ b/src/modules/locale/GeoIPXML.cpp @@ -24,7 +24,7 @@ #include GeoIP::RegionZonePair -XMLGeoIP::processReply( const QByteArray& data ) +GeoIPXML::processReply( const QByteArray& data ) { QString domError; int errorLine, errorColumn; diff --git a/src/modules/locale/GeoIPXML.h b/src/modules/locale/GeoIPXML.h index 8eec22a75..adffc5bca 100644 --- a/src/modules/locale/GeoIPXML.h +++ b/src/modules/locale/GeoIPXML.h @@ -28,7 +28,7 @@ * element, which contains the text (string) for the region/zone. This * format is expected by, e.g. the Ubiquity installer. */ -struct XMLGeoIP : public GeoIP +struct GeoIPXML : public GeoIP { virtual RegionZonePair processReply( const QByteArray& ); } ; diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 243ad436c..73121581c 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -133,7 +133,7 @@ LocaleViewStep::fetchGeoIpTimezone() #if defined(HAVE_XML) else if ( m_geoipStyle == "xml" ) { - handler = new XMLGeoIP; + handler = new GeoIPXML; } #endif else diff --git a/src/modules/locale/test_geoip.cpp b/src/modules/locale/test_geoip.cpp index 70c205b40..7b6584f0c 100644 --- a/src/modules/locale/test_geoip.cpp +++ b/src/modules/locale/test_geoip.cpp @@ -42,7 +42,7 @@ int main(int argc, char** argv) handler = new GeoIPJSON; #ifdef HAVE_XML else if ( QLatin1String( "xml" ) == argv[1] ) - handler = new XMLGeoIP; + handler = new GeoIPXML; #endif if ( !handler ) From fe20416a54184ef913fab5958e53e3f6c8b4fbe9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 04:55:37 -0400 Subject: [PATCH 3/7] [locale] Make the selector configurable - GeoIP gets a string selector; the interpretation is up to derived classes. - GeoIPXML and GeoIPJSON use the selector to select an element by tag or an attribute, respectively. --- src/modules/locale/GeoIP.cpp | 5 +++++ src/modules/locale/GeoIP.h | 8 +++++++- src/modules/locale/GeoIPJSON.cpp | 19 ++++++++++++++++--- src/modules/locale/GeoIPJSON.h | 6 +++++- src/modules/locale/GeoIPTests.cpp | 3 +++ src/modules/locale/GeoIPXML.cpp | 13 ++++++++++++- src/modules/locale/GeoIPXML.h | 8 +++++++- 7 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/modules/locale/GeoIP.cpp b/src/modules/locale/GeoIP.cpp index 1bed7d3f4..f11a44db7 100644 --- a/src/modules/locale/GeoIP.cpp +++ b/src/modules/locale/GeoIP.cpp @@ -20,6 +20,11 @@ #include "utils/Logger.h" +GeoIP::GeoIP(const QString& e) + : m_element( e ) +{ +} + GeoIP::~GeoIP() { } diff --git a/src/modules/locale/GeoIP.h b/src/modules/locale/GeoIP.h index c84a9b5e4..6088c8a45 100644 --- a/src/modules/locale/GeoIP.h +++ b/src/modules/locale/GeoIP.h @@ -32,8 +32,9 @@ class QByteArray; * and can handle the data returned from its interpretation of that * configured URL, returning a region and zone. */ -struct GeoIP +class GeoIP { +public: using RegionZonePair = QPair; virtual ~GeoIP(); @@ -51,6 +52,11 @@ struct GeoIP /** @brief Splits a region/zone string into a pair. */ static RegionZonePair splitTZString( const QString& s ); + +protected: + GeoIP( const QString& e = QString() ); + + QString m_element; // string for selecting from data } ; #endif diff --git a/src/modules/locale/GeoIPJSON.cpp b/src/modules/locale/GeoIPJSON.cpp index 78f6c67a7..aa5a2b411 100644 --- a/src/modules/locale/GeoIPJSON.cpp +++ b/src/modules/locale/GeoIPJSON.cpp @@ -26,6 +26,17 @@ #include +GeoIPJSON::GeoIPJSON(const QString& attribute) + : GeoIP( attribute ) +{ +} + +GeoIPJSON::GeoIPJSON() + : GeoIPJSON( QLatin1Literal( "time_zone" ) ) +{ +} + + GeoIP::RegionZonePair GeoIPJSON::processReply( const QByteArray& data ) { @@ -39,12 +50,14 @@ GeoIPJSON::processReply( const QByteArray& data ) var.type() == QVariant::Map ) { QVariantMap map = var.toMap(); - if ( map.contains( "time_zone" ) && - !map.value( "time_zone" ).toString().isEmpty() ) + if ( map.contains( m_element ) && + !map.value( m_element ).toString().isEmpty() ) { - return splitTZString( map.value( "time_zone" ).toString() ); + return splitTZString( map.value( m_element ).toString() ); } } + else + cWarning() << "Invalid YAML data for GeoIPJSON"; } catch ( YAML::Exception& e ) { diff --git a/src/modules/locale/GeoIPJSON.h b/src/modules/locale/GeoIPJSON.h index 26a0560f6..dbe1eeffa 100644 --- a/src/modules/locale/GeoIPJSON.h +++ b/src/modules/locale/GeoIPJSON.h @@ -28,8 +28,12 @@ * * The data is assumed to be in JSON format with a time_zone attribute. */ -struct GeoIPJSON : public GeoIP +class GeoIPJSON : public GeoIP { +public: + explicit GeoIPJSON( const QString& attribute ); + explicit GeoIPJSON(); + virtual RegionZonePair processReply( const QByteArray& ); } ; diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp index d4522836c..4848f1a2d 100644 --- a/src/modules/locale/GeoIPTests.cpp +++ b/src/modules/locale/GeoIPTests.cpp @@ -76,6 +76,9 @@ GeoIPTests::testJSONbad() tz = handler.processReply( "404 Forbidden" ); QCOMPARE( tz.first, QString() ); + + tz = handler.processReply( "{ time zone = 'America/LosAngeles'}" ); + QCOMPARE( tz.first, QString() ); } diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp index 28761e163..a2117b2f2 100644 --- a/src/modules/locale/GeoIPXML.cpp +++ b/src/modules/locale/GeoIPXML.cpp @@ -23,6 +23,17 @@ #include #include +GeoIPXML::GeoIPXML( const QString& element ) + : GeoIP( element ) +{ +} + +GeoIPXML::GeoIPXML() + : GeoIPXML( QLatin1Literal( "TimeZone" ) ) +{ +} + + GeoIP::RegionZonePair GeoIPXML::processReply( const QByteArray& data ) { @@ -32,7 +43,7 @@ GeoIPXML::processReply( const QByteArray& data ) QDomDocument doc; if ( doc.setContent( data, false, &domError, &errorLine, &errorColumn ) ) { - const auto tzElements = doc.elementsByTagName( "TimeZone" ); + const auto tzElements = doc.elementsByTagName( m_element ); cDebug() << "GeoIP found" << tzElements.length() << "elements"; for ( int it = 0; it < tzElements.length(); ++it ) { diff --git a/src/modules/locale/GeoIPXML.h b/src/modules/locale/GeoIPXML.h index adffc5bca..bda359485 100644 --- a/src/modules/locale/GeoIPXML.h +++ b/src/modules/locale/GeoIPXML.h @@ -28,8 +28,14 @@ * element, which contains the text (string) for the region/zone. This * format is expected by, e.g. the Ubiquity installer. */ -struct GeoIPXML : public GeoIP +class GeoIPXML : public GeoIP { +public: + /** @brief Configure the element name which is selected. */ + explicit GeoIPXML( const QString& element ); + /** @brief Use default TimeZone element. */ + explicit GeoIPXML(); + virtual RegionZonePair processReply( const QByteArray& ); } ; From b1b59b27b277d9d294da5c16eece56754395a951 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 05:05:06 -0400 Subject: [PATCH 4/7] [locale] Expand tests for alternate selectors - Check that the alternate selectors are used --- src/modules/locale/GeoIPTests.cpp | 42 ++++++++++++++++++++++++------- src/modules/locale/GeoIPTests.h | 2 ++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp index 4848f1a2d..554e6f0c0 100644 --- a/src/modules/locale/GeoIPTests.cpp +++ b/src/modules/locale/GeoIPTests.cpp @@ -40,14 +40,14 @@ GeoIPTests::initTestCase() { } +static const char json_data_attribute[] = + "{\"time_zone\":\"Europe/Amsterdam\"}"; + void GeoIPTests::testJSON() { - static const char data[] = - "{\"time_zone\":\"Europe/Amsterdam\"}"; - GeoIPJSON handler; - auto tz = handler.processReply( data ); + auto tz = handler.processReply( json_data_attribute ); QCOMPARE( tz.first, QLatin1String( "Europe" ) ); QCOMPARE( tz.second, QLatin1String( "Amsterdam" ) ); @@ -60,6 +60,18 @@ GeoIPTests::testJSON() QCOMPARE( tz.first, "America" ); } +void GeoIPTests::testJSONalt() +{ + GeoIPJSON handler( "zona_de_hora" ); + + auto tz = handler.processReply( json_data_attribute ); + QCOMPARE( tz.first, QString() ); // Not found + + tz = handler.processReply( "tarifa: 12\nzona_de_hora: Europe/Madrid" ); + QCOMPARE( tz.first, QLatin1String( "Europe" ) ); + QCOMPARE( tz.second, QLatin1String( "Madrid" ) ); +} + void GeoIPTests::testJSONbad() { @@ -82,10 +94,7 @@ GeoIPTests::testJSONbad() } -void -GeoIPTests::testXML() -{ - static const char data[] = +static const char xml_data_ubiquity[] = R"( 85.150.1.1 OK @@ -102,9 +111,12 @@ GeoIPTests::testXML() Europe/Amsterdam )"; +void +GeoIPTests::testXML() +{ #ifdef HAVE_XML GeoIPXML handler; - auto tz = handler.processReply( data ); + auto tz = handler.processReply( xml_data_ubiquity ); QCOMPARE( tz.first, QLatin1String( "Europe" ) ); QCOMPARE( tz.second, QLatin1String( "Amsterdam" ) ); @@ -126,6 +138,18 @@ GeoIPTests::testXML2() #endif } + +void GeoIPTests::testXMLalt() +{ +#ifdef HAvE_XML + GeoIPXML handler( "ZT" ); + + auto tz = handler.processReply( "Moon/Dark_side" ); + QCOMPARE( tz.first, QLatin1String( "Moon" ) ); + QCOMPARE( tz.second, QLatin1String( "Dark_side" ) ); +#endif +} + void GeoIPTests::testXMLbad() { diff --git a/src/modules/locale/GeoIPTests.h b/src/modules/locale/GeoIPTests.h index e673a0740..87918ace0 100644 --- a/src/modules/locale/GeoIPTests.h +++ b/src/modules/locale/GeoIPTests.h @@ -31,9 +31,11 @@ public: private Q_SLOTS: void initTestCase(); void testJSON(); + void testJSONalt(); void testJSONbad(); void testXML(); void testXML2(); + void testXMLalt(); void testXMLbad(); }; From 352b385b121e447c11db80ec0d9c6b541342a084 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 05:16:23 -0400 Subject: [PATCH 5/7] [locale] Make the selector configurable via the config file --- src/modules/locale/LocaleViewStep.cpp | 7 ++++--- src/modules/locale/LocaleViewStep.h | 6 ++++-- src/modules/locale/locale.conf | 13 ++++++++++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 73121581c..4a6eb229a 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -124,16 +124,16 @@ LocaleViewStep::fetchGeoIpTimezone() if ( m_geoipStyle.isEmpty() || m_geoipStyle == "legacy" ) { actualUrl.append( "/json/" ); - handler = new GeoIPJSON; + handler = new GeoIPJSON( m_geoipSelector ); } else if ( m_geoipStyle == "json" ) { - handler = new GeoIPJSON; + handler = new GeoIPJSON( m_geoipSelector ); } #if defined(HAVE_XML) else if ( m_geoipStyle == "xml" ) { - handler = new GeoIPXML; + handler = new GeoIPXML( m_geoipSelector ); } #endif else @@ -295,4 +295,5 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) // Optional m_geoipUrl = CalamaresUtils::getString( configurationMap, "geoipUrl" ); m_geoipStyle = CalamaresUtils::getString( configurationMap, "geoipStyle" ); + m_geoipSelector = CalamaresUtils::getString( configurationMap, "geoipSelector" ); } diff --git a/src/modules/locale/LocaleViewStep.h b/src/modules/locale/LocaleViewStep.h index 003978d6a..b02f6adbd 100644 --- a/src/modules/locale/LocaleViewStep.h +++ b/src/modules/locale/LocaleViewStep.h @@ -75,8 +75,10 @@ private: QPair< QString, QString > m_startingTimezone; QString m_localeGenPath; - QString m_geoipUrl; - QString m_geoipStyle; + + QString m_geoipUrl; // The URL, depening on style might be modified on lookup + QString m_geoipStyle; // String selecting which kind of geoip data to expect + QString m_geoipSelector; // String selecting data from the geoip lookup QList< Calamares::job_ptr > m_jobs; }; diff --git a/src/modules/locale/locale.conf b/src/modules/locale/locale.conf index 327b53784..643ccd7c1 100644 --- a/src/modules/locale/locale.conf +++ b/src/modules/locale/locale.conf @@ -37,7 +37,9 @@ zone: "New_York" # the URL may be modified before use. The request should return # valid data in a suitable format, depending on geoipStyle; # generally this includes a string value with the timezone -# in / format. +# in / format. For services that return data which +# does not follow the conventions of "suitable data" described +# below, *geoIPSelector* may be used to pick different data. # # Note that this example URL works, but the service is shutting # down in June 2018. @@ -68,3 +70,12 @@ zone: "New_York" # shutting down in June 2018. There are other providers with the same # format. XML format is provided for Ubiquity. #geoipStyle: "legacy" + +# GeoIP selector. Leave commented out for the default selector +# (which depends on the style: JSON uses "time_zone" and XML +# uses TimeZone, for the FreeGeoIP-alike and the Ubiquity-alike +# respectively). If the service configured via *geoipUrl* uses +# a different attribute name (e.g. "timezone") in JSON or a +# different element tag (e.g. "") in XML, set this +# string to the name or tag to be used. +#geoipSelector: "" From fa5d40006cc22b51dd231052367684eb107e3915 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 05:27:01 -0400 Subject: [PATCH 6/7] [locale] Fix interpretation of configured selector - In GeoIP handler constructors that take a string (to configure the selector to use), interpret the empty string (which generally isn't a meaningful selector) as meaning "use the default". - Drop the no-argument constructors in favor of a default-argument which is empty. --- src/modules/locale/GeoIPJSON.cpp | 8 +------- src/modules/locale/GeoIPJSON.h | 8 ++++++-- src/modules/locale/GeoIPXML.cpp | 8 +------- src/modules/locale/GeoIPXML.h | 10 ++++++---- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/modules/locale/GeoIPJSON.cpp b/src/modules/locale/GeoIPJSON.cpp index aa5a2b411..b5cf40214 100644 --- a/src/modules/locale/GeoIPJSON.cpp +++ b/src/modules/locale/GeoIPJSON.cpp @@ -27,16 +27,10 @@ #include GeoIPJSON::GeoIPJSON(const QString& attribute) - : GeoIP( attribute ) + : GeoIP( attribute.isEmpty() ? QLatin1String( "time_zone" ) : attribute ) { } -GeoIPJSON::GeoIPJSON() - : GeoIPJSON( QLatin1Literal( "time_zone" ) ) -{ -} - - GeoIP::RegionZonePair GeoIPJSON::processReply( const QByteArray& data ) { diff --git a/src/modules/locale/GeoIPJSON.h b/src/modules/locale/GeoIPJSON.h index dbe1eeffa..3c08f577b 100644 --- a/src/modules/locale/GeoIPJSON.h +++ b/src/modules/locale/GeoIPJSON.h @@ -31,8 +31,12 @@ class GeoIPJSON : public GeoIP { public: - explicit GeoIPJSON( const QString& attribute ); - explicit GeoIPJSON(); + /** @brief Configure the attribute name which is selected. + * + * If an empty string is passed in (not a valid attribute name), + * then "time_zone" is used. + */ + explicit GeoIPJSON( const QString& attribute = QString() ); virtual RegionZonePair processReply( const QByteArray& ); } ; diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp index a2117b2f2..a9aa43f76 100644 --- a/src/modules/locale/GeoIPXML.cpp +++ b/src/modules/locale/GeoIPXML.cpp @@ -24,16 +24,10 @@ #include GeoIPXML::GeoIPXML( const QString& element ) - : GeoIP( element ) + : GeoIP( element.isEmpty() ? QLatin1String( "TimeZone" ) : element ) { } -GeoIPXML::GeoIPXML() - : GeoIPXML( QLatin1Literal( "TimeZone" ) ) -{ -} - - GeoIP::RegionZonePair GeoIPXML::processReply( const QByteArray& data ) { diff --git a/src/modules/locale/GeoIPXML.h b/src/modules/locale/GeoIPXML.h index bda359485..bc3f23bec 100644 --- a/src/modules/locale/GeoIPXML.h +++ b/src/modules/locale/GeoIPXML.h @@ -31,10 +31,12 @@ class GeoIPXML : public GeoIP { public: - /** @brief Configure the element name which is selected. */ - explicit GeoIPXML( const QString& element ); - /** @brief Use default TimeZone element. */ - explicit GeoIPXML(); + /** @brief Configure the element tag which is selected. + * + * If an empty string is passed in (not a valid element tag), + * then "TimeZone" is used. + */ + explicit GeoIPXML( const QString& element = QString() ); virtual RegionZonePair processReply( const QByteArray& ); } ; From d04e243c4e1ce57237516de51d605a089c96d3d9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 16 Apr 2018 05:45:48 -0400 Subject: [PATCH 7/7] [locale] Auto-clean up time zone data - Some providers return weirdly escaped data; strip out useless escaping before splitting (there are no characters in correct time zone names that need escaping) - Add some tests for TZ splitting --- src/modules/locale/GeoIP.cpp | 5 ++++- src/modules/locale/GeoIPTests.cpp | 24 ++++++++++++++++++++++++ src/modules/locale/GeoIPTests.h | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/modules/locale/GeoIP.cpp b/src/modules/locale/GeoIP.cpp index f11a44db7..3001273bb 100644 --- a/src/modules/locale/GeoIP.cpp +++ b/src/modules/locale/GeoIP.cpp @@ -30,8 +30,11 @@ GeoIP::~GeoIP() } GeoIP::RegionZonePair -GeoIP::splitTZString( const QString& timezoneString ) +GeoIP::splitTZString( const QString& tz ) { + QString timezoneString( tz ); + timezoneString.remove( '\\' ); + QStringList tzParts = timezoneString.split( '/', QString::SkipEmptyParts ); if ( tzParts.size() >= 2 ) { diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp index 554e6f0c0..5b5c27bff 100644 --- a/src/modules/locale/GeoIPTests.cpp +++ b/src/modules/locale/GeoIPTests.cpp @@ -165,3 +165,27 @@ GeoIPTests::testXMLbad() QCOMPARE( tz.first, QString() ); #endif } + +void GeoIPTests::testSplitTZ() +{ + auto tz = GeoIP::splitTZString( QLatin1String("Moon/Dark_side") ); + QCOMPARE( tz.first, QLatin1String("Moon") ); + QCOMPARE( tz.second, QLatin1String("Dark_side") ); + + // Some providers return weirdly escaped data + tz = GeoIP::splitTZString( QLatin1String("America\\/NewYork") ); + QCOMPARE( tz.first, QLatin1String("America") ); + QCOMPARE( tz.second, QLatin1String("NewYork") ); // That's not actually the zone name + + // Check that bogus data fails + tz = GeoIP::splitTZString( QString() ); + QCOMPARE( tz.first, QString() ); + + tz = GeoIP::splitTZString( QLatin1String("America.NewYork") ); + QCOMPARE( tz.first, QString() ); + + // Check that three-level is split properly + tz = GeoIP::splitTZString( QLatin1String("America/North Dakota/Beulah") ); + QCOMPARE( tz.first, QLatin1String("America") ); + QCOMPARE( tz.second, QLatin1String("North Dakota/Beulah") ); +} diff --git a/src/modules/locale/GeoIPTests.h b/src/modules/locale/GeoIPTests.h index 87918ace0..7aaefee81 100644 --- a/src/modules/locale/GeoIPTests.h +++ b/src/modules/locale/GeoIPTests.h @@ -37,6 +37,7 @@ private Q_SLOTS: void testXML2(); void testXMLalt(); void testXMLbad(); + void testSplitTZ(); }; #endif