EmptyValues updates should work (also for files from 0.18.1) (#5347)

* EmptyValues updates should also work for files from 0.18.1 Make sure changes are updated in database Also prepare the upgradeDB function in databaseInterface for future changes * Workspace emptyvalues must have default values When loading a 0.18.1 JASP file, the workspace default values are empty. They should be set to the preferences empty values as default (since there is no workspace empty values in 0.18.1). In DataSet::setEmptyValuesJson, there was already an handling to overcome this issue, but as DataSet has no access to the PreferencesModel object, it was depending of setting the _defaultEmptyvalues property just after the DataSet was created. But as DataSet might be deleted and created several times, the _defaultEmptyvalues is not always set. So just make it a static property, so that it just has to be set once. --------- Co-authored-by: boutinb <[email protected]>
jasp-stats · Jan 2, 2024 · ff20125 · ff20125
1 parent 16369a7
commit ff20125
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 56 deletions.
diff --git a/Common/version.cpp b/Common/version.cpp
@@ -25,7 +25,17 @@ const char * Version::encodingError::what() const noexcept
 }
 
 
-Version::Version(std::string version)
+Version::Version(const char * version)
+{
+	fromString(version);
+}
+
+Version::Version(const std::string & version)
+{
+	fromString(version);
+}
+
+void Version::fromString(const std::string & version)
 {
 	const static std::regex parseIt("(\\d+)(\\.(\\d+))?(\\.(\\d+))?(\\.(\\d+))?"); //(sub)groups: 1 3 5 7//0 is whole match/line
 

diff --git a/Common/version.h b/Common/version.h
@@ -30,15 +30,17 @@ class Version
 public:
 	struct encodingError  : public std::runtime_error
 	{
-		encodingError(std::string versionStr) : std::runtime_error("Module version not understood: " + versionStr) {}
+		encodingError(const std::string & versionStr) : std::runtime_error("Module version not understood: " + versionStr) {}
 		const char* what() const noexcept override;
 	};
 
-	Version(std::string version);
+	Version(const char * version);
+	Version(const std::string & version);
 	Version(unsigned int major = 0, unsigned int minor = 0, unsigned int release = 0, unsigned int fourth = 0) : _major(major), _minor(minor), _release(release), _fourth(fourth) {}
 
 	///By default this tries to minimize the string, so all trailing zeroes + dots are removed. Unless versionNumbersToInclude is set to something >1. If 2 then major and minor are always shown, etc.
 	std::string asString(size_t versionNumbersToInclude = 0) const;
+
 
 
 	unsigned int major()	const { return _major; }
@@ -57,6 +59,9 @@ class Version
 	bool		operator ==	(const Version & other) const;
 	bool		operator !=	(const Version & other) const;
 
+protected:
+	void		fromString(const std::string &  version);
+
 
 private:
 	unsigned int	_major		= 0,

diff --git a/CommonData/databaseinterface.cpp b/CommonData/databaseinterface.cpp
@@ -16,10 +16,22 @@ const std::string DatabaseInterface::_dbConstructionSql =
 "CREATE TABLE Filters		( id INTEGER PRIMARY KEY, dataSet INT, rFilter TEXT, generatedFilter TEXT, constructorJson TEXT, constructorR TEXT, errorMsg TEXT"
 							", revision INT DEFAULT 0, FOREIGN KEY(dataSet) REFERENCES DataSets(id));\n"
 "CREATE TABLE Columns		( id INTEGER PRIMARY KEY, dataSet INT, name TEXT, title TEXT, description TEXT, columnType TEXT, colIdx INT, isComputed INT, invalidated INT NULL, "
-							"codeType TEXT NULL, rCode TEXT NULL, error TEXT NULL, constructorJson TEXT NULL, emptyValuesJson TEXT, "
+							"codeType TEXT NULL, rCode TEXT NULL, error TEXT NULL, constructorJson TEXT NULL"
 							"analysisId INT NULL, revision INT DEFAULT 0, FOREIGN KEY(dataSet) REFERENCES DataSets(id));\n"
 "CREATE TABLE Labels		( id INTEGER PRIMARY KEY, columnId INT, value INT, ordering INT, filterAllows INT, label TEXT, originalValueJson TEXT, description TEXT, FOREIGN KEY(columnId) REFERENCES Columns(id));\n";
 
+void DatabaseInterface::upgradeDBFromVersion(Version originalVersion)
+{
+	   transactionWriteBegin();
+
+	   if(originalVersion < "0.18.2")
+			   runStatements("ALTER TABLE DataSets ADD COLUMN description     TEXT;"                 "\n");
+
+	   //Later versions can add new originalVersion < blabla blocks at the end of this "list"
+
+		transactionWriteEnd();
+}
+
 DatabaseInterface::DatabaseInterface(bool createDb)
 {
 	assert(!_singleton);
@@ -99,7 +111,6 @@ void DatabaseInterface::dataSetLoad(int dataSetId, std::string & dataFilePath, s
 		Log::log() << "Output loadDataset(dataSetId="<<dataSetId<<") had (dataFilePath='"<<dataFilePath<<"', databaseJson='"<<databaseJson<<"', emptyValuesJson='"<<emptyValuesJson<<"')" << std::endl;
 	};
 
-	ensureCorrectDb();
 	runStatements("SELECT dataFilePath, description, databaseJson, emptyValuesJson, revision, dataFileSynch FROM DataSets WHERE id = ?;", prepare, processRow);
 }
 
@@ -1609,7 +1620,6 @@ void DatabaseInterface::load()
 	else
 		Log::log() << "Opened internal sqlite database for loading at '" << dbFile() << "'." << std::endl;
 
-	ensureCorrectDb();
 }
 
 void DatabaseInterface::close()
@@ -1622,19 +1632,6 @@ void DatabaseInterface::close()
 	}
 }
 
-void DatabaseInterface::ensureCorrectDb()
-{
-	transactionWriteBegin();
-
-	// Check whether the description column exists in DataSets table
-	int count = runStatementsId("SELECT COUNT(*) AS CNTREC FROM pragma_table_info('DataSets') WHERE name='description'");
-	if (count == 0)
-		runStatements("ALTER TABLE DataSets ADD COLUMN description TEXT");
-
-	transactionWriteEnd();
-
-}
-
 void DatabaseInterface::transactionWriteBegin()
 {
 	JASPTIMER_SCOPE(DatabaseInterface::transactionWriteBegin);

diff --git a/CommonData/databaseinterface.h b/CommonData/databaseinterface.h
@@ -63,6 +63,7 @@ class DatabaseInterface
 	static		DatabaseInterface * singleton() { return _singleton; }					///< There can be only one! https://www.youtube.com/watch?v=sqcLjcSloXs
 
 	bool		hasConnection() { return _db; }
+	void		upgradeDBFromVersion(Version originalVersion);							///< Ensures that the database has all the fields configured as required for the current JASP version, useful when loading older sqlite-containing jasp-files
 
 	void		runQuery(		const std::string & query,		std::function<void(sqlite3_stmt *stmt)>		bindParameters,				std::function<void(size_t row, sqlite3_stmt *stmt)>		processRow);	///< Runs a single query and then goes through the resultrows while calling processRow for each.
 	void		runStatements(	const std::string & statements);																																				///< Runs several sql statements without looking at the results.
@@ -165,7 +166,6 @@ class DatabaseInterface
 	void		create();										///< Creates a new sqlite database in sessiondir and loads it
 	void		load();											///< Loads a sqlite database from sessiondir (after loading a jaspfile)
 	void		close();										///< Closes the loaded database and disconnects
-	void		ensureCorrectDb();
 
 	int			_transactionWriteDepth	= 0,
 				_transactionReadDepth	= 0;

diff --git a/CommonData/dataset.cpp b/CommonData/dataset.cpp
@@ -3,6 +3,8 @@
 #include <regex>
 #include "databaseinterface.h"
 
+stringset DataSet::_defaultEmptyvalues;
+
 DataSet::DataSet(int index)
 	: DataSetBaseNode(dataSetBaseNodeType::dataSet, nullptr)
 {
@@ -221,7 +223,7 @@ void DataSet::dbUpdate()
 	incRevision();
 }
 
-void DataSet::dbLoad(int index, const Version& loadedJaspVersion, std::function<void(float)> progressCallback)
+void DataSet::dbLoad(int index, std::function<void(float)> progressCallback)
 {
 	//Log::log() << "loadDataSet(index=" << index << "), _dataSetID="<< _dataSetID <<";" << std::endl;
 
@@ -417,6 +419,7 @@ std::map<std::string, intstrmap> DataSet::resetMissingData(const std::vector<Col
 			colChanged[col->name()] = missingDataMap;
 	}
 
+	dbUpdate();
 	incRevision();
 
 	return colChanged;

diff --git a/CommonData/dataset.h b/CommonData/dataset.h
@@ -35,7 +35,7 @@ class DataSet : public DataSetBaseNode
 
 			void			dbCreate();
 			void			dbUpdate();
-			void			dbLoad(int index = -1, const Version& loadedJaspVersion = Version(), std::function<void(float)> progressCallback = [](float){});
+			void			dbLoad(int index = -1, std::function<void(float)> progressCallback = [](float){});
 			void			dbDelete();
 
 			void			beginBatchedToDB();
@@ -101,7 +101,7 @@ class DataSet : public DataSetBaseNode
 	EmptyValues					_emptyValues;
 	bool						_writeBatchedToDB		= false,
 								_dataFileSynch			= false;
-	stringset					_defaultEmptyvalues;	// Default empty values if workspace do not have its own empty values (used for backward compatibility)
+	static stringset			_defaultEmptyvalues;	// Default empty values if workspace do not have its own empty values (used for backward compatibility)
 	std::string					_description;
 };
 

diff --git a/Desktop/data/datasetpackage.cpp b/Desktop/data/datasetpackage.cpp
@@ -1280,8 +1280,12 @@ void DataSetPackage::loadDataSet(std::function<void(float)> progressCallback)
 	if(_dataSet)
 		deleteDataSet(); //no dbDelete necessary cause we just copied an old sqlite file here from the JASP file
 
+	_db->close();
+	_db->load();
+	_db->upgradeDBFromVersion(_jaspVersion);
+
 	_dataSet = new DataSet(0);
-	_dataSet->dbLoad(1, jaspVersion(), progressCallback); //Right now there can only be a dataSet with ID==1 so lets keep it simple
+	_dataSet->dbLoad(1, progressCallback); //Right now there can only be a dataSet with ID==1 so lets keep it simple
 	_dataSubModel->selectNode(_dataSet->dataNode());
 	_filterSubModel->selectNode(_dataSet->filtersNode());
 

diff --git a/Desktop/data/importers/jaspimporter.cpp b/Desktop/data/importers/jaspimporter.cpp
@@ -107,9 +107,6 @@ void JASPImporter::loadDataArchive(const std::string &path, std::function<void(i
 
 void JASPImporter::loadJASPArchive(const std::string &path, std::function<void(int)> progressCallback)
 {
-	if (DataSetPackage::pkg()->archiveVersion().major() != 4)
-		throw std::runtime_error("The file version is not supported (too new).\nPlease update to the latest version of JASP to view this file.");
-
 	JASPTIMER_SCOPE(JASPImporter::loadJASPArchive_1_00 read analyses.json);
 	Json::Value analysesData;
 
@@ -127,35 +124,6 @@ void JASPImporter::loadJASPArchive(const std::string &path, std::function<void(i
 
 			resourceEntry.writeEntryToTempFiles(); //this one doesnt really need to give feedback as the files are pretty tiny
 
-			/* This is double, since writeEntryToTempFiles do that already...
-			JASPTIMER_RESUME(JASPImporter::loadJASPArchive_1_00 Write file stream);
-			std::ofstream file(destination.c_str(),  std::ios::out | std::ios::binary);
-
-			static char streamBuff[8192 * 32];
-			file.rdbuf()->pubsetbuf(streamBuff, sizeof(streamBuff)); //Set the buffer manually to make it much faster our issue https://github.com/jasp-stats/INTERNAL-jasp/issues/436 and solution from:  https://stackoverflow.com/a/15177770
-	
-			static char copyBuff[8192 * 4];
-			int			bytes		= 0,
-						errorCode	= 0;
-
-			do
-			{
-				bytes = resourceEntry.readData(copyBuff, sizeof(copyBuff), errorCode);
-
-                if(bytes > 0 && errorCode == 0)		file.write(copyBuff, bytes);
-                else                                break;
-			}
-			while (true);
-
-			file.flush();
-			file.close();
-			JASPTIMER_STOP(JASPImporter::loadJASPArchive_1_00 Write file stream);
-	
-			if (errorCode != 0)
-				throw std::runtime_error("Could not read resource files.");
-
-			JASPTIMER_STOP(JASPImporter::loadJASPArchive_1_00 Create resource files);
-			*/
             progressCallback( 66.666 + int((33.333 / double(resources.size())) * ++resourceCounter));// "Loading Analyses",
 		}
 	}