Skip to content

Commit

Permalink
[irods#7570] Complete port of GenQuery2 parser.
Browse files Browse the repository at this point in the history
  • Loading branch information
korydraughn committed Apr 8, 2024
1 parent d2eb5f0 commit fb49dcf
Show file tree
Hide file tree
Showing 19 changed files with 530 additions and 222 deletions.
13 changes: 6 additions & 7 deletions lib/api/include/irods/genquery2.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ struct RcComm;
/// \warning This data structure is part of an experimental API and may change in the future.
///
/// \since 4.3.2
typedef struct Genquery2Input // NOLINT(modernize-use-using)
typedef struct Genquery2Input
{
/// The GenQuery2 query string to execute.
///
/// This member variable is allowed to be set to NULL when
/// This member variable is allowed to be set to NULL.
///
/// \since 4.3.2
char* query_string;
Expand Down Expand Up @@ -46,7 +46,6 @@ typedef struct Genquery2Input // NOLINT(modernize-use-using)
int column_mappings;
} genquery2Inp_t;

// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
#define Genquery2Input_PI "str *query_string; str *zone; int sql_only; int column_mappings;"

#ifdef __cplusplus
Expand Down Expand Up @@ -75,7 +74,7 @@ extern "C" {
///
/// Limitations:
/// - Groups are not yet fully supported
/// - Cannot resolve tickets to data objects and collections using a single query
/// - Tickets are not yet supported.
/// - Integer values must be treated as strings, except when used for OFFSET, LIMIT, FETCH FIRST \a N ROWS ONLY
///
/// When the query does not include the FETCH FIRST \a N ROWS ONLY or LIMIT clause, the API will clamp the
Expand All @@ -87,9 +86,9 @@ extern "C" {
/// The column mappings between GenQuery2 and the catalog can be obtained via the API. See the Genquery2Input
/// structure for details.
///
/// \param[in] _comm A pointer to a RcComm.
/// \param[in] _input A pointer to a Genquery2Input.
/// \param[in,out] _output \parblock A pointer that will hold the results of the operation.
/// \param[in] _comm A pointer to a RcComm.
/// \param[in] _input A pointer to a Genquery2Input.
/// \param[out] _output \parblock A pointer that will hold the results of the operation.
/// On success, the pointer will either hold a JSON string or a string representing the SQL derived from
/// the GenQuery2 query string. See Genquery2Input::sql_only for details.
///
Expand Down
4 changes: 2 additions & 2 deletions plugins/database/src/db_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15833,7 +15833,7 @@ auto db_execute_genquery2_sql(irods::plugin_context& _ctx,

if (!_sql || !_values || !_output) {
log_db::error("{}: Received one or more null pointers.", __func__);
return ERROR(SYS_INVALID_INPUT_PARAM, "Received one or more null pointers.");
return ERROR(SYS_INTERNAL_NULL_INPUT_ERR, "Received one or more null pointers.");
}

*_output = nullptr;
Expand Down Expand Up @@ -15871,7 +15871,7 @@ auto db_execute_genquery2_sql(irods::plugin_context& _ctx,
}
catch (const irods::exception& e) {
log_db::error("{}: {}", __func__, e.client_display_what());
return ERROR(SYS_LIBRARY_ERROR, e.what());
return ERROR(e.code(), e.what());
}
catch (const std::exception& e) {
log_db::error("{}: {}", __func__, e.what());
Expand Down
170 changes: 170 additions & 0 deletions scripts/irods/test/test_dynamic_peps.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from .. import paths
from .. import test
from ..configuration import IrodsConfig
from ..controller import IrodsController
from ..core_file import temporary_core_file
from textwrap import dedent

Expand Down Expand Up @@ -322,3 +323,172 @@ def test_pep_database_data_object_finalize__issue_6385(self):

self.user.run_icommand(['irm', '-f', logical_path])
lib.remove_resource(self.admin, other_resource)

@unittest.skipIf(plugin_name != 'irods_rule_engine_plugin-irods_rule_language' or test.settings.RUN_IN_TOPOLOGY, "Requires NREP and single node zone.")
def test_if_const_vector_of_strings_is_exposed__issue_7570(self):
with temporary_core_file() as core:
prefix = 'test_if_const_vector_of_strings_is_exposed__issue_7570'
attr_name_1 = f'{prefix}_iquery_input_value'
attr_value_1 = f'{prefix}_vector_of_strings'
attr_name_2 = f'{prefix}_vector_of_strings_size'

# Add a rule to core.re which when triggered will add the vector size and elements
# of "*values" to the session collection as AVUs.
core.add_rule(dedent('''
pep_database_execute_genquery2_sql_pre(*inst, *ctx, *out, *sql, *values, *output) {{
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{attr_name_1}', *values."0", '');
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{attr_name_2}', *values."size", '');
}}
'''.format(**locals())))

try:
# Trigger PEP, ignore output.
self.user.assert_icommand(['iquery', f"select COLL_NAME where COLL_NAME = '{attr_value_1}'"], 'STDOUT')

# Show the session collection has an AVU attached to it.
# This is only possible if vectors of strings are serializable.
self.user.assert_icommand(['imeta', 'ls', '-C', self.user.session_collection], 'STDOUT', [
f'attribute: {attr_name_1}\nvalue: {attr_value_1}\n',
# The value is 2 due to the parser adding additional bind variables.
# For this specific case, a second bind variable for the username is included.
f'attribute: {attr_name_2}\nvalue: 2\n'
])

finally:
# This isn't required, but we do it here to keep the database metadata
# tables small, for performance reasons.
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, attr_name_1, attr_value_1])
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, attr_name_2, '2'])

@unittest.skipIf(plugin_name != 'irods_rule_engine_plugin-irods_rule_language' or test.settings.RUN_IN_TOPOLOGY, 'Requires NREP and single node zone.')
def test_if_vector_of_strings_is_exposed__issue_7570(self):
config = IrodsConfig()

with lib.file_backed_up(config.server_config_path):
try:
# Lower the delay server's sleep time so that rule are executed quicker.
config.server_config['advanced_settings']['delay_server_sleep_time_in_seconds'] = 1
lib.update_json_file_from_dict(config.server_config_path, config.server_config)

# Restart so the new server configuration takes effect.
IrodsController().restart(test_mode=True)

with temporary_core_file() as core:
prefix = 'test_if_vector_of_strings_is_exposed__issue_7570'
attr_name_1 = f'{prefix}_delay_rule_info_element_value'
attr_name_2 = f'{prefix}_vector_of_strings_size'

# Add a rule to core.re which when triggered will add the vector size and elements
# of "*delay_rule_info" to "self.admin's" session collection as AVUs.
core.add_rule(dedent('''
pep_database_get_delay_rule_info_post(*inst, *ctx, *out, *rule_id, *delay_rule_info) {{
*trimmed_value = trimr(triml(*delay_rule_info."0", ' '), ' '); # Remove leading/trailing whitespace.
msiModAVUMetadata('-C', '{self.admin.session_collection}', 'add', '{attr_name_1}', *trimmed_value, '');
msiModAVUMetadata('-C', '{self.admin.session_collection}', 'add', '{attr_name_2}', *delay_rule_info."size", '');
}}
'''.format(**locals())))

# Give the service account user permission to add metadata to "self.admin's" session collection.
# Remember, "self.admin" represents a completely different rodsadmin AND the PEP that was added
# to core.re is only triggered by the service account user.
self.admin.assert_icommand(
['ichmod', 'modify_object', config.client_environment['irods_user_name'], self.admin.session_collection])

# Placed here for clean-up purposes.
rep_name = self.plugin_name + '-instance'
delay_rule_body = 'writeLine("serverLog", "test_if_vector_of_strings_is_exposed__issue_7570");'
rule = f'delay("<INST_NAME>{rep_name}</INST_NAME>") {{ {delay_rule_body} }}'

try:
# Schedule a delay rule to trigger the database PEP.
self.admin.assert_icommand(['irule', '-r', rep_name, rule, 'null', 'null'])

# Show the session collection has an AVU attached to it.
# This is only possible if vectors of strings are serializable.

def has_avu():
out, _, _ = self.admin.run_icommand(['imeta', 'ls', '-C', self.admin.session_collection])
print(out) # For debugging purposes.
return all(avu in out for avu in [
f'attribute: {attr_name_1}\nvalue: {delay_rule_body}\n',
f'attribute: {attr_name_2}\nvalue: 12\n'
])

lib.delayAssert(lambda: has_avu())

finally:
# This isn't required, but we do it here to keep the database metadata
# tables small, for performance reasons.
self.admin.run_icommand(['imeta', 'rm', '-C', self.admin.session_collection, attr_name_1, delay_rule_body])
self.admin.run_icommand(['imeta', 'rm', '-C', self.admin.session_collection, attr_name_2, '12'])

finally:
# Restart so the server's original configuration takes effect.
IrodsController().restart(test_mode=True)

@unittest.skipIf(plugin_name != 'irods_rule_engine_plugin-irods_rule_language' or test.settings.RUN_IN_TOPOLOGY, "Requires NREP and single node zone.")
def test_if_Genquery2Input_is_exposed__issue_7570(self):
with temporary_core_file() as core:
avu_prefix = 'test_if_Genquery2Input_is_exposed__issue_7570'

# Add a rule to core.re which when triggered will add AVUs to the user's
# session collection.
core.add_rule(dedent('''
pep_api_genquery2_pre(*inst, *comm, *input, *output) {{
*v = *input.query_string;
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{avu_prefix}_0', *v, '');
*v = *input.zone;
if (strlen(*v) > 0) {{
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{avu_prefix}_1', *v, '');
}}
else {{
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{avu_prefix}_1', 'unspecified', '');
}}
*v = *input.sql_only;
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{avu_prefix}_2', *v, '');
*v = *input.column_mappings;
msiModAVUMetadata('-C', '{self.user.session_collection}', 'add', '{avu_prefix}_3', *v, '');
}}
'''.format(**locals())))

try:
# Trigger PEP, ignore output.
query_string = f"select COLL_NAME where COLL_NAME = '{self.user.session_collection}'"
self.user.assert_icommand(['iquery', query_string], 'STDOUT')

# Show the session collection has the expected AVUs attached to it.
self.user.assert_icommand(['imeta', 'ls', '-C', self.user.session_collection], 'STDOUT', [
f'attribute: {avu_prefix}_0\nvalue: {query_string}\n',
f'attribute: {avu_prefix}_1\nvalue: unspecified\n',
f'attribute: {avu_prefix}_2\nvalue: 0\n',
f'attribute: {avu_prefix}_3\nvalue: 0\n',
])

# Remove the AVUs to avoid duplicate AVU errors. These will cause the test to fail.
self.user.assert_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_0', query_string])
self.user.assert_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_1', 'unspecified'])
self.user.assert_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_2', '0'])
self.user.assert_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_3', '0'])

# Now, with the zone explicitly passed.
self.user.assert_icommand(['iquery', '-z', self.user.zone_name, query_string], 'STDOUT')

# Show the session collection has the expected AVUs attached to it.
self.user.assert_icommand(['imeta', 'ls', '-C', self.user.session_collection], 'STDOUT', [
f'attribute: {avu_prefix}_0\nvalue: {query_string}\n',
f'attribute: {avu_prefix}_1\nvalue: {self.user.zone_name}\n',
f'attribute: {avu_prefix}_2\nvalue: 0\n',
f'attribute: {avu_prefix}_3\nvalue: 0\n',
])

finally:
# This isn't required, but we do it here to keep the database metadata
# tables small, for performance reasons.
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_0', query_string])
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_1', 'unspecified'])
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_1', self.user.zone_name])
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_2', '0'])
self.user.run_icommand(['imeta', 'rm', '-C', self.user.session_collection, f'{avu_prefix}_3', '0'])
16 changes: 14 additions & 2 deletions scripts/irods/test/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@
SessionsMixin = session.make_sessions_mixin(
test.settings.FEDERATION.RODSADMIN_NAME_PASSWORD_LIST, test.settings.FEDERATION.RODSUSER_NAME_PASSWORD_LIST)

# This test script expects tempZone to contain the following users:
#
# - rods#tempZone
# - zonehopper#tempZone
# - zonehopper#otherZone
#
# otherZone must contain the following user:
#
# - rods#otherZone
#
# The test script must be launched from a server in otherZone. That means tempZone
# is identified as the remote federated zone.

class Test_ICommands(SessionsMixin, unittest.TestCase):

Expand Down Expand Up @@ -2386,8 +2398,8 @@ class Test_GenQuery2_IQuery(SessionsMixin, unittest.TestCase):
# This test suite expects tempZone to contain the following users:
#
# - rods#tempZone
# - zonehopper#tempZone (created by the irods_testing_environment)
# - zonehopper#otherZone (created by the irods_testing_environment)
# - zonehopper#tempZone
# - zonehopper#otherZone
#
# otherZone must contain the following users:
#
Expand Down
36 changes: 16 additions & 20 deletions scripts/irods/test/test_genquery2_microservices.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ def setUp(self):
def tearDown(self):
super(Test_GenQuery2_Microservices, self).tearDown()

@unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', 'Designed for the NREP')
def test_microservices_can_be_used_in_the_native_rule_engine_plugin__issue_7570(self):
rule_file = f'{self.user.local_session_dir}/test_microservices_can_be_used_in_the_native_rule_engine_plugin__issue_7570.nrep.r'

with open(rule_file, 'w') as f:
f.write(textwrap.dedent(f'''
@unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', '#7638: Need access to error code in PREP for msi_genquery2_next_row()')
def test_genquery2_microservices_can_be_used_within_policy__issue_7570(self):
rule_map = {
# NREP rule
'irods_rule_engine_plugin-irods_rule_language': textwrap.dedent(f'''
test_issue_7570_nrep {{
*END_OF_RESULTSET = -408000;
Expand All @@ -51,24 +50,16 @@ def test_microservices_can_be_used_in_the_native_rule_engine_plugin__issue_7570(
INPUT *handle="", *coll_name=""
OUTPUT ruleExecOut
'''))

rep_name = self.plugin_name + '-instance'
self.user.assert_icommand(['irule', '-r', rep_name, '-F', rule_file], 'STDOUT', [self.user.session_collection])

@unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-python', 'Designed for the PREP')
def test_microservices_can_be_used_in_the_python_rule_engine_plugin(self):
rule_file = f'{self.user.local_session_dir}/test_microservices_can_be_used_in_the_python_rule_engine_plugin.prep.r'

with open(rule_file, 'w') as f:
f.write(textwrap.dedent(f'''
'''),
# PREP rule
'irods_rule_engine_plugin-python': textwrap.dedent(f'''
test_issue_7570_prep(rule_args, callback, rei) {{
result = callback.msi_genquery2_execute('', "select COLL_NAME where COLL_NAME = '{self.user.session_collection}'");
handle = result['arguments'][0]
while True:
try:
msi_genquery2_next_row(*handle)
callback.msi_genquery2_next_row(*handle)
except:
break
Expand All @@ -82,13 +73,18 @@ def test_microservices_can_be_used_in_the_python_rule_engine_plugin(self):
INPUT *handle=%*coll_name=
OUTPUT *ruleExecOut
'''))
''')
}

rule_file = f'{self.user.local_session_dir}/test_genquery2_microservices_can_be_used_within_policy__issue_7570.{self.plugin_name}.r'
with open(rule_file, 'w') as f:
f.write(rule_map[self.plugin_name])

rep_name = self.plugin_name + '-instance'
self.user.assert_icommand(['irule', '-r', rep_name, '-F', rule_file], 'STDOUT', [self.user.session_collection])

@unittest.skipUnless(plugin_name == 'irods_rule_engine_plugin-irods_rule_language', 'Designed for the NREP')
def test_microservices_support_interleaved_execution__issue_7570(self):
def test_genquery2_microservices_support_interleaved_execution__issue_7570(self):
# This test is about proving GenQuery2 handles are tracked correctly.
#
# The original GenQuery2 implementation used a std::vector to track handles. This
Expand Down
Loading

0 comments on commit fb49dcf

Please sign in to comment.