query: Fix default expand level with delegated

With delegate expand, the default expand level is -=
`queryCapabilities.canDelegateExpandLevel`. This creates an overlap of
expand process between delegate expand vs. default expand.

With
query.expandLevel = 2 ->
  query.expandLevel = 1 and delegated.expandLevel = 1.

Both delegated and default expand will try to only expand of level one
instead of level 2 for the default.

The code in
https://github.com/openbmc/bmcweb/blob/479e899d5f57a67647f83b7f615d2c8565290bcf/redfish-core/include/utils/query_param.hpp#L583-L597
stated that the level with "@odata.id" + other property is treated as a
seperate level. So with `query.expandLevel = 1` it just loop through the
id that was already expanded and is noop.

Tested:

Before:
/redfish/v1/Chassis/BMC/Sensors?$expand=.($levels=2) returns
the same result as level=1. Needs level=3 to expand to the next level.

The RelatedItem in here doesn't get expanded with level=2.
```
 wget -qO- 'http://localhost:80/redfish/v1/Chassis/BMC/Sensors?$expand=.($levels=1)'
 ...
    {
      "@odata.id": "/redfish/v1/Chassis/BMC/Sensors/temperature_DIMMXX",
      "@odata.type": "#Sensor.v1_2_0.Sensor",
      "Id": "temperature_DIMMXX",
      "Name": "DIMMXX",
      "Reading": 30.0,
      "ReadingRangeMax": 127.0,
      "ReadingRangeMin": -128.0,
      "ReadingType": "Temperature",
      "ReadingUnits": "Cel",
      "RelatedItem": [
        {
          "@odata.id": "/redfish/v1/Systems/system/Memory/dimmXX"
        }
      ],
      "Status": {
        "Health": "OK",
        "State": "Enabled"
      },
      "Thresholds": {
        "LowerCaution": {
          "Reading": null
        },
        "LowerCritical": {
          "Reading": null
        },
        "UpperCaution": {
          "Reading": 93.0
        },
        "UpperCritical": {
          "Reading": 95.0
        }
      }
    }
  ],
  "Members@odata.count": 242,
  "Name": "Sensors"
}
```

After:
level=2 was able to expand to the next level.

Change-Id: I542177a94a33f8df7afbb68837f3a53b86140c86
Signed-off-by: Willy Tu <wltu@google.com>

3 files changed, 100 insertions, 47 deletions

diff --git a/redfish-core/include/query.hpp b/redfish-core/include/query.hpp
index 10a337b96b..c1ca38e957 100644
--- a/redfish-core/include/query.hpp
+++ b/redfish-core/include/query.hpp
@@ -166,9 +166,9 @@ inline bool handleIfMatch(crow::App& app, const crow::Request& req,
         asyncResp->res.releaseCompleteRequestHandler();
 
     asyncResp->res.setCompleteRequestHandler(
-        [&app, handler(std::move(handler)),
-         query{std::move(*queryOpt)}](crow::Response& resIn) mutable {
-        processAllParams(app, query, handler, resIn);
+        [&app, handler(std::move(handler)), query{std::move(*queryOpt)},
+         delegated{delegated}](crow::Response& resIn) mutable {
+        processAllParams(app, query, delegated, handler, resIn);
     });
 
     return needToCallHandlers;
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 71fcbd7e05..e5402d50e3 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -221,7 +221,6 @@ inline Query delegate(const QueryCapabilities& queryCapabilities, Query& query)
         }
         else
         {
-            query.expandLevel -= queryCapabilities.canDelegateExpandLevel;
             delegated.expandLevel = queryCapabilities.canDelegateExpandLevel;
         }
     }
@@ -534,8 +533,8 @@ struct ExpandNode
 // with the keys from the jsonResponse object
 inline void findNavigationReferencesRecursive(
     ExpandType eType, nlohmann::json& jsonResponse,
-    const nlohmann::json::json_pointer& p, int depth, bool inLinks,
-    std::vector<ExpandNode>& out)
+    const nlohmann::json::json_pointer& p, int depth, int skipDepth,
+    bool inLinks, std::vector<ExpandNode>& out)
 {
     // If no expand is needed, return early
     if (eType == ExpandType::None)
@@ -554,7 +553,7 @@ inline void findNavigationReferencesRecursive(
             nlohmann::json::json_pointer newPtr = p / index;
             BMCWEB_LOG_DEBUG << "Traversing response at " << newPtr.to_string();
             findNavigationReferencesRecursive(eType, element, newPtr, depth,
-                                              inLinks, out);
+                                              skipDepth, inLinks, out);
             index++;
         }
     }
@@ -574,7 +573,10 @@ inline void findNavigationReferencesRecursive(
             if (uri != nullptr)
             {
                 BMCWEB_LOG_DEBUG << "Found " << *uri << " at " << p.to_string();
-                out.push_back({p, *uri});
+                if (skipDepth == 0)
+                {
+                    out.push_back({p, *uri});
+                }
                 return;
             }
         }
@@ -589,11 +591,22 @@ inline void findNavigationReferencesRecursive(
         // "@odata.id" then that means we have entered a new level / expanded
         // resource.  We need to stop traversing if we're already at the desired
         // depth
-        if ((obj->size() > 1) && (depth == 0))
+        if (obj->size() > 1)
         {
-            return;
+            if (depth == 0)
+            {
+                return;
+            }
+            if (skipDepth > 0)
+            {
+                skipDepth--;
+            }
+        }
+
+        if (skipDepth == 0)
+        {
+            newDepth--;
         }
-        newDepth--;
     }
 
     // Loop the object and look for links
@@ -619,7 +632,8 @@ inline void findNavigationReferencesRecursive(
         BMCWEB_LOG_DEBUG << "Traversing response at " << newPtr;
 
         findNavigationReferencesRecursive(eType, element.second, newPtr,
-                                          newDepth, localInLinks, out);
+                                          newDepth, skipDepth, localInLinks,
+                                          out);
     }
 }
 
@@ -630,13 +644,14 @@ inline void findNavigationReferencesRecursive(
 // lands.  May want to avoid forwarding query params when request is uptree from
 // a top level collection.
 inline std::vector<ExpandNode>
-    findNavigationReferences(ExpandType eType, int depth,
+    findNavigationReferences(ExpandType eType, int depth, int skipDepth,
                              nlohmann::json& jsonResponse)
 {
     std::vector<ExpandNode> ret;
     const nlohmann::json::json_pointer root = nlohmann::json::json_pointer("");
-    findNavigationReferencesRecursive(eType, jsonResponse, root, depth, false,
-                                      ret);
+    // SkipDepth +1 since we are skipping the root by default.
+    findNavigationReferencesRecursive(eType, jsonResponse, root, depth,
+                                      skipDepth + 1, false, ret);
     return ret;
 }
 
@@ -795,10 +810,11 @@ class MultiAsyncResp : public std::enable_shared_from_this<MultiAsyncResp>
 
     // Handles the very first level of Expand, and starts a chain of sub-queries
     // for deeper levels.
-    void startQuery(const Query& query)
+    void startQuery(const Query& query, const Query& delegated)
     {
         std::vector<ExpandNode> nodes = findNavigationReferences(
-            query.expandType, query.expandLevel, finalRes->res.jsonValue);
+            query.expandType, query.expandLevel, delegated.expandLevel,
+            finalRes->res.jsonValue);
         BMCWEB_LOG_DEBUG << nodes.size() << " nodes to traverse";
         const std::optional<std::string> queryStr = formatQueryForExpand(query);
         if (!queryStr)
@@ -960,7 +976,7 @@ inline void processSelect(crow::Response& intermediateResponse,
 }
 
 inline void
-    processAllParams(crow::App& app, const Query& query,
+    processAllParams(crow::App& app, const Query& query, const Query& delegated,
                      std::function<void(crow::Response&)>& completionHandler,
                      crow::Response& intermediateResponse)
 {
@@ -998,7 +1014,7 @@ inline void
 
         asyncResp->res.setCompleteRequestHandler(std::move(completionHandler));
         auto multi = std::make_shared<MultiAsyncResp>(app, asyncResp);
-        multi->startQuery(query);
+        multi->startQuery(query, delegated);
         return;
     }
 
diff --git a/test/redfish-core/include/utils/query_param_test.cpp b/test/redfish-core/include/utils/query_param_test.cpp
index 8c75241d77..7aa4ad519b 100644
--- a/test/redfish-core/include/utils/query_param_test.cpp
+++ b/test/redfish-core/include/utils/query_param_test.cpp
@@ -53,7 +53,7 @@ TEST(Delegate, ExpandPositive)
     EXPECT_FALSE(delegated.isOnly);
     EXPECT_EQ(delegated.expandLevel, capabilities.canDelegateExpandLevel);
     EXPECT_EQ(delegated.expandType, ExpandType::Both);
-    EXPECT_EQ(query.expandLevel, 2);
+    EXPECT_EQ(query.expandLevel, 5);
 }
 
 TEST(Delegate, OnlyNegative)
@@ -659,23 +659,25 @@ TEST(QueryParams, FindNavigationReferencesNonLink)
         "Foo" : {"@odata.id": "/foobar"}})"_json;
 
     // Parsing as the root should net one entry
-    EXPECT_THAT(findNavigationReferences(ExpandType::Both, 1, singleTreeNode),
-                UnorderedElementsAre(
-                    ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
+    EXPECT_THAT(
+        findNavigationReferences(ExpandType::Both, 1, 0, singleTreeNode),
+        UnorderedElementsAre(
+            ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
 
     // Parsing in Non-hyperlinks mode should net one entry
     EXPECT_THAT(
-        findNavigationReferences(ExpandType::NotLinks, 1, singleTreeNode),
+        findNavigationReferences(ExpandType::NotLinks, 1, 0, singleTreeNode),
         UnorderedElementsAre(
             ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
 
     // Searching for not types should return empty set
-    EXPECT_TRUE(
-        findNavigationReferences(ExpandType::None, 1, singleTreeNode).empty());
+    EXPECT_TRUE(findNavigationReferences(ExpandType::None, 1, 0, singleTreeNode)
+                    .empty());
 
     // Searching for hyperlinks only should return empty set
     EXPECT_TRUE(
-        findNavigationReferences(ExpandType::Links, 1, singleTreeNode).empty());
+        findNavigationReferences(ExpandType::Links, 1, 0, singleTreeNode)
+            .empty());
 
     // Responses must include their "@odata.id" property for $expand to work
     // correctly
@@ -686,7 +688,7 @@ TEST(QueryParams, FindNavigationReferencesNonLink)
 
     // Should still find Foo
     EXPECT_THAT(
-        findNavigationReferences(ExpandType::NotLinks, 1, multiTreeNodes),
+        findNavigationReferences(ExpandType::NotLinks, 1, 0, multiTreeNodes),
         UnorderedElementsAre(
             ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
 }
@@ -702,21 +704,23 @@ TEST(QueryParams, FindNavigationReferencesLink)
         "Links" : {"Sessions": {"@odata.id": "/foobar"}}})"_json;
 
     // Parsing as the root should net one entry
-    EXPECT_THAT(findNavigationReferences(ExpandType::Both, 1, singleLinkNode),
-                UnorderedElementsAre(ExpandNode{
-                    json::json_pointer("/Links/Sessions"), "/foobar"}));
+    EXPECT_THAT(
+        findNavigationReferences(ExpandType::Both, 1, 0, singleLinkNode),
+        UnorderedElementsAre(
+            ExpandNode{json::json_pointer("/Links/Sessions"), "/foobar"}));
     // Parsing in hyperlinks mode should net one entry
-    EXPECT_THAT(findNavigationReferences(ExpandType::Links, 1, singleLinkNode),
-                UnorderedElementsAre(ExpandNode{
-                    json::json_pointer("/Links/Sessions"), "/foobar"}));
+    EXPECT_THAT(
+        findNavigationReferences(ExpandType::Links, 1, 0, singleLinkNode),
+        UnorderedElementsAre(
+            ExpandNode{json::json_pointer("/Links/Sessions"), "/foobar"}));
 
     // Searching for not types should return empty set
-    EXPECT_TRUE(
-        findNavigationReferences(ExpandType::None, 1, singleLinkNode).empty());
+    EXPECT_TRUE(findNavigationReferences(ExpandType::None, 1, 0, singleLinkNode)
+                    .empty());
 
     // Searching for non-hyperlinks only should return empty set
     EXPECT_TRUE(
-        findNavigationReferences(ExpandType::NotLinks, 1, singleLinkNode)
+        findNavigationReferences(ExpandType::NotLinks, 1, 0, singleLinkNode)
             .empty());
 }
 
@@ -754,10 +758,10 @@ TEST(QueryParams, PreviouslyExpanded)
 
     // Expand has already occurred so we should not do anything
     EXPECT_TRUE(
-        findNavigationReferences(ExpandType::NotLinks, 1, expNode).empty());
+        findNavigationReferences(ExpandType::NotLinks, 1, 0, expNode).empty());
 
     // Previous expand was only a single level so we should further expand
-    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, expNode),
+    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, 0, expNode),
                 UnorderedElementsAre(
                     ExpandNode{json::json_pointer("/Members/0/Sensors"),
                                "/redfish/v1/Chassis/5B247A_Sat1/Sensors"},
@@ -768,12 +772,12 @@ TEST(QueryParams, PreviouslyExpanded)
     json expNode2 = R"({"@odata.id" : "/redfish/v1"})"_json;
     expNode2["Chassis"] = std::move(expNode);
     EXPECT_TRUE(
-        findNavigationReferences(ExpandType::NotLinks, 1, expNode2).empty());
+        findNavigationReferences(ExpandType::NotLinks, 1, 0, expNode2).empty());
     EXPECT_TRUE(
-        findNavigationReferences(ExpandType::NotLinks, 2, expNode2).empty());
+        findNavigationReferences(ExpandType::NotLinks, 2, 0, expNode2).empty());
 
     // Previous expand was two levels so we should further expand
