Skip to content

Commit

Permalink
revise unset of a session variable
Browse files Browse the repository at this point in the history
  • Loading branch information
@t-horikawa
t-horikawa committed Sep 17, 2024
1 parent 05f841f commit 58ab7b2
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 28 deletions.
4 changes: 2 additions & 2 deletions docs/internal/session-control-design_ja.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,9 +541,9 @@ public:
explicit session_variable_set(std::vector<std::tuple<std::string, variable_type, value_type>> declarations);
/**
* @brief returns whether or not the variable with the specified name exists.
* @brief returns whether or not the variable with the specified name has been declared.
* @param name the variable name
* @return true the variable with the name exists
* @return true the variable with the name has been declared
* @return false there are no variables with the name
*/
[[nodiscard]] bool exists(std::string_view name) const noexcept;
Expand Down
12 changes: 2 additions & 10 deletions include/tateyama/session/variable_set.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ class session_variable_set {
explicit session_variable_set(std::vector<std::tuple<std::string, variable_type, value_type>> declarations);

/**
* @brief returns whether or not the variable with the specified name exists.
* @brief returns whether or not the variable with the specified name has been declared.
* @param name the variable name
* @return true the variable with the name exists
* @return true the variable with the name has been declared
* @return false there are no variables with the name
*/
[[nodiscard]] bool exists(std::string_view name) const noexcept;
Expand Down Expand Up @@ -88,14 +88,6 @@ class session_variable_set {
*/
bool set(std::string_view name, value_type value);

/**
* @brief unsets the value to the variable with the specified name.
* @param name the variable name
* @return true if successfully unset the value
* @return false if the target variable is not declared
*/
bool unset(std::string_view name);

// ...

private:
Expand Down
5 changes: 3 additions & 2 deletions src/tateyama/session/resource/bridge.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ std::optional<error_descriptor> bridge::unset_valiable(std::string_view session_
if (!vs.exists(name)) {
return std::make_pair(tateyama::proto::session::diagnostic::ErrorCode::SESSION_VARIABLE_NOT_DECLARED, "session variable by that name has not been declared");
}
if (vs.unset(name)) {
if (vs.set(name, sessions_core_impl_.variable_declarations().make_variable_set().get(name))) {
return std::nullopt;
}
return std::make_pair(tateyama::proto::session::diagnostic::ErrorCode::OPERATION_NOT_PERMITTED, "should not reach here");
Expand All @@ -265,7 +265,8 @@ class value {
explicit value(::tateyama::proto::session::response::SessionGetVariable_Success* mutable_success) : mutable_success_(mutable_success) {
}
std::optional<error_descriptor> operator()([[maybe_unused]] const std::monostate& data) {
return std::make_pair(tateyama::proto::session::diagnostic::ErrorCode::OPERATION_NOT_PERMITTED, "operation is not permitted");
// does not set value
return std::nullopt;
}
std::optional<error_descriptor> operator()(const bool data) {
mutable_success_->set_bool_value(data);
Expand Down
9 changes: 0 additions & 9 deletions src/tateyama/session/variable_set.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,4 @@ bool session_variable_set::set(std::string_view name, value_type value) {
return false;
}

bool session_variable_set::unset(std::string_view name) {
auto it = variable_set_.find(std::string(name));
if (it != variable_set_.end()) {
variable_set_.erase(it);
return true;
}
return false;
}

}
135 changes: 130 additions & 5 deletions test/tateyama/session/session_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class session_test :
private:
tateyama::endpoint::common::session_info_impl session_info_for_existing_session_{111, "IPC", "9999", "label_fot_test", "application_for_test", "user_fot_test"};
std::vector<std::tuple<std::string, tateyama::session::session_variable_set::variable_type, tateyama::session::session_variable_set::value_type>> variable_declarations_ {
{"test_integer", tateyama::session::session_variable_type::signed_integer, static_cast<std::int64_t>(123)}
{"test_integer", tateyama::session::session_variable_type::signed_integer, static_cast<std::int64_t>(123)},
{"test_monostate", tateyama::session::session_variable_type::signed_integer, std::monostate{}},
};
tateyama::session::session_variable_set session_variable_set{variable_declarations_};

Expand Down Expand Up @@ -308,7 +309,7 @@ TEST_F(session_test, session_set_variable) {
sv.shutdown();
}

TEST_F(session_test, session_unset_variable) {
TEST_F(session_test, session_unset_variable_monostate) {
auto cfg = api::configuration::create_configuration("", tateyama::test::default_configuration_for_tests);
set_dbpath(*cfg);
framework::server sv{framework::boot_mode::database_server, cfg};
Expand All @@ -321,6 +322,7 @@ TEST_F(session_test, session_unset_variable) {
auto ss = sv.find_resource<session::resource::bridge>();
ss->register_session(session_context_);

// set variable
{
std::string str{};
{
Expand All @@ -329,7 +331,129 @@ TEST_F(session_test, session_unset_variable) {
rq.set_service_message_version_minor(0);
auto svrq = rq.mutable_session_set_variable();
svrq->set_session_specifier(":111");
svrq->set_name("test_integer");
svrq->set_name("test_monostate");
svrq->set_value("321");
str = rq.SerializeAsString();
rq.clear_session_set_variable();
}

auto svrreq = std::make_shared<test_request>(10, session::service::bridge::tag, str);
auto svrres = std::make_shared<test_response>();

(*router)(svrreq, svrres);
EXPECT_EQ(10, svrres->session_id_);
auto& body = svrres->body_;

::tateyama::proto::session::response::SessionSetVariable svrs{};
EXPECT_TRUE(svrs.ParseFromString(body));
EXPECT_TRUE(svrs.has_success());
}

// unset variable
{
std::string str{};
{
::tateyama::proto::session::request::Request rq{};
rq.set_service_message_version_major(1);
rq.set_service_message_version_minor(0);
auto svrq = rq.mutable_session_set_variable();
svrq->set_session_specifier(":111");
svrq->set_name("test_monostate");
str = rq.SerializeAsString();
rq.clear_session_set_variable();
}

auto svrreq = std::make_shared<test_request>(10, session::service::bridge::tag, str);
auto svrres = std::make_shared<test_response>();

(*router)(svrreq, svrres);
EXPECT_EQ(10, svrres->session_id_);
auto& body = svrres->body_;

::tateyama::proto::session::response::SessionSetVariable svrs{};
EXPECT_TRUE(svrs.ParseFromString(body));
EXPECT_TRUE(svrs.has_success());
}

{ // do get variable to check the result
std::string str{};
{
::tateyama::proto::session::request::Request rq{};
rq.set_service_message_version_major(1);
rq.set_service_message_version_minor(0);
auto gvrq = rq.mutable_session_get_variable();
gvrq->set_session_specifier(":111");
gvrq->set_name("test_monostate");
str = rq.SerializeAsString();
rq.clear_session_get_variable();
}

auto svrreq = std::make_shared<test_request>(10, session::service::bridge::tag, str);
auto svrres = std::make_shared<test_response>();

(*router)(svrreq, svrres);
EXPECT_EQ(10, svrres->session_id_);
auto& body = svrres->body_;

::tateyama::proto::session::response::SessionGetVariable gvrs{};
EXPECT_TRUE(gvrs.ParseFromString(body));
EXPECT_TRUE(gvrs.has_success());
EXPECT_EQ(gvrs.success().value_case(), ::tateyama::proto::session::response::SessionGetVariable_Success::ValueCase::VALUE_NOT_SET);
}

sv.shutdown();
}

TEST_F(session_test, session_unset_variable_value) {
auto cfg = api::configuration::create_configuration("", tateyama::test::default_configuration_for_tests);
set_dbpath(*cfg);
framework::server sv{framework::boot_mode::database_server, cfg};
add_core_components(sv);
sv.start();
auto router = sv.find_service<framework::routing_service>();
EXPECT_TRUE(router);
EXPECT_EQ(framework::routing_service::tag, router->id());

auto ss = sv.find_resource<session::resource::bridge>();
ss->register_session(session_context_);

// set variable
{
std::string str{};
{
::tateyama::proto::session::request::Request rq{};
rq.set_service_message_version_major(1);
rq.set_service_message_version_minor(0);
auto svrq = rq.mutable_session_set_variable();
svrq->set_session_specifier(":111");
svrq->set_name("test_monostate");
svrq->set_value("321");
str = rq.SerializeAsString();
rq.clear_session_set_variable();
}

auto svrreq = std::make_shared<test_request>(10, session::service::bridge::tag, str);
auto svrres = std::make_shared<test_response>();

(*router)(svrreq, svrres);
EXPECT_EQ(10, svrres->session_id_);
auto& body = svrres->body_;

::tateyama::proto::session::response::SessionSetVariable svrs{};
EXPECT_TRUE(svrs.ParseFromString(body));
EXPECT_TRUE(svrs.has_success());
}

// unset variable
{
std::string str{};
{
::tateyama::proto::session::request::Request rq{};
rq.set_service_message_version_major(1);
rq.set_service_message_version_minor(0);
auto svrq = rq.mutable_session_set_variable();
svrq->set_session_specifier(":111");
svrq->set_name("test_monostate");
str = rq.SerializeAsString();
rq.clear_session_set_variable();
}
Expand Down Expand Up @@ -368,8 +492,9 @@ TEST_F(session_test, session_unset_variable) {

::tateyama::proto::session::response::SessionGetVariable gvrs{};
EXPECT_TRUE(gvrs.ParseFromString(body));
EXPECT_TRUE(gvrs.has_error());
EXPECT_EQ(gvrs.error().error_code(), ::tateyama::proto::session::diagnostic::ErrorCode::SESSION_VARIABLE_NOT_DECLARED);
EXPECT_TRUE(gvrs.has_success());
EXPECT_EQ(gvrs.success().value_case(), ::tateyama::proto::session::response::SessionGetVariable_Success::ValueCase::kSignedIntegerValue);
EXPECT_EQ(gvrs.success().signed_integer_value(), static_cast<std::int64_t>(123));
}

sv.shutdown();
Expand Down

1 comment on commit 58ab7b2

@t-horikawa
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/project-tsurugi/tsurugi-issues/issues/973

Please sign in to comment.