refactor: Update public key column type from String to PublicKey #29

Manually merged
awiteb merged 6 commits from awiteb/refactor-entities-public-key into master 2024-07-24 00:20:10 +02:00 AGit
19 changed files with 124 additions and 38 deletions

2
Cargo.lock generated
View file

@ -1954,6 +1954,7 @@ dependencies = [
"k256",
"rand",
"salvo-oapi",
"sea-orm",
"serde",
"sha2",
"thiserror",
@ -1964,6 +1965,7 @@ name = "oxidetalis_entities"
version = "0.1.0"
dependencies = [
"chrono",
"oxidetalis_core",
"sea-orm",
]

View file

@ -42,7 +42,7 @@ impl InChatRequestsExt for DatabaseConnection {
) -> ServerResult<()> {
InChatRequestsEntity::insert(InChatRequestsActiveModel {
recipient_id: Set(recipient.id),
sender: Set(sender.to_string()),
sender: Set(*sender),
in_on: Set(Utc::now()),
..Default::default()
})

View file

@ -57,7 +57,7 @@ impl OutChatRequestsExt for DatabaseConnection {
) -> ServerResult<Option<OutChatRequestsModel>> {
requester
.find_related(OutChatRequestsEntity)
.filter(OutChatRequestsColumn::Recipient.eq(recipient.to_string()))
.filter(OutChatRequestsColumn::Recipient.eq(recipient))
.one(self)
.await
.map_err(Into::into)
@ -71,7 +71,7 @@ impl OutChatRequestsExt for DatabaseConnection {
) -> ServerResult<()> {
if let Err(err) = (OutChatRequestsActiveModel {
sender_id: Set(requester.id),
recipient: Set(recipient.to_string()),
recipient: Set(*recipient),
out_on: Set(Utc::now()),
..Default::default()
}

View file

@ -45,7 +45,7 @@ impl UserTableExt for DatabaseConnection {
#[logcall]
async fn register_user(&self, public_key: &PublicKey, is_admin: bool) -> ServerResult<()> {
if let Err(err) = (UserActiveModel {
public_key: Set(public_key.to_string()),
public_key: Set(*public_key),
is_admin: Set(is_admin),
..Default::default()
})
@ -63,7 +63,7 @@ impl UserTableExt for DatabaseConnection {
#[logcall]
async fn get_user_by_pubk(&self, public_key: &PublicKey) -> ServerResult<Option<UserModel>> {
UserEntity::find()
.filter(UserColumn::PublicKey.eq(public_key.to_string()))
.filter(UserColumn::PublicKey.eq(public_key))
.one(self)
.await
.map_err(Into::into)

View file

@ -138,7 +138,7 @@ impl UsersStatusExt for DatabaseConnection {
whitelister: &UserModel,
target_public_key: &PublicKey,
) -> ServerResult<()> {
if whitelister.public_key == target_public_key.to_string() {
if &whitelister.public_key == target_public_key {
return Err(WsError::CannotAddSelfToWhitelist.into());
}
@ -156,7 +156,7 @@ impl UsersStatusExt for DatabaseConnection {
user.update(self).await?;
} else if let Err(err) = (UsersStatusActiveModel {
user_id: Set(whitelister.id),
target: Set(target_public_key.to_string()),
target: Set(*target_public_key),
status: Set(AccessStatus::Whitelisted),
updated_at: Set(Utc::now()),
..Default::default()
@ -181,7 +181,7 @@ impl UsersStatusExt for DatabaseConnection {
blacklister: &UserModel,
target_public_key: &PublicKey,
) -> ServerResult<()> {
if blacklister.public_key == target_public_key.to_string() {
if &blacklister.public_key == target_public_key {
return Err(WsError::CannotAddSelfToBlacklist.into());
}
@ -199,7 +199,7 @@ impl UsersStatusExt for DatabaseConnection {
user.update(self).await?;
} else if let Err(err) = (UsersStatusActiveModel {
user_id: Set(blacklister.id),
target: Set(target_public_key.to_string()),
target: Set(*target_public_key),
status: Set(AccessStatus::Blacklisted),
updated_at: Set(Utc::now()),
..Default::default()
@ -297,7 +297,7 @@ async fn get_user_status(
user.find_related(UsersStatusEntity)
.filter(
UsersStatusColumn::Target
.eq(target_public_key.to_string())
.eq(target_public_key)
.and(UsersStatusColumn::Status.eq(status)),
)
.one(conn)

View file

@ -54,7 +54,7 @@ impl Default for WhiteListedUser {
impl From<UsersStatusModel> for WhiteListedUser {
fn from(user: UsersStatusModel) -> Self {
Self {
public_key: PublicKey::from_str(&user.target).expect("Is valid public key"),
public_key: user.target,
whitelisted_at: user.updated_at,
}
}
@ -72,7 +72,7 @@ impl Default for BlackListedUser {
impl From<UsersStatusModel> for BlackListedUser {
fn from(user: UsersStatusModel) -> Self {
Self {
public_key: PublicKey::from_str(&user.target).expect("Is valid public key"),
public_key: user.target,
blacklisted_at: user.updated_at,
}
}

View file

@ -16,8 +16,6 @@
//! Handler for incoming and outgoing chat requests.
use std::str::FromStr;
use oxidetalis_core::types::PublicKey;
use oxidetalis_entities::prelude::*;
use sea_orm::DatabaseConnection;
@ -47,14 +45,12 @@ pub async fn handle_chat_request(
if from_user.id == to_user.id {
return Some(WsError::CannotSendChatRequestToSelf.into());
}
// FIXME: When change the entity public key to a PublicKey type, change this
let from_public_key = PublicKey::from_str(&from_user.public_key).expect("Is valid public key");
if try_ws!(Some db.get_chat_request_to(from_user, to_public_key).await).is_some() {
return Some(WsError::AlreadySendChatRequest.into());
}
if try_ws!(Some db.is_blacklisted(&to_user, &from_public_key).await) {
if try_ws!(Some db.is_blacklisted(&to_user, &from_user.public_key).await) {
return Some(WsError::RecipientBlacklist.into());
}
@ -64,17 +60,17 @@ pub async fn handle_chat_request(
return Some(WsError::InternalServerError.into());
}
if try_ws!(Some db.is_whitelisted(&to_user, &from_public_key).await) {
if try_ws!(Some db.is_whitelisted(&to_user, &from_user.public_key).await) {
return Some(WsError::AlreadyInRecipientWhitelist.into());
}
try_ws!(Some db.save_out_chat_request(from_user, to_public_key).await);
if let Some(conn_id) = ONLINE_USERS.is_online(to_public_key).await {
ONLINE_USERS
.send(&conn_id, ServerEvent::chat_request(&from_public_key))
.send(&conn_id, ServerEvent::chat_request(&from_user.public_key))
.await;
} else {
try_ws!(Some db.save_in_chat_request(&from_public_key, &to_user).await);
try_ws!(Some db.save_in_chat_request(&from_user.public_key, &to_user).await);
}
None
}
@ -96,12 +92,8 @@ pub async fn handle_chat_response(
return Some(WsError::CannotRespondToOwnChatRequest.into());
}
// FIXME: When change the entity public key to a PublicKey type, change this
let recipient_public_key =
PublicKey::from_str(&recipient_user.public_key).expect("Is valid public key");
if try_ws!(Some
db.get_chat_request_to(&sender_user, &recipient_public_key)
db.get_chat_request_to(&sender_user, &recipient_user.public_key)
.await
)
.is_none()
@ -118,7 +110,7 @@ pub async fn handle_chat_response(
};
try_ws!(Some
db.remove_out_chat_request(&sender_user, &recipient_public_key)
db.remove_out_chat_request(&sender_user, &recipient_user.public_key)
.await
);
@ -126,7 +118,7 @@ pub async fn handle_chat_response(
ONLINE_USERS
.send(
&conn_id,
ServerEvent::chat_request_response(recipient_public_key, accepted),
ServerEvent::chat_request_response(recipient_user.public_key, accepted),
)
.await;
} else {

View file

@ -15,6 +15,7 @@ base58 = { workspace = true }
thiserror = { workspace = true }
salvo-oapi = { workspace = true, optional = true }
serde = { workspace = true, optional = true }
sea-orm = { workspace = true, optional = true }
cbc = { version = "0.1.2", features = ["alloc", "std"] }
k256 = { version = "0.13.3", default-features = false, features = ["ecdh"] }
rand = { version = "0.8.5", default-features = false, features = ["std_rng", "std"] }
@ -26,6 +27,7 @@ sha2 = "0.10.8"
[features]
openapi = ["dep:salvo-oapi"]
serde = ["dep:serde"]
sea-orm = ["dep:sea-orm"]
[lints.rust]

View file

@ -0,0 +1,84 @@
// OxideTalis Messaging Protocol homeserver core implementation
// Copyright (C) 2024 Awiteb <a@4rs.nl>, OxideTalis Contributors
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.
//! Implemented SeaORM support for core types, enabling the use of these types
//! as column types in SeaORM
use sea_orm::{
sea_query::{ArrayType, BlobSize, ValueType, ValueTypeErr},
ColumnType,
DbErr,
QueryResult,
TryGetError,
TryGetable,
Value,
};
use super::PublicKey;
impl From<PublicKey> for Value {
fn from(public_key: PublicKey) -> Self {
public_key.as_bytes().as_slice().into()
}
}
awiteb marked this conversation as resolved
Review

Wouldn't it be better to use Bytes type? easier to convert as its just copying bytes and making sure its the length expected and maybe more efficient

Wouldn't it be better to use `Bytes` type? easier to convert as its just copying bytes and making sure its the length expected and maybe more efficient
Review

Wouldn't it be better to use Bytes type?

Yes I think so, it is actually a good idea.

> Wouldn't it be better to use Bytes type? Yes I think so, it is actually a good idea.
impl From<&PublicKey> for Value {
fn from(public_key: &PublicKey) -> Self {
public_key.as_bytes().as_slice().into()
}
}
awiteb marked this conversation as resolved
Review

replace map by and_then and you won't need and_then(|res| res)

<String as TryGetable>::try_get_by(res, idx).and_then(|v| {
    PublicKey::from_str(&v).map_err(|err| TryGetError::DbErr(DbErr::Type(err.to_string())))
})
replace `map` by `and_then` and you won't need `and_then(|res| res)` ```rust <String as TryGetable>::try_get_by(res, idx).and_then(|v| { PublicKey::from_str(&v).map_err(|err| TryGetError::DbErr(DbErr::Type(err.to_string()))) }) ```
impl TryGetable for PublicKey {
fn try_get_by<I: sea_orm::ColIdx>(res: &QueryResult, idx: I) -> Result<Self, TryGetError> {
let db_err = |err: &str| TryGetError::DbErr(DbErr::Type(err.to_owned()));
<Vec<u8> as TryGetable>::try_get_by(res, idx).and_then(|v| {
v.try_into()
.map_err(|_| db_err("Invalid binary length"))
.and_then(|bytes| {
<PublicKey as TryFrom<[u8; 33]>>::try_from(bytes)
.map_err(|_| db_err("Invalid Public Key"))
})
awiteb marked this conversation as resolved
Review

same as above, and_then instead of map

same as above, `and_then` instead of `map`
})
}
}
impl ValueType for PublicKey {
fn try_from(v: Value) -> Result<Self, ValueTypeErr> {
<Vec<u8> as ValueType>::try_from(v).and_then(|v| {
v.try_into().map_err(|_| ValueTypeErr).and_then(|bytes| {
<PublicKey as TryFrom<[u8; 33]>>::try_from(bytes).map_err(|_| ValueTypeErr)
})
})
}
fn type_name() -> String {
String::from("PublicKey")
}
fn array_type() -> ArrayType {
ArrayType::Bytes
}
fn column_type() -> ColumnType {
ColumnType::Binary(BlobSize::Blob(None))
}
}

View file

@ -22,6 +22,8 @@
//! Oxidetalis server types
mod cipher;
#[cfg(feature = "sea-orm")]
mod impl_sea_orm;
#[cfg(feature = "serde")]
mod impl_serde;
mod size;

View file

@ -11,6 +11,7 @@ rust-version.workspace = true
[dependencies]
oxidetalis_core = { workspace = true, features = ["sea-orm"] }
sea-orm = { workspace = true }
chrono = { workspace = true }

View file

@ -22,6 +22,7 @@
//! Entity for `in_chat_requests` table
use chrono::Utc;
use oxidetalis_core::types::PublicKey;
Amjad50 marked this conversation as resolved
Review

Why is it here CorePublicKey? while in oxidetails crate is just PublicKey, I think it might be confusing as we would think its 2 types.

Why is it here `CorePublicKey`? while in `oxidetails` crate is just `PublicKey`, I think it might be confusing as we would think its 2 types.
Review

while in oxidetails crate is just PublicKey

Because oxidetalis only work with one PublicKey from the core, while
the core work with two PublicKey, the first one from k256 crate and
the second one it its own.

> while in `oxidetails` crate is just `PublicKey` Because `oxidetalis` only work with one `PublicKey` from the core, while the core work with two `PublicKey`, the first one from `k256` crate and the second one it its own.
Review

Ah right, didn't notice that

Ah right, didn't notice that
Review

Amjad50: Why is it here CorePublicKey? while in oxidetails crate is just PublicKey

You're right, I was mean if we use it in the core not entities.

awiteb: while the core work with two PublicKey

I didn't realize that was in entities.

> Amjad50: Why is it here CorePublicKey? while in oxidetails crate is just PublicKey You're right, I was mean if we use it in the core not entities. > awiteb: while the core work with two PublicKey I didn't realize that was in entities.
use sea_orm::entity::prelude::*;
use crate::prelude::*;
@ -33,7 +34,7 @@ pub struct Model {
pub id: UserId,
pub recipient_id: UserId,
/// Public key of the sender
pub sender: String,
pub sender: PublicKey,
/// The timestamp of the request, when it was received
pub in_on: chrono::DateTime<Utc>,
}

View file

@ -22,6 +22,7 @@
//! Entity for `out_chat_requests` table
use chrono::Utc;
use oxidetalis_core::types::PublicKey;
use sea_orm::entity::prelude::*;
use crate::prelude::*;
@ -33,7 +34,7 @@ pub struct Model {
pub id: UserId,
pub sender_id: UserId,
/// Public key of the recipient
pub recipient: String,
pub recipient: PublicKey,
/// The timestamp of the request, when it was sent
pub out_on: chrono::DateTime<Utc>,
}

View file

@ -21,6 +21,7 @@
//! Entity for `users` table
use oxidetalis_core::types::PublicKey;
use sea_orm::entity::prelude::*;
use crate::prelude::*;
@ -30,7 +31,7 @@ use crate::prelude::*;
pub struct Model {
#[sea_orm(primary_key)]
pub id: UserId,
pub public_key: String,
pub public_key: PublicKey,
pub is_admin: bool,
}

View file

@ -22,6 +22,7 @@
//! Entity for `users_status` table
use chrono::Utc;
use oxidetalis_core::types::PublicKey;
use sea_orm::entity::prelude::*;
use crate::prelude::*;
@ -41,8 +42,7 @@ pub struct Model {
#[sea_orm(primary_key)]
pub id: UserId,
pub user_id: UserId,
/// Public key of the target
pub target: String,
pub target: PublicKey,
pub status: AccessStatus,
pub updated_at: chrono::DateTime<Utc>,
}

View file

@ -57,7 +57,7 @@ impl MigrationTrait for Migration {
.on_update(ForeignKeyAction::NoAction)
.on_delete(ForeignKeyAction::Cascade),
)
.col(ColumnDef::new(InChatRequests::Sender).string().not_null())
.col(ColumnDef::new(InChatRequests::Sender).binary().not_null())
.col(
ColumnDef::new(InChatRequests::InOn)
.timestamp_with_time_zone()

View file

@ -59,7 +59,7 @@ impl MigrationTrait for Migration {
)
.col(
ColumnDef::new(OutChatRequests::Recipient)
.string()
.binary()
.not_null(),
)
.col(

View file

@ -65,7 +65,7 @@ impl MigrationTrait for Migration {
.on_update(ForeignKeyAction::NoAction)
.on_delete(ForeignKeyAction::Cascade),
)
.col(ColumnDef::new(UsersStatus::Target).string().not_null())
.col(ColumnDef::new(UsersStatus::Target).binary().not_null())
.col(
ColumnDef::new(UsersStatus::Status)
.enumeration(

View file

@ -43,7 +43,7 @@ impl MigrationTrait for Migration {
)
.col(
ColumnDef::new(Users::PublicKey)
.string()
.binary()
.not_null()
.unique_key(),
)