Skip to content

Commit

Permalink
Make it so UserState DB indexes are no longer written to N42 files.
Browse files Browse the repository at this point in the history 
Instead of saving to the indexes to N42, we just store in memory when we remove SpecFile from memory, and cache on disk.
Saving to N42, and then reading back in, seemed potentially error prone and dangerous.
  • Loading branch information
@wcjohns
wcjohns committed Feb 14, 2024
1 parent 4947b28 commit 8bf87fd
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 37 deletions.
22 changes: 15 additions & 7 deletions InterSpec/SpecMeas.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class SpecMeas : public SpecUtils::SpecFile
*/
const std::set<int> &displayedSampleNumbers() const;

//aboutToBeDeleted(): signal emited right before object destructions - useful
//aboutToBeDeleted(): signal emitted right before object destructions - useful
// if you want to serialize changes to disk
Wt::Signal<> &aboutToBeDeleted();

Expand Down Expand Up @@ -236,7 +236,7 @@ class SpecMeas : public SpecUtils::SpecFile
@returns The db index value that is >=0, if there is a state associated with this spectrum, otherwise returns -1.
This quantity is stored when the user loads or saves the application state.
This value is currently persisted into the XML, but may change in the future.
This value is not currently persisted into the XML.
*/
long long int dbStateId( const std::set<int> &samplenums ) const;

Expand All @@ -249,6 +249,9 @@ class SpecMeas : public SpecUtils::SpecFile
/** Clears all UserState table index associations. */
void clearAllDbStateId();

/** Returns the full map from sample numbers, to database UserState indexes. */
const std::map<std::set<int>,long long int> &dbUserStateIndexes() const;

//shiftPeaksForRecalibration: shift the peaks for when you apply a
// recalibration to the spectrum, for instance after calling
// SpecFile::recalibrate_by_eqn(...). Note that the PeakModel is not
Expand Down Expand Up @@ -304,8 +307,14 @@ class SpecMeas : public SpecUtils::SpecFile
void addPeaksToXml( ::rapidxml::xml_node<char> *peaksnode ) const;
void addPeaksFromXml( const ::rapidxml::xml_node<char> *peaksnode );

void addDbStateIdsToXml( ::rapidxml::xml_node<char> *db_state_index_node ) const;
void addDbStateIdsFromXml( const ::rapidxml::xml_node<char> *db_state_index_node );
// I dont think we ever want to store the database index(es) into the written N42
// file, but if we ever do, these next two functions implement writing to, and
// reading from the xml.
// If you change this, at a minimum, check for commented out calls to
// `SpecMeas::clearAllDbStateId()`, so we don't erroneously setup to write over
// existing database states, that may not be related to the data being loaded.
// void addDbStateIdsToXml( ::rapidxml::xml_node<char> *db_state_index_node ) const;
// void addDbStateIdsFromXml( const ::rapidxml::xml_node<char> *db_state_index_node );

#if( PERFORM_DEVELOPER_CHECKS )
//equalEnough(...): tests whether the passed in Measurement objects are
Expand Down Expand Up @@ -395,14 +404,13 @@ class SpecMeas : public SpecUtils::SpecFile

/** A mapping of sample numbers, to the UserState table, if the user has saved state to the DB.
Not currently serialized to N42...
Not currently serialized to N42.
*/
std::map<std::set<int>,long long int> m_dbUserStateIndex;
std::map<std::set<int>,long long int> m_dbUserStateIndexes;

/** Version of XML serialization of the <DHS:InterSpec> node.
Changes:
- Added version field to xml 20200807, with initial value 1. Added <DisplayedDetectors> field.
- Added "DbUserStateIndexes" node to xml 20230214, but did not increment version as it is not required field.
*/
static const int sm_specMeasSerializationVersion;

Expand Down
11 changes: 11 additions & 0 deletions InterSpec/SpectraFileModel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "InterSpec_config.h"

#include <map>
#include <set>
#include <mutex>
#include <deque>
Expand Down Expand Up @@ -358,6 +359,16 @@ class SpectraFileHeader
std::string m_appId;
mutable Wt::WApplication *m_app;

/** Keep a mapping of UserState database indexes, when files are removed from memory, and written back to disk.
This will be restored when the file is parsed back in.
Values in this variable are only valid if the measurement is not in memory.
This variable is only accessed or modified when there is a lock on `m_mutex`, and is mutable so that
it can be set and cleared in `saveToFileSystem(...)` and `parseFile()`, respectively.
*/
mutable std::map<std::set<int>,long long int> m_userStateDbIndexes;

#ifdef WT_USE_BOOST_SIGNALS2
mutable boost::signals2::connection m_aboutToBeDeletedConnection;
#else
Expand Down
52 changes: 29 additions & 23 deletions src/SpecMeas.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ void SpecMeas::uniqueCopyContents( const SpecMeas &rhs )

m_fileWasFromInterSpec = rhs.m_fileWasFromInterSpec;

