Skip to content

Commit 9c2d2a9

Browse files
committed
Post merge fixes for XAPI session option handling logic.
Using iterators to store position of the last tcp/socket in the host list posed problems when settings object was copied. In such scenario iterators in the new copy still referred to the list in the source object which was wrong.
1 parent 80ba8e3 commit 9c2d2a9

File tree

6 files changed

+59
-76
lines changed

6 files changed

+59
-76
lines changed

devapi/session.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ Session::Session(SessionSettings settings)
528528
database = settings.find(SessionOption::DB).get<string>();
529529
}
530530

531-
AuthMethod auth_method;
531+
AuthMethod auth_method = AuthMethod::MYSQL41; // avoid compiler warning
532532
bool has_auth = settings.has_option(SessionOption::AUTH);
533533

534534
if (has_auth)

xapi/mysqlx_cc_internal.h

Lines changed: 6 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ typedef struct mysqlx_session_options_struct
456456
};
457457

458458

459-
typedef cdk::foundation::variant<
459+
typedef cdk::variant<
460460
TCPIP_t
461461
#ifndef _WIN32
462462
,cdk::ds::Unix_socket
@@ -467,44 +467,8 @@ typedef struct mysqlx_session_options_struct
467467
std::multimap<unsigned short,Ds_variant>
468468
Host_list;
469469

470-
struct Add_list
471-
{
472-
Host_list &m_list;
473-
unsigned short priority = 0;
474-
bool socket_only = true;
475-
Host_list::const_iterator &m_last_tcpip;
476-
#ifndef _WIN32
477-
Host_list::const_iterator &m_last_socket;
478-
#endif
479-
480-
Add_list(Host_list &list
481-
, Host_list::const_iterator &last_tcpip
482-
#ifndef _WIN32
483-
, Host_list::const_iterator &last_socket
484-
#endif
485-
)
486-
: m_list(list)
487-
, m_last_tcpip(last_tcpip)
488-
#ifndef _WIN32
489-
, m_last_socket(last_socket)
490-
#endif
491-
{}
492-
493-
494-
void operator() (const TCPIP_t &ds_tcp)
495-
{
496-
m_last_tcpip = m_list.emplace(priority, ds_tcp);
497-
socket_only = false;
498-
}
499-
500-
#ifndef _WIN32
501-
void operator() (const cdk::ds::Unix_socket &ds_socket)
502-
{
503-
m_last_socket = m_list.emplace(std::make_pair(priority, ds_socket));
504-
}
505-
#endif //_WIN32
506-
507-
};
470+
struct Add_list;
471+
friend Add_list;
508472

509473
enum source_state
510474
{ unknown, priority, non_priority }
@@ -515,9 +479,10 @@ typedef struct mysqlx_session_options_struct
515479

516480
Host_sources m_ms;
517481
Host_list m_host_list;
518-
Host_list::const_iterator m_last_tcpip;
482+
unsigned short m_last_prio = 0;
483+
cdk::opt<cdk::ds::TCPIP> m_last_tcpip;
519484
#ifndef _WIN32
520-
Host_list::const_iterator m_last_socket;
485+
cdk::opt<cdk::ds::Unix_socket> m_last_socket;
521486
#endif
522487

523488
bool m_explicit_mode = false;
@@ -528,10 +493,6 @@ typedef struct mysqlx_session_options_struct
528493

