[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [tellico] src: Account for a new local directory name when saving to a new location
From: Robby Stephenson <robby () periapsis ! org>
Date: 2016-11-07 0:10:32
Message-ID: E1c3XWC-0006qR-7w () code ! kde ! org
[Download RAW message or body]
Git commit 368c7c5eb5bd120914b66156ac5a8705d48f1b63 by Robby Stephenson.
Committed on 07/11/2016 at 00:07.
Pushed by rstephenson into branch 'master'.
Account for a new local directory name when saving to a new location
For some situations, when saving to a new directory location and with
images saved locally, depending on the cache situation, the image might
not be found.
Don't change the local directory until after the images are saved.
Create a new static method for saving outside of the config locations.
Add a unit test.
M +15 -5 src/document.cpp
M +43 -28 src/images/imagefactory.cpp
M +2 -0 src/images/imagefactory.h
M +1 -1 src/mainwindow.cpp
M +18 -9 src/tests/documenttest.cpp
http://commits.kde.org/tellico/368c7c5eb5bd120914b66156ac5a8705d48f1b63
diff --git a/src/document.cpp b/src/document.cpp
index 4480097..5af0931 100644
--- a/src/document.cpp
+++ b/src/document.cpp
@@ -33,6 +33,7 @@
#include "fieldformat.h"
#include "tellico_strings.h"
#include "images/imagefactory.h"
+#include "images/imagedirectory.h"
#include "images/image.h"
#include "images/imageinfo.h"
#include "utils/stringset.h"
@@ -553,15 +554,18 @@ void Document::slotLoadAllImages() {
m_importer = 0;
}
+// cacheDir_ is the location dir to write the images
+// localDir_ provide the new file location which is only needed if cacheDir == \
LocalDir void Document::writeAllImages(int cacheDir_, const QUrl& localDir_) {
// images get 80 steps in saveDocument()
const uint stepSize = 1 + qMax(1, m_coll->entryCount()/80); // add 1 since it \
could round off uint j = 1;
- QString oldLocalDir = ImageFactory::localDir();
- ImageFactory::setLocalDirectory(localDir_);
-
ImageFactory::CacheDir cacheDir = static_cast<ImageFactory::CacheDir>(cacheDir_);
+ ImageDirectory* imgDir = 0;
+ if(cacheDir == ImageFactory::LocalDir) {
+ imgDir = new ImageDirectory(ImageFactory::localDirectory(localDir_));
+ }
QString id;
StringSet images;
@@ -577,7 +581,14 @@ void Document::writeAllImages(int cacheDir_, const QUrl& \
localDir_) { if(ImageFactory::imageInfo(id).linkOnly) {
continue;
}
- if(!ImageFactory::writeCachedImage(id, cacheDir)) {
+ // careful here, if we're writing to LocalDir, need to read from the old \
LocalDir and write to new + bool success;
+ if(cacheDir == ImageFactory::LocalDir) {
+ success = ImageFactory::writeCachedImage(id, imgDir);
+ } else {
+ success = ImageFactory::writeCachedImage(id, cacheDir);
+ }
+ if(!success) {
myDebug() << "did not write image for entry title:" << entry->title();
}
if(m_cancelImageWriting) {
@@ -599,7 +610,6 @@ void Document::writeAllImages(int cacheDir_, const QUrl& \
localDir_) { }
m_cancelImageWriting = false;
- ImageFactory::setLocalDirectory(QUrl::fromLocalFile(oldLocalDir));
}
bool Document::pruneImages() {
diff --git a/src/images/imagefactory.cpp b/src/images/imagefactory.cpp
index 6409271..499ad19 100644
--- a/src/images/imagefactory.cpp
+++ b/src/images/imagefactory.cpp
@@ -285,15 +285,7 @@ bool ImageFactory::writeCachedImage(const QString& id_, CacheDir \
dir_, bool forc (dir_ == TempDir ? &factory->d->tempImageDir :
&factory->d->localImageDir);
Q_ASSERT(imgDir);
- bool exists = imgDir->hasImage(id_);
- // only write if it doesn't exist
- bool success = (!force_ && exists);
- if(!success) {
- const Data::Image& img = imageById(id_);
- if(!img.isNull()) {
- success = imgDir->writeImage(img);
- }
- }
+ bool success = writeCachedImage(id_, imgDir, force_);
if(success) {
// remove from dict and add to cache
@@ -310,6 +302,24 @@ bool ImageFactory::writeCachedImage(const QString& id_, CacheDir \
dir_, bool forc return success;
}
+bool ImageFactory::writeCachedImage(const QString& id_, ImageDirectory* imgDir_, \
bool force_ /*=false*/) { + if(id_.isEmpty() || !imgDir_) {
+ return false;
+ }
+// myLog() << "looking in" << imgDir_->path();
+ bool exists = imgDir_->hasImage(id_);
+ // only write if it doesn't exist
+ bool success = (!force_ && exists);
+ if(!success) {
+ const Data::Image& img = imageById(id_);
+ if(!img.isNull()) {
+// myLog() << "writing image";
+ success = imgDir_->writeImage(img);
+ }
+ }
+ return success;
+}
+
const Tellico::Data::Image& ImageFactory::imageById(const QString& id_) {
Q_ASSERT(factory);
if(id_.isEmpty() || !factory) {
@@ -374,37 +384,40 @@ const Tellico::Data::Image& ImageFactory::imageById(const \
QString& id_) {
// This section uses the config image setting plus the fallback location
// to provide confidence that the user's image can be found
- CacheDir dir1 = TempDir;
- CacheDir dir2 = TempDir;
- ImageDirectory* imgDir1 = 0;
- ImageDirectory* imgDir2 = 0;
+ CacheDir configLoc = TempDir;
+ CacheDir fallbackLoc = TempDir;
+ ImageDirectory* configImgDir = 0;
+ ImageDirectory* fallbackImgDir = 0;
if(Config::imageLocation() == Config::ImagesInLocalDir) {
- dir1 = LocalDir;
- dir2 = DataDir;
- imgDir1 = &factory->d->localImageDir;
- imgDir2 = &factory->d->dataImageDir;
+ configLoc = LocalDir;
+ fallbackLoc = DataDir;
+ configImgDir = &factory->d->localImageDir;
+ fallbackImgDir = &factory->d->dataImageDir;
} else if(Config::imageLocation() == Config::ImagesInAppDir) {
- dir1 = DataDir;
- dir2 = LocalDir;
- imgDir1 = &factory->d->dataImageDir;
- imgDir2 = &factory->d->localImageDir;
+ configLoc = DataDir;
+ fallbackLoc = LocalDir;
+ configImgDir = &factory->d->dataImageDir;
+ fallbackImgDir = &factory->d->localImageDir;
}
- if(imgDir1 && imgDir1->hasImage(id_)) {
- const Data::Image& img2 = factory->addCachedImageImpl(id_, dir1);
+ // check the configured location first
+ if(configImgDir && configImgDir->hasImage(id_)) {
+ const Data::Image& img2 = factory->addCachedImageImpl(id_, configLoc);
if(!img2.isNull()) {
+// myLog() << "found image in configured location" << configImgDir->path();
return img2;
} else {
- myDebug() << "tried to add" << id_ << "from" << imgDir1->path() << "but \
failed"; + myDebug() << "tried to add" << id_ << "from" << configImgDir->path() \
<< "but failed"; }
- } else if(imgDir2 && imgDir2->hasImage(id_)) {
- const Data::Image& img2 = factory->addCachedImageImpl(id_, dir2);
+ } else if(fallbackImgDir && fallbackImgDir->hasImage(id_)) {
+ const Data::Image& img2 = factory->addCachedImageImpl(id_, fallbackLoc);
if(!img2.isNull()) {
+// myLog() << "found image in fallback location" << fallbackImgDir->path();
// the img is in the other location
factory->emitImageMismatch();
return img2;
} else {
- myDebug() << "tried to add" << id_ << "from" << imgDir2->path() << "but \
failed"; + myDebug() << "tried to add" << id_ << "from" << \
fallbackImgDir->path() << "but failed"; }
}
// at this point, there's a possibility that the user has changed settings so that \
the images @@ -412,12 +425,14 @@ const Tellico::Data::Image& \
ImageFactory::imageById(const QString& id_) { \
if(factory->d->dataImageDir.hasImage(id_)) { const Data::Image& img2 = \
factory->addCachedImageImpl(id_, DataDir); if(!img2.isNull()) {
+// myLog() << "found image in data dir location";
return img2;
}
}
if(factory->d->localImageDir.hasImage(id_)) {
const Data::Image& img2 = factory->addCachedImageImpl(id_, LocalDir);
if(!img2.isNull()) {
+// myLog() << "found image in local dir location";
return img2;
}
}
@@ -432,7 +447,7 @@ const Tellico::Data::Image& ImageFactory::imageById(const \
QString& id_) { if(d.cd(cdString)) {
factory->d->localImageDir.setPath(d.path() + QDir::separator());
if(factory->d->localImageDir.hasImage(id_)) {
- myDebug() << "Reading image from old local directory" << (cdString+id_);
+// myDebug() << "Reading image from old local directory" << (cdString+id_);
const Data::Image& img2 = factory->addCachedImageImpl(id_, LocalDir);
// Be sure to reset the image directory location!!
factory->d->localImageDir.setPath(realImageDir);
diff --git a/src/images/imagefactory.h b/src/images/imagefactory.h
index 6a1e8ab..3d1d21d 100644
--- a/src/images/imagefactory.h
+++ b/src/images/imagefactory.h
@@ -41,6 +41,7 @@ namespace Tellico {
class Image;
class ImageInfo;
}
+ class ImageDirectory;
class StyleOptions {
public:
@@ -117,6 +118,7 @@ public:
static QString addImage(const QByteArray& data, const QString& format, const \
QString& id);
static bool writeCachedImage(const QString& id, CacheDir dir, bool force = false);
+ static bool writeCachedImage(const QString& id, ImageDirectory* dir, bool force = \
false);
/**
* Returns an image reference given its id. If none is found, a null image
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 69154bf..8eefa77 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1208,7 +1208,7 @@ bool MainWindow::fileSave() {
ret = fileSaveAs();
} else {
// special check: if there are more than 200 images AND the "Write Images In \
File" config key
- // is not set, then warn user that performance may suffer, and write result
+ // is set, then warn user that performance may suffer, and write result
if(Config::imageLocation() == Config::ImagesInFile &&
Config::askWriteImagesInFile() &&
Data::Document::self()->imageCount() > MAX_IMAGES_WARN_PERFORMANCE) {
diff --git a/src/tests/documenttest.cpp b/src/tests/documenttest.cpp
index d945573..697934e 100644
--- a/src/tests/documenttest.cpp
+++ b/src/tests/documenttest.cpp
@@ -88,36 +88,45 @@ void DocumentTest::testImageLocalDirectory() {
// clear the internal image cache
Tellico::ImageFactory::clean(true);
+ // verify that the images are copied from the old directory when saving to a new \
file + QString fileName2 = tempDirName + "/with-image2.tc";
+ QString imageDirName2 = tempDirName + "/with-image2_files/";
+ QVERIFY(doc->saveDocument(QUrl::fromLocalFile(fileName2)));
+ QVERIFY(QFile::exists(fileName2));
+ QDir imageDir2(imageDirName2);
+ QVERIFY(imageDir2.exists());
+ QVERIFY(imageDir2.exists(e->field(QLatin1String("cover"))));
+
/*************************************************************************/
/* now also verify image directory when file name has multiple periods */
/* see https://bugs.kde.org/show_bug.cgi?id=348088 */
/* also have to check backwards compatibility with prior behavior */
/*************************************************************************/
- QString fileName2 = tempDirName + "/with-image.1.tc";
- QString imageDirName2 = tempDirName + "/with-image.1_files/";
+ QString fileName3 = tempDirName + "/with-image.1.tc";
+ QString imageDirName3 = tempDirName + "/with-image.1_files/";
// copy the collection file, which no longer contains the images inside
- QVERIFY(QFile::copy(fileName, fileName2));
- QVERIFY(doc->openDocument(QUrl::fromLocalFile(fileName2)));
- QCOMPARE(Tellico::ImageFactory::localDir(), imageDirName2);
- QDir imageDir2(imageDirName2);
+ QVERIFY(QFile::copy(fileName, fileName3));
+ QVERIFY(doc->openDocument(QUrl::fromLocalFile(fileName3)));
+ QCOMPARE(Tellico::ImageFactory::localDir(), imageDirName3);
+ QDir imageDir3(imageDirName3);
// verify that the images can be loaded from the image directory that does NOT \
have multiple periods // since that was the behavior prior to the bug being fixed
coll = doc->collection();
e = coll->entries().at(0);
// image should not be in the next image dir yet since we haven't saved
- QVERIFY(!imageDir2.exists(e->field(QLatin1String("cover"))));
+ QVERIFY(!imageDir3.exists(e->field(QLatin1String("cover"))));
QVERIFY(!Tellico::ImageFactory::imageById(e->field("cover")).isNull());
// now remove the first image from the first image directory, save the document, \
and verify that // the proper image exists and is written
QVERIFY(imageDir.remove(e->field("cover")));
QVERIFY(!imageDir.exists(e->field(QLatin1String("cover"))));
- QVERIFY(doc->saveDocument(QUrl::fromLocalFile(fileName2)));
+ QVERIFY(doc->saveDocument(QUrl::fromLocalFile(fileName3)));
// now the file should exist in the proper location
- QVERIFY(imageDir2.exists(e->field(QLatin1String("cover"))));
+ QVERIFY(imageDir3.exists(e->field(QLatin1String("cover"))));
// clear the cache
Tellico::ImageFactory::clean(true);
QVERIFY(!Tellico::ImageFactory::imageById(e->field("cover")).isNull());
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic