Conversation
A report handed to save_attachment's or attachment_data's progress handler carried only the message id, so a caller had to look the message up to find out whether the transfer was for anything it was showing. The conversation is already known at both sites, so the report carries it. callbacks::attachment_progress keeps its leading ConversationId: it is now the same value as the field, but other consumers take it.
There was a getter for the cache limit and none for what is in it, so a client could show the ceiling but not how close to it the cache was. Summed from the cache index, in bytes on disk, which is the measure the limit is in and the one eviction compares against.
It is on the report now, so the parameter said the same thing twice. A handler that files these by conversation needs the value wherever a report comes from, and the one handed to attachment_data or save_attachment has no second argument to carry it - so the struct is the only place that can hold it for both. display_picture_progress keeps its parameter: it has no struct to put one on, which is the same reason it has no message or index.
Nothing about a transfer is stored, so a client that was not listening -- it had not started, the conversation was not open, its process was restarted -- cannot learn from the reports what it missed. Asking for the bytes to find out would be a download rather than a question. Takes a page of message ids rather than one attachment because that is how a transcript reads: the answer arrives with the messages it describes, and a row never draws a state it is about to correct. It also suits a caller that must not block the loop, which is one call with one handler rather than thirty. Cached is read off the file rather than its index row: what decides whether the next fetch answers from disk is the file being there, and the two can disagree until a sweep reconciles them. An attachment with nothing to fetch is left out rather than reported idle -- there is a difference between nobody fetching it and it not being fetchable, and only the first deserves a row offering to fetch it.
Collaborator
Author
|
Split into three independent pull requests, each mergeable on its own and in any order:
Each was built and run against the client test suite on its own, off The branch this PR was on is left in place for now because session-app pins its submodule at its head while all three are in review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two small additions to the client attachment API, both wanted by session-app.
AttachmentProgress::conversation_idA progress report broadcast through
callbacks::attachment_progresscomes with the conversation beside it, but one handed to theon_progresshandler a caller passed toattachment_dataorsave_attachmentcarried onlymessage_id— so an application that wants to route a report to the conversation view it belongs to had to look the message up again for every tick of every transfer.The Client knows the conversation at both sites, so the struct carries it.
_attachment_pointernow joinsmessagesfor it, which is one join on a query that was already being made.callbacks::attachment_progressloses its leadingConversationIdparameter, which the field makes a duplicate. An earlier revision of this branch kept it on the grounds that removing it would break session-cli — that was wrong: session-cli never registers this callback, it only passeson_progresstoattachment_data. The only consumer is session-app's shim, which is pinned to an older commit and updates when it bumps the submodule.display_picture_progresskeeps its parameter. It has no struct to put one on, which is the same reason it carries no message or index.attachment_cache_size()There is a getter for the cache limit and none for the current usage, so a client can show the ceiling but not how full it is — "2.1 GB of 5 GB" needs both halves. Added in the same two forms as
attachment_cache_limit, returning bytes.It sums the
attachment_cacheindex, so it is bytes on disk: the same measure the limit is in and the same total eviction compares against, which is what makes the two worth showing side by side. Display pictures are excluded, exactly as they are from the limit.Tests
tests/test_client/attachments.cpp: the existing save and auto-download cases now assert the conversation on the reports they already collect, and there is a new case for the cache size — 0 before anything is cached, then the sum of the cached files' sizes on disk after each arrival.Built and run locally against the client test suite (
testAll "[client]", 135 cases, 1016 assertions). Note thattests/static-bundle-testfails to link on this branch already, before these changes (undefined reference to SQLite::Database::exec); it is unrelated and untouched here.