AscendingCreations / AxumSession

Axum Session Management Libraries that use Sqlx
MIT License
142 stars 29 forks source link

Add support for other databases #23

Closed 89Q12 closed 2 years ago

89Q12 commented 2 years ago

This adds support for arbitrary Databases via a trait, the is available under the feature flag DatabaseTrait. But this can be changed maybe a better name would OtherDatabase or something like this. Internally the trait is shipped anyway since the default supported database implementations now rely on that trait but I thought it would make sense to hide the trait behind a feature flag.

What I didn't include is rewritten examples, since it requires some more changes as you can read below but I will rewrite them asap once its decided what to do. Also I'm sure the error implementation could be improved since its currently like this:

pub enum SessionError {
    .....
    #[error("Generic Database insert error")]
    GenericInsertError,
    #[error("Generic Database select error")]
    GenericSelectError,
    #[error("Generic Database create error")]
    GenericCreateError,
    #[error("Generic Database delete error")]
    GenericDeleteError,
}

The only problem that would require a workaround or restructuring of the underlying functions is that AxumSessionStore<T> takes a generic parameter T which conflicts with using AxumSessionStore::new(None, config);, since the compiler cannot infer type for type parameter T. A fix would be to implement the trait for the in memory storage which would in turn remove the need for the Option<T> type for the client parameter that AxumSessionStore::new() takes.

This wouldn't introduce that much changes for the users since they need to import the Axum*Pool for the respective database anyways, since its not enforced that a trait implementation also implements Into<T> and pass into the new function. So we could just easily make a AxumInMemoryPool struct that implements the current in memory storage functions. That would also come with the benefits of removing some logic, for checking if in memory should be used or not, since we can use a few functions to use any storage/database backend.

genusistimelord commented 2 years ago

The only problem that would require a workaround or restructuring of the underlying functions is that AxumSessionStore<T> takes a generic parameter T which conflicts with using AxumSessionStore::new(None, config);, since the compiler cannot infer type for type parameter T. A fix would be to implement the trait for the in memory storage which would in turn remove the need for the Option<T> type for the client parameter that AxumSessionStore::new() takes.

We should be able to work around this while keeping the possibility to store in memory as memory storage should Always come first before any other type of storage. This is a buffer zone as it saves us from massively requesting data from Databases for no reason. Also the T should be infer-able even within a Option. We will just need to work this out.

This wouldn't introduce that much changes for the users since they need to import the Axum*Pool for the respective database anyways, since its not enforced that a trait implementation also implements Into<T> and pass into the new function.

Yeah there is no need to enforce Into since the Struct would be built by the end user for their Respective database anyways. the only Time we would Request Into is if they want to implement their Database type into the library itself.

So we could just easily make a AxumInMemoryPool struct that implements the current in memory storage functions. That would also come with the benefits of removing some logic, for checking if in memory should be used or not, since we can use a few functions to use any storage/database backend.

I want memory to always exist as this saves from SQL load calls and speeds up the ability to reply to Requests. If we remove this then the library itself will be much slower and Require the Memory load functions to also be implemented at the Database side within the internal functions by the End users or by us which means more code.

89Q12 commented 2 years ago

I want memory to always exist as this saves from SQL load calls and speeds up the ability to reply to Requests. If we remove this then the library itself will be much slower and Require the Memory load functions to also be implemented at the Database side within the internal functions by the End users or by us which means more code.

Ah okay then my bad for miss understanding the reason behind the in memory store^^ and thanks for the feedback