Skip to content

Commit 04b38e4

Browse files
agarnyagarny
authored
File: don't add The file does not exist. error if we create a (non-existing) file with pRetrieveContents set to false.
2 parents c771c03 + 29c57be commit 04b38e4

File tree

8 files changed

+58
-51
lines changed

8 files changed

+58
-51
lines changed

‎src/file/file.cpp‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ File::Impl::Impl(const std::string &pFileNameOrUrl, bool pRetrieveContents)
5252
} else {
5353
mFilePath = stringToPath("/some/path/file");
5454
}
55-
} else if (!std::filesystem::exists(mFilePath)) {
55+
} else if (!std::filesystem::exists(mFilePath) && pRetrieveContents) {
5656
mType = Type::IRRETRIEVABLE_FILE;
5757

5858
addError("The file does not exist.");
@@ -87,30 +87,32 @@ void File::Impl::checkType(const FilePtr &pOwner, bool pResetType)
8787
#endif
8888
}
8989

90-
// Try to get a CellML file, a SED-ML file, or a COMBINE archive.
90+
// Try to get a CellML file, a SED-ML file, or a COMBINE archive, but only if we have some contents.
9191

92-
mCellmlFile = CellmlFile::create(pOwner);
92+
if (!contents().empty()) {
93+
mCellmlFile = CellmlFile::create(pOwner);
9394

94-
if (mCellmlFile != nullptr) {
95-
mType = Type::CELLML_FILE;
95+
if (mCellmlFile != nullptr) {
96+
mType = Type::CELLML_FILE;
9697

97-
addIssues(mCellmlFile);
98-
} else {
99-
mSedmlFile = SedmlFile::create(pOwner);
100-
101-
if (mSedmlFile != nullptr) {
102-
mType = Type::SEDML_FILE;
103-
104-
addIssues(mSedmlFile);
98+
addIssues(mCellmlFile);
10599
} else {
106-
mCombineArchive = CombineArchive::create(pOwner);
100+
mSedmlFile = SedmlFile::create(pOwner);
107101

108-
if (mCombineArchive != nullptr) {
109-
mType = Type::COMBINE_ARCHIVE;
102+
if (mSedmlFile != nullptr) {
103+
mType = Type::SEDML_FILE;
110104

111-
addIssues(mCombineArchive);
105+
addIssues(mSedmlFile);
112106
} else {
113-
addError("The file is not a CellML file, a SED-ML file, or a COMBINE archive.");
107+
mCombineArchive = CombineArchive::create(pOwner);
108+
109+
if (mCombineArchive != nullptr) {
110+
mType = Type::COMBINE_ARCHIVE;
111+
112+
addIssues(mCombineArchive);
113+
} else {
114+
addError("The file is not a CellML file, a SED-ML file, or a COMBINE archive.");
115+
}
114116
}
115117
}
116118
}

‎src/support/cellml/cellmlfile.cpp‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,16 +154,11 @@ CellmlFilePtr CellmlFile::create(const FilePtr &pFile)
154154

155155
// Try to parse the file contents as a CellML file, be it a CellML 1.x or a CellML 2.0 file.
156156

157-
auto fileContents = pFile->contents();
157+
auto parser = libcellml::Parser::create(false);
158+
auto model = parser->parseModel(toString(pFile->contents()));
158159

159-
if (!fileContents.empty() && (fileContents[0] != '\0')) {
160-
auto contents = toString(fileContents);
161-
auto parser = libcellml::Parser::create(false);
162-
auto model = parser->parseModel(contents);
163-
164-
if (parser->errorCount() == 0) {
165-
return CellmlFilePtr {new CellmlFile {pFile, model, false}};
166-
}
160+
if (parser->errorCount() == 0) {
161+
return CellmlFilePtr {new CellmlFile {pFile, model, false}};
167162
}
168163

169164
return {};

‎src/support/sedml/sedmlfile.cpp‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -289,26 +289,22 @@ SedmlFilePtr SedmlFile::create(const FilePtr &pFile)
289289

290290
// Try to retrieve a SED-ML document.
291291

292-
auto fileContents = pFile->contents();
293-
294-
if (!fileContents.empty() && (fileContents[0] != '\0')) {
295-
auto *document = libsedml::readSedMLFromString(toString(fileContents).c_str());
296-
297-
// A non-SED-ML file results in our SED-ML document having at least one error. That error may be the result of a
298-
// malformed XML file (e.g., an HTML file is an XML-like file but not actually an XML file or a COMBINE archive
299-
// 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
300-
// but not a SED-ML file). So, we use these facts to determine whether our current SED-ML document is indeed a
301-
// SED-ML file.
302-
303-
if ((document->getNumErrors() == 0)
304-
|| ((document->getError(0)->getErrorId() > libsbml::XMLErrorCodesUpperBound)
305-
&& (document->getError(0)->getErrorId() != libsedml::SedNotSchemaConformant))) {
306-
return SedmlFilePtr {new SedmlFile {pFile, document}};
307-
}
308-
309-
delete document;
292+
auto *document = libsedml::readSedMLFromString(toString(pFile->contents()).c_str());
293+
294+
// A non-SED-ML file results in our SED-ML document having at least one error. That error may be the result of a
295+
// malformed XML file (e.g., an HTML file is an XML-like file but not actually an XML file or a COMBINE archive
296+
// 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
297+
// not a SED-ML file). So, we use these facts to determine whether our current SED-ML document is indeed a SED-ML
298+
// file.
299+
300+
if ((document->getNumErrors() == 0)
301+
|| ((document->getError(0)->getErrorId() > libsbml::XMLErrorCodesUpperBound)
302+
&& (document->getError(0)->getErrorId() != libsedml::SedNotSchemaConformant))) {
303+
return SedmlFilePtr {new SedmlFile {pFile, document}};
310304
}
311305

306+
delete document;
307+
312308
return {};
313309
}
314310

‎tests/api/file/basictests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ TEST(BasicFileTest, localVirtualFile)
113113
EXPECT_EQ(file->url(), "");
114114
EXPECT_EQ(file->path(), libOpenCOR::resourcePath(libOpenCOR::UNKNOWN_FILE));
115115
EXPECT_TRUE(file->contents().empty());
116-
EXPECT_EQ_ISSUES(file, EXPECTED_UNKNOWN_FILE_ISSUES);
116+
EXPECT_EQ_ISSUES(file, EXPECTED_NO_ISSUES);
117117

118118
auto someUnknownContents = libOpenCOR::charArrayToUnsignedChars(libOpenCOR::UNKNOWN_CONTENTS);
119119

@@ -137,7 +137,7 @@ TEST(BasicFileTest, remoteVirtualFile)
137137
EXPECT_EQ(file->url(), libOpenCOR::UNKNOWN_REMOTE_FILE);
138138
EXPECT_EQ(file->path(), libOpenCOR::UNKNOWN_REMOTE_FILE);
139139
EXPECT_TRUE(file->contents().empty());
140-
EXPECT_EQ_ISSUES(file, EXPECTED_UNKNOWN_FILE_ISSUES);
140+
EXPECT_EQ_ISSUES(file, EXPECTED_NO_ISSUES);
141141

142142
auto someUnknownContents = libOpenCOR::charArrayToUnsignedChars(libOpenCOR::UNKNOWN_CONTENTS);
143143

‎tests/api/file/coveragetests.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ TEST(CoverageFileTest, sedmlFileWithNoParent)
4343
file->setContents(libOpenCOR::charArrayToUnsignedChars(libOpenCOR::SEDML_CONTENTS));
4444
}
4545

46+
TEST(CoverageFileTest, irretrievableVirtualFile)
47+
{
48+
auto file = libOpenCOR::File::create(libOpenCOR::IRRETRIEVABLE_FILE, false);
49+
50+
EXPECT_FALSE(file->hasIssues());
51+
}
52+
4653
TEST(CoverageFileTest, irretrievableRemoteFile)
4754
{
4855
libOpenCOR::File::create(libOpenCOR::IRRETRIEVABLE_REMOTE_FILE);

‎tests/bindings/javascript/file.basic.test.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { assertIssues } from './utils.js';
2323

2424
const loc = await libOpenCOR();
2525

26+
const expectedNoIssues = [];
2627
const expectedUnknownFileIssues = [
2728
[loc.Issue.Type.ERROR, 'The file is not a CellML file, a SED-ML file, or a COMBINE archive.']
2829
];
@@ -50,7 +51,7 @@ test.describe('File basic tests', () => {
5051
assert.strictEqual(file.url, '');
5152
assert.strictEqual(file.path, utils.LOCAL_FILE);
5253
assert.deepStrictEqual(file.contents(), utils.NO_CONTENTS);
53-
assertIssues(loc, file, expectedUnknownFileIssues);
54+
assertIssues(loc, file, expectedNoIssues);
5455

5556
file.setContents(unknownContentsPtr, utils.UNKNOWN_CONTENTS.length);
5657

@@ -67,7 +68,7 @@ test.describe('File basic tests', () => {
6768
assert.strictEqual(file.url, utils.REMOTE_FILE);
6869
assert.strictEqual(file.path, utils.REMOTE_FILE);
6970
assert.deepStrictEqual(file.contents(), utils.NO_CONTENTS);
70-
assertIssues(loc, file, expectedUnknownFileIssues);
71+
assertIssues(loc, file, expectedNoIssues);
7172

7273
file.setContents(unknownContentsPtr, utils.UNKNOWN_CONTENTS.length);
7374

‎tests/bindings/python/test_file_basic.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ def test_local_virtual_file():
112112
assert file.url == ""
113113
assert file.path == utils.resource_path(utils.UnknownFile)
114114
assert file.contents == []
115-
assert_issues(file, expected_unknown_file_issues)
115+
assert_issues(file, expected_no_issues)
116116

117117
some_unknown_contents_list = utils.string_to_list(utils.SomeUnknownContents)
118118

@@ -136,7 +136,7 @@ def test_remote_virtual_file():
136136
assert file.url == utils.UnknownRemoteFile
137137
assert file.path == utils.UnknownRemoteFile
138138
assert file.contents == []
139-
assert_issues(file, expected_unknown_file_issues)
139+
assert_issues(file, expected_no_issues)
140140

141141
some_unknown_contents_list = utils.string_to_list(utils.SomeUnknownContents)
142142

‎tests/bindings/python/test_file_coverage.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ def test_sedml_file_with_no_parent():
3939
file.contents = utils.string_to_list(utils.SomeSedmlContents)
4040

4141

42+
def test_irretrievable_virtual_file():
43+
file = loc.File(utils.IrretrievableFile, False)
44+
45+
assert not file.has_issues
46+
47+
4248
def test_irretrievable_remote_file():
4349
loc.File(utils.IrretrievableRemoteFile)
4450

0 commit comments

Comments
 (0)