From 85983229db8d915cc9241b329c0d9f27ec0f531a Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Wed, 15 Jul 2026 12:03:04 -0400 Subject: [PATCH 1/4] RDKEMW-20911 : Fix intent type as object and not string --- docs/openrpc/the-spec/firebolt-open-rpc.json | 12 ++++++------ src/json_types/actions.h | 5 +++-- test/component/actionsGeneratedTest.cpp | 14 +++++++++----- test/unit/actionsTest.cpp | 8 +++++--- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/docs/openrpc/the-spec/firebolt-open-rpc.json b/docs/openrpc/the-spec/firebolt-open-rpc.json index 2292d55..2d07437 100644 --- a/docs/openrpc/the-spec/firebolt-open-rpc.json +++ b/docs/openrpc/the-spec/firebolt-open-rpc.json @@ -71,8 +71,8 @@ "type": "object", "required": ["intent", "intentId"], "properties": { - "intent": { "type": "string" }, - "intentId": { "type": "integer" } + "intent": { "type": "object" }, + "intentId": { "type": "integer", "minimum": 0 } } } }, @@ -81,7 +81,7 @@ "name": "Get the current intent", "result": { "name": "Default Result", - "value": { "intent": "launch", "intentId": 1 } + "value": { "intent": {"action": "pre-load", "context": {"source": "system"}}, "intentId": 0 } } } ] @@ -117,8 +117,8 @@ "type": "object", "required": ["intent", "intentId"], "properties": { - "intent": { "type": "string" }, - "intentId": { "type": "integer" } + "intent": { "type": "object" }, + "intentId": { "type": "integer", "minimum": 0 } } } }, @@ -133,7 +133,7 @@ ], "result": { "name": "Default Result", - "value": { "intent": "launch", "intentId": 1 } + "value": { "intent": {"action": "pre-load", "context": {"source": "system"}}, "intentId": 0 } } } ] diff --git a/src/json_types/actions.h b/src/json_types/actions.h index 732775c..a27e961 100644 --- a/src/json_types/actions.h +++ b/src/json_types/actions.h @@ -36,8 +36,9 @@ namespace JsonData // Serialises any JSON value (object, string, …) to its compact JSON text // representation. Used for Actions.intent / Actions.onIntent whose wire format -// is the object {"intent":"...","intentId":N} but whose public C++ API surface -// exposes the whole document as a std::string, per the Firebolt 9 spec. +// is the object {"intent":{"action":"...","context":{...}},"intentId":N} but whose +// public C++ API surface exposes the whole document as a std::string, per the +// Firebolt 9 spec. class JsonString : public Firebolt::JSON::NL_Json_Basic { public: diff --git a/test/component/actionsGeneratedTest.cpp b/test/component/actionsGeneratedTest.cpp index a8105c0..c5e38c9 100644 --- a/test/component/actionsGeneratedTest.cpp +++ b/test/component/actionsGeneratedTest.cpp @@ -37,8 +37,10 @@ TEST_F(ActionsGeneratedCTest, Intent) auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); ASSERT_TRUE(result) << toError(result); auto parsed = nlohmann::json::parse(*result); - EXPECT_EQ(parsed.at("intent").get(), "launch"); - EXPECT_EQ(parsed.at("intentId").get(), 1); + EXPECT_TRUE(parsed.at("intent").is_object()); + EXPECT_EQ(parsed.at("intent").at("action").get(), "pre-load"); + EXPECT_EQ(parsed.at("intent").at("context").at("source").get(), "system"); + EXPECT_EQ(parsed.at("intentId").get(), 0u); } TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) @@ -47,8 +49,10 @@ TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) [&](const std::string& intent) { auto parsed = nlohmann::json::parse(intent); - EXPECT_EQ(parsed.at("intent").get(), "launch"); - EXPECT_EQ(parsed.at("intentId").get(), 1); + EXPECT_TRUE(parsed.at("intent").is_object()); + EXPECT_EQ(parsed.at("intent").at("action").get(), "pre-load"); + EXPECT_EQ(parsed.at("intent").at("context").at("source").get(), "system"); + EXPECT_EQ(parsed.at("intentId").get(), 0u); { std::lock_guard lock(mtx); eventReceived = true; @@ -59,7 +63,7 @@ TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) ASSERT_TRUE(id) << toError(id); verifyEventSubscription(id); - triggerEvent("Actions.onIntent", R"({"intent":"launch","intentId":1})"); + triggerEvent("Actions.onIntent", R"({"intent":{"action":"pre-load","context":{"source":"system"}},"intentId":0})"); verifyEventReceived(mtx, cv, eventReceived); auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id.value()); diff --git a/test/unit/actionsTest.cpp b/test/unit/actionsTest.cpp index feec5ed..773cc69 100644 --- a/test/unit/actionsTest.cpp +++ b/test/unit/actionsTest.cpp @@ -28,13 +28,15 @@ class ActionsUTest : public ::testing::Test, protected MockBase TEST_F(ActionsUTest, Start) { - mock_with_response("Actions.intent", nlohmann::json({{"intent", "launch"}, {"intentId", 1}})); + mock_with_response("Actions.intent", nlohmann::json({{"intent", {{"action", "pre-load"}, {"context", {{"source", "system"}}}}}, {"intentId", 0u}})); auto result = actionsImpl_.intent(); ASSERT_TRUE(result) << "ActionsImpl::intent() returned an error"; auto parsed = nlohmann::json::parse(*result); - EXPECT_EQ(parsed.at("intent").get(), "launch"); - EXPECT_EQ(parsed.at("intentId").get(), 1); + EXPECT_TRUE(parsed.at("intent").is_object()); + EXPECT_EQ(parsed.at("intent").at("action").get(), "pre-load"); + EXPECT_EQ(parsed.at("intent").at("context").at("source").get(), "system"); + EXPECT_EQ(parsed.at("intentId").get(), 0u); } TEST_F(ActionsUTest, SubscribeOnIntent) From a6d447677b828040a18e0a3f831117c55696b6f8 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Thu, 16 Jul 2026 13:57:23 -0400 Subject: [PATCH 2/4] RDKEMW-21724 : Update Actions module per Firebolt 9 spec --- docs/openrpc/the-spec/firebolt-open-rpc.json | 104 +++++++++++++++++-- include/firebolt/actions.h | 28 ++++- src/actions_impl.cpp | 19 +++- src/actions_impl.h | 7 +- src/json_types/actions.h | 20 ++-- test/api_test_app/apis/actionsDemo.cpp | 96 +++++++++++++++++ test/api_test_app/apis/actionsDemo.h | 30 ++++++ test/api_test_app/main.cpp | 2 + test/component/actionsGeneratedTest.cpp | 25 +++-- test/unit/actionsTest.cpp | 27 +++-- 10 files changed, 312 insertions(+), 46 deletions(-) create mode 100644 test/api_test_app/apis/actionsDemo.cpp create mode 100644 test/api_test_app/apis/actionsDemo.h diff --git a/docs/openrpc/the-spec/firebolt-open-rpc.json b/docs/openrpc/the-spec/firebolt-open-rpc.json index 2d07437..a790cee 100644 --- a/docs/openrpc/the-spec/firebolt-open-rpc.json +++ b/docs/openrpc/the-spec/firebolt-open-rpc.json @@ -69,10 +69,18 @@ "summary": "The current intent as a JSON document.", "schema": { "type": "object", - "required": ["intent", "intentId"], + "required": [ + "intent", + "intentId" + ], "properties": { - "intent": { "type": "object" }, - "intentId": { "type": "integer", "minimum": 0 } + "intent": { + "type": "object" + }, + "intentId": { + "type": "integer", + "minimum": 0 + } } } }, @@ -81,7 +89,15 @@ "name": "Get the current intent", "result": { "name": "Default Result", - "value": { "intent": {"action": "pre-load", "context": {"source": "system"}}, "intentId": 0 } + "value": { + "intent": { + "action": "pre-load", + "context": { + "source": "system" + } + }, + "intentId": 0 + } } } ] @@ -115,10 +131,18 @@ "summary": "The current intent as a JSON document.", "schema": { "type": "object", - "required": ["intent", "intentId"], + "required": [ + "intent", + "intentId" + ], "properties": { - "intent": { "type": "object" }, - "intentId": { "type": "integer", "minimum": 0 } + "intent": { + "type": "object" + }, + "intentId": { + "type": "integer", + "minimum": 0 + } } } }, @@ -133,7 +157,71 @@ ], "result": { "name": "Default Result", - "value": { "intent": {"action": "pre-load", "context": {"source": "system"}}, "intentId": 0 } + "value": { + "intent": { + "action": "pre-load", + "context": { + "source": "system" + } + }, + "intentId": 0 + } + } + } + ] + }, + { + "name": "Actions.start", + "summary": "Sends an intent to the platform.", + "tags": [ + { + "name": "capabilities", + "x-uses": [ + "xrn:firebolt:capability:actions:intent" + ] + } + ], + "params": [ + { + "name": "intent", + "summary": "The intent to send, as a JSON document.", + "required": true, + "schema": { + "type": "object" + } + }, + { + "name": "handlerAppId", + "summary": "Optional ID of the application that should handle the intent.", + "required": false, + "schema": { + "type": "string" + } + } + ], + "result": { + "name": "result", + "schema": { + "type": "null" + } + }, + "examples": [ + { + "name": "Start an intent", + "params": [ + { + "name": "intent", + "value": { + "action": "pre-load", + "context": { + "source": "system" + } + } + } + ], + "result": { + "name": "Default Result", + "value": null } } ] diff --git a/include/firebolt/actions.h b/include/firebolt/actions.h index 660abd6..b5396e6 100644 --- a/include/firebolt/actions.h +++ b/include/firebolt/actions.h @@ -26,22 +26,37 @@ #include #include #include -#include #include -#include namespace Firebolt::Actions { +struct IntentContext +{ + std::string source; +}; + +struct IntentData +{ + std::string action; + IntentContext context; +}; + +struct Intent +{ + IntentData intent; + unsigned intentId{0}; +}; + class IActions { public: virtual ~IActions() = default; - virtual Result intent() const = 0; + virtual Result intent() const = 0; - virtual Result subscribeOnIntent(std::function&& notification) = 0; - virtual Result subscribeOnIntentChanged(std::function&& notification) + virtual Result subscribeOnIntent(std::function&& notification) = 0; + virtual Result subscribeOnIntentChanged(std::function&& notification) { return subscribeOnIntent(std::move(notification)); } @@ -49,6 +64,9 @@ class IActions virtual Result unsubscribe(SubscriptionId id) = 0; virtual void unsubscribeAll() = 0; + virtual Result start(const std::string& intent, + std::optional handlerAppId = std::nullopt) const = 0; + }; // class IActions } // namespace Firebolt::Actions diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index e65fc4e..02462ef 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -32,14 +32,25 @@ ActionsImpl::ActionsImpl(Firebolt::Helpers::IHelper& helper) { } -Result ActionsImpl::intent() const +Result ActionsImpl::intent() const { - return helper_.get("Actions.intent"); + return helper_.get("Actions.intent"); } -Result ActionsImpl::subscribeOnIntent(std::function&& notification) +Result ActionsImpl::subscribeOnIntent(std::function&& notification) { - return subscriptionManager_.subscribe("Actions.onIntent", std::move(notification)); + return subscriptionManager_.subscribe("Actions.onIntent", std::move(notification)); +} + +Result ActionsImpl::start(const std::string& intent, std::optional handlerAppId) const +{ + nlohmann::json params; + params["intent"] = nlohmann::json::parse(intent); + if (handlerAppId) + { + params["handlerAppId"] = *handlerAppId; + } + return helper_.invoke("Actions.start", params); } Result ActionsImpl::unsubscribe(SubscriptionId id) diff --git a/src/actions_impl.h b/src/actions_impl.h index 4c8d6ba..f317573 100644 --- a/src/actions_impl.h +++ b/src/actions_impl.h @@ -36,9 +36,12 @@ class ActionsImpl : public IActions ActionsImpl& operator=(const ActionsImpl&) = delete; ~ActionsImpl() override = default; - Result intent() const override; + Result intent() const override; - Result subscribeOnIntent(std::function&& notification) override; + Result subscribeOnIntent(std::function&& notification) override; + + Result start(const std::string& intent, + std::optional handlerAppId) const override; Result unsubscribe(SubscriptionId id) override; void unsubscribeAll() override; diff --git a/src/json_types/actions.h b/src/json_types/actions.h index a27e961..0512c6c 100644 --- a/src/json_types/actions.h +++ b/src/json_types/actions.h @@ -34,19 +34,21 @@ namespace Firebolt::Actions namespace JsonData { -// Serialises any JSON value (object, string, …) to its compact JSON text -// representation. Used for Actions.intent / Actions.onIntent whose wire format -// is the object {"intent":{"action":"...","context":{...}},"intentId":N} but whose -// public C++ API surface exposes the whole document as a std::string, per the -// Firebolt 9 spec. -class JsonString : public Firebolt::JSON::NL_Json_Basic +// Deserialises the wire object {"intent":{"action":"...","context":{"source":"..."}},"intentId":N} +// into Firebolt::Actions::Intent. nlohmann stays hidden in this impl-layer header. +class JsonValue : public Firebolt::JSON::NL_Json_Basic { public: - void fromJson(const nlohmann::json& json) override { value_ = json.dump(); } - std::string value() const override { return value_; } + void fromJson(const nlohmann::json& json) override + { + value_.intent.action = json.at("intent").at("action").get(); + value_.intent.context.source = json.at("intent").at("context").at("source").get(); + value_.intentId = json.at("intentId").get(); + } + Intent value() const override { return value_; } private: - std::string value_; + Intent value_; }; } // namespace JsonData diff --git a/test/api_test_app/apis/actionsDemo.cpp b/test/api_test_app/apis/actionsDemo.cpp new file mode 100644 index 0000000..7e63eae --- /dev/null +++ b/test/api_test_app/apis/actionsDemo.cpp @@ -0,0 +1,96 @@ +/** + * Copyright 2026 Comcast Cable Communications Management, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "actionsDemo.h" +#include +#include +#include + +using namespace Firebolt; +using namespace Firebolt::Actions; + +ActionsDemo::ActionsDemo() + : DemoBase("Actions") +{ + methods_.push_back("Actions.intent"); + methods_.push_back("Actions.start"); + methods_.push_back("Actions.onIntent"); + methods_.push_back("Actions.unsubscribe"); + methods_.push_back("Actions.unsubscribeAll"); +} + +void ActionsDemo::runOption(const std::string& method) +{ + std::cout << "Running Actions method: " << method << std::endl; + + if (method == "Actions.intent") + { + auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); + if (succeed(r)) + { + std::cout << "Current Intent - action: " << r->intent.action + << ", source: " << r->intent.context.source + << ", intentId: " << r->intentId << std::endl; + } + } + else if (method == "Actions.start") + { + std::string intent = paramFromConsole("intent (JSON)", + R"({"action":"pre-load","context":{"source":"system"}})"); + std::string handlerAppIdStr = paramFromConsole("handlerAppId (leave empty to skip)", ""); + std::optional handlerAppId; + if (!handlerAppIdStr.empty()) + { + handlerAppId = handlerAppIdStr; + } + auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().start(intent, handlerAppId); + if (succeed(r)) + { + std::cout << "Actions.start: Success" << std::endl; + } + } + else if (method == "Actions.onIntent") + { + auto callback = [&](const Intent& payload) + { + std::cout << "Intent received - action: " << payload.intent.action + << ", source: " << payload.intent.context.source + << ", intentId: " << payload.intentId << std::endl; + }; + auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().subscribeOnIntent(std::move(callback)); + if (succeed(r)) + { + std::cout << "Subscribed to Actions.onIntent with Subscription ID: " << *r << std::endl; + } + } + else if (method == "Actions.unsubscribe") + { + std::string idStr = paramFromConsole("subscription ID", "0"); + SubscriptionId id = static_cast(std::stoul(idStr)); + auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id); + if (succeed(r)) + { + std::cout << "Unsubscribed from Actions subscription " << id << std::endl; + } + } + else if (method == "Actions.unsubscribeAll") + { + Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribeAll(); + std::cout << "Unsubscribed from all Actions subscriptions" << std::endl; + } +} diff --git a/test/api_test_app/apis/actionsDemo.h b/test/api_test_app/apis/actionsDemo.h new file mode 100644 index 0000000..ae564cc --- /dev/null +++ b/test/api_test_app/apis/actionsDemo.h @@ -0,0 +1,30 @@ +/** + * Copyright 2026 Comcast Cable Communications Management, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#pragma once + +#include "utils.h" +#include + +class ActionsDemo : public DemoBase +{ +public: + ActionsDemo(); + ~ActionsDemo() = default; + void runOption(const std::string& method) override; +}; diff --git a/test/api_test_app/main.cpp b/test/api_test_app/main.cpp index 611cfd7..a6c2f05 100644 --- a/test/api_test_app/main.cpp +++ b/test/api_test_app/main.cpp @@ -17,6 +17,7 @@ */ #include "accessibilityDemo.h" +#include "actionsDemo.h" #include "advertisingDemo.h" #include "deviceDemo.h" #include "discoveryDemo.h" @@ -157,6 +158,7 @@ int main(int argc, char** argv) std::vector> interfaces; interfaces.emplace_back(std::make_unique()); + interfaces.emplace_back(std::make_unique()); interfaces.emplace_back(std::make_unique()); interfaces.emplace_back(std::make_unique()); interfaces.emplace_back(std::make_unique()); diff --git a/test/component/actionsGeneratedTest.cpp b/test/component/actionsGeneratedTest.cpp index c5e38c9..2aac746 100644 --- a/test/component/actionsGeneratedTest.cpp +++ b/test/component/actionsGeneratedTest.cpp @@ -36,23 +36,19 @@ TEST_F(ActionsGeneratedCTest, Intent) { auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); ASSERT_TRUE(result) << toError(result); - auto parsed = nlohmann::json::parse(*result); - EXPECT_TRUE(parsed.at("intent").is_object()); - EXPECT_EQ(parsed.at("intent").at("action").get(), "pre-load"); - EXPECT_EQ(parsed.at("intent").at("context").at("source").get(), "system"); - EXPECT_EQ(parsed.at("intentId").get(), 0u); + EXPECT_EQ(result->intent.action, "pre-load"); + EXPECT_EQ(result->intent.context.source, "system"); + EXPECT_EQ(result->intentId, 0u); } TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) { auto id = Firebolt::IFireboltAccessor::Instance().ActionsInterface().subscribeOnIntent( - [&](const std::string& intent) + [&](const Firebolt::Actions::Intent& payload) { - auto parsed = nlohmann::json::parse(intent); - EXPECT_TRUE(parsed.at("intent").is_object()); - EXPECT_EQ(parsed.at("intent").at("action").get(), "pre-load"); - EXPECT_EQ(parsed.at("intent").at("context").at("source").get(), "system"); - EXPECT_EQ(parsed.at("intentId").get(), 0u); + EXPECT_EQ(payload.intent.action, "pre-load"); + EXPECT_EQ(payload.intent.context.source, "system"); + EXPECT_EQ(payload.intentId, 0u); { std::lock_guard lock(mtx); eventReceived = true; @@ -69,3 +65,10 @@ TEST_F(ActionsGeneratedCTest, SubscribeOnIntent) auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id.value()); verifyUnsubscribeResult(result); } + +TEST_F(ActionsGeneratedCTest, Start) +{ + auto result = Firebolt::IFireboltAccessor::Instance().ActionsInterface().start( + R"({"action":"pre-load","context":{"source":"system"}})"); + ASSERT_TRUE(result) << toError(result); +} diff --git a/test/unit/actionsTest.cpp b/test/unit/actionsTest.cpp index 773cc69..85b7676 100644 --- a/test/unit/actionsTest.cpp +++ b/test/unit/actionsTest.cpp @@ -20,23 +20,24 @@ #include "json_engine.h" #include "mock_helper.h" +using ::testing::_; +using ::testing::Invoke; + class ActionsUTest : public ::testing::Test, protected MockBase { protected: Firebolt::Actions::ActionsImpl actionsImpl_{mockHelper}; }; -TEST_F(ActionsUTest, Start) +TEST_F(ActionsUTest, Intent) { mock_with_response("Actions.intent", nlohmann::json({{"intent", {{"action", "pre-load"}, {"context", {{"source", "system"}}}}}, {"intentId", 0u}})); auto result = actionsImpl_.intent(); ASSERT_TRUE(result) << "ActionsImpl::intent() returned an error"; - auto parsed = nlohmann::json::parse(*result); - EXPECT_TRUE(parsed.at("intent").is_object()); - EXPECT_EQ(parsed.at("intent").at("action").get(), "pre-load"); - EXPECT_EQ(parsed.at("intent").at("context").at("source").get(), "system"); - EXPECT_EQ(parsed.at("intentId").get(), 0u); + EXPECT_EQ(result->intent.action, "pre-load"); + EXPECT_EQ(result->intent.context.source, "system"); + EXPECT_EQ(result->intentId, 0u); } TEST_F(ActionsUTest, SubscribeOnIntent) @@ -44,7 +45,7 @@ TEST_F(ActionsUTest, SubscribeOnIntent) nlohmann::json expectedValue = 1; mockSubscribe("Actions.onIntent"); - auto result = actionsImpl_.subscribeOnIntent([&](const std::string& /*value*/) {}); + auto result = actionsImpl_.subscribeOnIntent([&](const Firebolt::Actions::Intent& /*value*/) {}); ASSERT_TRUE(result) << "ActionsImpl::subscribeOnIntent() returned an error"; EXPECT_EQ(*result, expectedValue); @@ -52,3 +53,15 @@ TEST_F(ActionsUTest, SubscribeOnIntent) auto unsubResult = actionsImpl_.unsubscribe(*result); ASSERT_TRUE(unsubResult) << "ActionsImpl::unsubscribe() returned an error"; } + +TEST_F(ActionsUTest, Start) +{ + nlohmann::json expectedParams; + expectedParams["intent"] = {{"action", "pre-load"}, {"context", {{"source", "system"}}}}; + EXPECT_CALL(mockHelper, invoke("Actions.start", expectedParams)) + .WillOnce(Invoke([&](const std::string& /*methodName*/, const nlohmann::json& /*parameters*/) + { return Firebolt::Result{Firebolt::Error::None}; })); + + auto result = actionsImpl_.start(R"({"action":"pre-load","context":{"source":"system"}})"); + ASSERT_TRUE(result) << "ActionsImpl::start() returned an error"; +} From 7fe75597d63ae7adb6f079e181232722fb1e0618 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Thu, 16 Jul 2026 15:03:49 -0400 Subject: [PATCH 3/4] RDKEMW-21724 : Address copilot comments --- include/firebolt/actions.h | 2 +- src/actions_impl.cpp | 9 ++++++++- src/actions_impl.h | 2 +- src/json_types/actions.h | 16 ++++++++++++---- test/unit/actionsTest.cpp | 7 +++++++ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/include/firebolt/actions.h b/include/firebolt/actions.h index b5396e6..e254232 100644 --- a/include/firebolt/actions.h +++ b/include/firebolt/actions.h @@ -45,7 +45,7 @@ struct IntentData struct Intent { IntentData intent; - unsigned intentId{0}; + uint32_t intentId{0}; }; class IActions diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index 02462ef..a7a286c 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -45,7 +45,14 @@ Result ActionsImpl::subscribeOnIntent(std::function ActionsImpl::start(const std::string& intent, std::optional handlerAppId) const { nlohmann::json params; - params["intent"] = nlohmann::json::parse(intent); + try + { + params["intent"] = nlohmann::json::parse(intent); + } + catch (const nlohmann::json::parse_error&) + { + return Firebolt::Result{Firebolt::Error::InvalidParams}; + } if (handlerAppId) { params["handlerAppId"] = *handlerAppId; diff --git a/src/actions_impl.h b/src/actions_impl.h index f317573..031dcad 100644 --- a/src/actions_impl.h +++ b/src/actions_impl.h @@ -41,7 +41,7 @@ class ActionsImpl : public IActions Result subscribeOnIntent(std::function&& notification) override; Result start(const std::string& intent, - std::optional handlerAppId) const override; + std::optional handlerAppId = std::nullopt) const override; Result unsubscribe(SubscriptionId id) override; void unsubscribeAll() override; diff --git a/src/json_types/actions.h b/src/json_types/actions.h index 0512c6c..02f3049 100644 --- a/src/json_types/actions.h +++ b/src/json_types/actions.h @@ -26,7 +26,7 @@ #include "firebolt/actions.h" #include #include -#include +#include namespace Firebolt::Actions { @@ -41,9 +41,17 @@ class JsonValue : public Firebolt::JSON::NL_Json_Basic public: void fromJson(const nlohmann::json& json) override { - value_.intent.action = json.at("intent").at("action").get(); - value_.intent.context.source = json.at("intent").at("context").at("source").get(); - value_.intentId = json.at("intentId").get(); + if (!checkRequiredFields(json, {"intent", "intentId"}) || + !json["intent"].is_object() || + !checkRequiredFields(json["intent"], {"action", "context"}) || + !json["intent"]["context"].is_object() || + !checkRequiredFields(json["intent"]["context"], {"source"})) + { + throw std::invalid_argument("Missing required fields in JSON"); + } + value_.intent.action = json["intent"]["action"].get(); + value_.intent.context.source = json["intent"]["context"]["source"].get(); + value_.intentId = json["intentId"].get(); } Intent value() const override { return value_; } diff --git a/test/unit/actionsTest.cpp b/test/unit/actionsTest.cpp index 85b7676..24ca675 100644 --- a/test/unit/actionsTest.cpp +++ b/test/unit/actionsTest.cpp @@ -65,3 +65,10 @@ TEST_F(ActionsUTest, Start) auto result = actionsImpl_.start(R"({"action":"pre-load","context":{"source":"system"}})"); ASSERT_TRUE(result) << "ActionsImpl::start() returned an error"; } + +TEST_F(ActionsUTest, StartInvalidJson) +{ + auto result = actionsImpl_.start("not-valid-json"); + ASSERT_FALSE(result) << "ActionsImpl::start() should fail for invalid JSON"; + EXPECT_EQ(result.error(), Firebolt::Error::InvalidParams); +} From 1a29962bb1dfd6e519abeab62566d9f527211080 Mon Sep 17 00:00:00 2001 From: swethasukumarr Date: Thu, 16 Jul 2026 15:59:32 -0400 Subject: [PATCH 4/4] RDKEMW-21724 : Address copilot comments --- include/firebolt/actions.h | 4 ++-- src/actions_impl.cpp | 7 ++++++- src/actions_impl.h | 3 +-- src/json_types/actions.h | 10 ++++------ test/api_test_app/apis/actionsDemo.cpp | 20 +++++++++++++------- test/api_test_app/apis/lifecycleDemo.cpp | 6 ++++-- test/api_test_app/apis/lifecycleDemo.h | 1 + test/unit/actionsTest.cpp | 11 ++++++++++- 8 files changed, 41 insertions(+), 21 deletions(-) diff --git a/include/firebolt/actions.h b/include/firebolt/actions.h index e254232..4fc0484 100644 --- a/include/firebolt/actions.h +++ b/include/firebolt/actions.h @@ -38,14 +38,14 @@ struct IntentContext struct IntentData { - std::string action; + std::string action; IntentContext context; }; struct Intent { IntentData intent; - uint32_t intentId{0}; + uint32_t intentId{0}; }; class IActions diff --git a/src/actions_impl.cpp b/src/actions_impl.cpp index a7a286c..7510744 100644 --- a/src/actions_impl.cpp +++ b/src/actions_impl.cpp @@ -47,7 +47,12 @@ Result ActionsImpl::start(const std::string& intent, std::optional{Firebolt::Error::InvalidParams}; + } + params["intent"] = std::move(parsedIntent); } catch (const nlohmann::json::parse_error&) { diff --git a/src/actions_impl.h b/src/actions_impl.h index 031dcad..40ef403 100644 --- a/src/actions_impl.h +++ b/src/actions_impl.h @@ -40,8 +40,7 @@ class ActionsImpl : public IActions Result subscribeOnIntent(std::function&& notification) override; - Result start(const std::string& intent, - std::optional handlerAppId = std::nullopt) const override; + Result start(const std::string& intent, std::optional handlerAppId = std::nullopt) const override; Result unsubscribe(SubscriptionId id) override; void unsubscribeAll() override; diff --git a/src/json_types/actions.h b/src/json_types/actions.h index 02f3049..1c661cd 100644 --- a/src/json_types/actions.h +++ b/src/json_types/actions.h @@ -41,17 +41,15 @@ class JsonValue : public Firebolt::JSON::NL_Json_Basic public: void fromJson(const nlohmann::json& json) override { - if (!checkRequiredFields(json, {"intent", "intentId"}) || - !json["intent"].is_object() || - !checkRequiredFields(json["intent"], {"action", "context"}) || - !json["intent"]["context"].is_object() || + if (!checkRequiredFields(json, {"intent", "intentId"}) || !json["intent"].is_object() || + !checkRequiredFields(json["intent"], {"action", "context"}) || !json["intent"]["context"].is_object() || !checkRequiredFields(json["intent"]["context"], {"source"})) { throw std::invalid_argument("Missing required fields in JSON"); } - value_.intent.action = json["intent"]["action"].get(); + value_.intent.action = json["intent"]["action"].get(); value_.intent.context.source = json["intent"]["context"]["source"].get(); - value_.intentId = json["intentId"].get(); + value_.intentId = json["intentId"].get(); } Intent value() const override { return value_; } diff --git a/test/api_test_app/apis/actionsDemo.cpp b/test/api_test_app/apis/actionsDemo.cpp index 7e63eae..2533bec 100644 --- a/test/api_test_app/apis/actionsDemo.cpp +++ b/test/api_test_app/apis/actionsDemo.cpp @@ -20,6 +20,7 @@ #include #include #include +#include using namespace Firebolt; using namespace Firebolt::Actions; @@ -43,15 +44,13 @@ void ActionsDemo::runOption(const std::string& method) auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().intent(); if (succeed(r)) { - std::cout << "Current Intent - action: " << r->intent.action - << ", source: " << r->intent.context.source + std::cout << "Current Intent - action: " << r->intent.action << ", source: " << r->intent.context.source << ", intentId: " << r->intentId << std::endl; } } else if (method == "Actions.start") { - std::string intent = paramFromConsole("intent (JSON)", - R"({"action":"pre-load","context":{"source":"system"}})"); + std::string intent = paramFromConsole("intent (JSON)", R"({"action":"pre-load","context":{"source":"system"}})"); std::string handlerAppIdStr = paramFromConsole("handlerAppId (leave empty to skip)", ""); std::optional handlerAppId; if (!handlerAppIdStr.empty()) @@ -69,8 +68,8 @@ void ActionsDemo::runOption(const std::string& method) auto callback = [&](const Intent& payload) { std::cout << "Intent received - action: " << payload.intent.action - << ", source: " << payload.intent.context.source - << ", intentId: " << payload.intentId << std::endl; + << ", source: " << payload.intent.context.source << ", intentId: " << payload.intentId + << std::endl; }; auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().subscribeOnIntent(std::move(callback)); if (succeed(r)) @@ -81,7 +80,14 @@ void ActionsDemo::runOption(const std::string& method) else if (method == "Actions.unsubscribe") { std::string idStr = paramFromConsole("subscription ID", "0"); - SubscriptionId id = static_cast(std::stoul(idStr)); + SubscriptionId id = 0; + try + { + id = static_cast(std::stoul(idStr)); + } + catch (const std::exception&) + { + } auto r = Firebolt::IFireboltAccessor::Instance().ActionsInterface().unsubscribe(id); if (succeed(r)) { diff --git a/test/api_test_app/apis/lifecycleDemo.cpp b/test/api_test_app/apis/lifecycleDemo.cpp index a550527..f05c133 100644 --- a/test/api_test_app/apis/lifecycleDemo.cpp +++ b/test/api_test_app/apis/lifecycleDemo.cpp @@ -73,15 +73,17 @@ void LifecycleDemo::runOption(const std::string& method) Firebolt::IFireboltAccessor::Instance().LifecycleInterface().subscribeOnStateChanged(std::move(callback)); if (succeed(r)) { + lastSubscriptionId_ = *r; std::cout << "Subscribed to Lifecycle state changes with Subscription ID: " << *r << std::endl; } } else if (method == "Lifecycle2.unsubscribe") { - SubscriptionId id = 0; + SubscriptionId id = lastSubscriptionId_; try { - id = static_cast(std::stoul(paramFromConsole("Subscription ID to unsubscribe", "0"))); + id = static_cast( + std::stoul(paramFromConsole("Subscription ID to unsubscribe", std::to_string(lastSubscriptionId_)))); } catch (const std::exception&) { diff --git a/test/api_test_app/apis/lifecycleDemo.h b/test/api_test_app/apis/lifecycleDemo.h index 9ccf509..d423741 100644 --- a/test/api_test_app/apis/lifecycleDemo.h +++ b/test/api_test_app/apis/lifecycleDemo.h @@ -31,4 +31,5 @@ class LifecycleDemo : public DemoBase private: Firebolt::Lifecycle::LifecycleState currentState_; + Firebolt::SubscriptionId lastSubscriptionId_{0}; }; diff --git a/test/unit/actionsTest.cpp b/test/unit/actionsTest.cpp index 24ca675..c1b5638 100644 --- a/test/unit/actionsTest.cpp +++ b/test/unit/actionsTest.cpp @@ -31,7 +31,9 @@ class ActionsUTest : public ::testing::Test, protected MockBase TEST_F(ActionsUTest, Intent) { - mock_with_response("Actions.intent", nlohmann::json({{"intent", {{"action", "pre-load"}, {"context", {{"source", "system"}}}}}, {"intentId", 0u}})); + mock_with_response("Actions.intent", + nlohmann::json({{"intent", {{"action", "pre-load"}, {"context", {{"source", "system"}}}}}, + {"intentId", 0u}})); auto result = actionsImpl_.intent(); ASSERT_TRUE(result) << "ActionsImpl::intent() returned an error"; @@ -72,3 +74,10 @@ TEST_F(ActionsUTest, StartInvalidJson) ASSERT_FALSE(result) << "ActionsImpl::start() should fail for invalid JSON"; EXPECT_EQ(result.error(), Firebolt::Error::InvalidParams); } + +TEST_F(ActionsUTest, StartNonObjectJson) +{ + auto result = actionsImpl_.start(R"([1,2,3])"); + ASSERT_FALSE(result) << "ActionsImpl::start() should fail for non-object JSON"; + EXPECT_EQ(result.error(), Firebolt::Error::InvalidParams); +}