Skip to content

Use shared error type map #214

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use shared error type map #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1dd7579
Use shared error map
Oct 28, 2024
a8d57b5
Use ConstPtr type
Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/utils/config.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <filesystem>
#include <list>
#include <memory>
#include <optional>
#include <regex>
#include <set>
Expand Down Expand Up @@ -122,7 +123,7 @@ class Config {

void load_and_validate_manifest(const std::string& module_id, const json& module_config);

error::ErrorTypeMap error_map;
error::ErrorTypeMap::ConstPtr error_map;

///
/// \brief loads and validates the given file \p file_path with the schema \p schema
Expand All @@ -131,7 +132,7 @@ class Config {
std::tuple<json, int> load_and_validate_with_schema(const fs::path& file_path, const json& schema);

public:
error::ErrorTypeMap get_error_map() const;
error::ErrorTypeMap::ConstPtr get_error_map() const;
std::string get_module_name(const std::string& module_id) const;
bool module_provides(const std::string& module_name, const std::string& impl_id);
json get_module_cmds(const std::string& module_name, const std::string& impl_id);
Expand Down
13 changes: 6 additions & 7 deletions include/utils/error/error_factory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,18 @@
#include <string>

#include <utils/error.hpp>
#include <utils/error/error_type_map.hpp>

namespace Everest {
namespace error {

struct ErrorTypeMap;
Copy link
Member

@andistorm andistorm Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep this forward declaration to avoid unnecessary dependencies in all these header files


class ErrorFactory {
public:
explicit ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map);
ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map, ImplementationIdentifier default_origin);
ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map, ImplementationIdentifier default_origin,
explicit ErrorFactory(ErrorTypeMap::ConstPtr error_type_map);
ErrorFactory(ErrorTypeMap::ConstPtr error_type_map, ImplementationIdentifier default_origin);
ErrorFactory(ErrorTypeMap::ConstPtr error_type_map, ImplementationIdentifier default_origin,
Severity default_severity);
ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map, std::optional<ImplementationIdentifier> default_origin,
ErrorFactory(ErrorTypeMap::ConstPtr error_type_map, std::optional<ImplementationIdentifier> default_origin,
std::optional<Severity> default_severity, std::optional<State> default_state,
std::optional<ErrorType> default_type, std::optional<ErrorSubType> default_sub_type,
std::optional<std::string> default_message, std::optional<std::string> default_vendor_id);
Expand Down Expand Up @@ -52,7 +51,7 @@ class ErrorFactory {
std::optional<std::string> default_message;
std::optional<std::string> default_vendor_id;

const std::shared_ptr<ErrorTypeMap> error_type_map;
ErrorTypeMap::ConstPtr error_type_map;

void set_description(Error& error) const;
};
Expand Down
5 changes: 3 additions & 2 deletions include/utils/error/error_manager_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>

#include <utils/error.hpp>
#include <utils/error/error_type_map.hpp>

namespace Everest {
namespace error {
Expand All @@ -19,7 +20,7 @@ class ErrorManagerImpl {
public:
using PublishErrorFunc = std::function<void(const error::Error&)>;

ErrorManagerImpl(std::shared_ptr<ErrorTypeMap> error_type_map, std::shared_ptr<ErrorDatabase> error_database,
ErrorManagerImpl(ErrorTypeMap::ConstPtr error_type_map, std::shared_ptr<ErrorDatabase> error_database,
std::list<ErrorType> allowed_error_types, PublishErrorFunc publish_raised_error,
PublishErrorFunc publish_cleared_error, const bool validate_error_types = true);

Expand Down Expand Up @@ -61,7 +62,7 @@ class ErrorManagerImpl {
PublishErrorFunc publish_cleared_error;

std::shared_ptr<ErrorDatabase> database;
std::shared_ptr<ErrorTypeMap> error_type_map;
ErrorTypeMap::ConstPtr error_type_map;
std::list<ErrorType> allowed_error_types;

const bool validate_error_types;
Expand Down
6 changes: 3 additions & 3 deletions include/utils/error/error_manager_req.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#define UTILS_ERROR_MANAGER_REQ_HPP

#include <utils/error.hpp>
#include <utils/error/error_type_map.hpp>

#include <list>
#include <map>
Expand All @@ -14,13 +15,12 @@ namespace Everest {
namespace error {

struct ErrorDatabase;
struct ErrorTypeMap;
Copy link
Collaborator Author

@dorezyuk dorezyuk Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really get what the idea behind all the forward defines was - I've removed it to just use the ErrorTypeMapPtr type

Copy link
Member

@andistorm andistorm Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of forward declarations is to avoid including dependencies in the header file to reduce compile times after changing something in the included files


class ErrorManagerReq {
public:
using SubscribeErrorFunc = std::function<void(const ErrorType&, const ErrorCallback&, const ErrorCallback&)>;

ErrorManagerReq(std::shared_ptr<ErrorTypeMap> error_type_map, std::shared_ptr<ErrorDatabase> error_database,
ErrorManagerReq(ErrorTypeMap::ConstPtr error_type_map, std::shared_ptr<ErrorDatabase> error_database,
std::list<ErrorType> allowed_error_types, SubscribeErrorFunc subscribe_error_func);

void subscribe_error(const ErrorType& type, const ErrorCallback& callback, const ErrorCallback& clear_callback);
Expand All @@ -42,7 +42,7 @@ class ErrorManagerReq {

SubscribeErrorFunc subscribe_error_func;

std::shared_ptr<ErrorTypeMap> error_type_map;
ErrorTypeMap::ConstPtr error_type_map;
std::shared_ptr<ErrorDatabase> database;
std::list<ErrorType> allowed_error_types;
};
Expand Down
6 changes: 3 additions & 3 deletions include/utils/error/error_manager_req_global.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#define UTILS_ERROR_MANAGER_REQ_GLOBAL_HPP

#include <utils/error.hpp>
#include <utils/error/error_type_map.hpp>

#include <list>
#include <map>
Expand All @@ -14,13 +15,12 @@ namespace Everest {
namespace error {

struct ErrorDatabase;
struct ErrorTypeMap;

class ErrorManagerReqGlobal {
public:
using SubscribeGlobalAllErrorsFunc = std::function<void(const ErrorCallback&, const ErrorCallback&)>;

ErrorManagerReqGlobal(std::shared_ptr<ErrorTypeMap> error_type_map, std::shared_ptr<ErrorDatabase> error_database,
ErrorManagerReqGlobal(ErrorTypeMap::ConstPtr error_type_map, std::shared_ptr<ErrorDatabase> error_database,
SubscribeGlobalAllErrorsFunc subscribe_global_all_errors_func);

void subscribe_global_all_errors(const ErrorCallback& callback, const ErrorCallback& clear_callback);
Expand All @@ -39,7 +39,7 @@ class ErrorManagerReqGlobal {

SubscribeGlobalAllErrorsFunc subscribe_global_all_errors_func;

std::shared_ptr<ErrorTypeMap> error_type_map;
ErrorTypeMap::ConstPtr error_type_map;
std::shared_ptr<ErrorDatabase> database;
};

Expand Down
7 changes: 7 additions & 0 deletions include/utils/error/error_type_map.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
#define UTILS_ERROR_TYPE_MAP_HPP

#include <filesystem>
#include <map>
#include <memory>
Copy link
Member

@andistorm andistorm Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#include <memory>

This dependency can be avoided, when not defining this ConstPtr

Copy link
Collaborator Author

@dorezyuk dorezyuk Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also not sure if I understand - we use it in this library a lot. What do you mean with "avoid" the dependency? Are you concerned about the preprocessor adding this?

#include <string>

#include <utils/error.hpp>

Expand Down Expand Up @@ -50,6 +53,10 @@ class ErrorTypeMap {
///
bool has(const ErrorType& error_type) const;

/// Const pointer to a ErrorTypeMap. This is the default how we share the
/// error type map between different classes.
using ConstPtr = std::shared_ptr<const ErrorTypeMap>;

Comment on lines +56 to +59
Copy link
Member

@andistorm andistorm Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep using std::shared_ptr<const ErrorTypeMap> since it fits the style arround here

Copy link
Collaborator Author

@dorezyuk dorezyuk Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if i understand - we're using it

private:
std::map<ErrorType, std::string> error_types;
};
Expand Down
6 changes: 3 additions & 3 deletions lib/config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ std::tuple<json, int> Config::load_and_validate_with_schema(const fs::path& file
Config::Config(std::shared_ptr<RuntimeSettings> rs) : Config(rs, false) {
}

Config::Config(std::shared_ptr<RuntimeSettings> rs, bool manager) : rs(rs), manager(manager) {
Config::Config(std::shared_ptr<RuntimeSettings> rs, bool manager) : rs(rs), manager(manager), error_map{nullptr} {
Copy link
Member

@andistorm andistorm Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This default value is not needed, especially because it is initialized in the body of the constructor

BOOST_LOG_FUNCTION();

this->manifests = json({});
Expand All @@ -406,7 +406,7 @@ Config::Config(std::shared_ptr<RuntimeSettings> rs, bool manager) : rs(rs), mana
this->types = json({});
this->errors = json({});
this->_schemas = Config::load_schemas(this->rs->schemas_dir);
this->error_map = error::ErrorTypeMap(this->rs->errors_dir);
this->error_map = std::make_shared<error::ErrorTypeMap>(error::ErrorTypeMap(this->rs->errors_dir));

// load and process config file
fs::path config_path = rs->config_file;
Expand Down Expand Up @@ -551,7 +551,7 @@ Config::Config(std::shared_ptr<RuntimeSettings> rs, bool manager) : rs(rs), mana
parse_3_tier_model_mapping();
}

error::ErrorTypeMap Config::get_error_map() const {
error::ErrorTypeMap::ConstPtr Config::get_error_map() const {
return this->error_map;
}

Expand Down
8 changes: 4 additions & 4 deletions lib/error/error_factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,23 @@
namespace Everest {
namespace error {

ErrorFactory::ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map_) :
ErrorFactory::ErrorFactory(ErrorTypeMap::ConstPtr error_type_map_) :
ErrorFactory(error_type_map_, std::nullopt, std::nullopt, std::nullopt, std::nullopt, std::nullopt, std::nullopt,
std::nullopt) {
}

ErrorFactory::ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map_, ImplementationIdentifier default_origin_) :
ErrorFactory::ErrorFactory(ErrorTypeMap::ConstPtr error_type_map_, ImplementationIdentifier default_origin_) :
ErrorFactory(error_type_map_, default_origin_, std::nullopt, std::nullopt, std::nullopt, std::nullopt, std::nullopt,
std::nullopt) {
}

ErrorFactory::ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map_, ImplementationIdentifier default_origin_,
ErrorFactory::ErrorFactory(ErrorTypeMap::ConstPtr error_type_map_, ImplementationIdentifier default_origin_,
Severity default_severity_) :
ErrorFactory(error_type_map_, default_origin_, default_severity_, std::nullopt, std::nullopt, std::nullopt,
std::nullopt, std::nullopt) {
}

ErrorFactory::ErrorFactory(std::shared_ptr<ErrorTypeMap> error_type_map_,
ErrorFactory::ErrorFactory(ErrorTypeMap::ConstPtr error_type_map_,
std::optional<ImplementationIdentifier> default_origin_,
std::optional<Severity> default_severity_, std::optional<State> default_state_,
std::optional<ErrorType> default_type_, std::optional<ErrorSubType> default_sub_type_,
Expand Down
3 changes: 1 addition & 2 deletions lib/error/error_manager_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include <utils/error.hpp>
#include <utils/error/error_database.hpp>
#include <utils/error/error_type_map.hpp>

#include <everest/logging.hpp>

Expand All @@ -15,7 +14,7 @@
namespace Everest {
namespace error {

ErrorManagerImpl::ErrorManagerImpl(std::shared_ptr<ErrorTypeMap> error_type_map_,
ErrorManagerImpl::ErrorManagerImpl(ErrorTypeMap::ConstPtr error_type_map_,
std::shared_ptr<ErrorDatabase> error_database_,
std::list<ErrorType> allowed_error_types_,
ErrorManagerImpl::PublishErrorFunc publish_raised_error_,
Expand Down
4 changes: 1 addition & 3 deletions lib/error/error_manager_req.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@
#include <utils/error.hpp>
#include <utils/error/error_database.hpp>
#include <utils/error/error_filter.hpp>
#include <utils/error/error_type_map.hpp>

#include <everest/logging.hpp>

namespace Everest {
namespace error {

ErrorManagerReq::ErrorManagerReq(std::shared_ptr<ErrorTypeMap> error_type_map_,
std::shared_ptr<ErrorDatabase> error_database_,
ErrorManagerReq::ErrorManagerReq(ErrorTypeMap::ConstPtr error_type_map_, std::shared_ptr<ErrorDatabase> error_database_,
std::list<ErrorType> allowed_error_types_, SubscribeErrorFunc subscribe_error_func_) :
error_type_map(error_type_map_),
database(error_database_),
Expand Down
3 changes: 1 addition & 2 deletions lib/error/error_manager_req_global.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@

#include <everest/logging.hpp>
#include <utils/error/error_database.hpp>
#include <utils/error/error_type_map.hpp>

namespace Everest {
namespace error {

ErrorManagerReqGlobal::ErrorManagerReqGlobal(std::shared_ptr<ErrorTypeMap> error_type_map_,
ErrorManagerReqGlobal::ErrorManagerReqGlobal(ErrorTypeMap::ConstPtr error_type_map_,
std::shared_ptr<ErrorDatabase> error_database_,
SubscribeGlobalAllErrorsFunc subscribe_global_all_errors_func_) :
error_type_map(error_type_map_),
Expand Down
16 changes: 7 additions & 9 deletions lib/everest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da
this->subscribe_global_all_errors(callback, clear_callback);
};
this->global_error_manager = std::make_shared<error::ErrorManagerReqGlobal>(
std::make_shared<error::ErrorTypeMap>(this->config.get_error_map()), global_error_database,
subscribe_global_all_errors_func);
this->config.get_error_map(), global_error_database, subscribe_global_all_errors_func);
this->global_error_state_monitor = std::make_shared<error::ErrorStateMonitor>(global_error_database);
} else {
this->global_error_manager = nullptr;
Expand Down Expand Up @@ -103,9 +102,9 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da
error::ErrorManagerImpl::PublishErrorFunc publish_cleared_error = [this, impl](const error::Error& error) {
this->publish_cleared_error(impl, error);
};
this->impl_error_managers[impl] = std::make_shared<error::ErrorManagerImpl>(
std::make_shared<error::ErrorTypeMap>(this->config.get_error_map()), error_database, allowed_error_types,
publish_raised_error, publish_cleared_error);
this->impl_error_managers[impl] =
std::make_shared<error::ErrorManagerImpl>(this->config.get_error_map(), error_database, allowed_error_types,
publish_raised_error, publish_cleared_error);
Comment on lines +105 to +107
Copy link
Member

@andistorm andistorm Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really get what changed here


// setup error state monitor
this->impl_error_state_monitors[impl] = std::make_shared<error::ErrorStateMonitor>(error_database);
Expand Down Expand Up @@ -153,8 +152,8 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da

// setup error factory
ImplementationIdentifier default_origin(this->module_id, impl, mapping);
this->error_factories[impl] = std::make_shared<error::ErrorFactory>(
std::make_shared<error::ErrorTypeMap>(this->config.get_error_map()), default_origin);
this->error_factories[impl] =
std::make_shared<error::ErrorFactory>(this->config.get_error_map(), default_origin);
}

// setup error_databases, error_managers and error_state_monitors for all requirements
Expand All @@ -177,8 +176,7 @@ Everest::Everest(std::string module_id_, const Config& config_, bool validate_da
this->subscribe_error(req, type, callback, clear_callback);
};
this->req_error_managers[req] = std::make_shared<error::ErrorManagerReq>(
std::make_shared<error::ErrorTypeMap>(this->config.get_error_map()), error_database, allowed_error_types,
subscribe_error_func);
this->config.get_error_map(), error_database, allowed_error_types, subscribe_error_func);

// setup error state monitor
this->req_error_state_monitors[req] = std::make_shared<error::ErrorStateMonitor>(error_database);
Expand Down
Loading