[irods#7443] Make copy_data_object() honor copy_options::none correctly.

This commit makes it so that use of copy_options::none does not result
in an exception being thrown.

A unit test is included to make sure all values of the copy_options
enumeration work as intended.

lib/filesystem/include/irods/filesystem/copy_options.hpp

@@ -6,15 +6,17 @@
 
 namespace irods::experimental::filesystem
 {
-    enum class copy_options
-    {
-        none                    = 0,
-        skip_existing           = 1 << 0,
-        overwrite_existing      = 1 << 1,
-        update_existing         = 1 << 2,
-        recursive               = 1 << 3,
-        collections_only        = 1 << 4,
-        in_recursive_copy       = 1 << 5
+    enum class copy_options : std::uint32_t
+    {
+        // clang-format off
+        none               = 0U,
+        skip_existing      = 1U,
+        overwrite_existing = 2U,
+        update_existing    = 4U,
+        recursive          = 8U,
+        collections_only   = 16U,
+        in_recursive_copy  = 32U
+        // clang-format on
     };
 
     inline auto operator|=(copy_options& _lhs, copy_options _rhs) noexcept -> copy_options&

lib/filesystem/src/filesystem.cpp

@@ -574,28 +574,24 @@ namespace irods::experimental::filesystem::NAMESPACE_IMPL
                 throw filesystem_error{"path does not point to a data object", _to, make_error_code(INVALID_OBJECT_TYPE)};
             }
 
-            if (copy_options::none == _options) {
-                throw filesystem_error{"copy options not set", make_error_code(SYS_INVALID_INPUT_PARAM)};
-            }
-
             if (copy_options::skip_existing == _options) {
                 return false;
             }
 
             if (copy_options::overwrite_existing == _options) {
                 addKeyVal(&input.destDataObjInp.condInput, FORCE_FLAG_KW, "");
             }
-
-            if (copy_options::update_existing == _options) {
+            else if (copy_options::update_existing == _options) {
                 if (last_write_time(_comm, _from) <= last_write_time(_comm, _to)) {
                     return false;
                 }
+
+                addKeyVal(&input.destDataObjInp.condInput, FORCE_FLAG_KW, "");
             }
         }
 
         std::strncpy(input.srcDataObjInp.objPath, _from.c_str(), std::strlen(_from.c_str()));
         std::strncpy(input.destDataObjInp.objPath, _to.c_str(), std::strlen(_to.c_str()));
-        addKeyVal(&input.destDataObjInp.condInput, DEST_RESC_NAME_KW, "");
 
         if (const auto ec = rxDataObjCopy(&_comm, &input); ec < 0) {
             throw filesystem_error{"cannot copy data object", _from, _to, make_error_code(ec)};

unit_tests/src/filesystem/test_filesystem.cpp

@@ -66,6 +66,7 @@ TEST_CASE("filesystem")
     namespace fs = irods::experimental::filesystem;
 
     // clang-format off
+    using idstream          = irods::experimental::io::idstream;
     using odstream          = irods::experimental::io::odstream;
     using default_transport = irods::experimental::io::client::default_transport;
     // clang-format on
@@ -769,4 +770,74 @@ TEST_CASE("filesystem")
         CHECK(err.code().message() == "USER_FILE_DOES_NOT_EXIST");
         CHECK(err.what() == msg + ": USER_FILE_DOES_NOT_EXIST");
     }
+
+    SECTION("copy_data_object")
+    {
+        constexpr std::string_view contents = "testing copy_data_object";
+        const auto data_object = sandbox / "data_object.txt";
+
+        // Create a non-empty data object.
+        {
+            default_transport tp{conn};
+            odstream{tp, data_object} << contents;
+        }
+
+        REQUIRE(fs::client::is_data_object(conn, data_object));
+
+        // Create a copy of the data object using copy_options::none.
+        // The assertion that follows specifically proves #7443 is resolved.
+        const auto new_data_object = sandbox / "data_object.txt.copy";
+        REQUIRE(fs::client::copy_data_object(conn, data_object, new_data_object, fs::copy_options::none));
+
+        // Show that attempting to copy over an existing data object using copy_options::none
+        // will result in an exception being thrown.
+        const auto matcher = Catch::Matchers::Message("cannot copy data object: OVERWRITE_WITHOUT_FORCE_FLAG");
+        CHECK_THROWS_MATCHES(fs::client::copy_data_object(conn, data_object, new_data_object, fs::copy_options::none),
+                             fs::filesystem_error,
+                             matcher);
+
+        // Show that the results are the same when the default copy_options are passed.
+        // The default is copy_options::none.
+        CHECK_THROWS_MATCHES(
+            fs::client::copy_data_object(conn, data_object, new_data_object), fs::filesystem_error, matcher);
+
+        // Show the new data object contains the data.
+        {
+            default_transport tp{conn};
+            idstream in{tp, new_data_object};
+
+            std::string line;
+            CHECK(std::getline(in, line));
+            CHECK(contents == line);
+        }
+
+        // Capture the mtime of the new data object.
+        // This will be used to assert correctness of copy_options::skip_existing.
+        auto old_mtime = fs::client::last_write_time(conn, new_data_object);
+
+        // Guarantee a difference in the mtime is noticeable if a change occurs.
+        using namespace std::chrono_literals;
+        std::this_thread::sleep_for(2s);
+
+        // Show copy_options::skip_existing works as intended.
+        // That is, if the target data object already exists, do nothing.
+        CHECK_FALSE(fs::client::copy_data_object(conn, data_object, new_data_object, fs::copy_options::skip_existing));
+        CHECK(old_mtime == fs::client::last_write_time(conn, new_data_object));
+
+        // Show that copy_options::overwrite_existing works as intended.
+        // That is, overwrite the contents of the target data object.
+        // This will result in the mtime of the target data object being updated.
+        CHECK(fs::client::copy_data_object(conn, data_object, new_data_object, fs::copy_options::overwrite_existing));
+        CHECK(old_mtime < fs::client::last_write_time(conn, new_data_object));
+
+        // Capture the mtime of the very first data object.
+        // This will be used to assert correctness of copy_options::update_existing.
+        old_mtime = fs::client::last_write_time(conn, data_object);
+
+        // Show that copy_options::update_existing works as intended.
+        // That is, only overwrite the contents of the target data object if the source
+        // data object is newer than the target data object, as defined by last_write_time().
+        CHECK(fs::client::copy_data_object(conn, new_data_object, data_object, fs::copy_options::update_existing));
+        CHECK(old_mtime < fs::client::last_write_time(conn, data_object));
+    }
 }