Quality 137/improve serialization deserialization (#142)

* general improvements to file sink and source

* fix exceptions, improve file sink constructors

* add auxiliary functions for binary read/write

* replace  {Read/Write}WithThrow in coord and cavity

* replace Read/WriteWithThrow with bin_read/write in rbcalcgrid

* replace Read/WriteWithThrow with bin_read/write in rbconvgrid

* replace Read/WriteWithThrow with bin_read/write in base grid

* update rbconvgrid with the specialized bin_read/write for strings

* replace Read/WriteWithThrow with bin_read/write in fft grid

* replace Read/WriteWithThrow with bin_read/write in pmf grid

* add modifications to bin_read/write string specialization for retrocompatibility

* replace Read/WriteWithThrow with bin_read/write in real grid

* replace Read/WriteWithThrow with bin_read/write in vdw grid

* replace Read/WriteWithThrow with bin_read/write in docking site

* remove original read/write with throw

* add diff test for cavity files

* refactor out title validation

* add test dependency for  centos stream 9

.github/docker/Dockerfile.centos-stream9

@@ -1,4 +1,4 @@
 FROM quay.io/centos/centos:stream9 as base
 
 # Install dependencies
-RUN yum install -y popt-devel gcc-c++ make
+RUN yum install -y popt-devel gcc-c++ make diffutils

.gitignore

@@ -12,3 +12,4 @@ lib/libRbt.so
 # test files
 *.as
 tests/results
+!tests/data/1YET_reference_out.as

Makefile

@@ -166,6 +166,7 @@ test_dock_run: build tests/data/1YET_test.as
 	@tests/scripts/check_results.sh ./tests/data/1YET_reference_out.sd ./tests/results/1YET_test_out.sd
 
 test_rbcavity: tests/data/1koc.as tests/data/1YET.as tests/data/1YET_test.as
+	diff -q tests/data/1YET_test.as tests/data/1YET_reference_out.as
 
 test_suite: build_tests
 	LD_LIBRARY_PATH=./lib tests/bin/test_suite
@@ -180,7 +181,9 @@ clean_lib: ## removes the compiled library file, libRbt.so
 	@rm -f lib/libRbt.so
 
 clean_tests: ## removes the files generated by test execution
+	@mv tests/data/1YET_reference_out.as tests/data/1YET_reference_out.as.bak
 	@rm -rf tests/results tests/data/*.as
+	@mv tests/data/1YET_reference_out.as.bak tests/data/1YET_reference_out.as
 
 clean_tests_objects: ## removes the object files generated by the tests
 	@rm -rf tests/obj

include/Rbt.h

@@ -105,20 +105,7 @@ RbtString ConvertListToDelimitedString(const RbtStringList& listOfValues, const
 // DM 19 Feb 1999 - include executable information
 std::ostream& PrintStdHeader(std::ostream& s, const RbtString& strExecutable = "");
 
-// Helper functions to read/write chars from iostreams
-// Throws error if stream state is not Good() before and after the read/write
-// It appears the STL ios_base exception throwing is not yet implemented
-// at least on RedHat 6.1, so this is a temporary workaround (yeah right)
-//
-// DM 25 Sep 2000 - with MIPSPro CC compiler, streamsize is not defined,
-// at least with the iostreams library we are using, so typedef it here
-//  it is not in gcc 3.2.1 either SJ
-// #ifdef __sgi
-typedef int streamsize;
-// #endif
-
-void WriteWithThrow(std::ostream& ostr, const char* p, streamsize n);
-void ReadWithThrow(std::istream& istr, char* p, streamsize n);
+void ValidateTitle(std::istream& istr, const std::string& className);
 
 }  // namespace Rbt
 

include/RbtBinaryIO.h

@@ -0,0 +1,70 @@
+
+#ifndef _RBT_BINARY_IO_H_
+#define _RBT_BINARY_IO_H_
+
+#include <iostream>
+
+#include "RbtError.h"
+#include "RbtFileError.h"
+
+namespace Rbt {
+
+template <typename T>
+inline void bin_write(std::ostream& ostr, const T& data) {
+    try {
+        ostr.write(reinterpret_cast<const char*>(&data), sizeof(T));
+    } catch (std::ios_base::failure& e) {
+        throw RbtFileWriteError(_WHERE_, e.what());
+    }
+}
+
+template <typename T>
+inline void bin_write(std::ostream& ostr, const T* data, size_t n) {
+    try {
+        ostr.write(reinterpret_cast<const char*>(data), n * sizeof(T));
+    } catch (std::ios_base::failure& e) {
+        throw RbtFileWriteError(_WHERE_, e.what());
+    }
+}
+
+template <typename T>
+inline void bin_read(std::istream& istr, T& data) {
+    try {
+        istr.read(reinterpret_cast<char*>(&data), sizeof(T));
+    } catch (std::ios_base::failure& e) {
+        throw RbtFileReadError(_WHERE_, e.what());
+    }
+}
+
+template <typename T>
+inline void bin_read(std::istream& istr, T* data, size_t n) {
+    try {
+        istr.read(reinterpret_cast<char*>(data), n * sizeof(T));
+
+    } catch (std::ios_base::failure& e) {
+        throw RbtFileReadError(_WHERE_, e.what());
+    }
+}
+
+template <>
+inline void bin_write(std::ostream& ostr, const std::string& data) {
+    // casting size_t to int for retro-compatibility with old code only.
+    // This is not the best way to do it and will be modified in the near future
+    bin_write(ostr, (int)(data.size()));
+    bin_write(ostr, data.data(), data.size());
+}
+
+template <>
+inline void bin_read(std::istream& istr, std::string& data) {
+    // similar to the bin_write(std::ostream& ostr, const std::string& data) function
+    // this is a temporary solution for retro-compatibility with old code
+    // the size_t is casted to int
+    int size;
+    bin_read(istr, size);
+    data.resize(size);
+    bin_read(istr, data.data(), size);
+}
+
+}  // namespace Rbt
+
+#endif  // _RBT_BINARY_IO_H_

include/RbtCavity.h

@@ -16,6 +16,7 @@
 #ifndef _RBTCAVITY_H_
 #define _RBTCAVITY_H_
 
+#include "RbtBinaryIO.h"
 #include "RbtConfig.h"
 #include "RbtCoord.h"
 #include "RbtPrincipalAxes.h"
@@ -63,9 +64,9 @@ class RbtCavity {
         m_gridStep.Write(ostr);
         // Write number of coords
         RbtInt nCoords = m_coordList.size();
-        Rbt::WriteWithThrow(ostr, (const char*)&nCoords, sizeof(nCoords));
-        for (RbtCoordListConstIter cIter = m_coordList.begin(); cIter != m_coordList.end(); cIter++) {
-            (*cIter).Write(ostr);  // Write each coord
+        bin_write(ostr, nCoords);
+        for (const RbtCoord& coord: m_coordList) {
+            coord.Write(ostr);
         }
     }
 
@@ -77,7 +78,7 @@ class RbtCavity {
         m_gridStep.Read(istr);
         // Read number of coords
         RbtInt nCoords;
-        Rbt::ReadWithThrow(istr, (char*)&nCoords, sizeof(nCoords));
+        bin_read(istr, nCoords);
         m_coordList.reserve(nCoords);
         RbtCoord c;
         // Read each coord and add it to the cavity list

include/RbtCoord.h

@@ -22,10 +22,14 @@
 #include <algorithm>  //for min,max
 #include <numeric>    //for accumulate
 
+#include "RbtBinaryIO.h"
 #include "RbtConfig.h"
 
 extern istream& eatSeps(istream& is);
 
+using Rbt::bin_read;
+using Rbt::bin_write;
+
 class RbtCoord {
     ///////////////////////////////////////////////
     // Data members
@@ -59,15 +63,15 @@ class RbtCoord {
 
     // DM 19 Jul 2000 - Read,Write methods to read/write coords to binary streams
     inline ostream& Write(ostream& ostr) const {
-        ostr.write((const char*)&x, sizeof(x));
-        ostr.write((const char*)&y, sizeof(y));
-        ostr.write((const char*)&z, sizeof(z));
+        bin_write(ostr, x);
+        bin_write(ostr, y);
+        bin_write(ostr, z);
         return ostr;
     }
     inline istream& Read(istream& istr) {
-        istr.read((char*)&x, sizeof(x));
-        istr.read((char*)&y, sizeof(y));
-        istr.read((char*)&z, sizeof(z));
+        bin_read(istr, x);
+        bin_read(istr, y);
+        bin_read(istr, z);
         return istr;
     }
 

src/exe/rbcalcgrid.cxx

@@ -17,8 +17,10 @@
 #include <iomanip>
 
 #include "RbtBiMolWorkSpace.h"
+#include "RbtBinaryIO.h"
 #include "RbtPRMFactory.h"
 #include "RbtParameterFileSource.h"
+#include "RbtPlatformCompatibility.h"
 #include "RbtRealGrid.h"
 #include "RbtSFFactory.h"
 #include "RbtTriposAtomType.h"
@@ -181,11 +183,7 @@ int main(int argc, char* argv[]) {
         RbtString strASFile = spWS->GetName() + ".as";
         RbtString strInputFile = Rbt::GetRbtFileName("data/grids", strASFile);
         // DM 26 Sep 2000 - ios_base::binary is invalid with IRIX CC
-#if defined(__sgi) && !defined(__GNUC__)
-        ifstream istr(strInputFile.c_str(), ios_base::in);
-#else
-        ifstream istr(strInputFile.c_str(), ios_base::in | ios_base::binary);
-#endif
+        ifstream istr(strInputFile, Rbt::inputMode);
         RbtDockingSitePtr spDS(new RbtDockingSite(istr));
         istr.close();
         spWS->SetDockingSite(spDS);
@@ -211,19 +209,15 @@ int main(int argc, char* argv[]) {
 
         // Open output file
         RbtString strOutputFile(spWS->GetName() + strSuffix);
-#if defined(__sgi) && !defined(__GNUC__)
-        ofstream ostr(strOutputFile.c_str(), ios_base::out | ios_base::trunc);
-#else
-        ofstream ostr(strOutputFile.c_str(), ios_base::out | ios_base::binary | ios_base::trunc);
-#endif
+        ofstream ostr(strOutputFile, Rbt::outputMode);
         // Write header string (RbtVdwGridSF)
         const char* const header = "RbtVdwGridSF";
         RbtInt length = strlen(header);
-        Rbt::WriteWithThrow(ostr, (const char*)&length, sizeof(length));
-        Rbt::WriteWithThrow(ostr, header, length);
+        bin_write(ostr, length);
+        bin_write(ostr, header, length);
         // Write number of grids
         RbtInt nGrids = probes.size();
-        Rbt::WriteWithThrow(ostr, (const char*)&nGrids, sizeof(nGrids));
+        bin_write(ostr, nGrids);
 
         // Store regular pointers to avoid smart pointer dereferencing overheads
         RbtRealGrid* pGrid(spGrid);
@@ -247,8 +241,8 @@ int main(int argc, char* argv[]) {
             // Write the atom type string to the grid file, before the grid itself
             const char* const szType = strType.c_str();
             RbtInt l = strlen(szType);
-            Rbt::WriteWithThrow(ostr, (const char*)&l, sizeof(l));
-            Rbt::WriteWithThrow(ostr, szType, l);
+            bin_write(ostr, l);
+            bin_write(ostr, szType, l);
             pGrid->Write(ostr);
         }
         ostr.close();

src/exe/rbconvgrid.cxx

@@ -12,7 +12,9 @@
 
 #include <fstream>
 
+#include "RbtBinaryIO.h"
 #include "RbtFileError.h"
+#include "RbtPlatformCompatibility.h"
 #include "RbtVdwGridSF.h"
 
 /////////////////////////////////////////////////////////////////////
@@ -58,31 +60,21 @@ int main(int argc, char* argv[]) {
 
     try {
         // Read the grid file header
-#if defined(__sgi) && !defined(__GNUC__)
-        ifstream istr(strInputFile.c_str(), ios_base::in);
-#else
-        ifstream istr(strInputFile.c_str(), ios_base::in | ios_base::binary);
-#endif
+        std::ifstream istr(strInputFile, Rbt::inputMode);
         if (istr) {
             cout << strInputFile << " opened OK" << endl;
         }
         // Read header string
-        RbtInt length;
-        Rbt::ReadWithThrow(istr, (char*)&length, sizeof(length));
-        char* header = new char[length + 1];
-        Rbt::ReadWithThrow(istr, header, length);
-        // Add null character to end of string
-        header[length] = '\0';
-        // Compare title with
+        RbtString header;
+        bin_read(istr, header);
         RbtBool match = (RbtVdwGridSF::_CT == header);
-        delete[] header;
         if (!match) {
             throw RbtFileParseError(_WHERE_, "Invalid title string in " + strInputFile);
         }
 
         // Skip the appropriate number of grids
         RbtInt nGrids;
-        Rbt::ReadWithThrow(istr, (char*)&nGrids, sizeof(nGrids));
+        bin_read(istr, nGrids);
         cout << "File contains " << nGrids << " grids..." << endl;
         if ((iGrid > nGrids) || (iGrid < 1)) {
             cout << "Listing grids..." << endl;
@@ -91,14 +83,9 @@ int main(int argc, char* argv[]) {
         }
         RbtRealGridPtr spGrid;
         for (RbtInt i = 1; (i <= nGrids) && (i <= iGrid); i++) {
+            RbtString strType;
             // Read the atom type string
-            Rbt::ReadWithThrow(istr, (char*)&length, sizeof(length));
-            char* szType = new char[length + 1];
-            Rbt::ReadWithThrow(istr, szType, length);
-            // Add null character to end of string
-            szType[length] = '\0';
-            RbtString strType(szType);
-            delete[] szType;
+            bin_read(istr, strType);
             RbtTriposAtomType triposType;
             RbtTriposAtomType::eType aType = triposType.Str2Type(strType);
             cout << "Grid# " << i << "\t"
@@ -109,7 +96,7 @@ int main(int argc, char* argv[]) {
         // If we are not in listing mode, write the grid
         if ((iGrid <= nGrids) && (iGrid >= 1)) {
             cout << "Writing grid# " << iGrid << " to " << strOutputFile << "..." << endl;
-            ofstream ostr(strOutputFile.c_str());
+            std::ofstream ostr(strOutputFile);
             spGrid->PrintInsightGrid(ostr);
             ostr.close();
         }

src/lib/Rbt.cxx

@@ -22,6 +22,7 @@
 #include <fstream>    //For ifstream
 //#include <ios>
 #include "Rbt.h"
+#include "RbtBinaryIO.h"
 #include "RbtFileError.h"
 #include "RbtResources.h"
 #include "RbtVersion.h"
@@ -280,22 +281,6 @@ std::ostream& Rbt::PrintStdHeader(std::ostream& s, const RbtString& strExecutabl
     return s;
 }
 
-// Helper functions to read/write chars from iostreams
-// Throws error if stream state is not Good() before and after the read/write
-// It appears the STL ios_base exception throwing is not yet implemented
-// at least on RedHat 6.1, so this is a temporary workaround (yeah right)
-void Rbt::WriteWithThrow(std::ostream& ostr, const char* p, streamsize n) {
-    if (!ostr) throw RbtFileWriteError(_WHERE_, "Error writing to output stream");
-    ostr.write(p, n);
-    if (!ostr) throw RbtFileWriteError(_WHERE_, "Error writing to output stream");
-}
-
-void Rbt::ReadWithThrow(std::istream& istr, char* p, streamsize n) {
-    if (!istr) throw RbtFileReadError(_WHERE_, "Error reading from input stream");
-    istr.read(p, n);
-    if (!istr) throw RbtFileReadError(_WHERE_, "Error reading from input stream");
-}
-
 // Used to read RbtCoord. The separator between x y z should be a
 // ',', but this takes care of small mistakes, reading any white
 // space or commas there is between each variable.
@@ -313,3 +298,13 @@ istream& eatSeps(istream& is) {
     }
     return is;
 }
+
+void Rbt::ValidateTitle(istream& istr, const std::string& className) {
+    std::string title;
+    bin_read(istr, title);
+    if (title != className) {
+        throw RbtFileParseError(
+            _WHERE_, "Invalid title string for class " + className + " while deserializing: " + title
+        );
+    }
+}

src/lib/RbtBaseFileSink.cxx

@@ -19,13 +19,9 @@ using std::ios;
 
 ////////////////////////////////////////
 // Constructors/destructors
-// RbtBaseFileSink::RbtBaseFileSink(const char* fileName) :
-//   m_strFileName(fileName)
-//{
-//   _RBTOBJECTCOUNTER_CONSTR_("RbtBaseFileSink");
-// }
 
 RbtBaseFileSink::RbtBaseFileSink(const RbtString& fileName): m_strFileName(fileName), m_bAppend(false) {
+    m_fileOut.exceptions(ios::badbit);
     _RBTOBJECTCOUNTER_CONSTR_("RbtBaseFileSink");
 }
 
@@ -101,8 +97,11 @@ void RbtBaseFileSink::ReplaceLine(const RbtString& fileRec, RbtUInt nRec) {
 void RbtBaseFileSink::Open(RbtBool bAppend) {
     std::_Ios_Openmode openMode = ios_base::out;
     if (bAppend) openMode = openMode | ios_base::app;
-    m_fileOut.open(m_strFileName.c_str(), openMode);
-    if (!m_fileOut) throw RbtFileWriteError(_WHERE_, "Error opening " + m_strFileName);
+    try {
+        m_fileOut.open(m_strFileName, openMode);
+    } catch (...) {
+        throw RbtFileWriteError(_WHERE_, "Error opening " + m_strFileName);
+    }
 }
 
 void RbtBaseFileSink::Close() { m_fileOut.close(); }