misc: fix some sporadic failures in conversationMembersEvent

+ addConversationMember was called without any conv ready sometimes
+ call was mostly disabled
+ ban/unban was unclear sometimes

Change-Id: I4919ec70af128a5bf623405ba6840a8fafc45e8c
This commit is contained in:
Sébastien Blin
2024-02-12 09:25:32 -05:00
parent 8eae314930
commit 66d8100264
3 changed files with 57 additions and 29 deletions

View File

@ -2000,6 +2000,11 @@ JamiAccount::doRegister_()
if (!cert || !cert->issuer)
return;
auto peerId = cert->issuer->getId().toString();
// A connection request can be sent just before member is banned and this must be ignored.
if (accountManager()->getCertificateStatus(peerId) == dhtnet::tls::TrustStore::PermissionStatus::BANNED) {
channel->shutdown();
return;
}
if (name == "sip") {
cacheSIPConnection(std::move(channel), peerId, deviceId);
} else if (name.find("git://") == 0) {

View File

@ -76,15 +76,15 @@ private:
void testDhtPublicInCall();
CPPUNIT_TEST_SUITE(CallTest);
//CPPUNIT_TEST(testCall);
//CPPUNIT_TEST(testCachedCall);
//CPPUNIT_TEST(testStopSearching);
//CPPUNIT_TEST(testDeclineMultiDevice);
//CPPUNIT_TEST(testTlsInfosPeerCertificate);
//CPPUNIT_TEST(testSocketInfos);
//CPPUNIT_TEST(testInvalidTurn);
CPPUNIT_TEST(testCall);
CPPUNIT_TEST(testCachedCall);
CPPUNIT_TEST(testStopSearching);
CPPUNIT_TEST(testDeclineMultiDevice);
CPPUNIT_TEST(testTlsInfosPeerCertificate);
CPPUNIT_TEST(testSocketInfos);
CPPUNIT_TEST(testInvalidTurn);
CPPUNIT_TEST(testTransfer);
//CPPUNIT_TEST(testDhtPublicInCall);
CPPUNIT_TEST(testDhtPublicInCall);
CPPUNIT_TEST_SUITE_END();
};

View File

