From a7f60f6512e635cd6277d5aa5c2c99cf8f73bdc7 2017-09-21 08:57:57 From: mperrinel Date: 2017-09-21 08:57:57 Subject: [PATCH] Fix bug when creating two variables crash the app. Variable as now invalid range and cache range at creation --- diff --git a/core/include/Variable/Variable.h b/core/include/Variable/Variable.h index eb9b144..bf30f18 100644 --- a/core/include/Variable/Variable.h +++ b/core/include/Variable/Variable.h @@ -25,8 +25,7 @@ class SCIQLOP_CORE_EXPORT Variable : public QObject { Q_OBJECT public: - explicit Variable(const QString &name, const SqpRange &dateTime, - const QVariantHash &metadata = {}); + explicit Variable(const QString &name, const QVariantHash &metadata = {}); /// Copy ctor explicit Variable(const Variable &other); diff --git a/core/include/Variable/VariableModel.h b/core/include/Variable/VariableModel.h index adbcd1d..960ea81 100644 --- a/core/include/Variable/VariableModel.h +++ b/core/include/Variable/VariableModel.h @@ -45,11 +45,10 @@ public: /** * Creates a new variable in the model * @param name the name of the new variable - * @param dateTime the dateTime of the new variable * @param metadata the metadata associated to the new variable * @return the pointer to the new variable */ - std::shared_ptr createVariable(const QString &name, const SqpRange &dateTime, + std::shared_ptr createVariable(const QString &name, const QVariantHash &metadata) noexcept; /** diff --git a/core/src/Variable/Variable.cpp b/core/src/Variable/Variable.cpp index ed9c248..988f06a 100644 --- a/core/src/Variable/Variable.cpp +++ b/core/src/Variable/Variable.cpp @@ -10,10 +10,10 @@ Q_LOGGING_CATEGORY(LOG_Variable, "Variable") struct Variable::VariablePrivate { - explicit VariablePrivate(const QString &name, const SqpRange &dateTime, - const QVariantHash &metadata) + explicit VariablePrivate(const QString &name, const QVariantHash &metadata) : m_Name{name}, - m_Range{dateTime}, + m_Range{INVALID_RANGE}, + m_CacheRange{INVALID_RANGE}, m_Metadata{metadata}, m_DataSeries{nullptr}, m_RealRange{INVALID_RANGE}, @@ -24,6 +24,7 @@ struct Variable::VariablePrivate { VariablePrivate(const VariablePrivate &other) : m_Name{other.m_Name}, m_Range{other.m_Range}, + m_CacheRange{other.m_CacheRange}, m_Metadata{other.m_Metadata}, m_DataSeries{other.m_DataSeries != nullptr ? other.m_DataSeries->clone() : nullptr}, m_RealRange{other.m_RealRange}, @@ -55,9 +56,10 @@ struct Variable::VariablePrivate { auto minXAxisIt = m_DataSeries->minXAxisData(m_Range.m_TStart); auto maxXAxisIt = m_DataSeries->maxXAxisData(m_Range.m_TEnd); - m_RealRange = (minXAxisIt != end && maxXAxisIt != end) - ? SqpRange{minXAxisIt->x(), maxXAxisIt->x()} - : INVALID_RANGE; + m_RealRange + = (minXAxisIt != end && maxXAxisIt != end && minXAxisIt->x() <= maxXAxisIt->x()) + ? SqpRange{minXAxisIt->x(), maxXAxisIt->x()} + : INVALID_RANGE; m_DataSeries->unlock(); } else { @@ -77,8 +79,8 @@ struct Variable::VariablePrivate { QReadWriteLock m_Lock; }; -Variable::Variable(const QString &name, const SqpRange &dateTime, const QVariantHash &metadata) - : impl{spimpl::make_unique_impl(name, dateTime, metadata)} +Variable::Variable(const QString &name, const QVariantHash &metadata) + : impl{spimpl::make_unique_impl(name, metadata)} { } @@ -242,31 +244,35 @@ bool Variable::cacheIsInside(const SqpRange &range) const noexcept QVector Variable::provideNotInCacheRangeList(const SqpRange &range) const noexcept { // This code assume that cach in contigue. Can return 0, 1 or 2 SqpRange - auto notInCache = QVector{}; - - if (!this->cacheContains(range)) { - if (range.m_TEnd <= impl->m_CacheRange.m_TStart - || range.m_TStart >= impl->m_CacheRange.m_TEnd) { - notInCache << range; - } - else if (range.m_TStart < impl->m_CacheRange.m_TStart - && range.m_TEnd <= impl->m_CacheRange.m_TEnd) { - notInCache << SqpRange{range.m_TStart, impl->m_CacheRange.m_TStart}; - } - else if (range.m_TStart < impl->m_CacheRange.m_TStart - && range.m_TEnd > impl->m_CacheRange.m_TEnd) { - notInCache << SqpRange{range.m_TStart, impl->m_CacheRange.m_TStart} - << SqpRange{impl->m_CacheRange.m_TEnd, range.m_TEnd}; - } - else if (range.m_TStart < impl->m_CacheRange.m_TEnd) { - notInCache << SqpRange{impl->m_CacheRange.m_TEnd, range.m_TEnd}; - } - else { - qCCritical(LOG_Variable()) << tr("Detection of unknown case.") - << QThread::currentThread(); + if (impl->m_CacheRange != INVALID_RANGE) { + + if (!this->cacheContains(range)) { + if (range.m_TEnd <= impl->m_CacheRange.m_TStart + || range.m_TStart >= impl->m_CacheRange.m_TEnd) { + notInCache << range; + } + else if (range.m_TStart < impl->m_CacheRange.m_TStart + && range.m_TEnd <= impl->m_CacheRange.m_TEnd) { + notInCache << SqpRange{range.m_TStart, impl->m_CacheRange.m_TStart}; + } + else if (range.m_TStart < impl->m_CacheRange.m_TStart + && range.m_TEnd > impl->m_CacheRange.m_TEnd) { + notInCache << SqpRange{range.m_TStart, impl->m_CacheRange.m_TStart} + << SqpRange{impl->m_CacheRange.m_TEnd, range.m_TEnd}; + } + else if (range.m_TStart < impl->m_CacheRange.m_TEnd) { + notInCache << SqpRange{impl->m_CacheRange.m_TEnd, range.m_TEnd}; + } + else { + qCCritical(LOG_Variable()) << tr("Detection of unknown case.") + << QThread::currentThread(); + } } } + else { + notInCache << range; + } return notInCache; } @@ -277,29 +283,31 @@ QVector Variable::provideInCacheRangeList(const SqpRange &range) const auto inCache = QVector{}; - - if (this->intersect(range)) { - if (range.m_TStart <= impl->m_CacheRange.m_TStart - && range.m_TEnd >= impl->m_CacheRange.m_TStart - && range.m_TEnd < impl->m_CacheRange.m_TEnd) { - inCache << SqpRange{impl->m_CacheRange.m_TStart, range.m_TEnd}; - } - - else if (range.m_TStart >= impl->m_CacheRange.m_TStart - && range.m_TEnd <= impl->m_CacheRange.m_TEnd) { - inCache << range; - } - else if (range.m_TStart > impl->m_CacheRange.m_TStart - && range.m_TEnd > impl->m_CacheRange.m_TEnd) { - inCache << SqpRange{range.m_TStart, impl->m_CacheRange.m_TEnd}; - } - else if (range.m_TStart <= impl->m_CacheRange.m_TStart - && range.m_TEnd >= impl->m_CacheRange.m_TEnd) { - inCache << impl->m_CacheRange; - } - else { - qCCritical(LOG_Variable()) << tr("Detection of unknown case.") - << QThread::currentThread(); + if (impl->m_CacheRange != INVALID_RANGE) { + + if (this->intersect(range)) { + if (range.m_TStart <= impl->m_CacheRange.m_TStart + && range.m_TEnd >= impl->m_CacheRange.m_TStart + && range.m_TEnd < impl->m_CacheRange.m_TEnd) { + inCache << SqpRange{impl->m_CacheRange.m_TStart, range.m_TEnd}; + } + + else if (range.m_TStart >= impl->m_CacheRange.m_TStart + && range.m_TEnd <= impl->m_CacheRange.m_TEnd) { + inCache << range; + } + else if (range.m_TStart > impl->m_CacheRange.m_TStart + && range.m_TEnd > impl->m_CacheRange.m_TEnd) { + inCache << SqpRange{range.m_TStart, impl->m_CacheRange.m_TEnd}; + } + else if (range.m_TStart <= impl->m_CacheRange.m_TStart + && range.m_TEnd >= impl->m_CacheRange.m_TEnd) { + inCache << impl->m_CacheRange; + } + else { + qCCritical(LOG_Variable()) << tr("Detection of unknown case.") + << QThread::currentThread(); + } } } diff --git a/core/src/Variable/VariableController.cpp b/core/src/Variable/VariableController.cpp index 3c8dde4..b54e23a 100644 --- a/core/src/Variable/VariableController.cpp +++ b/core/src/Variable/VariableController.cpp @@ -265,7 +265,7 @@ VariableController::createVariable(const QString &name, const QVariantHash &meta auto range = impl->m_TimeController->dateTime(); - if (auto newVariable = impl->m_VariableModel->createVariable(name, range, metadata)) { + if (auto newVariable = impl->m_VariableModel->createVariable(name, metadata)) { auto identifier = QUuid::createUuid(); // store the provider @@ -573,7 +573,7 @@ void VariableController::VariableControllerPrivate::processRequest(std::shared_p varProvider); if (!varRequestIdCanceled.isNull()) { - qCDebug(LOG_VariableAcquisitionWorker()) << tr("varRequestIdCanceled: ") + qCDebug(LOG_VariableAcquisitionWorker()) << tr("vsarRequestIdCanceled: ") << varRequestIdCanceled; cancelVariableRequest(varRequestIdCanceled); } @@ -588,7 +588,6 @@ void VariableController::VariableControllerPrivate::processRequest(std::shared_p } } else { - varRequest.m_RangeRequested = varStrategyRangesRequested.first; varRequest.m_CacheRangeRequested = varStrategyRangesRequested.second; // store VarRequest diff --git a/core/src/Variable/VariableModel.cpp b/core/src/Variable/VariableModel.cpp index 13e4d70..e5f9947 100644 --- a/core/src/Variable/VariableModel.cpp +++ b/core/src/Variable/VariableModel.cpp @@ -97,10 +97,9 @@ bool VariableModel::containsVariable(std::shared_ptr variable) const n } std::shared_ptr VariableModel::createVariable(const QString &name, - const SqpRange &dateTime, const QVariantHash &metadata) noexcept { - auto variable = std::make_shared(name, dateTime, metadata); + auto variable = std::make_shared(name, metadata); addVariable(variable); return variable; diff --git a/core/tests/Variable/TestVariable.cpp b/core/tests/Variable/TestVariable.cpp index 3fb2623..e6688fa 100644 --- a/core/tests/Variable/TestVariable.cpp +++ b/core/tests/Variable/TestVariable.cpp @@ -24,10 +24,12 @@ void TestVariable::testNotInCacheRangeList() auto varCRS = QDateTime{QDate{2017, 01, 01}, QTime{2, 3, 0, 0}}; auto varCRE = QDateTime{QDate{2017, 01, 01}, QTime{2, 4, 0, 0}}; + auto sqpCR = SqpRange{DateUtils::secondsSinceEpoch(varCRS), DateUtils::secondsSinceEpoch(varCRE)}; - Variable var{"Var test", sqpR}; + Variable var{"Var test"}; + var.setRange(sqpR); var.setCacheRange(sqpCR); // 1: [ts,te] < varTS @@ -109,7 +111,8 @@ void TestVariable::testInCacheRangeList() auto sqpCR = SqpRange{DateUtils::secondsSinceEpoch(varCRS), DateUtils::secondsSinceEpoch(varCRE)}; - Variable var{"Var test", sqpR}; + Variable var{"Var test"}; + var.setRange(sqpR); var.setCacheRange(sqpCR); // 1: [ts,te] < varTS diff --git a/core/tests/Variable/TestVariableCacheController.cpp b/core/tests/Variable/TestVariableCacheController.cpp index 326410b..628da67 100644 --- a/core/tests/Variable/TestVariableCacheController.cpp +++ b/core/tests/Variable/TestVariableCacheController.cpp @@ -32,7 +32,8 @@ void TestVariableCacheController::testProvideNotInCacheDateTimeList() auto te2 = QDateTime{QDate{2017, 01, 01}, QTime{2, 20, 0, 0}}; auto sqp2 = SqpRange{DateUtils::secondsSinceEpoch(ts2), DateUtils::secondsSinceEpoch(te2)}; - auto var0 = std::make_shared("", sqp0); + auto var0 = std::make_shared(""); + var0->setRange(sqp0); variableCacheController.addDateTime(var0, sqp0); variableCacheController.addDateTime(var0, sqp1); @@ -267,7 +268,8 @@ void TestVariableCacheController::testAddDateTime() auto sqp03 = SqpRange{DateUtils::secondsSinceEpoch(ts03), DateUtils::secondsSinceEpoch(te03)}; - auto var0 = std::make_shared("", sqp0); + auto var0 = std::make_shared(""); + var0->setRange(sqp0); // First case: add the first interval to the variable :sqp0 diff --git a/plugins/amda/src/AmdaProvider.cpp b/plugins/amda/src/AmdaProvider.cpp index 874aebb..9b0bc8c 100644 --- a/plugins/amda/src/AmdaProvider.cpp +++ b/plugins/amda/src/AmdaProvider.cpp @@ -107,7 +107,6 @@ void AmdaProvider::onReplyDownloadProgress(QUuid acqIdentifier, auto acqIdToRequestProgressMapIt = m_AcqIdToRequestProgressMap.find(acqIdentifier); if (acqIdToRequestProgressMapIt != m_AcqIdToRequestProgressMap.end()) { - qCDebug(LOG_AmdaProvider()) << tr("1 onReplyDownloadProgress found") << progress; auto requestPtr = networkRequest; auto findRequest = [requestPtr](const auto &entry) { return requestPtr == entry.first; }; @@ -159,7 +158,6 @@ void AmdaProvider::retrieveData(QUuid token, const SqpRange &dateTime, const QVa qCCritical(LOG_AmdaProvider()) << tr("Can't retrieve data: unknown product id"); return; } - qCDebug(LOG_AmdaProvider()) << tr("AmdaProvider::retrieveData") << dateTime; // Retrieves the data type that determines whether the expected format for the result file is // scalar, vector... @@ -210,7 +208,7 @@ void AmdaProvider::retrieveData(QUuid token, const SqpRange &dateTime, const QVa if (reply->error() != QNetworkReply::OperationCanceledError) { auto downloadFileUrl = QUrl{QString{reply->readAll()}}; - qCInfo(LOG_AmdaProvider()) + qCDebug(LOG_AmdaProvider()) << tr("TORM AmdaProvider::retrieveData downloadFileUrl:") << downloadFileUrl; // Executes request for downloading file // diff --git a/plugins/amda/tests/TestAmdaAcquisition.cpp b/plugins/amda/tests/TestAmdaAcquisition.cpp index 60662bb..92d0b35 100644 --- a/plugins/amda/tests/TestAmdaAcquisition.cpp +++ b/plugins/amda/tests/TestAmdaAcquisition.cpp @@ -100,8 +100,6 @@ void TestAmdaAcquisition::testAcquisition() auto var = vc.createVariable("bx_gse", metaData, provider); // 1 : Variable creation - QCOMPARE(var->range().m_TStart, sqpR.m_TStart); - QCOMPARE(var->range().m_TEnd, sqpR.m_TEnd); qDebug() << " 1: TIMECONTROLLER" << timeController->dateTime(); qDebug() << " 1: RANGE " << var->range();