diff --git a/src/CurlUtil.cc b/src/CurlUtil.cc index 2b6b2fc..0aa57df 100644 --- a/src/CurlUtil.cc +++ b/src/CurlUtil.cc @@ -201,6 +201,13 @@ void CurlWorker::Run() { std::vector waitfds; waitfds.resize(2); + // The `curl_multi_wait` call in the event loop needs to be interrupted when + // additional work comes into one of the two queues (either the global queue + // or the per-worker unpause queue). To do this, the queue objects will write + // to a file descriptor when a new HTTP request is ready; we add these FDs to + // the list of FDs for libcurl to poll in order to trigger a wakeup. The + // `Consume`/`TryConsume` methods will have a side-effect of reading from the + // pipe if a request is available. waitfds[0].fd = queue.PollFD(); waitfds[0].events = CURL_WAIT_POLLIN; waitfds[0].revents = 0; @@ -237,9 +244,9 @@ void CurlWorker::Run() { } } catch (...) { m_logger.Log(LogMask::Debug, "Run", - "Unable to setup the curl handle"); + "Unable to set up the curl handle"); op->Fail("E_NOMEM", - "Failed to setup the curl handle for the operation"); + "Failed to set up the curl handle for the operation"); continue; } m_op_map[curl] = op; diff --git a/test/s3_tests.cc b/test/s3_tests.cc index 099b334..c421fef 100644 --- a/test/s3_tests.cc +++ b/test/s3_tests.cc @@ -16,13 +16,12 @@ * ***************************************************************/ +#include "s3_tests_common.hh" #include "../src/S3Commands.hh" #include "../src/S3FileSystem.hh" -#include "../src/shortfile.hh" #include #include -#include #include class TestAmazonRequest : public AmazonRequest { @@ -65,41 +64,6 @@ TEST(TestS3URLGeneration, Test1) { ASSERT_EQ(generatedHostUrl, "https://s3-service.com:443/test-object"); } -class FileSystemFixtureBase : public testing::Test { - protected: - FileSystemFixtureBase() - : m_log(new XrdSysLogger(2, 0)) // Log to stderr, no log rotation - {} - - void SetUp() override { - setenv("XRDINSTANCE", "xrootd", 1); - char tmp_configfn[] = "/tmp/xrootd-s3-gtest.cfg.XXXXXX"; - auto result = mkstemp(tmp_configfn); - ASSERT_NE(result, -1) << "Failed to create temp file (" - << strerror(errno) << ", errno=" << errno << ")"; - m_configfn = std::string(tmp_configfn); - - auto contents = GetConfig(); - ASSERT_FALSE(contents.empty()); - ASSERT_TRUE(writeShortFile(m_configfn, contents, 0)) - << "Failed to write to temp file (" << strerror(errno) - << ", errno=" << errno << ")"; - } - - void TearDown() override { - if (!m_configfn.empty()) { - auto rv = unlink(m_configfn.c_str()); - ASSERT_EQ(rv, 0) << "Failed to delete temp file (" - << strerror(errno) << ", errno=" << errno << ")"; - } - } - - virtual std::string GetConfig() = 0; - - std::string m_configfn; - std::unique_ptr m_log; -}; - class FileSystemS3VirtualBucket : public FileSystemFixtureBase { protected: virtual std::string GetConfig() override { diff --git a/test/s3_tests_common.hh b/test/s3_tests_common.hh new file mode 100644 index 0000000..d5e797c --- /dev/null +++ b/test/s3_tests_common.hh @@ -0,0 +1,64 @@ +/*************************************************************** + * + * Copyright (C) 2024, Pelican Project, Morgridge Institute for Research + * + * Licensed under the Apache License, Version 2.0 (the "License"); you + * may not use this file except in compliance with the License. You may + * obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ***************************************************************/ + +#include "../src/shortfile.hh" + +#include +#include + +#include +#include + +#include +#include +#include + +class FileSystemFixtureBase : public testing::Test { + protected: + FileSystemFixtureBase() + : m_log(new XrdSysLogger(2, 0)) // Log to stderr, no log rotation + {} + + void SetUp() override { + setenv("XRDINSTANCE", "xrootd", 1); + char tmp_configfn[] = "/tmp/xrootd-s3-gtest.cfg.XXXXXX"; + auto result = mkstemp(tmp_configfn); + ASSERT_NE(result, -1) << "Failed to create temp file (" + << strerror(errno) << ", errno=" << errno << ")"; + m_configfn = std::string(tmp_configfn); + + auto contents = GetConfig(); + ASSERT_FALSE(contents.empty()); + ASSERT_TRUE(writeShortFile(m_configfn, contents, 0)) + << "Failed to write to temp file (" << strerror(errno) + << ", errno=" << errno << ")"; + } + + void TearDown() override { + if (!m_configfn.empty()) { + auto rv = unlink(m_configfn.c_str()); + ASSERT_EQ(rv, 0) << "Failed to delete temp file (" + << strerror(errno) << ", errno=" << errno << ")"; + } + } + + virtual std::string GetConfig() = 0; + + std::string m_configfn; + std::unique_ptr m_log; +}; diff --git a/test/s3_unit_tests.cc b/test/s3_unit_tests.cc index ca37714..15984ae 100644 --- a/test/s3_unit_tests.cc +++ b/test/s3_unit_tests.cc @@ -21,14 +21,13 @@ // meaning no internet connectivity is needed to run them. // +#include "s3_tests_common.hh" #include "../src/S3Commands.hh" #include "../src/S3File.hh" #include "../src/S3FileSystem.hh" -#include "../src/shortfile.hh" #include #include -#include #include #include @@ -71,186 +70,6 @@ void parseEnvFile(const std::string &fname) { } } -class TestAmazonRequest : public AmazonRequest { - public: - XrdSysLogger log{}; - XrdSysError err{&log, "TestS3CommandsLog"}; - - TestAmazonRequest(const std::string &url, const std::string &akf, - const std::string &skf, const std::string &bucket, - const std::string &object, const std::string &path, - int sigVersion) - : AmazonRequest(url, akf, skf, bucket, object, path, sigVersion, err) {} - - // For getting access to otherwise-protected members - std::string getHostUrl() const { return hostUrl; } -}; - -TEST(TestS3URLGeneration, Test1) { - const std::string serviceUrl = "https://s3-service.com:443"; - const std::string b = "test-bucket"; - const std::string o = "test-object"; - - // Test path-style URL generation - TestAmazonRequest pathReq{serviceUrl, "akf", "skf", b, o, "path", 4}; - std::string generatedHostUrl = pathReq.getHostUrl(); - ASSERT_EQ(generatedHostUrl, - "https://s3-service.com:443/test-bucket/test-object"); - - // Test virtual-style URL generation - TestAmazonRequest virtReq{serviceUrl, "akf", "skf", b, o, "virtual", 4}; - generatedHostUrl = virtReq.getHostUrl(); - ASSERT_EQ(generatedHostUrl, - "https://test-bucket.s3-service.com:443/test-object"); - - // Test path-style with empty bucket (which we use for exporting an entire - // endpoint) - TestAmazonRequest pathReqNoBucket{serviceUrl, "akf", "skf", "", - o, "path", 4}; - generatedHostUrl = pathReqNoBucket.getHostUrl(); - ASSERT_EQ(generatedHostUrl, "https://s3-service.com:443/test-object"); -} - -class FileSystemFixtureBase : public testing::Test { - protected: - FileSystemFixtureBase() - : m_log(new XrdSysLogger(2, 0)) // Log to stderr, no log rotation - {} - - void SetUp() override { - - setenv("XRDINSTANCE", "xrootd", 1); - char tmp_configfn[] = "/tmp/xrootd-s3-gtest.cfg.XXXXXX"; - auto result = mkstemp(tmp_configfn); - ASSERT_NE(result, -1) << "Failed to create temp file (" - << strerror(errno) << ", errno=" << errno << ")"; - m_configfn = std::string(tmp_configfn); - - auto contents = GetConfig(); - ASSERT_FALSE(contents.empty()); - ASSERT_TRUE(writeShortFile(m_configfn, contents, 0)) - << "Failed to write to temp file (" << strerror(errno) - << ", errno=" << errno << ")"; - } - - void TearDown() override { - if (!m_configfn.empty()) { - auto rv = unlink(m_configfn.c_str()); - ASSERT_EQ(rv, 0) << "Failed to delete temp file (" - << strerror(errno) << ", errno=" << errno << ")"; - } - } - - virtual std::string GetConfig() = 0; - - std::string m_configfn; - std::unique_ptr m_log; -}; - -class FileSystemS3VirtualBucket : public FileSystemFixtureBase { - protected: - virtual std::string GetConfig() override { - return R"( -s3.begin -s3.path_name /test -s3.bucket_name genome-browser -s3.service_name s3.amazonaws.com -s3.region us-east-1 -s3.service_url https://s3.us-east-1.amazonaws.com -s3.url_style virtual -s3.end -)"; - } -}; - -class FileSystemS3VirtualNoBucket : public FileSystemFixtureBase { - protected: - virtual std::string GetConfig() override { - return R"( -s3.begin -s3.path_name /test -s3.service_name s3.amazonaws.com -s3.region us-east-1 -s3.service_url https://s3.us-east-1.amazonaws.com -s3.url_style virtual -s3.end -)"; - } -}; - -class FileSystemS3PathBucket : public FileSystemFixtureBase { - protected: - virtual std::string GetConfig() override { - return R"( -s3.begin -s3.path_name /test -s3.service_name s3.amazonaws.com -s3.region us-east-1 -s3.bucket_name genome-browser -s3.service_url https://s3.us-east-1.amazonaws.com -s3.url_style path -s3.end -)"; - } -}; - -class FileSystemS3PathNoBucket : public FileSystemFixtureBase { - protected: - virtual std::string GetConfig() override { - return R"( -s3.begin -s3.path_name /test -s3.service_name s3.amazonaws.com -s3.region us-east-1 -s3.service_url https://s3.us-east-1.amazonaws.com -s3.url_style path -s3.end -)"; - } -}; - -// Regression test for when the service_url ends in a `/` -class FileSystemS3PathBucketSlash : public FileSystemFixtureBase { - protected: - virtual std::string GetConfig() override { - return R"( -s3.begin -s3.path_name /test -s3.service_name s3.amazonaws.com -s3.region us-east-1 -s3.bucket_name genome-browser -s3.service_url https://s3.us-east-1.amazonaws.com/ -s3.url_style path -s3.end -)"; - } -}; - -TEST_F(FileSystemS3VirtualBucket, Create) { - EXPECT_NO_THROW( - { S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr); }); -} - -TEST_F(FileSystemS3VirtualNoBucket, Create) { - EXPECT_NO_THROW( - { S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr); }); -} - -TEST_F(FileSystemS3PathBucket, Create) { - EXPECT_NO_THROW( - { S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr); }); -} - -TEST_F(FileSystemS3PathNoBucket, Create) { - EXPECT_NO_THROW( - { S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr); }); -} - -TEST_F(FileSystemS3PathBucketSlash, Create) { - EXPECT_NO_THROW( - { S3FileSystem fs(m_log.get(), m_configfn.c_str(), nullptr); }); -} - // Tests where we query S3 test fixture class FileSystemS3Fixture : public FileSystemFixtureBase { void SetUp() override {