Device apis cleanup (#676)

* feat: DRYing, device.name cleanup

* chore: clippy

* cleanup: remove device.sku

* cleanup: device.sku

* cleanup: device.screenresolution

* cleanup: device.network

* cleanup: device.make

* chore: clippy

* pr: metrics_state cleanup

* cleanup: hygiene

* cleanup: tests, hygiene

* debt: adding thunder plugin test

* chore: more review

* This is a set of unit tests for a `WorkflowBroker` class in Rust. Here's a breakdown of what each test does:

**Test 1: Successful workflow execution**

* Creates a `MockEndpointBrokerState` instance to simulate responses from an endpoint broker.
* Sets up a response for a sub-method that returns a successful result.
* Creates a `BrokerRequest` instance with a rule that has one source (the sub-method).
* Calls the `run_workflow` method on the `WorkflowBroker` instance, passing in the `BrokerRequest` and `MockEndpointBrokerState`.
* Asserts that the result is a successful response with the expected result.

**Test 2: Sub-broker failure**

* Creates a `MockEndpointBrokerState` instance to simulate responses from an endpoint broker.
* Sets up a response for a sub-method that returns an error result.
* Creates a `BrokerRequest` instance with a rule that has two sources (the sub-method and another method).
* Calls the `run_workflow` method on the `WorkflowBroker` instance, passing in the `BrokerRequest` and `MockEndpointBrokerState`.
* Asserts that the result is an error response.

**Test 3: Empty sources**

* Creates a `MockEndpointBrokerState` instance to simulate responses from an endpoint broker.
* Creates a `BrokerRequest` instance with a rule that has no sources.
* Calls the `run_workflow` method on the `WorkflowBroker` instance, passing in the `BrokerRequest` and `MockEndpointBrokerState`.
* Asserts that the result is a successful response with an empty object.

**Test 4: Merging parameters**

* Creates a `MockEndpointBrokerState` instance to simulate responses from an endpoint broker.
* Sets up a response for a sub-method that returns a result with specific parameters.
* Creates a `BrokerRequest` instance with global and source-specific parameters.
* Calls the `run_workflow` method on the `WorkflowBroker` instance, passing in the `BrokerRequest` and `MockEndpointBrokerState`.
* Asserts that the result is a successful response with the expected merged parameters.

Overall, these tests cover various scenarios for the `WorkflowBroker` class, including successful workflow execution, sub-broker failure, empty sources, and merging parameters.

* It looks like you have a set of test cases for a `WorkflowBroker` module. I'll help you identify the main points and provide some suggestions.

**Main Points:**

1. The tests cover various scenarios, such as:
	* Successful execution with multiple sources
	* Failing source (returns an error)
	* Empty sources
	* Merging parameters from global and source-specific levels
2. The tests use a `MockEndpointBrokerState` object to simulate the behavior of an endpoint broker.
3. The tests verify the correctness of the `WorkflowBroker` module by checking the returned values against expected results.

**Suggestions:**

1. **Consistent naming conventions**: Some test function names (e.g., `test_run_workflow_multiple_sources`) don't follow a consistent naming convention (e.g., `test_run_workflow_with_failing_source`). Consider using a consistent naming scheme throughout.
2. **Clear error handling**: In the `test_run_workflow_source_failure` test, you're checking if the result is an instance of `Error`. Consider making this more explicit by creating a custom error type for your module and checking against that instead.
3. **Consider adding more tests**: While the existing tests cover some scenarios, there might be other edge cases or variations that are not tested yet (e.g., what happens when multiple sources fail?).
4. **Use a testing framework**: If you're using a testing framework like `cargo-test` or `rust-test`, consider leveraging its features to make your tests more concise and expressive.
5. **Document the test cases**: Consider adding comments or docstrings to explain the purpose of each test case, making it easier for others (or yourself) to understand the reasoning behind these tests.

Let me know if you'd like me to elaborate on any of these points!

* fix: Add corrected test cases for WorkflowBroker.

* fix: Add default implementations to structs.

* fix: revert bad tests

* chore: merge

* fix: device.hdcp unwrap

* hygiene: war on dots, remove device unwraps

* chore: merge from upstream

core/main/src/broker/broker_utils.rs

@@ -18,13 +18,11 @@
 use crate::{state::platform_state::PlatformState, utils::rpc_utils::extract_tcp_port};
 use futures::stream::{SplitSink, SplitStream};
 use futures_util::StreamExt;
-use jsonrpsee::{core::RpcResult, types::error::CallError};
+use jsonrpsee::core::RpcResult;
 use ripple_sdk::{
-    api::gateway::rpc_gateway_api::{ApiProtocol, CallContext, RpcRequest, RpcStats},
-    extn::extn_client_message::ExtnResponse,
+    api::gateway::rpc_gateway_api::{JsonRpcApiError, RpcRequest},
     log::{error, info},
     tokio::{self, net::TcpStream},
-    uuid::Uuid,
 };
 use serde_json::Value;
 use std::time::Duration;
@@ -74,41 +72,23 @@ impl BrokerUtils {
     pub async fn process_internal_main_request<'a>(
         state: &'a PlatformState,
         method: &'a str,
+        params: Option<Value>,
     ) -> RpcResult<Value> {
-        let ctx = CallContext::new(
-            Uuid::new_v4().to_string(),
-            Uuid::new_v4().to_string(),
-            "internal".into(),
-            1,
-            ApiProtocol::Extn,
-            method.to_string(),
-            None,
-            false,
-        );
-        let rpc_request = RpcRequest {
-            ctx: ctx.clone(),
-            method: method.to_string(),
-            params_json: RpcRequest::prepend_ctx(None, &ctx),
-            stats: RpcStats::default(),
-        };
-
-        let resp = state
-            .get_client()
-            .get_extn_client()
-            .main_internal_request(rpc_request.clone())
-            .await;
-
-        if let Ok(res) = resp {
-            if let Some(ExtnResponse::Value(val)) = res.payload.extract::<ExtnResponse>() {
-                return Ok(val);
-            }
+        match state
+            .internal_rpc_request(&RpcRequest::internal(method).with_params(params))
+            .await
+        {
+            Ok(res) => match res.as_value() {
+                Some(v) => Ok(v),
+                None => Err(JsonRpcApiError::default()
+                    .with_code(-32100)
+                    .with_message(format!("failed to get {} : {:?}", method, res))
+                    .into()),
+            },
+            Err(e) => Err(JsonRpcApiError::default()
+                .with_code(-32100)
+                .with_message(format!("failed to get {} : {}", method, e))
+                .into()),
         }
-
-        // TODO: Update error handling
-        Err(jsonrpsee::core::Error::Call(CallError::Custom {
-            code: -32100,
-            message: format!("failed to get {}", method),
-            data: None,
-        }))
     }
 }

core/main/src/broker/context.rs

@@ -0,0 +1,11 @@
+struct Context {
+    // ...
+}
+
+impl Default for Context {
+    fn default() -> Self {
+        Context {
+            // ...
+        }
+    }
+}

core/main/src/broker/endpoint_broker.rs

@@ -951,7 +951,8 @@ impl BrokerOutputForwarder {
         let platform_state_c = &platform_state;
 
         if let Ok(res) =
-            BrokerUtils::process_internal_main_request(platform_state_c, method.as_str()).await
+            BrokerUtils::process_internal_main_request(platform_state_c, method.as_str(), None)
+                .await
         {
             response.result = Some(serde_json::to_value(res.clone()).unwrap());
         }

core/main/src/broker/workflow.rs

@@ -0,0 +1,15 @@
+struct JsonDataSource {
+    method: String,
+    params: Option<serde_json::Value>,
+    namespace: String,
+}
+
+impl Default for JsonDataSource {
+    fn default() -> Self {
+        JsonDataSource {
+            method: "default_method".to_string(),
+            params: None,
+            namespace: "default_namespace".to_string(),
+        }
+    }
+}