[#175,#203] Fix read-only objects failing to update access_time (4-3-stable) #301
Conversation
This commit adds a simple iRODS client which uses the rcDataObj* APIs to open, read, and close a data object, and print the contents read from the data object to stdout. This program is only meant to be used by one test in storage tiering in order to exercise the rcDataObjClose endpoint and see whether the irods::access_time AVU is updated on the target data object.
This commit adds some tests which access a read-only data object in a variety of ways and ensures that the irods::access_time metadata is updated appropriately. Only the rcDataObjOpen/Read/Close test is active right now because the other ways of accessing the data object do not have implemented PEPs yet.
This commit updates the access_time updating logic to use a rodsadmin connection instead of the clientUser in the rei->rsComm. This ensures that the access_time is updated even if the user executing the API which caused the access_time to be updated does not have permission to update metadata on the data object. Updating the access_time metadata on the data object should be considered a "system" operation, so this is not a violation of permissions.
| std::cout << "Usage: irods_test_read_object [OPTION] ... [DATA_OBJECT_PATH]\n"; | ||
| std::cout << od << "\n"; |
There was a problem hiding this comment.
I think printing the po::options_description requires a stringstream to work properly (feeding od to fmt::print produced much compilation anger). Happy to hear suggestions on how to get around this. Given the nature of this thing, I didn't feel it was worth the effort.
There was a problem hiding this comment.
Given this is for testing, what you have is fine.
There was a problem hiding this comment.
I guess if you're curious, fmt docs look like they want you to do something like #include <fmt/ostream.h>? Based on https://fmt.dev/11.1/api/ based on section std::ostream Support
| @@ -80,6 +80,8 @@ set(IRODS_PLUGIN_TARGET_NAME "${IRODS_TARGET_NAME_PREFIX}-${IRODS_POLICY_NAME}") | |||
| string(REPLACE "_" "-" IRODS_POLICY_NAME_HYPHENS "${IRODS_POLICY_NAME}") | |||
| set(IRODS_POLICY_PACKAGE_COMPONENT "${IRODS_POLICY_NAME_HYPHENS}") | |||
|
|
|||
| add_subdirectory(test/stream_test) | |||
There was a problem hiding this comment.
Perhaps this should require a cmake option to include this step. That way we don't include it in the release build.
I'm mostly thinking about how the binary doesn't have a limit on the size of the read buffer as well as not exposing a tool which a malicious user could gain access to.
There was a problem hiding this comment.
Good point. I'll look into adding an option a la IRODS_UNIT_TESTS_BUILD.
There was a problem hiding this comment.
Noting here that we should probably add the build option to the documentation somewhere, as well...
| std::cout << "Usage: irods_test_read_object [OPTION] ... [DATA_OBJECT_PATH]\n"; | ||
| std::cout << od << "\n"; |
There was a problem hiding this comment.
I guess if you're curious, fmt docs look like they want you to do something like #include <fmt/ostream.h>? Based on https://fmt.dev/11.1/api/ based on section std::ostream Support
Addresses #175
Addresses #203
I imagine much of this review will be focused on the new executable and its packaging, so please eyeball that part carefully. I modeled the CMake after the
irodsTestPutGetexecutable. Subdirectory, filename, and option names all open for debate.Test suite passed.