Features: iq::Bind and iq::Roster #4

Merged
sha512sum merged 3 commits from feature_roster_on_login into main 2024-11-26 15:25:56 +00:00
Member
No description provided.
Ivan-lis added 2 commits 2024-11-17 23:34:48 +00:00
Feature: iq::Roster
Some checks failed
PR Check / on-push-commit-check (push) Failing after 11m10s
7685385559
Author
Member

image

![image](/attachments/c1868912-d42b-4573-9799-5b440ee3b255)
sha512sum requested changes 2024-11-18 07:10:28 +00:00
Dismissed
@ -0,0 +29,4 @@
return {};
}
auto* jid_el = dynamic_cast<const xmlpp::Element*>(jid_node);
Owner

You can create a structure that contains jid as an attribute and use automatic generation of serialization and deserialization

You can create a structure that contains jid as an attribute and use automatic generation of serialization and deserialization
Author
Member

I tried, but roster result should has 'jid' attr, while with Serialization with BareJid I got <NO_NAME username="n" server="s" >

Expected ResultRoster

<iq id='bv1bs71f'
       to='juliet@example.com/chamber'
       type='result'>
    <query xmlns='jabber:iq:roster' ver='ver7'>
      <item jid='nurse@example.com'/>
      <item jid='romeo@example.net'/>
    </query>
  </iq>
I tried, but roster result should has **'jid' attr,** while with Serialization with BareJid I got `<NO_NAME username="n" server="s" >` Expected ResultRoster ```json <iq id='bv1bs71f' to='juliet@example.com/chamber' type='result'> <query xmlns='jabber:iq:roster' ver='ver7'> <item jid='nurse@example.com'/> <item jid='romeo@example.net'/> </query> </iq> ```
Owner

I tried, but roster result should has 'jid' attr, while with Serialization with BareJid I got <NO_NAME username="n" server="s" >

Expected ResultRoster

<iq id='bv1bs71f'
       to='juliet@example.com/chamber'
       type='result'>
    <query xmlns='jabber:iq:roster' ver='ver7'>
      <item jid='nurse@example.com'/>
      <item jid='romeo@example.net'/>
    </query>
  </iq>

See how it's done in SomeStruct5 in tests/serialization.cpp

> I tried, but roster result should has **'jid' attr,** while with Serialization with BareJid I got `<NO_NAME username="n" server="s" >` > > Expected ResultRoster > ```json > <iq id='bv1bs71f' > to='juliet@example.com/chamber' > type='result'> > <query xmlns='jabber:iq:roster' ver='ver7'> > <item jid='nurse@example.com'/> > <item jid='romeo@example.net'/> > </query> > </iq> > ``` > > See how it's done in SomeStruct5 in tests/serialization.cpp
Author
Member

Interesting, looks like last time I did something wrong and got <username="n" server="s"> instead. Will try t use BareJid

Interesting, looks like last time I did something wrong and got <username="n" server="s"> instead. Will try t use BareJid
Author
Member

Done

