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
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 184043b..5e9ef13 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -158,7 +158,6 @@
             nestedProperty.remove_prefix(index + 1);
             index = nestedProperty.find_first_of('/');
         }
-
         currNode->setToSelected();
         return true;
     }
@@ -372,14 +371,6 @@
             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 @@
             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 @@
             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 ddc7e33..7358821 100644
--- a/redfish-core/include/utils/query_param_test.cpp
+++ b/redfish-core/include/utils/query_param_test.cpp
@@ -219,29 +219,14 @@
     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, 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 @@
   ],
   "Prefix3":[
     "OmitMe"
+  ],
+  "Prefix4":[
+    {
+      "ExplicitSelectMe":"123",
+      "OmitMe": "456"
+    }
   ]
 }
 )"_json;
@@ -307,6 +299,11 @@
     {
       "ImplicitSelectMe":"123"
     }
+  ],
+  "Prefix4":[
+    {
+      "ExplicitSelectMe":"123"
+    }
   ]
 }
 )"_json;
@@ -314,7 +311,7 @@
     EXPECT_EQ(root, expected);
 }
 
-TEST(RecursiveSelect, OdataPropertiesAreSelected)
+TEST(RecursiveSelect, ReservedPropertiesAreSelected)
 {
     nlohmann::json root = R"(
 {
@@ -323,15 +320,17 @@
   "@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 @@
   "@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);