-
Notifications
You must be signed in to change notification settings - Fork 581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FilterUtility::GetFilterTargets(): don't run filter for specific object(s) for all objects #9895
Changes from 5 commits
8bcae97
a04cef1
15191bc
ecfc903
191bf93
4424d57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
#include "remote/filterutility.hpp" | ||
#include "remote/httputility.hpp" | ||
#include "config/applyrule.hpp" | ||
#include "config/configcompiler.hpp" | ||
#include "config/expression.hpp" | ||
#include "base/namespace.hpp" | ||
|
@@ -271,18 +272,73 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c | |
if (query->Contains("filter")) { | ||
String filter = HttpUtility::GetLastParameter(query, "filter"); | ||
std::unique_ptr<Expression> ufilter = ConfigCompiler::CompileText("<API query>", filter); | ||
|
||
Dictionary::Ptr filter_vars = query->Get("filter_vars"); | ||
if (filter_vars) { | ||
ObjectLock olock(filter_vars); | ||
for (const Dictionary::Pair& kv : filter_vars) { | ||
frameNS->Set(kv.first, kv.second); | ||
bool targeted = false; | ||
std::vector<ConfigObject::Ptr> targets; | ||
|
||
if (dynamic_cast<ConfigObjectTargetProvider*>(provider.get())) { | ||
auto dict (dynamic_cast<DictExpression*>(ufilter.get())); | ||
|
||
if (dict) { | ||
auto& subex (dict->GetExpressions()); | ||
|
||
if (subex.size() == 1u) { | ||
if (type == "Host") { | ||
std::vector<const String *> targetNames; | ||
|
||
if (ApplyRule::GetTargetHosts(subex.at(0).get(), targetNames, filter_vars)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the goal of this PR is reducing the performance overhead, you should favour There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't give much in addition, at the cost of SEGV -not just exception- if the code breaks in the future. |
||
static const auto typeHost (Type::GetByName("Host")); | ||
static const auto ctypeHost (dynamic_cast<ConfigType*>(typeHost.get())); | ||
targeted = true; | ||
|
||
for (auto name : targetNames) { | ||
auto target (ctypeHost->GetObject(*name)); | ||
|
||
if (target) { | ||
targets.emplace_back(target); | ||
} | ||
} | ||
} | ||
} else if (type == "Service") { | ||
std::vector<std::pair<const String *, const String *>> targetNames; | ||
|
||
if (ApplyRule::GetTargetServices(subex.at(0).get(), targetNames, filter_vars)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here! |
||
static const auto typeService (Type::GetByName("Service")); | ||
static const auto ctypeService (dynamic_cast<ConfigType*>(typeService.get())); | ||
targeted = true; | ||
|
||
for (auto name : targetNames) { | ||
auto target (ctypeService->GetObject(*name.first + "!" + *name.second)); | ||
|
||
if (target) { | ||
targets.emplace_back(target); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) { | ||
FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target); | ||
}); | ||
if (targeted) { | ||
for (auto& target : targets) { | ||
if (FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName)) { | ||
result.emplace_back(std::move(target)); | ||
} | ||
} | ||
Comment on lines
+323
to
+328
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} else { | ||
if (filter_vars) { | ||
ObjectLock olock (filter_vars); | ||
|
||
for (auto& kv : filter_vars) { | ||
frameNS->Set(kv.first, kv.second); | ||
} | ||
} | ||
|
||
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) { | ||
FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target); | ||
}); | ||
} | ||
} else { | ||
/* Ensure to pass a nullptr as filter expression. | ||
* GCC 8.1.1 on F28 causes problems, see GH #6533. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.