File: don't retrieve the contents of a file which was requested to be created without retrieving its contents.

Author: agarny

src/file/file.cpp

@@ -23,6 +23,7 @@ namespace libOpenCOR {
 
 File::Impl::Impl(const std::string &pFileNameOrUrl, bool pRetrieveContents)
     : Logger::Impl()
+    , mRetrieveContents(pRetrieveContents)
 {
     // Check whether we are dealing with a local file or a URL.
 
@@ -140,7 +141,7 @@ UnsignedChars File::Impl::contents()
 #ifndef __EMSCRIPTEN__
     // Retrieve the contents of the file, if needed.
 
-    if (!mContentsRetrieved) {
+    if (mRetrieveContents && !mContentsRetrieved) {
         setContents(fileContents(mFilePath));
     }
 #endif

src/file/file_p.h

@@ -36,6 +36,7 @@ class File::Impl: public Logger::Impl
     std::filesystem::path mFilePath;
     std::string mUrl;
 
+    bool mRetrieveContents = true;
     bool mContentsRetrieved = false;
     UnsignedChars mContents;
 

src/misc/utils.cpp

@@ -76,8 +76,6 @@ std::string pathToString(const std::filesystem::path &pPath)
 #endif
 }
 
-namespace {
-
 #ifdef BUILDING_USING_MSVC
 std::string canonicalFileName(const std::string &pFileName, bool pIsRemoteFile)
 #else
@@ -145,8 +143,6 @@ std::string canonicalFileName(const std::string &pFileName)
     return res;
 }
 
-} // namespace
-
 std::tuple<bool, std::string> retrieveFileInfo(const std::string &pFileNameOrUrl)
 {
     // Check whether the given file name or URL is a local file name or a URL.

src/misc/utils.h

@@ -78,6 +78,11 @@ std::string forwardSlashPath(const std::string &pPath);
 std::filesystem::path LIBOPENCOR_UNIT_TESTING_EXPORT stringToPath(const std::string &pString);
 std::string LIBOPENCOR_UNIT_TESTING_EXPORT 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<bool, std::string> retrieveFileInfo(const std::string &pFileNameOrUrl);
 std::string relativePath(const std::string &pPath, const std::string &pBasePath);
 std::string urlPath(const std::string &pPath);

tests/api/file/basictests.cpp

@@ -23,9 +23,6 @@ static const libOpenCOR::ExpectedIssues EXPECTED_NO_ISSUES = {};
 static const libOpenCOR::ExpectedIssues EXPECTED_NON_EXISTING_FILE_ISSUES = {
     {libOpenCOR::Issue::Type::ERROR, "The file does not exist."},
 };
-static const libOpenCOR::ExpectedIssues EXPECTED_NON_DOWNLOADABLE_FILE_ISSUES = {
-    {libOpenCOR::Issue::Type::ERROR, "The file could not be downloaded."},
-};
 static const libOpenCOR::ExpectedIssues EXPECTED_UNKNOWN_FILE_ISSUES = {
     {libOpenCOR::Issue::Type::ERROR, "The file is not a CellML file, a SED-ML file, or a COMBINE archive."},
 };
@@ -42,7 +39,21 @@ TEST(BasicFileTest, localFile)
     EXPECT_EQ_ISSUES(file, EXPECTED_NON_EXISTING_FILE_ISSUES);
 }
 
-TEST(BasicFileTest, relativeLocalFile)
+TEST(BasicSedTest, existingRelativeLocalFile)
+{
+    auto origDir = std::filesystem::current_path();
+
+    std::filesystem::current_path(libOpenCOR::resourcePath());
+
+    auto file = libOpenCOR::File::create(libOpenCOR::CELLML_2_FILE);
+
+    EXPECT_FALSE(file->contents().empty());
+    EXPECT_EQ_ISSUES(file, EXPECTED_NO_ISSUES);
+
+    std::filesystem::current_path(origDir);
+}
+
+TEST(BasicFileTest, nonExistingRelativeLocalFile)
 {
 #ifdef BUILDING_ON_WINDOWS
     auto file = libOpenCOR::File::create(R"(some\.\relative\..\..\path\.\..\dir\file.txt)");
@@ -66,20 +77,6 @@ TEST(BasicFileTest, relativeLocalFile)
     EXPECT_EQ_ISSUES(file, EXPECTED_NON_EXISTING_FILE_ISSUES);
 }
 
-TEST(BasicSedTest, existingRelativeLocalFile)
-{
-    auto origDir = std::filesystem::current_path();
-
-    std::filesystem::current_path(libOpenCOR::resourcePath());
-
-    auto file = libOpenCOR::File::create(libOpenCOR::CELLML_2_FILE);
-
-    EXPECT_FALSE(file->contents().empty());
-    EXPECT_EQ_ISSUES(file, EXPECTED_NO_ISSUES);
-
-    std::filesystem::current_path(origDir);
-}
-
 TEST(BasicFileTest, urlBasedLocalFile)
 {
 #ifdef BUILDING_ON_WINDOWS
@@ -109,14 +106,14 @@ TEST(BasicFileTest, remoteFile)
 
 TEST(BasicFileTest, localVirtualFile)
 {
-    auto file = libOpenCOR::File::create(libOpenCOR::LOCAL_FILE);
+    auto file = libOpenCOR::File::create(libOpenCOR::resourcePath(libOpenCOR::UNKNOWN_FILE), false);
 
-    EXPECT_EQ(file->type(), libOpenCOR::File::Type::IRRETRIEVABLE_FILE);
-    EXPECT_EQ(file->fileName(), libOpenCOR::LOCAL_FILE);
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE);
+    EXPECT_EQ(file->fileName(), libOpenCOR::resourcePath(libOpenCOR::UNKNOWN_FILE));
     EXPECT_EQ(file->url(), "");
-    EXPECT_EQ(file->path(), libOpenCOR::LOCAL_FILE);
+    EXPECT_EQ(file->path(), libOpenCOR::resourcePath(libOpenCOR::UNKNOWN_FILE));
     EXPECT_TRUE(file->contents().empty());
-    EXPECT_EQ_ISSUES(file, EXPECTED_NON_EXISTING_FILE_ISSUES);
+    EXPECT_EQ_ISSUES(file, EXPECTED_UNKNOWN_FILE_ISSUES);
 
     auto someUnknownContents = libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SOME_UNKNOWN_CONTENTS);
 
@@ -129,14 +126,18 @@ TEST(BasicFileTest, localVirtualFile)
 
 TEST(BasicFileTest, remoteVirtualFile)
 {
-    auto file = libOpenCOR::File::create(libOpenCOR::IRRETRIEVABLE_REMOTE_FILE);
+    auto file = libOpenCOR::File::create(libOpenCOR::UNKNOWN_REMOTE_FILE, false);
 
-    EXPECT_EQ(file->type(), libOpenCOR::File::Type::IRRETRIEVABLE_FILE);
-    EXPECT_EQ(file->fileName(), "");
-    EXPECT_EQ(file->url(), libOpenCOR::IRRETRIEVABLE_REMOTE_FILE);
-    EXPECT_EQ(file->path(), libOpenCOR::IRRETRIEVABLE_REMOTE_FILE);
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE);
+#ifdef BUILDING_USING_MSVC
+    EXPECT_EQ(file->fileName(), "\\some\\path\\file");
+#else
+    EXPECT_EQ(file->fileName(), "/some/path/file");
+#endif
+    EXPECT_EQ(file->url(), libOpenCOR::UNKNOWN_REMOTE_FILE);
+    EXPECT_EQ(file->path(), libOpenCOR::UNKNOWN_REMOTE_FILE);
     EXPECT_TRUE(file->contents().empty());
-    EXPECT_EQ_ISSUES(file, EXPECTED_NON_DOWNLOADABLE_FILE_ISSUES);
+    EXPECT_EQ_ISSUES(file, EXPECTED_UNKNOWN_FILE_ISSUES);
 
     auto someUnknownContents = libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SOME_UNKNOWN_CONTENTS);
 