m_dbUserStateIndex = rhs.m_dbUserStateIndex;
m_dbUserStateIndexes = rhs.m_dbUserStateIndexes;
}//void uniqueCopyContents( const SpecMeas &rhs )


Expand Down Expand Up @@ -919,14 +919,15 @@ void SpecMeas::addPeaksFromXml( const ::rapidxml::xml_node<char> *peaksnode )
}//void addPeaksFromXml( ::rapidxml::xml_node<char> *peaksnode );


/*
void SpecMeas::addDbStateIdsToXml( ::rapidxml::xml_node<char> *db_state_index_node ) const
{
auto doc = db_state_index_node ? db_state_index_node->document() : nullptr;
assert( doc );
if( !doc )
return;
for( const auto &index_id : m_dbUserStateIndex )
for( const auto &index_id : m_dbUserStateIndexes )
{
rapidxml::xml_node<char> *state_node = doc->allocate_node( rapidxml::node_element, "SamplesToUserState" );
db_state_index_node->append_node( state_node );
Expand All @@ -943,7 +944,7 @@ void SpecMeas::addDbStateIdsToXml( ::rapidxml::xml_node<char> *db_state_index_no
val = doc->allocate_string( std::to_string(index_id.second).c_str() );
rapidxml::xml_node<char> *index_node = doc->allocate_node( rapidxml::node_element, "DbIndex", val );
state_node->append_node( index_node );
}//for( const auto &index_id : m_dbUserStateIndex )
}//for( const auto &index_id : m_dbUserStateIndexes )
}//void addDbStateIdsToXml( ::rapidxml::xml_node<char> *peaksnode ) const
Expand Down Expand Up @@ -975,14 +976,14 @@ void SpecMeas::addDbStateIdsFromXml( const ::rapidxml::xml_node<char> *node )
if( !(stringstream( SpecUtils::xml_value_str(index) ) >> index_value) || (index_value < 0) )
throw runtime_error( "Invalid index value" );
m_dbUserStateIndex[sample_set] = index_value;
m_dbUserStateIndexes[sample_set] = index_value;
}catch( std::exception &e )
{
cerr << "Failed to parse UserStates XML node: " << e.what() << endl;
}//try / catch
}//XML_FOREACH_CHILD( state_node , node, "SamplesToUserState")
}//void addDbStateIdsFromXml( const ::rapidxml::xml_node<char> *db_state_index_node );

*/

rapidxml::xml_document<char> *SpecMeas::shieldingSourceModel()
{
Expand Down Expand Up @@ -1542,11 +1543,10 @@ void SpecMeas::decodeSpecMeasStuffFromXml( const ::rapidxml::xml_node<char> *int
}//if( !filename_.empty() )


m_dbUserStateIndex.clear();
node = XML_FIRST_NODE( interspecnode, "DbUserStateIndexes" );
if( node )
addDbStateIdsFromXml( node );

m_dbUserStateIndexes.clear();
//node = XML_FIRST_NODE( interspecnode, "DbUserStateIndexes" );
//if( node )
// addDbStateIdsFromXml( node );
}//void decodeSpecMeasStuffFromXml( ::rapidxml::xml_node<char> *parent )


Expand Down Expand Up @@ -1673,12 +1673,12 @@ ::rapidxml::xml_node<char> *SpecMeas::appendSpecMeasStuffToXml(
interspec_node->append_node( node );
}//if( !filename_.empty() )

if( !m_dbUserStateIndex.empty() )
{
xml_node<char> *db_state_index_node = doc->allocate_node( node_element, "DbUserStateIndexes" );
interspec_node->append_node( db_state_index_node );
addDbStateIdsToXml( db_state_index_node );
}//if( !m_dbUserStateIndex.empty() )
//if( !m_dbUserStateIndexes.empty() )
//{
// xml_node<char> *db_state_index_node = doc->allocate_node( node_element, "DbUserStateIndexes" );
// interspec_node->append_node( db_state_index_node );
// addDbStateIdsToXml( db_state_index_node );
//}//if( !m_dbUserStateIndexes.empty() )

return interspec_node;
}//appendSpecMeasStuffToXml(...);
Expand Down Expand Up @@ -2143,8 +2143,8 @@ void SpecMeas::setModified()

