From 06e43a731278edd894319bb04ce5b91587bc57cc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Apr 2018 11:20:40 -0400 Subject: [PATCH 01/22] CI: Switch to stable Neon, to avoid all-builds-failing. The dev-unstable Neon branch now has a too-new KPMCore for this branch of Calamares. --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index cd0e4f365..a4d424f45 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,2 +1,2 @@ -FROM kdeneon/all +FROM kdeneon/all:dev-stable RUN sudo apt-get update && sudo apt-get -y install build-essential cmake extra-cmake-modules gettext kio-dev libatasmart-dev libboost-python-dev libkf5config-dev libkf5coreaddons-dev libkf5i18n-dev libkf5iconthemes-dev libkf5parts-dev libkf5service-dev libkf5solid-dev libkpmcore-dev libparted-dev libpolkit-qt5-1-dev libqt5svg5-dev libqt5webkit5-dev libyaml-cpp-dev os-prober pkg-config python3-dev qtbase5-dev qtdeclarative5-dev qttools5-dev qttools5-dev-tools From 5ab01eba9f0ba5e5ec73d8abd4f6c6af8927c057 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Apr 2018 11:26:43 -0400 Subject: [PATCH 02/22] [plasmalnf] Don't even try to load an empty filename for screenshot. - Avoid one attempt-to-load if the filename is empty, and one re-creating of the Pixmap. --- src/modules/plasmalnf/ThemeWidget.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/modules/plasmalnf/ThemeWidget.cpp b/src/modules/plasmalnf/ThemeWidget.cpp index 125085db4..9f892c6d9 100644 --- a/src/modules/plasmalnf/ThemeWidget.cpp +++ b/src/modules/plasmalnf/ThemeWidget.cpp @@ -42,13 +42,8 @@ ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) qMax(12 * CalamaresUtils::defaultFontHeight(), 120), qMax(8 * CalamaresUtils::defaultFontHeight(), 80) }; - QPixmap image( info.imagePath ); - if ( info.imagePath.isEmpty() ) - { - // Image can't possibly be valid - image = QPixmap( ":/view-preview.png" ); - } - else if ( image.isNull() ) + QPixmap image( info.imagePath.isEmpty() ? ":/view-preview.png" : info.imagePath ); + if ( image.isNull() ) { // Not found or not specified, so convert the name into some (horrible, likely) // color instead. From cb616ec1bbd1d8eb0e67d5590bba3daa00c548fd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Apr 2018 11:38:34 -0400 Subject: [PATCH 03/22] [plasmalnf] Keep fixed size of screenshots (relative to font size) --- src/modules/plasmalnf/ThemeWidget.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/plasmalnf/ThemeWidget.cpp b/src/modules/plasmalnf/ThemeWidget.cpp index 9f892c6d9..6f0cc5fdb 100644 --- a/src/modules/plasmalnf/ThemeWidget.cpp +++ b/src/modules/plasmalnf/ThemeWidget.cpp @@ -57,6 +57,8 @@ ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) QLabel* image_label = new QLabel( this ); image_label->setPixmap( image ); + image_label->setMinimumSize( image_size ); + image_label->setMaximumSize( image_size ); layout->addWidget( image_label, 1 ); layout->addWidget( m_description, 3 ); From bfb37e6b374350dba5d0cbe07db65103c0f7ed3f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Apr 2018 11:39:45 -0400 Subject: [PATCH 04/22] [plasmalnf] Avoid use of 'uint' --- src/modules/plasmalnf/ThemeWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/plasmalnf/ThemeWidget.cpp b/src/modules/plasmalnf/ThemeWidget.cpp index 6f0cc5fdb..6a09dceb9 100644 --- a/src/modules/plasmalnf/ThemeWidget.cpp +++ b/src/modules/plasmalnf/ThemeWidget.cpp @@ -48,7 +48,7 @@ ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) // Not found or not specified, so convert the name into some (horrible, likely) // color instead. image = QPixmap( image_size ); - uint hash_color = qHash( info.imagePath.isEmpty() ? info.id : info.imagePath ); + auto hash_color = qHash( info.imagePath.isEmpty() ? info.id : info.imagePath ); cDebug() << "Theme image" << info.imagePath << "not found, hash" << hash_color; image.fill( QColor( QRgb( hash_color ) ) ); } From fa933b9a16f54604294e53f1fabb00e0c759e0f9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 10 Apr 2018 08:54:47 -0400 Subject: [PATCH 05/22] [plasmalnf] Search for theme screenshots - Search in branding dir, and ., for relative paths, - Absolute paths used as-is. - Document search as such. --- src/modules/plasmalnf/ThemeWidget.cpp | 31 ++++++++++++++++++++++++++- src/modules/plasmalnf/plasmalnf.conf | 4 ++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/modules/plasmalnf/ThemeWidget.cpp b/src/modules/plasmalnf/ThemeWidget.cpp index 6a09dceb9..eb4e5c356 100644 --- a/src/modules/plasmalnf/ThemeWidget.cpp +++ b/src/modules/plasmalnf/ThemeWidget.cpp @@ -22,10 +22,39 @@ #include "utils/CalamaresUtilsGui.h" #include "utils/Logger.h" +#include "Branding.h" +#include +#include #include #include #include +#include + +/** + * Massage the given @p path to the most-likely + * path that actually contains a screenshot. For + * empty image paths, returns the QRC path for an + * empty screenshot. Returns blank if the path + * doesn't exist anywhere in the search paths. + */ +static QString _munge_imagepath( const QString& path ) +{ + if ( path.isEmpty() ) + return ":/view-preview.png"; + + if ( path.startsWith( '/' ) ) + return path; + + if ( QFileInfo::exists( path ) ) + return path; + + QFileInfo fi( QDir( Calamares::Branding::instance()->componentDirectory() ), path ); + if ( fi.exists() ) + return fi.absoluteFilePath(); + + return QString(); +} ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) : QWidget( parent ) @@ -42,7 +71,7 @@ ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) qMax(12 * CalamaresUtils::defaultFontHeight(), 120), qMax(8 * CalamaresUtils::defaultFontHeight(), 80) }; - QPixmap image( info.imagePath.isEmpty() ? ":/view-preview.png" : info.imagePath ); + QPixmap image( _munge_imagepath( info.imagePath ) ); if ( image.isNull() ) { // Not found or not specified, so convert the name into some (horrible, likely) diff --git a/src/modules/plasmalnf/plasmalnf.conf b/src/modules/plasmalnf/plasmalnf.conf index 85df64f0a..a954c685a 100644 --- a/src/modules/plasmalnf/plasmalnf.conf +++ b/src/modules/plasmalnf/plasmalnf.conf @@ -32,6 +32,10 @@ lnftool: "/usr/bin/lookandfeeltool" # Themes with no image set at all get a "missing screenshot" image; if the # image file is not found, they get a color swatch based on the image name. # +# The image may be an absolute path. If it is a relative path, though, +# it is searched in the current directory and in the branding directory +# (i.e. relative to the directory where your branding.desc lives). +# # Valid forms of entries in the *themes* key: # - A single string (unquoted), which is the theme id # - A pair of *theme* and *image* keys, e.g. From 445f181cc3296a98e68d31ac90b8d027b0588898 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 09:45:48 -0400 Subject: [PATCH 06/22] [locale] Start refactoring geoip handling - Introduce a handler interface for GeoIP providers - Move the implementation of FreeGeoIP into a struct of its own --- src/modules/locale/CMakeLists.txt | 1 + src/modules/locale/GeoIP.h | 61 ++++++++++++++++++++++ src/modules/locale/GeoIPFreeGeoIP.cpp | 74 +++++++++++++++++++++++++++ src/modules/locale/GeoIPFreeGeoIP.h | 30 +++++++++++ src/modules/locale/LocaleViewStep.cpp | 56 +++++++------------- 5 files changed, 185 insertions(+), 37 deletions(-) create mode 100644 src/modules/locale/GeoIP.h create mode 100644 src/modules/locale/GeoIPFreeGeoIP.cpp create mode 100644 src/modules/locale/GeoIPFreeGeoIP.h diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index e32f6e613..f35a9b4bd 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -4,6 +4,7 @@ calamares_add_plugin( locale TYPE viewmodule EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES + GeoIPFreeGeoIP.cpp LCLocaleDialog.cpp LocaleConfiguration.cpp LocalePage.cpp diff --git a/src/modules/locale/GeoIP.h b/src/modules/locale/GeoIP.h new file mode 100644 index 000000000..e9e0fadfd --- /dev/null +++ b/src/modules/locale/GeoIP.h @@ -0,0 +1,61 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef GEOIP_H +#define GEOIP_H + +#include +#include +#include + +class QNetworkReply; + +/** + * @brief Interface for GeoIP retrievers. + * + * A GeoIP retriever takes a configured URL (from the config file) + * and can handle the data returned from its interpretation of that + * configured URL, returning a region and zone. + */ +struct GeoIP +{ + using RegionZonePair = QPair; + + /** @brief Convert configured URL to a complete URL. + * + * Some GeoIP providers are configured with one URL, but actually + * do retrieval with another (e.g. when using the original + * implementation of FreeGeoIP, or when adding an API key). + */ + virtual QUrl fullUrl( const QString& configUrl ) = 0; + + /** @brief Handle a (successful) request by interpreting the data. + * + * Should return a ( , ) pair, e.g. + * ( "Europe", "Amsterdam" ). This is called **only** if the + * request to the fullUrl was successful; the handler + * is free to read as much, or as little, data as it + * likes. On error, returns a RegionZonePair with empty + * strings (e.g. ( "", "" ) ). + */ + virtual RegionZonePair processReply( QNetworkReply* ) = 0; + + virtual ~GeoIP(); // Defined in LocaleViewStep +} ; + +#endif diff --git a/src/modules/locale/GeoIPFreeGeoIP.cpp b/src/modules/locale/GeoIPFreeGeoIP.cpp new file mode 100644 index 000000000..be77dc2e4 --- /dev/null +++ b/src/modules/locale/GeoIPFreeGeoIP.cpp @@ -0,0 +1,74 @@ +/* === This file is part of Calamares - === + * + * Copyright 2014-2016, Teo Mrnjavac + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "GeoIPFreeGeoIP.h" + +#include "utils/Logger.h" +#include "utils/YamlUtils.h" + +#include + +#include + +QUrl +FreeGeoIP::fullUrl( const QString& configUrl ) +{ + // FIXME: derpy way to append "/json" to the user-specified config URL + QString requestUrl = QString( "%1/json" ) + .arg( QUrl::fromUserInput( configUrl ).toString() ); + return QUrl( requestUrl ); +} + +GeoIP::RegionZonePair +FreeGeoIP::processReply( QNetworkReply* reply ) +{ + QByteArray data = reply->readAll(); + + try + { + YAML::Node doc = YAML::Load( data ); + + QVariant var = CalamaresUtils::yamlToVariant( doc ); + if ( !var.isNull() && + var.isValid() && + var.type() == QVariant::Map ) + { + QVariantMap map = var.toMap(); + if ( map.contains( "time_zone" ) && + !map.value( "time_zone" ).toString().isEmpty() ) + { + QString timezoneString = map.value( "time_zone" ).toString(); + QStringList tzParts = timezoneString.split( '/', QString::SkipEmptyParts ); + if ( tzParts.size() >= 2 ) + { + cDebug() << "GeoIP reporting" << timezoneString; + QString region = tzParts.takeFirst(); + QString zone = tzParts.join( '/' ); + return qMakePair( region, zone ); + } + } + } + } + catch ( YAML::Exception& e ) + { + CalamaresUtils::explainYamlException( e, data, "GeoIP data"); + } + + return qMakePair( QString(), QString() ); +} diff --git a/src/modules/locale/GeoIPFreeGeoIP.h b/src/modules/locale/GeoIPFreeGeoIP.h new file mode 100644 index 000000000..3fe6157a2 --- /dev/null +++ b/src/modules/locale/GeoIPFreeGeoIP.h @@ -0,0 +1,30 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef GEOIPFREEGEOIP_H +#define GEOIPFREEGEOIP_H + +#include "GeoIP.h" + +struct FreeGeoIP : public GeoIP +{ + virtual QUrl fullUrl( const QString& configUrl ); + virtual RegionZonePair processReply( QNetworkReply* ); +} ; + +#endif diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 73efc266f..d9f5d1338 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014-2016, Teo Mrnjavac + * Copyright 2018, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -18,11 +19,14 @@ #include "LocaleViewStep.h" +#include "GeoIP.h" +#include "GeoIPFreeGeoIP.h" +#include "GlobalStorage.h" +#include "JobQueue.h" #include "LocalePage.h" + #include "timezonewidget/localeglobal.h" #include "widgets/WaitingWidget.h" -#include "JobQueue.h" -#include "GlobalStorage.h" #include "utils/CalamaresUtilsGui.h" #include "utils/Logger.h" @@ -111,53 +115,25 @@ void LocaleViewStep::fetchGeoIpTimezone() { QNetworkAccessManager *manager = new QNetworkAccessManager( this ); + GeoIP *handler = new FreeGeoIP; + connect( manager, &QNetworkAccessManager::finished, [=]( QNetworkReply* reply ) { if ( reply->error() == QNetworkReply::NoError ) { - QByteArray data = reply->readAll(); - - try - { - YAML::Node doc = YAML::Load( data ); - - QVariant var = CalamaresUtils::yamlToVariant( doc ); - if ( !var.isNull() && - var.isValid() && - var.type() == QVariant::Map ) - { - QVariantMap map = var.toMap(); - if ( map.contains( "time_zone" ) && - !map.value( "time_zone" ).toString().isEmpty() ) - { - QString timezoneString = map.value( "time_zone" ).toString(); - QStringList tzParts = timezoneString.split( '/', QString::SkipEmptyParts ); - if ( tzParts.size() >= 2 ) - { - cDebug() << "GeoIP reporting" << timezoneString; - QString region = tzParts.takeFirst(); - QString zone = tzParts.join( '/' ); - m_startingTimezone = qMakePair( region, zone ); - } - } - } - } - catch ( YAML::Exception& e ) - { - CalamaresUtils::explainYamlException( e, data, "GeoIP data"); - } + auto tz = handler->processReply( reply ); + if ( !tz.first.isEmpty() ) + m_startingTimezone = tz; } - + delete handler; reply->deleteLater(); manager->deleteLater(); setUpPage(); } ); QNetworkRequest request; - QString requestUrl = QString( "%1/json" ) - .arg( QUrl::fromUserInput( m_geoipUrl ).toString() ); - request.setUrl( QUrl( requestUrl ) ); + request.setUrl( handler->fullUrl( m_geoipUrl ) ); request.setAttribute( QNetworkRequest::FollowRedirectsAttribute, true ); manager->get( request ); } @@ -294,3 +270,9 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) m_geoipUrl = configurationMap.value( "geoipUrl" ).toString(); } } + + +// Defined here since the struct has nothing else in it +GeoIP::~GeoIP() +{ +} From c0d5a153d4499fc283fcc56b2664e1ef84db2bac Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 09:54:22 -0400 Subject: [PATCH 07/22] [locale] Refactor GeoIP handler - Move GeoIP to its own cpp file - Provide a default implementation of the URL mangler --- src/modules/locale/CMakeLists.txt | 1 + src/modules/locale/GeoIP.cpp | 29 +++++++++++++++++++++++++++ src/modules/locale/GeoIP.h | 6 ++++-- src/modules/locale/LocaleViewStep.cpp | 6 ------ 4 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 src/modules/locale/GeoIP.cpp diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index f35a9b4bd..774868a28 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -4,6 +4,7 @@ calamares_add_plugin( locale TYPE viewmodule EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES + GeoIP.cpp GeoIPFreeGeoIP.cpp LCLocaleDialog.cpp LocaleConfiguration.cpp diff --git a/src/modules/locale/GeoIP.cpp b/src/modules/locale/GeoIP.cpp new file mode 100644 index 000000000..d20d398c7 --- /dev/null +++ b/src/modules/locale/GeoIP.cpp @@ -0,0 +1,29 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "GeoIP.h" + +GeoIP::~GeoIP() +{ +} + +QUrl +GeoIP::fullUrl(const QString& configUrl) +{ + return QUrl::fromUserInput( configUrl ); +} diff --git a/src/modules/locale/GeoIP.h b/src/modules/locale/GeoIP.h index e9e0fadfd..6d4992781 100644 --- a/src/modules/locale/GeoIP.h +++ b/src/modules/locale/GeoIP.h @@ -41,8 +41,10 @@ struct GeoIP * Some GeoIP providers are configured with one URL, but actually * do retrieval with another (e.g. when using the original * implementation of FreeGeoIP, or when adding an API key). + * + * The default implementation uses the @p configUrl unchanged. */ - virtual QUrl fullUrl( const QString& configUrl ) = 0; + virtual QUrl fullUrl( const QString& configUrl ); /** @brief Handle a (successful) request by interpreting the data. * @@ -55,7 +57,7 @@ struct GeoIP */ virtual RegionZonePair processReply( QNetworkReply* ) = 0; - virtual ~GeoIP(); // Defined in LocaleViewStep + virtual ~GeoIP(); } ; #endif diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index d9f5d1338..bd9bef2fe 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -270,9 +270,3 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) m_geoipUrl = configurationMap.value( "geoipUrl" ).toString(); } } - - -// Defined here since the struct has nothing else in it -GeoIP::~GeoIP() -{ -} From d5623af8efb6f0e00b172254be067fd0d14265d4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 10:11:48 -0400 Subject: [PATCH 08/22] [locale] Refactor geoip handling - Configuration **must** be a complete URL. The implementation no longer appends /json to the URL. --- src/modules/locale/GeoIP.cpp | 6 ------ src/modules/locale/GeoIP.h | 10 ---------- src/modules/locale/GeoIPFreeGeoIP.cpp | 9 --------- src/modules/locale/GeoIPFreeGeoIP.h | 9 ++++++++- src/modules/locale/LocaleViewStep.cpp | 2 +- 5 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/modules/locale/GeoIP.cpp b/src/modules/locale/GeoIP.cpp index d20d398c7..5ada6e10c 100644 --- a/src/modules/locale/GeoIP.cpp +++ b/src/modules/locale/GeoIP.cpp @@ -21,9 +21,3 @@ GeoIP::~GeoIP() { } - -QUrl -GeoIP::fullUrl(const QString& configUrl) -{ - return QUrl::fromUserInput( configUrl ); -} diff --git a/src/modules/locale/GeoIP.h b/src/modules/locale/GeoIP.h index 6d4992781..f05c4d383 100644 --- a/src/modules/locale/GeoIP.h +++ b/src/modules/locale/GeoIP.h @@ -36,16 +36,6 @@ struct GeoIP { using RegionZonePair = QPair; - /** @brief Convert configured URL to a complete URL. - * - * Some GeoIP providers are configured with one URL, but actually - * do retrieval with another (e.g. when using the original - * implementation of FreeGeoIP, or when adding an API key). - * - * The default implementation uses the @p configUrl unchanged. - */ - virtual QUrl fullUrl( const QString& configUrl ); - /** @brief Handle a (successful) request by interpreting the data. * * Should return a ( , ) pair, e.g. diff --git a/src/modules/locale/GeoIPFreeGeoIP.cpp b/src/modules/locale/GeoIPFreeGeoIP.cpp index be77dc2e4..354e6c011 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.cpp +++ b/src/modules/locale/GeoIPFreeGeoIP.cpp @@ -26,15 +26,6 @@ #include -QUrl -FreeGeoIP::fullUrl( const QString& configUrl ) -{ - // FIXME: derpy way to append "/json" to the user-specified config URL - QString requestUrl = QString( "%1/json" ) - .arg( QUrl::fromUserInput( configUrl ).toString() ); - return QUrl( requestUrl ); -} - GeoIP::RegionZonePair FreeGeoIP::processReply( QNetworkReply* reply ) { diff --git a/src/modules/locale/GeoIPFreeGeoIP.h b/src/modules/locale/GeoIPFreeGeoIP.h index 3fe6157a2..474ce8496 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.h +++ b/src/modules/locale/GeoIPFreeGeoIP.h @@ -21,9 +21,16 @@ #include "GeoIP.h" +/** @brief GeoIP lookup via freegeoip.com + * + * This is the original implementation of GeoIP lookup, + * using the FreeGeoIP service, or similar which returns + * data in the same format. + * + * The data is assumed to be in JSON format with a time_zone attribute. + */ struct FreeGeoIP : public GeoIP { - virtual QUrl fullUrl( const QString& configUrl ); virtual RegionZonePair processReply( QNetworkReply* ); } ; diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index bd9bef2fe..7eaaebbf8 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -133,7 +133,7 @@ LocaleViewStep::fetchGeoIpTimezone() } ); QNetworkRequest request; - request.setUrl( handler->fullUrl( m_geoipUrl ) ); + request.setUrl( QUrl::fromUserInput( m_geoipUrl ) ); request.setAttribute( QNetworkRequest::FollowRedirectsAttribute, true ); manager->get( request ); } From fe98b789f0bb6714a13de30ca056267de62950df Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 29 Mar 2018 16:50:02 -0400 Subject: [PATCH 09/22] [locale] Document the settings in locale.conf - The geoipUrl is weird, because it is not a complete URL. Document that, and what kind of data is expected. FIXES #920 --- src/modules/locale/locale.conf | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/modules/locale/locale.conf b/src/modules/locale/locale.conf index 824c8abeb..88148f47a 100644 --- a/src/modules/locale/locale.conf +++ b/src/modules/locale/locale.conf @@ -1,7 +1,33 @@ --- +# The starting timezone (e.g. the pin-on-the-map) when entering +# the locale page can be set through keys *region* and *zone*. +# If either is not set, defaults to America/New_York. +# region: "America" zone: "New_York" -# GeoIP settings. Leave commented out to disable GeoIP. +# Some distros come with a meaningfully commented and easy to parse +# `/etc/locale.gen`, and others ship a separate file +# `/usr/share/i18n/SUPPORTED` with a clean list of supported locales. +# We first try SUPPORTED, and if it doesn't exist, we fall back +# to parsing the lines from `locale.gen`. For distro's that ship +# the `locale.gen` file installed elsewhere, the key *localeGenPath* +# can be used to specify where it is. If not set, the default +# `/etc/locale.gen` is used. +# #localeGenPath: "/etc/locale.gen" + +# GeoIP settings. Leave commented out to disable GeoIP. +# +# An HTTP request is made to http://*geoipUrl*/json (which just happens +# to be the GET path needed by freegeoip.net, so calling this a URL +# is a stretch). The request must return valid JSON data; there should +# be an attribute *time_zone*, with a string value set to the +# timezone, in / form. +# +# Suitable data looks like +# ``` +# {"time_zone":"America/New_York"} +# ``` +# #geoipUrl: "freegeoip.net" From 363622642592561dc773d7828c7fe4484c84d239 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 10:18:15 -0400 Subject: [PATCH 10/22] [locale] Document change to the way GeoIPURL is handled. --- src/modules/locale/locale.conf | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/modules/locale/locale.conf b/src/modules/locale/locale.conf index 88148f47a..c51a5d1bc 100644 --- a/src/modules/locale/locale.conf +++ b/src/modules/locale/locale.conf @@ -19,15 +19,18 @@ zone: "New_York" # GeoIP settings. Leave commented out to disable GeoIP. # -# An HTTP request is made to http://*geoipUrl*/json (which just happens -# to be the GET path needed by freegeoip.net, so calling this a URL -# is a stretch). The request must return valid JSON data; there should +# An HTTP request is made to *geoipUrl* -- prior to Calamares 3.1.13, +# an implicit "/json" was added at the end.. The request must return +# valid JSON data in the FreeGeoIP format; there should # be an attribute *time_zone*, with a string value set to the -# timezone, in / form. +# timezone, in / format. +# +# Note that this example URL works, but the service is shutting +# down in June 2018. # # Suitable data looks like # ``` # {"time_zone":"America/New_York"} # ``` # -#geoipUrl: "freegeoip.net" +#geoipUrl: "freegeoip.net/json" From aaae1507cda8a0a80d384099f3ab776765d6b81a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 11:51:50 -0400 Subject: [PATCH 11/22] [locale] Convenience function for TZ splitting --- src/modules/locale/GeoIP.cpp | 17 +++++++++++++++++ src/modules/locale/GeoIP.h | 5 ++++- src/modules/locale/GeoIPFreeGeoIP.cpp | 10 +--------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/modules/locale/GeoIP.cpp b/src/modules/locale/GeoIP.cpp index 5ada6e10c..1bed7d3f4 100644 --- a/src/modules/locale/GeoIP.cpp +++ b/src/modules/locale/GeoIP.cpp @@ -18,6 +18,23 @@ #include "GeoIP.h" +#include "utils/Logger.h" + GeoIP::~GeoIP() { } + +GeoIP::RegionZonePair +GeoIP::splitTZString( const QString& timezoneString ) +{ + QStringList tzParts = timezoneString.split( '/', QString::SkipEmptyParts ); + if ( tzParts.size() >= 2 ) + { + cDebug() << "GeoIP reporting" << timezoneString; + QString region = tzParts.takeFirst(); + QString zone = tzParts.join( '/' ); + return qMakePair( region, zone ); + } + + return qMakePair( QString(), QString() ); +} diff --git a/src/modules/locale/GeoIP.h b/src/modules/locale/GeoIP.h index f05c4d383..b3d84c118 100644 --- a/src/modules/locale/GeoIP.h +++ b/src/modules/locale/GeoIP.h @@ -36,6 +36,8 @@ struct GeoIP { using RegionZonePair = QPair; + virtual ~GeoIP(); + /** @brief Handle a (successful) request by interpreting the data. * * Should return a ( , ) pair, e.g. @@ -47,7 +49,8 @@ struct GeoIP */ virtual RegionZonePair processReply( QNetworkReply* ) = 0; - virtual ~GeoIP(); + /** @brief Splits a region/zone string into a pair. */ + static RegionZonePair splitTZString( const QString& s ); } ; #endif diff --git a/src/modules/locale/GeoIPFreeGeoIP.cpp b/src/modules/locale/GeoIPFreeGeoIP.cpp index 354e6c011..d7e724145 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.cpp +++ b/src/modules/locale/GeoIPFreeGeoIP.cpp @@ -44,15 +44,7 @@ FreeGeoIP::processReply( QNetworkReply* reply ) if ( map.contains( "time_zone" ) && !map.value( "time_zone" ).toString().isEmpty() ) { - QString timezoneString = map.value( "time_zone" ).toString(); - QStringList tzParts = timezoneString.split( '/', QString::SkipEmptyParts ); - if ( tzParts.size() >= 2 ) - { - cDebug() << "GeoIP reporting" << timezoneString; - QString region = tzParts.takeFirst(); - QString zone = tzParts.join( '/' ); - return qMakePair( region, zone ); - } + return splitTZString( map.value( "time_zone" ).toString() ); } } } From 939cdff93b7efaf7764dcbe41a321095a37fd710 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 12:18:43 -0400 Subject: [PATCH 12/22] [locale] Add alternate GeoIP data format --- src/modules/locale/CMakeLists.txt | 13 ++++++-- src/modules/locale/GeoIPXML.cpp | 52 +++++++++++++++++++++++++++++++ src/modules/locale/GeoIPXML.h | 36 +++++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 src/modules/locale/GeoIPXML.cpp create mode 100644 src/modules/locale/GeoIPXML.h diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index 774868a28..442ca0d18 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -1,11 +1,19 @@ include_directories( ${PROJECT_BINARY_DIR}/src/libcalamaresui ) +set( geoip_src GeoIP.cpp GeoIPFreeGeoIP.cpp ) +set( geoip_libs ) + +find_package(Qt5 COMPONENTS Xml) +if( Qt5Xml_FOUND ) + list( APPEND geoip_src GeoIPXML.cpp ) + list( APPEND geoip_libs Qt5::Xml ) +endif() + calamares_add_plugin( locale TYPE viewmodule EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES - GeoIP.cpp - GeoIPFreeGeoIP.cpp + ${geoip_src} LCLocaleDialog.cpp LocaleConfiguration.cpp LocalePage.cpp @@ -19,6 +27,7 @@ calamares_add_plugin( locale LINK_PRIVATE_LIBRARIES calamaresui Qt5::Network + ${geoip_libs} ${YAMLCPP_LIBRARY} SHARED_LIB ) diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp new file mode 100644 index 000000000..0b52c9612 --- /dev/null +++ b/src/modules/locale/GeoIPXML.cpp @@ -0,0 +1,52 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "GeoIPXML.h" + +#include "utils/Logger.h" + +#include +#include + +GeoIP::RegionZonePair +XMLGeoIP::processReply( QNetworkReply* reply ) +{ + QString domError; + int errorLine, errorColumn; + + QDomDocument doc; + if ( doc.setContent( reply->readAll(), false, &domError, &errorLine, &errorColumn ) ) + { + const auto tzElements = doc.elementsByTagName( "TimeZone" ); + cDebug() << "GeoIP found" << tzElements.length() << "elements"; + for ( int it = 0; it < tzElements.length(); ++it ) + { + if ( tzElements.at(it).isText() ) + { + return splitTZString( tzElements.at(it).nodeValue() ); + } + } + } + else + { + cDebug() << "GeoIP XML data error:" << domError << "(line" << errorLine << errorColumn << ')'; + } + + return qMakePair( QString(), QString() ); + +} diff --git a/src/modules/locale/GeoIPXML.h b/src/modules/locale/GeoIPXML.h new file mode 100644 index 000000000..d83a46daa --- /dev/null +++ b/src/modules/locale/GeoIPXML.h @@ -0,0 +1,36 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef GEOIPXML_H +#define GEOIPXML_H + +#include "GeoIP.h" + +/** @brief GeoIP lookup with XML data + * + * The data is assumed to be in XML format with a + * + * element, which contains the text (string) for the region/zone. This + * format is expected by, e.g. the Ubiquity installer. + */ +struct XMLGeoIP : public GeoIP +{ + virtual RegionZonePair processReply( QNetworkReply* ); +} ; + +#endif From 5b98e58ae71d09895fbc0ed56e687dab40e82607 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 12:22:43 -0400 Subject: [PATCH 13/22] [locale] Refactor GeoIP handlers - Read the data in the caller of the handler, instead of in the callers --- src/modules/locale/GeoIP.h | 4 ++-- src/modules/locale/GeoIPFreeGeoIP.cpp | 6 ++---- src/modules/locale/GeoIPFreeGeoIP.h | 2 +- src/modules/locale/GeoIPXML.cpp | 4 ++-- src/modules/locale/GeoIPXML.h | 2 +- src/modules/locale/LocaleViewStep.cpp | 2 +- 6 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/modules/locale/GeoIP.h b/src/modules/locale/GeoIP.h index b3d84c118..c84a9b5e4 100644 --- a/src/modules/locale/GeoIP.h +++ b/src/modules/locale/GeoIP.h @@ -23,7 +23,7 @@ #include #include -class QNetworkReply; +class QByteArray; /** * @brief Interface for GeoIP retrievers. @@ -47,7 +47,7 @@ struct GeoIP * likes. On error, returns a RegionZonePair with empty * strings (e.g. ( "", "" ) ). */ - virtual RegionZonePair processReply( QNetworkReply* ) = 0; + virtual RegionZonePair processReply( const QByteArray& ) = 0; /** @brief Splits a region/zone string into a pair. */ static RegionZonePair splitTZString( const QString& s ); diff --git a/src/modules/locale/GeoIPFreeGeoIP.cpp b/src/modules/locale/GeoIPFreeGeoIP.cpp index d7e724145..9ef534a5d 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.cpp +++ b/src/modules/locale/GeoIPFreeGeoIP.cpp @@ -22,15 +22,13 @@ #include "utils/Logger.h" #include "utils/YamlUtils.h" -#include +#include #include GeoIP::RegionZonePair -FreeGeoIP::processReply( QNetworkReply* reply ) +FreeGeoIP::processReply( const QByteArray& data ) { - QByteArray data = reply->readAll(); - try { YAML::Node doc = YAML::Load( data ); diff --git a/src/modules/locale/GeoIPFreeGeoIP.h b/src/modules/locale/GeoIPFreeGeoIP.h index 474ce8496..e8986db3f 100644 --- a/src/modules/locale/GeoIPFreeGeoIP.h +++ b/src/modules/locale/GeoIPFreeGeoIP.h @@ -31,7 +31,7 @@ */ struct FreeGeoIP : public GeoIP { - virtual RegionZonePair processReply( QNetworkReply* ); + virtual RegionZonePair processReply( const QByteArray& ); } ; #endif diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp index 0b52c9612..1bba559d0 100644 --- a/src/modules/locale/GeoIPXML.cpp +++ b/src/modules/locale/GeoIPXML.cpp @@ -24,13 +24,13 @@ #include GeoIP::RegionZonePair -XMLGeoIP::processReply( QNetworkReply* reply ) +XMLGeoIP::processReply( const QByteArray& data ) { QString domError; int errorLine, errorColumn; QDomDocument doc; - if ( doc.setContent( reply->readAll(), false, &domError, &errorLine, &errorColumn ) ) + if ( doc.setContent( data, false, &domError, &errorLine, &errorColumn ) ) { const auto tzElements = doc.elementsByTagName( "TimeZone" ); cDebug() << "GeoIP found" << tzElements.length() << "elements"; diff --git a/src/modules/locale/GeoIPXML.h b/src/modules/locale/GeoIPXML.h index d83a46daa..8eec22a75 100644 --- a/src/modules/locale/GeoIPXML.h +++ b/src/modules/locale/GeoIPXML.h @@ -30,7 +30,7 @@ */ struct XMLGeoIP : public GeoIP { - virtual RegionZonePair processReply( QNetworkReply* ); + virtual RegionZonePair processReply( const QByteArray& ); } ; #endif diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 7eaaebbf8..04870ff76 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -122,7 +122,7 @@ LocaleViewStep::fetchGeoIpTimezone() { if ( reply->error() == QNetworkReply::NoError ) { - auto tz = handler->processReply( reply ); + auto tz = handler->processReply( reply->readAll() ); if ( !tz.first.isEmpty() ) m_startingTimezone = tz; } From 6b7c8a694abebae5a3334d575ddce54421ac3282 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 14:37:38 -0400 Subject: [PATCH 14/22] [locale] Make the style of GeoIP retrieval selectable - Unchanged config files will continue to use the weird addition of /json, and interpret JSON data. - Allow to specify full URL with data format through one of geoipStyle: json geoipStyle: xml - XML support is optional --- src/modules/locale/CMakeLists.txt | 1 + src/modules/locale/LocaleViewStep.cpp | 36 +++++++++++++++++++++++++-- src/modules/locale/LocaleViewStep.h | 1 + src/modules/locale/locale.conf | 20 +++++++++++++-- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index 442ca0d18..376c19b00 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -7,6 +7,7 @@ find_package(Qt5 COMPONENTS Xml) if( Qt5Xml_FOUND ) list( APPEND geoip_src GeoIPXML.cpp ) list( APPEND geoip_libs Qt5::Xml ) + add_definitions( -DHAVE_XML ) endif() calamares_add_plugin( locale diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 04870ff76..09589a732 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -21,6 +21,9 @@ #include "GeoIP.h" #include "GeoIPFreeGeoIP.h" +#ifdef HAVE_XML +#include "GeoIPXML.h" +#endif #include "GlobalStorage.h" #include "JobQueue.h" #include "LocalePage.h" @@ -114,9 +117,32 @@ LocaleViewStep::setUpPage() void LocaleViewStep::fetchGeoIpTimezone() { - QNetworkAccessManager *manager = new QNetworkAccessManager( this ); - GeoIP *handler = new FreeGeoIP; + QString actualUrl( m_geoipUrl ); + GeoIP *handler = nullptr; + if ( m_geoipStyle.isEmpty() || m_geoipStyle == "legacy" ) + { + actualUrl.append( "/json" ); + handler = new FreeGeoIP; + } + else if ( m_geoipStyle == "json" ) + { + handler = new FreeGeoIP; + } +#if defined(HAVE_XML) + else if ( m_geoipStyle == "xml" ) + { + handler = new XMLGeoIP; + } +#endif + else + { + cDebug() << "WARNING: GeoIP Style" << m_geoipStyle << "is not recognized."; + setUpPage(); + return; + } + + QNetworkAccessManager *manager = new QNetworkAccessManager( this ); connect( manager, &QNetworkAccessManager::finished, [=]( QNetworkReply* reply ) { @@ -269,4 +295,10 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { m_geoipUrl = configurationMap.value( "geoipUrl" ).toString(); } + if ( configurationMap.contains( "geoipStyle" ) && + configurationMap.value( "geoipStyle" ).type() == QVariant::String && + !configurationMap.value( "geoipStyle" ).toString().isEmpty() ) + { + m_geoipStyle = configurationMap.value( "geoipStyle" ).toString(); + } } diff --git a/src/modules/locale/LocaleViewStep.h b/src/modules/locale/LocaleViewStep.h index 402fb7ce9..e6983ef15 100644 --- a/src/modules/locale/LocaleViewStep.h +++ b/src/modules/locale/LocaleViewStep.h @@ -76,6 +76,7 @@ private: QPair< QString, QString > m_startingTimezone; QString m_localeGenPath; QString m_geoipUrl; + QString m_geoipStyle; QList< Calamares::job_ptr > m_jobs; }; diff --git a/src/modules/locale/locale.conf b/src/modules/locale/locale.conf index c51a5d1bc..54a7b584c 100644 --- a/src/modules/locale/locale.conf +++ b/src/modules/locale/locale.conf @@ -19,8 +19,8 @@ zone: "New_York" # GeoIP settings. Leave commented out to disable GeoIP. # -# An HTTP request is made to *geoipUrl* -- prior to Calamares 3.1.13, -# an implicit "/json" was added at the end.. The request must return +# An HTTP request is made to *geoipUrl* -- depending on the geoipStyle, +# the URL may be modified before use. The request must return # valid JSON data in the FreeGeoIP format; there should # be an attribute *time_zone*, with a string value set to the # timezone, in / format. @@ -34,3 +34,19 @@ zone: "New_York" # ``` # #geoipUrl: "freegeoip.net/json" + +# GeoIP style. Leave commented out for the "legacy" interpretation. +# This setting only makes sense if geoipUrl is set, enabliing geoIP. +# +# Possible values are: +# unset same as "legacy" +# blank same as "legacy" +# "legacy" appends "/json" to geoipUrl, above, and uses JSON format +# (which is what freegeoip.net provides there). +# "json" URL is not modified, uses JSON format. +# "xml" URL is not modified, uses XML format. +# +# The JSON format is provided by freegeoip.net, but that service is +# shutting down in June 2018. There are other providers with the same +# format. XML format is provided for Ubiquity. +#geoipStyle: "legacy" From eea421f499f043524e1732dca53d47f7d5580e6c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 16:05:02 -0400 Subject: [PATCH 15/22] [locale] Add tests for GeoIP handlers - One sample JSON result - Two sample XML results --- src/modules/locale/CMakeLists.txt | 23 ++++++++ src/modules/locale/GeoIPTests.cpp | 98 +++++++++++++++++++++++++++++++ src/modules/locale/GeoIPTests.h | 38 ++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 src/modules/locale/GeoIPTests.cpp create mode 100644 src/modules/locale/GeoIPTests.h diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index 376c19b00..df758ac8e 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -1,3 +1,10 @@ +find_package(ECM ${ECM_VERSION} NO_MODULE) +if( ECM_FOUND ) + include( ECMAddTests ) +endif() + +find_package( Qt5 COMPONENTS Core Test REQUIRED ) + include_directories( ${PROJECT_BINARY_DIR}/src/libcalamaresui ) set( geoip_src GeoIP.cpp GeoIPFreeGeoIP.cpp ) @@ -32,3 +39,19 @@ calamares_add_plugin( locale ${YAMLCPP_LIBRARY} SHARED_LIB ) + +if( ECM_FOUND ) + ecm_add_test( + GeoIPTests.cpp + ${geoip_src} + TEST_NAME + geoiptest + LINK_LIBRARIES + calamaresui + Qt5::Network + Qt5::Test + ${geoip_libs} + ${YAMLCPP_LIBRARY} + ) + set_target_properties( geoiptest PROPERTIES AUTOMOC TRUE ) +endif() diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp new file mode 100644 index 000000000..9ad7ad7fc --- /dev/null +++ b/src/modules/locale/GeoIPTests.cpp @@ -0,0 +1,98 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "GeoIPTests.h" + +#include "GeoIPFreeGeoIP.h" +#ifdef HAVE_XML +#include "GeoIPXML.h" +#endif + +#include + +QTEST_GUILESS_MAIN( GeoIPTests ) + +GeoIPTests::GeoIPTests() +{ +} + +GeoIPTests::~GeoIPTests() +{ +} + +void +GeoIPTests::initTestCase() +{ +} + +void +GeoIPTests::testJSON() +{ + static const char data[] = + "{\"time_zone\":\"Europe/Amsterdam\"}"; + + FreeGeoIP handler; + auto tz = handler.processReply( data ); + + QCOMPARE( tz.first, QLatin1String( "Europe" ) ); + QCOMPARE( tz.second, QLatin1String( "Amsterdam" ) ); +} + +void +GeoIPTests::testXML() +{ + static const char data[] = + R"( + 85.150.1.1 + OK + NL + NLD + Netherlands + None + None + None + + 50.0 + 4.0 + 0 + Europe/Amsterdam +)"; + +#ifdef HAVE_XML + XMLGeoIP handler; + auto tz = handler.processReply( data ); + + QCOMPARE( tz.first, QLatin1String( "Europe" ) ); + QCOMPARE( tz.second, QLatin1String( "Amsterdam" ) ); +#endif +} + +void +GeoIPTests::testXML2() +{ + static const char data[] = + "America/North Dakota/Beulah"; + +#ifdef HAVE_XML + XMLGeoIP handler; + auto tz = handler.processReply( data ); + + QCOMPARE( tz.first, QLatin1String( "America" ) ); + QCOMPARE( tz.second, QLatin1String( "North Dakota/Beulah" ) ); +#endif +} diff --git a/src/modules/locale/GeoIPTests.h b/src/modules/locale/GeoIPTests.h new file mode 100644 index 000000000..b153d5c5f --- /dev/null +++ b/src/modules/locale/GeoIPTests.h @@ -0,0 +1,38 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef GEOIPTESTS_H +#define GEOIPTESTS_H + +#include + +class GeoIPTests : public QObject +{ + Q_OBJECT +public: + GeoIPTests(); + ~GeoIPTests() override; + +private Q_SLOTS: + void initTestCase(); + void testJSON(); + void testXML(); + void testXML2(); +}; + +#endif From 0c1453ff1812cb672c06aa4e8cc0ac748df6f917 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 16:05:38 -0400 Subject: [PATCH 16/22] [locale] Fix string value handled by XML parser --- src/modules/locale/GeoIPXML.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp index 1bba559d0..f6b3ff342 100644 --- a/src/modules/locale/GeoIPXML.cpp +++ b/src/modules/locale/GeoIPXML.cpp @@ -36,10 +36,8 @@ XMLGeoIP::processReply( const QByteArray& data ) cDebug() << "GeoIP found" << tzElements.length() << "elements"; for ( int it = 0; it < tzElements.length(); ++it ) { - if ( tzElements.at(it).isText() ) - { - return splitTZString( tzElements.at(it).nodeValue() ); - } + auto e = tzElements.at(it).toElement(); + return splitTZString( e.text() ); } } else From 76e37402b3ec5ee66c4918a43c5afa7f60fb55d1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 16:22:35 -0400 Subject: [PATCH 17/22] [locale] Extend tests with negative results --- src/modules/locale/GeoIPTests.cpp | 42 +++++++++++++++++++++++++++++++ src/modules/locale/GeoIPTests.h | 2 ++ 2 files changed, 44 insertions(+) diff --git a/src/modules/locale/GeoIPTests.cpp b/src/modules/locale/GeoIPTests.cpp index 9ad7ad7fc..2c59aa729 100644 --- a/src/modules/locale/GeoIPTests.cpp +++ b/src/modules/locale/GeoIPTests.cpp @@ -51,8 +51,34 @@ GeoIPTests::testJSON() QCOMPARE( tz.first, QLatin1String( "Europe" ) ); QCOMPARE( tz.second, QLatin1String( "Amsterdam" ) ); + + // JSON is quite tolerant + tz = handler.processReply( "time_zone: \"Europe/Brussels\"" ); + QCOMPARE( tz.second, QLatin1String( "Brussels" ) ); + + tz = handler.processReply( "time_zone: America/New_York\n" ); + QCOMPARE( tz.first, "America" ); } +void +GeoIPTests::testJSONbad() +{ + static const char data[] = "time_zone: 1"; + + FreeGeoIP handler; + auto tz = handler.processReply( data ); + + tz = handler.processReply( data ); + QCOMPARE( tz.first, QString() ); + + tz = handler.processReply( "" ); + QCOMPARE( tz.first, QString() ); + + tz = handler.processReply( "404 Forbidden" ); + QCOMPARE( tz.first, QString() ); +} + + void GeoIPTests::testXML() { @@ -96,3 +122,19 @@ GeoIPTests::testXML2() QCOMPARE( tz.second, QLatin1String( "North Dakota/Beulah" ) ); #endif } + +void +GeoIPTests::testXMLbad() +{ +#ifdef HAVE_XML + XMLGeoIP handler; + auto tz = handler.processReply( "{time_zone: \"Europe/Paris\"}" ); + QCOMPARE( tz.first, QString() ); + + tz = handler.processReply( "" ); + QCOMPARE( tz.first, QString() ); + + tz = handler.processReply( "fnord" ); + QCOMPARE( tz.first, QString() ); +#endif +} diff --git a/src/modules/locale/GeoIPTests.h b/src/modules/locale/GeoIPTests.h index b153d5c5f..e673a0740 100644 --- a/src/modules/locale/GeoIPTests.h +++ b/src/modules/locale/GeoIPTests.h @@ -31,8 +31,10 @@ public: private Q_SLOTS: void initTestCase(); void testJSON(); + void testJSONbad(); void testXML(); void testXML2(); + void testXMLbad(); }; #endif From 1340613ef56dd1a0b88dcdaa0a1a66d69642b827 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 12 Apr 2018 16:55:24 -0400 Subject: [PATCH 18/22] [locale] Additional test application for GeoIP processing --- src/modules/locale/CMakeLists.txt | 12 +++-- src/modules/locale/test_geoip.cpp | 73 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 src/modules/locale/test_geoip.cpp diff --git a/src/modules/locale/CMakeLists.txt b/src/modules/locale/CMakeLists.txt index df758ac8e..3c0cc3712 100644 --- a/src/modules/locale/CMakeLists.txt +++ b/src/modules/locale/CMakeLists.txt @@ -1,10 +1,9 @@ find_package(ECM ${ECM_VERSION} NO_MODULE) -if( ECM_FOUND ) +if( ECM_FOUND AND BUILD_TESTING ) include( ECMAddTests ) + find_package( Qt5 COMPONENTS Core Test REQUIRED ) endif() -find_package( Qt5 COMPONENTS Core Test REQUIRED ) - include_directories( ${PROJECT_BINARY_DIR}/src/libcalamaresui ) set( geoip_src GeoIP.cpp GeoIPFreeGeoIP.cpp ) @@ -40,7 +39,7 @@ calamares_add_plugin( locale SHARED_LIB ) -if( ECM_FOUND ) +if( ECM_FOUND AND BUILD_TESTING ) ecm_add_test( GeoIPTests.cpp ${geoip_src} @@ -55,3 +54,8 @@ if( ECM_FOUND ) ) set_target_properties( geoiptest PROPERTIES AUTOMOC TRUE ) endif() + +if( BUILD_TESTING ) + add_executable( test_geoip test_geoip.cpp ${geoip_src} ) + target_link_libraries( test_geoip calamaresui Qt5::Network ${geoip_libs} ${YAMLCPP_LIBRARY} ) +endif() diff --git a/src/modules/locale/test_geoip.cpp b/src/modules/locale/test_geoip.cpp new file mode 100644 index 000000000..5797ad9ff --- /dev/null +++ b/src/modules/locale/test_geoip.cpp @@ -0,0 +1,73 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +/** + * This is a test-application that does one GeoIP parse. + */ + +#include + +#include "GeoIPFreeGeoIP.h" +#ifdef HAVE_XML +#include "GeoIPXML.h" +#endif + +using std::cerr; + +int main(int argc, char** argv) +{ + if (argc != 2) + { + cerr << "Usage: curl url | test_geoip \n"; + return 1; + } + + GeoIP* handler = nullptr; + if ( QLatin1String( "json" ) == argv[1] ) + handler = new FreeGeoIP; +#ifdef HAVE_XML + else if ( QLatin1String( "xml" ) == argv[1] ) + handler = new XMLGeoIP; +#endif + + if ( !handler ) + { + cerr << "Unknown format '" << argv[1] << "'\n"; + return 1; + } + + QByteArray ba; + while( !std::cin.eof() ) { + char arr[1024]; + std::cin.read(arr,sizeof(arr)); + int s = std::cin.gcount(); + ba.append(arr, s); + } + + auto tz = handler->processReply( ba ); + if ( tz.first.isEmpty() ) + { + std::cout << "No TimeZone determined from input.\n"; + } + else + { + std::cout << "TimeZone Region=" << tz.first.toLatin1().constData() << "\nTimeZone Zone=" << tz.second.toLatin1().constData() << '\n'; + } + + return 0; +} From 14fcc2fad647ddf73f83e82f6df812e603ca788b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 13 Apr 2018 08:44:02 -0400 Subject: [PATCH 19/22] [plasmalnf] Continue fighting with layout - The screenshot stays one size, but different ThemeWidgets may overlap partially when you shrink the screen or have more than three / four themes listed. - Probably needs work in the surrounding container and overall better page-scrollbar support. --- src/modules/plasmalnf/ThemeWidget.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/modules/plasmalnf/ThemeWidget.cpp b/src/modules/plasmalnf/ThemeWidget.cpp index eb4e5c356..92a88197f 100644 --- a/src/modules/plasmalnf/ThemeWidget.cpp +++ b/src/modules/plasmalnf/ThemeWidget.cpp @@ -62,15 +62,15 @@ ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) , m_check( new QRadioButton( info.name.isEmpty() ? info.id : info.name, parent ) ) , m_description( new QLabel( info.description, parent ) ) { + const QSize image_size{ + qMax(12 * CalamaresUtils::defaultFontHeight(), 120), + qMax(8 * CalamaresUtils::defaultFontHeight(), 80) }; + QHBoxLayout* layout = new QHBoxLayout( this ); this->setLayout( layout ); layout->addWidget( m_check, 1 ); - const QSize image_size{ - qMax(12 * CalamaresUtils::defaultFontHeight(), 120), - qMax(8 * CalamaresUtils::defaultFontHeight(), 80) }; - QPixmap image( _munge_imagepath( info.imagePath ) ); if ( image.isNull() ) { @@ -88,6 +88,7 @@ ThemeWidget::ThemeWidget(const ThemeInfo& info, QWidget* parent) image_label->setPixmap( image ); image_label->setMinimumSize( image_size ); image_label->setMaximumSize( image_size ); + image_label->setSizePolicy( QSizePolicy::Fixed, QSizePolicy::Fixed ); layout->addWidget( image_label, 1 ); layout->addWidget( m_description, 3 ); From 47b7040897a7972ab9d9b8f67dbd08ee1eb3f42b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 13 Apr 2018 09:24:59 -0400 Subject: [PATCH 20/22] [locale] Adjust to Calamares 3.2 idiom --- src/modules/locale/GeoIPXML.cpp | 2 +- src/modules/locale/LocaleViewStep.cpp | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/modules/locale/GeoIPXML.cpp b/src/modules/locale/GeoIPXML.cpp index f6b3ff342..a0f9b7a2d 100644 --- a/src/modules/locale/GeoIPXML.cpp +++ b/src/modules/locale/GeoIPXML.cpp @@ -42,7 +42,7 @@ XMLGeoIP::processReply( const QByteArray& data ) } else { - cDebug() << "GeoIP XML data error:" << domError << "(line" << errorLine << errorColumn << ')'; + cWarning() << "GeoIP XML data error:" << domError << "(line" << errorLine << errorColumn << ')'; } return qMakePair( QString(), QString() ); diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 6da0b3367..76e89babb 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -31,6 +31,7 @@ #include "timezonewidget/localeglobal.h" #include "widgets/WaitingWidget.h" +#include "utils/CalamaresUtils.h" #include "utils/CalamaresUtilsGui.h" #include "utils/Logger.h" #include "utils/YamlUtils.h" @@ -137,7 +138,7 @@ LocaleViewStep::fetchGeoIpTimezone() #endif else { - cDebug() << "WARNING: GeoIP Style" << m_geoipStyle << "is not recognized."; + cWarning() << "GeoIP Style" << m_geoipStyle << "is not recognized."; setUpPage(); return; } @@ -289,16 +290,6 @@ LocaleViewStep::setConfigurationMap( const QVariantMap& configurationMap ) } // Optional - if ( configurationMap.contains( "geoipUrl" ) && - configurationMap.value( "geoipUrl" ).type() == QVariant::String && - !configurationMap.value( "geoipUrl" ).toString().isEmpty() ) - { - m_geoipUrl = configurationMap.value( "geoipUrl" ).toString(); - } - if ( configurationMap.contains( "geoipStyle" ) && - configurationMap.value( "geoipStyle" ).type() == QVariant::String && - !configurationMap.value( "geoipStyle" ).toString().isEmpty() ) - { - m_geoipStyle = configurationMap.value( "geoipStyle" ).toString(); - } + m_geoipUrl = CalamaresUtils::getString( configurationMap, "geoipUrl" ); + m_geoipStyle = CalamaresUtils::getString( configurationMap, "geoipStyle" ); } From d6f082752d69e5456271e7d94236062510474d8f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 13 Apr 2018 09:36:39 -0400 Subject: [PATCH 21/22] [locale] On GeoIP failure, log URL --- src/modules/locale/LocaleViewStep.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 76e89babb..53dea05c7 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -152,6 +152,8 @@ LocaleViewStep::fetchGeoIpTimezone() auto tz = handler->processReply( reply->readAll() ); if ( !tz.first.isEmpty() ) m_startingTimezone = tz; + else + cWarning() << "GeoIP lookup at" << reply->url() << "failed."; } delete handler; reply->deleteLater(); From ec113e3df3427925ddae077b4b4991e9f6b6e4a3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 13 Apr 2018 09:42:28 -0400 Subject: [PATCH 22/22] [locale] Log GeoIP attempt URL, use possibly-modified form --- src/modules/locale/LocaleViewStep.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/locale/LocaleViewStep.cpp b/src/modules/locale/LocaleViewStep.cpp index 53dea05c7..7cafd8793 100644 --- a/src/modules/locale/LocaleViewStep.cpp +++ b/src/modules/locale/LocaleViewStep.cpp @@ -123,7 +123,7 @@ LocaleViewStep::fetchGeoIpTimezone() if ( m_geoipStyle.isEmpty() || m_geoipStyle == "legacy" ) { - actualUrl.append( "/json" ); + actualUrl.append( "/json/" ); handler = new FreeGeoIP; } else if ( m_geoipStyle == "json" ) @@ -142,6 +142,7 @@ LocaleViewStep::fetchGeoIpTimezone() setUpPage(); return; } + cDebug() << "Fetching GeoIP data from" << actualUrl; QNetworkAccessManager *manager = new QNetworkAccessManager( this ); connect( manager, &QNetworkAccessManager::finished, @@ -162,7 +163,7 @@ LocaleViewStep::fetchGeoIpTimezone() } ); QNetworkRequest request; - request.setUrl( QUrl::fromUserInput( m_geoipUrl ) ); + request.setUrl( QUrl::fromUserInput( actualUrl ) ); request.setAttribute( QNetworkRequest::FollowRedirectsAttribute, true ); manager->get( request ); }