tests/api/file/typetests.cpp

@@ -101,7 +101,10 @@ TEST(TypeFileTest, combine2Archive)
 
 TEST(TypeFileTest, unknownVirtualFile)
 {
-    auto file = libOpenCOR::File::create(libOpenCOR::LOCAL_FILE);
+    auto file = libOpenCOR::File::create(libOpenCOR::resourcePath(libOpenCOR::UNKNOWN_FILE), false);
+
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE);
+    EXPECT_EQ(file->contents(), libOpenCOR::UnsignedChars());
 
     file->setContents(libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SOME_UNKNOWN_CONTENTS));
 
@@ -111,7 +114,10 @@ TEST(TypeFileTest, unknownVirtualFile)
 
 TEST(TypeFileTest, cellmlVirtualFile)
 {
-    auto file = libOpenCOR::File::create(libOpenCOR::LOCAL_FILE);
+    auto file = libOpenCOR::File::create(libOpenCOR::resourcePath(libOpenCOR::CELLML_2_FILE), false);
+
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE);
+    EXPECT_EQ(file->contents(), libOpenCOR::UnsignedChars());
 
     file->setContents(libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SOME_CELLML_CONTENTS));
 
@@ -120,7 +126,10 @@ TEST(TypeFileTest, cellmlVirtualFile)
 
 TEST(TypeFileTest, sedmlVirtualFile)
 {
-    auto file = libOpenCOR::File::create(libOpenCOR::LOCAL_FILE);
+    auto file = libOpenCOR::File::create(libOpenCOR::resourcePath(libOpenCOR::SEDML_2_FILE), false);
+
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE);
+    EXPECT_EQ(file->contents(), libOpenCOR::UnsignedChars());
 
     file->setContents(libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SOME_SEDML_CONTENTS));
 
