Commit e0204e40 authored by Jim Nelson's avatar Jim Nelson

#2666: Mark photo as dirty whenever the appropriate metadata is updated, even...

#2666: Mark photo as dirty whenever the appropriate metadata is updated, even if autocommit-metadata is disabled.  This persists the dirty flag at all times, meaning if it is enabled MetadataWriter will commit all changes to the backing files.
parent baa2f0ea
......@@ -128,7 +128,7 @@ SRC_FILES = \
Tombstone.vala \
MetadataWriter.vala \
Application.vala \
DelayedQueue.vala \
TimedQueue.vala \
MediaPage.vala
ifndef LINUX
......
......@@ -483,12 +483,13 @@ private DatabaseVerifyResult upgrade_database(int version) {
//
// Version 9:
// * Added metadata_dirty flag to PhotoTable.
// * Added metadata_dirty flag to PhotoTable. Default to 1 rather than 0 on upgrades so
// changes to metadata prior to upgrade will be caught by MetadataWriter.
//
if (!DatabaseTable.has_column("PhotoTable", "metadata_dirty")) {
message("upgrade_database: adding metadata_dirty column to PhotoTable");
if (!DatabaseTable.add_column("PhotoTable", "metadata_dirty", "INTEGER DEFAULT 0"))
if (!DatabaseTable.add_column("PhotoTable", "metadata_dirty", "INTEGER DEFAULT 1"))
return DatabaseVerifyResult.UPGRADE_ERROR;
}
......
......@@ -4,6 +4,13 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
// MetadataWriter tracks LibraryPhotos for alterations to their metadata and commits those changes
// in a timely manner to their backing files. Because only the MetadataWriter knows when the
// metadata has been properly committed, it is also responsible for updating the metadata-dirty
// flag in Photo. Thus, MetadataWriter should *always* be running, even if the user has turned off
// the feature, so if they turn it on MetadataWriter can properly go out and update the backing
// files.
public class MetadataWriter : Object {
public const uint COMMIT_DELAY_MSEC = 3000;
public const uint COMMIT_SPACING_MSEC = 50;
......@@ -89,6 +96,7 @@ public class MetadataWriter : Object {
changed = true;
}
// add the software name/version only if updating the metadata in the file
if (changed)
metadata.set_software(Resources.APP_TITLE, Resources.APP_VERSION);
......@@ -99,7 +107,7 @@ public class MetadataWriter : Object {
private static MetadataWriter instance = null;
private Workers workers = new Workers(Workers.thread_per_cpu_minus_one(), false);
private HashDelayedQueue<LibraryPhoto> dirty;
private HashTimedQueue<LibraryPhoto> dirty;
private Gee.HashMap<LibraryPhoto, CommitJob> pending = new Gee.HashMap<LibraryPhoto, CommitJob>();
private Gee.HashSet<string> interested_photo_details = new Gee.HashSet<string>();
private LibraryPhoto? ignore_photo_alteration = null;
......@@ -110,7 +118,7 @@ public class MetadataWriter : Object {
public signal void progress(uint completed, uint total);
private MetadataWriter() {
dirty = new HashDelayedQueue<LibraryPhoto>(COMMIT_DELAY_MSEC, on_photo_dequeued);
dirty = new HashTimedQueue<LibraryPhoto>(COMMIT_DELAY_MSEC, on_photo_dequeued);
dirty.set_dequeue_spacing_msec(COMMIT_SPACING_MSEC);
// convert all interested metadata Alteration details into lookup hash
......@@ -172,9 +180,6 @@ public class MetadataWriter : Object {
private void on_photos_added_removed(Gee.Iterable<DataObject>? added,
Gee.Iterable<DataObject>? removed) {
if (!autocommit_metadata)
return;
if (added != null) {
Gee.ArrayList<LibraryPhoto> photos = null;
foreach (DataObject object in added) {
......@@ -188,7 +193,7 @@ public class MetadataWriter : Object {
}
if (photos != null)
enqueue_many(photos, "dirty photo added to LibraryPhoto.global");
photos_dirty(photos, "dirty photo added to LibraryPhoto.global");
}
if (removed != null) {
......@@ -198,9 +203,6 @@ public class MetadataWriter : Object {
}
private void on_photos_altered(Gee.Map<DataObject, Alteration> items) {
if (!autocommit_metadata)
return;
Gee.HashSet<LibraryPhoto> photos = null;
foreach (DataObject object in items.keys) {
LibraryPhoto photo = (LibraryPhoto) object;
......@@ -210,8 +212,7 @@ public class MetadataWriter : Object {
if (photo == ignore_photo_alteration)
continue;
Alteration alteration = items.get(object);
Gee.Collection<string>? details = alteration.get_details("metadata");
Gee.Collection<string>? details = items.get(object).get_details("metadata");
if (details == null)
continue;
......@@ -229,61 +230,55 @@ public class MetadataWriter : Object {
}
if (photos != null)
enqueue_many(photos, "alteration");
photos_dirty(photos, "alteration");
}
private void on_tag_altered(ContainerSource container, Gee.Collection<DataSource>? added,
Gee.Collection<DataSource>? removed) {
if (!autocommit_metadata)
return;
Tag tag = (Tag) container;
if (added != null)
enqueue_many((Gee.Collection<LibraryPhoto>) added, "added to %s".printf(tag.to_string()));
photos_dirty((Gee.Collection<LibraryPhoto>) added, "added to %s".printf(tag.to_string()));
if (removed != null)
enqueue_many((Gee.Collection<LibraryPhoto>) removed, "removed from %s".printf(tag.to_string()));
photos_dirty((Gee.Collection<LibraryPhoto>) removed, "removed from %s".printf(tag.to_string()));
}
private void enqueue(LibraryPhoto photo, string reason) {
cancel_job(photo);
if (dirty.contains(photo)) {
bool removed = dirty.remove_first(photo);
assert(removed);
private void photos_dirty(Gee.Collection<LibraryPhoto> photos, string reason) {
// cancel all outstanding and pending jobs
foreach (LibraryPhoto photo in photos) {
cancel_job(photo);
assert(!dirty.contains(photo));
if (dirty.contains(photo)) {
bool removed = dirty.remove_first(photo);
assert(removed);
assert(!dirty.contains(photo));
}
}
// mark as dirty; if app is closed before written out, MetadataWriter will try again when
// the app is restarted
// mark all the photos as dirty
try {
photo.set_master_metadata_dirty(true);
Photo.set_many_master_metadata_dirty(LibraryPhoto.global, photos, true);
} catch (DatabaseError err) {
AppWindow.database_error(err);
}
// ok to drop this on the floor -- now that it's marked dirty, will attempt to write next
// time the MetadataWriter runs
if (closed)
// ok to drop this on the floor, now that they're marked dirty (will attempt to write them
// out the next time MetadataWriter runs)
if (closed || !autocommit_metadata)
return;
foreach (LibraryPhoto photo in photos) {
#if TRACE_METADATA_WRITER
debug("Enqueuing %s for metadata commit: %s", photo.to_string(), reason);
debug("Enqueuing %s for metadata commit: %s", photo.to_string(), reason);
#endif
bool enqueued = dirty.enqueue(photo);
assert(enqueued);
bool enqueued = dirty.enqueue(photo);
assert(enqueued);
}
outstanding_total = dirty.size + pending.size;
outstanding_completed = 0;
progress(outstanding_completed, outstanding_total);
}
private void enqueue_many(Gee.Collection<LibraryPhoto> photos, string reason) {
foreach (LibraryPhoto photo in photos)
enqueue(photo, reason);
}
private void cancel_job(LibraryPhoto photo) {
......@@ -329,10 +324,11 @@ public class MetadataWriter : Object {
if (++outstanding_completed >= outstanding_total) {
outstanding_completed = 0;
outstanding_total = 0;
// fire this to specify a reset
progress(outstanding_completed, outstanding_total);
}
progress(outstanding_completed, outstanding_total);
if (job.reimport_master_state != null || job.reimport_editable_state != null) {
#if TRACE_METADATA_WRITER
debug("[%u/%u] %s metadata committed", outstanding_completed, outstanding_total,
......@@ -355,6 +351,9 @@ public class MetadataWriter : Object {
assert(ignore_photo_alteration == job.photo);
ignore_photo_alteration = null;
}
if (outstanding_total > 0)
progress(outstanding_completed, outstanding_total);
} else {
#if TRACE_METADATA_WRITER
debug("[%u/%u] No metadata changes for %s", outstanding_completed, outstanding_total,
......
......@@ -1393,6 +1393,29 @@ public abstract class Photo : PhotoSource {
}
}
// If the SourceCollection is provided, it will be frozen and thawed to generate a singal
// items-altered signal.
public static void set_many_master_metadata_dirty(SourceCollection? sources,
Gee.Collection<Photo> photos, bool dirty) throws DatabaseError {
if (sources != null)
sources.freeze_notifications();
PhotoTable.get_instance().begin_transaction();
foreach (Photo photo in photos) {
try {
photo.set_master_metadata_dirty(dirty);
} catch (DatabaseError err) {
AppWindow.database_error(err);
}
}
PhotoTable.get_instance().commit_transaction();
if (sources != null)
sources.thaw_notifications();
}
public void set_master_metadata_dirty(bool dirty) throws DatabaseError {
bool committed = false;
lock (row) {
......
......@@ -4,7 +4,7 @@
* (version 2.1 or later). See the COPYING file in this distribution.
*/
// DelayedQueue is a specialized collection class. It holds items in order, but rather than being
// TimedQueue is a specialized collection class. It holds items in order, but rather than being
// manually dequeued, they are dequeued automatically after a specified amount of time has elapsed
// for that item. As of today, it's possible the item will be dequeued a bit later than asked
// for, but it will never be early. Future implementations might tighten up the lateness.
......@@ -13,12 +13,12 @@
// a bug with passing an unnamed type as a signal parameter:
// https://bugzilla.gnome.org/show_bug.cgi?id=628639
//
// The rate the items come off the queue can be spaced out. Note that this can cause hysteresis.
// As of today, DelayedQueue makes no effort to combat this.
// The rate the items come off the queue can be spaced out. Note that this can cause items to back
// up. As of today, TimedQueue makes no effort to combat this.
public delegate void DequeuedCallback<G>(G item);
public class DelayedQueue<G> {
public class TimedQueue<G> {
private class Element<G> {
public G item;
public time_t ready;
......@@ -45,7 +45,7 @@ public class DelayedQueue<G> {
// Initial design was to have a signal that passed the dequeued G, but bug in valac meant
// finding a workaround, namely using a delegate:
// https://bugzilla.gnome.org/show_bug.cgi?id=628639
public DelayedQueue(uint hold_msec, DequeuedCallback<G> callback, EqualFunc? equal_func = null,
public TimedQueue(uint hold_msec, DequeuedCallback<G> callback, EqualFunc? equal_func = null,
int priority = Priority.DEFAULT) requires (hold_msec > 0) {
this.hold_msec = hold_msec;
this.callback = callback;
......@@ -57,7 +57,7 @@ public class DelayedQueue<G> {
timer_id = Timeout.add(get_heartbeat_timeout(), on_heartbeat, priority);
}
~DelayedQueue() {
~TimedQueue() {
if (timer_id != 0)
Source.remove(timer_id);
}
......@@ -105,6 +105,16 @@ public class DelayedQueue<G> {
return queue.add(new Element<G>(item, calc_ready_time()));
}
public virtual bool enqueue_many(Gee.Collection<G> items) {
time_t ready_time = calc_ready_time();
Gee.ArrayList<Element<G>> elements = new Gee.ArrayList<Element<G>>();
foreach (G item in items)
elements.add(new Element<G>(item, ready_time));
return queue.add_list(elements);
}
public virtual bool remove_first(G item) {
Gee.Iterator<Element<G>> iter = queue.iterator();
while (iter.next()) {
......@@ -165,12 +175,12 @@ public class DelayedQueue<G> {
}
}
// HashDelayedQueue uses a HashMap for quick lookups of elements via contains().
// HashTimedQueue uses a HashMap for quick lookups of elements via contains().
public class HashDelayedQueue<G> : DelayedQueue<G> {
public class HashTimedQueue<G> : TimedQueue<G> {
private Gee.HashMap<G, int> item_count;
public HashDelayedQueue(uint hold_msec, DequeuedCallback<G> callback, HashFunc? hash_func = null,
public HashTimedQueue(uint hold_msec, DequeuedCallback<G> callback, HashFunc? hash_func = null,
EqualFunc? equal_func = null, int priority = Priority.DEFAULT) {
base (hold_msec, callback, equal_func, priority);
......@@ -202,6 +212,16 @@ public class HashDelayedQueue<G> : DelayedQueue<G> {
return true;
}
public override bool enqueue_many(Gee.Collection<G> items) {
if (!base.enqueue_many(items))
return false;
foreach (G item in items)
item_count.set(item, item_count.has_key(item) ? item_count.get(item) + 1 : 1);
return true;
}
public override bool remove_first(G item) {
if (!base.remove_first(item))
return false;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment