Skip to content

Commit

Permalink
Merge pull request #39 from jagerman/remove-iterator-deletion
Browse files Browse the repository at this point in the history 
Remove broken erase(iterator) interface
  • Loading branch information
@jagerman
jagerman committed Jul 11, 2023
2 parents 880500c + 54f1c8a commit 7eb8702
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 300 deletions.
41 changes: 1 addition & 40 deletions include/session/config/contacts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,26 +223,7 @@ typedef struct contacts_iterator {
/// }
/// contacts_iterator_free(it);
///
/// It is permitted to modify records (e.g. with a call to `contacts_set`) and add records while
/// iterating.
///
/// If you need to remove while iterating then usage is slightly different: you must advance the
/// iteration by calling either contacts_iterator_advance if not deleting, or
/// contacts_iterator_erase to erase and advance. Usage looks like this:
///
/// contacts_contact c;
/// contacts_iterator *it = contacts_iterator_new(contacts);
/// while (!contacts_iterator_done(it, &c)) {
/// // c.session_id, c.nickname, etc. are loaded
///
/// bool should_delete = /* ... */;
///
/// if (should_delete)
/// contacts_iterator_erase(it);
/// else
/// contacts_iterator_advance(it);
/// }
/// contacts_iterator_free(it);
/// It is NOT permitted to add/remove/modify records while iterating.
///
/// Declaration:
/// ```cpp
Expand Down Expand Up @@ -315,26 +296,6 @@ LIBSESSION_EXPORT bool contacts_iterator_done(contacts_iterator* it, contacts_co
/// - `void` -- Nothing Returned
LIBSESSION_EXPORT void contacts_iterator_advance(contacts_iterator* it);

/// API: contacts/contacts_iterator_erase
///
/// Erases the current contact while advancing the iterator to the next contact in the iteration.
///
/// Declaration:
/// ```cpp
/// VOID contacts_iterator_erase(
/// [in] config_object* conf,
/// [in] contacts_iterator* it
/// );
/// ```
///
/// Inputs:
/// - `conf` -- [in] Pointer to the config object
/// - `it` -- [in] Pointer to the contacts_iterator
///
/// Outputs:
/// - `void` -- Nothing Returned
LIBSESSION_EXPORT void contacts_iterator_erase(config_object* conf, contacts_iterator* it);

#ifdef __cplusplus
} // extern "C"
#endif
44 changes: 4 additions & 40 deletions include/session/config/contacts.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,27 +442,6 @@ class Contacts : public ConfigBase {
/// - `bool` - Returns true if contact was found and removed, false otherwise
bool erase(std::string_view session_id);

struct iterator;

/// API: contacts/contacts::erase(iterator)
///
/// This works like erase, but takes an iterator to the contact to remove. The element is
/// removed and the iterator to the next element after the removed one is returned. This is
/// intended for use where elements are to be removed during iteration: see below for an
/// example.
///
/// Declaration:
/// ```cpp
/// iterator erase(iterator it);
/// ```
///
/// Inputs:
/// - `it` -- iterator resolving to the contact to remove
///
/// Outputs:
/// - `iterator` - Returns the next element after the removed one
iterator erase(iterator it);

/// API: contacts/contacts::size
///
/// Returns the number of contacts.
Expand Down Expand Up @@ -493,6 +472,7 @@ class Contacts : public ConfigBase {
/// - `bool` - Returns true if the contact list is empty
bool empty() const { return size() == 0; }

struct iterator;
/// API: contacts/contacts::begin
///
/// Iterators for iterating through all contacts. Typically you access this implicit via a for
Expand All @@ -506,25 +486,9 @@ class Contacts : public ConfigBase {
///
/// This iterates in sorted order through the session_ids.
///
/// It is permitted to modify and add records while iterating (e.g. by modifying `contact` and
/// then calling set()).
///
/// If you need to erase the current contact during iteration then care is required: you need to
/// advance the iterator via the iterator version of erase when erasing an element rather than
/// incrementing it regularly. For example:
///
///```cpp
/// for (auto it = contacts.begin(); it != contacts.end(); ) {
/// if (should_remove(*it))
/// it = contacts.erase(it);
/// else
/// ++it;
/// }
///```
///
/// Alternatively, you can use the first version with two loops: the first loop through all
/// contacts doesn't erase but just builds a vector of IDs to erase, then the second loops
/// through that vector calling `erase()` for each one.
/// It is NOT permitted to add/modify/remove records while iterating; instead such modifications
/// require two passes: an iterator loop to collect the required modifications, then a second
/// pass to apply the modifications.
///
/// Declaration:
/// ```cpp
Expand Down
45 changes: 2 additions & 43 deletions include/session/config/convo_info_volatile.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,28 +502,8 @@ typedef struct convo_info_volatile_iterator convo_info_volatile_iterator;
/// convo_info_volatile_iterator_free(it);
/// ```
///
/// It is permitted to modify records (e.g. with a call to one of the `convo_info_volatile_set_*`
/// functions) and add records while iterating.
///
/// If you need to remove while iterating then usage is slightly different: you must advance the
/// iteration by calling either convo_info_volatile_iterator_advance if not deleting, or
/// convo_info_volatile_iterator_erase to erase and advance. Usage looks like this:
/// ```cpp
/// convo_info_volatile_1to1 c1;
/// convo_info_volatile_iterator *it = convo_info_volatile_iterator_new(my_convos);
/// while (!convo_info_volatile_iterator_done(it)) {
/// if (convo_it_is_1to1(it, &c1)) {
/// bool should_delete = /* ... */;
/// if (should_delete)
/// convo_info_volatile_iterator_erase(it);
/// else
/// convo_info_volatile_iterator_advance(it);
/// } else {
/// convo_info_volatile_iterator_advance(it);
/// }
/// }
/// convo_info_volatile_iterator_free(it);
/// ```
/// It is NOT permitted to add/modify/remove records while iterating; instead you must use two
/// loops: a first one to identify changes, and a second to apply them.
///
/// Declaration:
/// ```cpp
Expand Down Expand Up @@ -729,27 +709,6 @@ LIBSESSION_EXPORT bool convo_info_volatile_it_is_community(
LIBSESSION_EXPORT bool convo_info_volatile_it_is_legacy_group(
convo_info_volatile_iterator* it, convo_info_volatile_legacy_group* c);

/// API: convo_info_volatile/convo_info_volatile_iterator_erase
///
/// Erases the current convo while advancing the iterator to the next convo in the iteration.
///
/// Declaration:
/// ```cpp
/// VOID convo_info_volatile_iterator_erase(
/// [in] config_object* conf,
/// [in] convo_info_volatile_iterator* it
/// );
/// ```
///
/// Inputs:
/// - `conf` -- [in] Pointer to the config object
/// - `it` -- [in] The convo_info_volatile_iterator
///
/// Outputs:
/// - `void` -- Nothing Returned
LIBSESSION_EXPORT void convo_info_volatile_iterator_erase(
config_object* conf, convo_info_volatile_iterator* it);

#ifdef __cplusplus
} // extern "C"
#endif
74 changes: 27 additions & 47 deletions include/session/config/convo_info_volatile.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,29 @@ class ConvoInfoVolatile : public ConfigBase {
const char* encryption_domain() const override { return "ConvoInfoVolatile"; }

/// Our pruning ages. We ignore added conversations that are more than PRUNE_LOW before now,
/// and we active remove (when doing a new push) any conversations that are more than PRUNE_HIGH
/// before now. Clients can mostly ignore these and just add all conversations; the class just
/// transparently ignores (or removes) pruned values.
/// and we actively remove (when doing a new push) any conversations that are more than
/// PRUNE_HIGH before now. Clients can mostly ignore these and just add all conversations; the
/// class just transparently ignores (or removes) pruned values.
static constexpr auto PRUNE_LOW = 30 * 24h;
static constexpr auto PRUNE_HIGH = 45 * 24h;

/// API: convo_info_volatile/ConvoInfoVolatile::prune_stale
///
/// Prunes any "stale" conversations: that is, ones with a last read more than `prune` ago that
/// are not specifically "marked as unread" by the client.
///
/// This method is called automatically by `push()` and does not typically need to be invoked
/// directly.
///
/// Inputs:
/// - `prune` the "too old" time; any conversations with a last_read time more than this
/// duration ago will be removed (unless they have the explicit `unread` flag set). If
/// omitted, defaults to the PRUNE_HIGH constant (45 days).
///
/// Outputs:
/// - returns nothing.
void prune_stale(std::chrono::milliseconds prune = PRUNE_HIGH);

/// API: convo_info_volatile/ConvoInfoVolatile::push
///
/// Overrides push() to prune stale last-read values before we do the push.
Expand Down Expand Up @@ -501,27 +518,6 @@ class ConvoInfoVolatile : public ConfigBase {

bool erase(const convo::any& c); // Variant of any of them

struct iterator;

/// API: convo_info_volatile/ConvoInfoVolatile::erase(iterator)
///
/// This works like erase, but takes an iterator to the conversation to remove. The element is
/// removed and the iterator to the next element after the removed one is returned. This is
/// intended for use where elements are to be removed during iteration: see below for an
/// example.
///
/// Declaration:
/// ```cpp
/// iterator erase(iterator it);
/// ```
///
/// Inputs:
/// - `it` -- iterator resolving to the conversation to remove
///
/// Outputs:
/// - `iterator` - Returns the next element after the removed conversation
iterator erase(iterator it);

/// API: convo_info_volatile/ConvoInfoVolatile::size
///
/// Returns the number of conversations, if `size()` is called it will return the total of any
Expand Down Expand Up @@ -562,18 +558,19 @@ class ConvoInfoVolatile : public ConfigBase {
/// - `bool` -- Returns true if the convesation list is empty
bool empty() const { return size() == 0; }

struct iterator;
/// API: convo_info_volatile/ConvoInfoVolatile::begin
///
/// Iterators for iterating through all conversations. Typically you access this implicit via a
/// for loop over the `ConvoInfoVolatile` object:
///
/// ```cpp
/// for (auto& convo : conversations) {
/// if (auto* dm = std::get_if<convo::one_to_one>(&convo)) {
/// if (const auto* dm = std::get_if<convo::one_to_one>(&convo)) {
/// // use dm->session_id, dm->last_read, etc.
/// } else if (auto* og = std::get_if<convo::community>(&convo)) {
/// } else if (const auto* og = std::get_if<convo::community>(&convo)) {
/// // use og->base_url, og->room, om->last_read, etc.
/// } else if (auto* lcg = std::get_if<convo::legacy_group>(&convo)) {
/// } else if (const auto* lcg = std::get_if<convo::legacy_group>(&convo)) {
/// // use lcg->id, lcg->last_read
/// }
/// }
Expand All @@ -582,26 +579,9 @@ class ConvoInfoVolatile : public ConfigBase {
/// This iterates through all conversations in sorted order (sorted first by convo type, then by
/// id within the type).
///
/// It is permitted to modify and add records while iterating (e.g. by modifying one of the
/// `dm`/`og`/`lcg` and then calling set()).
///
/// If you need to erase the current conversation during iteration then care is required: you
/// need to advance the iterator via the iterator version of erase when erasing an element
/// rather than incrementing it regularly. For example:
///
/// ```cpp
/// for (auto it = conversations.begin(); it != conversations.end(); ) {
/// if (should_remove(*it))
/// it = converations.erase(it);
/// else
/// ++it;
/// }
/// ```
///
/// Alternatively, you can use the first version with two loops: the first loop through all
/// converations doesn't erase but just builds a vector of IDs to erase, then the second loops
/// through that vector calling `erase_1to1()`/`erase_community()`/`erase_legacy_group()` for
/// each one.
/// It is NOT permitted to add/modify/remove records while iterating; performing modifications
/// based on a condition requires two passes: one to collect the required changes, and another
/// to apply them key by key.
///
/// Declaration:
/// ```cpp
Expand Down
43 changes: 1 addition & 42 deletions include/session/config/user_groups.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -613,28 +613,7 @@ typedef struct user_groups_iterator user_groups_iterator;
/// user_groups_iterator_free(it);
/// ```
///
/// It is permitted to modify records (e.g. with a call to one of the `user_groups_set_*`
/// functions) and add records while iterating.
///
/// If you need to remove while iterating then usage is slightly different: you must advance the
/// iteration by calling either user_groups_iterator_advance if not deleting, or
/// user_groups_iterator_erase to erase and advance. Usage looks like this:
/// ```cpp
/// ugroups_community_info comm;
/// ugroups_iterator *it = ugroups_iterator_new(my_groups);
/// while (!user_groups_iterator_done(it)) {
/// if (user_groups_it_is_community(it, &comm)) {
/// bool should_delete = /* ... */;
/// if (should_delete)
/// user_groups_iterator_erase(it);
/// else
/// user_groups_iterator_advance(it);
/// } else {
/// user_groups_iterator_advance(it);
/// }
/// }
/// user_groups_iterator_free(it);
/// ```
/// It is NOT permitted to add/remove/modify records while iterating.
///
/// Declaration:
/// ```cpp
Expand Down Expand Up @@ -794,26 +773,6 @@ LIBSESSION_EXPORT bool user_groups_it_is_community(
LIBSESSION_EXPORT bool user_groups_it_is_legacy_group(
user_groups_iterator* it, ugroups_legacy_group_info* c);

/// API: user_groups/user_groups_iterator_erase
///
/// Erases the current group while advancing the iterator to the next group in the iteration.
///
/// Declaration:
/// ```cpp
/// VOID user_groups_iterator_erase(
/// [in] config_object* conf,
/// [in] user_groups_iterator* it
/// );
/// ```
///
/// Inputs:
/// - `conf` -- [in] Pointer to the config object
/// - `it` -- [in] The user_groups_iterator
///
/// Outputs:
/// - `void` -- Nothing Returned
LIBSESSION_EXPORT void user_groups_iterator_erase(config_object* conf, user_groups_iterator* it);

#ifdef __cplusplus
} // extern "C"
#endif
Loading

0 comments on commit 7eb8702

Please sign in to comment.