diff --git a/src/file/file.cpp b/src/file/file.cpp index 34a9af779..789f046fc 100644 --- a/src/file/file.cpp +++ b/src/file/file.cpp @@ -52,7 +52,7 @@ File::Impl::Impl(const std::string &pFileNameOrUrl, bool pRetrieveContents) } else { mFilePath = stringToPath("/some/path/file"); } - } else if (!std::filesystem::exists(mFilePath)) { + } else if (!std::filesystem::exists(mFilePath) && pRetrieveContents) { mType = Type::IRRETRIEVABLE_FILE; addError("The file does not exist."); @@ -87,30 +87,32 @@ void File::Impl::checkType(const FilePtr &pOwner, bool pResetType) #endif } - // Try to get a CellML file, a SED-ML file, or a COMBINE archive. + // Try to get a CellML file, a SED-ML file, or a COMBINE archive, but only if we have some contents. - mCellmlFile = CellmlFile::create(pOwner); + if (!contents().empty()) { + mCellmlFile = CellmlFile::create(pOwner); - if (mCellmlFile != nullptr) { - mType = Type::CELLML_FILE; + if (mCellmlFile != nullptr) { + mType = Type::CELLML_FILE; - addIssues(mCellmlFile); - } else { - mSedmlFile = SedmlFile::create(pOwner); - - if (mSedmlFile != nullptr) { - mType = Type::SEDML_FILE; - - addIssues(mSedmlFile); + addIssues(mCellmlFile); } else { - mCombineArchive = CombineArchive::create(pOwner); + mSedmlFile = SedmlFile::create(pOwner); - if (mCombineArchive != nullptr) { - mType = Type::COMBINE_ARCHIVE; + if (mSedmlFile != nullptr) { + mType = Type::SEDML_FILE; - addIssues(mCombineArchive); + addIssues(mSedmlFile); } else { - addError("The file is not a CellML file, a SED-ML file, or a COMBINE archive."); + mCombineArchive = CombineArchive::create(pOwner); + + if (mCombineArchive != nullptr) { + mType = Type::COMBINE_ARCHIVE; + + addIssues(mCombineArchive); + } else { + addError("The file is not a CellML file, a SED-ML file, or a COMBINE archive."); + } } } } diff --git a/src/support/cellml/cellmlfile.cpp b/src/support/cellml/cellmlfile.cpp index 57a65649f..866ad360e 100644 --- a/src/support/cellml/cellmlfile.cpp +++ b/src/support/cellml/cellmlfile.cpp @@ -154,16 +154,11 @@ CellmlFilePtr CellmlFile::create(const FilePtr &pFile) // Try to parse the file contents as a CellML file, be it a CellML 1.x or a CellML 2.0 file. - auto fileContents = pFile->contents(); + auto parser = libcellml::Parser::create(false); + auto model = parser->parseModel(toString(pFile->contents())); - if (!fileContents.empty() && (fileContents[0] != '\0')) { - auto contents = toString(fileContents); - auto parser = libcellml::Parser::create(false); - auto model = parser->parseModel(contents); - - if (parser->errorCount() == 0) { - return CellmlFilePtr {new CellmlFile {pFile, model, false}}; - } + if (parser->errorCount() == 0) { + return CellmlFilePtr {new CellmlFile {pFile, model, false}}; } return {}; diff --git a/src/support/sedml/sedmlfile.cpp b/src/support/sedml/sedmlfile.cpp index eecb69042..69cc9ad97 100644 --- a/src/support/sedml/sedmlfile.cpp +++ b/src/support/sedml/sedmlfile.cpp @@ -289,26 +289,22 @@ SedmlFilePtr SedmlFile::create(const FilePtr &pFile) // Try to retrieve a SED-ML document. - auto fileContents = pFile->contents(); - - if (!fileContents.empty() && (fileContents[0] != '\0')) { - auto *document = libsedml::readSedMLFromString(toString(fileContents).c_str()); - - // A non-SED-ML file results in our SED-ML document having at least one error. That error may be the result of a - // malformed XML file (e.g., an HTML file is an XML-like file but not actually an XML file or a COMBINE archive - // which is just not an XML file) or a valid XML file but not a SED-ML file (e.g., a CellML file is an XML file - // but not a SED-ML file). So, we use these facts to determine whether our current SED-ML document is indeed a - // SED-ML file. - - if ((document->getNumErrors() == 0) - || ((document->getError(0)->getErrorId() > libsbml::XMLErrorCodesUpperBound) - && (document->getError(0)->getErrorId() != libsedml::SedNotSchemaConformant))) { - return SedmlFilePtr {new SedmlFile {pFile, document}}; - } - - delete document; + auto *document = libsedml::readSedMLFromString(toString(pFile->contents()).c_str()); + + // A non-SED-ML file results in our SED-ML document having at least one error. That error may be the result of a + // malformed XML file (e.g., an HTML file is an XML-like file but not actually an XML file or a COMBINE archive + // which is just not an XML file) or a valid XML file but not a SED-ML file (e.g., a CellML file is an XML file but + // not a SED-ML file). So, we use these facts to determine whether our current SED-ML document is indeed a SED-ML + // file. + + if ((document->getNumErrors() == 0) + || ((document->getError(0)->getErrorId() > libsbml::XMLErrorCodesUpperBound) + && (document->getError(0)->getErrorId() != libsedml::SedNotSchemaConformant))) { + return SedmlFilePtr {new SedmlFile {pFile, document}}; } + delete document; + return {}; } diff --git a/tests/api/file/basictests.cpp b/tests/api/file/basictests.cpp index 467943248..00466beda 100644 --- a/tests/api/file/basictests.cpp +++ b/tests/api/file/basictests.cpp @@ -113,7 +113,7 @@ TEST(BasicFileTest, localVirtualFile) EXPECT_EQ(file->url(), ""); EXPECT_EQ(file->path(), libOpenCOR::resourcePath(libOpenCOR::UNKNOWN_FILE)); EXPECT_TRUE(file->contents().empty()); - EXPECT_EQ_ISSUES(file, EXPECTED_UNKNOWN_FILE_ISSUES); + EXPECT_EQ_ISSUES(file, EXPECTED_NO_ISSUES); auto someUnknownContents = libOpenCOR::charArrayToUnsignedChars(libOpenCOR::UNKNOWN_CONTENTS); @@ -137,7 +137,7 @@ TEST(BasicFileTest, remoteVirtualFile) 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_UNKNOWN_FILE_ISSUES); + EXPECT_EQ_ISSUES(file, EXPECTED_NO_ISSUES); auto someUnknownContents = libOpenCOR::charArrayToUnsignedChars(libOpenCOR::UNKNOWN_CONTENTS); diff --git a/tests/api/file/coveragetests.cpp b/tests/api/file/coveragetests.cpp index d70fd7061..6700b8015 100644 --- a/tests/api/file/coveragetests.cpp +++ b/tests/api/file/coveragetests.cpp @@ -43,6 +43,13 @@ TEST(CoverageFileTest, sedmlFileWithNoParent) file->setContents(libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SEDML_CONTENTS)); } +TEST(CoverageFileTest, irretrievableVirtualFile) +{ + auto file = libOpenCOR::File::create(libOpenCOR::IRRETRIEVABLE_FILE, false); + + EXPECT_FALSE(file->hasIssues()); +} + TEST(CoverageFileTest, irretrievableRemoteFile) { libOpenCOR::File::create(libOpenCOR::IRRETRIEVABLE_REMOTE_FILE); diff --git a/tests/bindings/javascript/file.basic.test.js b/tests/bindings/javascript/file.basic.test.js index eb7324a13..e0d3b5082 100644 --- a/tests/bindings/javascript/file.basic.test.js +++ b/tests/bindings/javascript/file.basic.test.js @@ -23,6 +23,7 @@ import { assertIssues } from './utils.js'; const loc = await libOpenCOR(); +const expectedNoIssues = []; const expectedUnknownFileIssues = [ [loc.Issue.Type.ERROR, 'The file is not a CellML file, a SED-ML file, or a COMBINE archive.'] ]; @@ -50,7 +51,7 @@ test.describe('File basic tests', () => { assert.strictEqual(file.url, ''); assert.strictEqual(file.path, utils.LOCAL_FILE); assert.deepStrictEqual(file.contents(), utils.NO_CONTENTS); - assertIssues(loc, file, expectedUnknownFileIssues); + assertIssues(loc, file, expectedNoIssues); file.setContents(unknownContentsPtr, utils.UNKNOWN_CONTENTS.length); @@ -67,7 +68,7 @@ test.describe('File basic tests', () => { assert.strictEqual(file.url, utils.REMOTE_FILE); assert.strictEqual(file.path, utils.REMOTE_FILE); assert.deepStrictEqual(file.contents(), utils.NO_CONTENTS); - assertIssues(loc, file, expectedUnknownFileIssues); + assertIssues(loc, file, expectedNoIssues); file.setContents(unknownContentsPtr, utils.UNKNOWN_CONTENTS.length); diff --git a/tests/bindings/python/test_file_basic.py b/tests/bindings/python/test_file_basic.py index 055bba1c0..757c6bc40 100644 --- a/tests/bindings/python/test_file_basic.py +++ b/tests/bindings/python/test_file_basic.py @@ -112,7 +112,7 @@ def test_local_virtual_file(): assert file.url == "" assert file.path == utils.resource_path(utils.UnknownFile) assert file.contents == [] - assert_issues(file, expected_unknown_file_issues) + assert_issues(file, expected_no_issues) some_unknown_contents_list = utils.string_to_list(utils.SomeUnknownContents) @@ -136,7 +136,7 @@ def test_remote_virtual_file(): assert file.url == utils.UnknownRemoteFile assert file.path == utils.UnknownRemoteFile assert file.contents == [] - assert_issues(file, expected_unknown_file_issues) + assert_issues(file, expected_no_issues) some_unknown_contents_list = utils.string_to_list(utils.SomeUnknownContents) diff --git a/tests/bindings/python/test_file_coverage.py b/tests/bindings/python/test_file_coverage.py index 6318d7d30..53cf1090b 100644 --- a/tests/bindings/python/test_file_coverage.py +++ b/tests/bindings/python/test_file_coverage.py @@ -39,6 +39,12 @@ def test_sedml_file_with_no_parent(): file.contents = utils.string_to_list(utils.SomeSedmlContents) +def test_irretrievable_virtual_file(): + file = loc.File(utils.IrretrievableFile, False) + + assert not file.has_issues + + def test_irretrievable_remote_file(): loc.File(utils.IrretrievableRemoteFile)