fix resolution issue with duplicate callouts
If two resolutions end up calling out the same part or procedure, only
the callout with the high priority should be added to the callout list.
This worked for the most. However, it did not take into account that the
guard action may have changed between callouts.
Change-Id: I1d42cdfcbac58086cdb54850f5b16aa686a3eb38
Signed-off-by: Zane Shelley <zshelle@us.ibm.com>
diff --git a/analyzer/service_data.cpp b/analyzer/service_data.cpp
index 62cd474..941c1ee 100644
--- a/analyzer/service_data.cpp
+++ b/analyzer/service_data.cpp
@@ -181,25 +181,27 @@
assert(i_callout.contains("Priority") &&
m.contains(i_callout.at("Priority")));
- bool addCallout = true;
+ // Look if this callout already exists in the list.
+ auto itr = std::find_if(
+ iv_calloutList.begin(), iv_calloutList.end(), [&](const auto& c) {
+ return c.contains(type) && c.at(type) == i_callout.at(type);
+ });
- for (auto& c : iv_calloutList)
+ if (iv_calloutList.end() != itr)
{
- if (c.contains(type) && (c.at(type) == i_callout.at(type)))
+ // The callout already exists in the list. If the priority of the
+ // callout in the list is lower than the new callout, replace it with
+ // the new callout. Otherwise, use the current callout in the list and
+ // ignore the new callout. This is done to maintain any guard
+ // information that may be associated with the highest priority callout.
+ if (m.at(itr->at("Priority")) < m.at(i_callout.at("Priority")))
{
- // The new callout already exists. Don't add a new callout.
- addCallout = false;
-
- if (m.at(c.at("Priority")) < m.at(i_callout.at("Priority")))
- {
- // The new callout has a higher priority, update it.
- c["Priority"] = i_callout.at("Priority");
- }
+ *itr = i_callout;
}
}
-
- if (addCallout)
+ else
{
+ // New callout. So push it to the list.
iv_calloutList.push_back(i_callout);
}
}
diff --git a/test/test-chnl-timeout.cpp b/test/test-chnl-timeout.cpp
index 711d193..b8cf8fd 100644
--- a/test/test-chnl-timeout.cpp
+++ b/test/test-chnl-timeout.cpp
@@ -39,7 +39,9 @@
s = R"([
{
"Deconfigured": false,
- "Guarded": false,
+ "EntityPath": [],
+ "GuardType": "GARD_Unrecoverable",
+ "Guarded": true,
"LocationCode": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0",
"Priority": "H"
},
@@ -105,7 +107,9 @@
s = R"([
{
"Deconfigured": false,
- "Guarded": false,
+ "EntityPath": [],
+ "GuardType": "GARD_Unrecoverable",
+ "Guarded": true,
"LocationCode": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0",
"Priority": "H"
},
@@ -178,7 +182,9 @@
},
{
"Deconfigured": false,
- "Guarded": false,
+ "EntityPath": [],
+ "GuardType": "GARD_Unrecoverable",
+ "Guarded": true,
"LocationCode": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0/ocmb0",
"Priority": "H"
},
@@ -244,7 +250,9 @@
},
{
"Deconfigured": false,
- "Guarded": false,
+ "EntityPath": [],
+ "GuardType": "GARD_Unrecoverable",
+ "Guarded": true,
"LocationCode": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0/ocmb0",
"Priority": "H"
},
diff --git a/test/test-resolution.cpp b/test/test-resolution.cpp
index 6898bc4..3c2c3f7 100644
--- a/test/test-resolution.cpp
+++ b/test/test-resolution.cpp
@@ -301,9 +301,23 @@
nlohmann::json j{};
std::string s{};
+ // Special case: The bus callout will add both ends of the bus to the
+ // callout list at LOW priority with no guarding. Generally, this would be
+ // the last thing to be added to the callout list because of the priority.
+ // However, there was a field defect where a higher priority callout of one
+ // of the endpoint was added to the list (with guard action) after the bus
+ // callout. The priority of the callout was updated to match the higher
+ // priority, but the guard action was not updated. So the following are
+ // added to the callout list in a specific order:
+ // - First, one of the endpoints with MED_A and guard.
+ // - Then, the bus callout with LOW and no guard. This should result in
+ // both endpoints in the list: one at MED_A (guard), the other at LOW (no
+ // guard).
+ // - Finally, the other endpoint with MED_A and guard. This should result
+ // in both endpoints in the list with MED_A priority and guard actions.
c1->resolve(sd);
- c2->resolve(sd);
c3->resolve(sd);
+ c2->resolve(sd);
// Verify the subsystem
std::pair<callout::SrcSubsystem, callout::Priority> subsys = {
@@ -349,17 +363,17 @@
},
{
"Bus Type": "OMI_BUS",
- "Callout Type": "Connected Callout",
- "Guard": true,
- "Priority": "medium_group_A",
+ "Callout Type": "Bus Callout",
+ "Guard": false,
+ "Priority": "low",
"RX Target": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0",
"TX Target": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0/ocmb0"
},
{
"Bus Type": "OMI_BUS",
- "Callout Type": "Bus Callout",
- "Guard": false,
- "Priority": "low",
+ "Callout Type": "Connected Callout",
+ "Guard": true,
+ "Priority": "medium_group_A",
"RX Target": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0",
"TX Target": "/proc0/pib/perv12/mc0/mi0/mcc0/omi0/ocmb0"
}