Done
Ivan-lis marked this conversation as resolved
@ -0,0 +48,4 @@
using IqBind = Iq<Bind>;
inline auto MakeSetBind() {
return SetBind{.id = "1", .payload = Bind{}};
Owner

For what?

For what?
Ivan-lis marked this conversation as resolved
@ -126,2 +146,2 @@
}) //
| std::ranges::to<std::unordered_map<std::string_view, std::string_view>>();
auto params = std::views::split(decoded, ',') | std::views::transform([](auto param) {
return std::string_view{param};
Owner

Why? It was more readable before.

Why? It was more readable before.
Ivan-lis marked this conversation as resolved
@ -72,0 +55,4 @@
std::string username;
std::string server;
constexpr operator FullJid() const {
return FullJid{.username = username, .server = server, .resource = ""};
Owner

Extra copying if there is an rvalue. Better to forward depending on the type with which it is called.

Extra copying if there is an rvalue. Better to forward depending on the type with which it is called.
Ivan-lis marked this conversation as resolved
@ -0,0 +38,4 @@
}
friend constexpr auto operator<<(xmlpp::Element* element, const Roster& roster) {
element->set_attribute("xmlns", Roster::kDefaultNamespace);
std::ranges::for_each(roster.items, [element](const auto& item) {
Owner

Why not range based for?

Why not range based for?
sha512sum marked this conversation as resolved
@ -0,0 +51,4 @@
return {.items = item_nodes | std::views::transform([](const xmlpp::Node* node) {
auto item_element = dynamic_cast<const xmlpp::Element*>(node);
if(!item_element) {
throw std::runtime_error("Can't convert xmlpp::Node to xmlpp::Element");
Owner

You can create a structure that contains jid as an attribute and use automatic generation of serialization and deserialization

You can create a structure that contains jid as an attribute and use automatic generation of serialization and deserialization
Author
Member

Done

Done
Ivan-lis marked this conversation as resolved
@ -0,0 +70,4 @@
using IqRoster = Iq<Roster>;
inline auto MakeGetRoster(const FullJid& jid) {
return GetRoster{.id = "1", .from = ToString(jid), .payload = Roster{}};
Owner

For what?

For what?
Ivan-lis marked this conversation as resolved
@ -297,1 +297,4 @@
template <typename T>
concept LengthCalculatable = requires(const T& obj) {
{ obj.length() } -> std::convertible_to<std::size_t>; // Checks if obj has a length() method returning a type convertible to std::size_t
Owner

A comment that doesn't explain anything and just repeats the code.

A comment that doesn't explain anything and just repeats the code.
sha512sum marked this conversation as resolved
tests/roster.cpp Outdated
@ -0,0 +10,4 @@
FullJid jid{.username = "test", .server = "server", .resource = "res"}; // NOLINT
auto roster = iq::MakeGetRoster(jid);
roster.payload.items.emplace_back("u1", "s1");
roster.payload.items.emplace_back("u2", "s2");
Owner

Why not just throw all the necessary elements through the vector constructor in Roster constructor?

Why not just throw all the necessary elements through the vector constructor in Roster constructor?
Ivan-lis marked this conversation as resolved
tests/roster.cpp Outdated
@ -0,0 +22,4 @@
ASSERT_EQ(roster.payload.items.size(), parse_res.payload.items.size());
for(const auto& [idx, expect_el, parsed_el] : std::views::zip(std::views::iota(0), roster.payload.items, parse_res.payload.items)) {
EXPECT_EQ(expect_el, parsed_el) << "Mismatched on idx: " << idx;
// std::cerr << " " << "idx: " << idx << "; expect_el: " << expect_el << "; parsed_el: " << parsed_el << '\n';
Owner

Code left in the comment

Code left in the comment
Ivan-lis marked this conversation as resolved
tests/roster.cpp Outdated
@ -0,0 +33,4 @@
roster.payload.items.emplace_back("u2", "s2");
roster.payload.items.emplace_back("u3", "s3");
EXPECT_NO_THROW({ std::cerr << "[ ] Roster payload: " << ToString(roster.payload) << '\n'; });
Owner

EXPECT_EQ and test content

EXPECT_EQ and test content
Ivan-lis marked this conversation as resolved
Ivan-lis added 1 commit 2024-11-18 23:06:10 +00:00
Fixed errors and improve roster tests
All checks were successful
PR Check / on-push-commit-check (push) Successful in 12m15s
54b3535ebc
sha512sum requested changes 2024-11-19 13:33:46 +00:00
Dismissed
@ -11,6 +12,8 @@ namespace iq {
template <auto& Name, typename PayloadType>
struct BaseImplWithPayload {
std::string id;
std::optional<std::string> from{};
Owner

Why std::string and not a more limited type like Jid ?

Why std::string and not a more limited type like Jid ?
Author
Member

Fixed

Fixed
Ivan-lis marked this conversation as resolved
Ivan-lis force-pushed feature_roster_on_login from 54b3535ebc to d88f14a66d 2024-11-21 22:34:18 +00:00 Compare
Ivan-lis reviewed 2024-11-21 22:36:40 +00:00
@ -75,0 +95,4 @@
co_return;
}
auto UpdateListOfContacts() -> boost::asio::awaitable<void> {
Author
Member

Maybe we can change namings later

Maybe we can change namings later
Ivan-lis force-pushed feature_roster_on_login from d88f14a66d to 1a9b293dee 2024-11-21 22:48:18 +00:00 Compare
Ivan-lis force-pushed feature_roster_on_login from 1a9b293dee to 604b423d1c 2024-11-21 23:15:44 +00:00 Compare
Ivan-lis force-pushed feature_roster_on_login from 604b423d1c to 30a5e69d14 2024-11-21 23:22:15 +00:00 Compare
Ivan-lis added 1 commit 2024-11-22 23:15:51 +00:00
Added auto serialization for Bind and Roster
Some checks failed
PR Check / on-push-commit-check (push) Failing after 11m51s
fbfd37e082
Author
Member

image

![image](/attachments/8cb9144e-1e5f-4df6-9687-d710a4b7fd92)
116 KiB
Ivan-lis force-pushed feature_roster_on_login from fbfd37e082 to 9b25ed1620 2024-11-22 23:42:25 +00:00 Compare
Ivan-lis force-pushed feature_roster_on_login from 9b25ed1620 to 495b58489e 2024-11-23 08:06:59 +00:00 Compare
sha512sum requested changes 2024-11-23 16:55:52 +00:00
Dismissed
@ -20,2 +24,4 @@
return utils::FieldSetHelper::With<"id", BaseImplWithPayload>(std::forward<Self>(self), std::move(id));
}
template <typename Self>
[[nodiscard]] constexpr auto To(this Self&& self, std::string to) -> std::decay_t<Self> {
Owner

Uses std::string for setters even though another type is stored.

Uses std::string for setters even though another type is stored.
Ivan-lis marked this conversation as resolved
@ -53,2 +88,3 @@
}
return {.id = idNode->get_value(), .payload = S::Parse(payload2)};
return {.id = idNode->get_value(),
.from = (from ? std::optional{BareJid::Parse(from->get_value())} : std::nullopt),
Owner

Use BareJid::Parse when there might not be a BareJid

Use BareJid::Parse when there might not be a BareJid
Ivan-lis marked this conversation as resolved
@ -0,0 +32,4 @@
static constexpr std::string_view prefix = "Roster: [\n\t";
static constexpr std::string_view suffix = "]";
// \n\r\t
std::size_t total_length = std::ranges::fold_left(roster.items | std::views::transform([](const auto& el) {
Owner

It would be better to move this to a .cpp file to make the header files lighter.

It would be better to move this to a .cpp file to make the header files lighter.
Ivan-lis marked this conversation as resolved
@ -0,0 +73,4 @@
S::Serialize(element, self);
}
constexpr auto RosterItem::Parse(xmlpp::Element* element) -> RosterItem {
return S::Parse<RosterItem>(element);
Owner

It would be better to move definitions to a .cpp file to make the header files lighter.

It would be better to move definitions to a .cpp file to make the header files lighter.
sha512sum marked this conversation as resolved
@ -298,0 +302,4 @@
template <typename T>
auto AccumulateFieldLength(const T& obj) -> std::size_t {
std::size_t total_length = 0;
Owner

camelCase

camelCase
sha512sum marked this conversation as resolved
tests/roster.cpp Outdated
@ -0,0 +16,4 @@
auto parse_res = decltype(roster)::Parse(node);
ASSERT_EQ(roster.payload.items.size(), parse_res.payload.items.size());
for(const auto& [idx, expect_el, parsed_el] : std::views::zip(std::views::iota(0), roster.payload.items, parse_res.payload.items)) {
Owner

camelCase

camelCase
sha512sum marked this conversation as resolved
Ivan-lis force-pushed feature_roster_on_login from 495b58489e to a4e973ca32 2024-11-24 14:24:57 +00:00 Compare
sha512sum requested changes 2024-11-24 14:25:45 +00:00
Dismissed
@ -7,1 +7,4 @@
#include <larra/stream_error.hpp>
#include <variant>
#include "utempl/utils.hpp"
Owner

????

????
Ivan-lis marked this conversation as resolved
Ivan-lis force-pushed feature_roster_on_login from a4e973ca32 to b6c31e8e85 2024-11-24 18:00:15 +00:00 Compare
sha512sum approved these changes 2024-11-25 10:30:44 +00:00
Ivan-lis force-pushed feature_roster_on_login from b6c31e8e85 to c52d237dc4 2024-11-25 13:25:18 +00:00 Compare
Ivan-lis added a new dependency 2024-11-25 13:33:14 +00:00
Ivan-lis removed a dependency 2024-11-25 13:33:22 +00:00
Ivan-lis added a new dependency 2024-11-25 13:33:30 +00:00
Ivan-lis removed a dependency 2024-11-25 13:33:33 +00:00
sha512sum merged commit cd76a71bb7 into main 2024-11-26 15:25:56 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Larra/larra#4
No description provided.