OpenXMLFile return value for not existing files or parsing errors (#463)

* OpenXMLFile: return error if file does not exist instead of xmlobject in error state * added docs * translate every xml open/parsing error to BError * updated core changes entry, to reflect the latest changes
polserver · Jun 21, 2022 · 737db93 · 737db93
1 parent 458356f
commit 737db93
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 3 deletions.
diff --git a/docs/docs.polserver.com/pol100/corechanges.xml b/docs/docs.polserver.com/pol100/corechanges.xml
@@ -2,9 +2,15 @@
 <ESCRIPT>
 	<header>
 		<topic>Latest Core Changes</topic>
-		<datemodified>06-16-2022</datemodified>
+		<datemodified>06-21-2022</datemodified>
 	</header>
 	<version name="POL100.1.0">
+		<entry>
+			<date>06-21-2022</date>
+			<author>Turley:</author>
+			<change type="Changed">OpenXMLFile(file) returns Error if file does not exists, or any parsing error occurs.<br/>
+Before it returned XMLRef in an error state, so 'if (!xml)' was and is a way to check for success.</change>
+		</entry>
 		<entry>
 			<date>06-16-2022</date>
 			<author>Kevin:</author>

diff --git a/pol-core/doc/core-changes.txt b/pol-core/doc/core-changes.txt
@@ -1,4 +1,7 @@
 -- POL100.1.0 --
+06-21-2022 Turley:
+  Changed: OpenXMLFile(file) returns Error if file does not exists, or any parsing error occurs.
+           Before it returned XMLRef in an error state, so 'if (!xml)' was and is a way to check for success.
 06-16-2022 Kevin:
     Fixed: Corrected objref documentation on Character.acct, Character.acctname, and Client.acctname.
     Added: client.acct member to return Account object

diff --git a/pol-core/pol/module/filemod.cpp b/pol-core/pol/module/filemod.cpp
@@ -678,8 +678,12 @@ Bscript::BObjectImp* FileAccessExecutorModule::mf_OpenXMLFile()
     filepath = path;
   else
     filepath = outpkg->dir() + path;
-
-  return new Core::BXMLfile( filepath );
+  if ( !Clib::FileExists( filepath ) )
+    return new BError( "File does not exist" );
+  std::unique_ptr<Core::BXMLfile> xml( new Core::BXMLfile( filepath ) );
+  if ( !xml->isTrue() )
+    return new BError( xml->getStringRep() );
+  return xml.release();
 }
 
 Bscript::BObjectImp* FileAccessExecutorModule::mf_CreateXMLFile()

diff --git a/testsuite/escript/filemod/xml.out b/testsuite/escript/filemod/xml.out
@@ -17,3 +17,7 @@ attributes= 2
 scopetest...
 root= root
 firstchild= Hello
+not existing file...
+Error
+invalid file...
+Error
diff --git a/testsuite/escript/filemod/xml.src b/testsuite/escript/filemod/xml.src
@@ -7,6 +7,10 @@ program XMLTest()
 	membertest();
 	print("scopetest...");
 	scopetest();
+	print("not existing file...");
+	notexistingtest();
+	print("invalid file...");
+	invalidtest();
 endprogram
 
 function itertest()
@@ -51,3 +55,14 @@ function scopeloose()
 	var xml:=OpenXMLFile("filemod/xmltest.xml");
 	return xml[2].clonenode();
 endfunction
+
+
+function notexistingtest()
+    var xml:=OpenXMLFile("filemod/xmltestnotexisting.xml");
+    print(typeof(xml));
+endfunction
+
+function invalidtest()
+    var xml:=OpenXMLFile("filemod/xmlinvalid.xml");
+    print(typeof(xml));
+endfunction
diff --git a/testsuite/escript/filemod/xmlinvalid.xml b/testsuite/escript/filemod/xmlinvalid.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" ?>
+<root>
+	<Hello>World</He>
+</root>