Skip to content

Commit c071f67

Browse files
alexey-milovidovalexey-milovidov
authored
Revert "Added new tests for session_log and fixed the inconsistency between login and logout."
1 parent 52c3704 commit c071f67

20 files changed

+26
-752
lines changed

‎src/Interpreters/Session.cpp‎

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,6 @@ ContextMutablePtr Session::makeSessionContext()
520520
{},
521521
session_context->getSettingsRef().max_sessions_for_user);
522522

523-
recordLoginSucess(session_context);
524-
525523
return session_context;
526524
}
527525

@@ -584,8 +582,6 @@ ContextMutablePtr Session::makeSessionContext(const String & session_name_, std:
584582
{ session_name_ },
585583
max_sessions_for_user);
586584

587-
recordLoginSucess(session_context);
588-
589585
return session_context;
590586
}
591587

@@ -659,35 +655,21 @@ ContextMutablePtr Session::makeQueryContextImpl(const ClientInfo * client_info_t
659655
if (user_id)
660656
user = query_context->getUser();
661657

662-
/// Interserver does not create session context
663-
recordLoginSucess(query_context);
664-
665-
return query_context;
666-
}
667-
668-
669-
void Session::recordLoginSucess(ContextPtr login_context) const
670-
{
671-
if (notified_session_log_about_login)
672-
return;
673-
674-
if (!login_context)
675-
throw Exception(ErrorCodes::LOGICAL_ERROR, "Session or query context must be created");
676-
677-
if (auto session_log = getSessionLog())
658+
if (!notified_session_log_about_login)
678659
{
679-
const auto & settings = login_context->getSettingsRef();
680-
const auto access = login_context->getAccess();
681-
682-
session_log->addLoginSuccess(auth_id,
683-
named_session ? named_session->key.second : "",
684-
settings,
685-
access,
686-
getClientInfo(),
687-
user);
660+
if (auto session_log = getSessionLog())
661+
{
662+
session_log->addLoginSuccess(
663+
auth_id,
664+
named_session ? std::optional<std::string>(named_session->key.second) : std::nullopt,
665+
*query_context,
666+
user);
688667

689-
notified_session_log_about_login = true;
668+
notified_session_log_about_login = true;
669+
}
690670
}
671+
672+
return query_context;
691673
}
692674

693675

‎src/Interpreters/Session.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,6 @@ class Session
9797
private:
9898
std::shared_ptr<SessionLog> getSessionLog() const;
9999
ContextMutablePtr makeQueryContextImpl(const ClientInfo * client_info_to_copy, ClientInfo * client_info_to_move) const;
100-
void recordLoginSucess(ContextPtr login_context) const;
101-
102100

103101
mutable bool notified_session_log_about_login = false;
104102
const UUID auth_id;

‎src/Interpreters/SessionLog.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,12 @@ void SessionLogElement::appendToBlock(MutableColumns & columns) const
199199
columns[i++]->insertData(auth_failure_reason.data(), auth_failure_reason.length());
200200
}
201201

202-
void SessionLog::addLoginSuccess(const UUID & auth_id,
203-
const String & session_id,
204-
const Settings & settings,
205-
const ContextAccessPtr & access,
206-
const ClientInfo & client_info,
207-
const UserPtr & login_user)
202+
void SessionLog::addLoginSuccess(const UUID & auth_id, std::optional<String> session_id, const Context & login_context, const UserPtr & login_user)
208203
{
204+
const auto access = login_context.getAccess();
205+
const auto & settings = login_context.getSettingsRef();
206+
const auto & client_info = login_context.getClientInfo();
207+
209208
DB::SessionLogElement log_entry(auth_id, SESSION_LOGIN_SUCCESS);
210209
log_entry.client_info = client_info;
211210

@@ -216,7 +215,8 @@ void SessionLog::addLoginSuccess(const UUID & auth_id,
216215
}
217216
log_entry.external_auth_server = login_user ? login_user->auth_data.getLDAPServerName() : "";
218217

219-
log_entry.session_id = session_id;
218+
if (session_id)
219+
log_entry.session_id = *session_id;
220220

221221
if (const auto roles_info = access->getRolesInfo())
222222
log_entry.roles = roles_info->getCurrentRolesNames();

‎src/Interpreters/SessionLog.h‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ enum SessionLogElementType : int8_t
2020
class ContextAccess;
2121
struct User;
2222
using UserPtr = std::shared_ptr<const User>;
23-
using ContextAccessPtr = std::shared_ptr<const ContextAccess>;
2423

2524
/** A struct which will be inserted as row into session_log table.
2625
*
@@ -73,13 +72,7 @@ class SessionLog : public SystemLog<SessionLogElement>
7372
using SystemLog<SessionLogElement>::SystemLog;
7473

7574
public:
76-
void addLoginSuccess(const UUID & auth_id,
77-
const String & session_id,
78-
const Settings & settings,
79-
const ContextAccessPtr & access,
80-
const ClientInfo & client_info,
81-
const UserPtr & login_user);
82-
75+
void addLoginSuccess(const UUID & auth_id, std::optional<String> session_id, const Context & login_context, const UserPtr & login_user);
8376
void addLoginFailure(const UUID & auth_id, const ClientInfo & info, const std::optional<String> & user, const Exception & reason);
8477
void addLogOut(const UUID & auth_id, const UserPtr & login_user, const ClientInfo & client_info);
8578
};

‎src/Server/HTTPHandler.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,8 @@ void HTTPHandler::processQuery(
561561
session->makeSessionContext();
562562
}
563563

564-
auto context = session->makeQueryContext();
564+
auto client_info = session->getClientInfo();
565+
auto context = session->makeQueryContext(std::move(client_info));
565566

566567
/// This parameter is used to tune the behavior of output formats (such as Native) for compatibility.
567568
if (params.has("client_protocol_version"))

‎tests/integration/test_profile_max_sessions_for_user/test.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@
2727
gen_dir = os.path.join(SCRIPT_DIR, "./_gen")
2828
os.makedirs(gen_dir, exist_ok=True)
2929
run_and_check(
30-
f"python3 -m grpc_tools.protoc -I{proto_dir} --python_out={gen_dir} --grpc_python_out={gen_dir} {proto_dir}/clickhouse_grpc.proto",
30+
"python3 -m grpc_tools.protoc -I{proto_dir} --python_out={gen_dir} --grpc_python_out={gen_dir} \
31+
{proto_dir}/clickhouse_grpc.proto".format(
32+
proto_dir=proto_dir, gen_dir=gen_dir
33+
),
3134
shell=True,
3235
)
3336

‎tests/integration/test_session_log/.gitignore‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎tests/integration/test_session_log/__init__.py‎

Whitespace-only changes.

‎tests/integration/test_session_log/configs/log.xml‎

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎tests/integration/test_session_log/configs/ports.xml‎

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)