diff --git a/docs/implementation.md b/docs/implementation.md index 3b26048f..ab761d60 100644 --- a/docs/implementation.md +++ b/docs/implementation.md @@ -152,6 +152,13 @@ Startup loads cached records, validates collection roots, scans stale files, que and rebuilds summaries, duplicate predictions, presence, and search candidates. Cached results may publish before extraction completes. +The existing `item_properties.properties` blob stores versioned binary property records. Private +cache IDs are separate from the public media-property registry, so opaque values such as the face +cache never become EXIF/XMP fields or UI metadata. Metadata-only writes preserve unknown records +from the same item's previous blob; binary-property edits preserve all other records byte for byte. +No extra table, column, or schema version is needed. The format and private IDs live in +[model_db_pack.h](../src/model_db_pack.h) and [model_property.h](../src/model_property.h). + Search parses into exact matchers and conservative candidate prefilters. Negation and OR never create a prefilter that could omit an exact result. Query generations prevent stale result publication. Folder-scoped searches validate their filesystem scope; index-wide searches answer from cached diff --git a/src/model_db.cpp b/src/model_db.cpp index 7b6a321d..484c79c1 100644 --- a/src/model_db.cpp +++ b/src/model_db.cpp @@ -208,7 +208,7 @@ class db_statement } } - bool read() const + bool read(const bool checked = false) const { if (_handle != nullptr) { @@ -222,12 +222,24 @@ class db_statement return false; default: db_trace_error(_db, "sqlite3_step"); + if (checked) + throw app_exception(std::format("Failed to read cached properties: {}", + str::utf8_cast(sqlite3_errmsg(_db)))); } } + else if (checked) + { + throw app_exception("Cached-property query is not prepared"s); + } return false; } + bool read_checked() const + { + return read(true); + } + void exec() const { while (read()) @@ -288,6 +300,18 @@ class db_statement return {data_ptr, static_cast(sqlite3_column_bytes(_handle, i))}; } + df::cspan data_checked(const int i) const + { + if (!_handle) throw app_exception("Cached-property query is not prepared"s); + const auto value = data(i); + if (!value.data && sqlite3_errcode(_db) == SQLITE_NOMEM) + { + db_trace_error(_db, "sqlite3_column_blob"); + throw app_exception("Failed to read cached-property bytes"s); + } + return value; + } + // Distinguishes a column an upgrade added but never filled from one holding zero, which the // typed readers cannot: both answer 0. bool is_null(const int i) const @@ -1703,6 +1727,8 @@ void database::perform_writes(std::deque writes) const const db_statement update_metadata_scanned( _db, "update item_properties set last_scanned = ?, last_indexed = ? where folder = ? and name = ?"s); + const db_statement previous_properties( + _db, "select properties from item_properties where folder=? and name=?"s); const db_statement update_hash(_db, "update item_properties set hash = ? where folder=? and name=?"s); const db_statement update_crc(_db, "update item_properties set crc = ? where folder=? and name=?"s); const db_statement update_phash( @@ -1734,8 +1760,8 @@ void database::perform_writes(std::deque writes) const { const auto md = write.md.value(); - // A null metadata pointer means "this item has no properties". The packed payload - // must then be an empty record, not whatever the previous item left in the packer. + // Reset public metadata even for a null pointer; opaque cache values belong to this item, + // not to the metadata snapshot or to the item the packer previously wrote. packer.reset_to_header(); if (md) @@ -1743,6 +1769,12 @@ void database::perform_writes(std::deque writes) const packer.pack(md); } + previous_properties.bind(1, path.folder().text()); + previous_properties.bind(2, path.name()); + if (previous_properties.read_checked()) + metadata_preserve_unknown_properties(packer, previous_properties.data_checked(0)); + previous_properties.reset(); + insert_properties.bind(1, path.folder().text()); insert_properties.bind(2, path.name()); insert_properties.bind(3, packer.cdata()); diff --git a/src/model_db_pack.h b/src/model_db_pack.h index ebea7345..eee880ed 100644 --- a/src/model_db_pack.h +++ b/src/model_db_pack.h @@ -6,12 +6,17 @@ // License details are available at https://www.gnu.org/licenses/lgpl-2.1.html // This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY -// Purpose: Metadata serialization for database storage. Packs and unpacks item -// metadata into compact binary format for efficient SQLite storage. +// Purpose: Database property serialization: public metadata and opaque, database-only cache values. #pragma once +class metadata_property_error final : public std::runtime_error +{ +public: + using std::runtime_error::runtime_error; +}; + class metadata_packer { public: @@ -70,6 +75,29 @@ class metadata_packer } } + void write_blob(const uint16_t id, df::cspan value) + { + if (value.size > std::numeric_limits::max() || (value.size && !value.data)) + throw metadata_property_error("Invalid binary cache-property value"); + + df::blob owned; + if (value.size && !_data.empty()) + { + const std::less before; + if (!before(value.data, _data.data()) && before(value.data, _data.data() + _data.size())) + { + const auto offset = static_cast(value.data - _data.data()); + if (value.size > _data.size() - offset) + throw metadata_property_error("Binary cache-property value exceeds its buffer"); + owned.assign(value.data, value.data + value.size); + value = owned; + } + } + write_prop_id(id); + write_len(value.size); + if (value.size) _data.insert(_data.end(), value.data, value.data + value.size); + } + template void write(const uint16_t id, const T& v) { @@ -168,7 +196,7 @@ class metadata_unpacker public: metadata_unpacker(const df::cspan data) : _data(data) { - if (_data.size >= 2 && _data.data[0] == 0xFF) + if (_data.data && _data.size >= 2 && _data.data[0] == 0xFF) { _version = _data.data[1]; } @@ -187,17 +215,22 @@ class metadata_unpacker return (_version == 0 || _version > metadata_packer::version) || remaining() < 2; } - // The property for the next record, or prop::null when this build does not know the id. - prop::key_ref read_type() + uint16_t read_id() { if (at_end()) { - return prop::null; + return 0; } auto id = static_cast(_data.data[_pos++]); id |= static_cast(_data.data[_pos++] << 8); - return prop::from_id(id); + return id; + } + + // The property for the next record, or prop::null when this build does not know the id. + prop::key_ref read_type() + { + return at_end() ? prop::null : prop::from_id(read_id()); } size_t read_len() @@ -223,6 +256,34 @@ class metadata_unpacker return result; } + struct property_record + { + uint16_t id; + df::cspan value; + df::cspan encoded; + }; + + // Borrowed views of complete records, for lossless cache-property edits without unpacking metadata. + std::optional read_record() + { + if (_data.size == 0) return std::nullopt; + if (_version == 0 || _version > metadata_packer::version) + throw metadata_property_error("Unsupported cached-property header"); + if (remaining() == 0) return std::nullopt; + if (remaining() < 3) throw metadata_property_error("Truncated cached-property record"); + + const auto start = _pos; + const auto id = read_id(); + const auto marker = _data.data[_pos]; + const auto length_bytes = marker == 0xff ? 3u : marker == 0xfe ? 5u : 1u; + if (remaining() < length_bytes) throw metadata_property_error("Truncated cached-property length"); + const auto length = read_len(); + if (length > remaining()) throw metadata_property_error("Truncated cached-property value"); + const df::cspan value{_data.data + _pos, length}; + _pos += length; + return property_record{id, value, {_data.data + start, _pos - start}}; + } + template void read_val(T& v) { @@ -380,3 +441,49 @@ class metadata_unpacker void unpack(const prop::item_metadata_ptr& md); }; + +inline std::optional metadata_find_blob_property(const df::cspan data, const uint16_t id) +{ + metadata_unpacker reader(data); + std::optional result; + while (const auto record = reader.read_record()) + { + if (record->id == id) result = record->value; + } + return result; +} + +inline df::blob metadata_replace_blob_property(const df::cspan data, const uint16_t id, + const std::optional value) +{ + metadata_packer result; + metadata_unpacker reader(data); + while (const auto record = reader.read_record()) + { + if (record->id != id) + result._data.insert(result._data.end(), record->encoded.data, + record->encoded.data + record->encoded.size); + } + if (value) result.write_blob(id, *value); + return std::move(result._data); +} + +inline void metadata_preserve_unknown_properties(metadata_packer& destination, const df::cspan previous) +{ + const auto original_size = destination.size(); + try + { + metadata_unpacker reader(previous); + while (const auto record = reader.read_record()) + { + if (prop::from_id(record->id) == prop::null) + destination._data.insert(destination._data.end(), record->encoded.data, + record->encoded.data + record->encoded.size); + } + } + catch (const metadata_property_error& error) + { + destination._data.resize(original_size); + df::log(__FUNCTION__, std::format("Invalid cached properties replaced by fresh metadata: {}", error.what())); + } +} diff --git a/src/model_property.h b/src/model_property.h index d32ba72b..6111052c 100644 --- a/src/model_property.h +++ b/src/model_property.h @@ -95,6 +95,12 @@ namespace prop struct item_metadata; class key; + // Database-only properties, deliberately absent from the public media-property registry. + namespace cache + { + constexpr uint16_t faces = 0xfffe; + } + // The projection a file's own GPano metadata declares. Persisted in the index, so a value's // meaning is fixed once assigned: retire one by leaving its number unused, never by reusing it. // `none` is the answer for a file carrying no panorama projection metadata at all, so anything diff --git a/src/test_index.cpp b/src/test_index.cpp index 3bd139e5..78fa2356 100644 --- a/src/test_index.cpp +++ b/src/test_index.cpp @@ -345,6 +345,166 @@ static void should_store_item_properties() // A rescan carries the position the index holds into the database row. A position saved only to the // row was written back over by the next rescan of the file, so the next session resumed from wherever // the index had been loaded. +static void should_round_trip_opaque_cache_properties() +{ + assert_equal(true, prop::from_id(prop::cache::faces) == prop::null, + "the face cache is not a public media property"); + const auto md = std::make_shared(); + md->album = "public metadata"_c; + for (const auto size : {0_z, 253_z, 254_z, 65534_z, 65535_z, 65536_z}) + { + df::blob payload(size); + for (auto i = 0_z; i < size; ++i) payload[i] = static_cast(i * 17); + metadata_packer metadata; + metadata.pack(md); + metadata_packer packer; + packer.write_blob(prop::cache::faces, payload); + packer._data.insert(packer._data.end(), metadata._data.begin() + 2, metadata._data.end()); + const auto found = metadata_find_blob_property(packer.cdata(), prop::cache::faces); + assert_equal(true, found.has_value(), "an empty value differs from an absent property"); + assert_equal(size, found->size, "binary lengths round trip at both framing boundaries"); + assert_equal(true, size == 0 || memcmp(found->data, payload.data(), size) == 0, "binary bytes round trip"); + const auto loaded = std::make_shared(); + metadata_unpacker reader(packer.cdata()); + reader.unpack(loaded); + assert_equal(md->album, loaded->album, "unknown binary values do not hide following metadata"); + + const auto stripped = metadata_replace_blob_property(packer.cdata(), prop::cache::faces, std::nullopt); + assert_equal(false, metadata_find_blob_property(stripped, prop::cache::faces).has_value(), + "removal leaves no cache value"); + metadata_packer expected; + expected.pack(md); + assert_equal(true, stripped == expected._data, "removal preserves every other property byte"); + } + metadata_packer alias; + const auto original = alias._data.clone(); + alias.write_blob(prop::cache::faces, alias.cdata()); + const auto found = metadata_find_blob_property(alias.cdata(), prop::cache::faces); + assert_equal(true, found && found->size == original.size() && + memcmp(found->data, original.data(), original.size()) == 0, + "a borrowed value may refer to the packer's own buffer"); + + for (const auto& invalid : {df::blob{0xff}, df::blob{0xff, 1, 1}, + df::blob{0xff, 1, 1, 0, 0xfe, 1}, + df::blob{0xff, 1, 1, 0, 4, 0}}) + { + auto rejected = false; + try + { + metadata_replace_blob_property(invalid, prop::cache::faces, std::nullopt); + } + catch (const metadata_property_error&) + { + rejected = true; + } + assert_equal(true, rejected, "malformed property storage is not rewritten as success"); + } +} + +static df::blob read_test_cached_properties(sqlite3* db, const df::file_path path) +{ + sqlite3_stmt* handle = nullptr; + const auto prepared = sqlite3_prepare_v2(db, + "select properties from item_properties where folder=? and name=?", -1, &handle, nullptr); + const std::unique_ptr statement(handle, sqlite3_finalize); + const auto folder = path.folder().text(); + const auto name = std::string(path.name()); + if (prepared != SQLITE_OK || + sqlite3_bind_text(handle, 1, folder.sz(), -1, SQLITE_TRANSIENT) != SQLITE_OK || + sqlite3_bind_text(handle, 2, name.c_str(), -1, SQLITE_TRANSIENT) != SQLITE_OK || + sqlite3_step(handle) != SQLITE_ROW) + throw test_assert_exception(std::format("Cannot read test cache: {}", sqlite3_errmsg(db))); + const auto size = sqlite3_column_bytes(handle, 0); + const auto* bytes = static_cast(sqlite3_column_blob(handle, 0)); + if (size && !bytes) throw test_assert_exception("Cannot read cached-property bytes"); + return size ? df::blob(bytes, bytes + size) : df::blob{}; +} + +// A metadata-only rescan used to erase opaque cache records. Exercise the actual database writer. +static void should_preserve_opaque_properties_when_metadata_is_rewritten() +{ + const auto index_path = _temps.next_path(); + const auto db_path = df::file_path(index_path.folder(), index_path.file_name_without_extension(), ".db"); + const auto path = test_files_folder.combine_file("Test.jpg"); + null_async_strategy async; + const location_cache locations; + index_state index(async, locations); + database db(index); + db.open(index_path.folder(), index_path.file_name_without_extension()); + auto md = std::make_shared(); + md->album = "before"_c; + const auto save = [&db](item_db_write write) + { + std::deque writes; + writes.emplace_back(std::move(write)); + db.perform_writes(std::move(writes)); + }; + item_db_write initial; + initial.path = path; + initial.md = md; + initial.crc32c = 0x1234u; + initial.media_position = 17.0; + initial.phash = crypto::phash_rotations{11, 22, 33, 44}; + save(std::move(initial)); + + sqlite3* handle = nullptr; + const auto opened = sqlite3_open(db_path.str().c_str(), &handle); + const std::unique_ptr raw(handle, sqlite3_close); + if (opened != SQLITE_OK) throw test_assert_exception("Cannot open test property cache"); + const df::blob payload(65536, 0x5a); + const df::blob other{1, 2, 3}; + auto stored = metadata_replace_blob_property(read_test_cached_properties(handle, path), + prop::cache::faces, df::cspan(payload)); + stored = metadata_replace_blob_property(stored, 0xfffd, df::cspan(other)); + { + sqlite3_stmt* update_handle = nullptr; + const auto prepared = sqlite3_prepare_v2(handle, + "update item_properties set properties=? where folder=? and name=?", -1, + &update_handle, nullptr); + const std::unique_ptr update(update_handle, sqlite3_finalize); + const auto folder = path.folder().text(); + const auto name = std::string(path.name()); + if (prepared != SQLITE_OK || + sqlite3_bind_blob(update_handle, 1, stored.data(), static_cast(stored.size()), SQLITE_TRANSIENT) != SQLITE_OK || + sqlite3_bind_text(update_handle, 2, folder.sz(), -1, SQLITE_TRANSIENT) != SQLITE_OK || + sqlite3_bind_text(update_handle, 3, name.c_str(), -1, SQLITE_TRANSIENT) != SQLITE_OK || + sqlite3_step(update_handle) != SQLITE_DONE) + throw test_assert_exception("Cannot seed test cache properties"); + } + auto changed = std::make_shared(*md); + changed->album = "after"_c; + item_db_write write; + write.path = path; + write.md = changed; + save(std::move(write)); + const auto reloaded = read_test_cached_properties(handle, path); + const auto found = metadata_find_blob_property(reloaded, prop::cache::faces); + assert_equal(true, found && found->size == payload.size() && + memcmp(found->data, payload.data(), payload.size()) == 0, + "ordinary metadata writes preserve the opaque face-cache value"); + const auto retained = metadata_find_blob_property(reloaded, 0xfffd); + assert_equal(true, retained && retained->size == other.size() && + memcmp(retained->data, other.data(), other.size()) == 0, + "other unknown properties survive too"); + db.load_index_values(); + const auto item = index.find_item(path); + const auto loaded_md = item.metadata.load(); + assert_equal(changed->album, loaded_md->album, "known metadata still changes"); + assert_equal(0x1234u, item.crc32c.load(), "CRC is retained"); + assert_equal(17, static_cast(loaded_md->media_position), "playback state is retained"); + const auto hashes = item.phash.load(); + assert_equal(true, hashes && hashes->rotations[0] == 11, "perceptual hashes are retained"); + + const auto empty_path = test_files_folder.combine_file("empty-cache.jpg"); + item_db_write empty; + empty.path = empty_path; + empty.md = prop::item_metadata_ptr{}; + save(std::move(empty)); + const auto empty_properties = read_test_cached_properties(handle, empty_path); + assert_equal(false, metadata_find_blob_property(empty_properties, prop::cache::faces).has_value(), + "a cache value never leaks from the previous item"); +} + static void should_hold_a_saved_playback_position_in_the_index() { const auto root = _temps.next_folder("saved-playback-position"); @@ -5699,6 +5859,9 @@ void register_index_tests(view_state& state, test_registry& tests) tests.add("Should reuse cached thumbnail surface"s, should_reuse_cached_thumbnail_surface); tests.add("Should index"s, should_index); tests.add("Should create database schema"s, should_create_database_schema); + tests.add("Should round trip opaque cache properties"s, should_round_trip_opaque_cache_properties); + tests.add("Should preserve opaque properties when metadata is rewritten"s, + should_preserve_opaque_properties_when_metadata_is_rewritten); // SRC-036 - rollback fallback must survive schema creation. tests.add("Should keep database rollback journal schema initialization"s, should_keep_rollback_journal_schema_initialization);