Skip to content

Commit

Permalink
Exporter: Take care of saving template data
Browse files Browse the repository at this point in the history
Before this change, template data saving was done while saving
template XML configuration. This worked only for the default file
format, and it delayed finishing the writing of the map file until
all modified templates were saved. This behaviour meant increasing
the risk of losing changes to the map, due to crash or forceful
termination (out-of-memory, or Android).

In Exporter::doExport(), it is now easy to catch errors from saving
template data and add messages the to the list of warnings.

Resolves GH-1790 (No error reporting from template saving).
Resolves GH-1791 (Template data saved only for default file format).
  • Loading branch information
dg0yt committed Dec 2, 2020
1 parent 9c260f5 commit 17bd91c
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 18 deletions.
35 changes: 29 additions & 6 deletions src/fileformats/file_import_export.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,21 +296,24 @@ bool Exporter::doExport()
}
}

auto success = true;
// Save the map
try
{
if (!exportImplementation())
{
Q_ASSERT(!warnings().empty());
return false;
success = false;
}
if (managed_file && !managed_file->commit())
if (success && managed_file && !managed_file->commit())
{
addWarning(tr("Cannot save file\n%1:\n%2").arg(path, managed_file->errorString()));
return false;
success = false;
}
#ifdef Q_OS_ANDROID
// Make the MediaScanner aware of the *updated* file.
if (auto* file_device = qobject_cast<QFileDevice*>(device_))
auto* file_device = qobject_cast<QFileDevice*>(device_);
if (success && file_device)
{
const auto file_info = QFileInfo(file_device->fileName());
Android::mediaScannerScanFile(file_info.absolutePath());
Expand All @@ -320,10 +323,30 @@ bool Exporter::doExport()
catch (std::exception &e)
{
addWarning(tr("Cannot save file\n%1:\n%2").arg(path, QString::fromLocal8Bit(e.what())));
return false;
success = false;
}

return true;
// Save modified templates
for (auto i = 0; i < map->getNumTemplates(); ++i)
{
auto const* temp = map->getTemplate(i);
auto const filename = temp->getTemplateFilename();
try
{
if (temp->hasUnsavedChanges() && !temp->saveTemplateFile())
{
addWarning(tr("Cannot save file\n%1:\n%2").arg(filename, temp->errorString()));
success = false;
}
}
catch (std::exception &e)
{
addWarning(tr("Cannot save file\n%1:\n%2").arg(filename, QString::fromLocal8Bit(e.what())));
success = false;
}
}

return success;
}


Expand Down
6 changes: 0 additions & 6 deletions src/templates/template.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,6 @@ void Template::saveTemplateConfiguration(QXmlStreamWriter& xml, bool open, const

saveTypeSpecificTemplateConfiguration(xml);

if (open && hasUnsavedChanges())
{
// Save the template itself (e.g. image, gpx file, etc.)
saveTemplateFile();
const_cast<Template*>(this)->setHasUnsavedChanges(false); // TODO: Error handling?
}
xml.writeEndElement(/*template*/);
}

Expand Down
4 changes: 2 additions & 2 deletions src/templates/template.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,10 @@ Q_OBJECT


/**
* Saves the template itself, returns true if successful.
* Saves the template contents and returns true if successful.
*
* This is called when saving the map if the template's hasUnsavedChanges()
* is true.
* is true. After successful saving, hasUnsavedChanges() shall return false.
*/
virtual bool saveTemplateFile() const;

Expand Down
8 changes: 6 additions & 2 deletions src/templates/template_image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,16 @@ TemplateImage* TemplateImage::duplicate() const

bool TemplateImage::saveTemplateFile() const
{
const auto result = image.save(template_path);
if (!image.save(template_path))
return false;

#ifdef Q_OS_ANDROID
// Make the MediaScanner aware of the *updated* file.
Android::mediaScannerScanFile(QFileInfo(template_path).absolutePath());
#endif
return result;

const_cast<TemplateImage*>(this)->setHasUnsavedChanges(false);
return true;
}


Expand Down
6 changes: 5 additions & 1 deletion src/templates/template_track.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,11 @@ bool TemplateTrack::loadTypeSpecificTemplateConfiguration(QXmlStreamReader& xml)

bool TemplateTrack::saveTemplateFile() const
{
return track.saveTo(template_path);
if (!track.saveTo(template_path))
return false;

const_cast<TemplateTrack*>(this)->setHasUnsavedChanges(false);
return true;
}

bool TemplateTrack::loadTemplateFileImpl()
Expand Down
1 change: 0 additions & 1 deletion test/template_t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ private slots:
QBuffer buffer;
exporter->setDevice(&buffer);
QVERIFY(exporter->doExport());
QEXPECT_FAIL("", "GH-1791 (Template data saved only for default file format)", Abort);
QVERIFY(!temp->hasUnsavedChanges());
}

Expand Down

0 comments on commit 17bd91c

Please sign in to comment.