@@ -129,7 +138,10 @@ TEST(TypeFileTest, sedmlVirtualFile)
 
 TEST(TypeFileTest, combineVirtualArchive)
 {
-    auto file = libOpenCOR::File::create(libOpenCOR::LOCAL_FILE);
+    auto file = libOpenCOR::File::create(libOpenCOR::resourcePath(libOpenCOR::COMBINE_2_ARCHIVE), false);
+
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::UNKNOWN_FILE);
+    EXPECT_EQ(file->contents(), libOpenCOR::UnsignedChars());
 
     file->setContents(libOpenCOR::base64Decode(libOpenCOR::SOME_BASE64_COMBINE_ARCHIVE_CONTENTS));
 

tests/bindings/python/test_file_basic.py

@@ -24,9 +24,6 @@
 expected_non_existing_file_issues = [
     [oc.Issue.Type.Error, "The file does not exist."],
 ]
-expected_non_downloadable_file_issues = [
-    [oc.Issue.Type.Error, "The file could not be downloaded."],
-]
 expected_unknown_file_issues = [
     [
         oc.Issue.Type.Error,
@@ -46,7 +43,20 @@ def test_local_file():
     assert_issues(file, expected_non_existing_file_issues)
 
 
-def test_relative_local_file():
+def test_existing_relative_local_file():
+    orig_dir = os.getcwd()
+
+    os.chdir(utils.resource_path())
+
+    file = oc.File(utils.Cellml2File)
+
+    assert file.contents != []
+    assert_issues(file, expected_no_issues)
+
+    os.chdir(orig_dir)
+
+
+def test_non_existing_relative_local_file():
     if platform.system() == "Windows":
         file = oc.File("some\\.\\relative\\..\\..\\path\\.\\..\\dir\\file.txt")
     else:
@@ -70,19 +80,6 @@ def test_relative_local_file():
     assert_issues(file, expected_non_existing_file_issues)
 
 
-def test_existing_relative_local_file():
-    orig_dir = os.getcwd()
-
-    os.chdir(utils.resource_path())
-
-    file = oc.File(utils.Cellml2File)
-
-    assert file.contents != []
-    assert_issues(file, expected_no_issues)
-
-    os.chdir(orig_dir)
-
-
 def test_url_based_local_file():
     if platform.system() == "Windows":
         file = oc.File("file:///P:/some/path/file.txt")
@@ -108,14 +105,14 @@ def test_remote_file():
 
 
 def test_local_virtual_file():
-    file = oc.File(utils.LocalFile)
+    file = oc.File(utils.resource_path(utils.UnknownFile), False)
 
-    assert file.type == oc.File.Type.IrretrievableFile
-    assert file.file_name == utils.LocalFile
+    assert file.type == oc.File.Type.UnknownFile
+    assert file.file_name == utils.resource_path(utils.UnknownFile)
     assert file.url == ""
-    assert file.path == utils.LocalFile
+    assert file.path == utils.resource_path(utils.UnknownFile)
     assert file.contents == []
-    assert_issues(file, expected_non_existing_file_issues)
+    assert_issues(file, expected_unknown_file_issues)
 
     some_unknown_contents_list = utils.string_to_list(utils.SomeUnknownContents)
 
@@ -127,14 +124,19 @@ def test_local_virtual_file():
 
 
 def test_remote_virtual_file():
-    file = oc.File(utils.IrretrievableRemoteFile)
+    file = oc.File(utils.UnknownRemoteFile, False)
 
-    assert file.type == oc.File.Type.IrretrievableFile
-    assert file.file_name == ""
-    assert file.url == utils.IrretrievableRemoteFile
-    assert file.path == utils.IrretrievableRemoteFile
+    assert file.type == oc.File.Type.UnknownFile
+
+    if platform.system() == "Windows":
+        assert file.file_name == "\\some\\path\\file"
+    else:
+        assert file.file_name == "/some/path/file"
+
+    assert file.url == utils.UnknownRemoteFile
+    assert file.path == utils.UnknownRemoteFile
     assert file.contents == []
-    assert_issues(file, expected_non_downloadable_file_issues)
+    assert_issues(file, expected_unknown_file_issues)
 
     some_unknown_contents_list = utils.string_to_list(utils.SomeUnknownContents)
 

tests/bindings/python/test_file_type.py

@@ -94,7 +94,10 @@ def test_type_combine_2_archive():
 
 
 def test_type_unknown_virtual_file():
-    file = oc.File(utils.LocalFile)
+    file = oc.File(utils.resource_path(utils.UnknownFile), False)
+
+    assert file.type == oc.File.Type.UnknownFile
+    assert file.contents == []
 
     file.contents = utils.string_to_list(utils.SomeUnknownContents)
 
@@ -103,23 +106,32 @@ def test_type_unknown_virtual_file():
 
 
 def test_type_cellml_virtual_file():
-    file = oc.File(utils.LocalFile)
+    file = oc.File(utils.resource_path(utils.Cellml2File), False)
+
+    assert file.type == oc.File.Type.UnknownFile
+    assert file.contents == []
 
     file.contents = utils.string_to_list(utils.SomeCellmlContents)
 
     assert file.type == oc.File.Type.CellmlFile
 
 
 def test_type_sedml_virtual_file():
-    file = oc.File(utils.LocalFile)
+    file = oc.File(utils.resource_path(utils.Sedml2File), False)
+
+    assert file.type == oc.File.Type.UnknownFile
+    assert file.contents == []
 
     file.contents = utils.string_to_list(utils.SomeSedmlContents)
 
     assert file.type == oc.File.Type.SedmlFile
 
 
 def test_type_combine_virtual_archive():
-    file = oc.File(utils.LocalFile)
+    file = oc.File(utils.resource_path(utils.Combine2Archive), False)
+
+    assert file.type == oc.File.Type.UnknownFile
+    assert file.contents == []
 
     file.contents = utils.binary_to_list(utils.SomeCombineArchiveContents)
 

tests/bindings/python/utils.py.in

@@ -15,6 +15,7 @@
 
 import base64
 import libopencor as oc
+import os
 import pathlib
 import platform
 import pytest
@@ -50,6 +51,7 @@ HttpRemoteCombineArchive = (
 )
 RemoteBasePath = "https://raw.githubusercontent.com/opencor/libopencor/master/tests/res"
 RemoteFile = "https://raw.githubusercontent.com/opencor/libopencor/master/tests/res/cellml_2.cellml"
+UnknownRemoteFile = "https://raw.githubusercontent.com/opencor/libopencor/master/tests/res/unknown_file.txt"
 IrretrievableRemoteFile = "https://some.domain.com/irretrievable_file.txt"
 
 
@@ -118,7 +120,7 @@ def assert_values(
 
 
 def resource_path(relative_path=""):
-    return ResourceLocation + "/" + relative_path
+    return os.path.realpath(ResourceLocation + "/" + relative_path)
 
 
 def string_to_list(string):