Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

the trait Clone is not implemented for DatabaseConnection #836

Closed
RogueJin opened this issue Jul 4, 2022 · 15 comments
Closed

the trait Clone is not implemented for DatabaseConnection #836

RogueJin opened this issue Jul 4, 2022 · 15 comments

Comments

@RogueJin
Copy link

RogueJin commented Jul 4, 2022

I got the below error when I set a filed as DatabaseConnection type, but such usage works in rocket example, so is there anything I missed?

the trait bound DatabaseConnection: Clone is not satisfied the trait Clone is not implemented for DatabaseConnection

#[derive(Clone)]
pub struct DbStruct {
    db_conn: DatabaseConnection
}
[dependencies]
sea-orm = { version ="^0.8.0", features=["sqlx-postgres", "sqlx-sqlite","runtime-tokio-rustls"]}
@billy1624
Copy link
Member

Hey @RogueJin, did you enabled sea-orm's mock feature somewhere in the workspace?

@RogueJin
Copy link
Author

RogueJin commented Jul 4, 2022

@billy1624, mock is referenced in another project for unit test purpose.

The compilation passed once removing the mock feature, but I have another question, how can I do mock stuffs without the feature?

@billy1624
Copy link
Member

I would suggest you to split the two into two different crate with each under different workspace. Then, you can have one crate with mock enabled and the other without it. Does it make sense?

The proposed solution:

@RogueJin
Copy link
Author

RogueJin commented Jul 4, 2022

Thanks @billy1624 , I have a last question, could I have any docs on the reason that DatabaseConnection shouldn't have clone under mock feature? I am just curious about it.

@billy1624
Copy link
Member

Okay, here is the context for anyone interested in this:

@RogueJin RogueJin closed this as completed Jul 4, 2022
@RogueJin
Copy link
Author

RogueJin commented Jul 4, 2022

if it is possible, I would suggest to separate the mock feature since mocking DB operations in unit tests is the major reason to choose sea orm.

@billy1624
Copy link
Member

Hey @RogueJin, what do you mean by separate the mock feature out? In what way?

@RogueJin
Copy link
Author

Hey @RogueJin, what do you mean by separate the mock feature out? In what way?

struggled again as your suggestion, but sqlite becomes more handy rather than the mock.

I copy the code of DatabaseConnection as below, as we can see, the clone attr will be disabled when the mock feature includes. So is that possible to separate MockDatabaseConnection from DatabaseConnection Enum? something like a new DatabaseAdapter to accept normal database connection and mock database connection.

#[cfg_attr(not(feature = "mock"), derive(Clone))]
pub enum DatabaseConnection {
    /// Create a MYSQL database connection and pool
    #[cfg(feature = "sqlx-mysql")]
    SqlxMySqlPoolConnection(crate::SqlxMySqlPoolConnection),
    /// Create a  PostgreSQL database connection and pool
    #[cfg(feature = "sqlx-postgres")]
    SqlxPostgresPoolConnection(crate::SqlxPostgresPoolConnection),
    /// Create a  SQLite database connection and pool
    #[cfg(feature = "sqlx-sqlite")]
    SqlxSqlitePoolConnection(crate::SqlxSqlitePoolConnection),
    /// Create a  Mock database connection useful for testing
    #[cfg(feature = "mock")]
    MockDatabaseConnection(Arc<crate::MockDatabaseConnection>),
    /// The connection to the database has been severed
    Disconnected,
}

@billy1624
Copy link
Member

The idea is to have a concrete type either enum or struct to represent a universal connection. Where it can be used on both production and testing. So, separate MockDatabaseConnection from DatabaseConnection isn't ideal.

@baoyachi
Copy link
Contributor

baoyachi commented Aug 24, 2022

In the actual production environment,it's conflict about MockDatabaseConnection not clone, but DatabaseConnection could clone . #866

@RogueJin
Copy link
Author

The idea is to have a concrete type either enum or struct to represent a universal connection. Where it can be used on both production and testing. So, separate MockDatabaseConnection from DatabaseConnection isn't ideal.

To my knowledge, mock stuffs only for testing purpose in reality. Anyway, it is just my suggestion and there might be a better way to mock things.

@RogueJin
Copy link
Author

In the actual production environment,it's conflict about MockDatabaseConnection not clone, but DatabaseConnection could clone . #866

yes, that's why I would suggestion to separate them.

@billy1624 billy1624 moved this to Triage in SeaQL Dev Tracker Sep 5, 2022
@billy1624 billy1624 moved this from Triage to In Progress in SeaQL Dev Tracker Sep 5, 2022
@fairking
Copy link

fairking commented Sep 19, 2022

In my case the test project can have mock connections and the real connections (sqlite in memory db), it depends whether it is BLL test or DAL test.

@billy1624
Copy link
Member

Personally, I think it's okay to derive Clone for DatabaseConnection. Having a shared MockDatabaseConnection across thread.

That's something user have to use with caution. When you have concurrent writes to a db. The result / task completion sequence is not deterministic.

It's a good mock for the real db. The behaviour is the same.

Thoughts? @tyt2y3

@billy1624 billy1624 moved this from In Progress to Done in SeaQL Dev Tracker Jan 13, 2023
@vnghia
Copy link

vnghia commented Jan 8, 2024

@billy1624 can we enable Clone and let the users use it with caution ? Because in my case, each test has their own mock database connection and each test only make one query to the database so I think I won't run into that problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

5 participants