-    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 3, expNode2),
+    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 3, 0, expNode2),
                 UnorderedElementsAre(
                     ExpandNode{json::json_pointer("/Chassis/Members/0/Sensors"),
                                "/redfish/v1/Chassis/5B247A_Sat1/Sensors"},
@@ -781,6 +785,39 @@ TEST(QueryParams, PreviouslyExpanded)
                                "/redfish/v1/Chassis/5B247A_Sat2/Sensors"}));
 }
 
+TEST(QueryParams, DelegatedSkipExpanded)
+{
+    using nlohmann::json;
+
+    // Responses must include their "@odata.id" property for $expand to work
+    // correctly
+    json expNode = json::parse(R"(
+{
+  "@odata.id": "/redfish/v1",
+  "Foo": {
+    "@odata.id": "/foo"
+  },
+  "Bar": {
+    "@odata.id": "/bar",
+    "Foo": {
+      "@odata.id": "/barfoo"
+    }
+  }
+}
+)",
+                               nullptr, false);
+
+    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, 0, expNode),
+                UnorderedElementsAre(
+                    ExpandNode{json::json_pointer("/Foo"), "/foo"},
+                    ExpandNode{json::json_pointer("/Bar/Foo"), "/barfoo"}));
+
+    // Skip the first expand level
+    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 1, 1, expNode),
+                UnorderedElementsAre(
+                    ExpandNode{json::json_pointer("/Bar/Foo"), "/barfoo"}));
+}
+
 TEST(QueryParams, PartiallyPreviouslyExpanded)
 {
     using nlohmann::json;
@@ -812,13 +849,13 @@ TEST(QueryParams, PartiallyPreviouslyExpanded)
     // The 5B247A_Sat1 Chassis was already expanded a single level so we should
     // only want to expand the Local Chassis
     EXPECT_THAT(
-        findNavigationReferences(ExpandType::NotLinks, 1, expNode),
+        findNavigationReferences(ExpandType::NotLinks, 1, 0, expNode),
         UnorderedElementsAre(ExpandNode{json::json_pointer("/Members/0"),
                                         "/redfish/v1/Chassis/Local"}));
 
     // The 5B247A_Sat1 Chassis was already expanded a single level so we should
     // further expand it as well as the Local Chassis
-    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, expNode),
+    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, 0, expNode),
                 UnorderedElementsAre(
                     ExpandNode{json::json_pointer("/Members/0"),
                                "/redfish/v1/Chassis/Local"},
@@ -830,19 +867,19 @@ TEST(QueryParams, PartiallyPreviouslyExpanded)
                         "Systems": {"@odata.id": "/redfish/v1/Systems"}})"_json;
     expNode2["Chassis"] = std::move(expNode);
 
-    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 1, expNode2),
+    EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 1, 0, expNode2),
                 UnorderedElementsAre(ExpandNode{json::json_pointer("/Systems"),
                                                 "/redfish/v1/Systems"}));
 
     EXPECT_THAT(
-        findNavigationReferences(ExpandType::NotLinks, 2, expNode2),
+        findNavigationReferences(ExpandType::NotLinks, 2, 0, expNode2),
         UnorderedElementsAre(
             ExpandNode{json::json_pointer("/Systems"), "/redfish/v1/Systems"},
             ExpandNode{json::json_pointer("/Chassis/Members/0"),
                        "/redfish/v1/Chassis/Local"}));
 
     EXPECT_THAT(
-        findNavigationReferences(ExpandType::NotLinks, 3, expNode2),
+        findNavigationReferences(ExpandType::NotLinks, 3, 0, expNode2),
         UnorderedElementsAre(
             ExpandNode{json::json_pointer("/Systems"), "/redfish/v1/Systems"},
             ExpandNode{json::json_pointer("/Chassis/Members/0"),