query: implement $select for objects array

This commit fixes one of the TODOs: make $select work on object arrays.
This is according to https://github.com/DMTF/Redfish/issues/5188, where
array index doesn't count as the properity prefix.

To make sure reserved properties are selected on every node, this commit
also refactors some of the logics when inserting new properties.

Tested:
1. unit test
2. tested on hardware,
URL: /redfish/v1/Chassis/chassis/Thermal?$select=Temperatures/Name
{
    "@odata.id": "/redfish/v1/Chassis/chassis/Thermal",
    "@odata.type": "#Thermal.v1_4_0.Thermal",
    "Temperatures": [
        {
            "@odata.id": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/0",
            "@odata.type": "#Thermal.v1_3_0.Temperature",
            "Name": "Abc"
        },
        {
            "@odata.id": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/1",
            "@odata.type": "#Thermal.v1_3_0.Temperature",
            "Name": "Xyz"
        }
    ]
}
3. no new service validator failures

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ibfa22c0f42018cd0d482b4a19fff6dcd0cd346d1

2 files changed, 58 insertions, 39 deletions

diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 184043bffa..5e9ef13e4e 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -158,7 +158,6 @@ struct SelectTrie
             nestedProperty.remove_prefix(index + 1);
             index = nestedProperty.find_first_of('/');
         }
-
         currNode->setToSelected();
         return true;
     }
@@ -372,14 +371,6 @@ inline bool getSelectParam(std::string_view value, Query& query)
             return false;
         }
     }
-    // Per the Redfish spec section 7.3.3, the service shall select certain
-    // properties as if $select was omitted.
-    constexpr std::array<std::string_view, 5> reservedProperties = {
-        "@odata.id", "@odata.type", "@odata.context", "@odata.etag", "error"};
-    for (auto const& str : reservedProperties)
-    {
-        query.selectTrie.insertNode(str.data());
-    }
     return true;
 }
 
@@ -812,7 +803,16 @@ inline void recursiveSelect(nlohmann::json& currRoot,
             auto nextIt = std::next(it);
             BMCWEB_LOG_DEBUG << "key=" << it.key();
             const SelectTrieNode* nextNode = currNode.find(it.key());
-            if (nextNode != nullptr && nextNode->isSelected())
+            // Per the Redfish spec section 7.3.3, the service shall select
+            // certain properties as if $select was omitted. This applies to
+            // every TrieNode that contains leaves and the root.
+            constexpr std::array<std::string_view, 5> reservedProperties = {
+                "@odata.id", "@odata.type", "@odata.context", "@odata.etag",
+                "error"};
+            bool reserved =
+                std::find(reservedProperties.begin(), reservedProperties.end(),
+                          it.key()) != reservedProperties.end();
+            if (reserved || (nextNode != nullptr && nextNode->isSelected()))
             {
                 it = nextIt;
                 continue;
@@ -828,15 +828,24 @@ inline void recursiveSelect(nlohmann::json& currRoot,
             it = currRoot.erase(it);
         }
     }
+    nlohmann::json::array_t* array =
+        currRoot.get_ptr<nlohmann::json::array_t*>();
+    if (array != nullptr)
+    {
+        BMCWEB_LOG_DEBUG << "Current JSON is an array";
+        // Array index is omitted, so reuse the same Trie node
+        for (nlohmann::json& nextRoot : *array)
+        {
+            recursiveSelect(nextRoot, currNode);
+        }
+    }
 }
 
 // The current implementation of $select still has the following TODOs due to
 //  ambiguity and/or complexity.
-// 1. select properties in array of objects;
-// https://github.com/DMTF/Redfish/issues/5188 was created for clarification.
-// 2. combined with $expand; https://github.com/DMTF/Redfish/issues/5058 was
+// 1. combined with $expand; https://github.com/DMTF/Redfish/issues/5058 was
 // created for clarification.
-// 3. respect the full odata spec; e.g., deduplication, namespace, star (*),
+// 2. respect the full odata spec; e.g., deduplication, namespace, star (*),
 // etc.
 inline void processSelect(crow::Response& intermediateResponse,
                           const SelectTrieNode& trieRoot)
diff --git a/redfish-core/include/utils/query_param_test.cpp b/redfish-core/include/utils/query_param_test.cpp
index ddc7e33736..7358821ae2 100644
--- a/redfish-core/include/utils/query_param_test.cpp
+++ b/redfish-core/include/utils/query_param_test.cpp
@@ -219,29 +219,14 @@ TEST(GetSelectParam, TrieNodesRespectAllProperties)
     ASSERT_TRUE(getSelectParam("foo/bar,bar", query));
     ASSERT_FALSE(query.selectTrie.root.empty());
 
