store: ensure updates update message sexp too

And turn all "add" into "replace" so old messages get removed.
Update tests too.

lib/message/meson.build

@@ -20,7 +20,6 @@ lib_mu_message=static_library(
   [
     'mu-message.cc',
     'mu-message-part.cc',
-    'mu-message-sexp.cc',
     'mu-contact.cc',
     'mu-document.cc',
     'mu-fields.cc',

lib/mu-store.cc

@@ -66,16 +66,12 @@ struct Store::Private {
 	enum struct XapianOpts { ReadOnly, Open, CreateOverwrite };
 
 	Private(const std::string& path, bool readonly)
-	    : read_only_{readonly}, db_{make_xapian_db(path,
+		: read_only_{readonly}, db_{make_xapian_db(path,
-						       read_only_ ? XapianOpts::ReadOnly
+							   read_only_ ? XapianOpts::ReadOnly
-								  : XapianOpts::Open)},
+							   : XapianOpts::Open)},
-	      properties_{make_properties(path)},
+		  properties_{make_properties(path)},
-	      contacts_cache_{db().get_metadata(ContactsKey),
+		  contacts_cache_{db().get_metadata(ContactsKey),
 		properties_.personal_addresses} {
-		/* we do our own flushing, set Xapian's internal one as the backstop*/
-		g_setenv("XAPIAN_FLUSH_THRESHOLD",
-			 format("%zu", properties_.batch_size + 100).c_str(), 1);
-
 	}
 
 	Private(const std::string& path,
@@ -85,9 +81,20 @@ struct Store::Private {
 	    : read_only_{false}, db_{make_xapian_db(path, XapianOpts::CreateOverwrite)},
 	      properties_{init_metadata(conf, path, root_maildir, personal_addresses)},
 	      contacts_cache_{"", properties_.personal_addresses} {
-				/* we do our own flushing, set Xapian's internal one as the backstop*/
+
-		g_setenv("XAPIAN_FLUSH_THRESHOLD",
+		/* add synonym */
-			 format("%zu", properties_.batch_size + 100).c_str(), 1);
+		for (auto&& info: AllMessageFlagInfos) {
+			constexpr auto field{field_from_id(Field::Id::Flags)};
+			const auto s1{field.xapian_term(info.name)};
+			const auto s2{field.xapian_term(info.shortcut)};
+			writable_db().add_synonym(s1, s2);
+		}
+		for (auto&& prio : AllMessagePriorities) {
+			constexpr auto field{field_from_id(Field::Id::Priority)};
+			const auto s1{field.xapian_term(to_string(prio))};
+			const auto s2{field.xapian_term(to_char(prio))};
+			writable_db().add_synonym(s1, s2);
+		}
 	}
 
 	~Private()
@@ -102,15 +109,23 @@ struct Store::Private {
 
 	std::unique_ptr<Xapian::Database> make_xapian_db(const std::string db_path, XapianOpts opts)
 	try {
+
+		/* we do our own flushing, set Xapian's internal one as the
+		 * backstop*/
+		g_setenv("XAPIAN_FLUSH_THRESHOLD",
+			 format("%zu", properties_.batch_size + 100).c_str(), 1);
+
 		switch (opts) {
-		case XapianOpts::ReadOnly: return std::make_unique<Xapian::Database>(db_path);
+		case XapianOpts::ReadOnly:
+			return std::make_unique<Xapian::Database>(db_path);
 		case XapianOpts::Open:
 			return std::make_unique<Xapian::WritableDatabase>(db_path, Xapian::DB_OPEN);
 		case XapianOpts::CreateOverwrite:
 			return std::make_unique<Xapian::WritableDatabase>(
-			    db_path,
+				db_path,
 			    Xapian::DB_CREATE_OR_OVERWRITE);
-		default: throw std::logic_error("invalid xapian options");
+		default:
+			throw std::logic_error("invalid xapian options");
 		}
 
 	} catch (const Xapian::DatabaseError& xde) {
@@ -145,8 +160,7 @@ struct Store::Private {
 
 	// Opportunistically commit a transaction if the transaction size
 	// filled up a batch, or with force.
-	void transaction_maybe_commit(bool force = false) noexcept
+	void transaction_maybe_commit(bool force = false) noexcept {
-	{
 		if (force || transaction_size_ >= properties_.batch_size) {
 			if (contacts_cache_.dirty()) {
 				xapian_try([&] {
@@ -168,29 +182,7 @@ struct Store::Private {
 		}
 	}
 
-	void add_synonyms()
+	time_t metadata_time_t(const std::string& key) const {
-	{
-		for (auto&& info: AllMessageFlagInfos) {
-			constexpr auto field{field_from_id(Field::Id::Flags)};
-			const auto s1{field.xapian_term(info.name)};
-			const auto s2{field.xapian_term(info.shortcut)};
-			writable_db().clear_synonyms(s1);
-			writable_db().clear_synonyms(s2);
-			writable_db().add_synonym(s1, s2);
-		}
-
-		for (auto&& prio : AllMessagePriorities) {
-			constexpr auto field{field_from_id(Field::Id::Priority)};
-			const auto s1{field.xapian_term(to_string(prio))};
-			const auto s2{field.xapian_term(to_char(prio))};
-			writable_db().clear_synonyms(s1);
-			writable_db().clear_synonyms(s2);
-			writable_db().add_synonym(s1, s2);
-		}
-	}
-
-	time_t metadata_time_t(const std::string& key) const
-	{
 		const auto ts = db().get_metadata(key);
 		return (time_t)atoll(db().get_metadata(key).c_str());
 	}
@@ -214,8 +206,8 @@ struct Store::Private {
 	Store::Properties init_metadata(const Store::Config& conf,
 					const std::string&   path,
 					const std::string&   root_maildir,
-					const StringVec&     personal_addresses)
+					const StringVec&     personal_addresses) {
-		{
+
 		writable_db().set_metadata(SchemaVersionKey, ExpectedSchemaVersion);
 		writable_db().set_metadata(CreatedKey, Mu::format("%" PRId64, (int64_t)::time({})));
 
@@ -240,6 +232,10 @@ struct Store::Private {
 		return make_properties(path);
 	}
 
+	Option<Message> find_message_unlocked(Store::Id docid) const;
+	Result<Store::Id> update_message_unlocked(Message& msg, Store::Id docid);
+	Result<Store::Id> update_message_unlocked(Message& msg, const std::string& old_path);
+
 	/* metadata to write as part of a transaction commit */
 	std::unordered_map<std::string, std::string> metadata_cache_;
 
@@ -254,6 +250,47 @@ struct Store::Private {
 	std::mutex lock_;
 };
 
+Result<Store::Id>
+Store::Private::update_message_unlocked(Message& msg, Store::Id docid)
+{
+	msg.update_cached_sexp();
+
+	return xapian_try_result([&]{
+		writable_db().replace_document(docid, msg.document().xapian_document());
+		g_debug("updated message @ %s; docid = %u", msg.path().c_str(), docid);
+		return Ok(std::move(docid));
+	});
+}
+
+Result<Store::Id>
+Store::Private::update_message_unlocked(Message& msg, const std::string& path_to_replace)
+{
+	msg.update_cached_sexp();
+
+	return xapian_try_result([&]{
+		auto id = writable_db().replace_document(
+			field_from_id(Field::Id::Path).xapian_term(path_to_replace),
+			msg.document().xapian_document());
+
+		return Ok(std::move(id));
+	});
+}
+
+Option<Message>
+Store::Private::find_message_unlocked(Store::Id docid) const
+{
+	return xapian_try([&]()->Option<Message> {
+		auto res = Message::make_from_document(db().get_document(docid));
+		if (res)
+			return Some(std::move(res.value()));
+		else
+			return Nothing;
+		}, Nothing);
+}
+
+
+
+
 Store::Store(const std::string& path, bool readonly)
     : priv_{std::make_unique<Private>(path, readonly)}
 {
@@ -293,12 +330,6 @@ Store::database() const
 	return priv_->db();
 }
 
-Xapian::WritableDatabase&
-Store::writable_database()
-{
-	return priv_->writable_db();
-}
-
 Indexer&
 Store::indexer()
 {
@@ -337,6 +368,8 @@ Store::add_message(const std::string& path, bool use_transaction)
 Result<Store::Id>
 Store::add_message(Message& msg, bool use_transaction)
 {
+	std::lock_guard guard{priv_->lock_};
+
 	const auto mdir{mu_maildir_from_path(msg.path(),
 					     properties().root_maildir)};
 	if (!mdir)
@@ -356,51 +389,28 @@ Store::add_message(Message& msg, bool use_transaction)
 	if (is_personal)
 		msg.set_flags(msg.flags() | Flags::Personal);
 
-	/* now, we're done with all the fields; generate the sexp string for this
+	if (use_transaction)
-	 * message */
+		priv_->transaction_inc();
-	msg.update_cached_sexp();
 
-	std::lock_guard guard{priv_->lock_};
+	auto res = priv_->update_message_unlocked(msg, msg.path());
+	if (!res)
+		return Err(res.error());
 
-	const auto docid = xapian_try([&]{
+	if (use_transaction) /* commit if batch is full */
+		priv_->transaction_maybe_commit();
 
-		if (use_transaction)
+	g_debug("added message @ %s; docid = %u", msg.path().c_str(), *res);
-			priv_->transaction_inc();
 
-		const auto docid = priv_->writable_db().add_document(
+	return res;
-			msg.document().xapian_document());
-
-		if (use_transaction) /* commit if batch is full */
-			priv_->transaction_maybe_commit();
-
-		return docid;
-	}, InvalidId);
-
-	if (G_UNLIKELY(docid == InvalidId))
-		return Err(Error::Code::Message, "failed to add message");
-
-	g_debug("added message @ %s; docid = %u", msg.path().c_str(), docid);
-	g_debug("%s", msg.document().xapian_document().get_description().c_str());
-
-	return Ok(static_cast<Store::Id>(docid));
 }
 
 
-bool
+Result<Store::Id>
-Store::update_message(Message& msg, unsigned docid)
+Store::update_message(Message& msg, Store::Id docid)
 {
-	msg.update_cached_sexp();
-
 	std::lock_guard guard{priv_->lock_};
 
-	return xapian_try(
+	return priv_->update_message_unlocked(msg, docid);
-		[&]{
-			priv_->writable_db().replace_document(
-				docid, msg.document().xapian_document());
-			g_debug("updated message @ %s; docid = %u",
-				msg.path().c_str(), docid);
-			return true;
-	}, false);
 }
 
 bool
@@ -435,48 +445,52 @@ Store::remove_messages(const std::vector<Store::Id>& ids)
 }
 
 
+Option<Message>
+Store::find_message(Store::Id docid) const
+{
+	std::lock_guard guard{priv_->lock_};
+
+	return priv_->find_message_unlocked(docid);
+}
+
+
 Result<Message>
 Store::move_message(Store::Id id,
 		    Option<const std::string&> target_mdir,
 		    Option<Flags> new_flags, bool change_name)
 {
-	auto msg = find_message(id);
+	std::lock_guard guard{priv_->lock_};
+
+	auto msg = priv_->find_message_unlocked(id);
 	if (!msg)
 		return Err(Error::Code::Store, "cannot find message <%u>", id);
 
+	const auto	old_path       = msg->path();
 	const auto	target_flags   = new_flags.value_or(msg->flags());
 	const auto	target_maildir = target_mdir.value_or(msg->maildir());
 
 	/* 1. first determine the file system path of the target */
 	const auto target_path =
-		mu_maildir_determine_target(msg->path(),
+		mu_maildir_determine_target(msg->path(), properties().root_maildir,
-					    properties().root_maildir,
+					    target_maildir,target_flags, change_name);
-					    target_maildir,
-					    target_flags,
-					    change_name);
 	if (!target_path)
 		return Err(target_path.error());
 
 	/* 2. let's move it */
-	const auto move_res =
+	if (const auto res = mu_maildir_move_message(
-		mu_maildir_move_message(msg->path(),
+		    msg->path(), target_path.value(), true/*ignore dups*/); !res)
-					target_path.value(),
+		return Err(res.error());
-					true/*ignore dups*/);
-	if (!move_res)
-		return Err(move_res.error());
 
 	/* 3. file move worked, now update the message with the new info.*/
-	const auto update_res = msg->update_after_move(target_path.value(),
+	if (auto&& res = msg->update_after_move(
-						       target_maildir,
+		    target_path.value(), target_maildir, target_flags); !res)
-						       target_flags);
+		return Err(res.error());
-	if (!update_res)
-		return Err(update_res.error());
 
 	/* 4. update message worked; re-store it */
-	if (!update_message(*msg, id))
+	if (auto&& res = priv_->update_message_unlocked(*msg, old_path); !res)
-		return Err(Error::Code::Store, "failed to update message <%u>", id);
+		return Err(res.error());
 
-	/* 5. Profit! */
+	/* 6. Profit! */
 	return Ok(std::move(msg.value()));
 }
 
@@ -529,21 +543,6 @@ Store::set_dirstamp(const std::string& path, time_t tstamp)
 	set_metadata(path, std::string{data.data(), len});
 }
 
-Option<Message>
-Store::find_message(Store::Id docid) const
-{
-	return xapian_try(
-	    [&]()->Option<Message> {
-		    std::lock_guard guard{priv_->lock_};
-		    auto res = Message::make_from_document(priv_->db().get_document(docid));
-		    if (res)
-			    return Some(std::move(res.value()));
-		    else
-			    return Nothing;
-	    },
-	    Nothing);
-}
-
 bool
 Store::contains_message(const std::string& path) const
 {
@@ -599,6 +598,7 @@ Store::for_each_term(Field::Id field_id, Store::ForEachTermFunc func) const
 		const auto prefix{field_from_id(field_id).xapian_term()};
 		for (auto it = priv_->db().allterms_begin(prefix);
 		     it != priv_->db().allterms_end(prefix); ++it) {
+			++n;
 			if (!func(*it))
 				break;
 		}

lib/mu-store.hh

@@ -67,6 +67,7 @@ public:
 	 * @param maildir maildir to use for this store
 	 * @param personal_addresses addresses that should be recognized as
 	 * 'personal' for identifying personal messages.
+	 * @param config a configuration object
 	 */
 	Store(const std::string& path,
 	      const std::string& maildir,
@@ -116,14 +117,6 @@ public:
 	 */
 	const Xapian::Database& database() const;
 
-	/**
-	 * Get the underlying writable Xapian database for this
-	 * store. Throws is this store is not writable.
-	 *
-	 * @return the writable database
-	 */
-	Xapian::WritableDatabase& writable_database();
-
 	/**
 	 * Get the Indexer associated with this store. It is an error to call
 	 * this on a read-only store.
@@ -205,9 +198,9 @@ public:
 	 * @param msg a message
 	 * @param id the id for this message
 	 *
-	 * @return false in case of failure; true otherwise.
+	 * @return Ok() or an error.
 	 */
-	bool update_message(Message& msg, Id id);
+	Result<Store::Id> update_message(Message& msg, Id id);
 
 	/**
 	 * Remove a message from the store. It will _not_ remove the message
@@ -384,6 +377,7 @@ public:
 	const std::unique_ptr<Private>& priv() const { return priv_; }
 
 private:
+
 	std::unique_ptr<Private> priv_;
 };
 

lib/tests/test-mu-store.cc

@@ -21,8 +21,10 @@
 
 #include <glib.h>
 #include <stdlib.h>
+#include <thread>
 #include <unistd.h>
 #include <time.h>
+#include <fstream>
 
 #include <locale.h>
 
@@ -30,6 +32,7 @@
 #include "mu-store.hh"
 #include "utils/mu-result.hh"
 #include <utils/mu-utils.hh>
+#include "mu-maildir.hh"
 
 using namespace Mu;
 
@@ -224,6 +227,93 @@ World!
 }
 
 
+static void
+test_index_move()
+{
+	using namespace std::chrono_literals;
+
+	 const std::string msg_text =
+R"(From: Valentine Michael Smith <mike@example.com>
+To: Raul Endymion <raul@example.com>
+Cc: emacs-devel@gnu.org
+Subject: Re: multi-eq hash tables
+Date: Tue, 03 May 2022 20:58:02 +0200
+Message-ID: <87h766tzzz.fsf@gnus.org>
+MIME-Version: 1.0
+Content-Type: text/plain
+Precedence: list
+List-Id: "Emacs development discussions." <emacs-devel.gnu.org>
+List-Post: <mailto:emacs-devel@gnu.org>
+
+Raul Endymion <raul@example.com> writes:
+
+> Maybe we should introduce something like:
+>
+>     (define-hash-table-test shallow-equal
+>       (lambda (x1 x2) (while (and (consp x1) (consp x2) (eql (car x1) (car x2)))
+>                         (setq x1 (cdr x1)) (setq x2 (cdr x2)))
+>                       (equal x1 x2)))
+>       ...)
+
+Yes, that would be excellent.
+)";
+
+	 TempDir tempdir2;
+
+	 { // create a message file.
+		 const auto res1 = mu_maildir_mkdir(tempdir2.path() + "/Maildir/a");
+		 assert_valid_result(res1);
+
+		 std::ofstream output{tempdir2.path() + "/Maildir/a/new/msg"};
+		 output.write(msg_text.c_str(), msg_text.size());
+		 output.close();
+		 g_assert_true(output.good());
+	 }
+
+	 // Index it into a store.
+	 TempDir tempdir;
+	 Store store{tempdir.path(), tempdir2.path() + "/Maildir", {}, {}};
+	 store.indexer().start({});
+	 size_t n{};
+	 while (store.indexer().is_running()) {
+		 std::this_thread::sleep_for(100ms);
+		 g_assert_cmpuint(n++,<=,25);
+	 }
+	 g_assert_true(!store.indexer().is_running());
+	 const auto& prog{store.indexer().progress()};
+	 g_assert_cmpuint(prog.updated,==,1);
+	 g_assert_cmpuint(store.size(), ==, 1);
+	 g_assert_false(store.empty());
+
+	 // Find the message
+	 auto qr = store.run_query("path:" + tempdir2.path() + "/Maildir/a/new/msg");
+	 assert_valid_result(qr);
+	 g_assert_cmpuint(qr->size(),==,1);
+
+	 const auto msg = qr->begin().message();
+	 g_assert_true(!!msg);
+
+	 // Check the message
+	 const auto oldpath{msg->path()};
+	 assert_equal(msg->subject(), "Re: multi-eq hash tables");
+	 g_assert_true(msg->docid() != 0);
+	 g_debug("%s", msg->to_sexp().to_sexp_string().c_str());
+
+	 // Move the message from new->cur
+	 std::this_thread::sleep_for(1s); /* ctime should change */
+	 const auto msg3 = store.move_message(msg->docid(), {}, Flags::Seen);
+	 assert_valid_result(msg3);
+	 assert_equal(msg3->maildir(), "/a");
+	 assert_equal(msg3->path(), tempdir2.path() + "/Maildir/a/cur/msg:2,S");
+	 g_assert_true(::access(msg3->path().c_str(), R_OK)==0);
+	 g_assert_false(::access(oldpath.c_str(), R_OK)==0);
+
+	 g_debug("%s", msg3->to_sexp().to_sexp_string().c_str());
+	 g_assert_cmpuint(store.size(), ==, 1);
+}
+
+
+
 int
 main(int argc, char* argv[])
 {
@@ -236,6 +326,8 @@ main(int argc, char* argv[])
 			test_message_mailing_list);
 	g_test_add_func("/store/message/attachments",
 			test_message_attachments);
+	g_test_add_func("/store/index/move",
+			test_index_move);
 
 	return g_test_run();
 }

lib/utils/mu-sexp.cc

@@ -190,7 +190,12 @@ Sexp::to_sexp_string() const
 		sstrm << ')';
 		break;
 	}
-	case Type::String: sstrm << quote(value()); break;
+	case Type::String:
+		sstrm << quote(value());
+		break;
+	case Type::Raw:
+		sstrm << value();
+		break;
 	case Type::Number:
 	case Type::Symbol:
 	case Type::Empty:
@@ -231,7 +236,12 @@ Sexp::to_json_string() const
 		}
 		break;
 	}
-	case Type::String: sstrm << quote(value()); break;
+	case Type::String:
+		sstrm << quote(value());
+		break;
+	case Type::Raw: // FIXME: implement this.
+		break;
+
 	case Type::Symbol:
 		if (is_nil())
 			sstrm << "false";

lib/utils/mu-sexp.hh

@@ -1,5 +1,5 @@
 /*
-** Copyright (C) 2021 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
+** Copyright (C) 2022 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
 **
 ** This program is free software; you can redistribute it and/or modify it
 ** under the terms of the GNU General Public License as published by the
@@ -37,7 +37,7 @@ namespace Mu {
 /// Parse node
 struct Sexp {
 	/// Node type
-	enum struct Type { Empty, List, String, Number, Symbol };
+	enum struct Type { Empty, List, String, Number, Symbol, Raw };
 
 	/**
 	 * Default CTOR
@@ -81,13 +81,30 @@ struct Sexp {
 	}
 
 	static Sexp make_number(int val) { return Sexp{Type::Number, format("%d", val)}; }
-	static Sexp make_symbol(std::string&& val)
+	static Sexp make_symbol(std::string&& val) {
-	{
 		if (val.empty())
-			throw Error(Error::Code::InvalidArgument, "symbol must be non-empty");
+			throw Error(Error::Code::InvalidArgument,
+				    "symbol must be non-empty");
 		return Sexp{Type::Symbol, std::move(val)};
 	}
-	static Sexp make_symbol_sv(std::string_view val) { return make_symbol(std::string{val}); }
+	static Sexp make_symbol_sv(std::string_view val) {
+		return make_symbol(std::string{val});
+	}
+
+	/**
+	 * Add a raw string sexp.
+	 *
+	 * @param val value
+	 *
+	 * @return A sexp
+	 */
+	static Sexp make_raw(std::string&& val) {
+		return Sexp{Type::Raw, std::string{val}};
+	}
+	static Sexp make_raw(const std::string& val) {
+		return make_raw(std::string{val});
+	}
+
 
 	/**
 	 *
@@ -96,8 +113,7 @@ struct Sexp {
 	 *
 	 * @return
 	 */
-	const std::string& value() const
+	const std::string& value() const {
-	{
 		if (is_list())
 			throw Error(Error::Code::InvalidArgument, "no value for list");
 		if (is_empty())
@@ -110,15 +126,14 @@ struct Sexp {
 	 *
 	 * @return
 	 */
-	const Seq& list() const
+	const Seq& list() const {
-	{
 		if (!is_list())
 			throw Error(Error::Code::InvalidArgument, "not a list");
 		return seq_;
 	}
 
 	/**
-	 * Convert a Sexp::Node to its S-expression string representation
+	 * Convert a Sexp to its S-expression string representation
 	 *
 	 * @return the string representation
 	 */
@@ -142,6 +157,9 @@ struct Sexp {
 	/// Helper struct to build mutable lists.
 	///
 	struct List {
+		List () = default;
+		List (const Seq& seq): seq_{seq} {}
+
 		/**
 		 * Add a sexp to the list
 		 *
@@ -164,7 +182,8 @@ struct Sexp {
 		}
 
 		/**
-		 * Add a property (i.e., :key sexp ) to the list
+		 * Add a property (i.e., :key sexp ) to the list. Remove any
+		 * prop with the same name
 		 *
 		 * @param name a property-name. Must start with ':', length > 1
 		 * @param sexp a sexp
@@ -172,8 +191,8 @@ struct Sexp {
 		 *
 		 * @return a ref to this List (for chaining)
 		 */
-		List& add_prop(std::string&& name, Sexp&& sexp)
+		List& add_prop(std::string&& name, Sexp&& sexp) {
-		{
+			remove_prop(name);
 			if (!is_prop_name(name))
 				throw Error{Error::Code::InvalidArgument,
 					    "invalid property name ('%s')",
@@ -183,13 +202,27 @@ struct Sexp {
 			return *this;
 		}
 		template <typename... Args>
-		List& add_prop(std::string&& name, Sexp&& sexp, Args... args)
+		List& add_prop(std::string&& name, Sexp&& sexp, Args... args) {
-		{
+			remove_prop(name);
 			add_prop(std::move(name), std::move(sexp));
 			add_prop(std::forward<Args>(args)...);
 			return *this;
 		}
 
+		void remove_prop(const std::string& name) {
+			if (!is_prop_name(name))
+				throw Error{Error::Code::InvalidArgument,
+					"invalid property name ('%s')", name.c_str()};
+			auto it = std::find_if(seq_.begin(), seq_.end(), [&](auto&& elm) {
+				return elm.type() == Sexp::Type::Symbol &&
+					elm.value() == name;
+			});
+			if (it != seq_.cend() && it + 1 != seq_.cend()) {
+				/* erase propname and value.*/
+				seq_.erase(it, it + 2);
+			}
+		}
+
 		/**
 		 * Remove all elements from the list.
 		 */
@@ -209,7 +242,7 @@ struct Sexp {
 		 */
 		size_t empty() const { return seq_.empty(); }
 
-		private:
+	private:
 		friend struct Sexp;
 		Seq seq_;
 	};
@@ -309,16 +342,14 @@ struct Sexp {
 			return is_prop_list(list().begin() + 1, list().end());
 	}
 
-	private:
+private:
-	Sexp(Type typearg, std::string&& valuearg) : type_{typearg}, value_{std::move(valuearg)}
+	Sexp(Type typearg, std::string&& valuearg) : type_{typearg}, value_{std::move(valuearg)} {
-	{
 		if (is_list())
 			throw Error{Error::Code::InvalidArgument, "cannot be a list type"};
 		if (is_empty())
 			throw Error{Error::Code::InvalidArgument, "cannot be an empty type"};
 	}
-	Sexp(Type typearg, Seq&& seq) : type_{Type::List}, seq_{std::move(seq)}
+	Sexp(Type typearg, Seq&& seq) : type_{Type::List}, seq_{std::move(seq)} {
-	{
 		if (!is_list())
 			throw Error{Error::Code::InvalidArgument, "must be a list type"};
 		if (is_empty())
@@ -353,10 +384,10 @@ struct Sexp {
 		return b == e;
 	}
 
-	const Type        type_;  /**<  Type of node */
+	Type	    type_;  /**<  Type of node */
-	const std::string value_; /**< String value of node (only for
+	std::string value_; /**< String value of node (only for
-				   * non-Type::Lst)*/
+			       * non-Type::Lst)*/
-	const Seq seq_;           /**< Children of node (only for
+	Seq seq_;	    /**< Children of node (only for
 				   * Type::Lst) */
 };
 

lib/utils/tests/test-sexp.cc

@@ -55,7 +55,7 @@ test_parser()
 
 	check_parse(R"("foo
 bar")",
-	            "\"foo\nbar\"");
+		    "\"foo\nbar\"");
 }
 
 static void
@@ -111,20 +111,47 @@ static void
 test_props()
 {
 	auto sexp2 = Sexp::make_list(Sexp::make_string("foo"),
-	                             Sexp::make_number(123),
+				     Sexp::make_number(123),
-	                             Sexp::make_symbol("blub"));
+				     Sexp::make_symbol("blub"));
 
 	auto sexp = Sexp::make_prop_list(":foo",
-	                                 Sexp::make_string("bär"),
+					 Sexp::make_string("bär"),
-	                                 ":cuux",
+					 ":cuux",
-	                                 Sexp::make_number(123),
+					 Sexp::make_number(123),
-	                                 ":flub",
+					 ":flub",
-	                                 Sexp::make_symbol("fnord"),
+					 Sexp::make_symbol("fnord"),
-	                                 ":boo",
+					 ":boo",
-	                                 std::move(sexp2));
+					 std::move(sexp2));
 
 	assert_equal(sexp.to_sexp_string(),
-	             "(:foo \"b\303\244r\" :cuux 123 :flub fnord :boo (\"foo\" 123 blub))");
+		     "(:foo \"b\303\244r\" :cuux 123 :flub fnord :boo (\"foo\" 123 blub))");
+}
+
+static void
+test_prop_list_remove()
+{
+	{
+		Sexp::List lst;
+		lst.add_prop(":foo", Sexp::make_string("123"))
+			.add_prop(":bar", Sexp::make_number(123));
+
+		assert_equal(Sexp::make_list(std::move(lst)).to_sexp_string(),
+			     R"((:foo "123" :bar 123))");
+	}
+
+	{
+		Sexp::List lst;
+		lst.add_prop(":foo", Sexp::make_string("123"))
+			.add_prop(":bar", Sexp::make_number(123));
+
+		assert_equal(Sexp::make_list(Sexp::List{lst}).to_sexp_string(),
+			     R"((:foo "123" :bar 123))");
+
+		lst.remove_prop(":bar");
+
+		assert_equal(Sexp::make_list(Sexp::List{lst}).to_sexp_string(),
+			     R"((:foo "123"))");
+	}
 }
 
 int
@@ -140,6 +167,7 @@ try {
 	g_test_add_func("/utils/sexp/parser", test_parser);
 	g_test_add_func("/utils/sexp/list", test_list);
 	g_test_add_func("/utils/sexp/proplist", test_prop_list);
+	g_test_add_func("/utils/sexp/proplist-remove", test_prop_list_remove);
 	g_test_add_func("/utils/sexp/props", test_props);
 
 	return g_test_run();

mu/mu-cmd-find.cc

@@ -374,7 +374,12 @@ static bool
 output_sexp(const Option<Message>& msg, const OutputInfo& info, const MuConfig* opts, GError** err)
 {
 	if (msg) {
-		fputs(msg->to_sexp().to_sexp_string().c_str(), stdout);
+
+		if (const auto sexp{msg->cached_sexp()}; !sexp.empty())
+			fputs(sexp.c_str(), stdout);
+		else
+			fputs(msg->to_sexp().to_sexp_string().c_str(), stdout);
+
 		fputs("\n", stdout);
 	}
 
@@ -476,7 +481,7 @@ output_query_results(const QueryResults& qres, const MuConfig* opts)
 		if (!msg)
 			continue;
 
-		if (opts->after != 0 && msg->mtime() < opts->after)
+		if (opts->after != 0 && msg->changed() < opts->after)
 			continue;
 
 		rv = output_func(msg,