From ceac74ef218ad6462d135f8ab9cef1244fa0fa45 2017-09-07 09:48:24 From: Alexandre Leroux Date: 2017-09-07 09:48:24 Subject: [PATCH] Merge branch 'feature/CloneVariable' into develop --- diff --git a/core/include/Common/StringUtils.h b/core/include/Common/StringUtils.h new file mode 100644 index 0000000..3dcdee2 --- /dev/null +++ b/core/include/Common/StringUtils.h @@ -0,0 +1,35 @@ +#ifndef SCIQLOP_STRINGUTILS_H +#define SCIQLOP_STRINGUTILS_H + +#include "CoreGlobal.h" + +#include + +class QString; + +/** + * Utility class with methods for strings + */ +struct SCIQLOP_CORE_EXPORT StringUtils { + /** + * Generates a unique name from a default name and a set of forbidden names. + * + * Generating the unique name is done by adding an index to the default name and stopping at the + * first index for which the generated name is not in the forbidden names. + * + * Examples (defaultName, forbiddenNames -> result): + * - "FGM", {"FGM"} -> "FGM1" + * - "FGM", {"ABC"} -> "FGM" + * - "FGM", {"FGM", "FGM1"} -> "FGM2" + * - "FGM", {"FGM", "FGM2"} -> "FGM1" + * - "", {"ABC"} -> "1" + * + * @param defaultName the default name + * @param forbiddenNames the set of forbidden names + * @return the unique name generated + */ + static QString uniqueName(const QString &defaultName, + const std::vector &forbiddenNames) noexcept; +}; + +#endif // SCIQLOP_STRINGUTILS_H diff --git a/core/include/Data/IDataProvider.h b/core/include/Data/IDataProvider.h index 805348d..55dd069 100644 --- a/core/include/Data/IDataProvider.h +++ b/core/include/Data/IDataProvider.h @@ -32,6 +32,7 @@ class SCIQLOP_CORE_EXPORT IDataProvider : public QObject { Q_OBJECT public: virtual ~IDataProvider() noexcept = default; + virtual std::shared_ptr clone() const = 0; /** * @brief requestDataLoading provide datas for the data identified by acqIdentifier and diff --git a/core/include/Variable/Variable.h b/core/include/Variable/Variable.h index d766027..5b731ab 100644 --- a/core/include/Variable/Variable.h +++ b/core/include/Variable/Variable.h @@ -28,6 +28,11 @@ public: explicit Variable(const QString &name, const SqpRange &dateTime, const QVariantHash &metadata = {}); + /// Copy ctor + explicit Variable(const Variable &other); + + std::shared_ptr clone() const; + QString name() const noexcept; void setName(const QString &name) noexcept; SqpRange range() const noexcept; diff --git a/core/include/Variable/VariableController.h b/core/include/Variable/VariableController.h index cf03521..3350511 100644 --- a/core/include/Variable/VariableController.h +++ b/core/include/Variable/VariableController.h @@ -42,6 +42,13 @@ public: void setTimeController(TimeController *timeController) noexcept; /** + * Clones the variable passed in parameter and adds the duplicate to the controller + * @param variable the variable to duplicate + * @return the duplicate created, nullptr if the variable couldn't be created + */ + std::shared_ptr cloneVariable(std::shared_ptr variable) noexcept; + + /** * Deletes from the controller the variable passed in parameter. * * Delete a variable includes: diff --git a/core/include/Variable/VariableModel.h b/core/include/Variable/VariableModel.h index f4d30fa..adbcd1d 100644 --- a/core/include/Variable/VariableModel.h +++ b/core/include/Variable/VariableModel.h @@ -28,6 +28,21 @@ public: explicit VariableModel(QObject *parent = nullptr); /** + * Adds an existing variable in the model. + * @param variable the variable to add. + * @remarks the variable's name is modified to avoid name duplicates + * @remarks this method does nothing if the variable already exists in the model + */ + void addVariable(std::shared_ptr variable) noexcept; + + /** + * Checks that a variable is contained in the model + * @param variable the variable to check + * @return true if the variable is in the model, false otherwise + */ + bool containsVariable(std::shared_ptr variable) const noexcept; + + /** * Creates a new variable in the model * @param name the name of the new variable * @param dateTime the dateTime of the new variable diff --git a/core/meson.build b/core/meson.build index bb77c2e..195cd0b 100644 --- a/core/meson.build +++ b/core/meson.build @@ -20,6 +20,7 @@ core_moc_files = qt5.preprocess(moc_headers : core_moc_headers) core_sources = [ 'src/Common/DateUtils.cpp', + 'src/Common/StringUtils.cpp', 'src/Data/ScalarSeries.cpp', 'src/Data/DataSeriesIterator.cpp', 'src/Data/ArrayDataIterator.cpp', diff --git a/core/src/Common/StringUtils.cpp b/core/src/Common/StringUtils.cpp new file mode 100644 index 0000000..784e313 --- /dev/null +++ b/core/src/Common/StringUtils.cpp @@ -0,0 +1,30 @@ +#include "Common/StringUtils.h" + +#include +#include + +#include + +QString StringUtils::uniqueName(const QString &defaultName, + const std::vector &forbiddenNames) noexcept +{ + // Gets the base of the unique name to generate, by removing trailing number (for example, base + // name of "FGM12" is "FGM") + auto baseName = defaultName; + baseName.remove(QRegExp{QStringLiteral("\\d*$")}); + + // Finds the unique name by adding an index to the base name and stops when the generated name + // isn't forbidden + QString newName{}; + auto forbidden = true; + for (auto i = 0; forbidden; ++i) { + newName = (i == 0) ? baseName : baseName + QString::number(i); + forbidden = newName.isEmpty() + || std::any_of(forbiddenNames.cbegin(), forbiddenNames.cend(), + [&newName](const auto &name) { + return name.compare(newName, Qt::CaseInsensitive) == 0; + }); + } + + return newName; +} diff --git a/core/src/Variable/Variable.cpp b/core/src/Variable/Variable.cpp index 2d56bbb..64874ba 100644 --- a/core/src/Variable/Variable.cpp +++ b/core/src/Variable/Variable.cpp @@ -20,6 +20,15 @@ struct Variable::VariablePrivate { { } + VariablePrivate(const VariablePrivate &other) + : m_Name{other.m_Name}, + m_Range{other.m_Range}, + m_Metadata{other.m_Metadata}, + m_DataSeries{other.m_DataSeries != nullptr ? other.m_DataSeries->clone() : nullptr}, + m_RealRange{other.m_RealRange} + { + } + void lockRead() { m_Lock.lockForRead(); } void lockWrite() { m_Lock.lockForWrite(); } void unlock() { m_Lock.unlock(); } @@ -67,6 +76,16 @@ Variable::Variable(const QString &name, const SqpRange &dateTime, const QVariant { } +Variable::Variable(const Variable &other) + : impl{spimpl::make_unique_impl(*other.impl)} +{ +} + +std::shared_ptr Variable::clone() const +{ + return std::make_shared(*this); +} + QString Variable::name() const noexcept { impl->lockRead(); diff --git a/core/src/Variable/VariableController.cpp b/core/src/Variable/VariableController.cpp index e7a3da5..50b88cc 100644 --- a/core/src/Variable/VariableController.cpp +++ b/core/src/Variable/VariableController.cpp @@ -188,6 +188,38 @@ void VariableController::setTimeController(TimeController *timeController) noexc impl->m_TimeController = timeController; } +std::shared_ptr +VariableController::cloneVariable(std::shared_ptr variable) noexcept +{ + if (impl->m_VariableModel->containsVariable(variable)) { + // Clones variable + auto duplicate = variable->clone(); + + // Adds clone to model + impl->m_VariableModel->addVariable(duplicate); + + // Generates clone identifier + impl->m_VariableToIdentifierMap[duplicate] = QUuid::createUuid(); + + // Registers provider + auto variableProvider = impl->m_VariableToProviderMap.at(variable); + auto duplicateProvider = variableProvider != nullptr ? variableProvider->clone() : nullptr; + + impl->m_VariableToProviderMap[duplicate] = duplicateProvider; + if (duplicateProvider) { + impl->registerProvider(duplicateProvider); + } + + return duplicate; + } + else { + qCCritical(LOG_VariableController()) + << tr("Can't create duplicate of variable %1: variable not registered in the model") + .arg(variable->name()); + return nullptr; + } +} + void VariableController::deleteVariable(std::shared_ptr variable) noexcept { if (!variable) { diff --git a/core/src/Variable/VariableModel.cpp b/core/src/Variable/VariableModel.cpp index 94f0a7f..ad88e27 100644 --- a/core/src/Variable/VariableModel.cpp +++ b/core/src/Variable/VariableModel.cpp @@ -2,6 +2,7 @@ #include #include +#include #include @@ -45,6 +46,17 @@ const auto COLUMN_PROPERTIES = QHash{ /// Format for datetimes const auto DATETIME_FORMAT = QStringLiteral("dd/MM/yyyy \nhh:mm:ss:zzz"); +QString uniqueName(const QString &defaultName, + const std::vector > &variables) +{ + auto forbiddenNames = std::vector(variables.size()); + std::transform(variables.cbegin(), variables.cend(), forbiddenNames.begin(), + [](const auto &variable) { return variable->name(); }); + auto uniqueName = StringUtils::uniqueName(defaultName, forbiddenNames); + Q_ASSERT(!uniqueName.isEmpty()); + + return uniqueName; +} } // namespace @@ -62,19 +74,32 @@ VariableModel::VariableModel(QObject *parent) { } -std::shared_ptr VariableModel::createVariable(const QString &name, - const SqpRange &dateTime, - const QVariantHash &metadata) noexcept +void VariableModel::addVariable(std::shared_ptr variable) noexcept { auto insertIndex = rowCount(); beginInsertRows({}, insertIndex, insertIndex); - auto variable = std::make_shared(name, dateTime, metadata); + // Generates unique name for the variable + variable->setName(uniqueName(variable->name(), impl->m_Variables)); impl->m_Variables.push_back(variable); connect(variable.get(), &Variable::updated, this, &VariableModel::onVariableUpdated); endInsertRows(); +} + +bool VariableModel::containsVariable(std::shared_ptr variable) const noexcept +{ + auto end = impl->m_Variables.cend(); + return std::find(impl->m_Variables.cbegin(), end, variable) != end; +} + +std::shared_ptr VariableModel::createVariable(const QString &name, + const SqpRange &dateTime, + const QVariantHash &metadata) noexcept +{ + auto variable = std::make_shared(name, dateTime, metadata); + addVariable(variable); return variable; } diff --git a/core/tests/Common/TestStringUtils.cpp b/core/tests/Common/TestStringUtils.cpp new file mode 100644 index 0000000..c6f66f1 --- /dev/null +++ b/core/tests/Common/TestStringUtils.cpp @@ -0,0 +1,50 @@ +#include + +#include +#include + +class TestStringUtils : public QObject { + Q_OBJECT + +private slots: + void testUniqueName_data(); + void testUniqueName(); +}; + +void TestStringUtils::testUniqueName_data() +{ + // ////////////// // + // Test structure // + // ////////////// // + + QTest::addColumn("defaultName"); + QTest::addColumn >("forbiddenNames"); + QTest::addColumn("expectedName"); + + // ////////// // + // Test cases // + // ////////// // + + QTest::newRow("uniqueName") << "FGM" << std::vector{"FGM2"} << "FGM"; + QTest::newRow("uniqueName2") << "FGM2" << std::vector{"FGM", "FGM1", "FGM2"} << "FGM3"; + QTest::newRow("uniqueName3") << "FGM1" << std::vector{"FGM1"} << "FGM"; + QTest::newRow("uniqueName4") << "FGM" << std::vector{"FGM"} << "FGM1"; + QTest::newRow("uniqueName5") << "FGM" << std::vector{"FGM", "FGM1", "FGM3"} << "FGM2"; + QTest::newRow("uniqueName6") << "FGM" << std::vector{"A", "B", "C"} << "FGM"; + QTest::newRow("uniqueName7") << "FGM" << std::vector{"fGm", "FGm1", "Fgm2"} << "FGM3"; + QTest::newRow("uniqueName8") << "" << std::vector{"A", "B", "C"} << "1"; + QTest::newRow("uniqueName9") << "24" << std::vector{"A", "B", "C"} << "1"; +} + +void TestStringUtils::testUniqueName() +{ + QFETCH(QString, defaultName); + QFETCH(std::vector, forbiddenNames); + QFETCH(QString, expectedName); + + auto result = StringUtils::uniqueName(defaultName, forbiddenNames); + QCOMPARE(result, expectedName); +} + +QTEST_MAIN(TestStringUtils) +#include "TestStringUtils.moc" diff --git a/core/tests/Variable/TestVariableController.cpp b/core/tests/Variable/TestVariableController.cpp index 1b278f1..ec1093e 100644 --- a/core/tests/Variable/TestVariableController.cpp +++ b/core/tests/Variable/TestVariableController.cpp @@ -12,6 +12,8 @@ namespace { /// Provider used for the tests class TestProvider : public IDataProvider { + std::shared_ptr clone() const { return std::make_shared(); } + void requestDataLoading(QUuid acqIdentifier, const DataProviderParameters ¶meters) override { // Does nothing diff --git a/core/tests/meson.build b/core/tests/meson.build index 7f15f45..83328c6 100644 --- a/core/tests/meson.build +++ b/core/tests/meson.build @@ -1,6 +1,7 @@ tests = [ + [['Common/TestStringUtils.cpp'],'test_string_utils','StringUtils test'], [['Data/TestDataSeries.cpp'],'test_data','DataSeries test'], [['Data/TestOneDimArrayData.cpp'],'test_1d','One Dim Array test'], [['Data/TestTwoDimArrayData.cpp'],'test_2d','Two Dim Array test'], diff --git a/gui/src/Variable/RenameVariableDialog.cpp b/gui/src/Variable/RenameVariableDialog.cpp index 1af8af5..2f49e6e 100644 --- a/gui/src/Variable/RenameVariableDialog.cpp +++ b/gui/src/Variable/RenameVariableDialog.cpp @@ -25,7 +25,7 @@ RenameVariableDialog::~RenameVariableDialog() noexcept QString RenameVariableDialog::name() const noexcept { - return ui->nameLineEdit->text(); + return ui->nameLineEdit->text().trimmed(); } void RenameVariableDialog::accept() @@ -38,7 +38,7 @@ void RenameVariableDialog::accept() }; // Empty name - auto name = ui->nameLineEdit->text(); + auto name = this->name(); if (name.isEmpty()) { invalidateInput(tr("A variable name must be specified")); return; diff --git a/gui/src/Variable/VariableInspectorWidget.cpp b/gui/src/Variable/VariableInspectorWidget.cpp index 53889b0..45907ae 100644 --- a/gui/src/Variable/VariableInspectorWidget.cpp +++ b/gui/src/Variable/VariableInspectorWidget.cpp @@ -176,10 +176,16 @@ void VariableInspectorWidget::onTableMenuRequested(const QPoint &pos) noexcept if (!selectedVariables.isEmpty()) { tableMenu.addSeparator(); - // 'Rename' action (only if one variable selected) + // 'Rename' and 'Duplicate' actions (only if one variable selected) if (selectedVariables.size() == 1) { auto selectedVariable = selectedVariables.front(); + auto duplicateFun = [&selectedVariable]() { + sqpApp->variableController().cloneVariable(selectedVariable); + }; + + tableMenu.addAction(tr("Duplicate"), duplicateFun); + auto renameFun = [&selectedVariable, &model, this]() { // Generates forbidden names (names associated to existing variables) auto allVariables = model->variables(); diff --git a/plugins/amda/include/AmdaProvider.h b/plugins/amda/include/AmdaProvider.h index 292d9b8..16a25b9 100644 --- a/plugins/amda/include/AmdaProvider.h +++ b/plugins/amda/include/AmdaProvider.h @@ -18,6 +18,7 @@ class QNetworkReply; class SCIQLOP_AMDA_EXPORT AmdaProvider : public IDataProvider { public: explicit AmdaProvider(); + std::shared_ptr clone() const override; void requestDataLoading(QUuid acqIdentifier, const DataProviderParameters ¶meters) override; diff --git a/plugins/amda/src/AmdaProvider.cpp b/plugins/amda/src/AmdaProvider.cpp index 8fe1f67..d17e6f9 100644 --- a/plugins/amda/src/AmdaProvider.cpp +++ b/plugins/amda/src/AmdaProvider.cpp @@ -68,6 +68,12 @@ AmdaProvider::AmdaProvider() } } +std::shared_ptr AmdaProvider::clone() const +{ + // No copy is made in the clone + return std::make_shared(); +} + void AmdaProvider::requestDataLoading(QUuid acqIdentifier, const DataProviderParameters ¶meters) { // NOTE: Try to use multithread if possible diff --git a/plugins/mockplugin/include/CosinusProvider.h b/plugins/mockplugin/include/CosinusProvider.h index 05db4d1..1ef2c6e 100644 --- a/plugins/mockplugin/include/CosinusProvider.h +++ b/plugins/mockplugin/include/CosinusProvider.h @@ -16,6 +16,8 @@ Q_DECLARE_LOGGING_CATEGORY(LOG_CosinusProvider) */ class SCIQLOP_MOCKPLUGIN_EXPORT CosinusProvider : public IDataProvider { public: + std::shared_ptr clone() const override; + /// @sa IDataProvider::requestDataLoading(). The current impl isn't thread safe. void requestDataLoading(QUuid acqIdentifier, const DataProviderParameters ¶meters) override; diff --git a/plugins/mockplugin/src/CosinusProvider.cpp b/plugins/mockplugin/src/CosinusProvider.cpp index 661a78f..6ba687f 100644 --- a/plugins/mockplugin/src/CosinusProvider.cpp +++ b/plugins/mockplugin/src/CosinusProvider.cpp @@ -11,6 +11,12 @@ Q_LOGGING_CATEGORY(LOG_CosinusProvider, "CosinusProvider") +std::shared_ptr CosinusProvider::clone() const +{ + // No copy is made in clone + return std::make_shared(); +} + std::shared_ptr CosinusProvider::retrieveData(QUuid acqIdentifier, const SqpRange &dataRangeRequested) {