From 9a79390f474a1dfb61b2d42c61d896ed5cfd109a Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Tue, 8 Jul 2014 08:42:40 +0200 Subject: [PATCH 01/16] Fix shared pointers access in Cynara class Change-Id: I85c98846655d8363e63640b88f6a756bb16c08d5 --- src/service/main/Cynara.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/service/main/Cynara.cpp b/src/service/main/Cynara.cpp index 57a15d5..c99bbe4 100644 --- a/src/service/main/Cynara.cpp +++ b/src/service/main/Cynara.cpp @@ -56,12 +56,21 @@ void Cynara::init(void) { } void Cynara::run(void) { + if (!m_socketManager) { + throw InitException(); + } + m_socketManager->run(); } void Cynara::finalize(void) { - m_logic->unbindAll(); - m_socketManager->unbindAll(); + if (m_logic) { + m_logic->unbindAll(); + } + + if (m_socketManager) { + m_socketManager->unbindAll(); + } m_logic.reset(); m_socketManager.reset(); -- 2.7.4 From 2ce06b010dd30e90cacb8c2ab5431a62e4cc5665 Mon Sep 17 00:00:00 2001 From: Lukasz Wojciechowski Date: Tue, 15 Jul 2014 14:20:44 +0200 Subject: [PATCH 02/16] Remove const specifiers in cynara_admin_policy structure With fileds defined as const, user of this structure would probably have some problems with freeing memory. Change-Id: I159ebc2ea772c76be70e5e58310f0ae1e8728c8f --- src/include/cynara-admin.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/include/cynara-admin.h b/src/include/cynara-admin.h index 39a612b..1fef8eb 100644 --- a/src/include/cynara-admin.h +++ b/src/include/cynara-admin.h @@ -62,14 +62,14 @@ extern "C" { //todo comments struct cynara_admin_policy { - const char *bucket; + char *bucket; - const char *client; - const char *user; - const char *privilege; + char *client; + char *user; + char *privilege; int result; - const char *result_extra; + char *result_extra; }; /** -- 2.7.4 From 503df5480725b3a7ff1742d6466ebd6941572a0a Mon Sep 17 00:00:00 2001 From: Lukasz Wojciechowski Date: Tue, 15 Jul 2014 14:46:22 +0200 Subject: [PATCH 03/16] Add declaration of struct cynara_admin in libcynara-admin header Change-Id: I8e441d67bec81ba83f5686a9d1d85c7c7f217cdf --- src/include/cynara-admin.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/include/cynara-admin.h b/src/include/cynara-admin.h index 1fef8eb..79f2b19 100644 --- a/src/include/cynara-admin.h +++ b/src/include/cynara-admin.h @@ -49,6 +49,9 @@ extern "C" { #endif //todo comment +struct cynara_admin; + +//todo comment #define CYNARA_ADMIN_WILDCARD "*" //todo comment -- 2.7.4 From bc4f1e398d01b70d3a8bd37384beb169951d62a7 Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 11:41:58 +0200 Subject: [PATCH 04/16] Enable logs in client library Change-Id: I46b423ddf7482392d6859496f53a5e29fa2b82b0 --- src/client/api/client-api.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/client/api/client-api.cpp b/src/client/api/client-api.cpp index d34817d..4fc1c1d 100644 --- a/src/client/api/client-api.cpp +++ b/src/client/api/client-api.cpp @@ -21,7 +21,11 @@ */ #include + #include + +#include + #include #include #include @@ -48,6 +52,9 @@ int cynara_initialize(cynara **pp_cynara, const cynara_configuration *p_conf UNU return cynara_api_result::CYNARA_API_OUT_OF_MEMORY; } + init_log(); + LOGD("Cynara client initialized"); + return cynara_api_result::CYNARA_API_SUCCESS; } -- 2.7.4 From 0d23d0cc7ba969afe332ccb131aed43c5ea92ddc Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 11:58:35 +0200 Subject: [PATCH 05/16] Fix client socket labeling with smack Change-Id: Ic859959b5b95feae93159429b976037b52fc44de --- systemd/cynara.socket | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemd/cynara.socket b/systemd/cynara.socket index 92f4f7f..9f2a870 100644 --- a/systemd/cynara.socket +++ b/systemd/cynara.socket @@ -1,7 +1,7 @@ [Socket] ListenStream=/run/cynara/cynara.socket SocketMode=0777 -SmackLabelIPIn=@ +SmackLabelIPIn=* SmackLabelIPOut=@ Service=cynara.service -- 2.7.4 From 45a30a7f9b8c126ae661fba5946ce62d64a94b33 Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 12:01:30 +0200 Subject: [PATCH 06/16] Fix memory leak in demangling symbols for backtrace functionality Change-Id: If4525dd6f610b6e2fcc2e64c5d6b89e0bf20d0d5 --- src/common/log/Backtrace.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/log/Backtrace.cpp b/src/common/log/Backtrace.cpp index 26e3e02..d2e9550 100644 --- a/src/common/log/Backtrace.cpp +++ b/src/common/log/Backtrace.cpp @@ -79,6 +79,7 @@ const std::string Backtrace::buildBacktrace(void) { snprintf(btstr, sizeof(btstr), "ip = %lx, sp = %lx, %s, %s:%d\n", (long) ip, (long) sp, realname ? realname : proc_name, m_fileName, m_lineNumber); + free(realname); backtrace += btstr; } -- 2.7.4 From 4be73eb696c997a70301767c7c4e76482c72ad0e Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 12:04:17 +0200 Subject: [PATCH 07/16] Remove unneeded return statement in Logic::check method Change-Id: Idfda59a64c76fd8721b3c5dd94c9db4b5dd8ee12 --- src/service/logic/Logic.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/service/logic/Logic.cpp b/src/service/logic/Logic.cpp index 40e2905..a7c66d5 100644 --- a/src/service/logic/Logic.cpp +++ b/src/service/logic/Logic.cpp @@ -80,7 +80,6 @@ bool Logic::check(RequestContextPtr context UNUSED, const PolicyKey &key, //in case no proper plugin is found throw PluginNotFoundException(result); - return false; } } // namespace Cynara -- 2.7.4 From 7c5e18601613330d00c905af61b5a2b377656b22 Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 12:14:39 +0200 Subject: [PATCH 08/16] Add log messages related to protocol (de)serialization Change-Id: I6ca994c29326f0f5bce2e668adc4f9a3185c1592 --- src/client/logic/Logic.cpp | 4 ++++ src/common/protocol/ProtocolClient.cpp | 29 +++++++++++++++++++------ src/common/protocol/ProtocolFrameSerializer.cpp | 15 +++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/client/logic/Logic.cpp b/src/client/logic/Logic.cpp index 6ac6fb9..ecdf34b 100644 --- a/src/client/logic/Logic.cpp +++ b/src/client/logic/Logic.cpp @@ -80,6 +80,10 @@ cynara_api_result Logic::check(const std::string &client, const std::string &ses LOGC("Critical error. Casting Response to CheckResponse failed."); throw UnexpectedErrorException("Error casting Response to CheckResponse"); } + + LOGD("checkResponse: policyType = %d, metadata = %s", + (int)checkResponse->m_resultRef.policyType(), + checkResponse->m_resultRef.metadata().c_str()); } catch (const ServerConnectionErrorException &ex) { LOGE("Cynara service not available."); onDisconnected(); diff --git a/src/common/protocol/ProtocolClient.cpp b/src/common/protocol/ProtocolClient.cpp index 9369b54..9ad801d 100644 --- a/src/common/protocol/ProtocolClient.cpp +++ b/src/common/protocol/ProtocolClient.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -54,6 +55,9 @@ RequestPtr ProtocolClient::deserializeCheckRequest(ProtocolFrameHeader &frame) { ProtocolDeserialization::deserialize(frame, userId); ProtocolDeserialization::deserialize(frame, privilegeId); + LOGD("Deserialized CheckRequest: client = %s, user = %s, privilege = %s", + clientId.c_str(), userId.c_str(), privilegeId.c_str()); + return std::make_shared(PolicyKey(clientId, userId, privilegeId), frame.sequenceNumber()); } @@ -62,11 +66,12 @@ RequestPtr ProtocolClient::extractRequestFromBuffer(BinaryQueue &bufferQueue) { ProtocolFrameSerializer::deserializeHeader(m_frameHeader, bufferQueue); if (m_frameHeader.isFrameComplete()) { - ProtocolOpCode requestId; + ProtocolOpCode opCode; m_frameHeader.resetState(); - ProtocolDeserialization::deserialize(m_frameHeader, requestId); - switch (requestId) { + ProtocolDeserialization::deserialize(m_frameHeader, opCode); + LOGD("Deserialized opCode = %d", (int)opCode); + switch (opCode) { case OpCheckPolicy: return deserializeCheckRequest(m_frameHeader); default: @@ -85,18 +90,24 @@ ResponsePtr ProtocolClient::deserializeCheckResponse(ProtocolFrameHeader &frame) ProtocolDeserialization::deserialize(frame, result); ProtocolDeserialization::deserialize(frame, additionalInfo); - return std::make_shared(PolicyResult(result, additionalInfo), frame.sequenceNumber()); + const PolicyResult policyResult(result, additionalInfo); + + LOGD("Deserialized CheckResponse: result = %d, metadata = %s", + (int)policyResult.policyType(), policyResult.metadata().c_str()); + + return std::make_shared(policyResult, frame.sequenceNumber()); } ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueue &bufferQueue) { ProtocolFrameSerializer::deserializeHeader(m_frameHeader, bufferQueue); if (m_frameHeader.isFrameComplete()) { - ProtocolOpCode requestId; + ProtocolOpCode opCode; m_frameHeader.resetState(); - ProtocolDeserialization::deserialize(m_frameHeader, requestId); - switch (requestId) { + ProtocolDeserialization::deserialize(m_frameHeader, opCode); + LOGD("Deserialized opCode = %d", (int)opCode); + switch (opCode) { case OpCheckPolicy: return deserializeCheckResponse(m_frameHeader); default: @@ -111,6 +122,10 @@ ResponsePtr ProtocolClient::extractResponseFromBuffer(BinaryQueue &bufferQueue) void ProtocolClient::execute(RequestContextPtr context, CheckRequestPtr request) { ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(request->sequenceNumber()); + LOGD("Serializing CheckRequest: client = %s, user = %s, privilege = %s", + request->key().client().value().c_str(), + request->key().user().value().c_str(), request->key().privilege().value().c_str()); + ProtocolSerialization::serialize(*frame, OpCheckPolicy); ProtocolSerialization::serialize(*frame, request->key().client().value()); ProtocolSerialization::serialize(*frame, request->key().user().value()); diff --git a/src/common/protocol/ProtocolFrameSerializer.cpp b/src/common/protocol/ProtocolFrameSerializer.cpp index 25a5649..d8c4c7b 100644 --- a/src/common/protocol/ProtocolFrameSerializer.cpp +++ b/src/common/protocol/ProtocolFrameSerializer.cpp @@ -24,6 +24,7 @@ #include #include +#include #include "ProtocolFrameSerializer.h" @@ -36,11 +37,16 @@ void ProtocolFrameSerializer::deserializeHeader(ProtocolFrameHeader &frameHeader return; } + LOGD("Deserializing frameHeader"); + frameHeader.setHeaderContent(BinaryQueuePtr(&data, [=] (BinaryQueue *) {})); ProtocolFrameSignature signature; ProtocolDeserialization::deserialize(frameHeader, frameHeader.m_signature.length(), signature); + + LOGD("Deserialized signature = %s", signature.c_str()); + if (ProtocolFrameHeader::m_signature != signature) { throw InvalidProtocolException(InvalidProtocolException::InvalidSignature); } @@ -48,6 +54,9 @@ void ProtocolFrameSerializer::deserializeHeader(ProtocolFrameHeader &frameHeader ProtocolDeserialization::deserialize(frameHeader, frameHeader.m_frameLength); ProtocolDeserialization::deserialize(frameHeader, frameHeader.m_sequenceNumber); + LOGD("Deserialized frameLength = %d, sequenceNumber = %d", + (int)frameHeader.m_frameLength, (int)frameHeader.m_sequenceNumber); + frameHeader.setHeaderComplete(); } @@ -57,6 +66,8 @@ void ProtocolFrameSerializer::deserializeHeader(ProtocolFrameHeader &frameHeader } ProtocolFramePtr ProtocolFrameSerializer::startSerialization(ProtocolFrameSequenceNumber sequenceNumber) { + LOGD("Serialization started"); + BinaryQueuePtr headerQueue = std::make_shared(); BinaryQueuePtr bodyQueue = std::make_shared(); ProtocolFrameHeaderPtr header = std::make_shared(headerQueue); @@ -71,6 +82,10 @@ void ProtocolFrameSerializer::finishSerialization(ProtocolFramePtr frame, Binary ProtocolSerialization::serialize(frameHeader, frameHeader.m_frameLength); ProtocolSerialization::serialize(frameHeader, frameHeader.m_sequenceNumber); + LOGD("Serialize frameHeader: signature = %s, frameLength = %d, sequenceNumber = %d", + ProtocolFrameHeader::m_signature.c_str(), (int)frameHeader.m_frameLength, + (int)frameHeader.m_sequenceNumber); + data.appendMoveFrom(frameHeader.headerContent()); data.appendMoveFrom(frame->bodyContent()); } -- 2.7.4 From ea569ba81dd293e8a9de7f94711cdc92992329b5 Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 13:48:19 +0200 Subject: [PATCH 09/16] Add serialization of CheckResponse object Change-Id: I78c0b69dd6502e3d1067f8e75da90324ff8d61da --- src/common/protocol/ProtocolClient.cpp | 14 ++++++++++++++ src/common/protocol/ProtocolClient.h | 1 + 2 files changed, 15 insertions(+) diff --git a/src/common/protocol/ProtocolClient.cpp b/src/common/protocol/ProtocolClient.cpp index 9ad801d..d87d7e7 100644 --- a/src/common/protocol/ProtocolClient.cpp +++ b/src/common/protocol/ProtocolClient.cpp @@ -134,4 +134,18 @@ void ProtocolClient::execute(RequestContextPtr context, CheckRequestPtr request) ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); } +void ProtocolClient::execute(RequestContextPtr context, CheckResponsePtr response) { + ProtocolFramePtr frame = ProtocolFrameSerializer::startSerialization(response->sequenceNumber()); + + LOGD("Serializing CheckResponse: op [%d], policyType [%d], metadata <%s>", + (int)OpCheckPolicy, (int)response->m_resultRef.policyType(), + response->m_resultRef.metadata().c_str()); + + ProtocolSerialization::serialize(*frame, OpCheckPolicy); + ProtocolSerialization::serialize(*frame, response->m_resultRef.policyType()); + ProtocolSerialization::serialize(*frame, response->m_resultRef.metadata()); + + ProtocolFrameSerializer::finishSerialization(frame, context->responseQueue()); +} + } // namespace Cynara diff --git a/src/common/protocol/ProtocolClient.h b/src/common/protocol/ProtocolClient.h index 5f20547..408b0a6 100644 --- a/src/common/protocol/ProtocolClient.h +++ b/src/common/protocol/ProtocolClient.h @@ -42,6 +42,7 @@ public: virtual ResponsePtr extractResponseFromBuffer(BinaryQueue &bufferQueue); virtual void execute(RequestContextPtr context, CheckRequestPtr request); + virtual void execute(RequestContextPtr context, CheckResponsePtr response); private: RequestPtr deserializeCheckRequest(ProtocolFrameHeader &frame); -- 2.7.4 From 317c496b68e037c7831705a05480454c36cb4f5b Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Tue, 15 Jul 2014 14:16:20 +0200 Subject: [PATCH 10/16] Add conditional logs building This feature is needed for correct building tests when tested units use logging mechanism. Tests should not depend on logger. Change-Id: I110f5f0238726d1110388eaefc028c90747aa747 --- src/common/log/Backtrace.h | 6 +++++- src/common/log/log.h | 16 +++++++++++----- test/CMakeLists.txt | 1 + 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/common/log/Backtrace.h b/src/common/log/Backtrace.h index b697195..df023ba 100644 --- a/src/common/log/Backtrace.h +++ b/src/common/log/Backtrace.h @@ -25,15 +25,17 @@ #ifndef SRC_COMMON_LOG_BACKTRACE_H_ #define SRC_COMMON_LOG_BACKTRACE_H_ +#ifndef CYNARA_NO_LOGS #define UNW_LOCAL_ONLY #include +#endif #include namespace Cynara { class Backtrace { public: -#ifdef BUILD_TYPE_DEBUG +#if defined(BUILD_TYPE_DEBUG) && !defined(CYNARA_NO_LOGS) static const std::string getBacktrace(void); #else static const std::string getBacktrace(void) { @@ -51,7 +53,9 @@ private: void operator=(Backtrace const &) = delete; const std::string buildBacktrace(void); +#ifndef CYNARA_NO_LOGS void getSourceInfo(unw_word_t proc_address); +#endif private: const char *m_fileName; diff --git a/src/common/log/log.h b/src/common/log/log.h index 6f1eca4..0ed2c3e 100644 --- a/src/common/log/log.h +++ b/src/common/log/log.h @@ -25,15 +25,21 @@ #ifndef CYNARA_COMMON_LOG_H #define CYNARA_COMMON_LOG_H +#ifndef CYNARA_NO_LOGS #include +#endif extern int __log_level; -#define __LOG(LEVEL, ...) \ - do { \ - if(LEVEL <= __log_level) \ - sd_journal_print(LEVEL, __VA_ARGS__); \ - } while (0) +#ifndef CYNARA_NO_LOGS + #define __LOG(LEVEL, ...) \ + do { \ + if(LEVEL <= __log_level) \ + sd_journal_print(LEVEL, __VA_ARGS__); \ + } while (0) +#else + #define __LOG(LEVEL, ...) +#endif #define LEGM(...) __LOG(LOG_EMERG, __VA_ARGS__) /* system is unusable */ #define LOGA(...) __LOG(LOG_ALERT, __VA_ARGS__) /* action must be taken immediately */ diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0b6f913..52117a5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -37,6 +37,7 @@ SET(CMAKE_CXX_FLAGS_RELEASE "-g -std=c++0x -O2") ADD_DEFINITIONS("-Werror") # Make all warnings into errors. ADD_DEFINITIONS("-Wall") # Generate all warnings ADD_DEFINITIONS("-Wextra") # Generate even more extra warnings +ADD_DEFINITIONS("-DCYNARA_NO_LOGS") # Disable building logs MESSAGE(STATUS "CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}") -- 2.7.4 From 2d8bee5aa14dcdf683a62761cac25e324f50d5e1 Mon Sep 17 00:00:00 2001 From: Adam Malinowski Date: Mon, 14 Jul 2014 14:39:55 +0200 Subject: [PATCH 11/16] Add backtrace to exception message This patch introduces new method 'message()' in class Exception which must be overloaded by all dervied classes instead of 'what()' method. This is needed to add backtrace info to exception message. Change-Id: Ic451578c72566300283843468a32297fdec42f56 --- .../exceptions/BucketAlreadyExistsException.h | 4 ++++ src/common/exceptions/BucketNotExistsException.h | 4 ++++ .../exceptions/BucketRecordCorruptedException.h | 4 ++-- .../exceptions/DefaultBucketDeletionException.h | 4 ++++ .../exceptions/DescriptorNotExistsException.h | 4 ++-- src/common/exceptions/Exception.h | 15 ++++++++++++- src/common/exceptions/InitException.h | 4 ++++ src/common/exceptions/InvalidProtocolException.h | 4 ++-- src/common/exceptions/NotImplementedException.h | 4 ++++ src/common/exceptions/NullPointerException.h | 4 ++-- src/common/exceptions/OutOfDataException.h | 4 ++-- src/common/exceptions/PluginNotFoundException.h | 4 ++-- .../exceptions/ServerConnectionErrorException.h | 2 +- src/common/exceptions/UnexpectedErrorException.h | 4 ++-- test/common/exceptions/bucketrecordcorrupted.cpp | 25 +++++++++++++++++----- 15 files changed, 69 insertions(+), 21 deletions(-) diff --git a/src/common/exceptions/BucketAlreadyExistsException.h b/src/common/exceptions/BucketAlreadyExistsException.h index 5fbdbe9..9fa7d6f 100644 --- a/src/common/exceptions/BucketAlreadyExistsException.h +++ b/src/common/exceptions/BucketAlreadyExistsException.h @@ -36,6 +36,10 @@ public: BucketAlreadyExistsException(const PolicyBucketId &bucketId) : m_bucketId(bucketId) {} virtual ~BucketAlreadyExistsException() = default; + virtual const std::string message(void) const { + return "BucketAlreadyExistsException"; + } + private: PolicyBucketId m_bucketId; diff --git a/src/common/exceptions/BucketNotExistsException.h b/src/common/exceptions/BucketNotExistsException.h index c1668a6..54c942e 100644 --- a/src/common/exceptions/BucketNotExistsException.h +++ b/src/common/exceptions/BucketNotExistsException.h @@ -36,6 +36,10 @@ public: BucketNotExistsException(const PolicyBucketId &bucketId) : m_bucketId(bucketId) {} virtual ~BucketNotExistsException() = default; + virtual const std::string message(void) const { + return "BucketNotExistsException"; + } + private: PolicyBucketId m_bucketId; diff --git a/src/common/exceptions/BucketRecordCorruptedException.h b/src/common/exceptions/BucketRecordCorruptedException.h index aaa5e11..227c1d9 100644 --- a/src/common/exceptions/BucketRecordCorruptedException.h +++ b/src/common/exceptions/BucketRecordCorruptedException.h @@ -50,14 +50,14 @@ public: return copy; } - virtual const char* what(void) const noexcept { + virtual const std::string message(void) const { if (m_whatMsg.empty()) { m_whatMsg = "Bucket record corrupted at" + formatedFilename() + formatedLineNumber() + ": <" + slicedLine() + ">"; } - return m_whatMsg.c_str(); + return m_whatMsg; } protected: diff --git a/src/common/exceptions/DefaultBucketDeletionException.h b/src/common/exceptions/DefaultBucketDeletionException.h index 18101e7..6a92ec5 100644 --- a/src/common/exceptions/DefaultBucketDeletionException.h +++ b/src/common/exceptions/DefaultBucketDeletionException.h @@ -33,6 +33,10 @@ class DefaultBucketDeletionException : public Exception { public: DefaultBucketDeletionException() = default; virtual ~DefaultBucketDeletionException() = default; + + virtual const std::string message(void) const { + return "DefaultBucketDeletionException"; + } }; } /* namespace Cynara */ diff --git a/src/common/exceptions/DescriptorNotExistsException.h b/src/common/exceptions/DescriptorNotExistsException.h index ab12771..5dd9553 100644 --- a/src/common/exceptions/DescriptorNotExistsException.h +++ b/src/common/exceptions/DescriptorNotExistsException.h @@ -45,8 +45,8 @@ public: virtual ~DescriptorNotExistsException() = default; - virtual const char *what(void) const noexcept { - return m_whatMsg.c_str(); + virtual const std::string message(void) const { + return m_whatMsg; } }; diff --git a/src/common/exceptions/Exception.h b/src/common/exceptions/Exception.h index b4d11b5..92546b2 100644 --- a/src/common/exceptions/Exception.h +++ b/src/common/exceptions/Exception.h @@ -23,13 +23,26 @@ #define SRC_COMMON_EXCEPTIONS_EXCEPTION_H_ #include +#include namespace Cynara { class Exception : public std::exception { public: - Exception() = default; + Exception() { + m_backtrace = Backtrace::getBacktrace(); + } + virtual ~Exception() = default; + + virtual const char *what(void) const noexcept { + return (message() + " From: " + m_backtrace).c_str(); + } + + virtual const std::string message(void) const = 0; + +private: + std::string m_backtrace; }; } /* namespace Cynara */ diff --git a/src/common/exceptions/InitException.h b/src/common/exceptions/InitException.h index f3f69fa..d06e1d7 100644 --- a/src/common/exceptions/InitException.h +++ b/src/common/exceptions/InitException.h @@ -33,6 +33,10 @@ class InitException : public Exception { public: InitException() = default; virtual ~InitException() = default; + + virtual const std::string message(void) const { + return "InitException"; + } }; } /* namespace Cynara */ diff --git a/src/common/exceptions/InvalidProtocolException.h b/src/common/exceptions/InvalidProtocolException.h index 8b1317b..a2d65d1 100644 --- a/src/common/exceptions/InvalidProtocolException.h +++ b/src/common/exceptions/InvalidProtocolException.h @@ -61,8 +61,8 @@ public: virtual ~InvalidProtocolException() = default; - virtual const char *what(void) const noexcept { - return m_whatMessage.c_str(); + virtual const std::string message(void) const { + return m_whatMessage; } ExceptionType exceptionTyp(void) const { diff --git a/src/common/exceptions/NotImplementedException.h b/src/common/exceptions/NotImplementedException.h index 90fa586..ceabb9a 100644 --- a/src/common/exceptions/NotImplementedException.h +++ b/src/common/exceptions/NotImplementedException.h @@ -33,6 +33,10 @@ class NotImplementedException : public Exception { public: NotImplementedException() = default; virtual ~NotImplementedException() = default; + + virtual const std::string message(void) const { + return "NotImplementedException"; + } }; } /* namespace Cynara */ diff --git a/src/common/exceptions/NullPointerException.h b/src/common/exceptions/NullPointerException.h index 0b5d30e..ffd0bef 100644 --- a/src/common/exceptions/NullPointerException.h +++ b/src/common/exceptions/NullPointerException.h @@ -44,8 +44,8 @@ public: virtual ~NullPointerException() = default; - virtual const char* what() const noexcept { - return m_whatMsg.c_str(); + virtual const std::string message(void) const { + return m_whatMsg; } }; diff --git a/src/common/exceptions/OutOfDataException.h b/src/common/exceptions/OutOfDataException.h index e7513b1..827eb7a 100644 --- a/src/common/exceptions/OutOfDataException.h +++ b/src/common/exceptions/OutOfDataException.h @@ -46,8 +46,8 @@ public: virtual ~OutOfDataException() = default; - virtual const char* what() const noexcept { - return m_whatMsg.c_str(); + virtual const std::string message(void) const { + return m_whatMsg; } }; diff --git a/src/common/exceptions/PluginNotFoundException.h b/src/common/exceptions/PluginNotFoundException.h index 5cbb155..345fd1d 100644 --- a/src/common/exceptions/PluginNotFoundException.h +++ b/src/common/exceptions/PluginNotFoundException.h @@ -47,8 +47,8 @@ public: virtual ~PluginNotFoundException() = default; - virtual const char *what(void) const noexcept { - return m_whatMessage.c_str(); + virtual const std::string message(void) const { + return m_whatMessage; } }; diff --git a/src/common/exceptions/ServerConnectionErrorException.h b/src/common/exceptions/ServerConnectionErrorException.h index ebdf574..a5a7b08 100644 --- a/src/common/exceptions/ServerConnectionErrorException.h +++ b/src/common/exceptions/ServerConnectionErrorException.h @@ -33,7 +33,7 @@ class ServerConnectionErrorException : public Exception { public: ServerConnectionErrorException() = default; virtual ~ServerConnectionErrorException() = default; - virtual const char* what() const noexcept { + virtual const std::string message(void) const { return "ServerConnectionError"; } }; diff --git a/src/common/exceptions/UnexpectedErrorException.h b/src/common/exceptions/UnexpectedErrorException.h index 0b85ad4..a7d100b 100644 --- a/src/common/exceptions/UnexpectedErrorException.h +++ b/src/common/exceptions/UnexpectedErrorException.h @@ -51,8 +51,8 @@ public: virtual ~UnexpectedErrorException() = default; - virtual const char* what() const noexcept { - return m_whatMessage.c_str(); + virtual const std::string message(void) const { + return m_whatMessage; } }; diff --git a/test/common/exceptions/bucketrecordcorrupted.cpp b/test/common/exceptions/bucketrecordcorrupted.cpp index ddc6039..f940442 100644 --- a/test/common/exceptions/bucketrecordcorrupted.cpp +++ b/test/common/exceptions/bucketrecordcorrupted.cpp @@ -29,48 +29,63 @@ using namespace Cynara; TEST(BucketRecordCorruptedException, line) { + using ::testing::StartsWith; + BucketRecordCorruptedException ex("line"); auto expected = "Bucket record corrupted at line: "; - ASSERT_STREQ(expected, ex.what()); + + ASSERT_THAT(ex.what(), StartsWith(expected)); ASSERT_EQ("line", ex.line()); ASSERT_EQ("", ex.filename()); ASSERT_EQ(0, ex.lineNumber()); } TEST(BucketRecordCorruptedException, line_lineno) { + using ::testing::StartsWith; + auto ex = BucketRecordCorruptedException("line").withLineNumber(10); auto expected = "Bucket record corrupted at line 10: "; - ASSERT_STREQ(expected, ex.what()); + + ASSERT_THAT(ex.what(), StartsWith(expected)); ASSERT_EQ("line", ex.line()); ASSERT_EQ("", ex.filename()); ASSERT_EQ(10, ex.lineNumber()); } TEST(BucketRecordCorruptedException, line_lineno_filename) { + using ::testing::StartsWith; + auto ex = BucketRecordCorruptedException("line").withLineNumber(10).withFilename("bucket.bkt"); auto expected = "Bucket record corrupted at bucket.bkt:10: "; - ASSERT_STREQ(expected, ex.what()); + + ASSERT_THAT(ex.what(), StartsWith(expected)); ASSERT_EQ("line", ex.line()); ASSERT_EQ("bucket.bkt", ex.filename()); ASSERT_EQ(10, ex.lineNumber()); } TEST(BucketRecordCorruptedException, line_filename) { + using ::testing::StartsWith; + auto ex = BucketRecordCorruptedException("line").withFilename("bucket.bkt"); auto expected = "Bucket record corrupted at bucket.bkt: "; - ASSERT_STREQ(expected, ex.what()); + + ASSERT_THAT(ex.what(), StartsWith(expected)); ASSERT_EQ("line", ex.line()); ASSERT_EQ("bucket.bkt", ex.filename()); ASSERT_EQ(0, ex.lineNumber()); } TEST(BucketRecordCorruptedException, line_sliced) { + using ::testing::StartsWith; + std::string line = "A very long line placed here just to check," " if slicing works as expected (83 chars)"; auto ex = BucketRecordCorruptedException(line); auto expected = "Bucket record corrupted at line:" " "; - ASSERT_STREQ(expected, ex.what()); + + ASSERT_THAT(ex.what(), StartsWith(expected)); ASSERT_EQ(line, ex.line()); ASSERT_EQ("", ex.filename()); ASSERT_EQ(0, ex.lineNumber()); -- 2.7.4 From 3120b1dd3b649009988768f4a067afc0957a43b6 Mon Sep 17 00:00:00 2001 From: Aleksander Zdyb Date: Mon, 14 Jul 2014 14:23:26 +0200 Subject: [PATCH 12/16] Add serializer for InMemoryStorageBackend::Buckets Change-Id: Ia5def8f5c8b5bb1e16c20f2adc1757849f440866 --- src/service/storage/StorageSerializer.cpp | 33 ++++++++- src/service/storage/StorageSerializer.h | 12 +++- test/CMakeLists.txt | 1 + test/storage/serializer/dump.cpp | 23 ++++-- test/storage/serializer/serialize.cpp | 116 ++++++++++++++++++++++++++++++ 5 files changed, 173 insertions(+), 12 deletions(-) create mode 100644 test/storage/serializer/serialize.cpp diff --git a/src/service/storage/StorageSerializer.cpp b/src/service/storage/StorageSerializer.cpp index 8813fc9..e7857b6 100644 --- a/src/service/storage/StorageSerializer.cpp +++ b/src/service/storage/StorageSerializer.cpp @@ -20,12 +20,14 @@ * @brief Implementation of Cynara::StorageSerializer methods */ -#include "StorageSerializer.h" +#include +#include +#include #include "types/PolicyBucket.h" #include "types/PolicyCollection.h" -#include +#include "StorageSerializer.h" namespace Cynara { @@ -34,6 +36,29 @@ char StorageSerializer::m_recordSeparator = '\n'; StorageSerializer::StorageSerializer(std::ostream &os) : m_outStream(os) {} +void StorageSerializer::dump(const InMemoryStorageBackend::Buckets &buckets, + BucketStreamOpener streamOpener) { + + for (const auto bucketIter : buckets) { + const auto &bucket = bucketIter.second; + + dumpFields(bucket.id(), bucket.defaultPolicy().policyType(), + bucket.defaultPolicy().metadata()); + } + + for (const auto bucketIter : buckets) { + const auto &bucketId = bucketIter.first; + const auto &bucket = bucketIter.second; + auto bucketSerializer = streamOpener(bucketId); + + if (bucketSerializer != nullptr) { + bucketSerializer->dump(bucket); + } else { + // TODO: Throw? + } + } +} + void StorageSerializer::dump(const PolicyBucket& bucket) { const auto &policies = bucket.policyCollection(); @@ -48,7 +73,9 @@ void StorageSerializer::dump(const PolicyKey &key) { } void StorageSerializer::dump(const PolicyType &policyType) { - outStream() << policyType; + auto oldFormat = m_outStream.flags(); + outStream() << "0x" << std::uppercase << std::hex << policyType; + m_outStream.flags(oldFormat); } void StorageSerializer::dump(const PolicyResult::PolicyMetadata &metadata) { diff --git a/src/service/storage/StorageSerializer.h b/src/service/storage/StorageSerializer.h index 33108a4..789f263 100644 --- a/src/service/storage/StorageSerializer.h +++ b/src/service/storage/StorageSerializer.h @@ -23,6 +23,7 @@ #ifndef SRC_SERVICE_STORAGE_STORAGESERIALIZER_H_ #define SRC_SERVICE_STORAGE_STORAGESERIALIZER_H_ +#include "InMemoryStorageBackend.h" #include "types/PolicyBucketId.h" #include "types/PolicyCollection.h" #include "types/PolicyResult.h" @@ -37,12 +38,19 @@ class PolicyKey; class StorageSerializer { public: + typedef std::function(const PolicyBucketId &)> + BucketStreamOpener; + StorageSerializer(std::ostream &os); - void dump(const PolicyBucket &bucket); + virtual ~StorageSerializer() = default; + + virtual void dump(const InMemoryStorageBackend::Buckets &buckets, + BucketStreamOpener streamOpener); + virtual void dump(const PolicyBucket &bucket); protected: template - inline void dumpFields(const Arg1 arg1, const Args&... args) { + inline void dumpFields(const Arg1 &arg1, const Args&... args) { dump(arg1); if (sizeof...(Args) > 0) { outStream() << fieldSeparator(); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 52117a5..f3eee4b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -70,6 +70,7 @@ SET(CYNARA_TESTS_SOURCES storage/serializer/bucket_load.cpp storage/serializer/deserialize.cpp storage/serializer/dump.cpp + storage/serializer/serialize.cpp common/types/policybucket.cpp helpers.cpp ) diff --git a/test/storage/serializer/dump.cpp b/test/storage/serializer/dump.cpp index b75928e..d85950f 100644 --- a/test/storage/serializer/dump.cpp +++ b/test/storage/serializer/dump.cpp @@ -17,7 +17,7 @@ * @file dump.cpp * @author Aleksander Zdyb * @version 1.0 - * @brief Test for dumping feature of Cynara::Serializer + * @brief Tests for dumping feature of Cynara::StorageSerializer */ #include @@ -49,7 +49,6 @@ TEST(serializer_dump, dump_bucket) { PolicyKey pk1 = Helpers::generatePolicyKey("1"); PolicyKey pk2 = Helpers::generatePolicyKey("2"); - PolicyBucket bucket = {{ Policy::simpleWithKey(pk1, PredefinedPolicyType::ALLOW), Policy::simpleWithKey(pk2, PredefinedPolicyType::DENY) }}; @@ -57,9 +56,14 @@ TEST(serializer_dump, dump_bucket) { StorageSerializer serializer(outputStream); serializer.dump(bucket); + // TODO: Cynara::PolicyCollection is a vector, but in future version this may change + // and so, we should not expect the exact order of records in serialized stream + // See: StorageSerializerFixture::dump_buckets in serialize.cpp std::stringstream expected; - expected << pk1.toString() << ";" << PredefinedPolicyType::ALLOW << ";" << "\n" - << pk2.toString() << ";" << PredefinedPolicyType::DENY << ";" << "\n"; + expected + << std::hex << std::uppercase + << pk1.toString() << ";" << "0x" << PredefinedPolicyType::ALLOW << ";" << "\n" + << pk2.toString() << ";" << "0x" << PredefinedPolicyType::DENY << ";" << "\n"; ASSERT_EQ(expected.str(), outputStream.str()); } @@ -78,10 +82,15 @@ TEST(serializer_dump, dump_bucket_bucket) { StorageSerializer serializer(outputStream); serializer.dump(bucket); + // TODO: Cynara::PolicyCollection is a vector, but in future version this may change + // and so, we should not expect the exact order of records in serialized stream + // See: StorageSerializerFixture::dump_buckets in serialize.cpp std::stringstream expected; - expected << pk1.toString() << ";" << PredefinedPolicyType::BUCKET << ";" << bucketId << "\n" - << pk2.toString() << ";" << PredefinedPolicyType::DENY << ";" << "\n" - << pk3.toString() << ";" << PredefinedPolicyType::BUCKET << ";" << bucketId << "\n"; + expected + << std::hex << std::uppercase + << pk1.toString() << ";" << "0x" << PredefinedPolicyType::BUCKET << ";" << bucketId << "\n" + << pk2.toString() << ";" << "0x" << PredefinedPolicyType::DENY << ";" << "\n" + << pk3.toString() << ";" << "0x" << PredefinedPolicyType::BUCKET << ";" << bucketId << "\n"; ASSERT_EQ(expected.str(), outputStream.str()); } diff --git a/test/storage/serializer/serialize.cpp b/test/storage/serializer/serialize.cpp new file mode 100644 index 0000000..dddd30d --- /dev/null +++ b/test/storage/serializer/serialize.cpp @@ -0,0 +1,116 @@ +/* + * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/* + * @file serialize.cpp + * @author Aleksander Zdyb + * @version 1.0 + * @brief Tests for dumping feature of Cynara::StorageSerializer + */ + +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include + +class FakeStreamForBucketId { +public: + MOCK_METHOD1(streamForBucketId, + std::shared_ptr(const Cynara::PolicyBucketId &)); + + Cynara::StorageSerializer::BucketStreamOpener streamOpener() { + return std::bind(&FakeStreamForBucketId::streamForBucketId, this, std::placeholders::_1); + } +}; + +// Fake StorageSerializer for Cynara::PolicyBucket +class FakeStorageSerializer : public Cynara::StorageSerializer { +public: + FakeStorageSerializer() : Cynara::StorageSerializer(outStream) {} + MOCK_METHOD1(dump, void(const Cynara::PolicyBucket &bucket)); + std::ostringstream outStream; +}; + +class StorageSerializerFixture : public ::testing::Test { +public: + virtual ~StorageSerializerFixture() = default; + + Cynara::InMemoryStorageBackend::Buckets buckets; + FakeStreamForBucketId fakeStreamOpener; +}; + +using namespace Cynara; + +// Be sure no calls to streamForBucketId() are made +// and output stream is not touched +TEST_F(StorageSerializerFixture, dump_buckets_empty) { + std::ostringstream outStream; + StorageSerializer serializer(outStream); + serializer.dump(InMemoryStorageBackend::Buckets(), fakeStreamOpener.streamOpener()); + + // Stream should be empty + ASSERT_EQ(0, outStream.tellp()); +} + +TEST_F(StorageSerializerFixture, dump_buckets) { + using ::testing::_; + using ::testing::Property; + using ::testing::Return; + using ::testing::UnorderedElementsAreArray; + + // Will be returned as serializer for buckets + auto fakeBucketSerializer = std::make_shared(); + + buckets = { + { "bucket1", PolicyBucket("bucket1", PredefinedPolicyType::DENY) }, + { "bucket2", PolicyBucket("bucket2", PredefinedPolicyType::DENY) }, + { "bucket3", + PolicyBucket("bucket3", PolicyResult(PredefinedPolicyType::BUCKET, "bucket2")) } + }; + + std::stringstream outStream; + StorageSerializer dbSerializer(outStream); + + // Make sure stream was opened for each bucket + EXPECT_CALL(fakeStreamOpener, streamForBucketId(_)) + .Times(buckets.size()).WillRepeatedly(Return(fakeBucketSerializer)); + + // Make sure every bucket was dumped + for (const auto &bucket : buckets) { + EXPECT_CALL(*fakeBucketSerializer, dump(Property(&PolicyBucket::id, bucket.first))); + } + + dbSerializer.dump(buckets, fakeStreamOpener.streamOpener()); + + std::vector expectedRecords = { + "bucket1;0x0;", + "bucket2;0x0;", + "bucket3;0xFFFE;bucket2" + }; + + // Split stream into records + auto actualRecords = std::vector(std::istream_iterator(outStream), + std::istream_iterator()); + + ASSERT_THAT(actualRecords, UnorderedElementsAreArray(expectedRecords)); +} -- 2.7.4 From 606b235439e87beeb2e676b19182b9b5104ed431 Mon Sep 17 00:00:00 2001 From: Aleksander Zdyb Date: Mon, 14 Jul 2014 12:57:46 +0200 Subject: [PATCH 13/16] Move parseMetadata() and parsePolicyType() to StorageDeserializer These static methods better fit in a more general Cynara::StorageDeserializer instead of Cynara::BucketDeserializer Change-Id: I2bad160683ac34b68d993dc7b10e8ec7ba1b1d1c --- src/service/storage/BucketDeserializer.cpp | 29 +++----------------------- src/service/storage/BucketDeserializer.h | 6 ------ src/service/storage/StorageDeserializer.cpp | 32 +++++++++++++++++++++++++---- src/service/storage/StorageDeserializer.h | 4 +++- 4 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/service/storage/BucketDeserializer.cpp b/src/service/storage/BucketDeserializer.cpp index c40cd8d..914f766 100644 --- a/src/service/storage/BucketDeserializer.cpp +++ b/src/service/storage/BucketDeserializer.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -45,8 +46,8 @@ PolicyCollection BucketDeserializer::loadPolicies(void) { try { std::size_t beginToken = 0; auto policyKey = parseKey(line, beginToken); - auto policyType = parsePolicyType(line, beginToken); - auto metadata = parseMetadata(line, beginToken); + auto policyType = StorageDeserializer::parsePolicyType(line, beginToken); + auto metadata = StorageDeserializer::parseMetadata(line, beginToken); PolicyResult policyResult(policyType, metadata); policies.push_back(std::make_shared(policyKey, policyResult)); } catch (const BucketRecordCorruptedException &ex) { @@ -73,28 +74,4 @@ PolicyKey BucketDeserializer::parseKey(const std::string &line, std::size_t &beg return PolicyKey(keyFeatures[0], keyFeatures[1], keyFeatures[2]); } -PolicyType BucketDeserializer::parsePolicyType(const std::string &line, std::size_t &beginToken) { - PolicyType policyType; - try { - size_t newBegin = 0; - policyType = std::stoi(line.substr(beginToken), &newBegin, 16); - beginToken += newBegin; - } catch(...) { - throw BucketRecordCorruptedException(line); - } - - return policyType; -} - -PolicyResult::PolicyMetadata BucketDeserializer::parseMetadata(const std::string &line, - std::size_t &beginToken) { - if (beginToken < line.size()) { - auto ret = line.substr(beginToken + 1); - beginToken = line.size(); - return ret; - } - - return std::string(); -} - } /* namespace Cynara */ diff --git a/src/service/storage/BucketDeserializer.h b/src/service/storage/BucketDeserializer.h index 5659c45..a9b7b8c 100644 --- a/src/service/storage/BucketDeserializer.h +++ b/src/service/storage/BucketDeserializer.h @@ -31,10 +31,7 @@ namespace Cynara { -class StorageDeserializer; - class BucketDeserializer { -friend StorageDeserializer; public: BucketDeserializer(std::istream &inStream) : m_inStream(inStream) {} @@ -42,9 +39,6 @@ public: protected: static PolicyKey parseKey(const std::string &line, std::size_t &beginToken); - static PolicyType parsePolicyType(const std::string &line, std::size_t &beginToken); - static PolicyResult::PolicyMetadata parseMetadata(const std::string &line, - std::size_t &beginToken); private: std::istream &m_inStream; diff --git a/src/service/storage/StorageDeserializer.cpp b/src/service/storage/StorageDeserializer.cpp index f1a433b..5f1b97b 100644 --- a/src/service/storage/StorageDeserializer.cpp +++ b/src/service/storage/StorageDeserializer.cpp @@ -45,10 +45,10 @@ void StorageDeserializer::initBuckets(InMemoryStorageBackend::Buckets &buckets) if (line.empty()) break; - size_t beginToken = 0; - auto bucketId = StorageDeserializer::parseBucketId(line, beginToken); - auto policyType = BucketDeserializer::parsePolicyType(line, beginToken); - auto metadata = BucketDeserializer::parseMetadata(line, beginToken); + std::size_t beginToken = 0; + auto bucketId = parseBucketId(line, beginToken); + auto policyType = parsePolicyType(line, beginToken); + auto metadata = parseMetadata(line, beginToken); buckets[bucketId] = PolicyBucket(bucketId, PolicyResult(policyType, metadata)); } @@ -81,4 +81,28 @@ PolicyBucketId StorageDeserializer::parseBucketId(const std::string &line, throw BucketRecordCorruptedException(line); } +PolicyType StorageDeserializer::parsePolicyType(const std::string &line, std::size_t &beginToken) { + PolicyType policyType; + try { + std::size_t newBegin = 0; + policyType = std::stoi(line.substr(beginToken), &newBegin, 16); + beginToken += newBegin; + } catch(...) { + throw BucketRecordCorruptedException(line); + } + + return policyType; +} + +PolicyResult::PolicyMetadata StorageDeserializer::parseMetadata(const std::string &line, + std::size_t &beginToken) { + if (beginToken < line.size()) { + auto ret = line.substr(beginToken + 1); + beginToken = line.size(); + return ret; + } + + return std::string(); +} + } /* namespace Cynara */ diff --git a/src/service/storage/StorageDeserializer.h b/src/service/storage/StorageDeserializer.h index 6ef5c4e..2fd0341 100644 --- a/src/service/storage/StorageDeserializer.h +++ b/src/service/storage/StorageDeserializer.h @@ -38,8 +38,10 @@ public: void initBuckets(InMemoryStorageBackend::Buckets &buckets); void loadBuckets(InMemoryStorageBackend::Buckets &buckets); -protected: static PolicyBucketId parseBucketId(const std::string &line, std::size_t &beginToken); + static PolicyType parsePolicyType(const std::string &line, std::size_t &beginToken); + static PolicyResult::PolicyMetadata parseMetadata(const std::string &line, + std::size_t &beginToken); private: std::istream &m_inStream; -- 2.7.4 From 69133dcfc371766e241f25c9a6b4d8fcf75939b4 Mon Sep 17 00:00:00 2001 From: Aleksander Zdyb Date: Mon, 14 Jul 2014 11:26:57 +0200 Subject: [PATCH 14/16] Add InMemoryStorageBackend::hasBucket() The function will be used by Cynara::Storage to check if new bucket should be created or an old one updated. Change-Id: I0c60724a112880010dcde7793346c1bc2fd687dc --- src/service/storage/InMemoryStorageBackend.cpp | 4 ++++ src/service/storage/InMemoryStorageBackend.h | 1 + src/service/storage/StorageBackend.h | 1 + test/storage/inmemorystoragebackend/buckets.cpp | 23 +++++++++++++++++++---- test/storage/storage/fakestoragebackend.h | 1 + 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/service/storage/InMemoryStorageBackend.cpp b/src/service/storage/InMemoryStorageBackend.cpp index 212c13a..51583c7 100644 --- a/src/service/storage/InMemoryStorageBackend.cpp +++ b/src/service/storage/InMemoryStorageBackend.cpp @@ -65,6 +65,10 @@ void InMemoryStorageBackend::deleteBucket(const PolicyBucketId &bucketId) { } } +bool InMemoryStorageBackend::hasBucket(const PolicyBucketId &bucketId) { + return buckets().find(bucketId) != buckets().end(); +} + void InMemoryStorageBackend::deletePolicy(const PolicyBucketId &bucketId, const PolicyKey &key) { try { // TODO: Move the erase code to PolicyCollection maybe? diff --git a/src/service/storage/InMemoryStorageBackend.h b/src/service/storage/InMemoryStorageBackend.h index 6ed0518..f4db588 100644 --- a/src/service/storage/InMemoryStorageBackend.h +++ b/src/service/storage/InMemoryStorageBackend.h @@ -47,6 +47,7 @@ public: virtual void insertPolicy(const PolicyBucketId &bucketId, PolicyPtr policy); virtual void createBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy); virtual void deleteBucket(const PolicyBucketId &bucketId); + virtual bool hasBucket(const PolicyBucketId &bucketId); virtual void deletePolicy(const PolicyBucketId &bucketId, const PolicyKey &key); virtual void deleteLinking(const PolicyBucketId &bucketId); diff --git a/src/service/storage/StorageBackend.h b/src/service/storage/StorageBackend.h index 06a3a74..005a00d 100644 --- a/src/service/storage/StorageBackend.h +++ b/src/service/storage/StorageBackend.h @@ -45,6 +45,7 @@ public: virtual void createBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy) = 0; virtual void deleteBucket(const PolicyBucketId &bucketId) = 0; + virtual bool hasBucket(const PolicyBucketId &bucketId) = 0; virtual void deletePolicy(const PolicyBucketId &bucketId, const PolicyKey &key) = 0; virtual void deleteLinking(const PolicyBucketId &bucket) = 0; diff --git a/test/storage/inmemorystoragebackend/buckets.cpp b/test/storage/inmemorystoragebackend/buckets.cpp index bf2c87c..37cf0ae 100644 --- a/test/storage/inmemorystoragebackend/buckets.cpp +++ b/test/storage/inmemorystoragebackend/buckets.cpp @@ -30,13 +30,13 @@ using namespace Cynara; -TEST_F(InMemeoryStorageBackendFixture, addBucket) { +TEST_F(InMemeoryStorageBackendFixture, createBucket) { using ::testing::ReturnRef; using ::testing::IsEmpty; FakeInMemoryStorageBackend backend; EXPECT_CALL(backend, buckets()) - .WillOnce(ReturnRef(m_buckets)); + .WillRepeatedly(ReturnRef(m_buckets)); PolicyResult defaultPolicy(PredefinedPolicyType::ALLOW); PolicyBucketId bucketId = "new-bucket"; @@ -55,7 +55,7 @@ TEST_F(InMemeoryStorageBackendFixture, deleteBucket) { FakeInMemoryStorageBackend backend; EXPECT_CALL(backend, buckets()) - .WillOnce(ReturnRef(m_buckets)); + .WillRepeatedly(ReturnRef(m_buckets)); PolicyBucketId bucketId = "delete-bucket"; m_buckets.insert({ bucketId, PolicyBucket() }); @@ -65,12 +65,27 @@ TEST_F(InMemeoryStorageBackendFixture, deleteBucket) { ASSERT_THAT(m_buckets, IsEmpty()); } +TEST_F(InMemeoryStorageBackendFixture, hasBucket) { + using ::testing::ReturnRef; + using ::testing::IsEmpty; + + FakeInMemoryStorageBackend backend; + EXPECT_CALL(backend, buckets()) + .WillRepeatedly(ReturnRef(m_buckets)); + + PolicyBucketId bucketId = "bucket"; + m_buckets.insert({ bucketId, PolicyBucket() }); + + ASSERT_TRUE(backend.hasBucket(bucketId)); + ASSERT_FALSE(backend.hasBucket("non-existent")); +} + TEST_F(InMemeoryStorageBackendFixture, deleteNonexistentBucket) { using ::testing::ReturnRef; FakeInMemoryStorageBackend backend; EXPECT_CALL(backend, buckets()) - .WillOnce(ReturnRef(m_buckets)); + .WillRepeatedly(ReturnRef(m_buckets)); EXPECT_THROW(backend.deleteBucket("non-existent"), BucketNotExistsException); } diff --git a/test/storage/storage/fakestoragebackend.h b/test/storage/storage/fakestoragebackend.h index d06c67d..b2f7a45 100644 --- a/test/storage/storage/fakestoragebackend.h +++ b/test/storage/storage/fakestoragebackend.h @@ -31,6 +31,7 @@ public: MOCK_METHOD2(searchBucket, PolicyBucket(const PolicyBucketId &bucket, const PolicyKey &key)); MOCK_METHOD2(createBucket, void(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy)); MOCK_METHOD1(deleteBucket, void(const PolicyBucketId &bucketId)); + MOCK_METHOD1(hasBucket, bool(const PolicyBucketId &bucketId)); MOCK_METHOD2(deletePolicy, void(const PolicyBucketId &bucketId, const PolicyKey &key)); MOCK_METHOD1(deleteLinking, void(const PolicyBucketId &bucket)); MOCK_METHOD2(insertPolicy, void(const PolicyBucketId &bucketId, PolicyPtr policy)); -- 2.7.4 From 75d8248c9e98ffd10b4948145602d8f82873c94b Mon Sep 17 00:00:00 2001 From: Aleksander Zdyb Date: Mon, 14 Jul 2014 11:22:32 +0200 Subject: [PATCH 15/16] Add InMemoryStorageBackend::updateBucket() The function will be used by Cynara::Storage to update bucket's default policy without altering its policies. Change-Id: I777c930eedc9c63d8ce1f4a29b144eeee205f145 --- src/common/types/PolicyBucket.h | 1 + src/service/storage/InMemoryStorageBackend.cpp | 10 ++++++++++ src/service/storage/InMemoryStorageBackend.h | 1 + src/service/storage/StorageBackend.h | 1 + test/storage/inmemorystoragebackend/buckets.cpp | 26 +++++++++++++++++++++++++ test/storage/storage/fakestoragebackend.h | 5 ++++- 6 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/common/types/PolicyBucket.h b/src/common/types/PolicyBucket.h index 2e76713..120cf36 100644 --- a/src/common/types/PolicyBucket.h +++ b/src/common/types/PolicyBucket.h @@ -76,6 +76,7 @@ public: return m_policyCollection; } + // TODO: Consider StorageBackend to be only one to alter this property void setDefaultPolicy(const PolicyResult &defaultPolicy) { m_defaultPolicy = defaultPolicy; } diff --git a/src/service/storage/InMemoryStorageBackend.cpp b/src/service/storage/InMemoryStorageBackend.cpp index 51583c7..ab80521 100644 --- a/src/service/storage/InMemoryStorageBackend.cpp +++ b/src/service/storage/InMemoryStorageBackend.cpp @@ -58,6 +58,16 @@ void InMemoryStorageBackend::createBucket(const PolicyBucketId &bucketId, buckets().insert({ bucketId, newBucket }); } +void InMemoryStorageBackend::updateBucket(const PolicyBucketId &bucketId, + const PolicyResult &defaultPolicy) { + try { + auto &bucket = buckets().at(bucketId); + bucket.setDefaultPolicy(defaultPolicy); + } catch (const std::out_of_range &) { + throw BucketNotExistsException(bucketId); + } +} + void InMemoryStorageBackend::deleteBucket(const PolicyBucketId &bucketId) { auto bucketErased = buckets().erase(bucketId); if (bucketErased == 0) { diff --git a/src/service/storage/InMemoryStorageBackend.h b/src/service/storage/InMemoryStorageBackend.h index f4db588..409a84c 100644 --- a/src/service/storage/InMemoryStorageBackend.h +++ b/src/service/storage/InMemoryStorageBackend.h @@ -46,6 +46,7 @@ public: virtual PolicyBucket searchBucket(const PolicyBucketId &bucketId, const PolicyKey &key); virtual void insertPolicy(const PolicyBucketId &bucketId, PolicyPtr policy); virtual void createBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy); + virtual void updateBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy); virtual void deleteBucket(const PolicyBucketId &bucketId); virtual bool hasBucket(const PolicyBucketId &bucketId); virtual void deletePolicy(const PolicyBucketId &bucketId, const PolicyKey &key); diff --git a/src/service/storage/StorageBackend.h b/src/service/storage/StorageBackend.h index 005a00d..6689bae 100644 --- a/src/service/storage/StorageBackend.h +++ b/src/service/storage/StorageBackend.h @@ -44,6 +44,7 @@ public: virtual void insertPolicy(const PolicyBucketId &bucket, PolicyPtr policy) = 0; virtual void createBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy) = 0; + virtual void updateBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy) = 0; virtual void deleteBucket(const PolicyBucketId &bucketId) = 0; virtual bool hasBucket(const PolicyBucketId &bucketId) = 0; diff --git a/test/storage/inmemorystoragebackend/buckets.cpp b/test/storage/inmemorystoragebackend/buckets.cpp index 37cf0ae..bc9fcf6 100644 --- a/test/storage/inmemorystoragebackend/buckets.cpp +++ b/test/storage/inmemorystoragebackend/buckets.cpp @@ -48,6 +48,32 @@ TEST_F(InMemeoryStorageBackendFixture, createBucket) { ASSERT_THAT(m_buckets.at(bucketId).policyCollection(), IsEmpty()); } +TEST_F(InMemeoryStorageBackendFixture, updateBucket) { + using ::testing::ReturnRef; + + FakeInMemoryStorageBackend backend; + EXPECT_CALL(backend, buckets()) + .WillRepeatedly(ReturnRef(m_buckets)); + + PolicyBucketId bucketId = "bucket"; + auto &bucket = this->createBucket(bucketId); + bucket.setDefaultPolicy(PredefinedPolicyType::ALLOW); + + backend.updateBucket(bucketId, PredefinedPolicyType::DENY); + + ASSERT_EQ(PredefinedPolicyType::DENY, bucket.defaultPolicy()); +} + +TEST_F(InMemeoryStorageBackendFixture, updateNonexistentBucket) { + using ::testing::ReturnRef; + + FakeInMemoryStorageBackend backend; + EXPECT_CALL(backend, buckets()) + .WillRepeatedly(ReturnRef(m_buckets)); + + EXPECT_THROW(backend.updateBucket("non-existent", PredefinedPolicyType::ALLOW), + BucketNotExistsException); +} TEST_F(InMemeoryStorageBackendFixture, deleteBucket) { using ::testing::ReturnRef; diff --git a/test/storage/storage/fakestoragebackend.h b/test/storage/storage/fakestoragebackend.h index b2f7a45..bfa212d 100644 --- a/test/storage/storage/fakestoragebackend.h +++ b/test/storage/storage/fakestoragebackend.h @@ -29,7 +29,10 @@ class FakeStorageBackend : public StorageBackend { public: MOCK_METHOD1(searchDefaultBucket, PolicyBucket(const PolicyKey &key)); MOCK_METHOD2(searchBucket, PolicyBucket(const PolicyBucketId &bucket, const PolicyKey &key)); - MOCK_METHOD2(createBucket, void(const PolicyBucketId &bucketId, const PolicyResult &defaultPolicy)); + MOCK_METHOD2(createBucket, void(const PolicyBucketId &bucketId, + const PolicyResult &defaultPolicy)); + MOCK_METHOD2(updateBucket, void(const PolicyBucketId &bucketId, + const PolicyResult &defaultPolicy)); MOCK_METHOD1(deleteBucket, void(const PolicyBucketId &bucketId)); MOCK_METHOD1(hasBucket, bool(const PolicyBucketId &bucketId)); MOCK_METHOD2(deletePolicy, void(const PolicyBucketId &bucketId, const PolicyKey &key)); -- 2.7.4 From 74e95bd76deed9310c3ea966d9e2b9f89c9a2f40 Mon Sep 17 00:00:00 2001 From: Aleksander Zdyb Date: Fri, 11 Jul 2014 10:58:35 +0200 Subject: [PATCH 16/16] Implement Storage::addOrUpdateBucket() createBucket() has been is renamed to addOrUpdateBucket() in Cynara::Storage. The function will now create new bucket or, if it existed, only update its default policy. Test asserting addition of default bucket has been removed, because the common function Storage::addOrUpdateBucket() will just update default policy in such case. Change-Id: I2099f9306873d192cbbbcd34ac9769ca663bdbe3 --- src/service/storage/Storage.cpp | 12 +++++------- src/service/storage/Storage.h | 2 +- test/storage/storage/buckets.cpp | 20 +++++++++++++------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/service/storage/Storage.cpp b/src/service/storage/Storage.cpp index a35da0c..9e9f00d 100644 --- a/src/service/storage/Storage.cpp +++ b/src/service/storage/Storage.cpp @@ -93,14 +93,12 @@ void Storage::insertPolicies(const std::vector &policies) { } } -void Storage::createBucket(const PolicyBucketId &newBucketId, const PolicyResult &defaultBucketPolicy) { - // TODO: Check if bucket already exists - - if (newBucketId == defaultPolicyBucketId) { - throw BucketAlreadyExistsException(newBucketId); +void Storage::addOrUpdateBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultBucketPolicy) { + if (m_backend.hasBucket(bucketId)) { + m_backend.updateBucket(bucketId, defaultBucketPolicy); + } else { + m_backend.createBucket(bucketId, defaultBucketPolicy); } - - m_backend.createBucket(newBucketId, defaultBucketPolicy); } void Storage::deleteBucket(const PolicyBucketId &bucketId) { diff --git a/src/service/storage/Storage.h b/src/service/storage/Storage.h index e16d60c..177765c 100644 --- a/src/service/storage/Storage.h +++ b/src/service/storage/Storage.h @@ -50,7 +50,7 @@ public: PolicyResult checkPolicy(const PolicyKey &key); void insertPolicies(const std::vector &policies); - void createBucket(const PolicyBucketId &newBucketId, const PolicyResult &defaultBucketPolicy); + void addOrUpdateBucket(const PolicyBucketId &bucketId, const PolicyResult &defaultBucketPolicy); void deletePolicies(const std::vector &policies); void deleteBucket(const PolicyBucketId &bucketId); diff --git a/test/storage/storage/buckets.cpp b/test/storage/storage/buckets.cpp index 44e4cfd..c7d19ec 100644 --- a/test/storage/storage/buckets.cpp +++ b/test/storage/storage/buckets.cpp @@ -44,27 +44,33 @@ using namespace Cynara; TEST(storage, addBucket) { + using ::testing::Return; + FakeStorageBackend backend; Cynara::Storage storage(backend); PolicyBucketId bucketId = "test-bucket"; PolicyResult defaultPolicy(PredefinedPolicyType::DENY); + EXPECT_CALL(backend, hasBucket(bucketId)).WillOnce(Return(false)); EXPECT_CALL(backend, createBucket(bucketId, defaultPolicy)); - storage.createBucket(bucketId, defaultPolicy); + storage.addOrUpdateBucket(bucketId, defaultPolicy); } -// Cannot add bucket with default bucket's name -TEST(storage, addDefaultBucket) { +TEST(storage, updateBucket) { + using ::testing::Return; + FakeStorageBackend backend; Cynara::Storage storage(backend); + + PolicyBucketId bucketId = "test-bucket"; PolicyResult defaultPolicy(PredefinedPolicyType::DENY); - ASSERT_THROW( - storage.createBucket(defaultPolicyBucketId, defaultPolicy), - BucketAlreadyExistsException - ); + EXPECT_CALL(backend, hasBucket(bucketId)).WillOnce(Return(true)); + EXPECT_CALL(backend, updateBucket(bucketId, defaultPolicy)); + + storage.addOrUpdateBucket(bucketId, defaultPolicy); } // Cannot delete default bucket -- 2.7.4