529494
mysqlx_session_options_struct(source_state state = source_state::unknown)
530495
: m_source_state(state)
531-
, m_last_tcpip(m_host_list.end())
532-
#ifndef _WIN32
533-
, m_last_socket(m_host_list.end())
534-
#endif
535496
{
536497
#ifdef WITH_SSL
537498
set_ssl_mode(SSL_MODE_REQUIRED);

xapi/session.cc

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const unsigned max_priority = 100;
3333
mysqlx_session_struct::mysqlx_session_struct(
3434
const mysqlx_session_options_t &opt
3535
)
36-
: m_session(mysqlx_session_options_t(opt).get_multi_source()),
36+
: m_session(opt.get_multi_source()),
3737
m_stmt(NULL)
3838
{
3939
const string *db = opt.get_db();
@@ -511,20 +511,43 @@ cdk::ds::mysqlx::Protocol_options::auth_method_t uint_to_auth_method(unsigned in
511511
return cdk::ds::mysqlx::Protocol_options::PLAIN;
512512
}
513513

514+
515+
struct mysqlx_session_options_struct::Add_list
516+
{
517+
mysqlx_session_options_struct *opts = NULL;
518+
unsigned short priority = 0;
519+
bool socket_only = true;
520+
521+
Add_list(mysqlx_session_options_struct &_opts)
522+
: opts(&_opts)
523+
{}
524+
525+
void operator() (const TCPIP_t &ds_tcp)
526+
{
527+
opts->m_last_tcpip = ds_tcp;
528+
opts->m_host_list.emplace(priority, ds_tcp);
529+
socket_only = false;
530+
}
531+
532+
#ifndef _WIN32
533+
void operator() (const cdk::ds::Unix_socket &ds_socket)
534+
{
535+
opts->m_last_socket = ds_socket;
536+
opts->m_host_list.emplace(std::make_pair(priority, ds_socket));
537+
}
538+
#endif //_WIN32
539+
540+
};
541+
542+
514543
void mysqlx_session_options_struct::set_multiple_options(va_list args)
515544
{
516545
mysqlx_opt_type_t type;
517546
enum {TCPIP_type, UNIX_type} selected_type = TCPIP_type;
518547
Ds_variant ds;
519548
m_options_used.reset();
520549

521-
522-
523-
#ifndef _WIN32
524-
Add_list add_list(m_host_list, m_last_tcpip, m_last_socket);
525-
#else
526-
Add_list add_list(m_host_list, m_last_tcpip);
527-
#endif
550+
Add_list add_list(*this);
528551

529552
// The type is promoted to int when passing into va_list
530553
while( (type = (mysqlx_opt_type_t)(va_arg(args, int))) > 0 )
@@ -593,7 +616,8 @@ void mysqlx_session_options_struct::set_multiple_options(va_list args)
593616
if (uint_data > max_priority)
594617
throw Mysqlx_exception(MYSQLX_ERROR_MAX_PRIORITY);
595618

596-
add_list.priority = (unsigned short)uint_data + 1;
619+
m_last_prio = (unsigned short)uint_data + 1;
620+
add_list.priority = m_last_prio;
597621
m_source_state = source_state::priority;
598622
break;
599623
case MYSQLX_OPT_USER:
@@ -745,29 +769,30 @@ mysqlx_session_options_struct::get_multi_source() const
745769

746770
const std::string mysqlx_session_options_struct::get_host()
747771
{
748-
if (m_last_tcpip != m_host_list.end())
749-
return m_last_tcpip->second.get<TCPIP_t>().host();
772+
static TCPIP_t defaults;
750773

751-
return TCPIP_t().host(); // return the default host name
774+
if (m_last_tcpip)
775+
return m_last_tcpip->host();
776+
777+
return defaults.host(); // return the default host name
752778
}
753779

754780
#ifndef _WIN32
755781
const std::string mysqlx_session_options_struct::get_socket()
756782
{
757-
if (m_last_socket == m_host_list.end())
783+
if (!m_last_socket)
758784
throw Mysqlx_exception("No socket defined");
759785

760-
return m_last_socket->second.get<cdk::ds::Unix_socket>().path();
786+
return m_last_socket->path();
761787
}
762788
#endif
763789

764790
unsigned int mysqlx_session_options_struct::get_priority()
765791
{
766-
if (m_source_state != source_state::priority || m_last_tcpip == m_host_list.end())
792+
if (m_source_state != source_state::priority || m_last_prio == 0)
767793
throw Mysqlx_exception("Priority is not available");
768794

769-
unsigned short prio = m_last_tcpip->first;
770-
return prio ? prio - 1 : 0;
795+
return m_last_prio - 1;
771796
}
772797

773798
unsigned int mysqlx_session_options_struct::get_auth_method()
@@ -788,10 +813,10 @@ unsigned int mysqlx_session_options_struct::get_auth_method()
788813

789814
unsigned int mysqlx_session_options_struct::get_port()
790815
{
791-
if (m_last_tcpip == m_host_list.end())
816+
if (!m_last_tcpip)
792817
throw Mysqlx_exception(MYSQLX_ERROR_MISSING_CONN_INFO);
793818

794-
return m_last_tcpip->second.get<TCPIP_t>().port();
819+
return m_last_tcpip->port();
795820
}
796821

797822
const std::string mysqlx_session_options_struct::get_user()
@@ -827,7 +852,8 @@ void mysqlx_session_options_struct::host(unsigned short priority,
827852
if (!port)
828853
throw Mysqlx_exception("Wrong value for port");
829854

830-
m_last_tcpip = m_host_list.emplace(priority, TCPIP_t(host, port));
855+
auto it = m_host_list.emplace(priority, TCPIP_t(host, port));
856+
m_last_tcpip = it->second.get<TCPIP_t>();
831857
}
832858

833859
// Implementing URI_Processor interface
@@ -836,8 +862,8 @@ void mysqlx_session_options_struct::host(unsigned short priority,
836862
{
837863
PRIORITY_CHECK;
838864

839-
m_last_tcpip = m_host_list.emplace(priority, TCPIP_t(host));
840-
865+
auto it = m_host_list.emplace(priority, TCPIP_t(host));
866+
m_last_tcpip = it->second.get<TCPIP_t>();
841867
}
842868

843869
#ifndef _WIN32
@@ -846,8 +872,8 @@ void mysqlx_session_options_struct::socket(unsigned short priority,
846872
{
847873
PRIORITY_CHECK;
848874

849-
m_last_socket =
850-
m_host_list.emplace(priority, cdk::ds::Unix_socket(path));
875+
auto it = m_host_list.emplace(priority, cdk::ds::Unix_socket(path));
876+
m_last_socket = it->second.get<cdk::ds::Unix_socket>();
851877

852878
}
853879
#endif

xapi/tests/test.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ class xapi : public ::testing::Test
289289
{\
290290
std::cerr <<"SKIPPED: " << \
291291
"Server version not supported (" \
292-
<< x << "." << y <<"." << ")" << z <<std::endl; \
292+
<< x << "." << y <<"." << z << ")" <<std::endl; \
293293
return; \
294294
}
295295

xapi/tests/xapi-t.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,6 +1775,7 @@ TEST_F(xapi, myc_344_sql_error_test)
17751775
printf("\nExpected error: %s\n", err_msg);
17761776
}
17771777

1778+
17781779
#ifndef _WIN32
17791780
TEST_F(xapi, unix_socket)
17801781
{

xapi/tests/xapi_crud-t.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,6 @@ TEST_F(xapi, test_having_group_by)
106106
{
107107
SKIP_IF_NO_XPLUGIN;
108108

109-
//TODO: Remove this when Bug #86754 is fixed
110-
SKIP_IF_SERVER_VERSION_LESS(5, 7, 19);
111-
112109
mysqlx_result_t *res;
113110
mysqlx_schema_t *schema;
114111
mysqlx_table_t *table;
@@ -120,8 +117,6 @@ TEST_F(xapi, test_having_group_by)
120117
size_t json_len = 0;
121118

122119
AUTHENTICATE();
123-
//TODO: Remove this when Bug #86754 is fixed
124-
SKIP_IF_SERVER_VERSION_LESS(5, 7, 19);
125120

126121
//TODO: Remove this when Bug #86754 is fixed
127122
SKIP_IF_SERVER_VERSION_LESS(5,7,19);

0 commit comments

Comments
 (0)