Merge remote-tracking branch 'remotes/jmarrec/PR_opened/Fix_72_LoadPa…

…th' into merge_julien_prs
NREL · Mar 21, 2019 · a864f86 · a864f86
2 parents c7c5bb3 + dc5bd72
commit a864f86
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 18 deletions.
diff --git a/openstudiocore/src/utilities/core/PathHelpers.cpp b/openstudiocore/src/utilities/core/PathHelpers.cpp
@@ -109,11 +109,7 @@ path setFileExtension(const path& p,
 {
   path result(p);
   path wext = toPath(ext);
-  std::string pext = openstudio::filesystem::extension(p);
-  if (!pext.empty()) {
-    // remove '.' from pext
-    pext = std::string(++pext.begin(),pext.end());
-  }
+  std::string pext = getFileExtension(p);
   if (!pext.empty()) {
     if (pext != wext.string()) {
       if (warnOnMismatch) {

diff --git a/openstudiocore/src/utilities/idf/IdfFile.cpp b/openstudiocore/src/utilities/idf/IdfFile.cpp
@@ -381,9 +381,21 @@ OptionalIdfFile IdfFile::load(const path& p,
   path wp(p);
 
   if (iddFileType == IddFileType::OpenStudio) {
+
     // can be Model or Component
+    std::string ext = getFileExtension(p);
+    if (! ( openstudio::istringEqual(ext, "osm") ||
+            openstudio::istringEqual(ext, "osc")) ) {
+      LOG_FREE(Warn,"openstudio.setFileExtension","Path p, '" << toString(p)
+                 << "', has an unexpected file extension. Was expecting 'osm' or 'osc'.");
+    }
+
+    // This isn't issuing warnings because we pass false (we have to, since it can be either osm or osc)
+    // hence why we do check above
     wp = completePathToFile(wp,path(),modelFileExtension(),false);
-    if (wp.empty()) { wp = completePathToFile(wp,path(),componentFileExtension(),false); }
+    if (wp.empty()) {
+      wp = completePathToFile(wp,path(),componentFileExtension(),false);
+    }
   }
   else {
     wp = completePathToFile(wp,path(),"idf",true);
@@ -612,9 +624,6 @@ bool IdfFile::m_load(std::istream& is, ProgressBar* progressBar, bool versionOnl
     else{
 
       bool foundEndLine(false);
-
-      // a valid Idf object to parse
-      ++objectNum;
       firstBlock = false;
       bool isVersion = false;
 
@@ -670,16 +679,22 @@ bool IdfFile::m_load(std::istream& is, ProgressBar* progressBar, bool versionOnl
       }
 
       // construct the object
-      if (!versionOnly || isVersion) {
+      if (foundEndLine && (!versionOnly || isVersion)) {
         OptionalIdfObject object = IdfObject::load(text,*iddObject);
         if (!object) {
           LOG(Error,"Unable to construct IdfObject from text: " << std::endl << text
               << std::endl << "Throwing this object out and parsing the remainder of the file.");
           continue;
+        } else {
+          // a valid Idf object to parse
+          if (object->iddObject().type() != IddObjectType::Catchall) {
+            ++objectNum;
+          }
+
+          // put it in the object list
+          addObject(*object);
         }
 
-        // put it in the object list
-        addObject(*object);
       }
 
       if (versionOnly && isVersion) {
@@ -689,7 +704,13 @@ bool IdfFile::m_load(std::istream& is, ProgressBar* progressBar, bool versionOnl
     }
   }
 
-  return true;
+  // If we sucessfully parsed at least one object, we return true, otherwise false
+  if (objectNum > 0) {
+    return true;
+  } else {
+    LOG(Error, "Could not parse a single valid object in file.");
+    return false;
+  }
 }
 
 IddFileAndFactoryWrapper IdfFile::iddFileAndFactoryWrapper() const {

diff --git a/openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp b/openstudiocore/src/utilities/idf/Test/IdfFile_GTest.cpp
@@ -86,18 +86,15 @@ TEST_F(IdfFixture, IdfFile_BasicTests_LoadedFile)
 }
 
 TEST_F(IdfFixture, IdfFile_Header) {
-  std::stringstream ss;
-  OptionalIdfFile oFile = IdfFile::load(ss,IddFileType(IddFileType::EnergyPlus));
-  ss.clear();
-  ASSERT_TRUE(oFile);
-  IdfFile file = *oFile;
+  IdfFile file(IddFileType::EnergyPlus);
   std::string header = "! A one-line header. ";
   file.setHeader(header);
   EXPECT_EQ(header,file.header());
 
   header = "Not actually a header, should get ! pre-pended.";
   file.setHeader(header);
   EXPECT_NE(header,file.header());
+  std::stringstream ss;
   ss << "! " << header;
   EXPECT_EQ(ss.str(),file.header());