Make List and Results notifications more consistent
Deliver the initial results for both of them and include the changeset in the initial delivery for both, rather than having them behave weirdly differently.
This commit is contained in:
parent
7a74a22558
commit
20d9da973b
|
@ -208,15 +208,8 @@ bool BackgroundCollection::deliver(SharedGroup& sg, std::exception_ptr err)
|
|||
|
||||
void BackgroundCollection::call_callbacks()
|
||||
{
|
||||
while (auto cb = next_callback()) {
|
||||
auto fn = cb->fn;
|
||||
if (!cb->initial_delivered && should_deliver_initial()) {
|
||||
cb->initial_delivered = true;
|
||||
fn({}, m_error); // note: may invalidate `cb`
|
||||
}
|
||||
else {
|
||||
fn(m_changes_to_deliver, m_error);
|
||||
}
|
||||
while (auto fn = next_callback()) {
|
||||
fn(m_changes_to_deliver, m_error);
|
||||
}
|
||||
|
||||
if (m_error) {
|
||||
|
@ -227,17 +220,17 @@ void BackgroundCollection::call_callbacks()
|
|||
}
|
||||
}
|
||||
|
||||
BackgroundCollection::Callback* BackgroundCollection::next_callback()
|
||||
CollectionChangeCallback BackgroundCollection::next_callback()
|
||||
{
|
||||
std::lock_guard<std::mutex> callback_lock(m_callback_mutex);
|
||||
|
||||
for (++m_callback_index; m_callback_index < m_callbacks.size(); ++m_callback_index) {
|
||||
auto& callback = m_callbacks[m_callback_index];
|
||||
bool deliver_initial = !callback.initial_delivered && should_deliver_initial();
|
||||
if (!m_error && !deliver_initial && m_changes_to_deliver.empty()) {
|
||||
if (!m_error && callback.initial_delivered && m_changes_to_deliver.empty()) {
|
||||
continue;
|
||||
}
|
||||
return &callback;
|
||||
callback.initial_delivered = true;
|
||||
return callback.fn;
|
||||
}
|
||||
|
||||
m_callback_index = npos;
|
||||
|
|
|
@ -119,7 +119,6 @@ private:
|
|||
virtual void do_prepare_handover(SharedGroup&) = 0;
|
||||
virtual bool do_deliver(SharedGroup&) { return true; }
|
||||
virtual bool do_add_required_change_info(TransactionChangeInfo&) { return true; }
|
||||
virtual bool should_deliver_initial() const noexcept { return false; }
|
||||
|
||||
const std::thread::id m_thread_id = std::this_thread::get_id();
|
||||
bool is_for_current_thread() const { return m_thread_id == std::this_thread::get_id(); }
|
||||
|
@ -158,7 +157,7 @@ private:
|
|||
// remove_callback() updates this when needed
|
||||
size_t m_callback_index = npos;
|
||||
|
||||
Callback* next_callback();
|
||||
CollectionChangeCallback next_callback();
|
||||
};
|
||||
|
||||
} // namespace _impl
|
||||
|
|
|
@ -86,7 +86,7 @@ bool ListNotifier::do_add_required_change_info(TransactionChangeInfo& info)
|
|||
|
||||
void ListNotifier::run()
|
||||
{
|
||||
if (!m_lv) {
|
||||
if (!m_lv || !m_lv->is_attached()) {
|
||||
// LV was deleted, so report all of the rows being removed if this is
|
||||
// the first run after that
|
||||
if (m_prev_size) {
|
||||
|
|
|
@ -75,8 +75,6 @@ private:
|
|||
void release_data() noexcept override;
|
||||
void do_attach_to(SharedGroup& sg) override;
|
||||
void do_detach_from(SharedGroup& sg) override;
|
||||
|
||||
bool should_deliver_initial() const noexcept override { return true; }
|
||||
};
|
||||
|
||||
} // namespace _impl
|
||||
|
|
|
@ -66,15 +66,21 @@ TEST_CASE("list") {
|
|||
};
|
||||
|
||||
auto require_change = [&] {
|
||||
return lst.add_notification_callback([&](CollectionChangeIndices c, std::exception_ptr err) {
|
||||
auto token = lst.add_notification_callback([&](CollectionChangeIndices c, std::exception_ptr err) {
|
||||
change = c;
|
||||
});
|
||||
advance_and_notify(*r);
|
||||
return token;
|
||||
};
|
||||
|
||||
auto require_no_change = [&] {
|
||||
return lst.add_notification_callback([&](CollectionChangeIndices c, std::exception_ptr err) {
|
||||
REQUIRE(false);
|
||||
bool first = true;
|
||||
auto token = lst.add_notification_callback([&, first](CollectionChangeIndices c, std::exception_ptr err) mutable {
|
||||
REQUIRE(first);
|
||||
first = false;
|
||||
});
|
||||
advance_and_notify(*r);
|
||||
return token;
|
||||
};
|
||||
|
||||
SECTION("modifying the list sends a change notifications") {
|
||||
|
|
|
@ -276,9 +276,9 @@ TEST_CASE("Results") {
|
|||
REQUIRE(notification_calls == 1);
|
||||
}
|
||||
|
||||
SECTION("the first call of a notification always passes an empty change even if it previously ran for a different callback") {
|
||||
SECTION("the first call of a notification can include changes if it previously ran for a different callback") {
|
||||
auto token2 = results.add_notification_callback([&](CollectionChangeIndices c, std::exception_ptr) {
|
||||
REQUIRE(c.empty());
|
||||
REQUIRE(!c.empty());
|
||||
});
|
||||
|
||||
write([&] {
|
||||
|
|
Loading…
Reference in New Issue