long long int SpecMeas::dbStateId( const set<int> &samplenums ) const
{
const auto pos = m_dbUserStateIndex.find(samplenums);
if( pos == end(m_dbUserStateIndex) )
const auto pos = m_dbUserStateIndexes.find(samplenums);
if( pos == end(m_dbUserStateIndexes) )
return -1;
return pos->second;
}//long long int dbStateId( const set<int> &samplenums ) const
Expand All @@ -2154,22 +2154,28 @@ void SpecMeas::setDbStateId( const long long int db_id, const std::set<int> &sam
{
if( db_id < 0 )
{
const auto pos = m_dbUserStateIndex.find(samplenums);
if( pos != end(m_dbUserStateIndex) )
m_dbUserStateIndex.erase(pos);
const auto pos = m_dbUserStateIndexes.find(samplenums);
if( pos != end(m_dbUserStateIndexes) )
m_dbUserStateIndexes.erase(pos);
return;
}//if( db_id < 0 )

m_dbUserStateIndex[samplenums] = db_id;
m_dbUserStateIndexes[samplenums] = db_id;
}//void setDbStateId( const long long int db_id, const std::set<int> &samplenums )


void SpecMeas::clearAllDbStateId()
{
m_dbUserStateIndex.clear();
m_dbUserStateIndexes.clear();
}


const map<set<int>,long long int> &SpecMeas::dbUserStateIndexes() const
{
return m_dbUserStateIndexes;
}//const map<set<int>,long long int> &dbUserStateIndexs() const


void SpecMeas::cleanup_after_load( const unsigned int flags )
{
if( m_fileWasFromInterSpec )
Expand Down
27 changes: 20 additions & 7 deletions src/SpectraFileModel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,8 @@ std::shared_ptr<SpecMeas> SpectraFileHeader::initFile(
//Make sure the file we're opening doesn't have erroneous DB state indexes in it, which could
// cause us to overwrite a state.
// I *think* all opening of spectrum files from filesystem go through here of `setFile(...)`.
info->clearAllDbStateId();
//Currently we are not writing this info to the SpecFile, so this call is not actually needed.
info->clearAllDbStateId(); //JIV

RecursiveLock lock( m_mutex );
m_weakMeasurmentPtr = info;
Expand Down Expand Up @@ -1131,8 +1132,7 @@ void SpectraFileHeader::saveToFileSystem( std::shared_ptr<SpecMeas> measurment )


if( (from_mem && measurment) && (from_mem!=measurment) )
throw runtime_error( "Measurment passed in is not same as currently "
"in memmorry" );
throw runtime_error( "Measurement passed in is not same as currently in memory" );

if( !measurment && !from_mem )
from_mem = parseFile();
Expand All @@ -1157,20 +1157,26 @@ void SpectraFileHeader::saveToFileSystem( std::shared_ptr<SpecMeas> measurment )
if( !m_fileSystemLocation.empty() )
SpecUtils::remove_file( m_fileSystemLocation );
m_fileSystemLocation = "";
}catch(...){}
}catch(...)
{
}

const string tempfile = SpecUtils::temp_file_name( m_displayName, InterSpecApp::tempDirectory() );

{
RecursiveLock lock( m_mutex );
m_fileSystemLocation = tempfile;

m_userStateDbIndexes = info->dbUserStateIndexes();
}

success = true;

// success = info->save_native_file( tempfile.generic_string() );
boost::function<void()> error_callback = boost::bind( &SpectraFileHeader::errorSavingCallback, this, tempfile, info );
SpecMeas::save2012N42FileInClientThread( info, tempfile, error_callback );


//#if( USE_DB_TO_STORE_SPECTRA )
// if( m_app && shouldSaveToDb() )
// {
Expand Down Expand Up @@ -1367,7 +1373,8 @@ void SpectraFileHeader::setFile( const std::string &displayFileName, std::shared
//Make sure the file we're opening doesn't have erroneous DB state indexes in it, which could
// cause us to overwrite a state.
// I *think* all opening of spectrum files from filesystem go through here of `initFile(...)`.
info->clearAllDbStateId();
//Currently we are not writing this info to the SpecFile, so this call is not actually needed.
info->clearAllDbStateId(); //JIC

m_modifiedSinceDecode = false;

Expand All @@ -1380,7 +1387,7 @@ std::shared_ptr<SpecMeas> SpectraFileHeader::setFile(
const std::string &filename,
SpecUtils::ParserType parseType )
{
cerr << "SpectraFileHeader::setFile" << endl;
cerr << "SpectraFileHeader::setFile('" << displayFileName << "', '" << filename << "')" << endl;
try
{
if( !SpecUtils::is_file(filename) )
Expand Down Expand Up @@ -1448,9 +1455,15 @@ std::shared_ptr<SpecMeas> SpectraFileHeader::parseFile() const
m_cachedMeasurement = info;
m_weakMeasurmentPtr = info;

//XXX - Should consider using an on error callback for following connections
// TODO: Should consider using an on error callback for following connections
if( info )
{
// Restore mappings of sample numbers in this file, to UserState indexes in the database
info->clearAllDbStateId(); //JIC, but not needed
for( const auto &sn_index : m_userStateDbIndexes )
info->setDbStateId( sn_index.second, sn_index.first );
m_userStateDbIndexes.clear();

if( m_aboutToBeDeletedConnection.connected() )
m_aboutToBeDeletedConnection.disconnect();
m_aboutToBeDeletedConnection = info->aboutToBeDeleted().connect(
Expand Down

0 comments on commit 8bf87fd

Please sign in to comment.