From 12a2d8009d9f7e0ef14011eb87e4a135539c7873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20St=C3=BCrz?= Date: Fri, 1 Apr 2016 18:48:43 +0200 Subject: [PATCH] fix unpacking/packing methods add first rest row for calendarItems --- guh.pri | 2 +- guh.pro | 1 - server/jsonrpc/jsontypes.cpp | 64 ++++++++--- server/ruleengine.cpp | 44 +++++++- server/ruleengine.h | 4 + server/time/repeatingoption.cpp | 17 +++ server/time/timeeventitem.cpp | 2 +- server/time/timeeventitem.h | 2 +- tests/auto/timemanager/testtimemanager.cpp | 123 +++++++++++++++++++++ 9 files changed, 239 insertions(+), 20 deletions(-) diff --git a/guh.pri b/guh.pri index 3829b9b4..663370c0 100644 --- a/guh.pri +++ b/guh.pri @@ -2,7 +2,7 @@ GUH_VERSION_STRING=$$system('dpkg-parsechangelog | sed -n -e "s/^Version: //p"') # define protocol versions -JSON_PROTOCOL_VERSION=38 +JSON_PROTOCOL_VERSION=39 REST_API_VERSION=1 DEFINES += GUH_VERSION_STRING=\\\"$${GUH_VERSION_STRING}\\\" \ diff --git a/guh.pro b/guh.pro index a0b90f4f..673b296f 100644 --- a/guh.pro +++ b/guh.pro @@ -51,7 +51,6 @@ disabletesting { } else { message("Building guh with tests") SUBDIRS += tests - DEFINES += TESTING_ENABLED } # Bluetooth LE support diff --git a/server/jsonrpc/jsontypes.cpp b/server/jsonrpc/jsontypes.cpp index ddbabfc9..985e43c9 100644 --- a/server/jsonrpc/jsontypes.cpp +++ b/server/jsonrpc/jsontypes.cpp @@ -532,7 +532,10 @@ QVariantMap JsonTypes::packStateEvaluator(const StateEvaluator &stateEvaluator) foreach (const StateEvaluator &childEvaluator, stateEvaluator.childEvaluators()) { childEvaluators.append(packStateEvaluator(childEvaluator)); } - variantMap.insert("operator", stateOperator().at(stateEvaluator.operatorType())); + + if (!childEvaluators.isEmpty() || stateEvaluator.stateDescriptor().isValid()) + variantMap.insert("operator", stateOperator().at(stateEvaluator.operatorType())); + if (childEvaluators.count() > 0) { variantMap.insert("childEvaluators", childEvaluators); } @@ -817,12 +820,21 @@ QVariantMap JsonTypes::packRepeatingOption(const RepeatingOption &option) { QVariantMap optionVariant; optionVariant.insert("mode", s_repeatingMode.at(option.mode())); - if (!option.weekDays().isEmpty()) - optionVariant.insert("weekDays", QVariant::fromValue< QList >(option.weekDays())); - - if (!option.monthDays().isEmpty()) - optionVariant.insert("monthDays", QVariant::fromValue< QList >(option.monthDays())); + if (!option.weekDays().isEmpty()) { + QVariantList weekDaysVariantList; + foreach (const int& weekDay, option.weekDays()) { + weekDaysVariantList.append(QVariant(weekDay)); + } + optionVariant.insert("weekDays", weekDaysVariantList); + } + if (!option.monthDays().isEmpty()) { + QVariantList monthDaysVariantList; + foreach (const int& monthDay, option.monthDays()) { + monthDaysVariantList.append(QVariant(monthDay)); + } + optionVariant.insert("monthDays", monthDaysVariantList); + } return optionVariant; } @@ -855,8 +867,8 @@ QVariantMap JsonTypes::packTimeEventItem(const TimeEventItem &timeEventItem) if (!timeEventItem.time().isNull()) timeEventItemVariant.insert("time", timeEventItem.time().toString("hh:mm")); - if (!timeEventItem.repatingOption().isEmtpy()) - timeEventItemVariant.insert("repeating", packRepeatingOption(timeEventItem.repatingOption())); + if (!timeEventItem.repeatingOption().isEmtpy()) + timeEventItemVariant.insert("repeating", packRepeatingOption(timeEventItem.repeatingOption())); return timeEventItemVariant; } @@ -874,8 +886,13 @@ QVariantMap JsonTypes::packTimeDescriptor(const TimeDescriptor &timeDescriptor) timeDescriptorVariant.insert("calendarItems", calendarItems); } - - // TODO: TimeEventItems + if (!timeDescriptor.timeEventItems().isEmpty()) { + QVariantList timeEventItems; + foreach (const TimeEventItem &timeEventItem, timeDescriptor.timeEventItems()) { + timeEventItems.append(packTimeEventItem(timeEventItem)); + } + timeDescriptorVariant.insert("timeEventItems", timeEventItems); + } return timeDescriptorVariant; } @@ -1279,12 +1296,13 @@ RepeatingOption JsonTypes::unpackRepeatingOption(const QVariantMap &repeatingOpt CalendarItem JsonTypes::unpackCalendarItem(const QVariantMap &calendarItemMap) { CalendarItem calendarItem; + calendarItem.setDuration(calendarItemMap.value("duration").toUInt()); if (calendarItemMap.contains("datetime")) calendarItem.setDateTime(QDateTime::fromTime_t(calendarItemMap.value("datetime").toUInt())); if (calendarItemMap.contains("startTime")) - calendarItem.setStartTime(calendarItemMap.value("startTime").toTime()); + calendarItem.setStartTime(QTime::fromString(calendarItemMap.value("startTime").toString(), "hh:mm")); if (calendarItemMap.contains("repeating")) calendarItem.setRepeatingOption(unpackRepeatingOption(calendarItemMap.value("repeating").toMap())); @@ -1322,6 +1340,14 @@ TimeDescriptor JsonTypes::unpackTimeDescriptor(const QVariantMap &timeDescriptor timeDescriptor.setCalendarItems(calendarItems); } + if (timeDescriptorMap.contains("timeEventItems")) { + QList timeEventItems; + foreach (const QVariant &timeEventItemValiant, timeDescriptorMap.value("timeEventItems").toList()) { + timeEventItems.append(unpackTimeEventItem(timeEventItemValiant.toMap())); + } + timeDescriptor.setTimeEventItems(timeEventItems); + } + return timeDescriptor; } @@ -1374,22 +1400,30 @@ QPair JsonTypes::validateProperty(const QVariant &templateValue, if (strippedTemplateValue == JsonTypes::basicTypeToString(JsonTypes::Variant)) { return report(true, ""); } - if (strippedTemplateValue == JsonTypes::basicTypeToString(JsonTypes::Uuid)) { + if (strippedTemplateValue == JsonTypes::basicTypeToString(QVariant::Uuid)) { QString errorString = QString("Param %1 is not a uuid.").arg(value.toString()); return report(value.canConvert(QVariant::Uuid), errorString); } - if (strippedTemplateValue == JsonTypes::basicTypeToString(JsonTypes::String)) { + if (strippedTemplateValue == JsonTypes::basicTypeToString(QVariant::String)) { QString errorString = QString("Param %1 is not a string.").arg(value.toString()); return report(value.canConvert(QVariant::String), errorString); } - if (strippedTemplateValue == JsonTypes::basicTypeToString(JsonTypes::Bool)) { + if (strippedTemplateValue == JsonTypes::basicTypeToString(QVariant::Bool)) { QString errorString = QString("Param %1 is not a bool.").arg(value.toString()); return report(value.canConvert(QVariant::Bool), errorString); } - if (strippedTemplateValue == JsonTypes::basicTypeToString(JsonTypes::Int)) { + if (strippedTemplateValue == JsonTypes::basicTypeToString(QVariant::Int)) { QString errorString = QString("Param %1 is not a int.").arg(value.toString()); return report(value.canConvert(QVariant::Int), errorString); } + if (strippedTemplateValue == JsonTypes::basicTypeToString(QVariant::UInt)) { + QString errorString = QString("Param %1 is not a uint.").arg(value.toString()); + return report(value.canConvert(QVariant::UInt), errorString); + } + if (strippedTemplateValue == JsonTypes::basicTypeToString(QVariant::Time)) { + QString errorString = QString("Param %1 is not a time (hh:mm).").arg(value.toString()); + return report(value.canConvert(QVariant::Time), errorString); + } qCWarning(dcJsonRpc) << QString("Unhandled property type: %1 (expected: %2)").arg(value.toString()).arg(strippedTemplateValue); QString errorString = QString("Unhandled property type: %1 (expected: %2)").arg(value.toString()).arg(strippedTemplateValue); return report(false, errorString); diff --git a/server/ruleengine.cpp b/server/ruleengine.cpp index 8ef914a6..34fe7d3f 100644 --- a/server/ruleengine.cpp +++ b/server/ruleengine.cpp @@ -326,10 +326,52 @@ RuleEngine::RuleError RuleEngine::addRule(const Rule &rule, bool fromEdit) // Check state evaluator if (!rule.stateEvaluator().isValid()) { - qCWarning(dcRuleEngine) << "Got an invalid StateEvaluator."; + qCWarning(dcRuleEngine) << "Cannot create rule. Got an invalid StateEvaluator."; return RuleErrorInvalidStateEvaluatorValue; } + // Check time descriptor + if (!rule.timeDescriptor().isEmpty()) { + + if (rule.timeDescriptor().isValid()) { + qCDebug(dcRuleEngine()) << "Cannot create rule. Got invalid timeDescriptor."; + return RuleErrorInvalidTimeDescriptor; + } + + // validate CalendarItems + if (!rule.timeDescriptor().calendarItems().isEmpty()) { + foreach (const CalendarItem &calendarItem, rule.timeDescriptor().calendarItems()) { + if (!calendarItem.isValid()) { + qCDebug(dcRuleEngine()) << "Cannot create rule. Got invalid calendarItem."; + return RuleErrorInvalidCalendarItem; + } + + // validate RepeatingOptions + if (!calendarItem.repeatingOption().isEmtpy() && !calendarItem.repeatingOption().isValid()) { + qCDebug(dcRuleEngine()) << "Cannot create rule. Got invalid repeatingOption in calendarItem."; + return RuleErrorInvalidRepeatingOption; + } + } + } + + // validate TimeEventItems + if (!rule.timeDescriptor().timeEventItems().isEmpty()) { + foreach (const TimeEventItem &timeEventItem, rule.timeDescriptor().timeEventItems()) { + if (!timeEventItem.isValid()) { + qCDebug(dcRuleEngine()) << "Cannot create rule. Got invalid timeEventItem."; + return RuleErrorInvalidTimeEventItem; + } + + // validate RepeatingOptions + if (!timeEventItem.repeatingOption().isEmtpy() && !timeEventItem.repeatingOption().isValid()) { + qCDebug(dcRuleEngine()) << "Cannot create rule. Got invalid repeatingOption in timeEventItem."; + return RuleErrorInvalidRepeatingOption; + } + } + } + } + + // Check actions foreach (const RuleAction &action, rule.actions()) { Device *device = GuhCore::instance()->deviceManager()->findConfiguredDevice(action.deviceId()); diff --git a/server/ruleengine.h b/server/ruleengine.h index b0a218f3..03c962f2 100644 --- a/server/ruleengine.h +++ b/server/ruleengine.h @@ -54,6 +54,10 @@ public: RuleErrorInvalidStateEvaluatorValue, RuleErrorTypesNotMatching, RuleErrorNotExecutable, + RuleErrorInvalidTimeDescriptor, + RuleErrorInvalidRepeatingOption, + RuleErrorInvalidCalendarItem, + RuleErrorInvalidTimeEventItem, RuleErrorContainsEventBasesAction, RuleErrorNoExitActions }; diff --git a/server/time/repeatingoption.cpp b/server/time/repeatingoption.cpp index f4e6e526..d30e03ec 100644 --- a/server/time/repeatingoption.cpp +++ b/server/time/repeatingoption.cpp @@ -77,6 +77,7 @@ */ #include "repeatingoption.h" +#include "loggingcategories.h" #include @@ -133,6 +134,22 @@ bool RepeatingOption::isEmtpy() const /*! Returns true if this \l{RepeatingOption} is valid. */ bool RepeatingOption::isValid() const { + // Validate weekdays range + foreach (const uint &weekDay, m_weekDays) { + if (weekDay <= 0 || weekDay > 7) { + qCWarning(dcRuleEngine()) << "Invalid week day value:" << weekDay << ". Value out of range [1,7]."; + return false; + } + } + + // Validate monthdays range + foreach (const uint &monthDay, m_monthDays) { + if (monthDay <= 0 || monthDay > 31) { + qCWarning(dcRuleEngine()) << "Invalid month day value:" << monthDay << ". Value out of range [1,31]."; + return false; + } + } + switch (m_mode) { case RepeatingModeNone: return m_weekDays.isEmpty() && m_monthDays.isEmpty(); diff --git a/server/time/timeeventitem.cpp b/server/time/timeeventitem.cpp index f9de0b88..716c29a9 100644 --- a/server/time/timeeventitem.cpp +++ b/server/time/timeeventitem.cpp @@ -64,7 +64,7 @@ void TimeEventItem::setTime(const QTime &time) } /*! Returns the \l{RepeatingOption} of this \l{TimeEventItem}. */ -RepeatingOption TimeEventItem::repatingOption() const +RepeatingOption TimeEventItem::repeatingOption() const { return m_repeatingOption; } diff --git a/server/time/timeeventitem.h b/server/time/timeeventitem.h index ed6a3d88..3eb53792 100644 --- a/server/time/timeeventitem.h +++ b/server/time/timeeventitem.h @@ -38,7 +38,7 @@ public: QTime time() const; void setTime(const QTime &time); - RepeatingOption repatingOption() const; + RepeatingOption repeatingOption() const; void setRepeatingOption(const RepeatingOption &repeatingOption); // TODO spectioalDayTime diff --git a/tests/auto/timemanager/testtimemanager.cpp b/tests/auto/timemanager/testtimemanager.cpp index 80e81358..abaa8ebc 100644 --- a/tests/auto/timemanager/testtimemanager.cpp +++ b/tests/auto/timemanager/testtimemanager.cpp @@ -43,6 +43,11 @@ private slots: void addTimeDescriptor_data(); void addTimeDescriptor(); +private: + QVariantMap createCalendarItem(const QString &time = "00:00", const uint &duration = 0, const QVariantMap &repeatingOption = QVariantMap()) const; + QVariantMap createTimeDescriptorCalendar(const QVariantMap calendarItem) const; + QVariantMap createTimeDescriptorCalendar(const QVariantList calendarItems) const; + }; void TestTimeManager::changeTimeZone_data() @@ -82,5 +87,123 @@ void TestTimeManager::changeTimeZone() } +void TestTimeManager::addTimeDescriptor_data() +{ + // valid RepeatingOptions + QVariantMap repeatingOptionDaily; + repeatingOptionDaily.insert("mode", "RepeatingModeDaily"); + + QVariantMap repeatingOptionWeeklyMultiple; + repeatingOptionWeeklyMultiple.insert("mode", "RepeatingModeWeekly"); + repeatingOptionWeeklyMultiple.insert("weekDays", QVariantList() << 2 << 4 << 5); + + QVariantMap repeatingOptionMonthlyMultiple; + repeatingOptionMonthlyMultiple.insert("mode", "RepeatingModeMonthly"); + repeatingOptionMonthlyMultiple.insert("monthDays", QVariantList() << 20 << 14 << 5); + + // invalid RepeatingOptions + QVariantMap repeatingOptionInvalidWeekly; + repeatingOptionInvalidWeekly.insert("mode", "RepeatingModeWeekly"); + repeatingOptionInvalidWeekly.insert("monthDays", QVariantList() << 12 << 2 << 7); + + QVariantMap repeatingOptionInvalidMonthly; + repeatingOptionInvalidMonthly.insert("mode", "RepeatingModeMonthly"); + repeatingOptionInvalidMonthly.insert("weekDays", QVariantList() << 1 << 2 << 7); + + QVariantMap repeatingOptionInvalidWeekDays; + repeatingOptionInvalidWeekDays.insert("mode", "RepeatingModeWeekly"); + repeatingOptionInvalidWeekDays.insert("weekDays", QVariantList() << -1); + + QVariantMap repeatingOptionInvalidWeekDays2; + repeatingOptionInvalidWeekDays2.insert("mode", "RepeatingModeWeekly"); + repeatingOptionInvalidWeekDays2.insert("weekDays", QVariantList() << 8); + + QVariantMap repeatingOptionInvalidMonthDays; + repeatingOptionInvalidMonthDays.insert("mode", "RepeatingModeMonthly"); + repeatingOptionInvalidMonthDays.insert("monthDays", QVariantList() << -1); + + QVariantMap repeatingOptionInvalidMonthDays2; + repeatingOptionInvalidMonthDays2.insert("mode", "RepeatingModeMonthly"); + repeatingOptionInvalidMonthDays2.insert("monthDays", QVariantList() << 32); + + // Multiple calendar items + QVariantList calendarItems; + calendarItems.append(createCalendarItem("08:00", 5, repeatingOptionDaily)); + calendarItems.append(createCalendarItem("09:00", 5, repeatingOptionWeeklyMultiple)); + + + QTest::addColumn("timeDescriptor"); + QTest::addColumn("error"); + + + QTest::newRow("valid: single calendarItem") << createTimeDescriptorCalendar(createCalendarItem("08:00", 5)) << RuleEngine::RuleErrorNoError; + QTest::newRow("valid: single calendarItem - daily") << createTimeDescriptorCalendar(createCalendarItem("08:00", 5, repeatingOptionDaily)) << RuleEngine::RuleErrorNoError; + QTest::newRow("valid: calendarItems - weekly - multiple days") << createTimeDescriptorCalendar(calendarItems) << RuleEngine::RuleErrorNoError; + QTest::newRow("valid: calendarItem - monthly - multiple days") << createTimeDescriptorCalendar(createCalendarItem("23:00", 5, repeatingOptionMonthlyMultiple)) << RuleEngine::RuleErrorNoError; + + QTest::newRow("invalid: calendarItem - monthly - weekDays") << createTimeDescriptorCalendar(createCalendarItem("13:13", 5, repeatingOptionInvalidMonthly)) << RuleEngine::RuleErrorInvalidRepeatingOption; + QTest::newRow("invalid: calendarItem - weekly - monthDays") << createTimeDescriptorCalendar(createCalendarItem("15:30", 20, repeatingOptionInvalidWeekly)) << RuleEngine::RuleErrorInvalidRepeatingOption; + QTest::newRow("invalid: calendarItem - invalid weekdays (negative)") << createTimeDescriptorCalendar(createCalendarItem("13:13", 5, repeatingOptionInvalidWeekDays)) << RuleEngine::RuleErrorInvalidRepeatingOption; + QTest::newRow("invalid: calendarItem - invalid weekdays (to big)") << createTimeDescriptorCalendar(createCalendarItem("13:13", 5, repeatingOptionInvalidWeekDays2)) << RuleEngine::RuleErrorInvalidRepeatingOption; + QTest::newRow("invalid: calendarItem - invalid monthdays (negative)") << createTimeDescriptorCalendar(createCalendarItem("13:13", 5, repeatingOptionInvalidMonthDays)) << RuleEngine::RuleErrorInvalidRepeatingOption; + QTest::newRow("invalid: calendarItem - invalid monthdays (to big)") << createTimeDescriptorCalendar(createCalendarItem("13:13", 5, repeatingOptionInvalidMonthDays2)) << RuleEngine::RuleErrorInvalidRepeatingOption; + + +} + +void TestTimeManager::addTimeDescriptor() +{ + QFETCH(QVariantMap, timeDescriptor); + QFETCH(RuleEngine::RuleError, error); + + // ADD the rule + QVariantMap params; QVariantMap action; + action.insert("actionTypeId", mockActionIdNoParams); + action.insert("deviceId", m_mockDeviceId); + action.insert("ruleActionParams", QVariantList()); + params.insert("name", "TimeBased rule"); + params.insert("timeDescriptor", timeDescriptor); + params.insert("actions", QVariantList() << action); + + QVariant response = injectAndWait("Rules.AddRule", params); + verifyRuleError(response, error); + + if (error != RuleEngine::RuleErrorNoError) + return; + + // Print rule + RuleId newRuleId = RuleId(response.toMap().value("params").toMap().value("ruleId").toString()); + params.clear(); + params.insert("ruleId", newRuleId); + response = injectAndWait("Rules.GetRuleDetails", params); + QVariantMap rule = response.toMap().value("params").toMap().value("rule").toMap(); + qDebug() << QJsonDocument::fromVariant(rule).toJson(); +} + +QVariantMap TestTimeManager::createCalendarItem(const QString &time, const uint &duration, const QVariantMap &repeatingOption) const +{ + QVariantMap calendarItem; + calendarItem.insert("startTime", time); + calendarItem.insert("duration", duration); + if (!repeatingOption.isEmpty()) + calendarItem.insert("repeating", repeatingOption); + + return calendarItem; +} + +QVariantMap TestTimeManager::createTimeDescriptorCalendar(const QVariantMap calendarItem) const +{ + QVariantMap timeDescriptorCalendar; + timeDescriptorCalendar.insert("calendarItems", QVariantList() << calendarItem); + return timeDescriptorCalendar; +} + +QVariantMap TestTimeManager::createTimeDescriptorCalendar(const QVariantList calendarItems) const +{ + QVariantMap timeDescriptorCalendar; + timeDescriptorCalendar.insert("calendarItems", calendarItems); + return timeDescriptorCalendar; +} + #include "testtimemanager.moc" QTEST_MAIN(TestTimeManager)