QIcuTimeZonePrivate constructor: save iteration over all zone IDs

ICU returns a "valid" representation of GMT when given an unrecognised
ID, so QTZ's constructor has been checking the ID is available before
passing it to the backend constructor. That availability check was
done by generating the list of available IDs to see if the given ID
was in it; this is very inefficient. Furthermore, the QTZ constructor
was also checking availability, to work round the same issue in only
this one backend, making the check redundant.

So overide isTimeZoneIdAvailable() in the ICU backend, calling
ucal_getCanonicalTimeZoneID(), which answers the question directly;
and drop the duplicate check in the QTZ constructor. Expand a test to
verify an invalid name is rejected.

Fixes: QTBUG-121807
Pick-to: 6.6 6.5
Change-Id: I34f996b607b958d12607a94eb273bb1b406cca1a
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 00d9a9a9b59650b8e297f91dcc600c377da5bceb)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Nicolas Fella 2021-08-22 19:21:45 +02:00 committed by Edward Welbourne
parent 975ee89cea
commit 74c59f8a23
4 changed files with 29 additions and 16 deletions

View File

@ -465,24 +465,15 @@ QTimeZone::Data &QTimeZone::Data::operator=(QTimeZonePrivate *dptr) noexcept
QTimeZone::QTimeZone(const QByteArray &ianaId) QTimeZone::QTimeZone(const QByteArray &ianaId)
{ {
// Try and see if it's a CLDR UTC offset ID - just as quick by creating as // Try and see if it's a recognized UTC offset ID - just as quick by
// by looking up. // creating as by looking up.
d = new QUtcTimeZonePrivate(ianaId); d = new QUtcTimeZonePrivate(ianaId);
// If not a CLDR UTC offset ID then try creating it with the system backend. // If not recognized, try creating it with the system backend.
// Relies on backend not creating valid TZ with invalid name.
if (!d->isValid()) { if (!d->isValid()) {
if (ianaId.isEmpty()) if (ianaId.isEmpty())
d = newBackendTimeZone(); d = newBackendTimeZone();
#ifdef Q_OS_ANDROID else // Constructor MUST produce invalid for unsupported ID.
// on Android the isTimeZoneIdAvailable() implementation is vastly more
// expensive than just trying to create a timezone
else
d = newBackendTimeZone(ianaId); d = newBackendTimeZone(ianaId);
#else
else if (global_tz->backend->isTimeZoneIdAvailable(ianaId))
d = newBackendTimeZone(ianaId);
#endif
// else: No such ID, avoid creating a TZ cache entry for it.
} }
// Can also handle UTC with arbitrary (valid) offset, but only do so as // Can also handle UTC with arbitrary (valid) offset, but only do so as
// fall-back, since either of the above may handle it more informatively. // fall-back, since either of the above may handle it more informatively.

View File

@ -254,8 +254,8 @@ QIcuTimeZonePrivate::QIcuTimeZonePrivate()
QIcuTimeZonePrivate::QIcuTimeZonePrivate(const QByteArray &ianaId) QIcuTimeZonePrivate::QIcuTimeZonePrivate(const QByteArray &ianaId)
: m_ucal(nullptr) : m_ucal(nullptr)
{ {
// Need to check validity here as ICu will create a GMT tz if name is invalid // ICU misleadingly maps invalid IDs to GMT.
if (availableTimeZoneIds().contains(ianaId)) if (isTimeZoneIdAvailable(ianaId))
init(ianaId); init(ianaId);
} }
@ -442,6 +442,25 @@ QByteArray QIcuTimeZonePrivate::systemTimeZoneId() const
return ucalDefaultTimeZoneId(); return ucalDefaultTimeZoneId();
} }
bool QIcuTimeZonePrivate::isTimeZoneIdAvailable(const QByteArray &ianaId) const
{
const QString ianaStr = QString::fromUtf8(ianaId);
const UChar *const name = reinterpret_cast<const UChar *>(ianaStr.constData());
// We are not interested in the value, but we have to pass something.
// No known IANA zone name is (up to 2023) longer than 30 characters.
constexpr size_t size = 64;
UChar buffer[size];
// TODO: convert to ucal_getIanaTimeZoneID(), new draft in ICU 74, once we
// can rely on its availability, assuming it works the same once not draft.
UErrorCode status = U_ZERO_ERROR;
UBool isSys = false;
// Returns the length of the IANA zone name (but we don't care):
ucal_getCanonicalTimeZoneID(name, ianaStr.size(), buffer, size, &isSys, &status);
// We're only interested if the result is a "system" (i.e. IANA) ID:
return isSys;
}
QList<QByteArray> QIcuTimeZonePrivate::availableTimeZoneIds() const QList<QByteArray> QIcuTimeZonePrivate::availableTimeZoneIds() const
{ {
UErrorCode status = U_ZERO_ERROR; UErrorCode status = U_ZERO_ERROR;

View File

@ -233,6 +233,7 @@ public:
QByteArray systemTimeZoneId() const override; QByteArray systemTimeZoneId() const override;
bool isTimeZoneIdAvailable(const QByteArray &ianaId) const override;
QList<QByteArray> availableTimeZoneIds() const override; QList<QByteArray> availableTimeZoneIds() const override;
QList<QByteArray> availableTimeZoneIds(QLocale::Territory territory) const override; QList<QByteArray> availableTimeZoneIds(QLocale::Territory territory) const override;
QList<QByteArray> availableTimeZoneIds(int offsetFromUtc) const override; QList<QByteArray> availableTimeZoneIds(int offsetFromUtc) const override;

View File

@ -1281,7 +1281,9 @@ void tst_QTimeZone::icuTest()
QIcuTimeZonePrivate tzpd; QIcuTimeZonePrivate tzpd;
QVERIFY(tzpd.isValid()); QVERIFY(tzpd.isValid());
// Test invalid constructor // Test invalid is not available:
QVERIFY(!tzpd.isTimeZoneIdAvailable("Gondwana/Erewhon"));
// and construction gives an invalid result:
QIcuTimeZonePrivate tzpi("Gondwana/Erewhon"); QIcuTimeZonePrivate tzpi("Gondwana/Erewhon");
QCOMPARE(tzpi.isValid(), false); QCOMPARE(tzpi.isValid(), false);