-    ASSERT_NE(query.selectTrie.root.find("bar"), nullptr);
-    EXPECT_TRUE(query.selectTrie.root.find("bar")->isSelected());
-
-    ASSERT_NE(query.selectTrie.root.find("@odata.id"), nullptr);
-    EXPECT_TRUE(query.selectTrie.root.find("@odata.id")->isSelected());
-
-    ASSERT_NE(query.selectTrie.root.find("@odata.type"), nullptr);
-    EXPECT_TRUE(query.selectTrie.root.find("@odata.type")->isSelected());
-
-    ASSERT_NE(query.selectTrie.root.find("@odata.context"), nullptr);
-    EXPECT_TRUE(query.selectTrie.root.find("@odata.context")->isSelected());
-
-    ASSERT_NE(query.selectTrie.root.find("@odata.etag"), nullptr);
-    EXPECT_TRUE(query.selectTrie.root.find("@odata.etag")->isSelected());
-
-    ASSERT_NE(query.selectTrie.root.find("error"), nullptr);
-    EXPECT_TRUE(query.selectTrie.root.find("error")->isSelected());
-
     const SelectTrieNode* child = query.selectTrie.root.find("foo");
     ASSERT_NE(child, nullptr);
     EXPECT_FALSE(child->isSelected());
     ASSERT_NE(child->find("bar"), nullptr);
     EXPECT_TRUE(child->find("bar")->isSelected());
+
+    ASSERT_NE(query.selectTrie.root.find("bar"), nullptr);
+    EXPECT_TRUE(query.selectTrie.root.find("bar")->isSelected());
 }
 
 SelectTrie getTrie(std::span<std::string_view> properties)
@@ -267,7 +252,8 @@ TEST(RecursiveSelect, ExpectedKeysAreSelectInSimpleObject)
 TEST(RecursiveSelect, ExpectedKeysAreSelectInNestedObject)
 {
     std::vector<std::string_view> properties = {
-        "SelectMe", "Prefix0/ExplicitSelectMe", "Prefix1", "Prefix2"};
+        "SelectMe", "Prefix0/ExplicitSelectMe", "Prefix1", "Prefix2",
+        "Prefix4/ExplicitSelectMe"};
     SelectTrie trie = getTrie(properties);
     nlohmann::json root = R"(
 {
@@ -289,6 +275,12 @@ TEST(RecursiveSelect, ExpectedKeysAreSelectInNestedObject)
   ],
   "Prefix3":[
     "OmitMe"
+  ],
+  "Prefix4":[
+    {
+      "ExplicitSelectMe":"123",
+      "OmitMe": "456"
+    }
   ]
 }
 )"_json;
@@ -307,6 +299,11 @@ TEST(RecursiveSelect, ExpectedKeysAreSelectInNestedObject)
     {
       "ImplicitSelectMe":"123"
     }
+  ],
+  "Prefix4":[
+    {
+      "ExplicitSelectMe":"123"
+    }
   ]
 }
 )"_json;
@@ -314,7 +311,7 @@ TEST(RecursiveSelect, ExpectedKeysAreSelectInNestedObject)
     EXPECT_EQ(root, expected);
 }
 
-TEST(RecursiveSelect, OdataPropertiesAreSelected)
+TEST(RecursiveSelect, ReservedPropertiesAreSelected)
 {
     nlohmann::json root = R"(
 {
@@ -323,15 +320,17 @@ TEST(RecursiveSelect, OdataPropertiesAreSelected)
   "@odata.type":2,
   "@odata.context":3,
   "@odata.etag":4,
-  "prefix1":{
+  "Prefix1":{
     "OmitMe":"bar",
-    "@odata.id":1
+    "@odata.id":1,
+    "ExplicitSelectMe": 1
   },
   "Prefix2":[1, 2, 3],
   "Prefix3":[
     {
       "OmitMe":"bar",
-      "@odata.id":1
+      "@odata.id":1,
+      "ExplicitSelectMe": 1
     }
   ]
 }
@@ -341,10 +340,21 @@ TEST(RecursiveSelect, OdataPropertiesAreSelected)
   "@odata.id":1,
   "@odata.type":2,
   "@odata.context":3,
-  "@odata.etag":4
+  "@odata.etag":4,
+  "Prefix1":{
+    "@odata.id":1,
+    "ExplicitSelectMe": 1
+  },
+  "Prefix3":[
+    {
+      "@odata.id":1,
+      "ExplicitSelectMe": 1
+    }
+  ]
 }
 )"_json;
-    auto ret = boost::urls::parse_relative_ref("/redfish/v1?$select=abc");
+    auto ret = boost::urls::parse_relative_ref(
+        "/redfish/v1?$select=Prefix1/ExplicitSelectMe,Prefix3/ExplicitSelectMe");
     ASSERT_TRUE(ret);
     crow::Response res;
     std::optional<Query> query = parseParameters(ret->params(), res);