diff --git a/.github/workflows/buildThirdPartyLibrary.yml b/.github/workflows/buildThirdPartyLibrary.yml index ec43ecf7c..e0a261c07 100644 --- a/.github/workflows/buildThirdPartyLibrary.yml +++ b/.github/workflows/buildThirdPartyLibrary.yml @@ -63,10 +63,12 @@ jobs: steps: - name: Set the timezone to New Zealand if: ${{ runner.os != 'Linux' }} - uses: szenius/set-timezone@v2.0 - with: - timezoneWindows: 'New Zealand Standard Time' - timezoneMacos: 'Pacific/Auckland' + shell: bash + run: | + case "${RUNNER_OS}" in + Windows) MSYS_NO_PATHCONV=1 tzutil /s "New Zealand Standard Time" ;; + macOS) sudo systemsetup -settimezone Pacific/Auckland ;; + esac - name: Set up the manylinux container if: ${{ runner.os == 'Linux' }} shell: bash @@ -74,16 +76,16 @@ jobs: echo "TZ=Pacific/Auckland" >> "$GITHUB_ENV" dnf install -y git make openssl-libs perl-core - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Install CMake and Ninja uses: lukka/get-cmake@latest - name: Install buildcache - uses: opencor/buildcache-action@v1 + uses: opencor/buildcache-action@v3 with: cache_key: ${{ inputs.third_party_library_name }}-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.build_type }} - name: Configure MSVC if: ${{ runner.os == 'Windows' }} - uses: ilammy/msvc-dev-cmd@v1 + uses: TheMrMilchmann/setup-msvc-dev@v4.1.0 with: arch: ${{ matrix.arch }} - name: Configure libOpenCOR (for LLVM) @@ -98,7 +100,7 @@ jobs: run: cmake -G Ninja -S . -B build -DBUILD_TYPE=${{ matrix.build_type }} -DONLY_BUILD_THIRD_PARTY_LIBRARIES=ON -DPREBUILT_LIBCELLML=OFF -DPREBUILT_LIBCOMBINE=OFF -DPREBUILT_LIBCURL=OFF -DPREBUILT_LIBNUML=OFF -DPREBUILT_LIBSBML=OFF -DPREBUILT_LIBSEDML=OFF -DPREBUILT_LIBXML2=OFF -DPREBUILT_OPENSSL=OFF -DPREBUILT_SUNDIALS=OFF -DPREBUILT_ZIPPER=OFF -DPREBUILT_ZLIB=OFF - name: Upload library artifact if: ${{ !startsWith(github.ref, 'refs/tags/v') }} - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: ${{ matrix.name }} path: ./build/*.tar.gz @@ -113,15 +115,13 @@ jobs: BUILDCACHE_LOG_FILE: "" steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneMacos: 'Pacific/Auckland' + run: sudo systemsetup -settimezone Pacific/Auckland - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Install CMake and Ninja uses: lukka/get-cmake@latest - name: Install buildcache - uses: opencor/buildcache-action@v1 + uses: opencor/buildcache-action@v3 with: cache_key: webassembly - name: Install Emscripten @@ -136,7 +136,7 @@ jobs: run: cmake --build build - name: Upload WebAssembly artifact if: ${{ !startsWith(github.ref, 'refs/tags/v') }} - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: WebAssembly path: ./build/*.tar.gz diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index f3c96f4e4..0fb0f9b24 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -86,22 +86,24 @@ jobs: BUILDCACHE_LOG_FILE: "" steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneWindows: 'New Zealand Standard Time' - timezoneLinux: 'Pacific/Auckland' - timezoneMacos: 'Pacific/Auckland' + shell: bash + run: | + case "${RUNNER_OS}" in + Windows) MSYS_NO_PATHCONV=1 tzutil /s "New Zealand Standard Time" ;; + Linux) sudo timedatectl set-timezone Pacific/Auckland ;; + macOS) sudo systemsetup -settimezone Pacific/Auckland ;; + esac - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Install CMake and Ninja uses: lukka/get-cmake@latest - name: Install buildcache - uses: opencor/buildcache-action@v1 + uses: opencor/buildcache-action@v3 with: cache_key: cd-${{ matrix.os }}-${{ matrix.build_type }}-${{ matrix.shared_libs }} - name: Configure MSVC if: ${{ runner.os == 'Windows' }} - uses: ilammy/msvc-dev-cmd@v1 + uses: TheMrMilchmann/setup-msvc-dev@v4.1.0 with: arch: ${{ matrix.arch }} - name: Install GCC 16 @@ -124,13 +126,13 @@ jobs: cpack - name: Upload libOpenCOR artifacts if: ${{ !startsWith(github.ref, 'refs/tags/v') }} - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: ${{ matrix.name }} path: ./build/libOpenCOR-* - name: Release libOpenCOR if: ${{ startsWith(github.ref, 'refs/tags/v') }} - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@v3 with: files: ./build/libOpenCOR-* python_wheels: @@ -156,13 +158,15 @@ jobs: os: macos-15 steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneWindows: 'New Zealand Standard Time' - timezoneLinux: 'Pacific/Auckland' - timezoneMacos: 'Pacific/Auckland' + shell: bash + run: | + case "${RUNNER_OS}" in + Windows) MSYS_NO_PATHCONV=1 tzutil /s "New Zealand Standard Time" ;; + Linux) sudo timedatectl set-timezone Pacific/Auckland ;; + macOS) sudo systemsetup -settimezone Pacific/Auckland ;; + esac - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Free disk space if: ${{ runner.os == 'Linux' }} run: | @@ -170,7 +174,7 @@ jobs: sudo docker image prune --all --force - name: Configure MSVC if: ${{ runner.os == 'Windows' }} - uses: ilammy/msvc-dev-cmd@v1 + uses: TheMrMilchmann/setup-msvc-dev@v4.1.0 with: arch: ${{ matrix.arch }} - name: Build Python wheels @@ -191,13 +195,13 @@ jobs: # must be kept in sync with the above images (and the version of cibuildwheel that we use). CIBW_SKIP: '*musllinux*' - name: Upload Python wheel artifacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: ${{ matrix.name }} path: ./wheelhouse/*.whl - name: Release Python wheels if: ${{ startsWith(github.ref, 'refs/tags/v') }} - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@v3 with: files: ./wheelhouse/*.whl python_pypi: @@ -208,7 +212,7 @@ jobs: if: ${{ startsWith(github.ref, 'refs/tags/v') }} steps: - name: Download Python wheels - uses: actions/download-artifact@v5 + uses: actions/download-artifact@v8 with: pattern: "*Python wheels*" path: ./dist @@ -226,13 +230,11 @@ jobs: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneLinux: 'Pacific/Auckland' + run: sudo timedatectl set-timezone Pacific/Auckland - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Install Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: 'lts/*' registry-url: 'https://registry.npmjs.org' @@ -248,24 +250,24 @@ jobs: - name: Install CMake and Ninja uses: lukka/get-cmake@latest - name: Install buildcache - uses: opencor/buildcache-action@v1 + uses: opencor/buildcache-action@v3 with: cache_key: cd-webassembly - name: Install Emscripten - uses: mymindstorm/setup-emsdk@v14 + uses: emscripten-core/setup-emsdk@v16 - name: Configure libOpenCOR run: cmake -G Ninja -S . -B build -DBUILD_TYPE=Release -DCODE_ANALYSIS=OFF -DCODE_COVERAGE=OFF -DDOCUMENTATION=OFF -DJAVASCRIPT_BINDINGS=ON -DJAVASCRIPT_UNIT_TESTING=OFF -DMEMORY_CHECKS=OFF -DPYTHON_BINDINGS=OFF -DPYTHON_UNIT_TESTING=OFF -DSHARED_LIBS=OFF -DUNIT_TESTING=OFF - name: Build WebAssembly run: cmake --build build - name: Upload WebAssembly artifact if: ${{ !startsWith(github.ref, 'refs/tags/v') }} - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: WebAssembly path: ./build/libopencor-*.tgz - name: Release WebAssembly if: ${{ startsWith(github.ref, 'refs/tags/v') }} - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@v3 with: files: ./build/libopencor-*.tgz - name: Compress WebAssembly using Brotli diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ebec7fd1..51c7477e2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -334,27 +334,29 @@ jobs: BUILDCACHE_LOG_FILE: "" steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneWindows: 'New Zealand Standard Time' - timezoneLinux: 'Pacific/Auckland' - timezoneMacos: 'Pacific/Auckland' + shell: bash + run: | + case "${RUNNER_OS}" in + Windows) MSYS_NO_PATHCONV=1 tzutil /s "New Zealand Standard Time" ;; + Linux) sudo timedatectl set-timezone Pacific/Auckland ;; + macOS) sudo systemsetup -settimezone Pacific/Auckland ;; + esac - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Install Python if: ${{ matrix.documentation == 'ON' || matrix.python_support == 'ON' || matrix.target == 'python_check_code_formatting' }} - uses: actions/setup-python@v5 + uses: actions/setup-python@v7 with: python-version: '3.12' - name: Install CMake and Ninja uses: lukka/get-cmake@latest - name: Install buildcache - uses: opencor/buildcache-action@v1 + uses: opencor/buildcache-action@v3 with: cache_key: ci-${{ matrix.os }}-${{ matrix.build_type }}-${{ matrix.code_analysis }}-${{ matrix.code_coverage }}-${{ matrix.documentation }}-${{ matrix.javascript_support }}-${{ matrix.memory_checks }}-${{ matrix.python_support }}-${{ matrix.shared_libs }}-${{ matrix.unit_testing }}-${{ matrix.target }} - name: Configure MSVC if: ${{ runner.os == 'Windows' }} - uses: ilammy/msvc-dev-cmd@v1 + uses: TheMrMilchmann/setup-msvc-dev@v4.1.0 with: arch: ${{ matrix.arch }} - name: Install GCC 16 @@ -384,7 +386,7 @@ jobs: sudo mv clang-tidy /usr/local/bin - name: Install Emscripten if: ${{ matrix.javascript_support == 'ON' }} - uses: mymindstorm/setup-emsdk@v14 + uses: emscripten-core/setup-emsdk@v16 - name: Install Biome if: ${{ matrix.target == 'javascript_check_code_formatting' }} run: | @@ -393,7 +395,7 @@ jobs: mv biome /usr/local/bin - name: Install uv if: ${{ matrix.documentation == 'ON' || matrix.python_support == 'ON' || matrix.target == 'python_check_code_formatting' }} - uses: astral-sh/setup-uv@v6 + uses: astral-sh/setup-uv@v10.2.0 - name: Install Ruff if: ${{ matrix.target == 'python_check_code_formatting' }} run: uv pip install --system ruff @@ -508,11 +510,9 @@ jobs: os: ubuntu-24.04-arm steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneLinux: 'Pacific/Auckland' + run: sudo timedatectl set-timezone Pacific/Auckland - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Free disk space run: | sudo rm -rf /opt/hostedtoolcache @@ -530,11 +530,9 @@ jobs: runs-on: ubuntu-24.04 steps: - name: Set the timezone to New Zealand - uses: szenius/set-timezone@v2.0 - with: - timezoneLinux: 'Pacific/Auckland' + run: sudo timedatectl set-timezone Pacific/Auckland - name: Check out libOpenCOR - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Spell check uses: codespell-project/actions-codespell@v2 with: diff --git a/README.md b/README.md index ffe109512..215c0bda4 100644 --- a/README.md +++ b/README.md @@ -10,5 +10,4 @@ You might also be interested in our [users](https://groups.google.com/forum/#!fo Please [contact us](https://opencor.ws/libopencor/contactUs.html) if you have any questions about libOpenCOR. -[![Ask DeepWiki](https://deepwiki.com/badge.svg)](https://deepwiki.com/opencor/libopencor) ![Alt](https://repobeats.axiom.co/api/embed/e57859796c7c35c163c88238f9bfd3f2aad9649f.svg "Repobeats analytics image") diff --git a/VERSION.txt b/VERSION.txt index 3473d135a..e34ebf6bb 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.20261009.3 +1.20261009.4 diff --git a/cmake/environmentchecks.cmake b/cmake/environmentchecks.cmake index 0e9c63c79..fb999dd54 100644 --- a/cmake/environmentchecks.cmake +++ b/cmake/environmentchecks.cmake @@ -168,12 +168,14 @@ if(Python_EXECUTABLE) endif() # Check some compiler flags. +# Note: we use -fprofile-update=atomic since some of our tests are multithreaded and non-atomic profile counters would +# then get corrupted (e.g., a covered branch could be reported as not covered). include(CheckCXXCompilerFlag) set(ORIG_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) -set(CODE_COVERAGE_COMPILER_FLAGS "-fprofile-instr-generate -fcoverage-mapping") +set(CODE_COVERAGE_COMPILER_FLAGS "-fprofile-instr-generate -fcoverage-mapping -fprofile-update=atomic") set(CODE_COVERAGE_LINKER_FLAGS "-fprofile-instr-generate") set(CMAKE_REQUIRED_FLAGS ${CODE_COVERAGE_COMPILER_FLAGS}) diff --git a/src/bindings/python/file.cpp b/src/bindings/python/file.cpp index b39973a71..a7d181913 100644 --- a/src/bindings/python/file.cpp +++ b/src/bindings/python/file.cpp @@ -35,7 +35,11 @@ void fileApi(nb::module_ &m) .value("CombineArchive", libOpenCOR::File::Type::COMBINE_ARCHIVE) .value("IrretrievableFile", libOpenCOR::File::Type::IRRETRIEVABLE_FILE); - file.def(nb::new_(&libOpenCOR::File::create), "Create a File object.", nb::arg("file_name_or_url"), nb::arg("retrieve_contents") = true) + // Note: we release the GIL when creating a file, as well as when retrieving managed files, since it may take a + // while (e.g., when downloading a remote file) and since it allows other Python threads to create files + // concurrently. + + file.def(nb::new_(&libOpenCOR::File::create), "Create a File object.", nb::arg("file_name_or_url"), nb::arg("retrieve_contents") = true, nb::call_guard()) .def_prop_ro("type", &libOpenCOR::File::type, "Return the type.") .def_prop_ro("file_name", &libOpenCOR::File::fileName, "Return the file name.") .def_prop_ro("url", &libOpenCOR::File::url, "Return the URL.") @@ -63,9 +67,9 @@ void fileApi(nb::module_ &m) .def("reset", &libOpenCOR::FileManager::reset, "Reset the file manager.") .def_prop_ro("has_files", &libOpenCOR::FileManager::hasFiles, "Return whether there are some managed files.") .def_prop_ro("file_count", &libOpenCOR::FileManager::fileCount, "Return the number of managed files.") - .def_prop_ro("files", &libOpenCOR::FileManager::files, "Return the managed files.") - .def("file", nb::overload_cast(&libOpenCOR::FileManager::file, nb::const_), "Return the managed file at the given index.", nb::arg("index")) - .def("file", nb::overload_cast(&libOpenCOR::FileManager::file, nb::const_), "Return the managed file with the given name or URL.", nb::arg("file_name_or_url")) + .def_prop_ro("files", &libOpenCOR::FileManager::files, "Return the managed files.", nb::call_guard()) + .def("file", nb::overload_cast(&libOpenCOR::FileManager::file, nb::const_), "Return the managed file at the given index.", nb::arg("index"), nb::call_guard()) + .def("file", nb::overload_cast(&libOpenCOR::FileManager::file, nb::const_), "Return the managed file with the given name or URL.", nb::arg("file_name_or_url"), nb::call_guard()) .def("__len__", &libOpenCOR::FileManager::fileCount) .def("__iter__", [](const libOpenCOR::FileManager &self) { return nb::iter(nb::cast(self.files())); diff --git a/src/file/file.cpp b/src/file/file.cpp index 558e2dfc2..b380207ca 100644 --- a/src/file/file.cpp +++ b/src/file/file.cpp @@ -44,6 +44,7 @@ File::Impl::Impl(const std::string &pFileNameOrUrl, bool pRetrieveContents) if (res) { mFilePath = filePath; + mDownloaded = true; } else { mType = Type::IRRETRIEVABLE_FILE; @@ -52,10 +53,17 @@ File::Impl::Impl(const std::string &pFileNameOrUrl, bool pRetrieveContents) } else { mFilePath = stringToPath("/some/path/file"); } - } else if (!std::filesystem::exists(mFilePath) && pRetrieveContents) { - mType = Type::IRRETRIEVABLE_FILE; + } else if (pRetrieveContents) { + // Note: we use the std::error_code version of std::filesystem::exists() so that nothing gets thrown (e.g., if + // the file name is too long), in which case we consider that the file doesn't exist. - addError("The file does not exist."); + std::error_code errorCode; + + if (!std::filesystem::exists(mFilePath, errorCode)) { + mType = Type::IRRETRIEVABLE_FILE; + + addError("The file does not exist."); + } } #else if (mFilePath.empty()) { @@ -68,10 +76,15 @@ File::Impl::Impl(const std::string &pFileNameOrUrl, bool pRetrieveContents) File::Impl::~Impl() { - // Delete the local file associated with a remote file. + // Delete the local copy of a remote file, if we downloaded it. + // Note #1: a remote file that we didn't download has a dummy file path, which must obviously not be deleted. + // Note #2: we use the std::error_code version of std::filesystem::remove() so that nothing gets thrown since we are + // in a destructor. + + if (mDownloaded) { + std::error_code errorCode; - if (!mUrl.empty() && !mFilePath.empty()) { - std::filesystem::remove(mFilePath); + std::filesystem::remove(mFilePath, errorCode); } } diff --git a/src/file/file_p.h b/src/file/file_p.h index 9dd0bdae7..9939962fe 100644 --- a/src/file/file_p.h +++ b/src/file/file_p.h @@ -38,10 +38,10 @@ class File::Impl: public Logger::Impl std::string mUrl; bool mTypeChecked {false}; - bool mRetrieveContents {true}; bool mContentsRetrieved {false}; UnsignedChars mContents; + bool mDownloaded {false}; CellmlFilePtr mCellmlFile; SedmlFilePtr mSedmlFile; diff --git a/src/file/filemanager.cpp b/src/file/filemanager.cpp index 5c8fcc1dd..ddc2d51ed 100644 --- a/src/file/filemanager.cpp +++ b/src/file/filemanager.cpp @@ -31,8 +31,37 @@ FileManager::Impl &FileManager::Impl::instance() return instance; } +FilePtr FileManager::Impl::managedFile(bool pIsLocalFile, const std::string &pFileNameOrUrl, FilePtrs &pLockedFiles) const +{ + // Return the managed file, if any, with the given name or URL. + // Note #1: our caller must hold our lock. + // Note #2: a file may expire at any time (i.e. even while we hold our lock) since another thread may release its + // last reference to it (it will then get unmanaged by its destructor), so we ignore it. We do this without + // a branch of our own since such a branch would only ever be taken when threads race, i.e. it could not be + // reliably tested. + // Note #3: another thread may also release its last reference to a file while we have it locked, in which case + // releasing our reference would destroy the file and its destructor would try to get our lock, i.e. we + // would deadlock. So, we keep track of the files that we lock and our caller must only release them after + // having released our lock. + + for (const auto &file : mFiles) { + pLockedFiles.push_back(file.lock()); + } + + std::erase(pLockedFiles, nullptr); + + for (const auto &lockedFile : pLockedFiles) { + if (pIsLocalFile ? lockedFile->fileName() == pFileNameOrUrl : lockedFile->url() == pFileNameOrUrl) { + return lockedFile; + } + } + + return nullptr; +} + FilePtr FileManager::Impl::manage(const FilePtr &pFile) { + FilePtrs lockedFiles; // Note: it must be declared before our lock (see managedFile()). const std::unique_lock lock(mMutex); // Opportunistically remove any expired entries and correct our file count. @@ -51,26 +80,24 @@ FilePtr FileManager::Impl::manage(const FilePtr &pFile) const bool isLocalFile = pFile->url().empty(); const auto &fileNameOrUrl = isLocalFile ? pFile->fileName() : pFile->url(); - - for (const auto &file : mFiles) { - auto managedFile {file.lock()}; - - if (isLocalFile ? managedFile->fileName() == fileNameOrUrl : managedFile->url() == fileNameOrUrl) { - return managedFile; - } - } + auto res {managedFile(isLocalFile, fileNameOrUrl, lockedFiles)}; // No duplicate found, so manage the new file. - mFiles.emplace_back(pFile); + if (res == nullptr) { + mFiles.emplace_back(pFile); + + ++mFileCount; - ++mFileCount; + res = pFile; + } - return pFile; + return res; } void FileManager::Impl::unmanage(File *pFile) { + FilePtrs lockedFiles; // Note: it must be declared before our lock (see managedFile()). const std::unique_lock lock(mMutex); // Iteratively unmanage the file and all its child files. @@ -95,10 +122,16 @@ void FileManager::Impl::unmanage(File *pFile) // Unmanage the current file. - const auto removeEnd = std::ranges::remove_if(mFiles.begin(), mFiles.end(), [&file](const auto &managedFile) { - auto managedFilePtr {managedFile.lock()}; + const auto removeEnd = std::ranges::remove_if(mFiles.begin(), mFiles.end(), [&file, &lockedFiles](const auto &fileEntry) { + auto managedFilePtr {fileEntry.lock()}; - return (managedFilePtr == nullptr) || (managedFilePtr.get() == file); + if (managedFilePtr == nullptr) { + return true; + } + + lockedFiles.push_back(managedFilePtr); + + return managedFilePtr.get() == file; }).begin(); mFileCount -= static_cast(mFiles.end() - removeEnd); @@ -133,14 +166,17 @@ size_t FileManager::Impl::fileCount() const FilePtrs FileManager::Impl::files() const { + FilePtrs res; // Note: it must be declared before our lock (see managedFile()). const std::shared_lock lock(mMutex); - FilePtrs res; + // Note: a file may expire at any time, so we ignore it (see managedFile()). for (const auto &file : mFiles) { res.push_back(file.lock()); } + std::erase(res, nullptr); + return res; } @@ -167,8 +203,6 @@ FilePtr FileManager::Impl::fileFromFileNameOrUrl(const std::string &pFileNameOrU FilePtr FileManager::Impl::file(const std::string &pFileNameOrUrl) const #endif { - const std::shared_lock lock(mMutex); - #if __clang_major__ < 16 auto [tIsLocalFile, tFileNameOrUrl] {retrieveFileInfo(pFileNameOrUrl)}; auto isLocalFile {tIsLocalFile}; @@ -176,16 +210,10 @@ FilePtr FileManager::Impl::file(const std::string &pFileNameOrUrl) const #else auto [isLocalFile, fileNameOrUrl] {retrieveFileInfo(pFileNameOrUrl)}; #endif + FilePtrs lockedFiles; // Note: it must be declared before our lock (see managedFile()). + const std::shared_lock lock(mMutex); - for (const auto &file : mFiles) { - auto managedFile {file.lock()}; - - if (isLocalFile ? managedFile->fileName() == fileNameOrUrl : managedFile->url() == fileNameOrUrl) { - return managedFile; - } - } - - return nullptr; + return managedFile(isLocalFile, fileNameOrUrl, lockedFiles); } FileManager &FileManager::instance() diff --git a/src/file/filemanager_p.h b/src/file/filemanager_p.h index c9c8a2803..5768683a7 100644 --- a/src/file/filemanager_p.h +++ b/src/file/filemanager_p.h @@ -36,6 +36,7 @@ class FileManager::Impl static Impl &instance(); + FilePtr managedFile(bool pIsLocalFile, const std::string &pFileNameOrUrl, FilePtrs &pLockedFiles) const; FilePtr manage(const FilePtr &pFile); void unmanage(File *pFile); diff --git a/src/misc/utils.cpp b/src/misc/utils.cpp index 66eb56df4..d835a392c 100644 --- a/src/misc/utils.cpp +++ b/src/misc/utils.cpp @@ -29,6 +29,7 @@ limitations under the License. # include #endif +#include #include #include #include @@ -36,6 +37,7 @@ limitations under the License. #include #include #include +#include #ifdef BUILDING_USING_MSVC # include @@ -49,6 +51,10 @@ limitations under the License. # undef NAN #endif +#ifdef min +# undef min +#endif + namespace libOpenCOR { #ifndef CODE_COVERAGE_ENABLED @@ -230,74 +236,140 @@ std::string pathToString(const std::filesystem::path &pPath) #endif } -#ifdef BUILDING_USING_MSVC -std::string canonicalFileName(const std::string &pFileName, bool pIsRemoteFile) -#else std::string canonicalFileName(const std::string &pFileName) -#endif { // Determine the canonical version of the file name. - // Note #1: if the file exists then we want to use std::filesystem::canonical() since this will resolve any symbolic - // links, etc. However, if the file doesn't exist then we want to use std::filesystem::weakly_canonical() - // since this will return a file name that is as close to the canonical version as possible without - // actually checking whether the file exists. - // Note #2: when building using Emscripten, std::filesystem::weakly_canonical() doesn't work as expected. For - // instance, if pFileName is equal to "/some/path/../.." then it will generate an exception rather than - // return "/". So, we prepend a dummy folder to pFileName and then remove it from the result of - // std::filesystem::weakly_canonical(). + // Note #1: we must never change the current working directory here since it is shared by all the threads of the + // process. + // Note #2: we use the std::error_code versions of the std::filesystem functions so that nothing gets thrown (e.g., + // if the file name is too long or if the file gets deleted while we are dealing with it). + // Note #3: when building using Emscripten, the file system is virtual and std::filesystem::weakly_canonical() + // doesn't work as expected (e.g., it throws an exception rather than return "/" for "/some/path/../.."), + // so we only ever normalise the file name lexically. + + // An empty file name stays empty (std::filesystem::canonical() would otherwise return the current working + // directory). + + if (pFileName.empty()) { + return {}; + } - static constexpr auto FORWARD_SLASH {"/"}; + auto filePath {stringToPath(pFileName)}; + +#ifndef __EMSCRIPTEN__ + std::error_code errorCode; + + // If the file exists then use std::filesystem::canonical() since it resolves symbolic links, etc. + // Note: this fails if the file doesn't exist. + + auto res {std::filesystem::canonical(filePath, errorCode)}; + + if (!errorCode) { + return pathToString(res); + } + + // The file doesn't exist, so if its file name has a root directory (i.e., it is an absolute file name or, on + // Windows, a file name relative to the root of the current drive) then use std::filesystem::weakly_canonical() + // since it returns a file name that is as close to the canonical version as possible. - auto fileExists {std::filesystem::exists(pFileName)}; - auto currentPath {std::filesystem::current_path()}; + if (filePath.has_root_directory()) { + res = std::filesystem::weakly_canonical(filePath, errorCode); - if (!fileExists) { - std::filesystem::current_path(FORWARD_SLASH); + if (!errorCode) { + return pathToString(res); + } } +#endif -#ifdef __EMSCRIPTEN__ - static constexpr auto DUMMY_FOLDER {"/dummy"}; + // The file name is relative (or something went wrong), so normalise it lexically. Unlike + // std::filesystem::weakly_canonical(), this doesn't depend on the current working directory, and it keeps leading + // ".." components (e.g., "a/../../b" becomes "../b"). - auto res {pFileName}; + return pathToString(filePath.lexically_normal()); +} + +std::string canonicalUrl(const std::string &pUrl) +{ + // Determine the canonical version of the URL, i.e. remove the "." and ".." segments from its path, as described in + // RFC 3986 (https://www.rfc-editor.org/rfc/rfc3986#section-5.2.4), as well as its empty segments (e.g., "/a//b" + // becomes "/a/b"). + // Note: a URL is never a local file name, so we must not access the file system (a local file that happens to have + // the same name as the host and path of the URL would otherwise be used) or use std::filesystem::path (its + // separators are platform specific). - res = DUMMY_FOLDER + std::string(res.starts_with(FORWARD_SLASH) ? "" : FORWARD_SLASH) + res; - res = pathToString(std::filesystem::weakly_canonical(stringToPath(res))); + static constexpr auto FORWARD_SLASH {'/'}; + static constexpr auto DOT {"."}; + static constexpr auto DOT_DOT {".."}; - res.erase(0, strlen(DUMMY_FOLDER)); +#ifdef BUILDING_USING_MSVC + auto url {forwardSlashPath(pUrl)}; #else - auto filePath {stringToPath(pFileName)}; - auto res {pathToString(fileExists ? - std::filesystem::canonical(filePath) : - std::filesystem::weakly_canonical(filePath))}; + const auto &url {pUrl}; #endif -#if defined(BUILDING_USING_MSVC) - // Replace "\"s with "/"s, if needed. + // Retrieve the scheme and authority (e.g., "https://example.com"), the path (e.g., "/some/path/file.txt"), and the + // query and/or fragment (e.g., "?a=b#c") of the URL. + + static constexpr auto SCHEME_SEPARATOR {"://"}; + static const auto SCHEME_SEPARATOR_LENGTH {strlen(SCHEME_SEPARATOR)}; - if (pIsRemoteFile) { - res = forwardSlashPath(pFileName); + auto schemeSeparatorPos {url.find(SCHEME_SEPARATOR)}; + auto authorityPos {(schemeSeparatorPos == std::string::npos) ? 0 : schemeSeparatorPos + SCHEME_SEPARATOR_LENGTH}; + auto queryOrFragmentPos {url.find_first_of("?#", authorityPos)}; + auto pathPos {std::min(url.find(FORWARD_SLASH, authorityPos), queryOrFragmentPos)}; + + if ((pathPos == std::string::npos) || (url[pathPos] != FORWARD_SLASH)) { + return url; } -#elif defined(BUILDING_USING_CLANG) || defined(__EMSCRIPTEN__) - // The file name may be relative rather than absolute, in which case we need to remove the forward slash that got - // added (at the beginning of the file name) by std::filesystem::weakly_canonical(). - if (!fileExists && !pFileName.starts_with(FORWARD_SLASH)) { - static const auto FORWARD_SLASH_LENGTH {strlen(FORWARD_SLASH)}; + auto path {url.substr(pathPos, (queryOrFragmentPos == std::string::npos) ? std::string::npos : queryOrFragmentPos - pathPos)}; + + // Remove the ".", "..", and empty segments from the path, keeping a trailing forward slash if the last segment is a + // ".", "..", or empty segment (e.g., "/a/b/.." and "/a//" become "/a/"). + // Note: path starts with a forward slash, so the first segment that we retrieve is always empty and we skip it. + + std::vector segments; + size_t segmentPos {1}; - res.erase(0, FORWARD_SLASH_LENGTH); + while (true) { + auto nextSegmentPos {path.find(FORWARD_SLASH, segmentPos)}; + auto isLastSegment {nextSegmentPos == std::string::npos}; + auto segment {path.substr(segmentPos, isLastSegment ? std::string::npos : nextSegmentPos - segmentPos)}; + + if (segment == DOT_DOT) { + if (!segments.empty()) { + segments.pop_back(); + } + } else if ((segment != DOT) && !segment.empty()) { + segments.push_back(segment); + } + + if (isLastSegment) { + if ((segment == DOT) || (segment == DOT_DOT) || segment.empty()) { + segments.emplace_back(); + } + + break; + } + + segmentPos = nextSegmentPos + 1; } -#endif - if (!fileExists) { - std::filesystem::current_path(currentPath); + std::string res {url.substr(0, pathPos)}; + + for (const auto &segment : segments) { + res += FORWARD_SLASH; + res += segment; } - // Return the canonical version of the file name. + if (queryOrFragmentPos != std::string::npos) { + res += url.substr(queryOrFragmentPos); + } return res; } -std::tuple retrieveFileInfo(const std::string &pFileNameOrUrl) +std::tuple retrieveFileInfo(const std::string &pFileNameOrUrl, bool pCanonicalise) { // Check whether the given file name or URL is a local file name or a URL. // Note: a URL represents a local file when used with the "file" scheme. @@ -309,38 +381,19 @@ std::tuple retrieveFileInfo(const std::string &pFileNameOrUrl #endif static auto FILE_SCHEME_LENGTH {strlen(FILE_SCHEME)}; static constexpr auto HTTP_SCHEME {"http://"}; - static auto HTTP_SCHEME_LENGTH {strlen(HTTP_SCHEME)}; static constexpr auto HTTPS_SCHEME {"https://"}; - static auto HTTPS_SCHEME_LENGTH {strlen(HTTPS_SCHEME)}; - auto res {pFileNameOrUrl}; - size_t schemeLength {0}; - auto requiresHttpScheme {false}; - auto requiresHttpsScheme {false}; - - if (pFileNameOrUrl.starts_with(FILE_SCHEME)) { - schemeLength = FILE_SCHEME_LENGTH; - } else if (pFileNameOrUrl.starts_with(HTTP_SCHEME)) { - schemeLength = HTTP_SCHEME_LENGTH; - requiresHttpScheme = true; - } else if (pFileNameOrUrl.starts_with(HTTPS_SCHEME)) { - schemeLength = HTTPS_SCHEME_LENGTH; - requiresHttpsScheme = true; + if (pFileNameOrUrl.starts_with(HTTP_SCHEME) || pFileNameOrUrl.starts_with(HTTPS_SCHEME)) { + return {false, pCanonicalise ? canonicalUrl(pFileNameOrUrl) : pFileNameOrUrl}; } - res.erase(0, schemeLength); + auto res {pFileNameOrUrl}; - return {!requiresHttpScheme && !requiresHttpsScheme, - (requiresHttpScheme ? - HTTP_SCHEME : - requiresHttpsScheme ? - HTTPS_SCHEME : - "") -#ifdef BUILDING_USING_MSVC - + canonicalFileName(res, requiresHttpScheme || requiresHttpsScheme)}; -#else - + canonicalFileName(res)}; -#endif + if (res.starts_with(FILE_SCHEME)) { + res.erase(0, FILE_SCHEME_LENGTH); + } + + return {true, pCanonicalise ? canonicalFileName(res) : res}; } namespace { @@ -546,7 +599,9 @@ std::tuple downloadFile(const std::string &pUrl) return {true, filePath}; } - std::filesystem::remove(filePath); + std::error_code errorCode; + + std::filesystem::remove(filePath, errorCode); return NO_TUPLE; } @@ -563,7 +618,16 @@ UnsignedChars fileContents(const std::filesystem::path &pFilePath) return NO_UNSIGNED_CHARS; } - const auto fileSize {std::filesystem::file_size(pFilePath)}; + // Note: we use the std::error_code version of std::filesystem::file_size() so that nothing gets thrown (e.g., if + // the file is actually a directory, which can be opened on some platforms). + + std::error_code errorCode; + const auto fileSize {std::filesystem::file_size(pFilePath, errorCode)}; + + if (errorCode) { + return NO_UNSIGNED_CHARS; + } + UnsignedChars contents; contents.resize(fileSize); diff --git a/src/misc/utils.h b/src/misc/utils.h index e07be9907..45bf27eb2 100644 --- a/src/misc/utils.h +++ b/src/misc/utils.h @@ -92,12 +92,9 @@ std::string LIBOPENCOR_UNIT_TESTING_EXPORT forwardSlashPath(const std::string &p std::filesystem::path stringToPath(const std::string &pString); std::string pathToString(const std::filesystem::path &pPath); -#ifdef BUILDING_USING_MSVC -std::string LIBOPENCOR_UNIT_TESTING_EXPORT canonicalFileName(const std::string &pFileName, bool pIsRemoteFile = false); -#else std::string LIBOPENCOR_UNIT_TESTING_EXPORT canonicalFileName(const std::string &pFileName); -#endif -std::tuple retrieveFileInfo(const std::string &pFileNameOrUrl); +std::string LIBOPENCOR_UNIT_TESTING_EXPORT canonicalUrl(const std::string &pUrl); +std::tuple retrieveFileInfo(const std::string &pFileNameOrUrl, bool pCanonicalise = true); std::string relativePath(const std::string &pPath, const std::string &pBasePath); std::string urlPath(const std::string &pPath); diff --git a/src/support/sedml/sedmlfile.cpp b/src/support/sedml/sedmlfile.cpp index 92d96895e..884bace57 100644 --- a/src/support/sedml/sedmlfile.cpp +++ b/src/support/sedml/sedmlfile.cpp @@ -53,15 +53,34 @@ limitations under the License. namespace libOpenCOR { SedmlFile::Impl::Impl(const FilePtr &pFile, libsedml::SedDocument *pDocument) - : mLocation(pathToString(stringToPath(pFile->url().empty() ? - pFile->fileName() : - pFile->url()) - .parent_path())) - , mDocument(pDocument) + : mDocument(pDocument) , mContents(toString(pFile->contents())) { - if (!mLocation.empty()) { - mLocation += "/"; + // Determine the location of the SED-ML file, i.e. what a relative model source is relative to. + // Note: for a remote SED-ML file, we must not use std::filesystem::path since its separators are platform specific + // (e.g., "https://example.com/dir" would become "https:\\example.com\dir" on Windows) and since the query + // and/or fragment of the URL, if any, may contain forward slashes. + + const auto &url {pFile->url()}; + + if (url.empty()) { + mLocation = pathToString(stringToPath(pFile->fileName()).parent_path()); + + if (!mLocation.empty()) { + mLocation += "/"; + } + } else { + // Note: the URL may have no path (e.g., "https://example.com?a=b"), in which case its location is its root + // (e.g., "https://example.com/"). + + static constexpr std::string_view SCHEME_SEPARATOR {"://"}; + + auto urlWithoutQueryAndFragment {url.substr(0, url.find_first_of("?#"))}; + auto pathPos {urlWithoutQueryAndFragment.find('/', urlWithoutQueryAndFragment.find(SCHEME_SEPARATOR) + SCHEME_SEPARATOR.size())}; + + mLocation = (pathPos == std::string::npos) ? + urlWithoutQueryAndFragment + "/" : + urlWithoutQueryAndFragment.substr(0, urlWithoutQueryAndFragment.rfind('/') + 1); } } @@ -355,8 +374,13 @@ void SedmlFile::Impl::populateDocument(const SedDocumentPtr &pDocument) for (unsigned int i {0}; i < mDocument->getNumModels(); ++i) { auto source {mDocument->getModel(i)->getSource()}; - auto [isLocalFile, fileNameOrUrl] {retrieveFileInfo(source)}; - auto modelSource {(isLocalFile && stringToPath(fileNameOrUrl).is_relative()) ? + + // Note: a relative model source is relative to the SED-ML file, not to the current working directory, so we + // must not canonicalise it before we know whether it is relative (a model source that exists relative to + // the current working directory would otherwise become absolute). + + auto [isLocalFile, fileNameOrUrl] {retrieveFileInfo(source, false)}; + auto modelSource {(isLocalFile && !stringToPath(fileNameOrUrl).has_root_directory()) ? mLocation + fileNameOrUrl : fileNameOrUrl}; #ifdef __EMSCRIPTEN__ diff --git a/tests/api/file/basictests.cpp b/tests/api/file/basictests.cpp index 7e5ce1ace..eff842f44 100644 --- a/tests/api/file/basictests.cpp +++ b/tests/api/file/basictests.cpp @@ -18,8 +18,11 @@ limitations under the License. #include "tests/utils.h" +#include #include +#include #include +#include namespace { @@ -95,6 +98,50 @@ TEST(BasicFileTest, nonExistingRelativeLocalFile) EXPECT_EQ_ISSUES(file, expectedNonExistingFileIssues()); } +TEST(BasicFileTest, nonExistingRelativeLocalFileWithLeadingParentDirectories) +{ + auto file {libOpenCOR::File::create("../models/./lorenz.cellml")}; + + EXPECT_EQ(file->type(), libOpenCOR::File::Type::IRRETRIEVABLE_FILE); +#ifdef BUILDING_ON_WINDOWS + EXPECT_EQ(file->fileName(), R"(..\models\lorenz.cellml)"); +#else + EXPECT_EQ(file->fileName(), "../models/lorenz.cellml"); +#endif + EXPECT_EQ(file->url(), ""); +#ifdef BUILDING_ON_WINDOWS + EXPECT_EQ(file->path(), R"(..\models\lorenz.cellml)"); +#else + EXPECT_EQ(file->path(), "../models/lorenz.cellml"); +#endif + EXPECT_TRUE(file->contents().empty()); + EXPECT_EQ_ISSUES(file, expectedNonExistingFileIssues()); +} + +TEST(BasicFileTest, tooLongLocalFileName) +{ + // A file name that is too long for the file system must not result in an exception being thrown. + + static constexpr auto TOO_LONG_NAME_LENGTH {5000}; + + auto file {libOpenCOR::File::create("/" + std::string(TOO_LONG_NAME_LENGTH, 'a'))}; + + EXPECT_EQ(file->type(), libOpenCOR::File::Type::IRRETRIEVABLE_FILE); + EXPECT_TRUE(file->contents().empty()); + EXPECT_EQ_ISSUES(file, expectedNonExistingFileIssues()); +} + +TEST(BasicFileTest, localDirectory) +{ + // A directory is not a file, but it must not result in an exception being thrown. + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api"))}; + + EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE); + EXPECT_TRUE(file->contents().empty()); + EXPECT_EQ_ISSUES(file, expectedNoIssues()); +} + TEST(BasicFileTest, urlBasedLocalFile) { auto filePath {libOpenCOR::resourcePath("file.txt")}; @@ -134,6 +181,77 @@ TEST(BasicFileTest, encodedRemoteFile) EXPECT_FALSE(file->contents().empty()); } +TEST(BasicFileTest, remoteFileWithDotSegments) +{ + auto file {libOpenCOR::File::create("https://example.com/a/./b/../../c/model.cellml", false)}; + + EXPECT_EQ(file->url(), "https://example.com/c/model.cellml"); + EXPECT_EQ(file->path(), "https://example.com/c/model.cellml"); +} + +TEST(BasicFileTest, remoteFileMatchingLocalFile) +{ + // The host and path of a URL must never be resolved as a local file, even if such a local file exists. + + auto origDir {std::filesystem::current_path()}; + auto tempDir {std::filesystem::temp_directory_path() / "libopencor_remote_file_matching_local_file"}; + + std::filesystem::create_directories(tempDir / "example.com"); + std::ofstream(tempDir / "example.com" / "model.cellml").close(); + + std::filesystem::current_path(tempDir); + + auto url {libOpenCOR::File::create("https://example.com/model.cellml", false)->url()}; + + std::filesystem::current_path(origDir); + std::filesystem::remove_all(tempDir); + + EXPECT_EQ(url, "https://example.com/model.cellml"); +} + +TEST(BasicFileTest, concurrentFileCreations) +{ + // Creating a file must never change the current working directory since it is shared by all the threads of the + // process. Also, the file manager must cope with files that get looked up while they are being destroyed by another + // thread. + + static constexpr auto FILE_CREATION_COUNT {1000}; + + auto origDir {std::filesystem::current_path()}; + std::atomic done {false}; + std::atomic workingDirectoryChanged {false}; + std::thread checker([&] { + auto &fileManager {libOpenCOR::FileManager::instance()}; + + while (!done) { + if (std::filesystem::current_path() != origDir) { + workingDirectoryChanged = true; + } + + fileManager.file("https://example.com/model.cellml"); + fileManager.files(); + } + }); + auto createFiles {[] { + for (int i {0}; i < FILE_CREATION_COUNT; ++i) { + libOpenCOR::File::create("https://example.com/model.cellml", false); + libOpenCOR::File::create("non_existing_dir/../non_existing_file.txt", false); + } + }}; + std::thread thread1(createFiles); + std::thread thread2(createFiles); + + thread1.join(); + thread2.join(); + + done = true; + + checker.join(); + + EXPECT_FALSE(workingDirectoryChanged); + EXPECT_EQ(std::filesystem::current_path(), origDir); +} + TEST(BasicFileTest, localVirtualFile) { auto filePath {libOpenCOR::resourcePath("unknown_file.txt")}; diff --git a/tests/api/sed/basictests.cpp b/tests/api/sed/basictests.cpp index f109aeed3..4964b9ec8 100644 --- a/tests/api/sed/basictests.cpp +++ b/tests/api/sed/basictests.cpp @@ -16,8 +16,25 @@ limitations under the License. #include "tests/utils.h" +#include #include +namespace { + +std::string sedmlContents(const std::string &pModelSource) +{ + return R"( + + + + + +)"; +} + +} // namespace + TEST(BasicSedTest, noFile) { auto document {libOpenCOR::SedDocument::create()}; @@ -73,6 +90,82 @@ TEST(BasicSedTest, sedmlFileWithAbsoluteCellmlFile) EXPECT_FALSE(document->hasIssues()); } +TEST(BasicSedTest, sedmlFileWithRelativeCellmlFile) +{ + // A relative model source is relative to the SED-ML file and its leading ".." components must be kept. + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/relative_cellml_file.sedml"), false)}; + + file->setContents(libOpenCOR::charArrayToUnsignedChars(sedmlContents("../../cellml_2.cellml").c_str())); + + auto document {libOpenCOR::SedDocument::create(file)}; + + EXPECT_EQ(document->models().size(), 1U); + EXPECT_EQ(document->models()[0]->file()->path(), libOpenCOR::resourcePath("cellml_2.cellml")); + + auto neededFile {libOpenCOR::File::create(libOpenCOR::resourcePath("cellml_2.cellml"))}; + + document = libOpenCOR::SedDocument::create(file); + + EXPECT_FALSE(document->hasIssues()); + EXPECT_EQ(document->models().size(), 1U); + EXPECT_EQ(document->models()[0]->file(), neededFile); +} + +TEST(BasicSedTest, sedmlFileWithRelativeCellmlFileInWorkingDirectory) +{ + // A relative model source is relative to the SED-ML file, not to the current working directory, even if the + // current working directory contains a file with that name. + + auto origDir {std::filesystem::current_path()}; + + std::filesystem::current_path(libOpenCOR::resourcePath()); + + auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/relative_cellml_file.sedml"), false)}; + + file->setContents(libOpenCOR::charArrayToUnsignedChars(sedmlContents("cellml_2.cellml").c_str())); + + auto document {libOpenCOR::SedDocument::create(file)}; + + std::filesystem::current_path(origDir); + + EXPECT_EQ(document->models().size(), 1U); + EXPECT_EQ(document->models()[0]->file()->path(), libOpenCOR::resourcePath("api/sed/cellml_2.cellml")); +} + +TEST(BasicSedTest, remoteSedmlFileWithRelativeCellmlFile) +{ + // A relative model source is relative to the remote SED-ML file, whatever the platform and even if the query and/or + // fragment of the SED-ML file's URL contain some forward slashes. + + auto file {libOpenCOR::File::create("https://example.com/simulations/simulation.sedml?a=b/c#d/e", false)}; + + file->setContents(libOpenCOR::charArrayToUnsignedChars(sedmlContents("../models/./model.cellml").c_str())); + + auto neededFile {libOpenCOR::File::create("https://example.com/models/model.cellml", false)}; + auto document {libOpenCOR::SedDocument::create(file)}; + + EXPECT_FALSE(document->hasIssues()); + EXPECT_EQ(document->models().size(), 1U); + EXPECT_EQ(document->models()[0]->file(), neededFile); +} + +TEST(BasicSedTest, remoteSedmlFileWithoutPathWithRelativeCellmlFile) +{ + // A relative model source is relative to the root of a remote SED-ML file which URL has no path. + + auto file {libOpenCOR::File::create("https://example.com?a=b/c", false)}; + + file->setContents(libOpenCOR::charArrayToUnsignedChars(sedmlContents("model.cellml").c_str())); + + auto neededFile {libOpenCOR::File::create("https://example.com/model.cellml", false)}; + auto document {libOpenCOR::SedDocument::create(file)}; + + EXPECT_FALSE(document->hasIssues()); + EXPECT_EQ(document->models().size(), 1U); + EXPECT_EQ(document->models()[0]->file(), neededFile); +} + TEST(BasicSedTest, sedmlFileWithRemoteCellmlFile) { auto file {libOpenCOR::File::create(libOpenCOR::resourcePath("api/sed/remote_cellml_file.sedml"))}; diff --git a/tests/bindings/javascript/file.basic.test.js b/tests/bindings/javascript/file.basic.test.js index a845d63c4..aa2be60e3 100644 --- a/tests/bindings/javascript/file.basic.test.js +++ b/tests/bindings/javascript/file.basic.test.js @@ -15,6 +15,9 @@ limitations under the License. */ import assert from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import test from 'node:test'; import libOpenCOR from './libopencor.js'; @@ -51,6 +54,39 @@ test.describe('File basic tests', () => { assertIssues(loc, file, expectedUnknownFileIssues); }); + test('Non-existing relative local file with leading parent directories', () => { + const file = new loc.File('../models/./lorenz.cellml'); + + assert.strictEqual(file.type.value, loc.File.Type.UNKNOWN_FILE.value); + assert.strictEqual(file.fileName, '../models/lorenz.cellml'); + assert.strictEqual(file.url, ''); + assert.strictEqual(file.path, '../models/lorenz.cellml'); + assert.deepStrictEqual(file.contents(), Uint8Array.from([])); + assertIssues(loc, file, expectedNoIssues); + }); + + test('Too long local file name', () => { + // A file name that is too long for the file system must not result in an exception being thrown. + + const TOO_LONG_NAME_LENGTH = 5000; + + const file = new loc.File(`/${'a'.repeat(TOO_LONG_NAME_LENGTH)}`); + + assert.strictEqual(file.type.value, loc.File.Type.UNKNOWN_FILE.value); + assert.deepStrictEqual(file.contents(), Uint8Array.from([])); + assertIssues(loc, file, expectedNoIssues); + }); + + test('Local directory', () => { + // A directory is not a file, but it must not result in an exception being thrown. + + const file = new loc.File(utils.resourcePath('api')); + + assert.strictEqual(file.type.value, loc.File.Type.UNKNOWN_FILE.value); + assert.deepStrictEqual(file.contents(), Uint8Array.from([])); + assertIssues(loc, file, expectedNoIssues); + }); + test('Remote file', () => { const file = new loc.File(utils.REMOTE_FILE); @@ -97,6 +133,73 @@ test.describe('File basic tests', () => { assertIssues(loc, file, expectedUnknownFileIssues); }); + test('Remote file with dot segments', () => { + const file = new loc.File('https://example.com/a/./b/../../c/model.cellml'); + + assert.strictEqual(file.url, 'https://example.com/c/model.cellml'); + assert.strictEqual(file.path, 'https://example.com/c/model.cellml'); + }); + + test('Remote file matching local file', () => { + // The host and path of a URL must never be resolved as a local file, even if such a local file exists. + + const origDir = process.cwd(); + const tempDir = path.join(os.tmpdir(), 'libopencor_remote_file_matching_local_file'); + + fs.mkdirSync(path.join(tempDir, 'example.com'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'example.com', 'model.cellml'), ''); + + process.chdir(tempDir); + + const url = new loc.File('https://example.com/model.cellml').url; + + process.chdir(origDir); + fs.rmSync(tempDir, { recursive: true, force: true }); + + assert.strictEqual(url, 'https://example.com/model.cellml'); + }); + + test('Concurrent file creations', () => { + // Creating a file must never change the current working directory since it is shared by all the threads of the + // process. Also, the file manager must cope with files that get looked up while they are being destroyed. + // Note: JavaScript code cannot call libOpenCOR from several threads at once, so we interleave what the two + // file-creating threads and the checking thread do in the C++ version of this test. We also delete our + // handles as soon as we are done with them so that, like in the C++ version of this test, files get destroyed + // as soon as they are not needed anymore. + + const FILE_CREATION_COUNT = 1000; + + const origDir = process.cwd(); + const fileManager = loc.FileManager.instance(); + let workingDirectoryChanged = false; + + const check = () => { + if (process.cwd() !== origDir) { + workingDirectoryChanged = true; + } + + fileManager.fileFromFileNameOrUrl('https://example.com/model.cellml')?.delete(); + + for (const file of fileManager.files) { + file.delete(); + } + }; + const createFiles = () => { + new loc.File('https://example.com/model.cellml').delete(); + check(); + new loc.File('non_existing_dir/../non_existing_file.txt').delete(); + check(); + }; + + for (let i = 0; i < FILE_CREATION_COUNT; ++i) { + createFiles(); + createFiles(); + } + + assert.strictEqual(workingDirectoryChanged, false); + assert.strictEqual(process.cwd(), origDir); + }); + test('File manager', () => { const fileManager = loc.FileManager.instance(); const fileName = utils.resourcePath('file.txt'); diff --git a/tests/bindings/javascript/sed.basic.test.js b/tests/bindings/javascript/sed.basic.test.js index bc3a1bbb1..02ceaef00 100644 --- a/tests/bindings/javascript/sed.basic.test.js +++ b/tests/bindings/javascript/sed.basic.test.js @@ -23,6 +23,16 @@ import { assertIssues } from './utils.js'; const loc = await libOpenCOR(); +function sedmlContents(modelSource) { + return new TextEncoder().encode(` + + + + + +`); +} + test.describe('Sed basic tests', () => { test.beforeEach(() => { loc.FileManager.instance().reset(); @@ -88,6 +98,76 @@ test.describe('Sed basic tests', () => { assert.strictEqual(document.hasIssues, false); }); + test('SED-ML file with relative CellML file', () => { + const file = new loc.File(utils.resourcePath('api/sed/relative_cellml_file.sedml')); + + file.setContents(sedmlContents('../../cellml_2.cellml')); + + let document = new loc.SedDocument(file); + + assert.strictEqual(document.models.length, 1); + assert.strictEqual(document.models[0].file.path, utils.resourcePath('cellml_2.cellml')); + + const neededFile = new loc.File(utils.resourcePath('cellml_2.cellml')); + + document = new loc.SedDocument(file); + + assert.strictEqual(document.hasIssues, false); + assert.strictEqual(document.models.length, 1); + assert.strictEqual(document.models[0].file.isAliasOf(neededFile), true); + }); + + test('SED-ML file with relative CellML file in working directory', () => { + // A relative model source is relative to the SED-ML file, not to the current working directory, even if the + // current working directory contains a file with that name. + + const origDir = process.cwd(); + + process.chdir(utils.resourcePath()); + + const file = new loc.File(utils.resourcePath('api/sed/relative_cellml_file.sedml')); + + file.setContents(sedmlContents('cellml_2.cellml')); + + const document = new loc.SedDocument(file); + + process.chdir(origDir); + + assert.strictEqual(document.models.length, 1); + assert.strictEqual(document.models[0].file.path, utils.resourcePath('api/sed/cellml_2.cellml')); + }); + + test('Remote SED-ML file with relative CellML file', () => { + // A relative model source is relative to the remote SED-ML file, whatever the platform and even if the query and/or + // fragment of the SED-ML file's URL contain some forward slashes. + + const file = new loc.File('https://example.com/simulations/simulation.sedml?a=b/c#d/e'); + + file.setContents(sedmlContents('../models/./model.cellml')); + + const neededFile = new loc.File('https://example.com/models/model.cellml'); + const document = new loc.SedDocument(file); + + assert.strictEqual(document.hasIssues, false); + assert.strictEqual(document.models.length, 1); + assert.strictEqual(document.models[0].file.isAliasOf(neededFile), true); + }); + + test('Remote SED-ML file without path with relative CellML file', () => { + // A relative model source is relative to the root of a remote SED-ML file which URL has no path. + + const file = new loc.File('https://example.com?a=b/c'); + + file.setContents(sedmlContents('model.cellml')); + + const neededFile = new loc.File('https://example.com/model.cellml'); + const document = new loc.SedDocument(file); + + assert.strictEqual(document.hasIssues, false); + assert.strictEqual(document.models.length, 1); + assert.strictEqual(document.models[0].file.isAliasOf(neededFile), true); + }); + test('SED-ML file with remote CellML file', () => { const file = new loc.File(utils.resourcePath('api/sed/remote_cellml_file.sedml')); diff --git a/tests/bindings/python/test_file_basic.py b/tests/bindings/python/test_file_basic.py index e30468ce4..cb5cf899c 100644 --- a/tests/bindings/python/test_file_basic.py +++ b/tests/bindings/python/test_file_basic.py @@ -16,6 +16,8 @@ import libopencor as loc import os import platform +import tempfile +import threading import utils from utils import assert_issues @@ -80,6 +82,49 @@ def test_non_existing_relative_local_file(): assert_issues(file, expected_non_existing_file_issues) +def test_non_existing_relative_local_file_with_leading_parent_directories(): + file = loc.File("../models/./lorenz.cellml") + + assert file.type == loc.File.Type.IrretrievableFile + + if platform.system() == "Windows": + assert file.file_name == "..\\models\\lorenz.cellml" + else: + assert file.file_name == "../models/lorenz.cellml" + + assert file.url == "" + + if platform.system() == "Windows": + assert file.path == "..\\models\\lorenz.cellml" + else: + assert file.path == "../models/lorenz.cellml" + + assert file.contents == [] + assert_issues(file, expected_non_existing_file_issues) + + +def test_too_long_local_file_name(): + # A file name that is too long for the file system must not result in an exception being thrown. + + TOO_LONG_NAME_LENGTH = 5000 + + file = loc.File("/" + "a" * TOO_LONG_NAME_LENGTH) + + assert file.type == loc.File.Type.IrretrievableFile + assert file.contents == [] + assert_issues(file, expected_non_existing_file_issues) + + +def test_local_directory(): + # A directory is not a file, but it must not result in an exception being thrown. + + file = loc.File(utils.resource_path("api")) + + assert file.type == loc.File.Type.UnknownFile + assert file.contents == [] + assert_issues(file, expected_no_issues) + + def test_url_based_local_file(): file_path = utils.resource_path("file.txt") @@ -124,6 +169,78 @@ def test_encoded_remote_file(): assert file.contents != [] +def test_remote_file_with_dot_segments(): + file = loc.File("https://example.com/a/./b/../../c/model.cellml", False) + + assert file.url == "https://example.com/c/model.cellml" + assert file.path == "https://example.com/c/model.cellml" + + +def test_remote_file_matching_local_file(): + orig_dir = os.getcwd() + + with tempfile.TemporaryDirectory() as temp_dir: + os.makedirs(os.path.join(temp_dir, "example.com")) + open(os.path.join(temp_dir, "example.com", "model.cellml"), "w").close() + + os.chdir(temp_dir) + + url = loc.File("https://example.com/model.cellml", False).url + + os.chdir(orig_dir) + + assert url == "https://example.com/model.cellml" + + +def test_concurrent_file_creations(): + # Creating a file must never change the current working directory since it is shared by all the threads of the + # process. Also, the file manager must cope with files that get looked up while they are being destroyed by another + # thread. + + FILE_CREATION_COUNT = 1000 + + orig_dir = os.getcwd() + done = threading.Event() + working_directory_changed = False + + def check(): + nonlocal working_directory_changed + + file_manager = loc.FileManager.instance() + + while not done.is_set(): + if os.getcwd() != orig_dir: + working_directory_changed = True + + file_manager.file("https://example.com/model.cellml") + file_manager.files + + def create_files(): + for _ in range(FILE_CREATION_COUNT): + loc.File("https://example.com/model.cellml", False) + loc.File("non_existing_dir/../non_existing_file.txt", False) + + checker = threading.Thread(target=check) + + checker.start() + + thread1 = threading.Thread(target=create_files) + thread2 = threading.Thread(target=create_files) + + thread1.start() + thread2.start() + + thread1.join() + thread2.join() + + done.set() + + checker.join() + + assert not working_directory_changed + assert os.getcwd() == orig_dir + + def test_local_virtual_file(): file_path = utils.resource_path("unknown_file.txt") file = loc.File(file_path, False) diff --git a/tests/bindings/python/test_sed_basic.py b/tests/bindings/python/test_sed_basic.py index b5606d24b..48920ee1e 100644 --- a/tests/bindings/python/test_sed_basic.py +++ b/tests/bindings/python/test_sed_basic.py @@ -14,10 +14,21 @@ import libopencor as loc +import os import utils from utils import assert_issues +def sedml_contents(model_source): + return f""" + + + + + +""" + + def test_no_file(): document = loc.SedDocument() @@ -75,6 +86,78 @@ def test_sedml_file_with_absolute_cellml_file(): assert not document.has_issues +def test_sedml_file_with_relative_cellml_file(): + file = loc.File(utils.resource_path("api/sed/relative_cellml_file.sedml"), False) + + file.contents = utils.text_to_list(sedml_contents("../../cellml_2.cellml")) + + document = loc.SedDocument(file) + + assert len(document.models) == 1 + assert document.models[0].file.path == utils.resource_path("cellml_2.cellml") + + needed_file = loc.File(utils.resource_path("cellml_2.cellml")) + + document = loc.SedDocument(file) + + assert not document.has_issues + assert len(document.models) == 1 + assert document.models[0].file == needed_file + + +def test_sedml_file_with_relative_cellml_file_in_working_directory(): + # A relative model source is relative to the SED-ML file, not to the current working directory, even if the + # current working directory contains a file with that name. + + orig_dir = os.getcwd() + + os.chdir(utils.resource_path()) + + file = loc.File(utils.resource_path("api/sed/relative_cellml_file.sedml"), False) + + file.contents = utils.text_to_list(sedml_contents("cellml_2.cellml")) + + document = loc.SedDocument(file) + + os.chdir(orig_dir) + + assert len(document.models) == 1 + assert document.models[0].file.path == utils.resource_path( + "api/sed/cellml_2.cellml" + ) + + +def test_remote_sedml_file_with_relative_cellml_file(): + # A relative model source is relative to the remote SED-ML file, whatever the platform and even if the query and/or + # fragment of the SED-ML file's URL contain some forward slashes. + + file = loc.File("https://example.com/simulations/simulation.sedml?a=b/c#d/e", False) + + file.contents = utils.text_to_list(sedml_contents("../models/./model.cellml")) + + needed_file = loc.File("https://example.com/models/model.cellml", False) + document = loc.SedDocument(file) + + assert not document.has_issues + assert len(document.models) == 1 + assert document.models[0].file == needed_file + + +def test_remote_sedml_file_without_path_with_relative_cellml_file(): + # A relative model source is relative to the root of a remote SED-ML file which URL has no path. + + file = loc.File("https://example.com?a=b/c", False) + + file.contents = utils.text_to_list(sedml_contents("model.cellml")) + + needed_file = loc.File("https://example.com/model.cellml", False) + document = loc.SedDocument(file) + + assert not document.has_issues + assert len(document.models) == 1 + assert document.models[0].file == needed_file + + def test_sedml_file_with_remote_cellml_file(): file = loc.File(utils.resource_path("api/sed/remote_cellml_file.sedml")) document = loc.SedDocument(file) diff --git a/tests/misc/utils.cpp b/tests/misc/utils.cpp index b3d765341..73166aef0 100644 --- a/tests/misc/utils.cpp +++ b/tests/misc/utils.cpp @@ -41,3 +41,70 @@ TEST(UtilsTest, isInfOrNan) EXPECT_TRUE(libOpenCOR::isInfOrNan(-libOpenCOR::INF)); EXPECT_TRUE(libOpenCOR::isInfOrNan(libOpenCOR::NAN)); } + +TEST(UtilsTest, canonicalFileName) +{ + // Existing file. + + EXPECT_EQ(libOpenCOR::canonicalFileName(std::string(libOpenCOR::RESOURCE_LOCATION) + "/api/../cellml_2.cellml"), + libOpenCOR::resourcePath("cellml_2.cellml")); + + // Non-existing absolute file. + + EXPECT_EQ(libOpenCOR::canonicalFileName(std::string(libOpenCOR::RESOURCE_LOCATION) + "/non/existing/../../file.txt"), + libOpenCOR::resourcePath("file.txt")); + + // Non-existing relative files, which must be normalised lexically (i.e. without using the current working directory + // and while keeping leading ".." components). + +#ifdef BUILDING_ON_WINDOWS + EXPECT_EQ(libOpenCOR::canonicalFileName(R"(some\.\relative\..\..\path\.\..\dir\file.txt)"), R"(dir\file.txt)"); + EXPECT_EQ(libOpenCOR::canonicalFileName("some/./relative/../../path/./../dir/file.txt"), R"(dir\file.txt)"); + EXPECT_EQ(libOpenCOR::canonicalFileName("../models/lorenz.cellml"), R"(..\models\lorenz.cellml)"); + EXPECT_EQ(libOpenCOR::canonicalFileName("a/../../b"), R"(..\b)"); + EXPECT_EQ(libOpenCOR::canonicalFileName("../../a/./b/../c"), R"(..\..\a\c)"); +#else + EXPECT_EQ(libOpenCOR::canonicalFileName("some/./relative/../../path/./../dir/file.txt"), "dir/file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName("../models/lorenz.cellml"), "../models/lorenz.cellml"); + EXPECT_EQ(libOpenCOR::canonicalFileName("a/../../b"), "../b"); + EXPECT_EQ(libOpenCOR::canonicalFileName("../../a/./b/../c"), "../../a/c"); +#endif + EXPECT_EQ(libOpenCOR::canonicalFileName("non_existing_file.txt"), "non_existing_file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName(""), ""); + + // File names that are too long for the file system. + + static const std::string TOO_LONG_NAME(5000, 'a'); + +#ifndef BUILDING_ON_WINDOWS + EXPECT_EQ(libOpenCOR::canonicalFileName("/" + TOO_LONG_NAME), "/" + TOO_LONG_NAME); + EXPECT_EQ(libOpenCOR::canonicalFileName("/" + TOO_LONG_NAME + "/../file.txt"), "/file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName(TOO_LONG_NAME + "/../file.txt"), "file.txt"); +#endif + EXPECT_NO_THROW(libOpenCOR::canonicalFileName("/" + TOO_LONG_NAME)); + EXPECT_NO_THROW(libOpenCOR::canonicalFileName(TOO_LONG_NAME)); +} + +TEST(UtilsTest, canonicalUrl) +{ + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com"), "https://example.com"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/"), "https://example.com/"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/model.cellml"), "https://example.com/model.cellml"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/./b/../model.cellml"), "https://example.com/a/model.cellml"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/b/c/./../../g"), "https://example.com/a/g"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/b/.."), "https://example.com/a/"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/b/."), "https://example.com/a/b/"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/b/./"), "https://example.com/a/b/"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/.."), "https://example.com/"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/../../model.cellml"), "https://example.com/model.cellml"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a//b/../model.cellml"), "https://example.com/a/model.cellml"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/..b/.c/model.cellml"), "https://example.com/a/..b/.c/model.cellml"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com/a/../model.cellml?b=../c#d/../e"), "https://example.com/model.cellml?b=../c#d/../e"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com?a=/../b"), "https://example.com?a=/../b"); + EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com#a/../b"), "https://example.com#a/../b"); + EXPECT_EQ(libOpenCOR::canonicalUrl("http://user@example.com:8080/a/../model.cellml"), "http://user@example.com:8080/model.cellml"); + EXPECT_EQ(libOpenCOR::canonicalUrl("example.com/a/../model.cellml"), "example.com/model.cellml"); +#ifdef BUILDING_USING_MSVC + EXPECT_EQ(libOpenCOR::canonicalUrl(R"(https://example.com\a\..\model.cellml)"), "https://example.com/model.cellml"); +#endif +}