@ -546,8 +546,11 @@ ConversationMembersEventTest::testAddOfflineMemberThenConnects()
auto carlaUri = carlaAccount->getUsername();
aliceAccount->trackBuddyPresence(carlaUri, true);
auto convId = libjami::startConversation(aliceId);
CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&] { return !aliceData.conversationId.empty(); }));
auto aliceMsgSize = aliceData.messages.size();
libjami::addConversationMember(aliceId, convId, carlaUri);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
Manager::instance().sendRegister(carlaId, true);
CPPUNIT_ASSERT(cv.wait_for(lk, 60s, [&] { return carlaData.requestReceived; }));
@ -691,23 +694,30 @@ ConversationMembersEventTest::testRemoveInvitedMember()
// Add carla
Manager::instance().sendRegister(carlaId, true);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return carlaData.deviceAnnounced; }));
libjami::addConversationMember(aliceId, convId, carlaUri);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return carlaData.requestReceived; }));
auto aliceMsgSize = aliceData.messages.size();
libjami::addConversationMember(aliceId, convId, carlaUri);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return carlaData.requestReceived && aliceMsgSize + 1 == aliceData.messages.size(); }));
libjami::acceptConversationRequest(carlaId, convId);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty() && aliceMsgSize + 2 == aliceData.messages.size(); }));
// Invite Alice
auto carlaMsgSize = carlaData.messages.size();
libjami::addConversationMember(aliceId, convId, bobUri);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; }));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 3 == aliceData.messages.size()
&& carlaMsgSize + 1 == carlaData.messages.size()
&& bobData.requestReceived; }));
auto members = libjami::getConversationMembers(aliceId, convId);
CPPUNIT_ASSERT(members.size() == 3);
members = libjami::getConversationMembers(carlaId, convId);
CPPUNIT_ASSERT(members.size() == 3);
// Now check that alice, has the only admin, can remove bob
aliceMsgSize = aliceData.messages.size();
carlaMsgSize = carlaData.messages.size();
libjami::removeConversationMember(aliceId, convId, bobUri);
CPPUNIT_ASSERT(
cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 2 == aliceData.messages.size(); }));
cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 2 == aliceData.messages.size() && carlaMsgSize + 2 == carlaData.messages.size(); }));
members = libjami::getConversationMembers(aliceId, convId);
auto bobBanned = false;
for (auto& member : members) {
@ -715,9 +725,16 @@ ConversationMembersEventTest::testRemoveInvitedMember()
bobBanned = member["role"] == "banned";
}
CPPUNIT_ASSERT(bobBanned);
members = libjami::getConversationMembers(carlaId, convId);
bobBanned = false;
for (auto& member : members) {
if (member["uri"] == bobUri)
bobBanned = member["role"] == "banned";
}
CPPUNIT_ASSERT(bobBanned);
// Check that Carla is still able to sync
auto carlaMsgSize = carlaData.messages.size();
carlaMsgSize = carlaData.messages.size();
libjami::sendMessage(aliceId, convId, "hi"s, "");
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return carlaMsgSize + 1 == carlaData.messages.size(); }));
}
@ -875,15 +892,17 @@ ConversationMembersEventTest::testMemberCannotBanOther()
cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; }));
auto aliceMsgSize = aliceData.messages.size();
libjami::acceptConversationRequest(bobId, convId);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !bobData.conversationId.empty() && aliceMsgSize + 1 == aliceData.messages.size(); }));
auto bobMsgSize = bobData.messages.size();
libjami::addConversationMember(aliceId, convId, carlaUri);
CPPUNIT_ASSERT(
cv.wait_for(lk, 30s, [&]() { return carlaData.requestReceived; }));
cv.wait_for(lk, 30s, [&]() {
return aliceMsgSize + 2 == aliceData.messages.size() && bobMsgSize + 1 == bobData.messages.size() && carlaData.requestReceived; }));
aliceMsgSize = aliceData.messages.size();
auto bobMsgSize = bobData.messages.size();
bobMsgSize = bobData.messages.size();
libjami::acceptConversationRequest(carlaId, convId);
CPPUNIT_ASSERT(
cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size() && bobMsgSize + 1 == bobData.messages.size(); }));
cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty() && aliceMsgSize + 1 == aliceData.messages.size() && bobMsgSize + 1 == bobData.messages.size(); }));
// Now Carla remove Bob as a member
// remove from member & add into banned without voting for the ban
@ -923,14 +942,19 @@ ConversationMembersEventTest::testMemberCannotUnBanOther()
auto bobMsgSize = bobData.messages.size();
libjami::acceptConversationRequest(carlaId, convId);
CPPUNIT_ASSERT(
cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size() && bobMsgSize + 1 == bobData.messages.size(); }));
cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty()
&& aliceMsgSize + 1 == aliceData.messages.size()
&& bobMsgSize + 1 == bobData.messages.size(); }));
// Now check that alice, has the only admin, can remove bob
auto carlaMsgSize = carlaData.messages.size();
aliceMsgSize = aliceData.messages.size();
libjami::removeConversationMember(aliceId, convId, bobUri);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() {
return carlaMsgSize + 2 == carlaData.messages.size();;
}));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 2 == aliceData.messages.size(); }));
auto msgId = aliceData.messages.rbegin()->id;
auto getMessage = [](const auto& data, const auto& mid) -> bool {
return std::find_if(data.messages.begin(), data.messages.end(), [&](auto& msg) { return msg.id == mid; }) != data.messages.end();
};
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return getMessage(carlaData, msgId); }));
aliceMsgSize = aliceData.messages.size();
libjami::addConversationMember(carlaId, convId, bobUri);
@ -1531,12 +1555,10 @@ ConversationMembersEventTest::testBanUnbanGotFirstConv()
// Bob bans alice, should update bob2
bobAccount->removeContact(aliceUri, true);
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2Data.contactRemoved; }));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.contactRemoved && bob2Data.contactRemoved; }));
// Alice sends messages, bob & bob2 should not get it!
aliceMsgSize = aliceData.messages.size();
auto bobMsgSize = bobData.messages.size();
auto bob2MsgSize = bob2Data.messages.size();
libjami::sendMessage(aliceId, aliceData.conversationId, "hi"s, "");
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize != aliceData.messages.size(); }));
auto msgId = aliceData.messages.rbegin()->id;
@ -1551,10 +1573,11 @@ ConversationMembersEventTest::testBanUnbanGotFirstConv()
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.conversationId == aliceData.conversationId && bob2Data.conversationId == aliceData.conversationId; }));
// Alice can sends some messages now
bobMsgSize = bobData.messages.size();
bob2MsgSize = bob2Data.messages.size();
aliceMsgSize = aliceData.messages.size();
libjami::sendMessage(aliceId, aliceData.conversationId, "hi"s, "");
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobMsgSize != bobData.messages.size() && bob2MsgSize != bob2Data.messages.size(); }));
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize != aliceData.messages.size(); }));
msgId = aliceData.messages.rbegin()->id;
CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return getMessage(bobData, msgId) && getMessage(bob2Data, msgId); }));
}
void