From dbc567ed89c90c9d00f7409b1f86cf2db8075429 Mon Sep 17 00:00:00 2001
From: EvilSpirit <anandamide@mail.ru>
Date: Mon, 28 Nov 2016 23:20:59 +0700
Subject: [PATCH] Reload linked files before upgrading legacy data.

Before this commit, if constraints with newly introduced params were
loaded from a file that linked other files, the upgrade code would
attempt to look up a non-existent entity.
---
 bench/harness.cpp  |  2 --
 src/file.cpp       | 17 ++++++++++-------
 src/solvespace.cpp | 10 ++++------
 src/solvespace.h   |  4 ++--
 test/harness.cpp   |  3 +--
 5 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/bench/harness.cpp b/bench/harness.cpp
index cb2367c..ba7f630 100644
--- a/bench/harness.cpp
+++ b/bench/harness.cpp
@@ -62,8 +62,6 @@ int main(int argc, char **argv) {
             [&] {
                 if(!SS.LoadFromFile(filename))
                     return false;
-                if(!SS.ReloadAllImported(/*canCancel=*/false))
-                    return false;
                 SS.AfterNewFile();
                 return true;
             },
diff --git a/src/file.cpp b/src/file.cpp
index f8a36b5..09acbbe 100644
--- a/src/file.cpp
+++ b/src/file.cpp
@@ -426,7 +426,7 @@ void SolveSpaceUI::LoadUsingTable(char *key, char *val) {
     }
 }
 
-bool SolveSpaceUI::LoadFromFile(const std::string &filename) {
+bool SolveSpaceUI::LoadFromFile(const std::string &filename, bool canCancel) {
     allConsistent = false;
     fileLoadError = false;
 
@@ -512,7 +512,9 @@ bool SolveSpaceUI::LoadFromFile(const std::string &filename) {
             NewFile();
         }
     }
-
+    if(!ReloadAllImported(filename, canCancel)) {
+        return false;
+    }
     UpgradeLegacyData();
 
     return true;
@@ -876,8 +878,9 @@ static void PathSepNormalize(std::string &filename)
     }
 }
 
-bool SolveSpaceUI::ReloadAllImported(bool canCancel)
+bool SolveSpaceUI::ReloadAllImported(const std::string &filename, bool canCancel)
 {
+    std::string saveFile = filename.empty() ? SS.saveFile : filename;
     std::map<std::string, std::string> linkMap;
     allConsistent = false;
 
@@ -906,7 +909,7 @@ bool SolveSpaceUI::ReloadAllImported(bool canCancel)
         // In a newly created group we only have an absolute path.
         if(!g->linkFileRel.empty()) {
             std::string rel = PathSepUnixToPlatform(g->linkFileRel);
-            std::string fromRel = MakePathAbsolute(SS.saveFile, rel);
+            std::string fromRel = MakePathAbsolute(saveFile, rel);
             FILE *test = ssfopen(fromRel, "rb");
             if(test) {
                 fclose(test);
@@ -922,14 +925,14 @@ bool SolveSpaceUI::ReloadAllImported(bool canCancel)
 try_load_file:
         if(LoadEntitiesFromFile(g->linkFile, &(g->impEntity), &(g->impMesh), &(g->impShell)))
         {
-            if(!SS.saveFile.empty()) {
+            if(!saveFile.empty()) {
                 // Record the linked file's name relative to our filename;
                 // if the entire tree moves, then everything will still work
-                std::string rel = MakePathRelative(SS.saveFile, g->linkFile);
+                std::string rel = MakePathRelative(saveFile, g->linkFile);
                 g->linkFileRel = PathSepPlatformToUnix(rel);
             } else {
                 // We're not yet saved, so can't make it absolute.
-                // This will only be used for display purposes, as SS.saveFile
+                // This will only be used for display purposes, as saveFile
                 // is always nonempty when we are actually writing anything.
                 g->linkFileRel = g->linkFile;
             }
diff --git a/src/solvespace.cpp b/src/solvespace.cpp
index 50efb5f..0b92ddf 100644
--- a/src/solvespace.cpp
+++ b/src/solvespace.cpp
@@ -123,7 +123,7 @@ bool SolveSpaceUI::LoadAutosaveFor(const std::string &filename) {
 
     if(LoadAutosaveYesNo() == DIALOG_YES) {
         unsaved = true;
-        return LoadFromFile(autosaveFile);
+        return LoadFromFile(autosaveFile, /*canCancel=*/true);
     }
 
     return false;
@@ -131,11 +131,9 @@ bool SolveSpaceUI::LoadAutosaveFor(const std::string &filename) {
 
 bool SolveSpaceUI::OpenFile(const std::string &filename) {
     bool autosaveLoaded = LoadAutosaveFor(filename);
-    bool fileLoaded = autosaveLoaded || LoadFromFile(filename);
-    if(fileLoaded)
+    bool fileLoaded = autosaveLoaded || LoadFromFile(filename, /*canCancel=*/true);
+    if(fileLoaded) {
         saveFile = filename;
-    bool success = fileLoaded && ReloadAllImported(/*canCancel=*/true);
-    if(success) {
         AddToRecentList(filename);
     } else {
         saveFile = "";
@@ -143,7 +141,7 @@ bool SolveSpaceUI::OpenFile(const std::string &filename) {
     }
     AfterNewFile();
     unsaved = autosaveLoaded;
-    return success;
+    return fileLoaded;
 }
 
 void SolveSpaceUI::Exit() {
diff --git a/src/solvespace.h b/src/solvespace.h
index 17ae3e0..c6bff08 100644
--- a/src/solvespace.h
+++ b/src/solvespace.h
@@ -812,11 +812,11 @@ public:
     void NewFile();
     bool SaveToFile(const std::string &filename);
     bool LoadAutosaveFor(const std::string &filename);
-    bool LoadFromFile(const std::string &filename);
+    bool LoadFromFile(const std::string &filename, bool canCancel = false);
     void UpgradeLegacyData();
     bool LoadEntitiesFromFile(const std::string &filename, EntityList *le,
                               SMesh *m, SShell *sh);
-    bool ReloadAllImported(bool canCancel=false);
+    bool ReloadAllImported(const std::string &filename = "", bool canCancel = false);
     // And the various export options
     void ExportAsPngTo(const std::string &filename);
     void ExportMeshTo(const std::string &filename);
diff --git a/test/harness.cpp b/test/harness.cpp
index 6cfa996..e281659 100644
--- a/test/harness.cpp
+++ b/test/harness.cpp
@@ -185,8 +185,7 @@ bool Test::Helper::CheckLoad(const char *file, int line, const char *fixture) {
     bool fixtureExists = (f != NULL);
     if(f) fclose(f);
 
-    bool result = fixtureExists &&
-        SS.LoadFromFile(fixturePath) && SS.ReloadAllImported(/*canCancel=*/false);
+    bool result = fixtureExists && SS.LoadFromFile(fixturePath);
     if(!RecordCheck(result)) {
         PrintFailure(file, line,
                      ssprintf("loading file '%s'", fixturePath.c_str()));