Rework addRule() and removeRule() in rule-apply-engine

- remove dirty code for rule verification in addRule()
- modify rule iteration code in removeRule()

Change-Id: I8fe18265190c067bd9929e2e0de9620e7b5cc5a4
Signed-off-by: seolheui, kim <s414.kim@samsung.com>

5 files changed, 25 insertions, 32 deletions

diff --git a/lib/audit-rule/rule.h b/lib/audit-rule/rule.h
index 9cb6858..6f24004 100644
--- a/lib/audit-rule/rule.h
+++ b/lib/audit-rule/rule.h
@@ -54,7 +54,6 @@ public:
 	Rule(Action action = Action::Always, Filter filter = Filter::Exit);
 	virtual ~Rule();
 
-	Rule(Rule &&) = delete;
 	Rule(const std::vector<char> &rule);
 	Rule(const Rule &rule);
 	Rule &operator = (const Rule &rule) = default;
@@ -71,7 +70,7 @@ public:
 		return *this;
 	}
 
-	bool operator == (const Rule &rule)
+	bool operator == (const Rule &rule) const
 	{
 		return (data() == rule.data());
 	}
diff --git a/server/rule-apply-engine.cpp b/server/rule-apply-engine.cpp
index 9c77967..0157ed5 100644
--- a/server/rule-apply-engine.cpp
+++ b/server/rule-apply-engine.cpp
@@ -13,6 +13,7 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License
  */
+#include <algorithm>
 #include "rule-apply-engine.h"
 
 namespace {
@@ -29,17 +30,14 @@ RuleApplyEngine::~RuleApplyEngine()
 {
 }
 
-void RuleApplyEngine::addRule(Audit &audit, const std::vector<char> &data)
+void RuleApplyEngine::addRule(Audit &audit, const Rule &rule)
 {
-	Rule rule(data);
+	//TODO: add verifier of rule
 	for (auto &r : adminList) {
 		if (r == rule)
 			return;
 	}
 
-	audit.addRule(data);
-	audit.removeRule(data);
-
 	adminList.emplace_back(rule);
 
 	removeAll(audit);
@@ -50,26 +48,21 @@ void RuleApplyEngine::addRule(Audit &audit, const std::vector<char> &data)
 	addAll(audit);
 }
 
-void RuleApplyEngine::removeRule(Audit &audit, const std::vector<char> &data)
+void RuleApplyEngine::removeRule(Audit &audit, const Rule &rule)
 {
-	Rule rule(data);
-	bool removed = false;
-	for (auto r = adminList.begin(); r != adminList.end(); r++) {
-		if (*r == rule) {
-			adminList.erase(r);
-			removed = true;
-			break;
-		}
-	}
+	auto it = std::find_if(adminList.begin(), adminList.end(), [&](const Rule &r) {
+			return rule == r;});
 
-	if (!removed)
-		throw runtime::Exception("Failed to remove rule");
+	if (it != adminList.end())
+		adminList.erase(it);
+	else
+		throw runtime::Exception("The rule does not exist");
 
 	removeAll(audit);
 	optimizedList.clear();
 	addNeverRules();
 
-	for (auto r : adminList) {
+	for (const auto &r : adminList) {
 		optimize(r);
 	}
 
@@ -101,21 +94,22 @@ void RuleApplyEngine::addAll(Audit &audit)
 	}
 }
 
-void RuleApplyEngine::optimize(Rule &rule)
+void RuleApplyEngine::optimize(const Rule &rule)
 {
-	rule.unsetCondition(FieldType::Tag);
+	Rule result(rule);
+	result.unsetCondition(FieldType::Tag);
 
 	for (auto r = optimizedList.begin()+1; r != optimizedList.end(); r++) {
-		if (*r == rule)
+		if (*r == result)
 			return;
 
-		bool ret = Rule::combine(*r, rule);
+		bool ret = Rule::combine(*r, result);
 		if (ret && r->getMask().empty())
 			optimizedList.erase(r);
 	}
 
-	if (!rule.getMask().empty())
-		optimizedList.emplace_back(rule);
+	if (!result.getMask().empty())
+		optimizedList.emplace_back(result);
 
 	//TODO: relocate optimizedList
 	for (auto r = optimizedList.begin()+1; r != optimizedList.end(); r++) {
diff --git a/server/rule-apply-engine.h b/server/rule-apply-engine.h
index 8aed28a..d4540b6 100644
--- a/server/rule-apply-engine.h
+++ b/server/rule-apply-engine.h
@@ -28,15 +28,15 @@ public:
 	RuleApplyEngine();
 	~RuleApplyEngine();
 
-	void addRule(Audit &audit, const std::vector<char> &data);
-	void removeRule(Audit &audit, const std::vector<char> &data);
+	void addRule(Audit &audit, const Rule &rule);
+	void removeRule(Audit &audit, const Rule &rule);
 	RuleList getRules() const;
 
 private:
 	void removeAll(Audit &audit);
 	void addAll(Audit &audit);
 
-	void optimize(Rule &r);
+	void optimize(const Rule &rule);
 	void addNeverRules();
 	void applyNeverRules();
 private:
diff --git a/server/rule-management.cpp b/server/rule-management.cpp
index 30a0be2..fd42d8b 100644
--- a/server/rule-management.cpp
+++ b/server/rule-management.cpp
@@ -38,7 +38,7 @@ RuleManagement::~RuleManagement()
 int RuleManagement::addRule(std::vector<char> data)
 {
 	try {
-		context.getRuleApplyEngine().addRule(context.getAudit(), data);
+		context.getRuleApplyEngine().addRule(context.getAudit(), Rule{data});
 	} catch (runtime::Exception &e) {
 		ERROR("Failed to add rule : " + std::string(e.what()));
 		return -1;
@@ -49,7 +49,7 @@ int RuleManagement::addRule(std::vector<char> data)
 int RuleManagement::removeRule(std::vector<char> data)
 {
 	try {
-		context.getRuleApplyEngine().removeRule(context.getAudit(), data);
+		context.getRuleApplyEngine().removeRule(context.getAudit(), Rule{data});
 	} catch (runtime::Exception &e) {
 		ERROR("Failed to remove rule : " + std::string(e.what()));
 		return -1;
diff --git a/server/server.cpp b/server/server.cpp
index 6037038..7197020 100644
--- a/server/server.cpp
+++ b/server/server.cpp
@@ -147,6 +147,6 @@ void Server::loadRuleSet(const std::string &name)
 	RuleSetLoader loader;
 	AbstractRuleSet::RuleList list = loader.load(name);
 	for (auto &r : list) {
-		ruleApplyEngine.addRule(*audit, r.data());
+		ruleApplyEngine.addRule(*audit, r);
 	}
 }